Merge pull request #14601 from edx/noraiz/ECOM-6939

ECOM-6939 Fixed social auth false password-validation error

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)
 

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"],

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(

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):