diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 73521de0f5..c2af79b603 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/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.""" diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index 477aa7d197..7fe1f004ec 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/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( + '
  • {msg}
    • ', { + 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,' + diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 268b5f5365..32a96e6c5c 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/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 diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index 6397103850..faeab3ef95 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/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 diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 49e6d84cda..df33328efa 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/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",