Merge pull request #15929 from edx/bexline/clean_saml_usernames

ENT-619 Clean usernames coming from identity providers

common/djangoapps/third_party_auth/models.py

@@ -7,6 +7,7 @@ from __future__ import absolute_import
 
 import json
 import logging
+import re
 
 from config_models.models import ConfigurationModel, cache
 from django.conf import settings
@@ -65,6 +66,11 @@ def clean_json(value, of_type):
     return json.dumps(value_python, indent=4)
 
 
+def clean_username(username=''):
+    """ Simple helper method to ensure a username is compatible with our system requirements. """
+    return re.sub(r'[^-\w]+', '_', username)[:30]
+
+
 class AuthNotConfigured(SocialAuthBaseException):
     """ Exception when SAMLProviderData or other required info is missing """
     def __init__(self, provider_name):
@@ -259,7 +265,7 @@ class ProviderConfig(ConfigurationModel):
         # technically a data race between the creation of this value and the
         # creation of the user object, so it is still possible for users to get
         # an error on submit.
-        registration_form_data['username'] = pipeline_kwargs.get('username')
+        registration_form_data['username'] = clean_username(pipeline_kwargs.get('username') or '')
 
         # Any other values that are present in the details dict should be copied
         # into the registration form details. This may include details that do

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

@@ -1117,14 +1117,19 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase):
         )
 
     @ddt.data(
-        ('pk', 'PK'),
-        ('Pk', 'PK'),
-        ('pK', 'PK'),
-        ('PK', 'PK'),
-        ('us', 'US'),
+        ('pk', 'PK', 'Bob123', 'Bob123'),
+        ('Pk', 'PK', None, ''),
+        ('pK', 'PK', 'Bob123@edx.org', 'Bob123_edx_org'),
+        ('PK', 'PK', 'Bob123123123123123123123123123123123123', 'Bob123123123123123123123123123'),
+        ('us', 'US', 'Bob-1231231&23123+1231(2312312312@3123123123', 'Bob-1231231_23123_1231_2312312'),
     )
     @ddt.unpack
-    def test_register_form_third_party_auth_running_google(self, input_country_code, expected_country_code):
+    def test_register_form_third_party_auth_running_google(
+            self,
+            input_country_code,
+            expected_country_code,
+            input_username,
+            expected_username):
         no_extra_fields_setting = {}
         country_options = (
             [
@@ -1148,7 +1153,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase):
             "openedx.core.djangoapps.user_api.api.third_party_auth.pipeline", "google-oauth2",
             email="bob@example.com",
             fullname="Bob",
-            username="Bob123",
+            username=input_username,
             country=input_country_code
         ):
             self._assert_password_field_hidden(no_extra_fields_setting)
@@ -1194,7 +1199,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase):
                 no_extra_fields_setting,
                 {
                     u"name": u"username",
-                    u"defaultValue": u"Bob123",
+                    u"defaultValue": expected_username,
                     u"type": u"text",
                     u"required": True,
                     u"label": u"Public Username",