Merge pull request #15305 from open-craft/uman/OC-2682

[ENT-423] Automatically populate Country field for SSO scenarios

common/djangoapps/third_party_auth/migrations/0011_auto_20170616_0112.py

+# -*- 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),
+        ),
+    ]

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):

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)

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 <strong>%s</strong>' % 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()

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:

lms/templates/student_account/form_field.underscore

@@ -26,7 +26,7 @@
             } %>
             <% if ( required ) { %> aria-required="true" required<% } %>>
         <% _.each(options, function(el) { %>
-            <option value="<%= el.value%>"<% if ( el.default ) { %> data-isdefault="true"<% } %>><%= el.name %></option>
+            <option value="<%= el.value%>"<% if ( el.default ) { %> data-isdefault="true" selected<% } %>><%= el.name %></option>
         <% }); %>
         </select>
         <% if ( instructions ) { %> <span class="tip tip-input" id="<%= form %>-<%= name %>-desc"><%= instructions %></span><% } %>

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": {},

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):

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]