Merge pull request #18990 from edx/jeskew/PLAT_2146_add_username_retirement_check

Check for username retirement in validation endpoint.

common/djangoapps/student/helpers.py

@@ -44,7 +44,8 @@ from student.models import (
     UserAttribute,
     UserProfile,
     unique_id_for_user,
-    email_exists_or_retired
+    email_exists_or_retired,
+    username_exists_or_retired
 )
 
 
@@ -628,7 +629,7 @@ def do_create_account(form, custom_form=None):
         # AccountValidationError and a consistent user message returned (i.e. both should
         # return "It looks like {username} belongs to an existing account. Try again with a
         # different username.")
-        if User.objects.filter(username=user.username):
+        if username_exists_or_retired(user.username):
             raise AccountValidationError(
                 USERNAME_EXISTS_MSG_FMT.format(username=proposed_username),
                 field="username"

common/djangoapps/student/models.py

@@ -233,6 +233,13 @@ def is_username_retired(username):
         raise
 
 
+def username_exists_or_retired(username):
+    """
+    Check a username for existence -or- retirement against the User model.
+    """
+    return User.objects.filter(username=username).exists() or is_username_retired(username)
+
+
 def is_email_retired(email):
     """
     Checks to see if the given email has been previously retired

openedx/core/djangoapps/user_api/accounts/api.py

@@ -14,7 +14,7 @@ from django.http import HttpResponseForbidden
 from openedx.core.djangoapps.theming.helpers import get_current_request
 from six import text_type
 
-from student.models import User, UserProfile, Registration, email_exists_or_retired
+from student.models import User, UserProfile, Registration, email_exists_or_retired, username_exists_or_retired
 from student import forms as student_forms
 from student import views as student_views
 from util.model_utils import emit_setting_changed_event
@@ -695,7 +695,7 @@ def _validate_username_doesnt_exist(username):
     :return: None
     :raises: errors.AccountUsernameAlreadyExists
     """
-    if username is not None and User.objects.filter(username=username).exists():
+    if username is not None and username_exists_or_retired(username):
         raise errors.AccountUsernameAlreadyExists(_(accounts.USERNAME_CONFLICT_MSG).format(username=username))
 
 

openedx/core/djangoapps/user_api/tests/test_views.py

@@ -28,7 +28,7 @@ from openedx.core.lib.time_zone_utils import get_display_time_zone
 from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration
 from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
 from student.tests.factories import UserFactory
-from student.models import get_retired_email_by_email
+from student.models import get_retired_email_by_email, get_retired_username_by_username
 from third_party_auth.tests.testutil import simulate_running_pipeline, ThirdPartyAuthTestMixin
 from third_party_auth.tests.utils import (
     ThirdPartyOAuthTestMixin, ThirdPartyOAuthTestMixinFacebook, ThirdPartyOAuthTestMixinGoogle
@@ -811,6 +811,7 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa
         """
         user = User.objects.get(username=self.USERNAME)
         UserRetirementStatus.create_retirement(user)
+        user.username = get_retired_username_by_username(user.username)
         user.email = get_retired_email_by_email(user.email)
         user.set_unusable_password()
         user.save()
@@ -897,6 +898,48 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa
             }
         )
 
+    def test_register_duplicate_retired_username_account_validation_error(self):
+        # Register the first user
+        response = self.client.post(self.url, {
+            "email": self.EMAIL,
+            "name": self.NAME,
+            "username": self.USERNAME,
+            "password": self.PASSWORD,
+            "honor_code": "true",
+        })
+        self.assertHttpOK(response)
+
+        # Initiate retirement for the above user.
+        self._retireRequestUser()
+
+        with mock.patch('openedx.core.djangoapps.user_authn.views.register.do_create_account') as dummy_do_create_acct:
+            # do_create_account should *not* be called - the duplicate retired username
+            # should be detected by check_account_exists before account creation is called.
+            dummy_do_create_acct.side_effect = Exception('do_create_account should *not* have been called!')
+            # Try to create a second user with the same username.
+            response = self.client.post(self.url, {
+                "email": "someone+else@example.com",
+                "name": "Someone Else",
+                "username": self.USERNAME,
+                "password": self.PASSWORD,
+                "honor_code": "true",
+            })
+        self.assertEqual(response.status_code, 409)
+        response_json = json.loads(response.content)
+        self.assertEqual(
+            response_json,
+            {
+                "username": [{
+                    "user_message": (
+                        "It looks like {} belongs to an existing account. "
+                        "Try again with a different username."
+                    ).format(
+                        self.USERNAME
+                    )
+                }]
+            }
+        )
+
     @mock.patch('openedx.core.djangoapps.user_api.views.check_account_exists')
     def test_register_duplicate_email_validation_error(self, dummy_check_account_exists):
         dummy_check_account_exists.return_value = []