fix: use forgiving jwt authentication (#197)

The idea behind this work is to use "forgiving" jwt authentication
when working with JWT cookies, which means that if jwt cookie
authentication fails, we would try other authentication mechanisms,
rather than failing and halting the authentication attempt.

This solution would replace an earlier solution to issues around
jwt cookies that introduced an HTTP_USE_JWT_COOKIE header.

For more background and details, see the new ADR:
0002-remove-use-jwt-cookie-header.rst.

Also contains:
- add custom attributes: 
    - is_forgiving_jwt_cookies_enabled
    - use_jwt_cookie_requested
    - has_jwt_cookie
    - jwt_auth_result
- ENABLE_FORGIVING_JWT_COOKIES toggle for rollout of what would
  be a future breaking change, when we have fully switched to
  forgiving jwt cookies.

DEPR for Use-JWT-COOKIE header:
#371

Co-authored-by: Feanil Patel <[email protected]>

.editorconfig

@@ -0,0 +1,100 @@
+# ***************************
+# ** DO NOT EDIT THIS FILE **
+# ***************************
+#
+# This file was generated by edx-lint: https://github.com/openedx/edx-lint
+#
+# If you want to change this file, you have two choices, depending on whether
+# you want to make a local change that applies only to this repo, or whether
+# you want to make a central change that applies to all repos using edx-lint.
+#
+# Note: If your .editorconfig file is simply out-of-date relative to the latest
+# .editorconfig in edx-lint, ensure you have the latest edx-lint installed
+# and then follow the steps for a "LOCAL CHANGE".
+#
+# LOCAL CHANGE:
+#
+# 1. Edit the local .editorconfig_tweaks file to add changes just to this
+#    repo's file.
+#
+# 2. Run:
+#
+#       $ edx_lint write .editorconfig
+#
+# 3. This will modify the local file.  Submit a pull request to get it
+#    checked in so that others will benefit.
+#
+#
+# CENTRAL CHANGE:
+#
+# 1. Edit the .editorconfig file in the edx-lint repo at
+#    https://github.com/openedx/edx-lint/blob/master/edx_lint/files/.editorconfig
+#
+# 2. install the updated version of edx-lint (in edx-lint):
+#
+#       $ pip install .
+#
+# 3. Run (in edx-lint):
+#
+#       $ edx_lint write .editorconfig
+#
+# 4. Make a new version of edx_lint, submit and review a pull request with the
+#    .editorconfig update, and after merging, update the edx-lint version and
+#    publish the new version.
+#
+# 5. In your local repo, install the newer version of edx-lint.
+#
+# 6. Run:
+#
+#       $ edx_lint write .editorconfig
+#
+# 7. This will modify the local file.  Submit a pull request to get it
+#    checked in so that others will benefit.
+#
+#
+#
+#
+#
+# STAY AWAY FROM THIS FILE!
+#
+#
+#
+#
+#
+# SERIOUSLY.
+#
+# ------------------------------
+# Generated by edx-lint version: 5.3.4
+# ------------------------------
+[*]
+end_of_line = lf
+insert_final_newline = true
+charset = utf-8
+indent_style = space
+indent_size = 4
+max_line_length = 120
+trim_trailing_whitespace = true
+
+[{Makefile, *.mk}]
+indent_style = tab
+indent_size = 8
+
+[*.{yml,yaml,json}]
+indent_size = 2
+
+[*.js]
+indent_size = 2
+
+[*.diff]
+trim_trailing_whitespace = false
+
+[.git/*]
+trim_trailing_whitespace = false
+
+[COMMIT_EDITMSG]
+max_line_length = 72
+
+[*.rst]
+max_line_length = 79
+
+# bbcbced841ed335dd8abb7456a6b13485d701b40

CHANGELOG.rst

@@ -12,6 +12,22 @@ Change Log
 Unreleased
 ----------
 
+[8.9.0] - 2023-08-14
+--------------------
+
+Added
+~~~~~
+
+* Added capability to forgive JWT cookie authentication failures as a replacement for the now deprecated ``USE-JWT-COOKIE`` header. See DEPR https://github.com/openedx/edx-drf-extensions/issues/371.
+  * For now, this capability must be enabled using the ``ENABLE_FORGIVING_JWT_COOKIES`` toggle.
+  * Added temporary custom attributes ``is_forgiving_jwt_cookies_enabled`` and ``use_jwt_cookie_requested`` to help with this deprecation.
+* Added custom attributes ``has_jwt_cookie`` and ``jwt_auth_result`` for JWT authentication observability.
+
+Changed
+~~~~~~~
+
+* Two features that were gated on the presence of the ``USE-JWT-COOKIE`` header will now be gated on the presence of a JWT cookie instead, regardless of the state of the new ``ENABLE_FORGIVING_JWT_COOKIES`` toggle. The new behavior should be nearly equivalent in most cases, and should cause no issues in the exceptional cases. The two features include CSRF protection for JWT cookies, and the setting of the request user when ``ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE`` is enabled.
+
 [8.8.0] - 2023-05-16
 --------------------
 

docs/authentication.rst

@@ -1,7 +1,7 @@
 Authentication
 ==============
-Authentication classes are used to associate a request with a user. Unless otherwise noted, all of the classes below
-adhere to the Django `REST Framework's API for authentication classes <http://www.django-rest-framework.org/api-guide/authentication/>`_.
+
+Authentication classes are used to associate a request with a user. Unless otherwise noted, all of the classes below adhere to the Django `REST Framework's API for authentication classes <http://www.django-rest-framework.org/api-guide/authentication/>`_.
 
 .. automodule:: edx_rest_framework_extensions.authentication
     :members:

docs/decisions/0002-remove-use-jwt-cookie-header.rst

@@ -0,0 +1,50 @@
+2. Remove HTTP_USE_JWT_COOKIE Header
+====================================
+
+Status
+------
+
+Accepted
+
+Context
+-------
+
+This ADR explains `why the request header HTTP_USE_JWT_COOKIE`_ was added.
+
+Use of this header has several problems:
+
+* The purpose and need for the ``HTTP_USE_JWT_COOKIE`` header is confusing. It has led to developer confusion when trying to test API calls using JWT cookies, but having the auth fail because they didn't realize this special header was also required.
+* In some cases, the JWT cookies are sent to services, but go unused because of this header. Additional oauth redirects then become required in circumstances where they otherwise wouldn't be needed.
+* Some features have been added, like `JwtRedirectToLoginIfUnauthenticatedMiddleware`_, that can be greatly simplified or possibly removed altogether if the ``HTTP_USE_JWT_COOKIE`` header were retired.
+
+
+Decision
+--------
+
+Replace the `HTTP_USE_JWT_COOKIE` header with forgiving authentication when using JWT cookies. By "forgiving", we mean that JWT authentication would no longer raise exceptions for failed authentication when using JWT cookies, but instead would simply return None.
+
+By returning None from JwtAuthentication, rather than raising an authentication failure, we enable services to move on to other classes, like SessionAuthentication, rather than aborting the authentication process. Failure messages could still be surfaced using `set_custom_metric` for debugging purposes.
+
+Rather than checking for the `HTTP_USE_JWT_COOKIE`, the `JwtAuthCookieMiddleware`_ would always reconstitute the JWT cookie if the parts were available.
+
+The proposal includes protecting all changes with a temporary rollout feature toggle ``ENABLE_FORGIVING_JWT_COOKIES``. This can be used to ensure no harm is done for each service before cleaning up the old header.
+
+.. _JwtAuthCookieMiddleware: https://github.com/edx/edx-drf-extensions/blob/270cf521a72b506d7df595c4c479c7ca232b4bec/edx_rest_framework_extensions/auth/jwt/middleware.py#L164
+
+Consequences
+------------
+
+* Makes authentication simpler, more clear, and more predictable.
+
+  * For example, local testing of endpoints outside of MFEs will use JWT cookies rather than failing, which has been misleading for engineers.
+
+* Greatly simplifies features like `JwtRedirectToLoginIfUnauthenticatedMiddleware`_.
+* Service authentication can take advantage of JWT cookies more often.
+* Services can more consistently take advantage of the JWT payload of the JWT cookie.
+* Additional clean-up when retiring the ``HTTP_USE_JWT_COOKIE`` header will be needed:
+
+  * ``HTTP_USE_JWT_COOKIE`` should be removed from frontend-platform auth code when ready.
+  * ADR that explains `why the request header HTTP_USE_JWT_COOKIE`_ was created should be updated to point to this ADR.
+
+.. _why the request header HTTP_USE_JWT_COOKIE: https://github.com/edx/edx-platform/blob/master/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0009-jwt-in-session-cookie.rst#login---cookie---api
+.. _JwtRedirectToLoginIfUnauthenticatedMiddleware: https://github.com/edx/edx-drf-extensions/blob/270cf521a72b506d7df595c4c479c7ca232b4bec/edx_rest_framework_extensions/auth/jwt/middleware.py#L87

edx_rest_framework_extensions/__init__.py

@@ -1,3 +1,3 @@
 """ edx Django REST Framework extensions. """
 
-__version__ = '8.8.0'  # pragma: no cover
+__version__ = '8.9.0'  # pragma: no cover

edx_rest_framework_extensions/auth/jwt/authentication.py

@@ -8,8 +8,9 @@
 from rest_framework import exceptions
 from rest_framework_jwt.authentication import JSONWebTokenAuthentication
 
-from edx_rest_framework_extensions.auth.jwt.constants import USE_JWT_COOKIE_HEADER
+from edx_rest_framework_extensions.auth.jwt.cookies import jwt_cookie_name
 from edx_rest_framework_extensions.auth.jwt.decoder import configured_jwt_decode_handler
+from edx_rest_framework_extensions.config import ENABLE_FORGIVING_JWT_COOKIES
 from edx_rest_framework_extensions.settings import get_setting
 
 
@@ -59,30 +60,65 @@ def get_jwt_claim_mergeable_attributes(self):
         return get_setting('JWT_PAYLOAD_MERGEABLE_USER_ATTRIBUTES')
 
     def authenticate(self, request):
+        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.
+        #      See docs/decisions/0002-remove-use-jwt-cookie-header.rst
+        set_custom_attribute('is_forgiving_jwt_cookies_enabled', is_forgiving_jwt_cookies_enabled)
+
+        # .. custom_attribute_name: jwt_auth_result
+        # .. custom_attribute_description: The result of the JWT authenticate process,
+        #      which can having the following values:
+        #        'skipped': When JWT Authentication doesn't apply.
+        #        'success-auth-header': Successfully authenticated using the Authorization header.
+        #        'success-cookie': Successfully authenticated using a JWT cookie.
+        #        'forgiven-failure': Returns None instead of failing for JWT cookies. This handles
+        #          the case where expired cookies won't prevent another authentication class, like
+        #          SessionAuthentication, from having a chance to succeed.
+        #          See docs/decisions/0002-remove-use-jwt-cookie-header.rst for details.
+        #        'failed-auth-header': JWT Authorization header authentication failed. This prevents
+        #          other authentication classes from attempting authentication.
+        #        'failed-cookie': JWT cookie authentication failed. This prevents other
+        #          authentication classes from attempting authentication.
+
+        has_jwt_cookie = jwt_cookie_name() in request.COOKIES
         try:
             user_and_auth = super().authenticate(request)
 
             # Unauthenticated, CSRF validation not required
             if not user_and_auth:
+                set_custom_attribute('jwt_auth_result', 'skipped')
                 return user_and_auth
 
-            # Not using JWT cookies, CSRF validation not required
-            use_jwt_cookie_requested = request.META.get(USE_JWT_COOKIE_HEADER)
-            if not use_jwt_cookie_requested:
+            # Not using JWT cookie, CSRF validation not required
+            if not has_jwt_cookie:
+                set_custom_attribute('jwt_auth_result', 'success-auth-header')
                 return user_and_auth
 
             self.enforce_csrf(request)
 
             # CSRF passed validation with authenticated user
+            set_custom_attribute('jwt_auth_result', 'success-cookie')
             return user_and_auth
 
         except Exception as exception:
             # Errors in production do not need to be logged (as they may be noisy),
             # but debug logging can help quickly resolve issues during development.
             logger.debug('Failed JWT Authentication,', exc_info=exception)
-            # Note: I think this case should only include AuthenticationFailed and PermissionDenied,
-            #   but will monitor the custom attribute to verify.
+            # .. custom_attribute_name: jwt_auth_failed
+            # .. custom_attribute_description: Includes a summary of the JWT failure exception
+            #       for debugging.
             set_custom_attribute('jwt_auth_failed', 'Exception:{}'.format(repr(exception)))
+
+            is_jwt_failure_forgiven = is_forgiving_jwt_cookies_enabled and has_jwt_cookie
+            if is_jwt_failure_forgiven:
+                set_custom_attribute('jwt_auth_result', 'forgiven-failure')
+                return None
+            if has_jwt_cookie:
+                set_custom_attribute('jwt_auth_result', 'failed-cookie')
+            else:
+                set_custom_attribute('jwt_auth_result', 'failed-auth-header')
             raise
 
     def authenticate_credentials(self, payload):

edx_rest_framework_extensions/auth/jwt/cookies.py

@@ -8,6 +8,11 @@
 
 
 def jwt_cookie_name():
+    # Warning: This method should probably not supply a default outside
+    # of JWT_AUTH_COOKIE, because JwtAuthentication will never see
+    # the cookie without the setting. This default should probably be
+    # removed, but that would take some further investigation. In the
+    # meantime, this default has been duplicated to test_settings.py.
     return settings.JWT_AUTH.get('JWT_AUTH_COOKIE') or 'edx-jwt-cookie'