Commit cc07afb9 by Will Daly

Use a 403 status to indicate that the user does not have a linked

account for third party auth.
parent b14a1291
...@@ -952,7 +952,7 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un ...@@ -952,7 +952,7 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un
platform_name=settings.PLATFORM_NAME platform_name=settings.PLATFORM_NAME
), ),
content_type="text/plain", content_type="text/plain",
status=401 status=403
) )
else: else:
......
...@@ -220,7 +220,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): ...@@ -220,7 +220,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase):
def assert_json_failure_response_is_missing_social_auth(self, response): def assert_json_failure_response_is_missing_social_auth(self, response):
"""Asserts failure on /login for missing social auth looks right.""" """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) 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): def assert_json_failure_response_is_username_collision(self, response):
......
...@@ -372,13 +372,20 @@ def shim_student_view(view_func, check_logged_in=False): ...@@ -372,13 +372,20 @@ def shim_student_view(view_func, check_logged_in=False):
and request.user.is_authenticated() and request.user.is_authenticated()
) )
if check_logged_in and not is_authenticated: if check_logged_in and not is_authenticated:
# Preserve the 401 status code so the client knows # If we get a 403 status code from the student view
# that the user successfully authenticated with third-party auth # this means we've successfully authenticated with a
# but does not have a linked account. # third party provider, but we don't have a linked
# Otherwise, send a 403 to indicate that the login failed. # EdX account. Send a helpful error code so the client
if response.status_code != 401: # 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.status_code = 403
response.content = msg response.content = msg
# If the view wants to redirect us, send a status 302 # If the view wants to redirect us, send a status 302
elif redirect_url is not None: elif redirect_url is not None:
...@@ -397,9 +404,6 @@ def shim_student_view(view_func, check_logged_in=False): ...@@ -397,9 +404,6 @@ def shim_student_view(view_func, check_logged_in=False):
# If the response is successful, then return the content # If the response is successful, then return the content
# of the response directly rather than including it # of the response directly rather than including it
# in a JSON-serialized dictionary. # 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: else:
response.content = msg response.content = msg
......
...@@ -144,14 +144,14 @@ class StudentViewShimTest(TestCase): ...@@ -144,14 +144,14 @@ class StudentViewShimTest(TestCase):
self.assertNotIn("enrollment_action", self.captured_request.POST) self.assertNotIn("enrollment_action", self.captured_request.POST)
self.assertNotIn("course_id", self.captured_request.POST) self.assertNotIn("course_id", self.captured_request.POST)
@ddt.data(True, False) def test_third_party_auth_login_failure(self):
def test_preserve_401_status(self, check_logged_in):
view = self._shimmed_view( view = self._shimmed_view(
HttpResponse(status=401), HttpResponse(status=403),
check_logged_in=check_logged_in check_logged_in=True
) )
response = view(HttpRequest()) 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): def test_non_json_response(self):
view = self._shimmed_view(HttpResponse(content="Not a JSON dict")) view = self._shimmed_view(HttpResponse(content="Not a JSON dict"))
......
...@@ -117,9 +117,10 @@ class LoginSessionView(APIView): ...@@ -117,9 +117,10 @@ class LoginSessionView(APIView):
Returns: Returns:
HttpResponse: 200 on success HttpResponse: 200 on success
HttpResponse: 400 if the request is not valid. 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. 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. HttpResponse: 302 if redirecting to another page.
Example Usage: Example Usage:
......
...@@ -83,13 +83,13 @@ var edx = edx || {}; ...@@ -83,13 +83,13 @@ var edx = edx || {};
saveError: function( error ) { saveError: function( error ) {
console.log(error.status, ' error: ', error.responseText); 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 * authenticated with a third-party provider, but we haven't
* linked the account to an EdX account. In this case, * linked the account to an EdX account. In this case,
* we need to prompt the user to enter a little more information * we need to prompt the user to enter a little more information
* to complete the registration process. * 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"); this.$alreadyAuthenticatedMsg.removeClass("hidden");
} }
else { else {
......
...@@ -50,7 +50,7 @@ ...@@ -50,7 +50,7 @@
$('#login-form').on('ajax:error', function(event, request, status_string) { $('#login-form').on('ajax:error', function(event, request, status_string) {
toggleSubmitButton(true); toggleSubmitButton(true);
if (request.status === 401) { if (request.status === 403) {
$('.message.submission-error').removeClass('is-shown'); $('.message.submission-error').removeClass('is-shown');
$('.third-party-signin.message').addClass('is-shown').focus(); $('.third-party-signin.message').addClass('is-shown').focus();
$('.third-party-signin.message .instructions').html(request.responseText); $('.third-party-signin.message .instructions').html(request.responseText);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment