Merge pull request #5930 from edx/will/logistration-handle-redirects

Logistration: Clean up redirect logic

common/djangoapps/user_api/helpers.py

@@ -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
         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.
 
     The shim will preserve any cookies set by the view.
@@ -389,17 +386,30 @@ def shim_student_view(view_func, check_logged_in=False):
         # Most responses from this view are JSON-encoded
         # dictionaries with keys "success", "value", and
         # (sometimes) "redirect_url".
+        #
         # We want to communicate some of this information
         # 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:
             response_dict = json.loads(response.content)
             msg = response_dict.get("value", u"")
-            redirect_url = response_dict.get("redirect_url") or response_dict.get("redirect")
             success = response_dict.get("success")
         except (ValueError, TypeError):
             msg = response.content
             success = True
-            redirect_url = None
 
         # If the user is not authenticated when we expect them to be
         # send the appropriate status code.
@@ -426,11 +436,6 @@ def shim_student_view(view_func, check_logged_in=False):
                 response.status_code = 403
                 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
         elif response.status_code != 200 or not success:
             # The student views tend to send status 200 even when an error occurs

common/djangoapps/user_api/tests/test_helpers.py

@@ -177,7 +177,7 @@ class StudentViewShimTest(TestCase):
         self.assertEqual(response.content, "Not a JSON dict")
 
     @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(
             HttpResponse(content=json.dumps({
                 "success": True,
@@ -185,8 +185,8 @@ class StudentViewShimTest(TestCase):
             }))
         )
         response = view(HttpRequest())
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(response['Location'], "/redirect")
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.content, "")
 
     def test_error_from_json(self):
         view = self._shimmed_view(

lms/static/js/student_account/views/RegisterView.js

@@ -57,9 +57,5 @@ var edx = edx || {};
         saveSuccess: function() {
             this.trigger('auth-complete');
         },
-
-        redirect: function( url ) {
-            window.location.href = url;
-        }
     });
 })(jQuery, _, gettext);