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

@@ -9,7 +9,9 @@
 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,6 +61,21 @@ 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)
+        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):
         try:
             user_and_auth = super().authenticate(request)
 
@@ -80,11 +97,54 @@ def authenticate(self, request):
             # 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.
+            # Useful monitoring for debugging various types of failures.
             set_custom_attribute('jwt_auth_failed', 'Exception:{}'.format(repr(exception)))
             raise
 
+    def _authenticate_forgiving_jwt_cookies(self, request):
+        """
+        Authenticate using a JWT token.
+
+        If the JWT is in the authorization header, and authentication fails, we will raise
+        an exception which will halt additional authentication methods, and is the default
+        behavior of DRF authentication classes.
+
+        If the JWT token is found in the JWT cookie, we've had issues with the cookie sometimes
+        containing an expired token. For failed authentication, instead of raising an exception,
+        we will return None which will enable other authentication classes to be attempted where
+        defined (e.g. SessionAuthentication).
+
+        See docs/decisions/0002-remove-use-jwt-cookie-header.rst
+
+        """
+        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:
+                return user_and_auth
+
+            # Not using JWT cookie, CSRF validation not required
+            if not has_jwt_cookie:
+                return user_and_auth
+
+            self.enforce_csrf(request)
+
+            # CSRF passed validation with authenticated user
+            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)
+            # Useful monitoring for debugging various types of failures.
+            set_custom_attribute('jwt_auth_failed', 'Exception:{}'.format(repr(exception)))
+            if has_jwt_cookie:
+                # See docs/decisions/0002-remove-use-jwt-cookie-header.rst for details.
+                return None
+            raise
+
     def authenticate_credentials(self, payload):
         """Get or create an active user with the username contained in the payload."""
         # TODO it would be good to refactor this heavily-nested function.

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):
         """
@@ -213,9 +224,18 @@ def _get_missing_cookie_message_and_attribute(self, cookie_name):
     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
 
+        is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES)
+        if is_forgiving_jwt_cookies_enabled:
+            self._process_view_forgiving_jwt_cookies(request, view_func)
+        else:
+            self._process_view_original(request, view_func)
+
+    def _process_view_original(self, request, view_func):
         use_jwt_cookie_requested = request.META.get(USE_JWT_COOKIE_HEADER)
         header_payload_cookie = request.COOKIES.get(jwt_cookie_header_payload_name())
         signature_cookie = request.COOKIES.get(jwt_cookie_signature_name())
@@ -251,9 +271,44 @@ def process_view(self, request, view_func, view_args, view_kwargs):  # pylint: d
         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)
+
+    def _process_view_forgiving_jwt_cookies(self, request, view_func):
+        header_payload_cookie = request.COOKIES.get(jwt_cookie_header_payload_name())
+        signature_cookie = request.COOKIES.get(jwt_cookie_signature_name())
+
+        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()
+                )
+            if not signature_cookie:
+                log_message, attribute_value = self._get_missing_cookie_message_and_attribute(
+                    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_jwt_cookie = jwt_cookie_name() in request.COOKIES
+        is_set_request_user_for_jwt_cookie_enabled = get_setting(ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE)
+        if has_jwt_cookie 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))
+
 
 def _get_user_from_jwt(request, view_func):
     user = get_user(request)
@@ -276,7 +331,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