fix: fix redirect unauthenticated users to login middleware for TPA [BB-5090]

openedx/core/djangoapps/user_authn/middleware.py

@@ -17,14 +17,20 @@ class RedirectUnauthenticatedToLoginMiddleware:
     register pages.
 
     If redirects, passes the requested url to login as 'next' query string
-    parameter.
+    parameter. If the original url has a 'tpa_hint' query parameter, the
+    parameter is passed to login without being escaped.
 
     To enable the middleware, ENABLE_REDIRECT_UNAUTHENTICATED_USERS_TO_LOGIN
     setting has to be set to True.
 
     Assumed that the requests passed to the middleware have user attribute set.
     """
 
+    WHITELIST = [
+        '/admin/',
+        '/auth/',
+    ]
+
     def __init__(self, get_response):
         self.get_response = get_response
 
@@ -43,10 +49,12 @@ def _should_redirect(self, request):
 
         is_get_request = request.method == 'GET'
         is_login_or_register_url = request.path in (settings.LOGIN_URL, '/register')
+        is_in_whitelist = self._check_whitelist(request.path)
 
         return (
             is_get_request and
             (not is_login_or_register_url) and
+            (not is_in_whitelist) and
             (not request.user.is_authenticated)
         )
 
@@ -55,6 +63,20 @@ def _get_redirect_query_string(self, request):
         Generates query string for redirect.
         """
         # calling copy to get mutable QueryDict
-        query = QueryDict().copy()
-        query['next'] = request.get_full_path()
-        return query.urlencode()
+        request_query = request.GET.copy()
+        redirect_query = QueryDict().copy()
+
+        if 'tpa_hint' in request_query:
+            redirect_query.setlist('tpa_hint', request_query.pop('tpa_hint'))
+        redirect_query['next'] = request.path + '?' + request_query.urlencode()
+
+        return redirect_query.urlencode()
+
+    def _check_whitelist(self, path):
+        """
+        Check if path is whitelisted.
+        """
+        return any(map(
+            lambda pattern: path.startswith(pattern),
+            RedirectUnauthenticatedToLoginMiddleware.WHITELIST,
+        ))

openedx/core/djangoapps/user_authn/tests/test_middlewares.py

@@ -73,6 +73,27 @@ def test_passes_url_in_next_query_string(self):
         self.assertIn('next', query)
         self.assertEqual(query['next'], '/dashboard?test=123')
 
+    def test_passes_tpa_hint_parameter_to_login(self):
+        """
+        Middleware passes 'tpa_hint' query parameter to login.
+
+        When redirecting, if the original url has 'tpa_hint' query string
+        parameter, the middleware does not pass the parameter throug the 'next'
+        query string parameter, doesn't escape it, and passes it to login as
+        normal query string parameter.
+        """
+        request = RequestFactory().get('/dashboard?tpa_hint=test-tpa&test=123')
+        request.user = AnonymousUserFactory.create()
+
+        response = self.middleware(request)
+
+        self.assertNotEqual(response, self.mock_response)
+        query = QueryDict(urlparse(response.url).query)
+        self.assertIn('next', query)
+        self.assertEqual(query['next'], '/dashboard?test=123')
+        self.assertIn('tpa_hint', query)
+        self.assertEqual(query['tpa_hint'], 'test-tpa')
+
     def test_does_not_redirect_if_user_is_authenticated(self):
         """
         Middleware doesn't redirect authenticated users.
@@ -120,3 +141,15 @@ def test_does_not_redirect_unauthenticated_user_if_setting_disabled(self):
         response = self.middleware(request)
 
         self.assertEqual(response, self.mock_response)
+
+    @ddt.data(*RedirectUnauthenticatedToLoginMiddleware.WHITELIST)
+    def test_does_not_redirect_unauthenticated_user_if_path_in_whitelist(self, whitelisted_url):
+        """
+        Middleware doesn't redirect if requested path is whitelisted.
+        """
+        request = RequestFactory().get(whitelisted_url + 'test')
+        request.user = AnonymousUserFactory.create()
+
+        response = self.middleware(request)
+
+        self.assertEqual(response, self.mock_response)