Merge ~maxiberta/canonical-identity-provider:skip-username-validation-if-unchanged into canonical-identity-provider:master

[Maximiliano Bertacchini]

Skip username validation in EditAccountForm if unchanged

The introduction of detailed username validation errors added form field validators on the username field. But, these validators run *before* the form's custom clean_username(), which contains the logic for skipping if unchanged.

https://docs.djangoproject.com/en/1.11/ref/forms/validation/#form-and-field-validation

[Daniel Manrique (roadmr)]

+1, I think this was my fault right? :(

review: Approve

6e67aed... by Maximiliano Bertacchini on 2021-02-03

Fix new username validation test

diff --git a/src/identityprovider/forms.py b/src/identityprovider/forms.py
index a23f2ba..61ab346 100644
--- a/src/identityprovider/forms.py
+++ b/src/identityprovider/forms.py
@@ -374,8 +374,7 @@ class EditAccountForm(ResetPasswordFormMixin, forms.ModelForm):
                 placeholder = _('Verify your email address first')
             self.fields['username'] = UsernameField(
                 required=False, placeholder=placeholder,
-                disabled=disabled, extra_class="p-form-validation__input",
-                validators=get_username_validators())
+                disabled=disabled, extra_class="p-form-validation__input")
             if self.instance.person is not None or require_username:
                 self.fields['username'].required = True
 
@@ -426,8 +425,19 @@ class EditAccountForm(ResetPasswordFormMixin, forms.ModelForm):
         # are now considered "distasteful".
         if username == self.instance.person_name:
             return username
-        if not valid_username(username):
-            raise forms.ValidationError(INVALID_USERNAME_MSG % username)
+
+        errors = []
+        for validator in USERNAME_VALIDATORS:
+            try:
+                validator.__call__(username)
+            except forms.ValidationError as ex:
+                errors.append(ex)
+        if errors:
+            if gargoyle.is_active('USERNAME_DETAILED_ERRORS'):
+                raise forms.ValidationError(errors)
+            else:
+                raise forms.ValidationError(INVALID_USERNAME_MSG % username)
+
         if username and username != self.instance.person_name:
             # The username is going to change, so ask LP to do a dry
             # run to see if it will fail.
diff --git a/src/identityprovider/tests/test_forms.py b/src/identityprovider/tests/test_forms.py
index 9ffa2e3..6b2c4a6 100644
--- a/src/identityprovider/tests/test_forms.py
+++ b/src/identityprovider/tests/test_forms.py
@@ -740,6 +740,21 @@ class EditAccountFormTestCase(FormBaseTestCase):
                 ['Capital letters are not allowed.',
                  'Must contain only lowercase letters, hyphens and numbers.']})
 
+    @switches(USERNAME_DETAILED_ERRORS=True)
+    def test_username_validation_skipped_if_unchanged(self):
+        self.factory.make_person(name='x', account=self.account)
+        data = {
+            'username': 'x',
+            'oldpassword': DEFAULT_USER_PASSWORD,
+            'password': 'testingpass',
+            'passwordconfirm': 'testingpass',
+            'displayname': self.account.displayname,
+            'preferred_email': self.account.preferredemail.id,
+        }
+        form = EditAccountForm(
+            data=data, instance=self.account, enable_username=True)
+        self.assertTrue(form.is_valid())
+
 
 class EditAccountFormDevicePrefsEnabledTestCase(EditAccountFormTestCase):