fix: use forgiving jwt authentication

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 `jwt_auth_has_jwt_cookie` for debugging.
* ENABLE_FORGIVING_JWT_COOKIES toggle for rollout of what would
  be a future breaking change, when we have fully switched to
  forgiving jwt cookies.

ARCHBOM-1218

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,52 @@
+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.
+
+Although this would ultimately be a breaking API change for JwtAuthentication (where enabled), the plan would be for the backward incompatibility to be encapsulated and fixed in ``edx-drf-extensions``.
+
+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,10 @@
 from rest_framework import exceptions
 from rest_framework_jwt.authentication import JSONWebTokenAuthentication
 
+from edx_rest_framework_extensions.auth.jwt.cookies import jwt_cookie_name
 from edx_rest_framework_extensions.auth.jwt.constants import USE_JWT_COOKIE_HEADER
 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,7 @@
     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 +133,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 +221,17 @@ 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 adds 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)
+        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 +267,43 @@ 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 +326,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
 

edx_rest_framework_extensions/config.py

@@ -12,3 +12,13 @@
 # .. toggle_warning: This feature fixed ecommerce, but broke edx-platform. The toggle enables us to fix over time.
 # .. toggle_tickets: ARCH-1210, ARCH-1199, ARCH-1197
 ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE = 'ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE'
+
+# .. toggle_name: EDX_DRF_EXTENSIONS[ENABLE_FORGIVING_JWT_COOKIES]
+# .. toggle_implementation: DjangoSetting
+# .. toggle_default: False
+# .. toggle_description: If True, return None rather than an exception when authentication fails with JWT cookies.
+# .. toggle_use_cases: temporary
+# .. toggle_creation_date: 2020-08-12
+# .. toggle_target_removal_date: 2021-12-31
+# .. toggle_tickets: https://openedx.atlassian.net/browse/ARCHBOM-1218
+ENABLE_FORGIVING_JWT_COOKIES = 'ENABLE_FORGIVING_JWT_COOKIES'

edx_rest_framework_extensions/middleware.py

@@ -168,10 +168,12 @@ def _set_request_auth_type_guess_attribute(self, request):
                 auth_type = token_parts[0].lower()  # 'jwt' or 'bearer' (for example)
             else:
                 auth_type = 'other-token-type'
-        elif USE_JWT_COOKIE_HEADER in request.META and jwt_cookie_name() in request.COOKIES:
+        elif jwt_cookie_name() in request.COOKIES:
             auth_type = 'jwt-cookie'
         else:
             auth_type = 'session-or-other'
+        # Note: This is a somewhat odd custom attribute, and could be enhanced with more
+        #   accurate standard custom attributes in our authentication classes.
         monitoring.set_custom_attribute('request_auth_type_guess', auth_type)
 
     AUTHENTICATED_USER_FOUND_CACHE_KEY = 'edx-drf-extensions.authenticated_user_found_in_middleware'

requirements/base.in

@@ -6,6 +6,7 @@ drf-jwt
 django-waffle
 edx-django-utils>=3.8.0      # using new set_custom_attribute method
 edx-opaque-keys
+edx-toggles
 pyjwkest
 pyjwt[crypto]>=2.1.0      # depends on newer jwt.decode and jwt.encode
 python-dateutil>=2.0