Commit 02485be3 by Will Daly

Clean up redirect logic

parent 34dd4e59
...@@ -323,9 +323,6 @@ def shim_student_view(view_func, check_logged_in=False): ...@@ -323,9 +323,6 @@ def shim_student_view(view_func, check_logged_in=False):
(instead of always using status 200, but setting "success" to False in (instead of always using status 200, but setting "success" to False in
the JSON-serialized content of the response) the JSON-serialized content of the response)
* Use status code 302 for redirects instead of
"redirect_url" in the JSON-serialized content of the response.
* Use status code 403 to indicate a login failure. * Use status code 403 to indicate a login failure.
The shim will preserve any cookies set by the view. The shim will preserve any cookies set by the view.
...@@ -389,17 +386,30 @@ def shim_student_view(view_func, check_logged_in=False): ...@@ -389,17 +386,30 @@ def shim_student_view(view_func, check_logged_in=False):
# Most responses from this view are JSON-encoded # Most responses from this view are JSON-encoded
# dictionaries with keys "success", "value", and # dictionaries with keys "success", "value", and
# (sometimes) "redirect_url". # (sometimes) "redirect_url".
#
# We want to communicate some of this information # We want to communicate some of this information
# using HTTP status codes instead. # using HTTP status codes instead.
#
# We ignore the "redirect_url" parameter, because we don't need it:
# 1) It's used to redirect on change enrollment, which
# our client will handle directly
# (that's why we strip out the enrollment params from the request)
# 2) It's used by third party auth when a user has already successfully
# authenticated and we're not sending login credentials. However,
# this case is never encountered in practice: on the old login page,
# the login form would be submitted directly, so third party auth
# would always be "trumped" by first party auth. If a user has
# successfully authenticated with us, we redirect them to the dashboard
# regardless of how they authenticated; and if a user is completing
# the third party auth pipeline, we redirect them from the pipeline
# completion end-point directly.
try: try:
response_dict = json.loads(response.content) response_dict = json.loads(response.content)
msg = response_dict.get("value", u"") msg = response_dict.get("value", u"")
redirect_url = response_dict.get("redirect_url") or response_dict.get("redirect")
success = response_dict.get("success") success = response_dict.get("success")
except (ValueError, TypeError): except (ValueError, TypeError):
msg = response.content msg = response.content
success = True success = True
redirect_url = None
# If the user is not authenticated when we expect them to be # If the user is not authenticated when we expect them to be
# send the appropriate status code. # send the appropriate status code.
...@@ -426,11 +436,6 @@ def shim_student_view(view_func, check_logged_in=False): ...@@ -426,11 +436,6 @@ def shim_student_view(view_func, check_logged_in=False):
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
elif redirect_url is not None:
response.status_code = 302
response['Location'] = redirect_url
# If an error condition occurs, send a status 400 # If an error condition occurs, send a status 400
elif response.status_code != 200 or not success: elif response.status_code != 200 or not success:
# The student views tend to send status 200 even when an error occurs # The student views tend to send status 200 even when an error occurs
......
...@@ -177,7 +177,7 @@ class StudentViewShimTest(TestCase): ...@@ -177,7 +177,7 @@ class StudentViewShimTest(TestCase):
self.assertEqual(response.content, "Not a JSON dict") self.assertEqual(response.content, "Not a JSON dict")
@ddt.data("redirect", "redirect_url") @ddt.data("redirect", "redirect_url")
def test_redirect_from_json(self, redirect_key): def test_ignore_redirect_from_json(self, redirect_key):
view = self._shimmed_view( view = self._shimmed_view(
HttpResponse(content=json.dumps({ HttpResponse(content=json.dumps({
"success": True, "success": True,
...@@ -185,8 +185,8 @@ class StudentViewShimTest(TestCase): ...@@ -185,8 +185,8 @@ class StudentViewShimTest(TestCase):
})) }))
) )
response = view(HttpRequest()) response = view(HttpRequest())
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 200)
self.assertEqual(response['Location'], "/redirect") self.assertEqual(response.content, "")
def test_error_from_json(self): def test_error_from_json(self):
view = self._shimmed_view( view = self._shimmed_view(
......
...@@ -57,9 +57,5 @@ var edx = edx || {}; ...@@ -57,9 +57,5 @@ var edx = edx || {};
saveSuccess: function() { saveSuccess: function() {
this.trigger('auth-complete'); this.trigger('auth-complete');
}, },
redirect: function( url ) {
window.location.href = url;
}
}); });
})(jQuery, _, gettext); })(jQuery, _, gettext);
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