From f59dc6c82020b74b82422dcbbb937f2fbfc2d6a5 Mon Sep 17 00:00:00 2001 From: Thomas Tracy Date: Thu, 28 Mar 2019 14:06:10 -0400 Subject: [PATCH] Fix's bug that does not allow an Oauth2 provider to have different slug and backend_names Added comment to explain model change. Removed accidental whitespace. Another pip issue. --- common/djangoapps/third_party_auth/models.py | 7 +++++++ common/djangoapps/third_party_auth/provider.py | 12 ++++++------ .../third_party_auth/tests/test_provider.py | 8 ++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 12400d506e..132c6b8787 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -332,6 +332,13 @@ class OAuth2ProviderConfig(ProviderConfig): .. no_pii: """ + # We are keying the provider config by backend_name here as suggested in the python social + # auth documentation. In order to reuse a backend for a second provider, a subclass can be + # created with seperate name. + # example: + # class SecondOpenIDProvider(OpenIDAuth): + # name = "second-openId-provider" + KEY_FIELDS = ('backend_name',) prefix = 'oa2' backend_name = models.CharField( max_length=50, blank=False, db_index=True, diff --git a/common/djangoapps/third_party_auth/provider.py b/common/djangoapps/third_party_auth/provider.py index 6dd669cecc..7ebfd110de 100644 --- a/common/djangoapps/third_party_auth/provider.py +++ b/common/djangoapps/third_party_auth/provider.py @@ -28,9 +28,9 @@ class Registry(object): Helper method that returns a generator used to iterate over all providers of the current site. """ - oauth2_slugs = OAuth2ProviderConfig.key_values('slug', flat=True) - for oauth2_slug in oauth2_slugs: - provider = OAuth2ProviderConfig.current(oauth2_slug) + oauth2_backend_names = OAuth2ProviderConfig.key_values('backend_name', flat=True) + for oauth2_backend_name in oauth2_backend_names: + provider = OAuth2ProviderConfig.current(oauth2_backend_name) if provider.enabled_for_current_site and provider.backend_name in _PSA_OAUTH2_BACKENDS: yield provider if SAMLConfiguration.is_enabled(Site.objects.get_current(get_current_request()), 'default'): @@ -112,9 +112,9 @@ class Registry(object): Instances of ProviderConfig. """ if backend_name in _PSA_OAUTH2_BACKENDS: - oauth2_slugs = OAuth2ProviderConfig.key_values('slug', flat=True) - for oauth2_slug in oauth2_slugs: - provider = OAuth2ProviderConfig.current(oauth2_slug) + oauth2_backend_names = OAuth2ProviderConfig.key_values('backend_name', flat=True) + for oauth2_backend_name in oauth2_backend_names: + provider = OAuth2ProviderConfig.current(oauth2_backend_name) if provider.backend_name == backend_name and provider.enabled_for_current_site: yield provider elif backend_name in _PSA_SAML_BACKENDS and SAMLConfiguration.is_enabled( diff --git a/common/djangoapps/third_party_auth/tests/test_provider.py b/common/djangoapps/third_party_auth/tests/test_provider.py index eddc08cb76..08e33a32f6 100644 --- a/common/djangoapps/third_party_auth/tests/test_provider.py +++ b/common/djangoapps/third_party_auth/tests/test_provider.py @@ -159,6 +159,14 @@ class RegistryTest(testutil.TestCase): self.assertIn(google_provider, provider.Registry._enabled_providers()) self.assertIn(google_provider, provider.Registry.get_enabled_by_backend_name('google-oauth2')) + def test_oath2_different_slug_from_backend_name(self): + """ + Test that an OAuth2 provider can have a slug that differs from the backend name. + """ + dummy_provider = self.configure_oauth_provider(enabled=True, name="dummy", slug="default", backend_name="dummy") + self.assertIn(dummy_provider, provider.Registry._enabled_providers()) + self.assertIn(dummy_provider, provider.Registry.get_enabled_by_backend_name('dummy')) + def test_oauth2_enabled_only_for_supplied_backend(self): """ Test to ensure that Registry.get_enabled_by_backend_name doesn't return OAuth2 providers with incorrect