diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 4fdfc94483..4fd41e85da 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -84,6 +84,7 @@ import student from edxmako.shortcuts import render_to_string from eventtracking import tracker from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from third_party_auth.utils import user_exists from . import provider @@ -501,7 +502,7 @@ def set_pipeline_timeout(strategy, user, *args, **kwargs): # choice of the user. -def redirect_to_custom_form(request, auth_entry, kwargs): +def redirect_to_custom_form(request, auth_entry, details, kwargs): """ If auth_entry is found in AUTH_ENTRY_CUSTOM, this is used to send provider data to an external server's registration/login page. @@ -520,7 +521,7 @@ def redirect_to_custom_form(request, auth_entry, kwargs): "auth_entry": auth_entry, "backend_name": backend_name, "provider_id": provider_id, - "user_details": kwargs['details'], + "user_details": details, }) digest = hmac.new(secret_key, msg=data_str, digestmod=hashlib.sha256).digest() # Store the data in the session temporarily, then redirect to a page that will POST it to @@ -535,7 +536,7 @@ def redirect_to_custom_form(request, auth_entry, kwargs): @partial.partial def ensure_user_information(strategy, auth_entry, backend=None, user=None, social=None, current_partial=None, - allow_inactive_user=False, *args, **kwargs): + allow_inactive_user=False, details=None, *args, **kwargs): """ Ensure that we have the necessary information about a user (either an existing account or registration data) to proceed with the pipeline. @@ -567,6 +568,11 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia (current_provider.skip_email_verification or current_provider.send_to_registration_first)) if not user: + if user_exists(details or {}): + # User has not already authenticated and the details sent over from + # identity provider belong to an existing user. + return dispatch_to_login() + if is_api(auth_entry): return HttpResponseBadRequest() elif auth_entry == AUTH_ENTRY_LOGIN: @@ -583,7 +589,7 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia raise AuthEntryError(backend, 'auth_entry is wrong. Settings requires a user.') elif auth_entry in AUTH_ENTRY_CUSTOM: # Pass the username, email, etc. via query params to the custom entry page: - return redirect_to_custom_form(strategy.request, auth_entry, kwargs) + return redirect_to_custom_form(strategy.request, auth_entry, details or {}, kwargs) else: raise AuthEntryError(backend, 'auth_entry invalid') diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index c1ede20680..bff9a595cf 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -894,8 +894,9 @@ class IntegrationTest(testutil.TestCase, test.TestCase): strategy.storage.user.create_user(username=self.get_username(), email='user@email.com', password='password') backend = strategy.request.backend backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + # If learner already has an account then make sure login page is served instead of registration. # pylint: disable=protected-access - self.assert_redirect_to_register_looks_correct(actions.do_complete(backend, social_views._do_login)) + self.assert_redirect_to_login_looks_correct(actions.do_complete(backend, social_views._do_login)) distinct_username = pipeline.get(request)['kwargs']['username'] self.assertNotEqual(original_username, distinct_username) 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 35c62198cf..56bcfc2e03 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_azuread.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_azuread.py @@ -35,8 +35,8 @@ class AzureADOauth2IntegrationTest(base.Oauth2IntegrationTest): 'aud': 'abcdefgh-1234-5678-900a-0aa0a00aa0aa', 'tid': 'abcdefgh-1234-5678-900a-0aa0a00aa0aa', 'amr': ['pwd'], - 'unique_name': 'email_value@example.com', - 'upn': 'email_value@example.com', + 'unique_name': 'user@email.com', + 'upn': 'user@email.com', 'family_name': 'family_name_value', 'name': 'name_value', 'given_name': 'given_name_value', 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 ff1ed6bf33..1c7b0f3589 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_google.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_google.py @@ -31,7 +31,7 @@ class GoogleOauth2IntegrationTest(base.Oauth2IntegrationTest): 'token_type': 'token_type_value', } USER_RESPONSE_DATA = { - 'email': 'email_value@example.com', + 'email': 'user@email.com', 'family_name': 'family_name_value', 'given_name': 'given_name_value', 'id': 'id_value', @@ -85,8 +85,8 @@ class GoogleOauth2IntegrationTest(base.Oauth2IntegrationTest): 'backend_name': 'google-oauth2', 'provider_id': 'oa2-google-oauth2', 'user_details': { - 'username': 'email_value', - 'email': 'email_value@example.com', + 'username': 'user', + 'email': 'user@email.com', 'fullname': 'name_value', 'first_name': 'given_name_value', 'last_name': 'family_name_value', diff --git a/common/djangoapps/third_party_auth/tests/test_utils.py b/common/djangoapps/third_party_auth/tests/test_utils.py new file mode 100644 index 0000000000..db1b3c9d54 --- /dev/null +++ b/common/djangoapps/third_party_auth/tests/test_utils.py @@ -0,0 +1,34 @@ +""" +Tests for third_party_auth utility functions. +""" +import unittest + +from django.conf import settings +from third_party_auth.tests.testutil import TestCase +from third_party_auth.utils import user_exists +from student.tests.factories import UserFactory + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class TestUtils(TestCase): + """ + Test the utility functions. + """ + def test_user_exists(self): + """ + Verify that user_exists function returns correct response. + """ + # Create users from factory + UserFactory(username='test_user', email='test_user@example.com') + self.assertTrue( + user_exists({'username': 'test_user', 'email': 'test_user@example.com'}), + ) + self.assertTrue( + user_exists({'username': 'test_user'}), + ) + self.assertTrue( + user_exists({'email': 'test_user@example.com'}), + ) + self.assertFalse( + user_exists({'username': 'invalid_user'}), + ) diff --git a/common/djangoapps/third_party_auth/utils.py b/common/djangoapps/third_party_auth/utils.py new file mode 100644 index 0000000000..885fb4f20b --- /dev/null +++ b/common/djangoapps/third_party_auth/utils.py @@ -0,0 +1,29 @@ +""" +Utility functions for third_party_auth +""" + +from django.contrib.auth.models import User + + +def user_exists(details): + """ + Return True if user with given details exist in the system. + + Arguments: + details (dict): dictionary containing user infor like email, username etc. + + Returns: + (bool): True if user with given details exists, `False` otherwise. + """ + user_queryset_filter = {} + email = details.get('email') + username = details.get('username') + if email: + user_queryset_filter['email'] = email + elif username: + user_queryset_filter['username'] = username + + if user_queryset_filter: + return User.objects.filter(**user_queryset_filter).exists() + + return False diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 37722ff1fc..e73abb227b 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -640,6 +640,7 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi "finishAuthUrl": finish_auth_url, "errorMessage": None, "registerFormSubmitButtonText": "Create Account", + "syncLearnerProfileData": False, } if expected_ec is not None: # If we set an EnterpriseCustomer, third-party auth providers ought to be hidden. diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index e11626e378..9f83f76228 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -326,6 +326,7 @@ def _third_party_auth_context(request, redirect_to, tpa_hint=None): "finishAuthUrl": None, "errorMessage": None, "registerFormSubmitButtonText": _("Create Account"), + "syncLearnerProfileData": False, } if third_party_auth.is_enabled(): @@ -357,6 +358,7 @@ def _third_party_auth_context(request, redirect_to, tpa_hint=None): if current_provider is not None: context["currentProvider"] = current_provider.name context["finishAuthUrl"] = pipeline.get_complete_url(current_provider.backend_name) + context["syncLearnerProfileData"] = current_provider.sync_learner_profile_data if current_provider.skip_registration_form: # For enterprise (and later for everyone), we need to get explicit consent to the diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index fde0d4bfb4..0c954927d7 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/lms/static/js/student_account/views/LoginView.js @@ -41,6 +41,7 @@ data.thirdPartyAuth.secondaryProviders && data.thirdPartyAuth.secondaryProviders.length ); this.currentProvider = data.thirdPartyAuth.currentProvider || ''; + this.syncLearnerProfileData = data.thirdPartyAuth.syncLearnerProfileData || false; this.errorMessage = data.thirdPartyAuth.errorMessage || ''; this.platformName = data.platformName; this.resetModel = data.resetModel; @@ -63,6 +64,7 @@ context: { fields: fields, currentProvider: this.currentProvider, + syncLearnerProfileData: this.syncLearnerProfileData, providers: this.providers, hasSecondaryProviders: this.hasSecondaryProviders, platformName: this.platformName, diff --git a/lms/static/js/student_account/views/RegisterView.js b/lms/static/js/student_account/views/RegisterView.js index 56fe8d5726..c0bb0337c7 100644 --- a/lms/static/js/student_account/views/RegisterView.js +++ b/lms/static/js/student_account/views/RegisterView.js @@ -55,6 +55,7 @@ data.thirdPartyAuth.secondaryProviders && data.thirdPartyAuth.secondaryProviders.length ); this.currentProvider = data.thirdPartyAuth.currentProvider || ''; + this.syncLearnerProfileData = data.thirdPartyAuth.syncLearnerProfileData || false; this.errorMessage = data.thirdPartyAuth.errorMessage || ''; this.platformName = data.platformName; this.autoSubmit = data.thirdPartyAuth.autoSubmitRegForm; @@ -141,6 +142,7 @@ context: { fields: fields, currentProvider: this.currentProvider, + syncLearnerProfileData: this.syncLearnerProfileData, providers: this.providers, hasSecondaryProviders: this.hasSecondaryProviders, platformName: this.platformName, diff --git a/lms/templates/student_account/login.underscore b/lms/templates/student_account/login.underscore index 8bb806a945..e0829bf9cf 100644 --- a/lms/templates/student_account/login.underscore +++ b/lms/templates/student_account/login.underscore @@ -1,7 +1,7 @@
-<% if ( context.createAccountOption !== false ) { %> +<% if ( context.createAccountOption !== false && !context.syncLearnerProfileData) { %>
<%- gettext("First time here?") %> <%- gettext("Create an Account.") %> diff --git a/lms/templates/student_account/register.underscore b/lms/templates/student_account/register.underscore index 1504fa5a2c..4f9b12b74a 100644 --- a/lms/templates/student_account/register.underscore +++ b/lms/templates/student_account/register.underscore @@ -1,10 +1,14 @@
-
- <%- edx.StringUtils.interpolate(gettext('Already have an {platformName} account?'), {platformName: context.platformName }) %> - <%- gettext("Sign in.") %> -
+<% if (!context.syncLearnerProfileData) { %> +
+ <%- edx.StringUtils.interpolate(gettext('Already have an {platformName} account?'), {platformName: context.platformName }) %> + <%- gettext("Sign in.") %> +
+<% } %> + +

<%- gettext('Create an Account')%>

diff --git a/openedx/core/djangoapps/user_api/api.py b/openedx/core/djangoapps/user_api/api.py index 7962485d13..a7c51ffd48 100644 --- a/openedx/core/djangoapps/user_api/api.py +++ b/openedx/core/djangoapps/user_api/api.py @@ -843,8 +843,11 @@ class RegistrationFormFactory(object): # When the TPA Provider is configured to skip the registration form and we are in an # enterprise context, we need to hide all fields except for terms of service and # ensure that the user explicitly checks that field. - hide_registration_fields_except_tos = (current_provider.skip_registration_form and - enterprise_customer_for_request(request)) + hide_registration_fields_except_tos = ( + ( + current_provider.skip_registration_form and enterprise_customer_for_request(request) + ) or current_provider.sync_learner_profile_data + ) for field_name in self.DEFAULT_FIELDS + self.EXTRA_FIELDS: if field_name in field_overrides: