handle password-less phone identifier change (#79891)

src/authentic2/forms/profile.py

@@ -59,16 +59,13 @@ class PhoneChangeFormNoPassword(forms.Form):
 
     def __init__(self, user, *args, **kwargs):
         self.user = user
+        self.authenticator = kwargs.pop('password_authenticator')
         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 (

src/authentic2/views.py

@@ -256,7 +256,7 @@ class EmailChangeView(HomeURLMixin, IdentifierChangeMixin, cbv.TemplateNamesMixi
     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.'),)
+    reauthn_message = _('You must re-authenticate to change your email address.')
     action = 'email-change'
 
     def get_form_class(self):
@@ -289,10 +289,14 @@ 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 get_form_class(self):
+        if self.can_validate_with_password():
+            return profile_forms.PhoneChangeForm
+        return profile_forms.PhoneChangeFormNoPassword
+
     def dispatch(self, *args, **kwargs):
         self.authenticator = utils_misc.get_password_authenticator()
         if not (

tests/test_change_email.py

@@ -14,6 +14,8 @@
 # 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 unittest import mock
+
 from . import utils
 
 
@@ -32,6 +34,39 @@ def change_email(app, user, email, mailoutbox):
     return mailoutbox[-1]
 
 
+def change_email_no_password(app, user, email, mailoutbox, recent_authn=True):
+    utils.login(app, user)
+    # for some reason session-stored authn events have been lost
+    app.session['authentication-events'] = []
+    app.session.save()
+    l = len(mailoutbox)
+    with mock.patch(
+        'authentic2.views.IdentifierChangeMixin.can_validate_with_password'
+    ) as mocked_can_validate:
+        with mock.patch(
+            'authentic2.views.IdentifierChangeMixin.has_recent_authentication'
+        ) as mocked_has_recent_authn:
+            mocked_can_validate.return_value = False
+            mocked_has_recent_authn.return_value = recent_authn
+            response = app.get('/accounts/change-email/')
+            if not recent_authn:
+                response = response.follow()
+                assert (
+                    response.pyquery('li.info')[0].text
+                    == 'You must re-authenticate to change your email address.'
+                )
+                response.form.set('username', user.username)
+                response.form.set('password', user.username)
+                response = response.form.submit(name='login-password-submit')
+                mocked_has_recent_authn.return_value = True
+                response = response.follow().maybe_follow()
+            response.form.set('email', email)
+            assert 'password' not in response.form.fields
+            response = response.form.submit()
+    assert len(mailoutbox) == l + 1
+    return mailoutbox[-1]
+
+
 def test_change_email(app, simple_user, user_ou1, mailoutbox):
     email = change_email(app, simple_user, user_ou1.email, mailoutbox)
     utils.assert_event(
@@ -78,6 +113,52 @@ def test_declare_first_email(app, nomail_user, user_ou1, mailoutbox):
     assert nomail_user.email == user_ou1.email
 
 
+def test_change_email_no_password(app, simple_user, user_ou1, mailoutbox):
+    email = change_email_no_password(app, simple_user, user_ou1.email, mailoutbox)
+    utils.assert_event(
+        'user.email.change.request',
+        user=simple_user,
+        session=app.session,
+        old_email=simple_user.email,
+        email=user_ou1.email,
+    )
+    link = utils.get_link_from_mail(email)
+    app.get(link)
+    utils.assert_event(
+        'user.email.change',
+        user=simple_user,
+        session=app.session,
+        old_email=simple_user.email,
+        email=user_ou1.email,
+    )
+    simple_user.refresh_from_db()
+    # ok it worked
+    assert simple_user.email == user_ou1.email
+
+
+def test_change_email_no_password_no_recent_authn(app, simple_user, user_ou1, mailoutbox):
+    email = change_email_no_password(app, simple_user, user_ou1.email, mailoutbox, recent_authn=False)
+    utils.assert_event(
+        'user.email.change.request',
+        user=simple_user,
+        session=app.session,
+        old_email=simple_user.email,
+        email=user_ou1.email,
+    )
+    link = utils.get_link_from_mail(email)
+    app.get(link)
+    utils.assert_event(
+        'user.email.change',
+        user=simple_user,
+        session=app.session,
+        old_email=simple_user.email,
+        email=user_ou1.email,
+    )
+    simple_user.refresh_from_db()
+    # ok it worked
+    assert simple_user.email == user_ou1.email
+
+
 def test_change_email_email_is_unique(app, settings, simple_user, user_ou1, mailoutbox):
     settings.A2_EMAIL_IS_UNIQUE = True
     email = change_email(app, simple_user, user_ou1.email, mailoutbox)

tests/test_change_phone.py

@@ -14,6 +14,8 @@
 # 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 unittest import mock
+
 from httmock import HTTMock, remember_called, urlmatch
 
 from authentic2.models import Attribute, SMSCode, Token
@@ -72,6 +74,105 @@ def test_change_phone(app, nomail_user, user_ou1, phone_activated_authn, setting
     assert nomail_user.phone_verified_on is not None
 
 
+def test_change_phone_no_password(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()
+
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/',
+        password=nomail_user.username,
+    )
+    with mock.patch(
+        'authentic2.views.IdentifierChangeMixin.can_validate_with_password'
+    ) as mocked_can_validate:
+        with mock.patch(
+            'authentic2.views.IdentifierChangeMixin.has_recent_authentication'
+        ) as mocked_has_recent_authn:
+            mocked_can_validate.return_value = False
+            mocked_has_recent_authn.return_value = True
+            resp = app.get('/accounts/change-phone/')
+            assert 'Your current phone number is +33122446688.' in resp.text
+            resp.form.set('phone_1', '122446666')
+            assert 'password' not in resp.form.fields
+            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_no_password_no_recent_authn(
+    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()
+
+    settings.SMS_URL = 'https://foo.whatever.none/'
+
+    resp = login(
+        app,
+        nomail_user,
+        login=nomail_user.attributes.phone,
+        path='/accounts/',
+        password=nomail_user.username,
+    )
+    with mock.patch(
+        'authentic2.views.IdentifierChangeMixin.can_validate_with_password'
+    ) as mocked_can_validate:
+        with mock.patch(
+            'authentic2.views.IdentifierChangeMixin.has_recent_authentication'
+        ) as mocked_has_recent_authn:
+            mocked_can_validate.return_value = False
+            mocked_has_recent_authn.return_value = False
+            resp = app.get('/accounts/change-phone/')
+            resp = resp.follow()
+            assert resp.pyquery('li.info')[0].text == 'You must re-authenticate to change your phone number.'
+            resp.form.set('username', nomail_user.phone_identifier)
+            resp.form.set('password', nomail_user.username)
+            resp = resp.form.submit(name='login-password-submit')
+            mocked_has_recent_authn.return_value = True
+            resp = resp.follow().maybe_follow()
+            resp.form.set('phone_1', '122446666')
+            assert 'Your current phone number is +33122446688.' in resp.text
+            assert 'password' not in resp.form.fields
+            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',