fix: use forgiving jwt authentication

.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

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/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'
 
 

edx_rest_framework_extensions/auth/jwt/middleware.py

@@ -23,7 +23,10 @@
     jwt_cookie_name,
     jwt_cookie_signature_name,
 )
-from edx_rest_framework_extensions.config import ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE
+from edx_rest_framework_extensions.config import (
+    ENABLE_FORGIVING_JWT_COOKIES,
+    ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE,
+)
 from edx_rest_framework_extensions.permissions import (
     LoginRedirectIfUnauthenticated,
     NotJwtRestrictedApplication,
@@ -133,8 +136,16 @@ def process_view(self, request, view_func, view_args, view_kwargs):  # pylint: d
         Enables Jwt Authentication for endpoints using the LoginRedirectIfUnauthenticated permission class.
         """
         self._check_and_cache_login_required_found(view_func)
-        if self.is_jwt_auth_enabled_with_login_required(request, view_func):
-            request.META[USE_JWT_COOKIE_HEADER] = 'true'
+
+        # Forgiving JWT cookies is an alternative to the older USE_JWT_COOKIE_HEADER.
+        # If the rollout for forgiving JWT cookies succeeds, we would need to see if
+        #   this middleware could be simplified or replaced by a simpler solution, because
+        #   at least one part of the original solution required this middleware to insert
+        #   the USE_JWT_COOKIE_HEADER.
+        is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES)
+        if not is_forgiving_jwt_cookies_enabled:
+            if self.is_jwt_auth_enabled_with_login_required(request, view_func):
+                request.META[USE_JWT_COOKIE_HEADER] = 'true'
 
     def process_response(self, request, response):
         """
@@ -200,59 +211,59 @@ class JwtAuthCookieMiddleware(MiddlewareMixin):
 
     """
 
-    def _get_missing_cookie_message_and_attribute(self, cookie_name):
-        """ Returns tuple with missing cookie (log_message, custom_attribute_value) """
-        cookie_missing_message = '{} cookie is missing. JWT auth cookies will not be reconstituted.'.format(
+    def _get_missing_cookie_message(self, cookie_name):
+        """ Returns missing cookie log_message """
+        return '{} cookie is missing. JWT auth cookies will not be reconstituted.'.format(
                 cookie_name
         )
-        request_jwt_cookie = f'missing-{cookie_name}'
-        return cookie_missing_message, request_jwt_cookie
 
     # Note: Using `process_view` over `process_request` so JwtRedirectToLoginIfUnauthenticatedMiddleware which
     # uses `process_view` can update the request before this middleware. Method `process_request` happened too early.
     def process_view(self, request, view_func, view_args, view_kwargs):  # pylint: disable=unused-argument
         """
         Reconstitute the full JWT and add a new cookie on the request object.
+
+        Additionally, may add the user to the request to make it available in process_view. (See below.)
         """
         assert hasattr(request, 'session'), "The Django authentication middleware requires session middleware to be installed. Edit your MIDDLEWARE setting to insert 'django.contrib.sessions.middleware.SessionMiddleware'."  # noqa E501 line too long
 
-        use_jwt_cookie_requested = request.META.get(USE_JWT_COOKIE_HEADER)
+        # .. custom_attribute_name: use_jwt_cookie_requested
+        # .. custom_attribute_description: True if USE_JWT_COOKIE_HEADER was found.
+        #   This is a temporary attribute, because this header is being deprecated/removed.
+        monitoring.set_custom_attribute('use_jwt_cookie_requested', bool(request.META.get(USE_JWT_COOKIE_HEADER)))
+
+        if not get_setting(ENABLE_FORGIVING_JWT_COOKIES):
+            if not request.META.get(USE_JWT_COOKIE_HEADER):
+                return
+
         header_payload_cookie = request.COOKIES.get(jwt_cookie_header_payload_name())
         signature_cookie = request.COOKIES.get(jwt_cookie_signature_name())
 
-        is_set_request_user_for_jwt_cookie_enabled = get_setting(ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE)
-        if use_jwt_cookie_requested and is_set_request_user_for_jwt_cookie_enabled:
-            # 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
-            request.user = SimpleLazyObject(lambda: _get_user_from_jwt(request, view_func))
-
-        if not use_jwt_cookie_requested:
-            attribute_value = 'not-requested'
-        elif header_payload_cookie and signature_cookie:
-            # Reconstitute JWT auth cookie if split cookies are available and jwt cookie
-            # authentication was requested by the client.
+        if header_payload_cookie and signature_cookie:
+            # Reconstitute JWT auth cookie if split cookies are available.
             request.COOKIES[jwt_cookie_name()] = '{}{}{}'.format(
                 header_payload_cookie,
                 JWT_DELIMITER,
                 signature_cookie,
             )
-            attribute_value = 'success'
         elif header_payload_cookie or signature_cookie:
             # Log unexpected case of only finding one cookie.
             if not header_payload_cookie:
-                log_message, attribute_value = self._get_missing_cookie_message_and_attribute(
-                    jwt_cookie_header_payload_name()
-                )
+                log_message = self._get_missing_cookie_message(jwt_cookie_header_payload_name())
             if not signature_cookie:
-                log_message, attribute_value = self._get_missing_cookie_message_and_attribute(
-                    jwt_cookie_signature_name()
-                )
+                log_message = self._get_missing_cookie_message(jwt_cookie_signature_name())
             log.warning(log_message)
-        else:
-            attribute_value = 'missing-both'
-            log.warning('Both JWT auth cookies missing. JWT auth cookies will not be reconstituted.')
 
-        monitoring.set_custom_attribute('request_jwt_cookie', attribute_value)
+        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
+            request.user = SimpleLazyObject(lambda: _get_user_from_jwt(request, view_func))
 
 
 def _get_user_from_jwt(request, view_func):
@@ -276,7 +287,7 @@ def _get_user_from_jwt(request, view_func):
                 'Jwt Authentication expected, but view %s is not using a JwtAuthentication class.', view_func
             )
     except Exception:  # pylint: disable=broad-except
-        log.exception('Unknown error attempting to complete Jwt Authentication.')  # pragma: no cover
+        log.exception('Unknown Jwt Authentication error attempting to retrieve the user.')  # pragma: no cover
 
     return user