diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 8ea71514a9..c7229a0199 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -194,7 +194,7 @@ def auth_pipeline_urls(auth_entry, redirect_url=None): return { provider.provider_id: third_party_auth.pipeline.get_login_url( provider.provider_id, auth_entry, redirect_url=redirect_url - ) for provider in third_party_auth.provider.Registry.accepting_logins() + ) for provider in third_party_auth.provider.Registry.displayed_for_login() } diff --git a/common/djangoapps/student/tests/test_login_registration_forms.py b/common/djangoapps/student/tests/test_login_registration_forms.py index ada18b7db7..09837fda19 100644 --- a/common/djangoapps/student/tests/test_login_registration_forms.py +++ b/common/djangoapps/student/tests/test_login_registration_forms.py @@ -54,8 +54,8 @@ class LoginFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStoreTes self.url = reverse("signin_user") self.course_id = unicode(self.course.id) self.courseware_url = reverse("courseware", args=[self.course_id]) - self.configure_google_provider(enabled=True) - self.configure_facebook_provider(enabled=True) + self.configure_google_provider(enabled=True, visible=True) + self.configure_facebook_provider(enabled=True, visible=True) @patch.dict(settings.FEATURES, {"ENABLE_THIRD_PARTY_AUTH": False}) @ddt.data(THIRD_PARTY_AUTH_PROVIDERS) @@ -170,8 +170,8 @@ class RegisterFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStore self.url = reverse("register_user") self.course_id = unicode(self.course.id) - self.configure_google_provider(enabled=True) - self.configure_facebook_provider(enabled=True) + self.configure_google_provider(enabled=True, visible=True) + self.configure_facebook_provider(enabled=True, visible=True) @patch.dict(settings.FEATURES, {"ENABLE_THIRD_PARTY_AUTH": False}) @ddt.data(*THIRD_PARTY_AUTH_PROVIDERS) diff --git a/common/djangoapps/third_party_auth/api/tests/test_views.py b/common/djangoapps/third_party_auth/api/tests/test_views.py index 8956a4f20a..7634368674 100644 --- a/common/djangoapps/third_party_auth/api/tests/test_views.py +++ b/common/djangoapps/third_party_auth/api/tests/test_views.py @@ -51,7 +51,11 @@ class TpaAPITestCase(ThirdPartyAuthTestMixin, APITestCase): self.configure_facebook_provider(enabled=True) self.configure_linkedin_provider(enabled=False) self.enable_saml() - testshib = self.configure_saml_provider(name='TestShib', enabled=True, idp_slug=IDP_SLUG_TESTSHIB) + testshib = self.configure_saml_provider( + name='TestShib', + enabled=True, + idp_slug=IDP_SLUG_TESTSHIB + ) # Create several users and link each user to Google and TestShib for username in LINKED_USERS: diff --git a/common/djangoapps/third_party_auth/migrations/0004_add_visible_field.py b/common/djangoapps/third_party_auth/migrations/0004_add_visible_field.py new file mode 100644 index 0000000000..736e01ceec --- /dev/null +++ b/common/djangoapps/third_party_auth/migrations/0004_add_visible_field.py @@ -0,0 +1,67 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('third_party_auth', '0001_initial'), + ('third_party_auth', '0002_schema__provider_icon_image'), + ('third_party_auth', '0003_samlproviderconfig_debug_mode'), + ] + + operations = [ + migrations.AddField( + model_name='LTIProviderConfig', + name='visible', + field=models.BooleanField( + help_text=b'If this option is not selected, users will not be presented with the provider as an option to authenticate with on the login screen, but manual authentication using the correct link is still possible.', + default=True + ), + preserve_default=False + ), + migrations.AlterField( + model_name='LTIProviderConfig', + name='visible', + field=models.BooleanField( + help_text=b'If this option is not selected, users will not be presented with the provider as an option to authenticate with on the login screen, but manual authentication using the correct link is still possible.', + default=False + ) + ), + migrations.AddField( + model_name='OAuth2ProviderConfig', + name='visible', + field=models.BooleanField( + help_text=b'If this option is not selected, users will not be presented with the provider as an option to authenticate with on the login screen, but manual authentication using the correct link is still possible.', + default=True + ), + preserve_default=False + ), + migrations.AlterField( + model_name='OAuth2ProviderConfig', + name='visible', + field=models.BooleanField( + help_text=b'If this option is not selected, users will not be presented with the provider as an option to authenticate with on the login screen, but manual authentication using the correct link is still possible.', + default=False + ) + ), + migrations.AddField( + model_name='SAMLProviderConfig', + name='visible', + field=models.BooleanField( + help_text=b'If this option is not selected, users will not be presented with the provider as an option to authenticate with on the login screen, but manual authentication using the correct link is still possible.', + default=True + ), + preserve_default=False + ), + migrations.AlterField( + model_name='SAMLProviderConfig', + name='visible', + field=models.BooleanField( + help_text=b'If this option is not selected, users will not be presented with the provider as an option to authenticate with on the login screen, but manual authentication using the correct link is still possible.', + default=False + ) + ), + ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 8a0d7635af..4ab0357d95 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -121,6 +121,14 @@ class ProviderConfig(ConfigurationModel): "email, and their account will be activated immediately upon registration." ), ) + visible = models.BooleanField( + default=False, + help_text=_( + "If this option is not selected, users will not be presented with the provider " + "as an option to authenticate with on the login screen, but manual " + "authentication using the correct link is still possible." + ), + ) prefix = None # used for provider_id. Set to a string value in subclass backend_name = None # Set to a field or fixed value in subclass accepts_logins = True # Whether to display a sign-in button when the provider is enabled @@ -212,6 +220,14 @@ class ProviderConfig(ConfigurationModel): """Gets associated Django settings.AUTHENTICATION_BACKEND string.""" return '{}.{}'.format(self.backend_class.__module__, self.backend_class.__name__) + @property + def display_for_login(self): + """ + Determines whether the provider ought to be shown as an option with + which to authenticate on the login screen, registration screen, and elsewhere. + """ + return bool(self.enabled and self.accepts_logins and self.visible) + class OAuth2ProviderConfig(ProviderConfig): """ diff --git a/common/djangoapps/third_party_auth/provider.py b/common/djangoapps/third_party_auth/provider.py index dde2b41608..426ac7fd32 100644 --- a/common/djangoapps/third_party_auth/provider.py +++ b/common/djangoapps/third_party_auth/provider.py @@ -37,9 +37,9 @@ class Registry(object): return sorted(cls._enabled_providers(), key=lambda provider: provider.name) @classmethod - def accepting_logins(cls): - """Returns list of providers that can be used to initiate logins currently""" - return [provider for provider in cls.enabled() if provider.accepts_logins] + 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] @classmethod def get(cls, provider_id): diff --git a/common/djangoapps/third_party_auth/tests/specs/test_azuread.py b/common/djangoapps/third_party_auth/tests/specs/test_azuread.py index 680983250a..35c62198cf 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_azuread.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_azuread.py @@ -11,6 +11,7 @@ class AzureADOauth2IntegrationTest(base.Oauth2IntegrationTest): super(AzureADOauth2IntegrationTest, self).setUp() self.provider = self.configure_azure_ad_provider( enabled=True, + visible=True, key='azure_ad_oauth2_key', secret='azure_ad_oauth2_secret', ) diff --git a/common/djangoapps/third_party_auth/tests/specs/test_generic.py b/common/djangoapps/third_party_auth/tests/specs/test_generic.py index 56b7fd8333..db710cbda3 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_generic.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_generic.py @@ -22,7 +22,7 @@ class GenericIntegrationTest(IntegrationTestMixin, testutil.TestCase): def setUp(self): super(GenericIntegrationTest, self).setUp() - self.configure_dummy_provider(enabled=True) + self.configure_dummy_provider(enabled=True, visible=True) def do_provider_login(self, provider_redirect_url): """ diff --git a/common/djangoapps/third_party_auth/tests/specs/test_google.py b/common/djangoapps/third_party_auth/tests/specs/test_google.py index a4d37e40db..a120ad70c3 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_google.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_google.py @@ -19,6 +19,7 @@ class GoogleOauth2IntegrationTest(base.Oauth2IntegrationTest): super(GoogleOauth2IntegrationTest, self).setUp() self.provider = self.configure_google_provider( enabled=True, + visible=True, key='google_oauth2_key', secret='google_oauth2_secret', ) diff --git a/common/djangoapps/third_party_auth/tests/specs/test_linkedin.py b/common/djangoapps/third_party_auth/tests/specs/test_linkedin.py index a5f0e61cc7..700c22ff86 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_linkedin.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_linkedin.py @@ -10,6 +10,7 @@ class LinkedInOauth2IntegrationTest(base.Oauth2IntegrationTest): super(LinkedInOauth2IntegrationTest, self).setUp() self.provider = self.configure_linkedin_provider( enabled=True, + visible=True, key='linkedin_oauth2_key', secret='linkedin_oauth2_secret', ) 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 ee7e6e7061..4cad831092 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py @@ -134,6 +134,7 @@ class TestShibIntegrationTest(IntegrationTestMixin, testutil.SAMLTestCase): fetch_metadata = kwargs.pop('fetch_metadata', True) kwargs.setdefault('name', self.PROVIDER_NAME) kwargs.setdefault('enabled', True) + kwargs.setdefault('visible', True) kwargs.setdefault('idp_slug', self.PROVIDER_IDP_SLUG) kwargs.setdefault('entity_id', TESTSHIB_ENTITY_ID) kwargs.setdefault('metadata_source', TESTSHIB_METADATA_URL) diff --git a/common/djangoapps/third_party_auth/tests/specs/test_twitter.py b/common/djangoapps/third_party_auth/tests/specs/test_twitter.py index 50377d549e..5c7bacea41 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_twitter.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_twitter.py @@ -13,6 +13,7 @@ class TwitterIntegrationTest(base.Oauth2IntegrationTest): super(TwitterIntegrationTest, self).setUp() self.provider = self.configure_twitter_provider( enabled=True, + visible=True, key='twitter_oauth1_key', secret='twitter_oauth1_secret', ) diff --git a/common/djangoapps/third_party_auth/tests/test_provider.py b/common/djangoapps/third_party_auth/tests/test_provider.py index ff67df5d0d..f55b0f0f06 100644 --- a/common/djangoapps/third_party_auth/tests/test_provider.py +++ b/common/djangoapps/third_party_auth/tests/test_provider.py @@ -48,7 +48,12 @@ class RegistryTest(testutil.TestCase): """ Test that only backend_names listed in settings.AUTHENTICATION_BACKENDS can be used """ self.configure_oauth_provider(enabled=True, name="Disallowed", backend_name="disallowed") self.enable_saml() - self.configure_saml_provider(enabled=True, name="Disallowed", idp_slug="test", backend_name="disallowed") + self.configure_saml_provider( + enabled=True, + name="Disallowed", + idp_slug="test", + backend_name="disallowed" + ) self.assertEqual(len(provider.Registry.enabled()), 0) def test_enabled_returns_list_of_enabled_providers_sorted_by_name(self): @@ -62,6 +67,23 @@ class RegistryTest(testutil.TestCase): with patch('third_party_auth.provider._PSA_OAUTH2_BACKENDS', backend_names): self.assertEqual(sorted(provider_names), [prov.name for prov in provider.Registry.enabled()]) + def test_providers_displayed_for_login(self): + """ + Tests to ensure that only providers that we can use to log in are presented + for rendering in the UI. + """ + hidden_provider = self.configure_google_provider(visible=False, enabled=True) + normal_provider = self.configure_facebook_provider(visible=True, enabled=True) + implicitly_hidden_provider = self.configure_linkedin_provider(enabled=True) + disabled_provider = self.configure_twitter_provider(visible=True, enabled=False) + no_log_in_provider = self.configure_lti_provider() + provider_ids = [idp.provider_id for idp in provider.Registry.displayed_for_login()] + self.assertNotIn(hidden_provider.provider_id, provider_ids) + self.assertNotIn(implicitly_hidden_provider.provider_id, provider_ids) + self.assertNotIn(disabled_provider.provider_id, provider_ids) + self.assertNotIn(no_log_in_provider.provider_id, provider_ids) + self.assertIn(normal_provider.provider_id, provider_ids) + def test_get_returns_enabled_provider(self): google_provider = self.configure_google_provider(enabled=True) self.assertEqual(google_provider.id, provider.Registry.get(google_provider.provider_id).id) diff --git a/common/djangoapps/third_party_auth/tests/utils.py b/common/djangoapps/third_party_auth/tests/utils.py index a5ac1e6bc4..48de8ceb8a 100644 --- a/common/djangoapps/third_party_auth/tests/utils.py +++ b/common/djangoapps/third_party_auth/tests/utils.py @@ -35,9 +35,9 @@ class ThirdPartyOAuthTestMixin(ThirdPartyAuthTestMixin): UserSocialAuth.objects.create(user=self.user, provider=self.BACKEND, uid=self.social_uid) self.oauth_client = self._create_client() if self.BACKEND == 'google-oauth2': - self.configure_google_provider(enabled=True) + self.configure_google_provider(enabled=True, visible=True) elif self.BACKEND == 'facebook': - self.configure_facebook_provider(enabled=True) + self.configure_facebook_provider(enabled=True, visible=True) def _create_client(self): """ diff --git a/common/test/db_fixtures/third_party_auth.json b/common/test/db_fixtures/third_party_auth.json index 9b4fad7bff..c414b0e53a 100644 --- a/common/test/db_fixtures/third_party_auth.json +++ b/common/test/db_fixtures/third_party_auth.json @@ -12,7 +12,8 @@ "backend_name": "google-oauth2", "key": "test", "secret": "test", - "other_settings": "{}" + "other_settings": "{}", + "visible": true } }, { @@ -28,7 +29,8 @@ "backend_name": "facebook", "key": "test", "secret": "test", - "other_settings": "{}" + "other_settings": "{}", + "visible": true } }, { @@ -44,7 +46,8 @@ "backend_name": "dummy", "key": "", "secret": "", - "other_settings": "{}" + "other_settings": "{}", + "visible": true } } ] diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 9595de6646..656c275062 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -237,9 +237,10 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi super(StudentAccountLoginAndRegistrationTest, self).setUp() # For these tests, three third party auth providers are enabled by default: - self.configure_google_provider(enabled=True) - self.configure_facebook_provider(enabled=True) + self.configure_google_provider(enabled=True, visible=True) + self.configure_facebook_provider(enabled=True, visible=True) self.configure_dummy_provider( + visible=True, enabled=True, icon_class='', icon_image=SimpleUploadedFile('icon.svg', ''), @@ -482,8 +483,8 @@ class AccountSettingsViewTest(ThirdPartyAuthTestMixin, TestCase, ProgramsApiConf self.request.user = self.user # For these tests, two third party auth providers are enabled by default: - self.configure_google_provider(enabled=True) - self.configure_facebook_provider(enabled=True) + self.configure_google_provider(enabled=True, visible=True) + self.configure_facebook_provider(enabled=True, visible=True) # Python-social saves auth failure notifcations in Django messages. # See pipeline.get_duplicate_provider() for details. diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 55e680f8fb..78e6f4d852 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -203,7 +203,7 @@ def _third_party_auth_context(request, redirect_to): } if third_party_auth.is_enabled(): - for enabled in third_party_auth.provider.Registry.accepting_logins(): + for enabled in third_party_auth.provider.Registry.displayed_for_login(): info = { "id": enabled.provider_id, "name": enabled.name, @@ -487,6 +487,8 @@ def account_settings_context(request): # If the user is connected, sending a POST request to this url removes the connection # information for this provider from their edX account. 'disconnect_url': pipeline.get_disconnect_url(state.provider.provider_id, state.association_id), - } for state in auth_states] + # We only want to include providers if they are either currently available to be logged + # in with, or if the user is already authenticated with them. + } for state in auth_states if state.provider.display_for_login or state.has_account] return context diff --git a/lms/templates/login.html b/lms/templates/login.html index 26338f5571..5b81c5fda6 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -218,7 +218,7 @@ from third_party_auth import provider, pipeline
- % for enabled in provider.Registry.accepting_logins(): + % for enabled in provider.Registry.displayed_for_login(): ## Translators: provider_name is the name of an external, third-party user authentication provider (like Google or LinkedIn). - % elif state.provider.accepts_logins: + % elif state.provider.display_for_login: ## Translators: clicking on this creates a link between a user's edX account and their account with an external authentication provider (like Google or LinkedIn). ${_("Link")} diff --git a/themes/stanford-style/lms/templates/register-form.html b/themes/stanford-style/lms/templates/register-form.html index cf5b017968..3bcf7c9992 100644 --- a/themes/stanford-style/lms/templates/register-form.html +++ b/themes/stanford-style/lms/templates/register-form.html @@ -24,7 +24,7 @@ from student.models import UserProfile
- % for enabled in provider.Registry.accepting_logins(): + % for enabled in provider.Registry.displayed_for_login(): ## Translators: provider_name is the name of an external, third-party user authentication service (like Google or LinkedIn).