From f844510b71725c64854663c2ccc9ec8e792c5839 Mon Sep 17 00:00:00 2001 From: Brittney Exline Date: Wed, 30 Aug 2017 13:34:52 -0400 Subject: [PATCH 1/2] ENT-619 Clean usernames coming from identity providers Since we have started integrating with more clients, we have found that the usernames that get passed are not compatible with our username restrictions. This PR introduces a function to clean usernames to make them compatible, particularly in the auto registration case. --- common/djangoapps/third_party_auth/models.py | 8 ++++++- .../djangoapps/user_api/tests/test_views.py | 21 ++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index af3e18a551..f7fa9a71fb 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/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 diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 4014bc088e..9753f20fca 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -1070,14 +1070,19 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): ) @ddt.data( - ('pk', 'PK'), - ('Pk', 'PK'), - ('pK', 'PK'), - ('PK', 'PK'), - ('us', 'US'), + ('pk', 'PK', 'Bob123', 'Bob123'), + ('Pk', 'PK', 'Bob123', 'Bob123'), + ('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 = ( [ @@ -1101,7 +1106,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) @@ -1147,7 +1152,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", From 716a608f764c243f524e4829ce3a193a5c60c2f5 Mon Sep 17 00:00:00 2001 From: Brittney Exline Date: Thu, 31 Aug 2017 10:56:07 -0400 Subject: [PATCH 2/2] adding suggested test case --- openedx/core/djangoapps/user_api/tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 9753f20fca..ffbb6b1750 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -1071,7 +1071,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): @ddt.data( ('pk', 'PK', 'Bob123', 'Bob123'), - ('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'),