Apply same username restrictions during SSO pipeline

Apply same username restrictions during SSO pipeline as we have user registeration flow to avoid SSO flow breakage at the time of user creation.
ENT-2730

Code quality fixes

common/djangoapps/third_party_auth/pipeline.py

@@ -85,6 +85,7 @@ from edxmako.shortcuts import render_to_string
 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_authn import cookies as user_authn_cookies
 from third_party_auth.utils import user_exists
 from track import segment
@@ -850,7 +851,10 @@ def set_id_verification_status(auth_entry, strategy, details, user=None, *args,
 
 def get_username(strategy, details, backend, user=None, *args, **kwargs):
     """
-    Copy of social_core.pipeline.user.get_username with additional logging and case insensitive username checks.
+    Copy of social_core.pipeline.user.get_username to achieve
+    1. additional logging
+    2. case insensitive username checks
+    3. enforce same maximum and minimum length restrictions we have in `user_api/accounts`
     """
     if 'username' not in backend.setting('USER_FIELDS', USER_FIELDS):
         return
@@ -859,7 +863,8 @@ def get_username(strategy, details, backend, user=None, *args, **kwargs):
     if not user:
         email_as_username = strategy.setting('USERNAME_IS_FULL_EMAIL', False)
         uuid_length = strategy.setting('UUID_LENGTH', 16)
-        max_length = storage.user.username_max_length()
+        min_length = strategy.setting('USERNAME_MIN_LENGTH', accounts.USERNAME_MIN_LENGTH)
+        max_length = strategy.setting('USERNAME_MAX_LENGTH', accounts.USERNAME_MAX_LENGTH)
         do_slugify = strategy.setting('SLUGIFY_USERNAMES', False)
         do_clean = strategy.setting('CLEAN_USERNAMES', True)
 
@@ -898,7 +903,7 @@ def get_username(strategy, details, backend, user=None, *args, **kwargs):
         # username is cut to avoid any field max_length.
         # 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 user_exists({'username': final_username}):
+        while not final_username or len(final_username) < min_length or user_exists({'username': final_username}):
             username = short_username + uuid4().hex[:uuid_length]
             final_username = slug_func(clean_func(username[:max_length]))
             logger.info(u'[THIRD_PARTY_AUTH] New username generated. Username: {username}'.format(

common/djangoapps/third_party_auth/settings.py

@@ -81,6 +81,9 @@ def apply_settings(django_settings):
     # enable this when you want to get stack traces rather than redirections.
     django_settings.SOCIAL_AUTH_RAISE_EXCEPTIONS = False
 
+    # Clean username to make sure username is compatible with our system requirements
+    django_settings.SOCIAL_AUTH_CLEAN_USERNAME_FUNCTION = 'third_party_auth.models.clean_username'
+
     # Allow users to login using social auth even if their account is not verified yet
     # This is required since we [ab]use django's 'is_active' flag to indicate verified
     # accounts; without this set to True, python-social-auth won't allow us to link the

common/djangoapps/third_party_auth/tests/test_pipeline.py

@@ -9,6 +9,8 @@ import mock
 
 from third_party_auth import pipeline
 from third_party_auth.tests import testutil
+from third_party_auth.tests.specs.base import IntegrationTestMixin
+from third_party_auth.tests.specs.test_testshib import SamlIntegrationTestUtilities
 from third_party_auth.tests.testutil import simulate_running_pipeline
 
 
@@ -50,3 +52,49 @@ class ProviderUserStateTestCase(testutil.TestCase):
         with simulate_running_pipeline("third_party_auth.pipeline", backend_name, **kwargs):
             logout_url = pipeline.get_idp_logout_url_from_running_pipeline(request)
             self.assertEqual(idp_config['logout_url'], logout_url)
+
+
+@unittest.skipUnless(testutil.AUTH_FEATURE_ENABLED, testutil.AUTH_FEATURES_KEY + ' not enabled')
+@ddt.ddt
+class PipelineOverridesTest(SamlIntegrationTestUtilities, IntegrationTestMixin, testutil.SAMLTestCase):
+    """
+    Tests for pipeline overrides
+    """
+
+    def setUp(self):
+        super(PipelineOverridesTest, self).setUp()
+        self.enable_saml()
+        self.provider = self.configure_saml_provider(
+            enabled=True,
+            name="Test Provider",
+            slug='test',
+            backend_name='tpa-saml'
+        )
+
+    @ddt.data(
+        ('S', 'S9fe2', False),
+        ('S', 'S9fe2', True),
+        ('S.K', 'S_K', False),
+        ('S.K.', 'S_K_', False),
+        ('S.K.', 'S_K_9fe2', True),
+        ('usernamewithcharacterlengthofmorethan30chars', 'usernamewithcharacterlengthofm', False),
+        ('usernamewithcharacterlengthofmorethan30chars', 'usernamewithcharacterlengt9fe2', True),
+    )
+    @ddt.unpack
+    @mock.patch('third_party_auth.pipeline.user_exists')
+    def test_get_username_in_pipeline(self, idp_username, expected_username, already_exists, mock_user_exists):
+        """
+        Test get_username method of running pipeline
+        """
+        details = {
+            "username": idp_username,
+            "email": "test@example.com"
+        }
+        mock_user_exists.side_effect = [already_exists, False]
+        __, strategy = self.get_request_and_strategy()
+        with mock.patch('third_party_auth.pipeline.uuid4') as mock_uuid:
+            uuid4 = mock.Mock()
+            type(uuid4).hex = mock.PropertyMock(return_value='9fe2c4e93f654fdbb24c02b15259716c')
+            mock_uuid.return_value = uuid4
+            final_username = pipeline.get_username(strategy, details, self.provider.backend_class())
+            self.assertEqual(expected_username, final_username['username'])