src/authentic2/forms/profile.py

@@ -22,7 +22,7 @@ from django.utils.translation import gettext_lazy as _
 from authentic2 import app_settings, models
 from authentic2.custom_user.models import User
 
-from .fields import ValidatedEmailField
+from .fields import PhoneField, ValidatedEmailField
 from .mixins import LockedFieldFormMixin
 from .utils import NextUrlFormMixin
 
@@ -54,6 +54,41 @@ class EmailChangeForm(EmailChangeFormNoPassword):
         return password
 
 
+class PhoneChangeFormNoPassword(forms.Form):
+    phone = PhoneField(label=_('New phone number'))
+
+    def __init__(self, user, *args, **kwargs):
+        self.user = user
+        super().__init__(*args, **kwargs)
+
+
+class PhoneChangeForm(PhoneChangeFormNoPassword):
+    password = forms.CharField(label=_("Password"), widget=forms.PasswordInput)
+
+    def __init__(self, *args, **kwargs):
+        self.authenticator = kwargs.pop('password_authenticator')
+        super().__init__(*args, **kwargs)
+
+    def clean_phone(self):
+        phone = self.cleaned_data['phone']
+        if (
+            models.AttributeValue.objects.with_owner(self.user)
+            .filter(attribute=self.authenticator.phone_identifier_field, content=phone)
+            .exists()
+        ):
+            raise forms.ValidationError(_('This is already your phone.'))
+        return phone
+
+    def clean_password(self):
+        password = self.cleaned_data["password"]
+        if not self.user.check_password(password):
+            raise forms.ValidationError(
+                _('Incorrect password.'),
+                code='password_incorrect',
+            )
+        return password
+
+
 class BaseUserForm(LockedFieldFormMixin, forms.ModelForm):
     error_messages = {
         'duplicate_username': _("A user with that username already exists."),

src/authentic2/journal_event_types.py

@@ -426,6 +426,43 @@ class UserEmailChange(EventTypeDefinition):
         return _('email address changed from "{0}" to "{1}"').format(old_email, new_email)
 
 
+class UserPhoneChangeRequest(EventTypeDefinition):
+    name = 'user.phone.change.request'
+    label = _('phone change request')
+
+    @classmethod
+    def record(cls, *, user, old_phone, session, new_phone):
+        data = {
+            'old_phone': old_phone,
+            'new_phone': new_phone,
+        }
+        super().record(user=user, session=session, data=data)
+
+    @classmethod
+    def get_message(cls, event, context):
+        new_phone = event.get_data('new_phone')
+        return _('phone change request to number "{0}"').format(new_phone)
+
+
+class UserPhoneChange(EventTypeDefinition):
+    name = 'user.phone.change'
+    label = _('phone change')
+
+    @classmethod
+    def record(cls, *, user, session, old_phone, new_phone):
+        data = {
+            'old_phone': old_phone,
+            'new_phone': new_phone,
+        }
+        super().record(user=user, session=session, data=data)
+
+    @classmethod
+    def get_message(cls, event, context):
+        new_phone = event.get_data('new_phone')
+        old_phone = event.get_data('old_phone')
+        return _('phone number changed from "{0}" to "{1}"').format(old_phone, new_phone)
+
+
 class UserProfileAdd(EventTypeDefinition):
     name = 'user.profile.add'
     label = _('user profile creation')

src/authentic2/models.py

@@ -815,9 +815,11 @@ class SMSCode(models.Model):
     CODE_DURATION = 120
     KIND_REGISTRATION = 'registration'
     KIND_PASSWORD_LOST = 'password-reset'
+    KIND_PHONE_CHANGE = 'phone-change'
     CODE_TO_TOKEN_KINDS = {
         KIND_REGISTRATION: 'registration',
         KIND_PASSWORD_LOST: 'pw-reset',
+        KIND_PHONE_CHANGE: 'phone-change',
     }
     value = models.CharField(
         verbose_name=_('Identifier'), default=create_sms_code, editable=False, max_length=32

src/authentic2/templates/authentic2/accounts.html

@@ -68,6 +68,9 @@
       {% if allow_email_change %}
         <p><a href="{% url 'email-change' %}">{% trans "Change email" %}</a></p>
       {% endif %}
+      {% if allow_phone_change %}
+        <p><a href="{% url 'phone-change' %}">{% if phone %}{% trans "Change phone" %}{% else %}{% trans "Declare your phone number" %}{% endif %}</a></p>
+      {% endif %}
       {% if allow_profile_edit %}
         <p><a href="{% url 'profile_edit' %}">{% trans "Edit account data" %}</a></p>
       {% endif %}

src/authentic2/templates/authentic2/change_phone.html

@@ -0,0 +1,30 @@
+{% extends "authentic2/base-page.html" %}
+{% load i18n gadjo %}
+
+{% block title %}
+  {{ view.title }}
+{% endblock %}
+
+{% block breadcrumb %}
+  {{ block.super }}
+  <a href="..">{% trans "Your account" %}</a>
+  <a href="">{{ view.title }}</a>
+{% endblock %}
+
+{% block content %}
+  {% if phone %}
+    <p>{% blocktrans trimmed %}Your current phone number is {{ phone }}.
+      A text message will be sent to validate the new one.{% endblocktrans %}</p>
+  {% else %}
+    <p>{% blocktrans %}Your account does not declare a phone number yet.
+      A text message will be sent to validate the newly-declared one.{% endblocktrans %}</p>
+  {% endif %}
+  <form method="post" class="pk-mark-optional-fields">
+    {% csrf_token %}
+    {{ form|with_template }}
+    <div class="buttons">
+      <button class="submit-button">{% trans "Validate" %}</button>
+      <button class="cancel-button" name="cancel" formnovalidate>{% trans "Cancel" %}</button>
+    </div>
+  </form>
+{% endblock %}

src/authentic2/templates/phone_change/sms_code_phone_change.txt

@@ -0,0 +1 @@
+{% load i18n %}{% blocktrans trimmed with value=code.value %}Your code is {{ value }}{% endblocktrans %}

src/authentic2/urls.py

@@ -56,6 +56,12 @@ accounts_urlpatterns = [
     re_path(r'^edit/(?P<scope>[-\w]+)/$', views.edit_profile, name='profile_edit_with_scope'),
     path('change-email/', views.email_change, name='email-change'),
     path('change-email/verify/', views.email_change_verify, name='email-change-verify'),
+    path('change-phone/', views.phone_change, name='phone-change'),
+    re_path(
+        'change-phone/verify/(?P<token>[A-Za-z0-9_ -]+)/$',
+        views.phone_change_verify,
+        name='phone-change-verify',
+    ),
     path(
         'consents/',
         login_required(views.consents),

src/authentic2/utils/sms.py

@@ -133,3 +133,17 @@ def send_password_reset_sms(phone_number, ou, user=None, template_names=None, co
         kind=SMSCode.KIND_PASSWORD_LOST,
         **kwargs,
     )
+
+
+def send_phone_change_sms(phone_number, ou, user=None, template_names=None, context=None, **kwargs):
+    from authentic2.models import SMSCode
+
+    return send_sms(
+        phone_number,
+        ou,
+        user=user,
+        template_names=template_names or ['phone_change/sms_code_phone_change.txt'],
+        context=context,
+        kind=SMSCode.KIND_PHONE_CHANGE,
+        **kwargs,
+    )

src/authentic2/views.py

@@ -68,6 +68,7 @@ from .forms import registration as registration_forms
 from .models import Lock
 from .utils import crypto, hooks
 from .utils import misc as utils_misc
+from .utils import sms as utils_sms
 from .utils import switch_user as utils_switch_user
 from .utils.evaluate import make_condition_context
 from .utils.service import get_service, set_home_url
@@ -138,8 +139,12 @@ class EditProfile(HomeURLMixin, cbv.HookMixin, cbv.TemplateNamesMixin, UpdateVie
         else:
             scopes = self.request.GET.get('scope', '').split()
         fields, labels = self.get_fields(scopes=scopes)
-        # Email must be edited through the change email view, as it needs validation
-        fields = [field for field in fields if field != 'email']
+        authn = utils_misc.get_password_authenticator()
+        # Email and identifier phone must be edited through the change email view, as they need validation
+        not_directly_modifiable = ['email']
+        if name := getattr(authn.phone_identifier_field, 'name', None):
+            not_directly_modifiable.append(name)
+        fields = [field for field in fields if field not in not_directly_modifiable]
         return profile_forms.modelform_factory(
             User, fields=fields, labels=labels, form=profile_forms.EditProfileForm
         )
@@ -216,42 +221,49 @@ class RecentAuthenticationMixin:
         return age < self.last_authentication_max_age
 
 
-class EmailChangeView(HomeURLMixin, RecentAuthenticationMixin, cbv.TemplateNamesMixin, FormView):
-    template_names = ['profiles/email_change.html', 'authentic2/change_email.html']
-    title = _('Email Change')
-    success_url = '..'
+class IdentifierChangeMixin(RecentAuthenticationMixin):
+    reauthn_message = ''
+    action = ''
 
     def can_validate_with_password(self):
         last_event = utils_misc.last_authentication_event(self.request)
         return last_event and last_event['how'] == 'password-on-https'
 
-    def get_form_class(self):
-        if self.can_validate_with_password():
-            return profile_forms.EmailChangeForm
-        return profile_forms.EmailChangeFormNoPassword
+    def dispatch(self, request, *args, **kwargs):
+        if not self.can_validate_with_password() and not self.has_recent_authentication():
+            return self.reauthenticate(
+                action=self.action,
+                message=self.reauthn_message,
+            )
+        return super().dispatch(request, *args, **kwargs)
+
+    def has_recent_authentication(self):
+        age = time.time() - utils_misc.last_authentication_event(request=self.request)['when']
+        return age < self.last_authentication_max_age
 
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
         kwargs['user'] = self.request.user
         return kwargs
 
-    def has_recent_authentication(self):
-        age = time.time() - utils_misc.last_authentication_event(request=self.request)['when']
-        return age < self.last_authentication_max_age
-
-    def dispatch(self, request, *args, **kwargs):
-        if not self.can_validate_with_password() and not self.has_recent_authentication():
-            return self.reauthenticate(
-                action='email-change',
-                message=_('You must re-authenticate to change your email address.'),
-            )
-        return super().dispatch(request, *args, **kwargs)
-
     def post(self, request, *args, **kwargs):
         if 'cancel' in request.POST:
             return utils_misc.redirect(request, 'account_management')
         return super().post(request, *args, **kwargs)
 
+
+class EmailChangeView(HomeURLMixin, IdentifierChangeMixin, cbv.TemplateNamesMixin, FormView):
+    template_names = ['profiles/email_change.html', 'authentic2/change_email.html']
+    title = _('Email Change')
+    success_url = '..'
+    reauthn_message = (_('You must re-authenticate to change your email address.'),)
+    action = 'email-change'
+
+    def get_form_class(self):
+        if self.can_validate_with_password():
+            return profile_forms.EmailChangeForm
+        return profile_forms.EmailChangeFormNoPassword
+
     def form_valid(self, form):
         email = form.cleaned_data['email']
         utils_misc.send_email_change_email(self.request.user, email, request=self.request)
@@ -275,6 +287,202 @@ email_change = decorators.setting_enabled('A2_PROFILE_CAN_CHANGE_EMAIL')(
 )
 
 
+class PhoneChangeView(HomeURLMixin, IdentifierChangeMixin, cbv.TemplateNamesMixin, FormView):
+    template_name = 'authentic2/change_phone.html'
+    form_class = profile_forms.PhoneChangeForm
+    reauthn_message = _('You must re-authenticate to change your phone number.')
+    action = 'phone-change'
+
+    def dispatch(self, *args, **kwargs):
+        self.authenticator = utils_misc.get_password_authenticator()
+        if not (
+            self.authenticator.phone_identifier_field
+            and self.authenticator.phone_identifier_field.user_editable
+            and not self.authenticator.phone_identifier_field.disabled
+        ):
+            raise Http404(_('Phone change is not allowed.'))
+        return super().dispatch(*args, **kwargs)
+
+    def get_form_kwargs(self, **kwargs):
+        kwargs = super().get_form_kwargs(**kwargs)
+        kwargs['password_authenticator'] = self.authenticator
+        return kwargs
+
+    def get_context_data(self, **kwargs):
+        ctx = super().get_context_data(**kwargs)
+        user_ct = ContentType.objects.get_for_model(get_user_model())
+        if phones := models.AttributeValue.objects.filter(
+            attribute=self.authenticator.phone_identifier_field,
+            content_type=user_ct,
+            object_id=self.request.user.pk,
+            content__isnull=False,
+        ).values_list('content', flat=True):
+            ctx['phone'] = phones[0]
+        return ctx
+
+    def get_success_url(self):
+        params = {}
+        if next_url := getattr(self, 'next_url', None):
+            params[REDIRECT_FIELD_NAME] = next_url
+        return utils_misc.make_url('input_sms_code', kwargs={'token': self.code.url_token}, params=params)
+
+    def form_valid(self, form):
+        phone = form.cleaned_data.get('phone')
+        self.request.session['phone'] = phone
+
+        # TODO handle multiple SMS warning message
+        if 'next_url' in form.cleaned_data:
+            self.next_url = form.cleaned_data['next_url']
+
+        if is_ratelimited(
+            self.request,
+            key=sms_ratelimit_key,
+            group='phone-change-sms',
+            rate=self.authenticator.sms_number_ratelimit or None,
+            increment=True,
+        ):
+            form.add_error(
+                'phone',
+                _(
+                    'Multiple SMSs have already been sent to this number. Further attempts are blocked,'
+                    ' try again later.'
+                ),
+            )
+            return self.form_invalid(form)
+        if is_ratelimited(
+            self.request,
+            key='ip',
+            group='phone-change-sms',
+            rate=self.authenticator.sms_ip_ratelimit or None,
+            increment=True,
+        ):
+            form.add_error(
+                'phone',
+                _(
+                    'Multiple registration attempts have already been made from this IP address. No further'
+                    ' SMS will be sent for now, try again later.'
+                ),
+            )
+            return self.form_invalid(form)
+
+        try:
+            self.code = utils_sms.send_phone_change_sms(
+                phone,
+                self.request.user.ou,
+                user=self.request.user,
+            )
+        except utils_sms.SMSError:
+            messages.error(
+                self.request,
+                _(
+                    'Something went wrong while trying to send the SMS code to you. '
+                    'Please contact your administrator and try again later.'
+                ),
+            )
+            return utils_misc.redirect(self.request, reverse('auth_homepage'))
+
+        old_atvs = (
+            models.AttributeValue.objects.with_owner(self.request.user)
+            .filter(attribute=self.authenticator.phone_identifier_field)
+            .values_list('content', flat=True)
+        )
+        old_phone = old_atvs[0] if old_atvs else ''
+
+        self.request.journal.record(
+            'user.phone.change.request',
+            user=self.request.user,
+            old_phone=old_phone,
+            session=self.request.session,
+            new_phone=phone,
+        )
+        return super().form_valid(form)
+
+
+phone_change = login_required(PhoneChangeView.as_view())
+
+
+class PhoneChangeVerifyView(TemplateView):
+    def get(self, request, *args, **kwargs):
+        token = kwargs['token'].replace(' ', '')
+        if not token:
+            return shortcuts.redirect('phone-change')
+        authn = utils_misc.get_password_authenticator()
+        user_ct = ContentType.objects.get_for_model(get_user_model())
+        try:
+            token = models.Token.objects.get(
+                uuid=token,
+                kind='phone-change',
+            )
+            user_pk = token.content['user']
+            phone = token.content['phone']
+            user = User.objects.get(pk=user_pk)
+        except (models.Token.DoesNotExist, models.Token.MultipleObjectsReturned, ValueError):
+            messages.error(request, _('Your phone number update request is invalid, try again'))
+            return shortcuts.redirect('phone-change')
+        except User.DoesNotExist:
+            messages.error(
+                request, _('Your phone number update request relates to an unknown user, try again')
+            )
+            return shortcuts.redirect('phone-change')
+
+        try:
+            with atomic():
+                Lock.lock_identifier(phone)
+                non_unique = (
+                    models.AttributeValue.objects.filter(
+                        attribute=authn.phone_identifier_field,
+                        content_type=user_ct,
+                        object_id__isnull=False,
+                        content=phone,
+                    )
+                    .exclude(object_id=user_pk)
+                    .exists()
+                )
+                if non_unique:
+                    raise ValidationError(_('This phone number is already used by another account.'))
+                atv, dummy = models.AttributeValue.objects.get_or_create(
+                    attribute=authn.phone_identifier_field,
+                    content_type=user_ct,
+                    object_id=user.pk,
+                )
+                old_phone = atv.content or ''
+                atv.content = phone
+                atv.save()
+                user.phone_verified_on = timezone.now()
+                user.save(
+                    update_fields=[
+                        'phone_verified_on',
+                    ]
+                )
+                token.delete()
+        except Lock.Error:
+            messages.error(
+                request,
+                _(
+                    'Something went wrong while updating your phone number. Try again later or contact your platform administrator.'
+                ),
+            )
+            return shortcuts.redirect('phone-change')
+        except ValidationError as e:
+            messages.error(request, e.message)
+            return shortcuts.redirect('phone-change')
+        else:
+            messages.info(request, _('Your phone number is now {0}').format(phone))
+            logger.info('user %s changed its phone number from "%s" to "%s"', user, phone, phone)
+            hooks.call_hooks('event', name='change-phone-confirm', user=user, phone=phone)
+            request.journal.record(
+                'user.phone.change',
+                user=user,
+                session=request.session,
+                old_phone=old_phone,
+                new_phone=phone,
+            )
+            return shortcuts.redirect('account_management')
+
+
+phone_change_verify = PhoneChangeVerifyView.as_view()
+
+
 class EmailChangeVerifyView(TemplateView):
     def get(self, request, *args, **kwargs):
         if 'token' in request.GET:
@@ -532,6 +740,7 @@ class ProfileView(HomeURLMixin, cbv.TemplateNamesMixin, TemplateView):
                 if field_name not in field_names:
                     field_names.append(field_name)
         attributes = []
+        authenticator = utils_misc.get_password_authenticator()
         for field_name in field_names:
             title = None
             if isinstance(field_name, (list, tuple)):
@@ -596,6 +805,21 @@ class ProfileView(HomeURLMixin, cbv.TemplateNamesMixin, TemplateView):
             for idp_backend in idp_backends:
                 if hasattr(idp_backend, 'federation_management'):
                     federation_management.extend(idp_backend.federation_management(request))
+        user_ct = ContentType.objects.get_for_model(get_user_model())
+        atvs = models.AttributeValue.objects.filter(
+            attribute=authenticator.phone_identifier_field,
+            content_type=user_ct,
+            object_id=request.user.pk,
+            content__isnull=False,
+        ).values_list('content', flat=True)
+        if atvs:
+            context.update({'phone': atvs[0]})
+
+        allow_phone_change = bool(
+            authenticator.phone_identifier_field
+            and authenticator.phone_identifier_field.user_editable
+            and not authenticator.phone_identifier_field.disabled
+        )
         context.update(
             {
                 'frontends_block': blocks,
@@ -605,6 +829,7 @@ class ProfileView(HomeURLMixin, cbv.TemplateNamesMixin, TemplateView):
                 'allow_account_deletion': app_settings.A2_REGISTRATION_CAN_DELETE_ACCOUNT,
                 'allow_profile_edit': EditProfile.can_edit_profile(),
                 'allow_email_change': app_settings.A2_PROFILE_CAN_CHANGE_EMAIL,
+                'allow_phone_change': allow_phone_change,
                 'allow_authorization_management': False,
                 # TODO: deprecated should be removed when publik-base-theme is updated
                 'allow_password_change': utils_misc.user_can_change_password(request=request),
@@ -1351,7 +1576,6 @@ class InputSMSCodeView(cbv.ValidateCSRFMixin, FormView):
             return utils_misc.redirect(request, reverse('auth_homepage'))
         return super().post(request, *args, **kwargs)
 
-    @atomic(savepoint=False)
     def form_valid(self, form):
         super().form_valid(form)
         sms_code = form.cleaned_data.pop('sms_code')
@@ -1363,7 +1587,6 @@ class InputSMSCodeView(cbv.ValidateCSRFMixin, FormView):
         if self.code.expires < timezone.now():
             form.add_error('sms_code', _('The code has expired.'))
             return self.form_invalid(form)
-        Lock.lock_identifier(self.code.phone)
         content = {
             # TODO missing ou registration management
             'authentication_method': 'phone',
@@ -1398,6 +1621,15 @@ class InputSMSCodeView(cbv.ValidateCSRFMixin, FormView):
                 ),
                 params=params,
             )
+        elif self.code.kind == models.SMSCode.KIND_PHONE_CHANGE:
+            return utils_misc.redirect(
+                self.request,
+                reverse(
+                    'phone-change-verify',
+                    kwargs={'token': token.uuid},
+                ),
+                params=params,
+            )
 
 
 input_sms_code = InputSMSCodeView.as_view()

tests/conftest.py

@@ -335,6 +335,7 @@ def phone_activated_authn(db):
     phone, dummy = Attribute.objects.get_or_create(
         name='phone',
         kind='phone_number',
+        user_editable=True,
         defaults={'label': 'Phone'},
     )
     LoginPasswordAuthenticator.objects.update(

tests/test_change_phone.py

@@ -0,0 +1,480 @@
+# authentic2 - versatile identity manager
+# Copyright (C) 2010-2019 Entr'ouvert
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License as published
+# by the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from httmock import HTTMock, remember_called, urlmatch
+
+from authentic2.models import Attribute, SMSCode, Token
+
+from .utils import login
+
+
+@urlmatch(netloc='foo.whatever.none')
+@remember_called
+def sms_service_mock(url, request):
+    return {
+        'content': {},
+        'headers': {
+            'content-type': 'application/json',
+        },
+        'status_code': 200,
+    }
+
+
+def test_change_phone(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    Attribute.objects.get_or_create(
+        name='another_phone',
+        kind='phone_number',
+        defaults={'label': 'Another phone'},
+    )
+
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.attributes.another_phone = '+33122444444'
+    nomail_user.phone = ''
+    nomail_user.save()
+
+    assert nomail_user.phone_verified_on is None
+
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    assert "Your current phone number is +33122446688." in resp.text
+
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+    resp.form.set('sms_code', code.value)
+    resp = resp.form.submit('').follow()
+    assert not Token.objects.count()
+    nomail_user.refresh_from_db()
+    assert nomail_user.attributes.phone == '+33122446666'
+    assert nomail_user.attributes.another_phone == '+33122444444'  # unchanged
+    assert nomail_user.phone_verified_on is not None
+
+
+def test_change_phone_nondefault_attribute(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    phone, dummy = Attribute.objects.get_or_create(
+        name='another_phone',
+        kind='phone_number',
+        user_editable=True,
+        defaults={'label': 'Another phone'},
+    )
+
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.attributes.another_phone = '+33122444444'
+    nomail_user.phone = ''
+    nomail_user.save()
+
+    assert nomail_user.phone_verified_on is None
+
+    phone_activated_authn.phone_identifier_field = phone
+    phone_activated_authn.save()
+
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.another_phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    assert "Your current phone number is +33122444444." in resp.text
+
+    resp.form.set('phone_1', '122666666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+    resp.form.set('sms_code', code.value)
+    resp = resp.form.submit('').follow()
+    assert not Token.objects.count()
+    nomail_user.refresh_from_db()
+    assert nomail_user.attributes.phone == '+33122446688'  # unchanged
+    assert nomail_user.attributes.another_phone == '+33122666666'
+    assert nomail_user.phone_verified_on is not None
+
+
+def test_change_phone_wrong_input(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    resp.form.set('phone_1', '12244666')
+    resp.form.set('password', nomail_user.username)
+    resp = resp.form.submit()
+    assert 'Invalid phone number.' in resp.pyquery('.error p')[0].text
+
+    assert not SMSCode.objects.count()
+    assert not Token.objects.count()
+    resp.form.set('phone_1', 'abc')
+    resp.form.set('password', nomail_user.username)
+    resp = resp.form.submit()
+    assert (
+        'Phone number must be either in E.164 globally unique format or dialable from 33 country code (FR).'
+        in resp.pyquery('.error p')[0].text
+    )
+    assert not SMSCode.objects.count()
+    assert not Token.objects.count()
+
+
+def test_change_phone_expired_code(app, nomail_user, user_ou1, phone_activated_authn, settings, freezer):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+    resp.form.set('sms_code', code.value)
+    freezer.tick(3600)  # user did not immediately submit code
+    resp = resp.form.submit('')
+    assert resp.pyquery('ul.errorlist li')[0].text == 'The code has expired.'
+    assert not Token.objects.count()
+
+
+def test_change_phone_code_modified(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit()
+    location = resp.location[:-5] + 'abcd/'  # oops, something went wrong with the url token
+    app.get(location, status=400)
+    assert not Token.objects.count()
+
+
+def test_change_phone_token_modified(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+    resp.form.set('sms_code', code.value)
+    resp = resp.form.submit('')
+    resp.location = resp.location.split('?')[0]
+    resp.location = resp.location[:-5] + 'abcd/'  # oops, something went wrong with the url token
+    resp = resp.follow().maybe_follow()
+    assert resp.pyquery('.error')[0].text == 'Your phone number update request is invalid, try again'
+    nomail_user.refresh_from_db()
+    assert nomail_user.attributes.phone == '+33122446688'
+
+
+def test_change_phone_identifier_attribute_changed(
+    app, nomail_user, user_ou1, phone_activated_authn, settings
+):
+    phone, dummy = Attribute.objects.get_or_create(
+        name='another_phone',
+        kind='phone_number',
+        defaults={'label': 'Another phone'},
+    )
+
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.attributes.another_phone = '+33122444444'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    assert "Your current phone number is +33122446688." in resp.text
+
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    phone_activated_authn.phone_identifier_field = phone
+    phone_activated_authn.save()
+    code = SMSCode.objects.get()
+    resp.form.set('sms_code', code.value)
+    resp = resp.form.submit('').follow()
+    assert not Token.objects.count()
+    nomail_user.refresh_from_db()
+    # phone fields have been swapped, nothing really worth doing about this
+    # we just check that the phone change request does not crash
+    assert nomail_user.attributes.phone == '+33122446688'  # unchanged
+    assert nomail_user.attributes.another_phone == '+33122446666'
+
+
+def test_change_phone_authn_deactivated(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/',
+        password=nomail_user.username,
+    )
+
+    phone_activated_authn.accept_phone_authentication = False
+    phone_activated_authn.save()
+
+    resp = app.get('/accounts/change-phone/')
+    assert "Your current phone number is +33122446688." in resp.text
+
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+    resp.form.set('sms_code', code.value)
+    resp = resp.form.submit('').follow()
+    assert not Token.objects.count()
+    # assert not SMSCode.objects.count()  # avoid multiple uses
+    nomail_user.refresh_from_db()
+
+
+def test_change_phone_identifier_field_unknown(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/',
+        password=nomail_user.username,
+    )
+
+    phone_activated_authn.phone_identifier_field = None
+    phone_activated_authn.save()
+
+    app.get('/accounts/change-phone/', status=404)
+
+
+def test_change_phone_identifier_field_not_user_editable(
+    app, nomail_user, user_ou1, phone_activated_authn, settings
+):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/',
+        password=nomail_user.username,
+    )
+
+    phone_activated_authn.phone_identifier_field.user_editable = False
+    phone_activated_authn.phone_identifier_field.save()
+
+    app.get('/accounts/change-phone/', status=404)
+
+
+def test_change_phone_identifier_field_disabled(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/',
+        password=nomail_user.username,
+    )
+
+    phone_activated_authn.phone_identifier_field.disabled = True
+    phone_activated_authn.phone_identifier_field.save()
+
+    app.get('/accounts/change-phone/', status=404)
+
+
+def test_phone_change_already_existing(
+    app, nomail_user, user_ou1, phone_activated_authn, settings, simple_user
+):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+    simple_user.attributes.phone = '+33122446666'
+    simple_user.save()
+
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+    resp.form.set('sms_code', code.value)
+    resp = resp.form.submit('').follow().maybe_follow()
+    assert resp.pyquery('li.error')[0].text == 'This phone number is already used by another account.'
+
+
+def test_phone_change_preempted_during_request(
+    app, nomail_user, user_ou1, phone_activated_authn, settings, simple_user
+):
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+    resp.form.set('sms_code', code.value)
+    # oops, some other user took this number during the change request
+    simple_user.attributes.phone = '+33122446666'
+    simple_user.save()
+    resp = resp.form.submit('').follow().maybe_follow()
+    assert resp.pyquery('li.error')[0].text == 'This phone number is already used by another account.'
+
+
+def test_phone_change_lock_identifier_error_token_use(
+    app, nomail_user, user_ou1, phone_activated_authn, settings, monkeypatch
+):
+    from authentic2.models import Lock
+
+    nomail_user.attributes.phone = '+33122446688'
+    nomail_user.save()
+
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    def erroneous_lock_identifier(identifier, nowait=False):
+        raise Lock.Error
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+
+    resp.form.set('sms_code', code.value)
+    resp = resp.form.submit('')
+    monkeypatch.setattr(Lock, 'lock_identifier', erroneous_lock_identifier)
+    resp = resp.follow().maybe_follow()
+    assert 'Something went wrong while updating' in resp.pyquery('li.error')[0].text
+    assert nomail_user.attributes.phone == '+33122446688'
+
+
+def test_phone_change_no_existing_number(app, nomail_user, user_ou1, phone_activated_authn, settings):
+    settings.SMS_URL = 'https://foo.whatever.none/'
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/change-phone/',
+        password=nomail_user.username,
+    )
+    assert 'Your account does not declare a phone number yet.' in resp.text
+    assert 'Your phone number is' not in resp.text
+    resp.form.set('phone_1', '122446666')
+    resp.form.set('password', nomail_user.username)
+    with HTTMock(sms_service_mock):
+        resp = resp.form.submit().follow()
+    code = SMSCode.objects.get()
+
+    resp.form.set('sms_code', code.value)
+    resp.form.submit('').follow()
+    nomail_user.refresh_from_db()
+    assert nomail_user.attributes.phone == '+33122446666'
+    assert nomail_user.phone_verified_on is not None
+
+
+def test_phone_change_no_existing_number_accounts_action_label_variation(
+    app,
+    nomail_user,
+    phone_activated_authn,
+):
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/',
+        password=nomail_user.username,
+    )
+    assert resp.pyquery("[href='/accounts/change-phone/']")[0].text == 'Declare your phone number'
+    nomail_user.attributes.phone = '+33122446666'
+    nomail_user.save()
+
+    resp = app.get('/accounts/')
+    assert resp.pyquery("[href='/accounts/change-phone/']")[0].text == 'Change phone'

tests/test_manager_journal.py

@@ -247,6 +247,20 @@ def events(db, superuser, freezer):
         role=role_user,
         admin_user=user,
     )
+    make(
+        'user.phone.change.request',
+        user=user,
+        old_phone='+33122334455',
+        session=session1,
+        new_phone='+33111223344',
+    )
+    make(
+        'user.phone.change',
+        user=user,
+        session=session1,
+        old_phone='+33122334455',
+        new_phone='+33111223344',
+    )
     make(
         'user.email.change.request',
         user=user,
@@ -613,19 +627,31 @@ def test_global_journal(app, superuser, events):
             'user': 'agent',
         },
         {
-            'message': 'email change request for email address "new@example.com"',
             'timestamp': 'Jan. 2, 2020, 3 p.m.',
+            'type': 'user.phone.change.request',
+            'user': 'Johnny doe',
+            'message': 'phone change request to number "+33111223344"',
+        },
+        {
+            'timestamp': 'Jan. 2, 2020, 4 p.m.',
+            'type': 'user.phone.change',
+            'user': 'Johnny doe',
+            'message': 'phone number changed from "+33122334455" to "+33111223344"',
+        },
+        {
+            'message': 'email change request for email address "new@example.com"',
+            'timestamp': 'Jan. 2, 2020, 5 p.m.',
             'type': 'user.email.change.request',
             'user': 'Johnny doe',
         },
         {
             'message': 'email address changed from "old@example.com" to "new@example.com"',
-            'timestamp': 'Jan. 2, 2020, 4 p.m.',
+            'timestamp': 'Jan. 2, 2020, 6 p.m.',
             'type': 'user.email.change',
             'user': 'Johnny doe',
         },
         {
-            'timestamp': 'Jan. 2, 2020, 5 p.m.',
+            'timestamp': 'Jan. 2, 2020, 7 p.m.',
             'type': 'manager.user.deactivation',
             'user': '-',
             'message': (
@@ -634,7 +660,7 @@ def test_global_journal(app, superuser, events):
             ),
         },
         {
-            'timestamp': 'Jan. 2, 2020, 6 p.m.',
+            'timestamp': 'Jan. 2, 2020, 8 p.m.',
             'type': 'manager.user.deactivation',
             'user': '-',
             'message': (
@@ -646,36 +672,36 @@ def test_global_journal(app, superuser, events):
             'message': (
                 'automatic activation of user "Johnny doe" because the associated LDAP account reappeared'
             ),
-            'timestamp': 'Jan. 2, 2020, 7 p.m.',
+            'timestamp': 'Jan. 2, 2020, 9 p.m.',
             'type': 'manager.user.activation',
             'user': '-',
         },
         {
             'message': 'refusal of single sign on with "service"',
-            'timestamp': 'Jan. 2, 2020, 8 p.m.',
+            'timestamp': 'Jan. 2, 2020, 10 p.m.',
             'type': 'user.service.sso.refusal',
             'user': 'Johnny doe',
         },
         {
             'message': 'was denied single sign on with "service"',
-            'timestamp': 'Jan. 2, 2020, 9 p.m.',
+            'timestamp': 'Jan. 2, 2020, 11 p.m.',
             'type': 'user.service.sso.denial',
             'user': 'Johnny doe',
         },
         {
-            'timestamp': 'Jan. 2, 2020, 10 p.m.',
+            'timestamp': 'Jan. 3, 2020, midnight',
             'type': 'user.profile.add',
             'user': 'agent',
             'message': 'profile "aaa" of type "One Type" created for user "Johnny doe"',
         },
         {
-            'timestamp': 'Jan. 2, 2020, 11 p.m.',
+            'timestamp': 'Jan. 3, 2020, 1 a.m.',
             'type': 'user.profile.update',
             'user': 'agent',
             'message': 'profile "aaa" of type "One Type" updated for user "Johnny doe"',
         },
         {
-            'timestamp': 'Jan. 3, 2020, midnight',
+            'timestamp': 'Jan. 3, 2020, 2 a.m.',
             'type': 'user.profile.delete',
             'user': 'agent',
             'message': 'profile "aaa" of type "One Type" deleted for user "Johnny doe"',
@@ -683,83 +709,83 @@ def test_global_journal(app, superuser, events):
         {
             'message': 'notification sent to "user@example.com" after 120 days of '
             'inactivity. Account will be deleted in 20 days.',
-            'timestamp': 'Jan. 3, 2020, 1 a.m.',
+            'timestamp': 'Jan. 3, 2020, 3 a.m.',
             'type': 'user.notification.inactivity',
             'user': 'Johnny doe',
         },
         {
             'message': 'user deletion after 140 days of inactivity, notification sent to '
             '"user@example.com".',
-            'timestamp': 'Jan. 3, 2020, 2 a.m.',
+            'timestamp': 'Jan. 3, 2020, 4 a.m.',
             'type': 'user.deletion.inactivity',
             'user': 'Johnny doe',
         },
         {
             'message': 'creation of authenticator "Password"',
-            'timestamp': 'Jan. 3, 2020, 3 a.m.',
+            'timestamp': 'Jan. 3, 2020, 5 a.m.',
             'type': 'authenticator.creation',
             'user': 'agent',
         },
         {
             'message': 'edit of authenticator "Password" (name)',
-            'timestamp': 'Jan. 3, 2020, 4 a.m.',
+            'timestamp': 'Jan. 3, 2020, 6 a.m.',
             'type': 'authenticator.edit',
             'user': 'agent',
         },
         {
             'message': 'enable of authenticator "Password"',
-            'timestamp': 'Jan. 3, 2020, 5 a.m.',
+            'timestamp': 'Jan. 3, 2020, 7 a.m.',
             'type': 'authenticator.enable',
             'user': 'agent',
         },
         {
             'message': 'disable of authenticator "Password"',
-            'timestamp': 'Jan. 3, 2020, 6 a.m.',
+            'timestamp': 'Jan. 3, 2020, 8 a.m.',
             'type': 'authenticator.disable',
             'user': 'agent',
         },
         {
             'message': 'deletion of authenticator "Password"',
-            'timestamp': 'Jan. 3, 2020, 7 a.m.',
+            'timestamp': 'Jan. 3, 2020, 9 a.m.',
             'type': 'authenticator.deletion',
             'user': 'agent',
         },
         {
             'message': 'creation of object "Set an attribute (%s)" in authenticator "SAML - saml"'
             % set_attribute_action.pk,
-            'timestamp': 'Jan. 3, 2020, 8 a.m.',
+            'timestamp': 'Jan. 3, 2020, 10 a.m.',
             'type': 'authenticator.related_object.creation',
             'user': 'agent',
         },
         {
             'message': 'edit of object "Set an attribute (%s)" in authenticator "SAML - saml" (from_name)'
             % set_attribute_action.pk,
-            'timestamp': 'Jan. 3, 2020, 9 a.m.',
+            'timestamp': 'Jan. 3, 2020, 11 a.m.',
             'type': 'authenticator.related_object.edit',
             'user': 'agent',
         },
         {
             'message': 'deletion of object "Set an attribute (%s)" in authenticator "SAML - saml"'
             % set_attribute_action.pk,
-            'timestamp': 'Jan. 3, 2020, 10 a.m.',
+            'timestamp': 'Jan. 3, 2020, noon',
             'type': 'authenticator.related_object.deletion',
             'user': 'agent',
         },
         {
             'message': 'user "Johnny doe" activity notified by service "service"',
-            'timestamp': 'Jan. 3, 2020, 11 a.m.',
+            'timestamp': 'Jan. 3, 2020, 1 p.m.',
             'type': 'user.notification.activity',
             'user': '-',
         },
         {
             'message': 'user "Johnny doe" activity notified by user "super user"',
-            'timestamp': 'Jan. 3, 2020, noon',
+            'timestamp': 'Jan. 3, 2020, 2 p.m.',
             'type': 'user.notification.activity',
             'user': 'super user',
         },
         {
             'message': 'password reset',
-            'timestamp': 'Jan. 3, 2020, 1 p.m.',
+            'timestamp': 'Jan. 3, 2020, 3 p.m.',
             'type': 'user.password.reset',
             'user': f'deleted user (#{deleted_user.old_user_id}, deleted@example.com)',
         },
@@ -945,70 +971,82 @@ def test_user_journal(app, superuser, events):
             'user': 'agent',
         },
         {
-            'message': 'email change request for email address "new@example.com"',
             'timestamp': 'Jan. 2, 2020, 3 p.m.',
+            'type': 'user.phone.change.request',
+            'user': 'Johnny doe',
+            'message': 'phone change request to number "+33111223344"',
+        },
+        {
+            'timestamp': 'Jan. 2, 2020, 4 p.m.',
+            'type': 'user.phone.change',
+            'user': 'Johnny doe',
+            'message': 'phone number changed from "+33122334455" to "+33111223344"',
+        },
+        {
+            'message': 'email change request for email address "new@example.com"',
+            'timestamp': 'Jan. 2, 2020, 5 p.m.',
             'type': 'user.email.change.request',
             'user': 'Johnny doe',
         },
         {
             'message': 'email address changed from "old@example.com" to "new@example.com"',
-            'timestamp': 'Jan. 2, 2020, 4 p.m.',
+            'timestamp': 'Jan. 2, 2020, 6 p.m.',
             'type': 'user.email.change',
             'user': 'Johnny doe',
         },
         {
-            'timestamp': 'Jan. 2, 2020, 5 p.m.',
+            'timestamp': 'Jan. 2, 2020, 7 p.m.',
             'type': 'manager.user.deactivation',
             'user': '-',
             'message': 'automatic deactivation because the associated LDAP account does not exist anymore',
         },
         {
-            'timestamp': 'Jan. 2, 2020, 6 p.m.',
+            'timestamp': 'Jan. 2, 2020, 8 p.m.',
             'type': 'manager.user.deactivation',
             'user': '-',
             'message': 'automatic deactivation because the associated LDAP source has been deleted',
         },
         {
             'message': 'automatic activation because the associated LDAP account reappeared',
-            'timestamp': 'Jan. 2, 2020, 7 p.m.',
+            'timestamp': 'Jan. 2, 2020, 9 p.m.',
             'type': 'manager.user.activation',
             'user': '-',
         },
         {
             'message': 'refusal of single sign on with "service"',
-            'timestamp': 'Jan. 2, 2020, 8 p.m.',
+            'timestamp': 'Jan. 2, 2020, 10 p.m.',
             'type': 'user.service.sso.refusal',
             'user': 'Johnny doe',
         },
         {
             'message': 'was denied single sign on with "service"',
-            'timestamp': 'Jan. 2, 2020, 9 p.m.',
+            'timestamp': 'Jan. 2, 2020, 11 p.m.',
             'type': 'user.service.sso.denial',
             'user': 'Johnny doe',
         },
         {
             'message': 'notification sent to "user@example.com" after 120 days of '
             'inactivity. Account will be deleted in 20 days.',
-            'timestamp': 'Jan. 3, 2020, 1 a.m.',
+            'timestamp': 'Jan. 3, 2020, 3 a.m.',
             'type': 'user.notification.inactivity',
             'user': 'Johnny doe',
         },
         {
             'message': 'user deletion after 140 days of inactivity, notification sent to '
             '"user@example.com".',
-            'timestamp': 'Jan. 3, 2020, 2 a.m.',
+            'timestamp': 'Jan. 3, 2020, 4 a.m.',
             'type': 'user.deletion.inactivity',
             'user': 'Johnny doe',
         },
         {
             'message': 'user activity notified by service "service"',
-            'timestamp': 'Jan. 3, 2020, 11 a.m.',
+            'timestamp': 'Jan. 3, 2020, 1 p.m.',
             'type': 'user.notification.activity',
             'user': '-',
         },
         {
             'message': 'user activity notified by user "super user"',
-            'timestamp': 'Jan. 3, 2020, noon',
+            'timestamp': 'Jan. 3, 2020, 2 p.m.',
             'type': 'user.notification.activity',
             'user': 'super user',
         },
@@ -1239,7 +1277,7 @@ def test_search(app, superuser, events):
 
     response.form.set('search', 'session:1234')
     response = response.form.submit()
-    assert len(response.pyquery('tbody tr')) == 13
+    assert len(response.pyquery('tbody tr')) == 15
     assert all(
         text_content(node) == 'Johnny doe'
         for node in response.pyquery('tbody tr td.journal-list--user-column')

tests/test_profile.py

@@ -19,6 +19,7 @@ import pytest
 from django.urls import reverse
 
 from authentic2.a2_rbac.utils import get_default_ou
+from authentic2.apps.authenticators.models import LoginPasswordAuthenticator
 from authentic2.models import Attribute
 from authentic2_idp_oidc.models import OIDCClient
 
@@ -27,7 +28,7 @@ from . import utils
 pytestmark = pytest.mark.django_db
 
 
-def test_account_edit_view(app, simple_user):
+def test_account_edit_view(app, simple_user, settings):
     utils.login(app, simple_user)
     url = reverse('profile_edit')
     resp = app.get(url, status=200)
@@ -117,6 +118,125 @@ def test_account_edit_view(app, simple_user):
     assert 'agreement@disabled' in resp
     assert phone.get_value(simple_user) == '+33123456789'
 
+    phone.disabled = False
+    phone.save()
+    LoginPasswordAuthenticator.objects.update(
+        accept_phone_authentication=True,
+        phone_identifier_field=phone,
+    )
+
+    resp = app.get(url, status=200)
+    assert 'Phone' not in resp.text
+    resp = app.get(reverse('account_management'), status=200)
+    profile = [
+        (dt.text.split('\xa0')[0], dd.text.strip())
+        for dt, dd in zip(resp.pyquery('dl dt'), resp.pyquery('dl dd'))
+    ]
+    # phone present in /accounts/ overview page
+    assert profile == [
+        ('First name', 'Jôhn'),
+        ('Last name', 'Dôe'),
+        ('Email address', 'user@example.net'),
+        ('Phone', '+33123456789'),
+        ('Title', 'Mr'),
+        ('Agreement', 'True'),
+        ('Language', 'French'),
+    ]
+
+    settings.A2_PROFILE_FIELDS = [attr.name for attr in Attribute.objects.all()]
+
+    resp = app.get(url, status=200)
+    assert 'Phone' not in resp.text
+    resp = app.get(reverse('account_management'), status=200)
+    profile = [
+        (dt.text.split('\xa0')[0], dd.text.strip())
+        for dt, dd in zip(resp.pyquery('dl dt'), resp.pyquery('dl dd'))
+    ]
+    assert profile == [
+        ('First name', 'Jôhn'),
+        ('Last name', 'Dôe'),
+        ('Phone', '+33123456789'),
+        ('Title', 'Mr'),
+        ('Agreement', 'True'),
+        ('Language', 'French'),
+    ]
+
+    settings.A2_PROFILE_FIELDS = []
+
+    another_phone = Attribute.objects.create(
+        name='another_phone',
+        label='Another phone',
+        kind='phone_number',
+        user_visible=True,
+        user_editable=True,
+    )
+    simple_user.attributes.another_phone = '+33122334455'
+    simple_user.save()
+
+    resp = app.get(url, status=200)
+    assert 'Another phone' in resp.text
+    resp = app.get(reverse('account_management'), status=200)
+    profile = [
+        (dt.text.split('\xa0')[0], dd.text.strip())
+        for dt, dd in zip(resp.pyquery('dl dt'), resp.pyquery('dl dd'))
+    ]
+
+    assert profile == [
+        ('First name', 'Jôhn'),
+        ('Last name', 'Dôe'),
+        ('Email address', 'user@example.net'),
+        ('Phone', '+33123456789'),
+        ('Title', 'Mr'),
+        ('Agreement', 'True'),
+        ('Language', 'French'),
+        ('Another phone', '+33122334455'),
+    ]
+
+    LoginPasswordAuthenticator.objects.update(phone_identifier_field=another_phone)
+
+    resp = app.get(url, status=200)
+    assert 'Another phone' not in resp.text
+    resp = app.get(reverse('account_management'), status=200)
+    profile = [
+        (dt.text.split('\xa0')[0], dd.text.strip())
+        for dt, dd in zip(resp.pyquery('dl dt'), resp.pyquery('dl dd'))
+    ]
+
+    assert profile == [
+        ('First name', 'Jôhn'),
+        ('Last name', 'Dôe'),
+        ('Email address', 'user@example.net'),
+        ('Phone', '+33123456789'),
+        ('Title', 'Mr'),
+        ('Agreement', 'True'),
+        ('Language', 'French'),
+        ('Another phone', '+33122334455'),
+    ]
+
+    LoginPasswordAuthenticator.objects.update(
+        phone_identifier_field=None,
+        accept_phone_authentication=False,
+    )
+
+    resp = app.get(url, status=200)
+    assert 'Another phone' in resp.text
+    resp = app.get(reverse('account_management'), status=200)
+    profile = [
+        (dt.text.split('\xa0')[0], dd.text.strip())
+        for dt, dd in zip(resp.pyquery('dl dt'), resp.pyquery('dl dd'))
+    ]
+
+    assert profile == [
+        ('First name', 'Jôhn'),
+        ('Last name', 'Dôe'),
+        ('Email address', 'user@example.net'),
+        ('Phone', '+33123456789'),
+        ('Title', 'Mr'),
+        ('Agreement', 'True'),
+        ('Language', 'French'),
+        ('Another phone', '+33122334455'),
+    ]
+
 
 def test_account_edit_next_url(app, simple_user, external_redirect_next_url, assert_external_redirect):
     utils.login(app, simple_user)
@@ -139,6 +259,73 @@ def test_account_edit_next_url(app, simple_user, external_redirect_next_url, ass
     assert attribute.get_value(simple_user) == '0123456789'
 
 
+def test_account_edit_no_direct_modification_fields(app, simple_user, settings):
+    utils.login(app, simple_user)
+    url = reverse('profile_edit')
+
+    phone = Attribute.objects.create(
+        name='phone', label='phone', kind='string', user_visible=True, user_editable=True, scopes='contact'
+    )
+    Attribute.objects.create(
+        name='mobile',
+        label='mobile phone',
+        kind='string',
+        user_visible=True,
+        user_editable=True,
+        scopes='contact',
+    )
+
+    Attribute.objects.create(
+        name='city', label='city', kind='string', user_visible=True, user_editable=True, scopes='address'
+    )
+    Attribute.objects.create(
+        name='zipcode',
+        label='zipcode',
+        kind='string',
+        user_visible=True,
+        user_editable=True,
+        scopes='address',
+    )
+
+    def get_fields(resp):
+        return {
+            key for key in resp.form.fields.keys() if key and key not in ['csrfmiddlewaretoken', 'cancel']
+        }
+
+    resp = app.get(url, status=200)
+    assert get_fields(resp) == {'first_name', 'last_name', 'phone', 'mobile', 'city', 'zipcode', 'next_url'}
+
+    LoginPasswordAuthenticator.objects.update(
+        accept_phone_authentication=True,
+        phone_identifier_field=phone,
+    )
+
+    resp = app.get(url, status=200)
+    assert get_fields(resp) == {'first_name', 'last_name', 'mobile', 'city', 'zipcode', 'next_url'}
+
+    # profile fields as set by hobo's settings loader
+    settings.A2_PROFILE_FIELDS = [attr.name for attr in Attribute.objects.exclude()]
+
+    resp = app.get(url, status=200)
+    assert get_fields(resp) == {'first_name', 'last_name', 'mobile', 'city', 'zipcode', 'next_url'}
+
+    # disabling effective authn does not change the identifier nature of the attribute
+    LoginPasswordAuthenticator.objects.update(
+        accept_phone_authentication=False,
+    )
+
+    resp = app.get(url, status=200)
+    assert get_fields(resp) == {'first_name', 'last_name', 'mobile', 'city', 'zipcode', 'next_url'}
+
+    # removing any known identifier changes the identifier nature of the attribute
+    LoginPasswordAuthenticator.objects.update(
+        phone_identifier_field=None,
+    )
+
+    resp = app.get(url, status=200)
+    assert get_fields(resp) == {'first_name', 'last_name', 'phone', 'mobile', 'city', 'zipcode', 'next_url'}
+
+
 def test_account_edit_scopes(app, simple_user):
     utils.login(app, simple_user)
     url = reverse('profile_edit')
@@ -257,6 +444,63 @@ def test_account_view(app, simple_user, settings):
     ]
     settings.INSTALLED_APPS += ('authentic2_idp_oidc',)
 
+    phone, dummy = Attribute.objects.get_or_create(
+        name='phone',
+        label='Phone',
+        kind='string',
+        user_editable=True,
+    )
+    LoginPasswordAuthenticator.objects.update(
+        phone_identifier_field=phone,
+        accept_phone_authentication=True,
+    )
+    url = reverse('account_management')
+    response = app.get(url, status=200)
+    assert [x['href'] for x in response.html.find('div', {'id': 'a2-profile'}).find_all('a')] == [
+        reverse('email-change'),
+        reverse('phone-change'),
+        reverse('profile_edit'),
+        reverse('consents'),
+        reverse('delete_account'),
+    ]
+
+    LoginPasswordAuthenticator.objects.update(accept_phone_authentication=False)
+    url = reverse('account_management')
+    response = app.get(url, status=200)
+    assert [x['href'] for x in response.html.find('div', {'id': 'a2-profile'}).find_all('a')] == [
+        reverse('email-change'),
+        reverse('phone-change'),
+        reverse('profile_edit'),
+        reverse('consents'),
+        reverse('delete_account'),
+    ]
+
+    phone.user_editable = False
+    phone.save()
+    url = reverse('account_management')
+    response = app.get(url, status=200)
+    assert [x['href'] for x in response.html.find('div', {'id': 'a2-profile'}).find_all('a')] == [
+        reverse('email-change'),
+        reverse('profile_edit'),
+        reverse('consents'),
+        reverse('delete_account'),
+    ]
+
+    phone.user_editable = True
+    phone.disabled = True
+    phone.save()
+    url = reverse('account_management')
+    response = app.get(url, status=200)
+    assert [x['href'] for x in response.html.find('div', {'id': 'a2-profile'}).find_all('a')] == [
+        reverse('email-change'),
+        reverse('profile_edit'),
+        reverse('consents'),
+        reverse('delete_account'),
+    ]
+    phone.disabled = False
+    phone.save()
+    LoginPasswordAuthenticator.objects.update(phone_identifier_field=None)
+
     # more disabled options -> less actions
     settings.A2_PROFILE_CAN_CHANGE_EMAIL = False
     settings.A2_PROFILE_CAN_MANAGE_SERVICE_AUTHORIZATIONS = False

tests/test_views.py

@@ -320,7 +320,7 @@ def test_custom_account(settings, app, simple_user):
     assert response['Location'] == settings.A2_ACCOUNTS_URL
 
 
-@pytest.mark.parametrize('view_name', ['registration_register'])  # password_lost to be added with #69890
+@pytest.mark.parametrize('view_name', ['registration_register', 'password_reset', 'phone-change'])
 def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name, phone_activated_authn):
     freezer.move_to('2020-01-01')
     LoginPasswordAuthenticator.objects.update(sms_ip_ratelimit='10/h', sms_number_ratelimit='3/d')
@@ -332,16 +332,21 @@ def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name,
         return httmock_response(200, {})
 
     with HTTMock(sms_endpoint_response):
-        # reach email limit
+        if view_name in ('phone-change',):
+            login(app, simple_user)
         response = app.get(reverse(view_name))
         response.form.set('phone_0', '33')
         response.form.set('phone_1', '0612345678')
+        if view_name in ('phone-change',):
+            response.form.set('password', simple_user.username)
         response = response.form.submit()
         assert 'try again later' not in response.text
         for _ in range(2):
             response = app.get(reverse(view_name))
             response.form.set('phone_0', '33')
             response.form.set('phone_1', '0612345678')
+            if view_name in ('phone-change',):
+                response.form.set('password', simple_user.username)
             response = response.form.submit()
             if view_name == 'registration_register':  # XXX not supported in password_reset yet
                 assert 'An SMS code has already been sent' in response.text
@@ -351,7 +356,11 @@ def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name,
         response = app.get(reverse(view_name))
         response.form.set('phone_0', '33')
         response.form.set('phone_1', '0612345678')
-        response = response.form.submit().form.submit()
+        if view_name in ('phone-change',):
+            response.form.set('password', simple_user.username)
+        response = response.form.submit()
+        if view_name in ('registration_register', 'password_reset'):
+            response = response.form.submit()
         assert 'try again later' in response.text
 
         # reach ip limit
@@ -360,6 +369,8 @@ def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name,
             random_suffix = randint(0, 9999)
             response.form.set('phone_0', '33')
             response.form.set('phone_1', f'061234{random_suffix:04d}')
+            if view_name in ('phone-change',):
+                response.form.set('password', simple_user.username)
             response = response.form.submit()
             assert 'try again later' not in response.text
 
@@ -367,6 +378,8 @@ def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name,
         random_suffix = randint(0, 9999)
         response.form.set('phone_0', '33')
         response.form.set('phone_1', f'061234{random_suffix:04d}')
+        if view_name in ('phone-change',):
+            response.form.set('password', simple_user.username)
         response = response.form.submit()
         assert 'try again later' in response.text
 
@@ -376,6 +389,8 @@ def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name,
         random_suffix = randint(0, 9999)
         response.form.set('phone_0', '33')
         response.form.set('phone_1', f'061234{random_suffix:04d}')
+        if view_name in ('phone-change',):
+            response.form.set('password', simple_user.username)
         response = response.form.submit()
         assert 'try again later' not in response.text
 
@@ -383,15 +398,20 @@ def test_views_sms_ratelimit(app, db, simple_user, settings, freezer, view_name,
         response = app.get(reverse(view_name))
         response.form.set('phone_0', '33')
         response.form.set('phone_1', '0612345678')
+        if view_name in ('phone-change',):
+            response.form.set('password', simple_user.username)
         response = response.form.submit()
         assert 'Multiple SMSs have already been sent to this number.' in response.text
-        response = response.form.submit()
-        assert 'try again later' in response.text
+        if view_name in ('registration_register', 'password_reset'):
+            response = response.form.submit()
+            assert 'try again later' in response.text
 
         freezer.tick(datetime.timedelta(days=1))
         response = app.get(reverse(view_name))
         response.form.set('phone_0', '33')
         response.form.set('phone_1', '0612345678')
+        if view_name in ('phone-change',):
+            response.form.set('password', simple_user.username)
         response = response.form.submit()
         assert 'try again later' not in response.text