fix: use forgiving jwt authentication

edx_rest_framework_extensions/auth/jwt/authentication.py

@@ -61,18 +61,15 @@ def get_jwt_claim_mergeable_attributes(self):
         return get_setting('JWT_PAYLOAD_MERGEABLE_USER_ATTRIBUTES')
 
     def authenticate(self, request):
-        # Add monitoring to help resolve issues with JWT cookies.
-        # Note: In the very exceptional case that a JWT authorization header is also used
-        #   and takes precedence over the JWT cookies, this custom attribute
-        #   could be slightly misleading.
-        has_jwt_cookie = jwt_cookie_name() in request.COOKIES
-        set_custom_attribute('jwt_auth_has_jwt_cookie', has_jwt_cookie)
-
         is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES)
+        # .. custom_attribute_name: is_forgiving_jwt_cookies_enabled
+        # .. custom_attribute_description: This is temporary custom attribute to show
+        #      whether ENABLE_FORGIVING_JWT_COOKIES is toggled on or off.
+        set_custom_attribute('is_forgiving_jwt_cookies_enabled', is_forgiving_jwt_cookies_enabled)
+
+        # TODO: Robert: Refactor back into this single method in a separate commit
         if is_forgiving_jwt_cookies_enabled:
-            set_custom_attribute('jwt_auth_forgiving_jwt_cookies', True)
             return self._authenticate_forgiving_jwt_cookies(request)
-        set_custom_attribute('jwt_auth_forgiving_jwt_cookies', False)
         return self._authenticate_original(request)
 
     def _authenticate_original(self, request):
@@ -142,9 +139,15 @@ def _authenticate_forgiving_jwt_cookies(self, request):
             # .. custom_attribute_description: Includes a summary of the JWT failure exception
             #       for debugging.
             set_custom_attribute('jwt_auth_failed', 'Exception:{}'.format(repr(exception)))
+            # .. custom_attribute_name: jwt_auth_failure_forgiven
+            # .. custom_attribute_description: This attribute will be True if the JWT failure
+            #      is forgiven. Only JWT cookie failures will be forgiven. In the case of a
+            #      forgiven failure, authenticate will return None rather than raise an
+            #      exception, allowing other authentication classes to process. This attribute
+            #      will be False for failures that are not forgiven.
+            #      See docs/decisions/0002-remove-use-jwt-cookie-header.rst for details.
+            set_custom_attribute('jwt_auth_failure_forgiven', has_jwt_cookie)
             if has_jwt_cookie:
-                # Returning None is how a JWT cookie failure becomes forgiving.
-                # See docs/decisions/0002-remove-use-jwt-cookie-header.rst for details.
                 return None
             raise
 

edx_rest_framework_extensions/auth/jwt/middleware.py

@@ -255,6 +255,11 @@ def process_view(self, request, view_func, view_args, view_kwargs):  # pylint: d
             log.warning(log_message)
 
         has_reconstituted_jwt_cookie = jwt_cookie_name() in request.COOKIES
+        # .. custom_attribute_name: has_jwt_cookie
+        # .. custom_attribute_description: Enables us to see requests which have the full reconstituted
+        #      JWT cookie. If this attribute is missing, it is assumed to be False.
+        monitoring.set_custom_attribute('has_jwt_cookie', has_reconstituted_jwt_cookie)
+
         if has_reconstituted_jwt_cookie and get_setting(ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE):
             # DRF does not set request.user until process_response. This makes it available in process_view.
             # For more info, see https://github.com/jpadilla/django-rest-framework-jwt/issues/45#issuecomment-74996698

edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py

@@ -179,7 +179,12 @@ def test_authenticate_credentials_no_usernames(self):
 
     @mock.patch('edx_rest_framework_extensions.auth.jwt.authentication.set_custom_attribute')
     def test_authenticate_csrf_protected(self, mock_set_custom_attribute):
-        """ Verify authenticate exception for CSRF protected cases. """
+        """
+        Ensure authenticate for JWTs properly handles CSRF errors.
+
+        Note: When using forgiving JWTs, all JWT cookie exceptions, including CSRF, will
+        result in a None so that other authentication classes will also be checked.
+        """
         request = RequestFactory().post('/')
 
         request.META[USE_JWT_COOKIE_HEADER] = 'true'
@@ -191,13 +196,13 @@ def test_authenticate_csrf_protected(self, mock_set_custom_attribute):
         with mock.patch.object(JSONWebTokenAuthentication, 'authenticate', return_value=('mock-user', "mock-auth")):
             if get_setting(ENABLE_FORGIVING_JWT_COOKIES):
                 assert JwtAuthentication().authenticate(request) is None
