diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 902095ae36..ea05c3bf13 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -952,7 +952,7 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un platform_name=settings.PLATFORM_NAME ), content_type="text/plain", - status=401 + status=403 ) else: diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 6d5ec1e353..3d9e3efa85 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -220,7 +220,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): def assert_json_failure_response_is_missing_social_auth(self, response): """Asserts failure on /login for missing social auth looks right.""" - self.assertEqual(401, response.status_code) + self.assertEqual(403, response.status_code) self.assertIn("successfully logged into your %s account, but this account isn't linked" % self.PROVIDER_CLASS.NAME, response.content) def assert_json_failure_response_is_username_collision(self, response): diff --git a/common/djangoapps/user_api/helpers.py b/common/djangoapps/user_api/helpers.py index f98f8996a8..2c9b0c41ab 100644 --- a/common/djangoapps/user_api/helpers.py +++ b/common/djangoapps/user_api/helpers.py @@ -372,13 +372,20 @@ def shim_student_view(view_func, check_logged_in=False): and request.user.is_authenticated() ) if check_logged_in and not is_authenticated: - # Preserve the 401 status code so the client knows - # that the user successfully authenticated with third-party auth - # but does not have a linked account. - # Otherwise, send a 403 to indicate that the login failed. - if response.status_code != 401: + # If we get a 403 status code from the student view + # this means we've successfully authenticated with a + # third party provider, but we don't have a linked + # EdX account. Send a helpful error code so the client + # knows this occurred. + if response.status_code == 403: + response.content = "third-party-auth" + + # Otherwise, it's a general authentication failure. + # Ensure that the status code is a 403 and pass + # along the message from the view. + else: response.status_code = 403 - response.content = msg + response.content = msg # If the view wants to redirect us, send a status 302 elif redirect_url is not None: @@ -397,9 +404,6 @@ def shim_student_view(view_func, check_logged_in=False): # If the response is successful, then return the content # of the response directly rather than including it # in a JSON-serialized dictionary. - # This will also preserve error status codes such as a 401 - # (if the user is trying to log in using a third-party provider - # but hasn't yet linked his or her account.) else: response.content = msg diff --git a/common/djangoapps/user_api/tests/test_helpers.py b/common/djangoapps/user_api/tests/test_helpers.py index e66cf777ca..257d7e78ad 100644 --- a/common/djangoapps/user_api/tests/test_helpers.py +++ b/common/djangoapps/user_api/tests/test_helpers.py @@ -144,14 +144,14 @@ class StudentViewShimTest(TestCase): self.assertNotIn("enrollment_action", self.captured_request.POST) self.assertNotIn("course_id", self.captured_request.POST) - @ddt.data(True, False) - def test_preserve_401_status(self, check_logged_in): + def test_third_party_auth_login_failure(self): view = self._shimmed_view( - HttpResponse(status=401), - check_logged_in=check_logged_in + HttpResponse(status=403), + check_logged_in=True ) response = view(HttpRequest()) - self.assertEqual(response.status_code, 401) + self.assertEqual(response.status_code, 403) + self.assertEqual(response.content, "third-party-auth") def test_non_json_response(self): view = self._shimmed_view(HttpResponse(content="Not a JSON dict")) diff --git a/common/djangoapps/user_api/views.py b/common/djangoapps/user_api/views.py index a0a8cfd19a..1e0630a65b 100644 --- a/common/djangoapps/user_api/views.py +++ b/common/djangoapps/user_api/views.py @@ -117,9 +117,10 @@ class LoginSessionView(APIView): Returns: HttpResponse: 200 on success HttpResponse: 400 if the request is not valid. - HttpResponse: 401 if the user successfully authenticated with a third-party - provider but does not have a linked account. HttpResponse: 403 if authentication failed. + 403 with content "third-party-auth" if the user + has successfully authenticated with a third party provider + but does not have a linked account. HttpResponse: 302 if redirecting to another page. Example Usage: diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index b2f4a850b6..cf16f32662 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/lms/static/js/student_account/views/LoginView.js @@ -83,13 +83,13 @@ var edx = edx || {}; saveError: function( error ) { console.log(error.status, ' error: ', error.responseText); - /* If we've gotten a 401 error, it means that we've successfully + /* If we've gotten a 403 error, it means that we've successfully * authenticated with a third-party provider, but we haven't * linked the account to an EdX account. In this case, * we need to prompt the user to enter a little more information * to complete the registration process. - */ - if (error.status === 401 && this.currentProvider) { + */ + if (error.status === 403 && error.responseText === "third-party-auth" && this.currentProvider) { this.$alreadyAuthenticatedMsg.removeClass("hidden"); } else { diff --git a/lms/templates/login.html b/lms/templates/login.html index e217496e3a..deb766abe3 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -50,7 +50,7 @@ $('#login-form').on('ajax:error', function(event, request, status_string) { toggleSubmitButton(true); - if (request.status === 401) { + if (request.status === 403) { $('.message.submission-error').removeClass('is-shown'); $('.third-party-signin.message').addClass('is-shown').focus(); $('.third-party-signin.message .instructions').html(request.responseText);