From cb84bbdd64ea506d4b58bca3d4d3aa9afa0ec4a1 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 25 Oct 2023 15:41:35 -0400 Subject: [PATCH] feat: update jwt vs session user monitoring 1. An issue was found when both ENABLE_JWT_VS_SESSION_USER_CHECK and ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE were enabled at the same time in an ecommerce stage environment for edx.org, that is not reproducible locally. Additionally, the issue killed the requests without providing errors, so potentially caused an infinite loop or some such issue. The guess is that the code for setting the user behind ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE, and the code to get the user in the new ENABLE_JWT_VS_SESSION_USER_CHECK check, were somehow in conflict. This update skips the JWT vs Session user check in the case that we have set the user based on the JWT, in which case it would be a JWT vs JWT check, which not only is unnecessary, but also may be the cause of the issue. 2. Also adds custom attributes - set_user_from_jwt_status - skip_jwt_vs_session_check 3. Additionally, enforces JWT and session user matching when the toggle ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE is enabled, because otherwise the user in the middleware might not match the user during JWT authentication. 4. Since the JWT cookie vs session user check is an intimate part of how ENABLE_FORGIVING_JWT_COOKIES should function appropriately, this removes ENABLE_JWT_VS_SESSION_USER_CHECK as a toggle and these checks will be performed whenever ENABLE_FORGIVING_JWT_COOKIES is enabled. This makes the ENABLE_FORGIVING_JWT_COOKIES toggle more safe, because it can no longer be enabled without this required functionality. Note that the earlier tests did not cover all combinations of ENABLE_FORGIVING_JWT_COOKIES and ENABLE_JWT_VS_SESSION_USER_CHECK being disabled and enabled, and the updated tests better cover both cases for the remaining toggle. 5. Lastly, the ADR was updated to explain that various cases regarding handling of a JWT cookie user and session user mismatch. --- CHANGELOG.rst | 19 ++ .../0002-remove-use-jwt-cookie-header.rst | 20 ++ edx_rest_framework_extensions/__init__.py | 2 +- .../auth/jwt/authentication.py | 116 ++++++++-- .../auth/jwt/middleware.py | 90 +++++++- .../auth/jwt/tests/test_authentication.py | 205 ++++++++++++------ .../auth/jwt/tests/test_middleware.py | 77 +++++-- edx_rest_framework_extensions/config.py | 14 -- edx_rest_framework_extensions/settings.py | 2 - 9 files changed, 412 insertions(+), 133 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7b2d16c3..5e6c033f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,25 @@ Change Log Unreleased ---------- +[8.13.0] - 2023-10-30 +--------------------- + +Fixed +~~~~~ +* Bug fix for when both ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE and the JWT cookie user vs session user check behind ENABLE_FORGIVING_JWT_COOKIES were enabled at the same time. + +Added +~~~~~ +* Added custom attributes set_user_from_jwt_status and skip_jwt_vs_session_check. + +Updated +~~~~~~~ +* ADR for removing HTTP_USE_JWT_COOKIE, which explains forgiven JWT cookies, was updated to explain the cases where the JWT cookie user and session user do not match. + +Removed +~~~~~~~ +* Toggle EDX_DRF_EXTENSIONS[ENABLE_JWT_VS_SESSION_USER_CHECK] has been removed. This check is now a default part of the ENABLE_FORGIVING_JWT_COOKIES functionality. ENABLE_JWT_VS_SESSION_USER_CHECK was just a temporary roll-out toggle that was already proven out everywhere ENABLE_FORGIVING_JWT_COOKIES was already enabled. + [8.12.0] - 2023-10-16 --------------------- diff --git a/docs/decisions/0002-remove-use-jwt-cookie-header.rst b/docs/decisions/0002-remove-use-jwt-cookie-header.rst index 69668b3c..0a13ad49 100644 --- a/docs/decisions/0002-remove-use-jwt-cookie-header.rst +++ b/docs/decisions/0002-remove-use-jwt-cookie-header.rst @@ -29,6 +29,15 @@ Rather than checking for the `HTTP_USE_JWT_COOKIE`, the `JwtAuthCookieMiddleware 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. +Unfortunately, there are certain rare cases where the user inside the JWT and the session user do not match: + +- If the JWT cookie succeeds authentication, and: + + - If ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE is enabled to make the JWT user available to middleware, then we also enforce the that the JWT user and session user match. If they do not, we will fail authentication instead. + - If ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE is disabled, we allow the successful JWT cookie authentication to proceed, even though the session user does not match. We will monitor this situation and may choose to enforce the match and fail instead. + +- If the JWT cookie fails authentication, but the failed JWT contains a user that does not match the session user, authentication will be failed, rather than moving on to SessionAuthentication which would have resulted in authentication for a different user. + .. _JwtAuthCookieMiddleware: https://github.com/edx/edx-drf-extensions/blob/270cf521a72b506d7df595c4c479c7ca232b4bec/edx_rest_framework_extensions/auth/jwt/middleware.py#L164 Consequences @@ -48,3 +57,14 @@ Consequences .. _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 + +Change History +-------------- + +2023-10-30 +~~~~~~~~~~ +* Details added for handling of a variety of situations when the JWT cookie user and the session user do not match. + +2023-08-14 +~~~~~~~~~~ +* Merged original ADR diff --git a/edx_rest_framework_extensions/__init__.py b/edx_rest_framework_extensions/__init__.py index d00558f4..0e576376 100644 --- a/edx_rest_framework_extensions/__init__.py +++ b/edx_rest_framework_extensions/__init__.py @@ -1,3 +1,3 @@ """ edx Django REST Framework extensions. """ -__version__ = '8.12.0' # pragma: no cover +__version__ = '8.13.0' # pragma: no cover diff --git a/edx_rest_framework_extensions/auth/jwt/authentication.py b/edx_rest_framework_extensions/auth/jwt/authentication.py index 4cb5d2e5..ec2b84a0 100644 --- a/edx_rest_framework_extensions/auth/jwt/authentication.py +++ b/edx_rest_framework_extensions/auth/jwt/authentication.py @@ -4,6 +4,7 @@ from django.contrib.auth import get_user_model from django.middleware.csrf import CsrfViewMiddleware +from edx_django_utils.cache import RequestCache from edx_django_utils.monitoring import set_custom_attribute from jwt import exceptions as jwt_exceptions from rest_framework import exceptions @@ -15,7 +16,7 @@ ) from edx_rest_framework_extensions.config import ( ENABLE_FORGIVING_JWT_COOKIES, - ENABLE_JWT_VS_SESSION_USER_CHECK, + ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE, ) from edx_rest_framework_extensions.settings import get_setting @@ -23,6 +24,16 @@ logger = logging.getLogger(__name__) +class JwtAuthenticationError(Exception): + """ + Custom base class for all exceptions + """ + + +class JwtSessionUserMismatchError(JwtAuthenticationError): + pass + + class CSRFCheck(CsrfViewMiddleware): def _reject(self, request, reason): # Return the failure reason instead of an HttpResponse @@ -87,6 +98,11 @@ def authenticate(self, request): # other authentication classes from attempting authentication. # 'failed-cookie': JWT cookie authentication failed. This prevents other # authentication classes from attempting authentication. + # 'user-mismatch-failure': JWT vs session user mismatch found for what would have been + # a forgiven-failure, but instead, the JWT failure will be final. + # 'user-mismatch-enforced-failure': JWT vs session user mismatch found for what would + # have been a successful JWT authentication, but we are enforcing a match, and thus + # we fail authentication. is_authenticating_with_jwt_cookie = self.is_authenticating_with_jwt_cookie(request) try: @@ -105,18 +121,29 @@ def authenticate(self, request): self.enforce_csrf(request) # CSRF passed validation with authenticated user + + # adds additional monitoring for mismatches; and raises errors in certain cases + self._monitor_or_enforce_successful_jwt_cookie_and_session_user_mismatch( + request, jwt_user_id=user_and_auth[0].id + ) set_custom_attribute('jwt_auth_result', 'success-cookie') - # adds additional monitoring for mismatches - self._monitor_successful_jwt_cookie_and_session_user_mismatch(request, jwt_user_id=user_and_auth[0].id) 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) + except JwtSessionUserMismatchError as exception: + # Warn against these errors because JWT vs session user should not be happening. + logger.warning('Failed JWT Authentication due to session user mismatch.') # .. 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))) + set_custom_attribute('jwt_auth_result', 'user-mismatch-enforced-failure') + raise + + 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) + exception_to_report = _deepest_jwt_exception(exception) set_custom_attribute('jwt_auth_failed', 'Exception:{}'.format(repr(exception_to_report))) @@ -256,15 +283,23 @@ def _is_failed_jwt_cookie_and_session_user_mismatch(self, request): return self._is_jwt_cookie_and_session_user_mismatch(request, jwt_user_id) - def _monitor_successful_jwt_cookie_and_session_user_mismatch(self, request, jwt_user_id): + def _monitor_or_enforce_successful_jwt_cookie_and_session_user_mismatch(self, request, jwt_user_id): """ - Provides monitoring when a successful JWT cookie and session user do not match. + Provides monitoring and possible enforcement when a successful JWT cookie and session user do not match. Notes: - Must only be called in the case of a successful JWT cookie. - Also provides monitoring details for mismatches. + - In the case where ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE is being used, we trigger a failure for + what otherwise would have been a successful authentication. """ - self._is_jwt_cookie_and_session_user_mismatch(request, jwt_user_id) + is_mismatch = self._is_jwt_cookie_and_session_user_mismatch(request, jwt_user_id) + if is_mismatch and get_setting(ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE): + # For failed_jwt_cookie_user_id docs, see custom attribute annotations elsewhere + set_custom_attribute('failed_jwt_cookie_user_id', jwt_user_id) + raise JwtSessionUserMismatchError( + 'Failing otherwise successful JWT authentication due to session user mismatch with set request user.' + ) def _is_jwt_cookie_and_session_user_mismatch(self, request, jwt_user_id): """ @@ -275,22 +310,30 @@ def _is_jwt_cookie_and_session_user_mismatch(self, request, jwt_user_id): jwt_user_id (int): The user_id of the JWT, None if not found. Other notes: - - If ENABLE_JWT_VS_SESSION_USER_CHECK is toggled off, always return False. + - If ENABLE_FORGIVING_JWT_COOKIES is toggled off, always return False. - Also adds monitoring details for mismatches. - Should only be called for JWT cookies. """ - is_jwt_vs_session_user_check_enabled = get_setting(ENABLE_JWT_VS_SESSION_USER_CHECK) - # .. custom_attribute_name: is_jwt_vs_session_user_check_enabled - # .. custom_attribute_description: This is temporary custom attribute to show - # whether ENABLE_JWT_VS_SESSION_USER_CHECK is toggled on or off. - set_custom_attribute('is_jwt_vs_session_user_check_enabled', is_jwt_vs_session_user_check_enabled) - if not is_jwt_vs_session_user_check_enabled: + is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES) + # This toggle provides a temporary safety valve for rollout. + if not is_forgiving_jwt_cookies_enabled: return False - has_request_user = ( - hasattr(request, '_request') and hasattr(request._request, 'user') # pylint: disable=protected-access - ) - if not has_request_user: # pragma: no cover + # If we set the request user in middleware for JWT auth, then we'd actually be checking JWT vs JWT user id. + # Additionally, somehow the setting of request.user and the retrieving of request.user below causes some + # unknown issue in production-like environments, and this allows us to skip that case. + if _is_request_user_set_for_jwt_auth(): + # .. custom_attribute_name: skip_jwt_vs_session_check + # .. custom_attribute_description: This is temporary custom attribute to show that we skipped the check. + # This is probably redundant with the custom attribute set_user_from_jwt_status, but temporarily + # adding during initial rollout. + set_custom_attribute('skip_jwt_vs_session_check', True) + return False + + # Get the session-based user from the underlying HttpRequest object. + # This line taken from DRF SessionAuthentication. + user = getattr(request._request, 'user', None) # pylint: disable=protected-access + if not user: # pragma: no cover # .. custom_attribute_name: jwt_auth_request_user_not_found # .. custom_attribute_description: This custom attribute shows when a # session user was not found during JWT cookie authentication. This @@ -298,9 +341,8 @@ def _is_jwt_cookie_and_session_user_mismatch(self, request, jwt_user_id): set_custom_attribute('jwt_auth_request_user_not_found', True) return False - wsgi_request_user = request._request.user # pylint: disable=protected-access - if wsgi_request_user and wsgi_request_user.is_authenticated: - session_user_id = wsgi_request_user.id + if user.is_authenticated: + session_user_id = user.id else: session_user_id = None @@ -320,6 +362,19 @@ def _is_jwt_cookie_and_session_user_mismatch(self, request, jwt_user_id): return True +_IS_REQUEST_USER_SET_FOR_JWT_AUTH_CACHE_KEY = '_is_request_user_for_jwt_set' + + +def set_flag_is_request_user_set_for_jwt_auth(): + """ + Sets a flag that the shows the request user was set to be based on JWT auth. + + Used to coordinate between middleware and JwtAuthentication. Note that the flag + is stored in this module to avoid circular dependencies. + """ + _get_module_request_cache()[_IS_REQUEST_USER_SET_FOR_JWT_AUTH_CACHE_KEY] = True + + def is_jwt_authenticated(request): successful_authenticator = getattr(request, 'successful_authenticator', None) if not isinstance(successful_authenticator, JSONWebTokenAuthentication): @@ -364,3 +419,16 @@ def _deepest_jwt_exception(exception): relevant_exception = cur_exception return relevant_exception + + +def _get_module_request_cache(): + return RequestCache(__name__).data + + +def _is_request_user_set_for_jwt_auth(): + """ + Returns whether the request user was set to be based on JWT auth in JwtAuthCookieMiddleware. + + This is a public method to enable coordination with the JwtAuthentication class. + """ + return _get_module_request_cache().get(_IS_REQUEST_USER_SET_FOR_JWT_AUTH_CACHE_KEY, False) diff --git a/edx_rest_framework_extensions/auth/jwt/middleware.py b/edx_rest_framework_extensions/auth/jwt/middleware.py index b403c756..932be78f 100644 --- a/edx_rest_framework_extensions/auth/jwt/middleware.py +++ b/edx_rest_framework_extensions/auth/jwt/middleware.py @@ -5,15 +5,19 @@ from django.contrib.auth.decorators import login_required from django.contrib.auth.middleware import get_user +from django.contrib.auth.models import AnonymousUser from django.utils.deprecation import MiddlewareMixin from django.utils.functional import SimpleLazyObject -from edx_django_utils import monitoring from edx_django_utils.cache import RequestCache +from edx_django_utils.monitoring import set_custom_attribute from rest_framework.permissions import OperandHolder, SingleOperandHolder from rest_framework.request import Request from rest_framework.settings import api_settings from rest_framework_jwt.authentication import JSONWebTokenAuthentication +from edx_rest_framework_extensions.auth.jwt.authentication import ( + set_flag_is_request_user_set_for_jwt_auth, +) from edx_rest_framework_extensions.auth.jwt.constants import ( JWT_DELIMITER, USE_JWT_COOKIE_HEADER, @@ -210,7 +214,6 @@ class JwtAuthCookieMiddleware(MiddlewareMixin): ) """ - 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( @@ -230,7 +233,7 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d # .. 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))) + 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): @@ -258,19 +261,78 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d # .. 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) - + set_custom_attribute('has_jwt_cookie', has_reconstituted_jwt_cookie) + + # DRF authentication does not set the request.user early enough for it to be used in process_request/ + # process_view of middleware. This code enables JWT cookie authentication to set the request.user for + # middleware, before it will presumably happen again during DRF authentication. + # For more info on DRF and the request.user, see + # https://github.com/jpadilla/django-rest-framework-jwt/issues/45#issuecomment-74996698 + # + # If the user has already been authenticated, we already have a user and intentionally avoid resetting it. As of + # Oct 2023, this would likely be due to session authentication in middleware. It is thus important for + # JwtAuthentication to verify that the session user and JWT user match. It is possible that this would be better + # handled through a more traditional AuthenticationMiddleware that handles both JWT cookies and sessions in + # the future. 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)) + if get_setting(ENABLE_FORGIVING_JWT_COOKIES): + # Since this call to the user is not made lazily, and has the potential to cause issues, we + # ensure it is only used in the case of ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE. + if not get_user(request).is_authenticated: + # Similar to django/contrib/auth/middleware.py AuthenticationMiddleware. + set_flag_is_request_user_set_for_jwt_auth() + request.user = SimpleLazyObject(lambda: _get_cached_user_from_jwt(request, view_func)) + else: + # Disabling ENABLE_FORGIVING_JWT_COOKIES will provide the original implementation, + # without checking the user outside of the lazy call (and without caching). This + # ensures there is no change in behavior until we test ENABLE_FORGIVING_JWT_COOKIES. + request.user = SimpleLazyObject(lambda: _get_user_from_jwt_original(request, view_func)) -def _get_user_from_jwt(request, view_func): +def _get_module_request_cache(): + return RequestCache(__name__).data + + +def _get_cached_user_from_jwt(request, view_func): + """ + Returns cached user from JWT authentication. + + Performs JWT authentication if not already cached. + """ + # Similar to django/contrib/auth/middleware.py get_user. + _JWT_USER_CACHE_KEY = '_cached_jwt_user' + if _get_module_request_cache().get(_JWT_USER_CACHE_KEY, None) is None: + cached_jwt_user = _get_user_from_jwt(request, view_func) + _get_module_request_cache()[_JWT_USER_CACHE_KEY] = cached_jwt_user + return _get_module_request_cache()[_JWT_USER_CACHE_KEY] + + +def _get_user_from_jwt_original(request, view_func): + """ + Returns user from JWT authentication using the original implementation. + + Although this implementation is not ideal, it is temporarily being left in place + for backward-compatibility when ENABLE_FORGIVING_JWT_COOKIES is disabled. + """ + # The original implementation first returned the result of the authenticated session + # user if it were available, which is not really what this method says it will do. user = get_user(request) if user.is_authenticated: return user + return _get_user_from_jwt(request, view_func) + + +def _get_user_from_jwt(request, view_func): + """ + Performs JWT Authentication and returns the user, or AnonymousUser if the user is + not authenticated. + + Uses the JWT authentication class associated the view. If not found, processing is skipped. + """ + # .. custom_attribute_name: set_user_from_jwt_status + # .. custom_attribute_description: Provides the status of setting the user from the JWT, using one of the + # following values: success, auth-failed, jwt-auth-class-not-found, and unknown-exception. try: jwt_authentication_class = _get_jwt_authentication_class(view_func) if jwt_authentication_class: @@ -279,17 +341,21 @@ def _get_user_from_jwt(request, view_func): parsers=api_settings.DEFAULT_PARSER_CLASSES )) if user_jwt is not None: + set_custom_attribute('set_user_from_jwt_status', 'success') return user_jwt[0] else: - log.warning('Jwt Authentication failed and request.user could not be set.') + set_custom_attribute('set_user_from_jwt_status', 'auth-failed') + log.debug('Jwt Authentication failed and request.user could not be set.') else: - log.warning( + set_custom_attribute('set_user_from_jwt_status', 'jwt-auth-class-not-found') + log.debug( 'Jwt Authentication expected, but view %s is not using a JwtAuthentication class.', view_func ) except Exception: # pylint: disable=broad-except + set_custom_attribute('set_user_from_jwt_status', 'unknown-exception') log.exception('Unknown Jwt Authentication error attempting to retrieve the user.') # pragma: no cover - return user + return AnonymousUser() def _get_jwt_authentication_class(view_func): diff --git a/edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py b/edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py index 781893e5..f0d110bd 100644 --- a/edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py +++ b/edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py @@ -8,17 +8,26 @@ from django.test import RequestFactory, TestCase, override_settings from django.urls import re_path as url_pattern from django.urls import reverse +from edx_django_utils.cache import RequestCache from jwt import exceptions as jwt_exceptions from rest_framework.exceptions import AuthenticationFailed, PermissionDenied from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView from rest_framework_jwt.authentication import JSONWebTokenAuthentication from edx_rest_framework_extensions.auth.jwt import authentication -from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.jwt.authentication import ( + JwtAuthentication, + JwtSessionUserMismatchError, +) 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.cookies import ( + jwt_cookie_header_payload_name, + jwt_cookie_name, + jwt_cookie_signature_name, +) from edx_rest_framework_extensions.auth.jwt.decoder import jwt_decode_handler from edx_rest_framework_extensions.auth.jwt.tests.utils import ( generate_jwt_token, @@ -26,7 +35,7 @@ ) from edx_rest_framework_extensions.config import ( ENABLE_FORGIVING_JWT_COOKIES, - ENABLE_JWT_VS_SESSION_USER_CHECK, + ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE, ) from edx_rest_framework_extensions.settings import get_setting from edx_rest_framework_extensions.tests import factories @@ -55,6 +64,9 @@ def get(self, request): # pylint: disable=unused-argument @ddt.ddt class JwtAuthenticationTests(TestCase): """ JWT Authentication class tests. """ + def setUp(self): + super().setUp() + RequestCache.clear_all_namespaces() def get_jwt_payload(self, **additional_claims): """ Returns a JWT payload with the necessary claims to create a new user. """ @@ -209,17 +221,14 @@ def test_authenticate_credentials_no_usernames(self): def test_authenticate_with_correct_jwt_cookie(self, mock_set_custom_attribute, mock_enforce_csrf): """ Verify authenticate succeeds with a valid JWT cookie. """ request = RequestFactory().post('/') - request.META[USE_JWT_COOKIE_HEADER] = 'true' - request.COOKIES[jwt_cookie_name()] = self._get_test_jwt_token() + drf_request = Request(request) - assert JwtAuthentication().authenticate(request) - mock_enforce_csrf.assert_called_with(request) - mock_set_custom_attribute.assert_any_call( - 'is_forgiving_jwt_cookies_enabled', - get_setting(ENABLE_FORGIVING_JWT_COOKIES) - ) + assert JwtAuthentication().authenticate(drf_request) + mock_enforce_csrf.assert_called_with(drf_request) + is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES) + mock_set_custom_attribute.assert_any_call('is_forgiving_jwt_cookies_enabled', is_forgiving_jwt_cookies_enabled) mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'success-cookie') @mock.patch('edx_rest_framework_extensions.auth.jwt.authentication.set_custom_attribute') @@ -231,20 +240,20 @@ def test_authenticate_csrf_protected(self, mock_set_custom_attribute): result in a None so that other authentication classes will also be checked. """ request = RequestFactory().post('/') - request.META[USE_JWT_COOKIE_HEADER] = 'true' # Set a sample JWT cookie. We mock the auth response but we still want # to ensure that there is jwt set because there is other logic that # checks for the jwt to be set before moving forward with CSRF checks. request.COOKIES[jwt_cookie_name()] = 'foo' + drf_request = Request(request) 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 + assert JwtAuthentication().authenticate(drf_request) is None mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'forgiven-failure') else: with self.assertRaises(PermissionDenied) as context_manager: - JwtAuthentication().authenticate(request) + JwtAuthentication().authenticate(drf_request) assert context_manager.exception.detail.startswith('CSRF Failed') mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'failed-cookie') @@ -315,7 +324,6 @@ def test_authenticate_with_bearer_token(self, mock_set_custom_attribute): mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'n/a') @override_settings( - EDX_DRF_EXTENSIONS={ENABLE_JWT_VS_SESSION_USER_CHECK: True}, MIDDLEWARE=( 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', @@ -335,11 +343,23 @@ def test_authenticate_jwt_and_session_mismatch(self, mock_set_custom_attribute): self.client.force_login(session_user) response = self.client.get(reverse('authenticated-view')) - mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_enabled', True) - mock_set_custom_attribute.assert_any_call('jwt_auth_mismatch_session_user_id', session_user_id) + is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES) + mock_set_custom_attribute.assert_any_call('is_forgiving_jwt_cookies_enabled', is_forgiving_jwt_cookies_enabled) mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'success-cookie') + if is_forgiving_jwt_cookies_enabled: + mock_set_custom_attribute.assert_any_call('jwt_auth_mismatch_session_user_id', session_user_id) + else: + set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] + assert 'jwt_auth_mismatch_session_user_id' not in set_custom_attribute_keys assert response.status_code == 200 + @override_settings( + MIDDLEWARE=( + 'django.contrib.sessions.middleware.SessionMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + ), + ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_authentication', + ) @mock.patch('edx_rest_framework_extensions.auth.jwt.authentication.set_custom_attribute') def test_authenticate_jwt_and_session_mismatch_bad_signature_cookie(self, mock_set_custom_attribute): """ Tests monitoring for JWT cookie with a bad signature when there is a session user mismatch """ @@ -351,30 +371,28 @@ def test_authenticate_jwt_and_session_mismatch_bad_signature_cookie(self, mock_s jwt_cookie_name(): self._get_test_jwt_token(user=jwt_user, is_valid_signature=False), }) - enable_forgiving_jwt_cookies = get_setting(ENABLE_FORGIVING_JWT_COOKIES) - with override_settings( - EDX_DRF_EXTENSIONS={ - ENABLE_FORGIVING_JWT_COOKIES: enable_forgiving_jwt_cookies, - ENABLE_JWT_VS_SESSION_USER_CHECK: True, - }, - MIDDLEWARE=( - 'django.contrib.sessions.middleware.SessionMiddleware', - 'django.contrib.auth.middleware.AuthenticationMiddleware', - ), - ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_authentication', - ): - self.client.force_login(session_user) - response = self.client.get(reverse('authenticated-view')) + self.client.force_login(session_user) + response = self.client.get(reverse('authenticated-view')) - mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_enabled', True) - mock_set_custom_attribute.assert_any_call('jwt_auth_mismatch_session_user_id', session_user_id) + is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES) + mock_set_custom_attribute.assert_any_call('is_forgiving_jwt_cookies_enabled', is_forgiving_jwt_cookies_enabled) mock_set_custom_attribute.assert_any_call('failed_jwt_cookie_user_id', jwt_user_id) - if enable_forgiving_jwt_cookies: + if is_forgiving_jwt_cookies_enabled: mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'user-mismatch-failure') + mock_set_custom_attribute.assert_any_call('jwt_auth_mismatch_session_user_id', session_user_id) else: mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'failed-cookie') + set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] + assert 'jwt_auth_mismatch_session_user_id' not in set_custom_attribute_keys assert response.status_code == 401 + @override_settings( + MIDDLEWARE=( + 'django.contrib.sessions.middleware.SessionMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + ), + ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_authentication', + ) @mock.patch('edx_rest_framework_extensions.auth.jwt.authentication.set_custom_attribute') def test_authenticate_jwt_and_session_mismatch_invalid_cookie(self, mock_set_custom_attribute): """ Tests monitoring for invalid JWT cookie when there is a session user mismatch """ @@ -384,32 +402,22 @@ def test_authenticate_jwt_and_session_mismatch_invalid_cookie(self, mock_set_cus jwt_cookie_name(): 'invalid-cookie', }) - enable_forgiving_jwt_cookies = get_setting(ENABLE_FORGIVING_JWT_COOKIES) - with override_settings( - EDX_DRF_EXTENSIONS={ - ENABLE_FORGIVING_JWT_COOKIES: enable_forgiving_jwt_cookies, - ENABLE_JWT_VS_SESSION_USER_CHECK: True, - }, - MIDDLEWARE=( - 'django.contrib.sessions.middleware.SessionMiddleware', - 'django.contrib.auth.middleware.AuthenticationMiddleware', - ), - ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_authentication', - ): - self.client.force_login(session_user) - response = self.client.get(reverse('authenticated-view')) + self.client.force_login(session_user) + response = self.client.get(reverse('authenticated-view')) - mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_enabled', True) - mock_set_custom_attribute.assert_any_call('jwt_auth_mismatch_session_user_id', session_user_id) + is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES) + mock_set_custom_attribute.assert_any_call('is_forgiving_jwt_cookies_enabled', is_forgiving_jwt_cookies_enabled) mock_set_custom_attribute.assert_any_call('failed_jwt_cookie_user_id', 'decode-error') - if enable_forgiving_jwt_cookies: + if is_forgiving_jwt_cookies_enabled: mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'user-mismatch-failure') + mock_set_custom_attribute.assert_any_call('jwt_auth_mismatch_session_user_id', session_user_id) else: mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'failed-cookie') + set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] + assert 'jwt_auth_mismatch_session_user_id' not in set_custom_attribute_keys assert response.status_code == 401 @override_settings( - EDX_DRF_EXTENSIONS={ENABLE_JWT_VS_SESSION_USER_CHECK: True}, MIDDLEWARE=( 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', @@ -427,14 +435,13 @@ def test_authenticate_jwt_and_session_match(self, mock_set_custom_attribute): self.client.force_login(test_user) response = self.client.get(reverse('authenticated-view')) - mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_enabled', True) + is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES) + mock_set_custom_attribute.assert_any_call('is_forgiving_jwt_cookies_enabled', is_forgiving_jwt_cookies_enabled) set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] - assert 'is_jwt_vs_session_user_check_enabled' in set_custom_attribute_keys assert 'jwt_auth_mismatch_session_user_id' not in set_custom_attribute_keys assert response.status_code == 200 @override_settings( - EDX_DRF_EXTENSIONS={ENABLE_JWT_VS_SESSION_USER_CHECK: True}, MIDDLEWARE=( 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', @@ -452,40 +459,98 @@ def test_authenticate_jwt_and_no_session(self, mock_set_custom_attribute): # unlike other tests, there is no force_login call to start the session response = self.client.get(reverse('authenticated-view')) - mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_enabled', True) + is_forgiving_jwt_cookies_enabled = get_setting(ENABLE_FORGIVING_JWT_COOKIES) + mock_set_custom_attribute.assert_any_call('is_forgiving_jwt_cookies_enabled', is_forgiving_jwt_cookies_enabled) set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] - assert 'is_jwt_vs_session_user_check_enabled' in set_custom_attribute_keys assert 'jwt_auth_mismatch_session_user_id' not in set_custom_attribute_keys assert response.status_code == 200 @override_settings( - EDX_DRF_EXTENSIONS={ENABLE_JWT_VS_SESSION_USER_CHECK: False}, + EDX_DRF_EXTENSIONS={ + ENABLE_FORGIVING_JWT_COOKIES: True, + ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE: True, + }, MIDDLEWARE=( 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'edx_rest_framework_extensions.auth.jwt.middleware.JwtAuthCookieMiddleware', ), ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_authentication', ) @mock.patch('edx_rest_framework_extensions.auth.jwt.authentication.set_custom_attribute') - def test_authenticate_jwt_and_session_mismatch_disabled(self, mock_set_custom_attribute): - """ Tests monitoring disabled for JWT cookie and session user mismatch """ - session_user = factories.UserFactory(id=111) - jwt_user = factories.UserFactory(id=222) + def test_authenticate_jwt_and_session_mismatch_and_set_request_user(self, mock_set_custom_attribute): + """ + Tests failure for JWT cookie when there is a session user mismatch and a request to set user. + + - This tests coordination between ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE in middleware and JwtAuthentication. + - This test requires ENABLE_FORGIVING_JWT_COOKIES to get to ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE. + - This test is kept with the rest of the JWT vs session user tests. + """ + session_user_id = 111 + session_user = factories.UserFactory(id=session_user_id) + jwt_user_id = 222 + jwt_user = factories.UserFactory(id=jwt_user_id) + jwt_header_payload, jwt_signature = self._get_test_jwt_token_payload_and_signature(user=jwt_user) + # Cookie parts will be recombined by JwtAuthCookieMiddleware self.client.cookies = SimpleCookie({ - jwt_cookie_name(): self._get_test_jwt_token(user=jwt_user), + jwt_cookie_header_payload_name(): jwt_header_payload, + jwt_cookie_signature_name(): jwt_signature, }) self.client.force_login(session_user) + + with self.assertRaises(JwtSessionUserMismatchError): + response = self.client.get(reverse('authenticated-view')) + assert response.status_code == 401 + + # The case where forgiving JWTs is disabled is tested under other tests, including the middleware tests. + mock_set_custom_attribute.assert_any_call('is_forgiving_jwt_cookies_enabled', True) + mock_set_custom_attribute.assert_any_call('jwt_auth_mismatch_session_user_id', session_user_id) + mock_set_custom_attribute.assert_any_call('failed_jwt_cookie_user_id', jwt_user_id) + mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'user-mismatch-enforced-failure') + mock_set_custom_attribute.assert_any_call('jwt_auth_failed', mock.ANY) + + @override_settings( + EDX_DRF_EXTENSIONS={ + ENABLE_FORGIVING_JWT_COOKIES: True, + ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE: True, + }, + MIDDLEWARE=( + 'django.contrib.sessions.middleware.SessionMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'edx_rest_framework_extensions.auth.jwt.middleware.JwtAuthCookieMiddleware', + ), + ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_authentication', + ) + @mock.patch('edx_rest_framework_extensions.auth.jwt.authentication.set_custom_attribute') + def test_authenticate_jwt_and_no_session_and_set_request_user(self, mock_set_custom_attribute): + """ + Tests success for JWT cookie when there is no session user and there is a request to set user. + + - This tests coordination between ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE in middleware and JwtAuthentication. + - This test requires ENABLE_FORGIVING_JWT_COOKIES to get to ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE. + - This test is kept with the rest of the JWT vs session user tests. + """ + test_user = factories.UserFactory() + jwt_header_payload, jwt_signature = self._get_test_jwt_token_payload_and_signature(user=test_user) + # Cookie parts will be recombined by JwtAuthCookieMiddleware + self.client.cookies = SimpleCookie({ + jwt_cookie_header_payload_name(): jwt_header_payload, + jwt_cookie_signature_name(): jwt_signature, + }) + + # unlike other tests, there is no force_login call to start the session response = self.client.get(reverse('authenticated-view')) - mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_enabled', False) + # The case where forgiving JWTs is disabled is tested under other tests, including the middleware tests. + mock_set_custom_attribute.assert_any_call('is_forgiving_jwt_cookies_enabled', True) + mock_set_custom_attribute.assert_any_call('skip_jwt_vs_session_check', True) set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] - assert 'is_jwt_vs_session_user_check_enabled' in set_custom_attribute_keys assert 'jwt_auth_mismatch_session_user_id' not in set_custom_attribute_keys assert response.status_code == 200 def _get_test_jwt_token(self, user=None, is_valid_signature=True): - """ Returns a user and jwt token """ + """ Returns a test jwt token for the provided user """ test_user = factories.UserFactory() if user is None else user payload = generate_latest_version_payload(test_user) if is_valid_signature: @@ -494,6 +559,14 @@ def _get_test_jwt_token(self, user=None, is_valid_signature=True): jwt_token = generate_jwt_token(payload, signing_key='invalid-key') return jwt_token + def _get_test_jwt_token_payload_and_signature(self, user=None): + """ Returns a test jwt token split into payload and signature """ + jwt_token = self._get_test_jwt_token(user=user) + jwt_token_parts = jwt_token.split('.') + header_and_payload = '.'.join(jwt_token_parts[0:2]) + signature = jwt_token_parts[2] + return header_and_payload, signature + # We want to duplicate these tests for now while we have two major code paths. It will get unified once we have a # single way of doing JWT authentication again. diff --git a/edx_rest_framework_extensions/auth/jwt/tests/test_middleware.py b/edx_rest_framework_extensions/auth/jwt/tests/test_middleware.py index 2dfeb251..3b250a01 100644 --- a/edx_rest_framework_extensions/auth/jwt/tests/test_middleware.py +++ b/edx_rest_framework_extensions/auth/jwt/tests/test_middleware.py @@ -28,6 +28,8 @@ EnsureJWTAuthSettingsMiddleware, JwtAuthCookieMiddleware, JwtRedirectToLoginIfUnauthenticatedMiddleware, + _get_cached_user_from_jwt, + _get_user_from_jwt_original, ) from edx_rest_framework_extensions.config import ( ENABLE_FORGIVING_JWT_COOKIES, @@ -394,12 +396,13 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d class TestJwtAuthCookieMiddleware(TestCase): def setUp(self): super().setUp() + RequestCache.clear_all_namespaces() self.request = RequestFactory().get('/') self.request.session = 'mock session' self.mock_response = Mock() self.middleware = JwtAuthCookieMiddleware(self.mock_response) - @patch('edx_django_utils.monitoring.set_custom_attribute') + @patch('edx_rest_framework_extensions.auth.jwt.middleware.set_custom_attribute') 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())) @@ -411,7 +414,7 @@ def test_do_not_use_jwt_cookies(self, mock_set_custom_attribute): ) @ddt.unpack @patch('edx_rest_framework_extensions.auth.jwt.middleware.log') - @patch('edx_django_utils.monitoring.set_custom_attribute') + @patch('edx_rest_framework_extensions.auth.jwt.middleware.set_custom_attribute') def test_missing_cookies( self, set_cookie_name, missing_cookie_name, mock_set_custom_attribute, mock_log ): @@ -426,7 +429,7 @@ def test_missing_cookies( 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') + @patch('edx_rest_framework_extensions.auth.jwt.middleware.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) @@ -434,7 +437,7 @@ def test_no_cookies(self, mock_set_custom_attribute): 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') + @patch('edx_rest_framework_extensions.auth.jwt.middleware.set_custom_attribute') def test_success(self, mock_set_custom_attribute): self.request.META[USE_JWT_COOKIE_HEADER] = 'true' self.request.COOKIES[jwt_cookie_header_payload_name()] = 'header.payload' @@ -448,6 +451,14 @@ def test_success(self, mock_set_custom_attribute): _LOG_WARN_MISSING_JWT_AUTHENTICATION_CLASS = 1 @patch('edx_rest_framework_extensions.auth.jwt.middleware.log') + @patch( + 'edx_rest_framework_extensions.auth.jwt.middleware._get_cached_user_from_jwt', + wraps=_get_cached_user_from_jwt + ) + @patch( + 'edx_rest_framework_extensions.auth.jwt.middleware._get_user_from_jwt_original', + wraps=_get_user_from_jwt_original + ) @ddt.data( ('/nopermissionsrequired/', True, True, True, None, False), ('/nopermissionsrequired/', True, False, False, None, False), @@ -459,8 +470,8 @@ def test_success(self, mock_set_custom_attribute): ) @ddt.unpack def test_set_request_user_with_use_jwt_cookie( - self, url, is_cookie_valid, is_request_user_set, is_toggle_enabled, log_warning, - send_post, mock_log, + self, url, is_cookie_valid, is_request_user_set, is_toggle_enabled, log_warning, send_post, + mock_protected_get_user_original, mock_protected_get_cached_user_from_jwt, mock_log, ): header = {USE_JWT_COOKIE_HEADER: 'true'} self.client.cookies = _get_test_cookie(is_cookie_valid=is_cookie_valid) @@ -480,6 +491,7 @@ def test_set_request_user_with_use_jwt_cookie( ), ), EDX_DRF_EXTENSIONS={ + ENABLE_FORGIVING_JWT_COOKIES: False, ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE: is_toggle_enabled, } ): @@ -489,14 +501,51 @@ def test_set_request_user_with_use_jwt_cookie( response = self.client.get(url, **header) self.assertEqual(200, response.status_code) - if log_warning == self._LOG_WARN_AUTHENTICATION_FAILED: - mock_log.warning.assert_called_once_with('Jwt Authentication failed and request.user could not be set.') - elif log_warning == self._LOG_WARN_MISSING_JWT_AUTHENTICATION_CLASS: - mock_log.warning.assert_called_once_with( - 'Jwt Authentication expected, but view %s is not using a JwtAuthentication class.', ANY - ) - else: - mock_log.warn.assert_not_called() + if log_warning == self._LOG_WARN_AUTHENTICATION_FAILED: + mock_log.debug.assert_called_once_with('Jwt Authentication failed and request.user could not be set.') + elif log_warning == self._LOG_WARN_MISSING_JWT_AUTHENTICATION_CLASS: + mock_log.debug.assert_called_once_with( + 'Jwt Authentication expected, but view %s is not using a JwtAuthentication class.', ANY + ) + else: + mock_log.warn.assert_not_called() + # Ensure _get_user_from_jwt_original is called, since ENABLE_FORGIVING_JWT_COOKIES is disabled + if is_request_user_set: + mock_protected_get_user_original.assert_called() + # Ensure _get_cached_user_from_jwt is not called, since ENABLE_FORGIVING_JWT_COOKIES is disabled + mock_protected_get_cached_user_from_jwt.assert_not_called() + + @patch( + 'edx_rest_framework_extensions.auth.jwt.middleware._get_cached_user_from_jwt', + wraps=_get_cached_user_from_jwt + ) + @override_settings( + ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_middleware', + MIDDLEWARE=( + 'django.contrib.sessions.middleware.SessionMiddleware', + 'edx_rest_framework_extensions.auth.jwt.middleware.JwtAuthCookieMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'edx_rest_framework_extensions.auth.jwt.tests.test_middleware.CheckRequestUserForJwtAuthMiddleware', + ), + EDX_DRF_EXTENSIONS={ + ENABLE_FORGIVING_JWT_COOKIES: True, + ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE: True, + } + ) + def test_set_request_user_with_use_jwt_cookie_and_forgiving_jwt_cookie_enabled( + self, mock_protected_get_cached_user_from_jwt + ): + """ + Tests set request user when ENABLE_FORGIVING_JWT_COOKIES is also enabled. + """ + self.client.cookies = _get_test_cookie(is_cookie_valid=True) + + response = self.client.get('/nopermissionsrequired/') + + # additional assertions in CheckRequestUserForJwtAuthMiddleware + self.assertEqual(200, response.status_code) + # Ensure _get_cached_user_from_jwt is called, since ENABLE_FORGIVING_JWT_COOKIES is enabled + mock_protected_get_cached_user_from_jwt.assert_called() # We want to duplicate these tests for now while we have two major code paths. It will get unified once we have a diff --git a/edx_rest_framework_extensions/config.py b/edx_rest_framework_extensions/config.py index 89279912..bf5e94c5 100644 --- a/edx_rest_framework_extensions/config.py +++ b/edx_rest_framework_extensions/config.py @@ -22,17 +22,3 @@ # .. toggle_target_removal_date: 2023-10-01 # .. toggle_tickets: https://github.com/openedx/edx-drf-extensions/issues/371 ENABLE_FORGIVING_JWT_COOKIES = 'ENABLE_FORGIVING_JWT_COOKIES' - -# .. toggle_name: EDX_DRF_EXTENSIONS[ENABLE_JWT_VS_SESSION_USER_CHECK] -# .. toggle_implementation: DjangoSetting -# .. toggle_default: False -# .. toggle_description: If True, checks for mismatches of JWT cookie user and session user. If forgiving -# JWT cookies is also enabled, mismatches will result in an error, rather than being forgiving. Also -# adds monitoring of session user vs JWT cookie user. -# .. toggle_warning: Since edx-platform has user caching, this toggle is a safety valve in case it -# starts causing memory issues, as has happened in the past. See ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE. -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2023-10-04 -# .. toggle_target_removal_date: 2023-12-01 -# .. toggle_tickets: https://github.com/openedx/edx-drf-extensions/issues/371 -ENABLE_JWT_VS_SESSION_USER_CHECK = 'ENABLE_JWT_VS_SESSION_USER_CHECK' diff --git a/edx_rest_framework_extensions/settings.py b/edx_rest_framework_extensions/settings.py index 6103e37e..57572afa 100644 --- a/edx_rest_framework_extensions/settings.py +++ b/edx_rest_framework_extensions/settings.py @@ -17,7 +17,6 @@ from edx_rest_framework_extensions.config import ( ENABLE_FORGIVING_JWT_COOKIES, - ENABLE_JWT_VS_SESSION_USER_CHECK, ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE, ) @@ -36,7 +35,6 @@ 'JWT_PAYLOAD_MERGEABLE_USER_ATTRIBUTES': (), ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE: False, ENABLE_FORGIVING_JWT_COOKIES: False, - ENABLE_JWT_VS_SESSION_USER_CHECK: False, }