From 8a3053828dc64b23abc4dd8f27eb38e19b46f789 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 4 Oct 2023 23:07:54 -0400 Subject: [PATCH] feat: add monitoring for jwt vs session user Adds custom attributes jwt_auth_session_user_id and jwt_auth_and_session_user_mismatch for monitoring JWT cookie auth. See toggle EDX_DRF_EXTENSIONS[ENABLE_JWT_VS_SESSION_USER_MONITORING] for enabling. --- CHANGELOG.rst | 7 + edx_rest_framework_extensions/__init__.py | 2 +- .../auth/jwt/authentication.py | 48 +++++- .../auth/jwt/tests/test_authentication.py | 154 +++++++++++++++++- edx_rest_framework_extensions/config.py | 12 ++ edx_rest_framework_extensions/settings.py | 2 + test_settings.py | 1 + 7 files changed, 219 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 741fd539..aaf1003d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,13 @@ Change Log Unreleased ---------- +[8.11.0] - 2023-10-04 +--------------------- + +Added +~~~~~ +* Added custom attributes jwt_auth_session_user_id and jwt_auth_and_session_user_mismatch for monitoring JWT cookie auth. See toggle EDX_DRF_EXTENSIONS[ENABLE_JWT_VS_SESSION_USER_MONITORING] for enabling. + [8.10.0] - 2023-09-19 --------------------- diff --git a/edx_rest_framework_extensions/__init__.py b/edx_rest_framework_extensions/__init__.py index 3066d7b6..b38d3487 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.10.0' # pragma: no cover +__version__ = '8.11.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 5615d0eb..790eaa24 100644 --- a/edx_rest_framework_extensions/auth/jwt/authentication.py +++ b/edx_rest_framework_extensions/auth/jwt/authentication.py @@ -10,7 +10,10 @@ from rest_framework_jwt.authentication import JSONWebTokenAuthentication 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.config import ( + ENABLE_FORGIVING_JWT_COOKIES, + ENABLE_JWT_VS_SESSION_USER_MONITORING, +) from edx_rest_framework_extensions.settings import get_setting @@ -100,6 +103,7 @@ def authenticate(self, request): # CSRF passed validation with authenticated user set_custom_attribute('jwt_auth_result', 'success-cookie') + self._monitor_jwt_vs_session_cookie_user(request, jwt_user=user_and_auth[0]) return user_and_auth except Exception as exception: @@ -216,6 +220,48 @@ def is_authenticating_with_jwt_cookie(cls, request): except Exception: # pylint: disable=broad-exception-caught return False + def _monitor_jwt_vs_session_cookie_user(self, request, jwt_user): + """ + Adds monitoring details when session auth and JWT cookie auth user don't match. + """ + is_jwt_vs_session_user_monitoring_enabled = get_setting(ENABLE_JWT_VS_SESSION_USER_MONITORING) + # .. custom_attribute_name: is_jwt_vs_session_user_monitoring_enabled + # .. custom_attribute_description: This is temporary custom attribute to show + # whether ENABLE_JWT_VS_SESSION_USER_MONITORING is toggled on or off. + set_custom_attribute('is_jwt_vs_session_user_monitoring_enabled', is_jwt_vs_session_user_monitoring_enabled) + if not is_jwt_vs_session_user_monitoring_enabled: + return + + if not (hasattr(request, '_request') and hasattr(request._request, 'user')): # pylint: disable=protected-access + # .. custom_attribute_name: jwt_auth_request_user_not_found + # .. custom_attribute_description: This custom attribute will show that we + # were unable to find the session user. This should not occur outside + # of tests, because there should still be an unauthenticated user, but + # this attribute could be used to check for the unexpected. + set_custom_attribute('jwt_auth_request_user_not_found', True) + return + + 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 + else: + session_user_id = None + if session_user_id and session_user_id != jwt_user.id: + # .. custom_attribute_name: jwt_auth_session_user_id + # .. custom_attribute_description: Session authentication may have completed + # in middleware before even getting to DRF. Although this authentication + # won't stick, because it will be replaced by DRF authentication, we + # record it, because it sometimes does not match the JWT cookie user. + # The name of this attribute is simply to clarify that this was found + # during JWT authentication. + set_custom_attribute('jwt_auth_session_user_id', session_user_id) + + # .. custom_attribute_name: jwt_auth_and_session_user_mismatch + # .. custom_attribute_description: True if session authentication user id and + # the JWT cookie user id may not match. When they match, this attribute + # won't be included. See jwt_auth_session_user_id for additional details. + set_custom_attribute('jwt_auth_and_session_user_mismatch', True) + def is_jwt_authenticated(request): successful_authenticator = getattr(request, 'successful_authenticator', None) 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 2b833b8f..9785498f 100644 --- a/edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py +++ b/edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py @@ -4,9 +4,15 @@ import ddt from django.contrib.auth import get_user_model -from django.test import RequestFactory, TestCase, override_settings +from django.http.cookie import SimpleCookie +from django.test import Client, RequestFactory, TestCase, override_settings +from django.urls import re_path as url_pattern +from django.urls import reverse from jwt import exceptions as jwt_exceptions from rest_framework.exceptions import AuthenticationFailed, PermissionDenied +from rest_framework.permissions import IsAuthenticated +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 @@ -18,7 +24,10 @@ generate_jwt_token, generate_latest_version_payload, ) -from edx_rest_framework_extensions.config import ENABLE_FORGIVING_JWT_COOKIES +from edx_rest_framework_extensions.config import ( + ENABLE_FORGIVING_JWT_COOKIES, + ENABLE_JWT_VS_SESSION_USER_MONITORING, +) from edx_rest_framework_extensions.settings import get_setting from edx_rest_framework_extensions.tests import factories @@ -26,6 +35,23 @@ User = get_user_model() +class IsAuthenticatedView(APIView): + authentication_classes = (JwtAuthentication,) + permission_classes = (IsAuthenticated,) + + def get(self, request): # pylint: disable=unused-argument + return Response({'success': True}) + + +urlpatterns = [ + url_pattern( + r'^isauthenticated/$', + IsAuthenticatedView.as_view(), + name='authenticated-view', + ), +] + + @ddt.ddt class JwtAuthenticationTests(TestCase): """ JWT Authentication class tests. """ @@ -288,10 +314,128 @@ def test_authenticate_with_bearer_token(self, mock_set_custom_attribute): self.assertIsNone(JwtAuthentication().authenticate(request)) mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'n/a') - def _get_test_jwt_token(self): + @override_settings( + EDX_DRF_EXTENSIONS={ENABLE_JWT_VS_SESSION_USER_MONITORING: True}, + 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(self, mock_set_custom_attribute): + """ Tests monitoring for JWT cookie when there is a session user mismatch """ + client = Client() + session_user_id = 111 + session_user = factories.UserFactory(id=session_user_id) + header = {USE_JWT_COOKIE_HEADER: 'true'} + jwt_user_id = 222 + jwt_user = factories.UserFactory(id=jwt_user_id) + client.cookies = SimpleCookie({ + jwt_cookie_name(): self._get_test_jwt_token(user=jwt_user), + }) + + client.force_login(session_user) + response = client.get(reverse('authenticated-view'), {}, content_type="application/json", **header) + + mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_monitoring_enabled', True) + mock_set_custom_attribute.assert_any_call('jwt_auth_session_user_id', session_user_id) + mock_set_custom_attribute.assert_any_call('jwt_auth_and_session_user_mismatch', True) + assert response.status_code == 200 + + @override_settings( + EDX_DRF_EXTENSIONS={ENABLE_JWT_VS_SESSION_USER_MONITORING: True}, + 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_match(self, mock_set_custom_attribute): + """ Tests monitoring for JWT cookie when session user matches """ + client = Client() + user_id = 111 + test_user = factories.UserFactory(id=user_id) + header = {USE_JWT_COOKIE_HEADER: 'true'} + client.cookies = SimpleCookie({ + jwt_cookie_name(): self._get_test_jwt_token(user=test_user), + }) + + client.force_login(test_user) + response = client.get(reverse('authenticated-view'), {}, content_type="application/json", **header) + + mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_monitoring_enabled', True) + set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] + assert 'is_jwt_vs_session_user_monitoring_enabled' in set_custom_attribute_keys + assert 'jwt_auth_session_user_id' not in set_custom_attribute_keys + assert 'jwt_auth_and_session_user_mismatch' not in set_custom_attribute_keys + assert response.status_code == 200 + + @override_settings( + EDX_DRF_EXTENSIONS={ENABLE_JWT_VS_SESSION_USER_MONITORING: True}, + 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_no_session(self, mock_set_custom_attribute): + """ Tests monitoring for JWT cookie when there is no session """ + client = Client() + user_id = 111 + test_user = factories.UserFactory(id=user_id) + header = {USE_JWT_COOKIE_HEADER: 'true'} + client.cookies = SimpleCookie({ + jwt_cookie_name(): self._get_test_jwt_token(user=test_user), + }) + + # unlike other tests, there is no force_login call to start the session + response = client.get(reverse('authenticated-view'), {}, content_type="application/json", **header) + + mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_monitoring_enabled', True) + set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] + assert 'is_jwt_vs_session_user_monitoring_enabled' in set_custom_attribute_keys + assert 'jwt_auth_session_user_id' not in set_custom_attribute_keys + assert 'jwt_auth_and_session_user_mismatch' not in set_custom_attribute_keys + assert response.status_code == 200 + + @override_settings( + EDX_DRF_EXTENSIONS={ENABLE_JWT_VS_SESSION_USER_MONITORING: False}, + 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_disabled(self, mock_set_custom_attribute): + """ Tests monitoring disabled for JWT cookie and session user mismatch """ + client = Client() + session_user_id = 111 + session_user = factories.UserFactory(id=session_user_id) + header = {USE_JWT_COOKIE_HEADER: 'true'} + jwt_user_id = 222 + jwt_user = factories.UserFactory(id=jwt_user_id) + client.cookies = SimpleCookie({ + jwt_cookie_name(): self._get_test_jwt_token(user=jwt_user), + }) + + client.force_login(session_user) + response = client.get(reverse('authenticated-view'), {}, content_type="application/json", **header) + + mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_monitoring_enabled', False) + set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] + assert 'is_jwt_vs_session_user_monitoring_enabled' in set_custom_attribute_keys + assert 'jwt_auth_session_user_id' not in set_custom_attribute_keys + assert 'jwt_auth_and_session_user_mismatch' not in set_custom_attribute_keys + assert response.status_code == 200 + + def _get_test_jwt_token(self, user=None): """ Returns a user and jwt token """ - user = factories.UserFactory() - payload = generate_latest_version_payload(user) + test_user = factories.UserFactory() if user is None else user + payload = generate_latest_version_payload(test_user) jwt_token = generate_jwt_token(payload) return jwt_token diff --git a/edx_rest_framework_extensions/config.py b/edx_rest_framework_extensions/config.py index bf5e94c5..1b03b3c4 100644 --- a/edx_rest_framework_extensions/config.py +++ b/edx_rest_framework_extensions/config.py @@ -22,3 +22,15 @@ # .. 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_MONITORING] +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: If True, 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_MONITORING = 'ENABLE_JWT_VS_SESSION_USER_MONITORING' diff --git a/edx_rest_framework_extensions/settings.py b/edx_rest_framework_extensions/settings.py index 57572afa..2705d8cd 100644 --- a/edx_rest_framework_extensions/settings.py +++ b/edx_rest_framework_extensions/settings.py @@ -17,6 +17,7 @@ from edx_rest_framework_extensions.config import ( ENABLE_FORGIVING_JWT_COOKIES, + ENABLE_JWT_VS_SESSION_USER_MONITORING, ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE, ) @@ -35,6 +36,7 @@ 'JWT_PAYLOAD_MERGEABLE_USER_ATTRIBUTES': (), ENABLE_SET_REQUEST_USER_FOR_JWT_COOKIE: False, ENABLE_FORGIVING_JWT_COOKIES: False, + ENABLE_JWT_VS_SESSION_USER_MONITORING: False, } diff --git a/test_settings.py b/test_settings.py index d1ac9ddc..dc0f0674 100644 --- a/test_settings.py +++ b/test_settings.py @@ -13,6 +13,7 @@ 'csrf.apps.CsrfAppConfig', 'django.contrib.auth', 'django.contrib.contenttypes', + 'django.contrib.sessions', 'edx_rest_framework_extensions', 'rest_framework_jwt', 'waffle',