diff --git a/cms/envs/test.py b/cms/envs/test.py index f33f0f3e76..4875b8447b 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -36,6 +36,7 @@ from lms.envs.test import ( MEDIA_URL, COMPREHENSIVE_THEME_DIRS, JWT_AUTH, + REGISTRATION_EXTRA_FIELDS, ) # mongo connection settings diff --git a/common/djangoapps/third_party_auth/saml.py b/common/djangoapps/third_party_auth/saml.py index 9b4ea285cb..568e785920 100644 --- a/common/djangoapps/third_party_auth/saml.py +++ b/common/djangoapps/third_party_auth/saml.py @@ -7,6 +7,7 @@ import requests from django.contrib.sites.models import Site from django.http import Http404 from django.utils.functional import cached_property +from django_countries import countries from social_core.backends.saml import OID_EDU_PERSON_ENTITLEMENT, SAMLAuth, SAMLIdentityProvider from social_core.exceptions import AuthForbidden @@ -134,6 +135,77 @@ class SapSuccessFactorsIdentityProvider(EdXSAMLIdentityProvider): 'odata_client_id', ) + # Define the relationships between SAPSF record fields and Open edX logistration fields. + default_field_mapping = { + 'username': 'username', + 'firstName': 'first_name', + 'lastName': 'last_name', + 'defaultFullName': 'fullname', + 'email': 'email', + 'country': 'country', + 'city': 'city', + } + + # Define a simple mapping to relate SAPSF values to Open edX-compatible values for + # any given field. By default, this only contains the Country field, as SAPSF supplies + # a country name, which has to be translated to a country code. + default_value_mapping = { + 'country': {name: code for code, name in countries} + } + + # Unfortunately, not everything has a 1:1 name mapping between Open edX and SAPSF, so + # we need some overrides. TODO: Fill in necessary mappings + default_value_mapping.update({ + 'United States': 'US', + }) + + def get_registration_fields(self, response): + """ + Get a dictionary mapping registration field names to default values. + """ + field_mapping = self.field_mappings + registration_fields = {edx_name: response['d'].get(odata_name, '') for odata_name, edx_name in field_mapping.items()} + value_mapping = self.value_mappings + for field, value in registration_fields.items(): + if field in value_mapping and value in value_mapping[field]: + registration_fields[field] = value_mapping[field][value] + return registration_fields + + @property + def field_mappings(self): + """ + Get a dictionary mapping the field names returned in an SAP SuccessFactors + user entity to the field names with which those values should be used in + the Open edX registration form. + """ + overrides = self.conf.get('sapsf_field_mappings', {}) + base = self.default_field_mapping.copy() + base.update(overrides) + return base + + @property + def value_mappings(self): + """ + Get a dictionary mapping of field names to override objects which each + map values received from SAP SuccessFactors to values expected in the + Open edX platform registration form. + """ + overrides = self.conf.get('sapsf_value_mappings', {}) + base = self.default_value_mapping.copy() + for field, override in overrides.items(): + if field in base: + base[field].update(override) + else: + base[field] = override[field] + return base + + @property + def timeout(self): + """ + The number of seconds OData API requests should wait for a response before failing. + """ + return self.conf.get('odata_api_request_timeout', 10) + @property def sapsf_idp_url(self): return self.conf['sapsf_oauth_root_url'] + 'idp' @@ -187,7 +259,7 @@ class SapSuccessFactorsIdentityProvider(EdXSAMLIdentityProvider): 'token_url': self.sapsf_token_url, 'private_key': self.sapsf_private_key, }, - timeout=10, + timeout=self.timeout, ) assertion.raise_for_status() assertion = assertion.text @@ -199,7 +271,7 @@ class SapSuccessFactorsIdentityProvider(EdXSAMLIdentityProvider): 'grant_type': 'urn:ietf:params:oauth:grant-type:saml2-bearer', 'assertion': assertion, }, - timeout=10, + timeout=self.timeout, ) token.raise_for_status() token = token.json()['access_token'] @@ -220,12 +292,14 @@ class SapSuccessFactorsIdentityProvider(EdXSAMLIdentityProvider): username = details['username'] try: client = self.get_odata_api_client(user_id=username) + fields = ','.join(self.field_mappings) response = client.get( - '{root_url}User(userId=\'{user_id}\')?$select=username,firstName,lastName,defaultFullName,email'.format( + '{root_url}User(userId=\'{user_id}\')?$select={fields}'.format( root_url=self.odata_api_root_url, - user_id=username + user_id=username, + fields=fields, ), - timeout=10, + timeout=self.timeout, ) response.raise_for_status() response = response.json() @@ -237,13 +311,7 @@ class SapSuccessFactorsIdentityProvider(EdXSAMLIdentityProvider): self.odata_company_id, ) return details - return { - 'username': response['d']['username'], - 'first_name': response['d']['firstName'], - 'last_name': response['d']['lastName'], - 'fullname': response['d']['defaultFullName'], - 'email': response['d']['email'], - } + return self.get_registration_fields(response) def get_saml_idp_choices(): diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 8c32eb8363..ffea3e0efa 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -54,7 +54,7 @@ class IntegrationTestMixin(object): self.addCleanup(patcher.stop) # Override this method in a subclass and enable at least one provider. - def test_register(self): + def test_register(self, **extra_defaults): # The user goes to the register page, and sees a button to register with the provider: provider_register_url = self._check_register_page() # The user clicks on the Dummy button: @@ -76,6 +76,8 @@ class IntegrationTestMixin(object): self.assertEqual(form_fields['email']['defaultValue'], self.USER_EMAIL) self.assertEqual(form_fields['name']['defaultValue'], self.USER_NAME) self.assertEqual(form_fields['username']['defaultValue'], self.USER_USERNAME) + for field_name, value in extra_defaults.items(): + self.assertEqual(form_fields[field_name]['defaultValue'], value) registration_values = { 'email': 'email-edited@tpa-test.none', 'name': 'My Customized Name', diff --git a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py index 6224d48148..d80607309d 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py @@ -309,6 +309,7 @@ class SuccessFactorsIntegrationTest(SamlIntegrationTestUtilities, IntegrationTes 'lastName': 'Smith', 'defaultFullName': 'John Smith', 'email': 'john@smith.com', + 'country': 'Australia', } }) ) @@ -331,23 +332,119 @@ class SuccessFactorsIntegrationTest(SamlIntegrationTestUtilities, IntegrationTes self.USER_USERNAME = "myself" super(SuccessFactorsIntegrationTest, self).test_register() + @patch.dict('django.conf.settings.REGISTRATION_EXTRA_FIELDS', country='optional') def test_register_sapsf_metadata_present(self): """ Configure the provider such that it can talk to a mocked-out version of the SAP SuccessFactors API, and ensure that the data it gets that way gets passed to the registration form. + + Check that value mappings overrides work in cases where we override a value other than + what we're looking for, and when an empty override is provided (expected behavior is that + existing value maps will be left alone). """ + expected_country = 'AU' + provider_settings = { + 'sapsf_oauth_root_url': 'http://successfactors.com/oauth/', + 'sapsf_private_key': 'fake_private_key_here', + 'odata_api_root_url': 'http://api.successfactors.com/odata/v2/', + 'odata_company_id': 'NCC1701D', + 'odata_client_id': 'TatVotSEiCMteSNWtSOnLanCtBGwNhGB', + } + self._configure_testshib_provider( identity_provider_type='sap_success_factors', metadata_source=TESTSHIB_METADATA_URL, - other_settings=json.dumps({ - 'sapsf_oauth_root_url': 'http://successfactors.com/oauth/', - 'sapsf_private_key': 'fake_private_key_here', - 'odata_api_root_url': 'http://api.successfactors.com/odata/v2/', - 'odata_company_id': 'NCC1701D', - 'odata_client_id': 'TatVotSEiCMteSNWtSOnLanCtBGwNhGB', - }) + other_settings=json.dumps(provider_settings) ) - super(SuccessFactorsIntegrationTest, self).test_register() + super(SuccessFactorsIntegrationTest, self).test_register(country=expected_country) + + @patch.dict('django.conf.settings.REGISTRATION_EXTRA_FIELDS', country='optional') + def test_register_sapsf_metadata_present_override_relevant_value(self): + """ + Configure the provider such that it can talk to a mocked-out version of the SAP SuccessFactors + API, and ensure that the data it gets that way gets passed to the registration form. + + Check that value mappings overrides work in cases where we override a value other than + what we're looking for, and when an empty override is provided (expected behavior is that + existing value maps will be left alone). + """ + value_map = {'country': {'Australia': 'NZ'}} + expected_country = 'NZ' + provider_settings = { + 'sapsf_oauth_root_url': 'http://successfactors.com/oauth/', + 'sapsf_private_key': 'fake_private_key_here', + 'odata_api_root_url': 'http://api.successfactors.com/odata/v2/', + 'odata_company_id': 'NCC1701D', + 'odata_client_id': 'TatVotSEiCMteSNWtSOnLanCtBGwNhGB', + } + if value_map: + provider_settings['sapsf_value_mappings'] = value_map + + self._configure_testshib_provider( + identity_provider_type='sap_success_factors', + metadata_source=TESTSHIB_METADATA_URL, + other_settings=json.dumps(provider_settings) + ) + super(SuccessFactorsIntegrationTest, self).test_register(country=expected_country) + + @patch.dict('django.conf.settings.REGISTRATION_EXTRA_FIELDS', country='optional') + def test_register_sapsf_metadata_present_override_other_value(self): + """ + Configure the provider such that it can talk to a mocked-out version of the SAP SuccessFactors + API, and ensure that the data it gets that way gets passed to the registration form. + + Check that value mappings overrides work in cases where we override a value other than + what we're looking for, and when an empty override is provided (expected behavior is that + existing value maps will be left alone). + """ + value_map = {'country': {'United States': 'blahfake'}} + expected_country = 'AU' + provider_settings = { + 'sapsf_oauth_root_url': 'http://successfactors.com/oauth/', + 'sapsf_private_key': 'fake_private_key_here', + 'odata_api_root_url': 'http://api.successfactors.com/odata/v2/', + 'odata_company_id': 'NCC1701D', + 'odata_client_id': 'TatVotSEiCMteSNWtSOnLanCtBGwNhGB', + } + if value_map: + provider_settings['sapsf_value_mappings'] = value_map + + self._configure_testshib_provider( + identity_provider_type='sap_success_factors', + metadata_source=TESTSHIB_METADATA_URL, + other_settings=json.dumps(provider_settings) + ) + super(SuccessFactorsIntegrationTest, self).test_register(country=expected_country) + + @patch.dict('django.conf.settings.REGISTRATION_EXTRA_FIELDS', country='optional') + def test_register_sapsf_metadata_present_empty_value_override(self): + """ + Configure the provider such that it can talk to a mocked-out version of the SAP SuccessFactors + API, and ensure that the data it gets that way gets passed to the registration form. + + Check that value mappings overrides work in cases where we override a value other than + what we're looking for, and when an empty override is provided (expected behavior is that + existing value maps will be left alone). + """ + + value_map = {'country': {}} + expected_country = 'AU' + provider_settings = { + 'sapsf_oauth_root_url': 'http://successfactors.com/oauth/', + 'sapsf_private_key': 'fake_private_key_here', + 'odata_api_root_url': 'http://api.successfactors.com/odata/v2/', + 'odata_company_id': 'NCC1701D', + 'odata_client_id': 'TatVotSEiCMteSNWtSOnLanCtBGwNhGB', + } + if value_map: + provider_settings['sapsf_value_mappings'] = value_map + + self._configure_testshib_provider( + identity_provider_type='sap_success_factors', + metadata_source=TESTSHIB_METADATA_URL, + other_settings=json.dumps(provider_settings) + ) + super(SuccessFactorsIntegrationTest, self).test_register(country=expected_country) def test_register_http_failure(self): """