From 07115be9e6c41761dc5e629e6e9fedf76d54b022 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Fri, 5 Jan 2024 10:09:54 -0500 Subject: [PATCH] squash! fix for 9.1.0 The fix was to use `preferred_username` in place of `username`. This also fixes/updates some tests for updated custom attributes. Squashed commit message below: ------------------------------ Restores and fixes simplified JWT cookie vs session user check by checking username instead of lms user id (originally introduced in 9.1.0, and removed in 9.1.1). - Removed ``VERIFY_LMS_USER_ID_PROPERTY_NAME``, which is no longer needed. - Removed custom attribute ``jwt_auth_get_lms_user_id_status``, since we no longer attempt to get the lms_user_id from the user object. - Renames custom attribute ``jwt_auth_mismatch_session_lms_user_id`` to ``jwt_auth_mismatch_session_username``. - Adds custom attribute ``jwt_auth_mismatch_jwt_cookie_username``. - Adds custom attribute ``jwt_cookie_unsafe_decode_issue`` for when a JWT cookie cannot even be unsafely decoded. - Fixes mock JWT creation for tests to use ``preferred_username``, which is configured in each Open edX service. Part of edx/edx-arch-experiments#429 --- CHANGELOG.rst | 13 +++++++++++++ edx_rest_framework_extensions/__init__.py | 2 +- .../auth/jwt/authentication.py | 2 +- .../auth/jwt/tests/test_authentication.py | 9 ++++++--- .../auth/jwt/tests/test_decoder.py | 2 +- .../auth/jwt/tests/utils.py | 9 +++++++-- test_settings.py | 3 +++ 7 files changed, 32 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 20463128..711896d2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,19 @@ Change Log Unreleased ---------- +[9.1.2] - 2024-01-05 +-------------------- +Updated +~~~~~~~ +* Restores and fixes simplified JWT cookie vs session user check by checking username instead of lms user id (originally introduced in 9.1.0, and removed in 9.1.1). + + * Removed ``VERIFY_LMS_USER_ID_PROPERTY_NAME``, which is no longer needed. + * Removed custom attribute ``jwt_auth_get_lms_user_id_status``, since we no longer attempt to get the lms_user_id from the user object. + * Renames custom attribute ``jwt_auth_mismatch_session_lms_user_id`` to ``jwt_auth_mismatch_session_username``. + * Adds custom attribute ``jwt_auth_mismatch_jwt_cookie_username``. + * Adds custom attribute ``jwt_cookie_unsafe_decode_issue`` for when a JWT cookie cannot even be unsafely decoded. + * Fixes mock JWT creation for tests to use ``preferred_username``, which is configured in each Open edX service. + [9.1.1] - 2024-01-04 -------------------- Updated diff --git a/edx_rest_framework_extensions/__init__.py b/edx_rest_framework_extensions/__init__.py index cc193cc1..3aa81165 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__ = '9.1.1' # pragma: no cover +__version__ = '9.1.2' # pragma: no cover diff --git a/edx_rest_framework_extensions/auth/jwt/authentication.py b/edx_rest_framework_extensions/auth/jwt/authentication.py index 0d882433..b0e8a050 100644 --- a/edx_rest_framework_extensions/auth/jwt/authentication.py +++ b/edx_rest_framework_extensions/auth/jwt/authentication.py @@ -356,7 +356,7 @@ def _get_unsafe_jwt_cookie_username_and_lms_user_id(self, request): try: cookie_token = JSONWebTokenAuthentication.get_token_from_cookies(request.COOKIES) unsafe_decoded_jwt = unsafe_jwt_decode_handler(cookie_token) - jwt_username = unsafe_decoded_jwt.get('username', None) + jwt_username = JSONWebTokenAuthentication.jwt_get_username_from_payload(unsafe_decoded_jwt) jwt_lms_user_id = unsafe_decoded_jwt.get('user_id', None) if not jwt_username or not jwt_lms_user_id: set_custom_attribute('jwt_cookie_unsafe_decode_issue', 'missing-claim') 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 d2321cc8..c3f1900b 100644 --- a/edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py +++ b/edx_rest_framework_extensions/auth/jwt/tests/test_authentication.py @@ -502,7 +502,8 @@ def test_authenticate_jwt_and_session_match(self, mock_set_custom_attribute): '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 'jwt_auth_mismatch_session_lms_user_id' not in set_custom_attribute_keys + assert 'jwt_auth_mismatch_session_username' not in set_custom_attribute_keys + assert 'jwt_auth_mismatch_jwt_cookie_username' not in set_custom_attribute_keys assert response.status_code == 200 @override_settings( @@ -526,7 +527,8 @@ def test_authenticate_jwt_and_no_session(self, mock_set_custom_attribute): 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 'jwt_auth_mismatch_session_lms_user_id' not in set_custom_attribute_keys + assert 'jwt_auth_mismatch_session_username' not in set_custom_attribute_keys + assert 'jwt_auth_mismatch_jwt_cookie_username' not in set_custom_attribute_keys assert response.status_code == 200 @override_settings( @@ -615,7 +617,8 @@ def test_authenticate_jwt_and_no_session_and_set_request_user(self, mock_set_cus mock_set_custom_attribute.assert_any_call('skip_jwt_vs_session_check', True) mock_set_custom_attribute.assert_any_call('jwt_cookie_lms_user_id', jwt_lms_user_id) set_custom_attribute_keys = [call.args[0] for call in mock_set_custom_attribute.call_args_list] - assert 'jwt_auth_mismatch_session_lms_user_id' not in set_custom_attribute_keys + assert 'jwt_auth_mismatch_session_username' not in set_custom_attribute_keys + assert 'jwt_auth_mismatch_jwt_cookie_username' not in set_custom_attribute_keys assert response.status_code == 200 def _get_test_jwt_token(self, user=None, is_valid_signature=True, lms_user_id=None): diff --git a/edx_rest_framework_extensions/auth/jwt/tests/test_decoder.py b/edx_rest_framework_extensions/auth/jwt/tests/test_decoder.py index 0ac9b1d6..9d62fb25 100644 --- a/edx_rest_framework_extensions/auth/jwt/tests/test_decoder.py +++ b/edx_rest_framework_extensions/auth/jwt/tests/test_decoder.py @@ -247,7 +247,7 @@ def test_unsafe_success_with_invalid_token(self): # Generate a token using the invalid signing key token = generate_jwt_token(self.payload, invalid_signing_key) decoded_token = unsafe_jwt_decode_handler(token) - assert decoded_token['username'] is not None + assert decoded_token['preferred_username'] is not None def _jwt_decode_handler_with_defaults(token): # pylint: disable=unused-argument diff --git a/edx_rest_framework_extensions/auth/jwt/tests/utils.py b/edx_rest_framework_extensions/auth/jwt/tests/utils.py index 8cd565db..5aa82bbf 100644 --- a/edx_rest_framework_extensions/auth/jwt/tests/utils.py +++ b/edx_rest_framework_extensions/auth/jwt/tests/utils.py @@ -62,6 +62,9 @@ def generate_latest_version_payload(user, scopes=None, filters=None, version=Non def generate_unversioned_payload(user): """ Generate an unversioned valid JWT payload given a user. + + WARNING: This test utility is mocking JWT creation of the identity service (LMS). + - A safer alternative might be to move the LMS's JWT creation code to this library. """ jwt_issuer_data = settings.JWT_AUTH['JWT_ISSUERS'][0] now = int(time()) @@ -69,8 +72,10 @@ def generate_unversioned_payload(user): payload = { 'iss': jwt_issuer_data['ISSUER'], 'aud': jwt_issuer_data['AUDIENCE'], - 'username': user.username, - 'user_id': user.id, # this should be conditionally added based on the scope + 'preferred_username': user.username, # preferred_username is used by Open edX JWTs. + # WARNING: This `user_id` implementation could lead to bugs because `user_id` should be + # conditionally added based on scope, and should not always be available. + 'user_id': user.id, 'email': user.email, 'iat': now, 'exp': now + ttl, diff --git a/test_settings.py b/test_settings.py index dc0f0674..da05aa35 100644 --- a/test_settings.py +++ b/test_settings.py @@ -46,6 +46,9 @@ 'JWT_LEEWAY': 1, + # This matches the configuration of all Open edX services. + 'JWT_PAYLOAD_GET_USERNAME_HANDLER': lambda d: d.get('preferred_username'), + 'JWT_SECRET_KEY': 'test-key', 'JWT_PUBLIC_SIGNING_JWK_SET': """