diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index d6c71d0095..777d58d6ad 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1704,17 +1704,29 @@ def create_account_with_params(request, params): 'REGISTRATION_EXTRA_FIELDS', getattr(settings, 'REGISTRATION_EXTRA_FIELDS', {}) ) + # registration via third party (Google, Facebook) using mobile application + # doesn't use social auth pipeline (no redirect uri(s) etc involved). + # In this case all related info (required for account linking) + # is sent in params. + # `third_party_auth_credentials_in_api` essentially means 'request + # is made from mobile application' + third_party_auth_credentials_in_api = 'provider' in params - # Boolean of whether a 3rd party auth provider and credentials were provided in - # the API so the newly created account can link with the 3rd party account. - # - # Note: this is orthogonal to the 3rd party authentication pipeline that occurs - # when the account is created via the browser and redirect URLs. - should_link_with_social_auth = third_party_auth.is_enabled() and 'provider' in params + is_third_party_auth_enabled = third_party_auth.is_enabled() - if should_link_with_social_auth or (third_party_auth.is_enabled() and pipeline.running(request)): + if is_third_party_auth_enabled and (pipeline.running(request) or third_party_auth_credentials_in_api): params["password"] = pipeline.make_random_password() + # in case user is registering via third party (Google, Facebook) and pipeline has expired, show appropriate + # error message + if is_third_party_auth_enabled and ('social_auth_provider' in params and not pipeline.running(request)): + raise ValidationError( + {'session_expired': [ + _(u"Registration using {provider} has timed out.").format( + provider=params.get('social_auth_provider')) + ]} + ) + # if doing signup for an external authorization, then get email, password, name from the eamap # don't use the ones from the form, since the user could have hacked those # unless originally we didn't get a valid email or name from the external auth @@ -1764,9 +1776,13 @@ def create_account_with_params(request, params): # first, create the account (user, profile, registration) = _do_create_account(form, custom_form, site=request.site) - # next, link the account with social auth, if provided via the API. + # If a 3rd party auth provider and credentials were provided in the API, link the account with social auth # (If the user is using the normal register page, the social auth pipeline does the linking, not this code) - if should_link_with_social_auth: + + # Note: this is orthogonal to the 3rd party authentication pipeline that occurs + # when the account is created via the browser and redirect URLs. + + if is_third_party_auth_enabled and third_party_auth_credentials_in_api: backend_name = params['provider'] request.social_strategy = social_utils.load_strategy(request) redirect_uri = reverse('social:complete', args=(backend_name, )) @@ -1808,7 +1824,7 @@ def create_account_with_params(request, params): # If the user is registering via 3rd party auth, track which provider they use third_party_provider = None running_pipeline = None - if third_party_auth.is_enabled() and pipeline.running(request): + if is_third_party_auth_enabled and pipeline.running(request): running_pipeline = pipeline.get(request) third_party_provider = provider.Registry.get_from_pipeline(running_pipeline) diff --git a/openedx/core/djangoapps/user_api/helpers.py b/openedx/core/djangoapps/user_api/helpers.py index 8321b315d5..c2c62d6618 100644 --- a/openedx/core/djangoapps/user_api/helpers.py +++ b/openedx/core/djangoapps/user_api/helpers.py @@ -117,7 +117,7 @@ class InvalidFieldError(Exception): class FormDescription(object): """Generate a JSON representation of a form. """ - ALLOWED_TYPES = ["text", "email", "select", "textarea", "checkbox", "password"] + ALLOWED_TYPES = ["text", "email", "select", "textarea", "checkbox", "password", "hidden"] ALLOWED_RESTRICTIONS = { "text": ["min_length", "max_length"], diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 5e4af86cfe..1aee8ecd9f 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -951,6 +951,17 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "required": False, } ) + # social_auth_provider should be present + # with value `Google`(we are setting up google provider for this test). + self._assert_reg_field( + no_extra_fields_setting, + { + "name": "social_auth_provider", + "type": "hidden", + "required": False, + "defaultValue": "Google" + } + ) # Email should be filled in self._assert_reg_field( @@ -1784,6 +1795,7 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe "username": user.username if user else "test_username", "name": user.first_name if user else "test name", "email": user.email if user else "test@test.com", + } def _assert_existing_user_error(self, response): @@ -1805,6 +1817,15 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe ) self.assertNotIn("partial_pipeline", self.client.session) + def _assert_third_party_session_expired_error(self, response, expected_error_message): + """Assert that given response is an error due to third party session expiry""" + self.assertEqual(response.status_code, 400) + response_json = json.loads(response.content) + self.assertEqual( + response_json, + {"session_expired": [{"user_message": expected_error_message}]} + ) + def _verify_user_existence(self, user_exists, social_link_exists, user_is_active=None, username=None): """Verifies whether the user object exists.""" users = User.objects.filter(username=(username if username else "test_username")) @@ -1879,6 +1900,32 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe ) self._verify_user_existence(user_exists=False, social_link_exists=False) + def test_expired_pipeline(self): + + """ + Test that there is an error and account is not created + when request is made for account creation using third (Google, Facebook etc) party with pipeline + getting expired using browser (not mobile application). + + NOTE: We are NOT using actual pipeline here so pipeline is always expired in this environment. + we don't have to explicitly expire pipeline. + + """ + + data = self.data() + # provider is sent along request when request is made from mobile application + data.pop("provider") + # to identify that request is made using browser + data.update({"social_auth_provider": "Google"}) + response = self.client.post(self.url, data) + # NO partial_pipeline in session means pipeline is expired + self.assertNotIn("partial_pipeline", self.client.session) + self._assert_third_party_session_expired_error( + response, + u"Registration using {provider} has timed out.".format(provider="Google") + ) + self._verify_user_existence(user_exists=False, social_link_exists=False) + @skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled") class TestFacebookRegistrationView( diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index 897aa9b371..e6125d8e6b 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -887,6 +887,14 @@ class RegistrationView(APIView): instructions="", restrictions={} ) + # used to identify that request is running third party social auth + form_desc.add_field( + "social_auth_provider", + field_type="hidden", + label="", + default=current_provider.name if current_provider.name else "Third Party", + required=False, + ) class PasswordResetView(APIView):