+                mock_set_custom_attribute.assert_any_call('jwt_auth_failure_forgiven', True)
             else:
                 with self.assertRaises(PermissionDenied) as context_manager:
                     JwtAuthentication().authenticate(request)
-
                 assert context_manager.exception.detail.startswith('CSRF Failed')
 
-        mock_set_custom_attribute.assert_called_with(
+        mock_set_custom_attribute.assert_any_call(
             'jwt_auth_failed',
             "Exception:PermissionDenied('CSRF Failed: CSRF cookie not set.')",
         )
@@ -217,7 +222,8 @@ def test_get_decoded_jwt_from_auth(self, is_jwt_authentication):
         decoded_jwt = authentication.get_decoded_jwt_from_auth(mock_request_with_cookie)
         self.assertEqual(expected_decoded_jwt, decoded_jwt)
 
-    def test_authenticate_with_correct_jwt_authorization(self):
+    @mock.patch('edx_rest_framework_extensions.auth.jwt.authentication.set_custom_attribute')
+    def test_authenticate_with_correct_jwt_authorization(self, mock_set_custom_attribute):
         """
         With JWT header it continues and validates the credentials and throws error.
 
@@ -226,6 +232,10 @@ def test_authenticate_with_correct_jwt_authorization(self):
         jwt_token = self._get_test_jwt_token()
         request = RequestFactory().get('/', HTTP_AUTHORIZATION=f"JWT {jwt_token}")
         assert JwtAuthentication().authenticate(request)
+        mock_set_custom_attribute.assert_any_call(
+            'is_forgiving_jwt_cookies_enabled',
+            get_setting(ENABLE_FORGIVING_JWT_COOKIES)
+        )
 
     def test_authenticate_with_incorrect_jwt_authorization(self):
         """ With JWT header it continues and validates the credentials and throws error. """

edx_rest_framework_extensions/auth/jwt/tests/test_middleware.py

@@ -401,7 +401,7 @@ def setUp(self):
     def test_do_not_use_jwt_cookies(self, mock_set_custom_attribute):
         self.middleware.process_view(self.request, None, None, None)
         self.assertIsNone(self.request.COOKIES.get(jwt_cookie_name()))
-        mock_set_custom_attribute.assert_called_once_with('use_jwt_cookie_requested', False)
+        mock_set_custom_attribute.assert_any_call('use_jwt_cookie_requested', False)
 
     @ddt.data(
         (jwt_cookie_header_payload_name(), jwt_cookie_signature_name()),
@@ -421,14 +421,18 @@ def test_missing_cookies(
             '%s cookie is missing. JWT auth cookies will not be reconstituted.' %
             missing_cookie_name
         )
-        mock_set_custom_attribute.assert_called_once_with('use_jwt_cookie_requested', True)
+        mock_set_custom_attribute.assert_any_call('use_jwt_cookie_requested', True)
+        mock_set_custom_attribute.assert_any_call('has_jwt_cookie', False)
+
 
     @patch('edx_django_utils.monitoring.set_custom_attribute')
     def test_no_cookies(self, mock_set_custom_attribute):
         self.request.META[USE_JWT_COOKIE_HEADER] = 'true'
         self.middleware.process_view(self.request, None, None, None)
         self.assertIsNone(self.request.COOKIES.get(jwt_cookie_name()))
-        mock_set_custom_attribute.assert_called_once_with('use_jwt_cookie_requested', True)
+        mock_set_custom_attribute.assert_any_call('use_jwt_cookie_requested', True)
+        mock_set_custom_attribute.assert_any_call('has_jwt_cookie', False)
+
 
     @patch('edx_django_utils.monitoring.set_custom_attribute')
     def test_success(self, mock_set_custom_attribute):
@@ -437,7 +441,8 @@ def test_success(self, mock_set_custom_attribute):
         self.request.COOKIES[jwt_cookie_signature_name()] = 'signature'
         self.middleware.process_view(self.request, None, None, None)
         self.assertEqual(self.request.COOKIES[jwt_cookie_name()], 'header.payload.signature')
-        mock_set_custom_attribute.assert_called_once_with('use_jwt_cookie_requested', True)
+        mock_set_custom_attribute.assert_any_call('use_jwt_cookie_requested', True)
+        mock_set_custom_attribute.assert_any_call('has_jwt_cookie', True)
 
     _LOG_WARN_AUTHENTICATION_FAILED = 0
     _LOG_WARN_MISSING_JWT_AUTHENTICATION_CLASS = 1