diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py
index 84ecefabec3e6e115bdd8c64c3089b014f7a9e21..18924f21ce894694e8a90170eeaf69bc8839bc38 100644
--- a/common/djangoapps/third_party_auth/pipeline.py
+++ b/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(
diff --git a/common/djangoapps/third_party_auth/settings.py b/common/djangoapps/third_party_auth/settings.py
index 0f6a4536c3f835e15b8a3d8f5fc93c191dc45ad5..b28f810df6d3fde52ec4b97bad440453b960eb3f 100644
--- a/common/djangoapps/third_party_auth/settings.py
+++ b/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
diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline.py b/common/djangoapps/third_party_auth/tests/test_pipeline.py
index 2374c4827e7d4fd819b828ba8d65c6f50f00ba1f..9613706cf65d1a5a78154668856f40bc2d333817 100644
--- a/common/djangoapps/third_party_auth/tests/test_pipeline.py
+++ b/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'])