feat: avoid certain strings in the suffix appended to edX usernames (#29524)

ENT-2824

common/djangoapps/third_party_auth/pipeline.py

@@ -87,6 +87,7 @@ from lms.djangoapps.verify_student.models import SSOVerification
 from lms.djangoapps.verify_student.utils import earliest_allowed_verification_date
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.user_api import accounts
+from openedx.core.djangoapps.user_api.accounts.utils import username_suffix_generator
 from openedx.core.djangoapps.user_authn import cookies as user_authn_cookies
 from openedx.core.djangoapps.user_authn.toggles import is_require_third_party_auth_enabled
 from common.djangoapps.third_party_auth.utils import (
@@ -1007,7 +1008,8 @@ def get_username(strategy, details, backend, user=None, *args, **kwargs):  # lin
         # The final_username may be empty and will skip the loop.
         # We are using our own version of user_exists to avoid possible case sensitivity issues.
         while not final_username or len(final_username) < min_length or user_exists({'username': final_username}):
-            username = short_username + uuid4().hex[:uuid_length]
+            # adding a dash between user-supplied and system-generated values to avoid weird combinations
+            username = short_username + '-' + username_suffix_generator(uuid_length)
             final_username = slug_func(clean_func(username[:max_length]))
             logger.info('[THIRD_PARTY_AUTH] New username generated. Username: {username}'.format(
                 username=final_username))

common/djangoapps/third_party_auth/tests/test_pipeline.py

@@ -72,13 +72,13 @@ class PipelineOverridesTest(SamlIntegrationTestUtilities, IntegrationTestMixin,
         )
 
     @ddt.data(
-        ('S', 'S9fe2', False),
-        ('S', 'S9fe2', True),
+        ('S', 'S-9fe2', False),
+        ('S', 'S-9fe2', True),
         ('S.K', 'S_K', False),
         ('S.K.', 'S_K', False),
-        ('S.K.', 'S_K_9fe2', True),
+        ('S.K.', 'S_K_-9fe2', True),
         ('usernamewithcharacterlengthofmorethan30chars', 'usernamewithcharacterlengthofm', False),
-        ('usernamewithcharacterlengthofmorethan30chars', 'usernamewithcharacterlengt9fe2', True),
+        ('usernamewithcharacterlengthofmorethan30chars', 'usernamewithcharacterlengt-9fe', True),
     )
     @ddt.unpack
     @mock.patch('common.djangoapps.third_party_auth.pipeline.user_exists')
@@ -92,9 +92,7 @@ class PipelineOverridesTest(SamlIntegrationTestUtilities, IntegrationTestMixin,
         }
         mock_user_exists.side_effect = [already_exists, False]
         __, strategy = self.get_request_and_strategy()
-        with mock.patch('common.djangoapps.third_party_auth.pipeline.uuid4') as mock_uuid:
-            uuid4 = mock.Mock()
-            type(uuid4).hex = mock.PropertyMock(return_value='9fe2c4e93f654fdbb24c02b15259716c')
-            mock_uuid.return_value = uuid4
+        with mock.patch('common.djangoapps.third_party_auth.pipeline.username_suffix_generator') as mock_suffix:
+            mock_suffix.return_value = '9fe2'
             final_username = pipeline.get_username(strategy, details, self.provider.backend_class())
             assert expected_username == final_username['username']

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

@@ -2,8 +2,9 @@
 Utility methods for the account settings.
 """
 
-
+import random
 import re
+import string
 from urllib.parse import urlparse  # pylint: disable=import-error
 
 import waffle  # lint-amnesty, pylint: disable=invalid-django-waffle-import
@@ -215,3 +216,18 @@ def create_retirement_request_and_deactivate_account(user):
     # Delete OAuth tokens associated with the user.
     retire_dot_oauth2_models(user)
     AccountRecovery.retire_recovery_email(user.id)
+
+
+def username_suffix_generator(suffix_length=4):
+    """
+    Generates a random, alternating number and letter string for the purpose of
+    appending to non-unique usernames. Alternating is less likey to produce
+    a significant/meaningful substring like an offensive word.
+    """
+    output = ''
+    for i in range(suffix_length):
+        if (i % 2) == 0:
+            output += random.choice(string.ascii_lowercase)
+        else:
+            output += random.choice(string.digits)
+    return output

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

@@ -7,7 +7,6 @@ https://openedx.atlassian.net/wiki/display/TNL/User+API
 
 import datetime
 import logging
-import uuid
 from functools import wraps
 
 import pytz
@@ -82,7 +81,7 @@ from .serializers import (
     UserSearchEmailSerializer
 )
 from .signals import USER_RETIRE_LMS_CRITICAL, USER_RETIRE_LMS_MISC, USER_RETIRE_MAILINGS
-from .utils import create_retirement_request_and_deactivate_account
+from .utils import create_retirement_request_and_deactivate_account, username_suffix_generator
 
 try:
     from coaching.api import has_ever_consented_to_coaching
@@ -1325,8 +1324,8 @@ class UsernameReplacementView(APIView):
         # Keep checking usernames in case desired_username + random suffix is already taken
         while True:
             if User.objects.filter(username=new_username).exists():
-                unique_suffix = uuid.uuid4().hex[:suffix_length]
-                new_username = desired_username + unique_suffix
+                # adding a dash between user-supplied and system-generated values to avoid weird combinations
+                new_username = desired_username + '-' + username_suffix_generator(suffix_length)
             else:
                 break
         return new_username