Merge pull request #22511 from edx/robrap/ARCH-1253-logistration-using-login-user-take-2

ARCH-1253: logistration using login user - take 2

common/djangoapps/third_party_auth/tests/specs/base.py

@@ -130,11 +130,10 @@ class HelperMixin(object):
 
     def assert_json_failure_response_is_missing_social_auth(self, response):
         """Asserts failure on /login for missing social auth looks right."""
-        self.assertContains(
-            response,
-            u"successfully signed in to your %s account, but this account isn't linked" % self.provider.name,
-            status_code=403,
-        )
+        self.assertEqual(403, response.status_code)
+        payload = json.loads(response.content.decode('utf-8'))
+        self.assertFalse(payload.get('success'))
+        self.assertEqual(payload.get('error_code'), 'third-party-auth-with-no-linked-account')
 
     def assert_json_failure_response_is_username_collision(self, response):
         """Asserts the json response indicates a username collision."""

lms/static/js/student_account/views/LoginView.js

@@ -189,6 +189,51 @@
             },
 
             saveError: function(error) {
+                if (error.responseJSON !== undefined) {
+                    this.saveErrorWithoutShim(error);
+                } else {
+                    this.saveErrorWithShim(error);
+                }
+            },
+
+            saveErrorWithoutShim: function(error) {
+                var errorCode;
+                var msg;
+                if (error.status === 0) {
+                    msg = gettext('An error has occurred. Check your Internet connection and try again.');
+                } else if (error.status === 500) {
+                    msg = gettext('An error has occurred. Try refreshing the page, or check your Internet connection.'); // eslint-disable-line max-len
+                } else if (error.responseJSON !== undefined) {
+                    msg = error.responseJSON.value;
+                    errorCode = error.responseJSON.error_code;
+                } else {
+                    msg = gettext('An unexpected error has occurred.');
+                }
+
+                this.errors = [
+                    StringUtils.interpolate(
+                        '<li>{msg}</li>', {
+                            msg: msg
+                        }
+                    )
+                ];
+                this.clearPasswordResetSuccess();
+
+                /* If the user successfully authenticated with a third-party provider, but they haven't
+                 * linked the accounts, instruct the user on how to link the accounts.
+                 */
+                if (errorCode === 'third-party-auth-with-no-linked-account' && this.currentProvider) {
+                    if (!this.hideAuthWarnings) {
+                        this.clearFormErrors();
+                        this.renderThirdPartyAuthWarning();
+                    }
+                } else {
+                    this.renderErrors(this.defaultFormErrorsTitle, this.errors);
+                }
+                this.toggleDisableButton(false);
+            },
+
+            saveErrorWithShim: function(error) {
                 var msg = error.responseText;
                 if (error.status === 0) {
                     msg = gettext('An error has occurred. Check your Internet connection and try again.');
@@ -215,7 +260,7 @@
                  this.currentProvider) {
                     if (!this.hideAuthWarnings) {
                         this.clearFormErrors();
-                        this.renderAuthWarning();
+                        this.renderThirdPartyAuthWarning();
                     }
                 } else {
                     this.renderErrors(this.defaultFormErrorsTitle, this.errors);
@@ -223,7 +268,7 @@
                 this.toggleDisableButton(false);
             },
 
-            renderAuthWarning: function() {
+            renderThirdPartyAuthWarning: function() {
                 var message = _.sprintf(
                     gettext('You have successfully signed into %(currentProvider)s, but your %(currentProvider)s' +
                             ' account does not have a linked %(platformName)s account. To link your accounts,' +

openedx/core/djangoapps/user_authn/views/login.py

@@ -400,6 +400,7 @@ def login_user(request):
         response = set_logged_in_cookies(request, response, possibly_authenticated_user)
         set_custom_metric('login_user_auth_failed_error', False)
         set_custom_metric('login_user_response_status', response.status_code)
+        set_custom_metric('login_user_redirect_url', redirect_url)
         return response
     except AuthFailedError as error:
         log.exception(error.get_response())
@@ -483,10 +484,15 @@ def _parse_analytics_param_for_course_id(request):
     modified_request = request.POST.copy()
     if isinstance(request, HttpRequest):
         # Works for an HttpRequest but not a rest_framework.request.Request.
+        # Note: This case seems to be used for tests only.
         request.POST = modified_request
+        set_custom_metric('login_user_request_type', 'django')
     else:
         # The request must be a rest_framework.request.Request.
+        # Note: Only DRF seems to be used in Production.
         request._data = modified_request  # pylint: disable=protected-access
+        set_custom_metric('login_user_request_type', 'drf')
+
     # Include the course ID if it's specified in the analytics info
     # so it can be included in analytics events.
     if "analytics" in modified_request:
@@ -566,6 +572,8 @@ def shim_student_view(view_func, check_logged_in=False):
             msg = response_dict.get("value", u"")
             success = response_dict.get("success")
             set_custom_metric('shim_original_response_is_json', True)
+            set_custom_metric('shim_original_redirect_url', response_dict.get("redirect_url"))
+            set_custom_metric('shim_original_redirect', response_dict.get("redirect"))
         except (ValueError, TypeError):
             msg = response.content
             success = True

openedx/core/djangoapps/user_authn/views/login_form.py

@@ -77,6 +77,20 @@ def _apply_third_party_auth_overrides(request, form_desc):
                 )
 
 
+# .. toggle_name: FEATURES[ENABLE_LOGIN_POST_WITHOUT_SHIM]
+# .. toggle_implementation: DjangoSetting
+# .. toggle_default: False
+# .. toggle_description: Toggle for enabling login post without shim_student_view (using `login_api`).
+# .. toggle_category: n/a
+# .. toggle_use_cases: incremental_release
+# .. toggle_creation_date: 2019-12-10
+# .. toggle_expiration_date: 2020-06-01
+# .. toggle_warnings: n/a
+# .. toggle_tickets: ARCH-1253
+# .. toggle_status: supported
+ENABLE_LOGIN_POST_WITHOUT_SHIM = 'ENABLE_LOGIN_POST_WITHOUT_SHIM'
+
+
 def get_login_session_form(request):
     """Return a description of the login form.
 
@@ -91,7 +105,12 @@ def get_login_session_form(request):
         HttpResponse
 
     """
-    form_desc = FormDescription("post", reverse("user_api_login_session"))
+    if settings.FEATURES.get(ENABLE_LOGIN_POST_WITHOUT_SHIM):
+        submit_url = reverse("login_api")
+    else:
+        submit_url = reverse("user_api_login_session")
+
+    form_desc = FormDescription("post", submit_url)
     _apply_third_party_auth_overrides(request, form_desc)
 
     # Translators: This label appears above a field on the login form

openedx/core/djangoapps/user_authn/views/tests/test_login.py

@@ -34,6 +34,7 @@ from openedx.core.djangoapps.user_authn.views.login import (
     AllowedAuthUser,
     ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY
 )
+from openedx.core.djangoapps.user_authn.views.login_form import ENABLE_LOGIN_POST_WITHOUT_SHIM
 from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client
 from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
 from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
@@ -661,15 +662,26 @@ class LoginSessionViewTest(ApiTestCase):
         response = self.client.patch(self.url)
         self.assertHttpMethodNotAllowed(response)
 
-    def test_login_form(self):
-        # Retrieve the login form
-        response = self.client.get(self.url, content_type="application/json")
-        self.assertHttpOK(response)
+    @ddt.data(
+        {ENABLE_LOGIN_POST_WITHOUT_SHIM: True},
+        {ENABLE_LOGIN_POST_WITHOUT_SHIM: False},
+        {},
+    )
+    def test_login_form(self, features_setting):
+        with patch.dict("django.conf.settings.FEATURES", features_setting):
+            # Retrieve the login form
+            response = self.client.get(self.url, content_type="application/json")
+            self.assertHttpOK(response)
+
+        if ENABLE_LOGIN_POST_WITHOUT_SHIM in features_setting and features_setting[ENABLE_LOGIN_POST_WITHOUT_SHIM]:
+            submit_url = reverse("login_api")
+        else:
+            submit_url = reverse("user_api_login_session")
 
         # Verify that the form description matches what we expect
         form_desc = json.loads(response.content.decode('utf-8'))
         self.assertEqual(form_desc["method"], "post")
-        self.assertEqual(form_desc["submit_url"], self.url)
+        self.assertEqual(form_desc["submit_url"], submit_url)
         self.assertEqual(form_desc["fields"], [
             {
                 "name": "email",