From 53265d506e4579f357fcd2e3e3e5c65da39f931e Mon Sep 17 00:00:00 2001 From: irfanuddinahmad Date: Wed, 22 May 2019 09:07:43 -0400 Subject: [PATCH] user existence check updated to use email only --- .../djangoapps/third_party_auth/pipeline.py | 13 +++++- .../tests/test_pipeline_integration.py | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 5417f85cb9..b55b616154 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -549,8 +549,19 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia return (current_provider and (current_provider.skip_email_verification or current_provider.send_to_registration_first)) + def is_provider_saml(): + current_provider = provider.Registry.get_from_pipeline({'backend': current_partial.backend, 'kwargs': kwargs}) + saml_providers_list = list(provider.Registry.get_enabled_by_backend_name('tpa-saml')) + return (current_provider and + current_provider.slug in [saml_provider.slug for saml_provider in saml_providers_list]) + if not user: - if user_exists(details or {}): + # Use only email for user existence check in case of saml provider + if is_provider_saml(): + user_details = {'email': details.get('email')} if details else None + else: + user_details = details + if user_exists(user_details or {}): # User has not already authenticated and the details sent over from # identity provider belong to an existing user. return dispatch_to_login() diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py index a9d87315a2..ff68e84aed 100644 --- a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py +++ b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py @@ -308,6 +308,14 @@ class TestPipelineUtilityFunctions(TestCase, test.TestCase): class EnsureUserInformationTestCase(testutil.TestCase, test.TestCase): """Tests ensuring that we have the necessary user information to proceed with the pipeline.""" + def setUp(self): + super(EnsureUserInformationTestCase, self).setUp() + self.user = social_models.DjangoStorage.user.create_user( + username='username', + password='password', + email='email@example.com', + ) + @ddt.data( (True, '/register'), (False, '/login') @@ -338,6 +346,44 @@ class EnsureUserInformationTestCase(testutil.TestCase, test.TestCase): assert response.status_code == 302 assert response.url == expected_redirect_url + @ddt.data( + ('non_existing_user_email@example.com', '/register', True), + ('email@example.com', '/login', True), + (None, '/register', True), + ('non_existing_user_email@example.com', '/register', False), + ('email@example.com', '/login', False), + (None, '/login', False), + ) + @ddt.unpack + def test_redirect_for_saml_based_on_email_only(self, email, expected_redirect_url, is_saml): + """ + Test that only email(and not username) is used by saml based auth flows + to determine if a user already exists + """ + saml_provider = mock.MagicMock( + slug='unique_slug', + send_to_registration_first=True, + skip_email_verification=False + ) + with mock.patch('third_party_auth.pipeline.provider.Registry.get_from_pipeline') as get_from_pipeline: + get_from_pipeline.return_value = saml_provider + with mock.patch( + 'third_party_auth.pipeline.provider.Registry.get_enabled_by_backend_name' + ) as enabled_saml_providers: + enabled_saml_providers.return_value = [saml_provider, ] if is_saml else [] + with mock.patch('social_core.pipeline.partial.partial_prepare') as partial_prepare: + partial_prepare.return_value = mock.MagicMock(token='') + strategy = mock.MagicMock() + response = pipeline.ensure_user_information( + strategy=strategy, + backend=None, + auth_entry=pipeline.AUTH_ENTRY_LOGIN, + pipeline_index=0, + details={'username': self.user.username, 'email': email} + ) + assert response.status_code == 302 + assert response.url == expected_redirect_url + @unittest.skipUnless(testutil.AUTH_FEATURE_ENABLED, testutil.AUTH_FEATURES_KEY + ' not enabled') class UserDetailsForceSyncTestCase(testutil.TestCase, test.TestCase):