diff --git a/common/djangoapps/third_party_auth/provider.py b/common/djangoapps/third_party_auth/provider.py index fe72bc6561..0bfb602c89 100644 --- a/common/djangoapps/third_party_auth/provider.py +++ b/common/djangoapps/third_party_auth/provider.py @@ -43,9 +43,23 @@ class Registry(object): return sorted(cls._enabled_providers(), key=lambda provider: provider.name) @classmethod - def displayed_for_login(cls): - """Returns list of providers that can be used to initiate logins in the UI""" - return [provider for provider in cls.enabled() if provider.display_for_login] + def displayed_for_login(cls, tpa_hint=None): + """ + Args: + tpa_hint (string): An override used in certain third-party authentication + scenarios that will cause the specified provider to be included in the + set along with any providers matching the 'display_for_login' criteria. + Note that 'provider_id' cannot have a value of None according to the + current implementation. + + Returns: + List of ProviderConfig entities + """ + return [ + provider + for provider in cls.enabled() + if provider.display_for_login or provider.provider_id == tpa_hint + ] @classmethod def get(cls, provider_id): diff --git a/common/djangoapps/third_party_auth/tests/test_provider.py b/common/djangoapps/third_party_auth/tests/test_provider.py index a9a7ec1c6b..b38bdc4ed2 100644 --- a/common/djangoapps/third_party_auth/tests/test_provider.py +++ b/common/djangoapps/third_party_auth/tests/test_provider.py @@ -89,6 +89,45 @@ class RegistryTest(testutil.TestCase): self.assertNotIn(no_log_in_provider.provider_id, provider_ids) self.assertIn(normal_provider.provider_id, provider_ids) + def test_tpa_hint_provider_displayed_for_login(self): + """ + Tests to ensure that an enabled-but-not-visible provider is presented + for use in the UI when the "tpa_hint" parameter is specified + """ + + # A hidden provider should be accessible with tpa_hint (this is the main case) + hidden_provider = self.configure_google_provider(visible=False, enabled=True) + provider_ids = [ + idp.provider_id + for idp in provider.Registry.displayed_for_login(tpa_hint=hidden_provider.provider_id) + ] + self.assertIn(hidden_provider.provider_id, provider_ids) + + # New providers are hidden (ie, not flagged as 'visible') by default + # The tpa_hint parameter should work for these providers as well + implicitly_hidden_provider = self.configure_linkedin_provider(enabled=True) + provider_ids = [ + idp.provider_id + for idp in provider.Registry.displayed_for_login(tpa_hint=implicitly_hidden_provider.provider_id) + ] + self.assertIn(implicitly_hidden_provider.provider_id, provider_ids) + + # Disabled providers should not be matched in tpa_hint scenarios + disabled_provider = self.configure_twitter_provider(visible=True, enabled=False) + provider_ids = [ + idp.provider_id + for idp in provider.Registry.displayed_for_login(tpa_hint=disabled_provider.provider_id) + ] + self.assertNotIn(disabled_provider.provider_id, provider_ids) + + # Providers not utilized for learner authentication should not match tpa_hint + no_log_in_provider = self.configure_lti_provider() + provider_ids = [ + idp.provider_id + for idp in provider.Registry.displayed_for_login(tpa_hint=no_log_in_provider.provider_id) + ] + self.assertNotIn(no_log_in_provider.provider_id, provider_ids) + def test_provider_enabled_for_current_site(self): """ Verify that enabled_for_current_site returns True when the provider matches the current site. diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index b1f31f5c84..064b7510b0 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -288,7 +288,7 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi def setUp(self): super(StudentAccountLoginAndRegistrationTest, self).setUp() - # For these tests, three third party auth providers are enabled by default: + # Several third party auth providers are created for these tests: self.configure_google_provider(enabled=True, visible=True) self.configure_facebook_provider(enabled=True, visible=True) self.configure_dummy_provider( @@ -297,6 +297,11 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi icon_class='', icon_image=SimpleUploadedFile('icon.svg', ''), ) + self.hidden_enabled_provider = self.configure_linkedin_provider( + visible=False, + enabled=True, + ) + self.hidden_disabled_provider = self.configure_azure_ad_provider() @ddt.data( ("signin_user", "login"), @@ -427,6 +432,16 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi response = self.client.get(reverse('signin_user'), params, HTTP_ACCEPT="text/html") self.assertContains(response, '"third_party_auth_hint": "oa2-google-oauth2"') + tpa_hint = self.hidden_enabled_provider.provider_id + params = [("next", "/courses/something/?tpa_hint={0}".format(tpa_hint))] + response = self.client.get(reverse('signin_user'), params, HTTP_ACCEPT="text/html") + self.assertContains(response, '"third_party_auth_hint": "{0}"'.format(tpa_hint)) + + tpa_hint = self.hidden_disabled_provider.provider_id + params = [("next", "/courses/something/?tpa_hint={0}".format(tpa_hint))] + response = self.client.get(reverse('signin_user'), params, HTTP_ACCEPT="text/html") + self.assertNotIn(response.content, tpa_hint) + @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) def test_microsite_uses_old_login_page(self): # Retrieve the login page from a microsite domain diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index e19c20fa12..9599dc810e 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -112,7 +112,7 @@ def login_and_registration_form(request, initial_mode="login"): 'data': { 'login_redirect_url': redirect_to, 'initial_mode': initial_mode, - 'third_party_auth': _third_party_auth_context(request, redirect_to), + 'third_party_auth': _third_party_auth_context(request, redirect_to, third_party_auth_hint), 'third_party_auth_hint': third_party_auth_hint or '', 'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), 'support_link': configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK), @@ -190,7 +190,7 @@ def password_change_request_handler(request): return HttpResponseBadRequest(_("No email address provided.")) -def _third_party_auth_context(request, redirect_to): +def _third_party_auth_context(request, redirect_to, tpa_hint=None): """Context for third party auth providers and the currently running pipeline. Arguments: @@ -198,6 +198,8 @@ def _third_party_auth_context(request, redirect_to): is currently running. redirect_to: The URL to send the user to following successful authentication. + tpa_hint (string): An override flag that will return a matching provider + as long as its configuration has been enabled Returns: dict @@ -212,7 +214,7 @@ def _third_party_auth_context(request, redirect_to): } if third_party_auth.is_enabled(): - for enabled in third_party_auth.provider.Registry.displayed_for_login(): + for enabled in third_party_auth.provider.Registry.displayed_for_login(tpa_hint=tpa_hint): info = { "id": enabled.provider_id, "name": enabled.name,