From d5c85331c5b8e9f1042490b47d9bb95eac0d9b6e Mon Sep 17 00:00:00 2001 From: Uman Shahzad Date: Tue, 13 Jun 2017 16:20:41 +0500 Subject: [PATCH] Automatically populate additional fields for SSO scenarios. When authenticating using an SAML IdP, gather additional user data besides what is standard. Requires admin to input JSON in settings to recognize the additional user data. --- .../migrations/0011_auto_20170616_0112.py | 19 ++ common/djangoapps/third_party_auth/models.py | 41 ++- common/djangoapps/third_party_auth/saml.py | 30 +- .../third_party_auth/tests/specs/base.py | 19 +- .../third_party_auth/tests/testutil.py | 14 +- .../student_account/form_field.underscore | 2 +- openedx/core/djangoapps/user_api/helpers.py | 24 +- .../djangoapps/user_api/tests/test_helpers.py | 38 +++ .../djangoapps/user_api/tests/test_views.py | 282 +++++++++++------- openedx/core/djangoapps/user_api/views.py | 2 +- 10 files changed, 320 insertions(+), 151 deletions(-) create mode 100644 common/djangoapps/third_party_auth/migrations/0011_auto_20170616_0112.py diff --git a/common/djangoapps/third_party_auth/migrations/0011_auto_20170616_0112.py b/common/djangoapps/third_party_auth/migrations/0011_auto_20170616_0112.py new file mode 100644 index 0000000000..954205d321 --- /dev/null +++ b/common/djangoapps/third_party_auth/migrations/0011_auto_20170616_0112.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('third_party_auth', '0010_add_skip_hinted_login_dialog_field'), + ] + + operations = [ + migrations.AlterField( + model_name='samlproviderconfig', + name='other_settings', + field=models.TextField(help_text=b'For advanced use cases, enter a JSON object with addtional configuration. The tpa-saml backend supports {"requiredEntitlements": ["urn:..."]}, which can be used to require the presence of a specific eduPersonEntitlement, and {"extra_field_definitions": [{"name": "...", "urn": "..."},...]}, which can be used to define registration form fields and the URNs that can be used to retrieve the relevant values from the SAML response. Custom provider types, as selected in the "Identity Provider Type" field, may make use of the information stored in this field for additional configuration.', verbose_name=b'Advanced settings', blank=True), + ), + ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 1956a91036..15da50d3d6 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -31,6 +31,11 @@ from .saml import STANDARD_SAML_PROVIDER_KEY, get_saml_idp_choices, get_saml_idp log = logging.getLogger(__name__) +REGISTRATION_FORM_FIELD_BLACKLIST = [ + 'name', + 'username' +] + # A dictionary of {name: class} entries for each python-social-auth backend available. # Because this setting can specify arbitrary code to load and execute, it is set via @@ -241,8 +246,13 @@ class ProviderConfig(ConfigurationModel): values for that field. Where there is no value, the empty string must be used. """ + registration_form_data = {} + # Details about the user sent back from the provider. - details = pipeline_kwargs.get('details') + details = pipeline_kwargs.get('details').copy() + + # Set the registration form to use the `fullname` detail for the `name` field. + registration_form_data['name'] = details.get('fullname', '') # Get the username separately to take advantage of the de-duping logic # built into the pipeline. The provider cannot de-dupe because it can't @@ -250,13 +260,19 @@ 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. - suggested_username = pipeline_kwargs.get('username') + registration_form_data['username'] = pipeline_kwargs.get('username') - return { - 'email': details.get('email', ''), - 'name': details.get('fullname', ''), - 'username': suggested_username, - } + # Any other values that are present in the details dict should be copied + # into the registration form details. This may include details that do + # not map to a value that exists in the registration form. However, + # because the fields that are actually rendered are not based on this + # list, only those values that map to a valid registration form field + # will actually be sent to the form as default values. + for blacklisted_field in REGISTRATION_FORM_FIELD_BLACKLIST: + details.pop(blacklisted_field, None) + registration_form_data.update(details) + + return registration_form_data def get_authentication_backend(self): """Gets associated Django settings.AUTHENTICATION_BACKEND string.""" @@ -401,10 +417,13 @@ class SAMLProviderConfig(ProviderConfig): verbose_name="Advanced settings", blank=True, help_text=( 'For advanced use cases, enter a JSON object with addtional configuration. ' - 'The tpa-saml backend supports only {"requiredEntitlements": ["urn:..."]} ' - 'which can be used to require the presence of a specific eduPersonEntitlement. ' - 'Custom provider types, as selected in the "Identity Provider Type" field, may make ' - 'use of the information stored in this field for configuration.' + 'The tpa-saml backend supports {"requiredEntitlements": ["urn:..."]}, ' + 'which can be used to require the presence of a specific eduPersonEntitlement, ' + 'and {"extra_field_definitions": [{"name": "...", "urn": "..."},...]}, which can be ' + 'used to define registration form fields and the URNs that can be used to retrieve ' + 'the relevant values from the SAML response. Custom provider types, as selected ' + 'in the "Identity Provider Type" field, may make use of the information stored ' + 'in this field for additional configuration.' )) def clean(self): diff --git a/common/djangoapps/third_party_auth/saml.py b/common/djangoapps/third_party_auth/saml.py index 123bc1cb88..b27ec86695 100644 --- a/common/djangoapps/third_party_auth/saml.py +++ b/common/djangoapps/third_party_auth/saml.py @@ -100,9 +100,29 @@ class SAMLAuthBackend(SAMLAuth): # pylint: disable=abstract-method return SAMLConfiguration.current(Site.objects.get_current(get_current_request())) -class SapSuccessFactorsIdentityProvider(SAMLIdentityProvider): +class EdXSAMLIdentityProvider(SAMLIdentityProvider): """ - Customized version of SAMLIdentityProvider that knows how to retrieve user details + Customized version of SAMLIdentityProvider that can retrieve details beyond the standard + details supported by the canonical upstream version. + """ + + def get_user_details(self, attributes): + """ + Overrides `get_user_details` from the base class; retrieves those details, + then updates the dict with values from whatever additional fields are desired. + """ + details = super(EdXSAMLIdentityProvider, self).get_user_details(attributes) + extra_field_definitions = self.conf.get('extra_field_definitions', []) + details.update({ + field['name']: attributes[field['urn']][0] if field['urn'] in attributes else None + for field in extra_field_definitions + }) + return details + + +class SapSuccessFactorsIdentityProvider(EdXSAMLIdentityProvider): + """ + Customized version of EdXSAMLIdentityProvider that knows how to retrieve user details from the SAPSuccessFactors OData API, rather than parse them directly off the SAML assertion that we get in response to a login attempt. """ @@ -244,12 +264,12 @@ def get_saml_idp_class(idp_identifier_string): the SAMLIdentityProvider subclass able to handle requests for that type of identity provider. """ choices = { - STANDARD_SAML_PROVIDER_KEY: SAMLIdentityProvider, + STANDARD_SAML_PROVIDER_KEY: EdXSAMLIdentityProvider, SAP_SUCCESSFACTORS_SAML_KEY: SapSuccessFactorsIdentityProvider, } if idp_identifier_string not in choices: log.error( - '%s is not a valid SAMLIdentityProvider subclass; using SAMLIdentityProvider base class.', + '%s is not a valid EdXSAMLIdentityProvider subclass; using EdXSAMLIdentityProvider base class.', idp_identifier_string ) - return choices.get(idp_identifier_string, SAMLIdentityProvider) + return choices.get(idp_identifier_string, EdXSAMLIdentityProvider) diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 36c88b2c7e..385f875775 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -251,7 +251,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): self.assertEqual(302, response.status_code) self.assertTrue(response.has_header('Location')) - def assert_register_response_in_pipeline_looks_correct(self, response, pipeline_kwargs): + def assert_register_response_in_pipeline_looks_correct(self, response, pipeline_kwargs, required_fields): """Performs spot checks of the rendered register.html page. When we display the new account registration form after the user signs @@ -267,9 +267,10 @@ class IntegrationTest(testutil.TestCase, test.TestCase): self.assertIn('successfully signed in with %s' % self.provider.name, response.content) # Expect that each truthy value we've prepopulated the register form # with is actually present. - for prepopulated_form_value in self.provider.get_register_form_data(pipeline_kwargs).values(): - if prepopulated_form_value: - self.assertIn(prepopulated_form_value, response.content) + form_field_data = self.provider.get_register_form_data(pipeline_kwargs) + for prepopulated_form_data in form_field_data: + if prepopulated_form_data in required_fields: + self.assertIn(form_field_data[prepopulated_form_data], response.content) # Implementation details and actual tests past this point -- no more # configuration needed. @@ -823,7 +824,10 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # fire off the view that displays the registration form. with self._patch_edxmako_current_request(request): self.assert_register_response_in_pipeline_looks_correct( - student_views.register_user(strategy.request), pipeline.get(request)['kwargs']) + student_views.register_user(strategy.request), + pipeline.get(request)['kwargs'], + ['name', 'username', 'email'] + ) # Next, we invoke the view that handles the POST. Not all providers # supply email. Manually add it as the user would have to; this @@ -892,7 +896,10 @@ class IntegrationTest(testutil.TestCase, test.TestCase): with self._patch_edxmako_current_request(request): self.assert_register_response_in_pipeline_looks_correct( - student_views.register_user(strategy.request), pipeline.get(request)['kwargs']) + student_views.register_user(strategy.request), + pipeline.get(request)['kwargs'], + ['name', 'username', 'email'] + ) with self._patch_edxmako_current_request(strategy.request): strategy.request.POST = self.get_registration_post_vars() diff --git a/common/djangoapps/third_party_auth/tests/testutil.py b/common/djangoapps/third_party_auth/tests/testutil.py index b1c11bf0fe..1befeb0d29 100644 --- a/common/djangoapps/third_party_auth/tests/testutil.py +++ b/common/djangoapps/third_party_auth/tests/testutil.py @@ -25,7 +25,7 @@ from third_party_auth.models import ( SAMLConfiguration, SAMLProviderConfig ) -from third_party_auth.saml import SAMLIdentityProvider, get_saml_idp_class +from third_party_auth.saml import EdXSAMLIdentityProvider, get_saml_idp_class AUTH_FEATURES_KEY = 'ENABLE_THIRD_PARTY_AUTH' AUTH_FEATURE_ENABLED = AUTH_FEATURES_KEY in settings.FEATURES @@ -219,14 +219,14 @@ class SAMLTestCase(TestCase): error_mock = log_mock.error idp_class = get_saml_idp_class('fake_idp_class_option') error_mock.assert_called_once_with( - '%s is not a valid SAMLIdentityProvider subclass; using SAMLIdentityProvider base class.', + '%s is not a valid EdXSAMLIdentityProvider subclass; using EdXSAMLIdentityProvider base class.', 'fake_idp_class_option' ) - self.assertIs(idp_class, SAMLIdentityProvider) + self.assertIs(idp_class, EdXSAMLIdentityProvider) @contextmanager -def simulate_running_pipeline(pipeline_target, backend, email=None, fullname=None, username=None): +def simulate_running_pipeline(pipeline_target, backend, email=None, fullname=None, username=None, **kwargs): """Simulate that a pipeline is currently running. You can use this context manager to test packages that rely on third party auth. @@ -269,6 +269,9 @@ def simulate_running_pipeline(pipeline_target, backend, email=None, fullname=Non app generates itself and should be available by the time the user is authenticating with a third-party provider. + kwargs (dict): If provided, simulate that the current provider has + included additional user details (useful for filling in the registration form). + Returns: None @@ -276,9 +279,10 @@ def simulate_running_pipeline(pipeline_target, backend, email=None, fullname=Non pipeline_data = { "backend": backend, "kwargs": { - "details": {} + "details": kwargs } } + if email is not None: pipeline_data["kwargs"]["details"]["email"] = email if fullname is not None: diff --git a/lms/templates/student_account/form_field.underscore b/lms/templates/student_account/form_field.underscore index 3dcf58fdfb..93ab3fa216 100644 --- a/lms/templates/student_account/form_field.underscore +++ b/lms/templates/student_account/form_field.underscore @@ -26,7 +26,7 @@ } %> <% if ( required ) { %> aria-required="true" required<% } %>> <% _.each(options, function(el) { %> - + <% }); %> <% if ( instructions ) { %> <%= instructions %><% } %> diff --git a/openedx/core/djangoapps/user_api/helpers.py b/openedx/core/djangoapps/user_api/helpers.py index 92a7bb7a8b..47df7da3c3 100644 --- a/openedx/core/djangoapps/user_api/helpers.py +++ b/openedx/core/djangoapps/user_api/helpers.py @@ -234,21 +234,29 @@ class FormDescription(object): "supplementalText": supplementalText } + field_override = self._field_overrides.get(name, {}) + if field_type == "select": if options is not None: field_dict["options"] = [] - # Include an empty "default" option at the beginning of the list + # Get an existing default value from the field override + existing_default_value = field_override.get('defaultValue') + + # Include an empty "default" option at the beginning of the list; + # preselect it if there isn't an overriding default. if include_default_option: field_dict["options"].append({ "value": "", "name": "--", - "default": True + "default": existing_default_value is None }) - field_dict["options"].extend([ - {"value": option_value, "name": option_name} - for option_value, option_name in options + { + 'value': option_value, + 'name': option_name, + 'default': option_value == existing_default_value + } for option_value, option_name in options ]) else: raise InvalidFieldError("You must provide options for a select field.") @@ -270,7 +278,7 @@ class FormDescription(object): # If there are overrides for this field, apply them now. # Any field property can be overwritten (for example, the default value or placeholder) - field_dict.update(self._field_overrides.get(name, {})) + field_dict.update(field_override) self.fields.append(field_dict) @@ -291,8 +299,8 @@ class FormDescription(object): "placeholder": "", "instructions": "", "options": [ - {"value": "cheese", "name": "Cheese"}, - {"value": "wine", "name": "Wine"} + {"value": "cheese", "name": "Cheese", "default": False}, + {"value": "wine", "name": "Wine", "default": False} ] "restrictions": {}, "errorMessages": {}, diff --git a/openedx/core/djangoapps/user_api/tests/test_helpers.py b/openedx/core/djangoapps/user_api/tests/test_helpers.py index 00576d588e..2e4cc66fb0 100644 --- a/openedx/core/djangoapps/user_api/tests/test_helpers.py +++ b/openedx/core/djangoapps/user_api/tests/test_helpers.py @@ -147,6 +147,44 @@ class FormDescriptionTest(TestCase): with self.assertRaises(InvalidFieldError): desc.add_field("name", field_type="text", restrictions={"invalid": 0}) + def test_option_overrides(self): + desc = FormDescription("post", "/submit") + field = { + "name": "country", + "label": "Country", + "field_type": "select", + "default": "PK", + "required": True, + "error_messages": { + "required": "You must provide a value!" + }, + "options": [ + ("US", "United States of America"), + ("PK", "Pakistan") + ] + } + desc.override_field_properties( + field["name"], + default="PK" + ) + desc.add_field(**field) + self.assertEqual( + desc.fields[0]["options"], + [ + { + 'default': False, + 'name': 'United States of America', + 'value': 'US' + }, + { + 'default': True, + 'name': 'Pakistan', + 'value': 'PK' + } + + ] + ) + @ddt.ddt class StudentViewShimTest(TestCase): diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index a928cad50e..40e283107f 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -31,6 +31,7 @@ from third_party_auth.tests.utils import ( from .test_helpers import TestCaseForm from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from ..helpers import FormDescription from ..accounts import ( NAME_MAX_LENGTH, EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH, USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH @@ -873,8 +874,6 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): u"restrictions": { 'min_length': PASSWORD_MIN_LENGTH, 'max_length': PASSWORD_MAX_LENGTH - # 'min_length': account_api.PASSWORD_MIN_LENGTH, - # 'max_length': account_api.PASSWORD_MAX_LENGTH }, } ) @@ -936,35 +935,35 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): } ) - def test_register_form_third_party_auth_running(self): + def test_register_form_third_party_auth_running_google(self): no_extra_fields_setting = {} + country_options = ( + [ + { + "name": "--", + "value": "", + "default": False + } + ] + [ + { + "value": country_code, + "name": unicode(country_name), + "default": True if country_code == "PK" else False + } + for country_code, country_name in SORTED_COUNTRIES + ] + ) - self.configure_google_provider(enabled=True) + provider = self.configure_google_provider(enabled=True) with simulate_running_pipeline( - "openedx.core.djangoapps.user_api.views.third_party_auth.pipeline", - "google-oauth2", email="bob@example.com", - fullname="Bob", username="Bob123" + "openedx.core.djangoapps.user_api.views.third_party_auth.pipeline", "google-oauth2", + email="bob@example.com", + fullname="Bob", + username="Bob123", + country="PK" ): - # Password field should be hidden - self._assert_reg_field( - no_extra_fields_setting, - { - "name": "password", - "type": "hidden", - "required": False, - } - ) - # social_auth_provider should be present - # with value `Google`(we are setting up google provider for this test). - self._assert_reg_field( - no_extra_fields_setting, - { - "name": "social_auth_provider", - "type": "hidden", - "required": False, - "defaultValue": "Google" - } - ) + self._assert_password_field_hidden(no_extra_fields_setting) + self._assert_social_auth_provider_present(no_extra_fields_setting, provider) # Email should be filled in self._assert_reg_field( @@ -1020,6 +1019,22 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): } ) + # Country should be filled in. + self._assert_reg_field( + {u"country": u"required"}, + { + u"label": u"Country", + u"name": u"country", + u"defaultValue": u"PK", + u"type": u"select", + u"required": True, + u"options": country_options, + u"errorMessages": { + u"required": u"Please select your Country." + }, + } + ) + def test_register_form_level_of_education(self): self._assert_reg_field( {"level_of_education": "optional"}, @@ -1030,15 +1045,15 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "label": "Highest level of education completed", "options": [ {"value": "", "name": "--", "default": True}, - {"value": "p", "name": "Doctorate"}, - {"value": "m", "name": "Master's or professional degree"}, - {"value": "b", "name": "Bachelor's degree"}, - {"value": "a", "name": "Associate degree"}, - {"value": "hs", "name": "Secondary/high school"}, - {"value": "jhs", "name": "Junior secondary/junior high/middle school"}, - {"value": "el", "name": "Elementary/primary school"}, - {"value": "none", "name": "No formal education"}, - {"value": "other", "name": "Other education"}, + {"value": "p", "name": "Doctorate", "default": False}, + {"value": "m", "name": "Master's or professional degree", "default": False}, + {"value": "b", "name": "Bachelor's degree", "default": False}, + {"value": "a", "name": "Associate degree", "default": False}, + {"value": "hs", "name": "Secondary/high school", "default": False}, + {"value": "jhs", "name": "Junior secondary/junior high/middle school", "default": False}, + {"value": "el", "name": "Elementary/primary school", "default": False}, + {"value": "none", "name": "No formal education", "default": False}, + {"value": "other", "name": "Other education", "default": False}, ], } ) @@ -1056,15 +1071,15 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "label": "Highest level of education completed TRANSLATED", "options": [ {"value": "", "name": "--", "default": True}, - {"value": "p", "name": "Doctorate TRANSLATED"}, - {"value": "m", "name": "Master's or professional degree TRANSLATED"}, - {"value": "b", "name": "Bachelor's degree TRANSLATED"}, - {"value": "a", "name": "Associate degree TRANSLATED"}, - {"value": "hs", "name": "Secondary/high school TRANSLATED"}, - {"value": "jhs", "name": "Junior secondary/junior high/middle school TRANSLATED"}, - {"value": "el", "name": "Elementary/primary school TRANSLATED"}, - {"value": "none", "name": "No formal education TRANSLATED"}, - {"value": "other", "name": "Other education TRANSLATED"}, + {"value": "p", "name": "Doctorate TRANSLATED", "default": False}, + {"value": "m", "name": "Master's or professional degree TRANSLATED", "default": False}, + {"value": "b", "name": "Bachelor's degree TRANSLATED", "default": False}, + {"value": "a", "name": "Associate degree TRANSLATED", "default": False}, + {"value": "hs", "name": "Secondary/high school TRANSLATED", "default": False}, + {"value": "jhs", "name": "Junior secondary/junior high/middle school TRANSLATED", "default": False}, + {"value": "el", "name": "Elementary/primary school TRANSLATED", "default": False}, + {"value": "none", "name": "No formal education TRANSLATED", "default": False}, + {"value": "other", "name": "Other education TRANSLATED", "default": False}, ], } ) @@ -1079,9 +1094,9 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "label": "Gender", "options": [ {"value": "", "name": "--", "default": True}, - {"value": "m", "name": "Male"}, - {"value": "f", "name": "Female"}, - {"value": "o", "name": "Other/Prefer Not to Say"}, + {"value": "m", "name": "Male", "default": False}, + {"value": "f", "name": "Female", "default": False}, + {"value": "o", "name": "Other/Prefer Not to Say", "default": False}, ], } ) @@ -1099,9 +1114,9 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "label": "Gender TRANSLATED", "options": [ {"value": "", "name": "--", "default": True}, - {"value": "m", "name": "Male TRANSLATED"}, - {"value": "f", "name": "Female TRANSLATED"}, - {"value": "o", "name": "Other/Prefer Not to Say TRANSLATED"}, + {"value": "m", "name": "Male TRANSLATED", "default": False}, + {"value": "f", "name": "Female TRANSLATED", "default": False}, + {"value": "o", "name": "Other/Prefer Not to Say TRANSLATED", "default": False}, ], } ) @@ -1109,8 +1124,18 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): def test_register_form_year_of_birth(self): this_year = datetime.datetime.now(UTC).year year_options = ( - [{"value": "", "name": "--", "default": True}] + [ - {"value": unicode(year), "name": unicode(year)} + [ + { + "value": "", + "name": "--", + "default": True + } + ] + [ + { + "value": unicode(year), + "name": unicode(year), + "default": False + } for year in range(this_year, this_year - 120, -1) ] ) @@ -1173,9 +1198,18 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): def test_registration_form_country(self): country_options = ( - [{"name": "--", "value": "", "default": True}] + [ - {"value": country_code, "name": unicode(country_name)} + { + "name": "--", + "value": "", + "default": True + } + ] + [ + { + "value": country_code, + "name": unicode(country_name), + "default": False + } for country_code, country_name in SORTED_COUNTRIES ] ) @@ -1820,63 +1854,6 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): } ) - def _assert_reg_field(self, extra_fields_setting, expected_field): - """Retrieve the registration form description from the server and - verify that it contains the expected field. - - Args: - extra_fields_setting (dict): Override the Django setting controlling - which extra fields are displayed in the form. - - expected_field (dict): The field definition we expect to find in the form. - - Raises: - AssertionError - - """ - # Add in fields that are always present - defaults = [ - ("label", ""), - ("instructions", ""), - ("placeholder", ""), - ("defaultValue", ""), - ("restrictions", {}), - ("errorMessages", {}), - ] - for key, value in defaults: - if key not in expected_field: - expected_field[key] = value - - # Retrieve the registration form description - with override_settings(REGISTRATION_EXTRA_FIELDS=extra_fields_setting): - response = self.client.get(self.url) - self.assertHttpOK(response) - - # Verify that the form description matches what we'd expect - form_desc = json.loads(response.content) - - # Search the form for this field - actual_field = None - for field in form_desc["fields"]: - if field["name"] == expected_field["name"]: - actual_field = field - break - - self.assertIsNot( - actual_field, None, - msg="Could not find field {name}".format(name=expected_field["name"]) - ) - - for key, value in expected_field.iteritems(): - self.assertEqual( - expected_field[key], actual_field[key], - msg=u"Expected {expected} for {key} but got {actual} instead".format( - key=key, - expected=expected_field[key], - actual=actual_field[key] - ) - ) - def test_country_overrides(self): """Test that overridden countries are available in country list.""" # Retrieve the registration form description @@ -1904,6 +1881,84 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): response = self.client.post(self.url, {"email": self.EMAIL, "username": self.USERNAME}) self.assertEqual(response.status_code, 403) + def _assert_fields_match(self, actual_field, expected_field): + self.assertIsNot( + actual_field, None, + msg="Could not find field {name}".format(name=expected_field["name"]) + ) + + for key, value in expected_field.iteritems(): + self.assertEqual( + actual_field[key], expected_field[key], + msg=u"Expected {expected} for {key} but got {actual} instead".format( + key=key, + actual=actual_field[key], + expected=expected_field[key] + ) + ) + + def _populate_always_present_fields(self, field): + defaults = [ + ("label", ""), + ("instructions", ""), + ("placeholder", ""), + ("defaultValue", ""), + ("restrictions", {}), + ("errorMessages", {}), + ] + field.update({ + key: value + for key, value in defaults if key not in field + }) + + def _assert_reg_field(self, extra_fields_setting, expected_field): + """Retrieve the registration form description from the server and + verify that it contains the expected field. + + Args: + extra_fields_setting (dict): Override the Django setting controlling + which extra fields are displayed in the form. + + expected_field (dict): The field definition we expect to find in the form. + + Raises: + AssertionError + + """ + # Add in fields that are always present + self._populate_always_present_fields(expected_field) + + # Retrieve the registration form description + with override_settings(REGISTRATION_EXTRA_FIELDS=extra_fields_setting): + response = self.client.get(self.url) + self.assertHttpOK(response) + + # Verify that the form description matches what we'd expect + form_desc = json.loads(response.content) + + actual_field = None + for field in form_desc["fields"]: + if field["name"] == expected_field["name"]: + actual_field = field + break + + self._assert_fields_match(actual_field, expected_field) + + def _assert_password_field_hidden(self, field_settings): + self._assert_reg_field(field_settings, { + "name": "password", + "type": "hidden", + "required": False + }) + + def _assert_social_auth_provider_present(self, field_settings, backend): + self._assert_reg_field(field_settings, { + "name": "social_auth_provider", + "type": "hidden", + "required": False, + "defaultValue": backend.name + }) + @httpretty.activate @ddt.ddt @@ -1931,8 +1986,7 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe "country": "US", "username": user.username if user else "test_username", "name": user.first_name if user else "test name", - "email": user.email if user else "test@test.com", - + "email": user.email if user else "test@test.com" } def _assert_existing_user_error(self, response): diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index 493431254a..6e0c95c54c 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -924,7 +924,7 @@ class RegistrationView(APIView): running_pipeline.get('kwargs') ) - for field_name in self.DEFAULT_FIELDS: + for field_name in self.DEFAULT_FIELDS + self.EXTRA_FIELDS: if field_name in field_overrides: form_desc.override_field_properties( field_name, default=field_overrides[field_name]