Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat! updated jwt vs session user monitoring #392

Merged
merged 1 commit into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@ Change Log
Unreleased
----------

[8.12.0] - 2023-10-16
---------------------

Changed
~~~~~~~
* Made changes to the recent ENABLE_JWT_VS_SESSION_USER_CHECK custom attributes. Although this is technically a breaking change, skipping major release because of limited use of these attributes.

* The jwt_auth_session_user_id attribute has been renamed to clarify that this attribute only appears in the case of a mismatch.
* Dropped jwt_auth_and_session_user_mismatch, which is redundant to simply checking for the existence of jwt_auth_mismatch_session_user_id.
* Updated annotations for jwt_auth_request_user_not_found, because it has proven to be a real case in Production and not just in testing.

[8.11.1] - 2023-10-11
---------------------

Expand Down
2 changes: 1 addition & 1 deletion edx_rest_framework_extensions/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
""" edx Django REST Framework extensions. """

__version__ = '8.11.1' # pragma: no cover
__version__ = '8.12.0' # pragma: no cover
30 changes: 12 additions & 18 deletions edx_rest_framework_extensions/auth/jwt/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,9 @@ def _is_jwt_cookie_and_session_user_mismatch(self, request, jwt_user_id):
)
if not has_request_user: # pragma: no cover
# .. 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.
# .. custom_attribute_description: This custom attribute shows when a
# session user was not found during JWT cookie authentication. This
# attribute will not exist if the session user is found.
set_custom_attribute('jwt_auth_request_user_not_found', True)
return False

Expand All @@ -308,20 +307,15 @@ def _is_jwt_cookie_and_session_user_mismatch(self, request, jwt_user_id):
if not session_user_id or session_user_id == jwt_user_id:
return False

# .. 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)
# .. custom_attribute_name: jwt_auth_mismatch_session_user_id
# .. custom_attribute_description: The session authentication user id if it
# does not match the JWT cookie user id. If there is no session user,
# or if it matches the JWT cookie user id, this attribute will not be
# included. Session authentication may have completed in middleware
# before 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.
set_custom_attribute('jwt_auth_mismatch_session_user_id', session_user_id)

return True

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,7 @@ def test_authenticate_jwt_and_session_mismatch(self, mock_set_custom_attribute):
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_session_user_id', session_user_id)
mock_set_custom_attribute.assert_any_call('jwt_auth_and_session_user_mismatch', True)
mock_set_custom_attribute.assert_any_call('jwt_auth_mismatch_session_user_id', session_user_id)
mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'success-cookie')
assert response.status_code == 200

Expand Down Expand Up @@ -368,8 +367,7 @@ def test_authenticate_jwt_and_session_mismatch_bad_signature_cookie(self, mock_s
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_session_user_id', session_user_id)
mock_set_custom_attribute.assert_any_call('jwt_auth_and_session_user_mismatch', 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)
if enable_forgiving_jwt_cookies:
mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'user-mismatch-failure')
Expand Down Expand Up @@ -402,8 +400,7 @@ def test_authenticate_jwt_and_session_mismatch_invalid_cookie(self, mock_set_cus
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_session_user_id', session_user_id)
mock_set_custom_attribute.assert_any_call('jwt_auth_and_session_user_mismatch', 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', 'decode-error')
if enable_forgiving_jwt_cookies:
mock_set_custom_attribute.assert_any_call('jwt_auth_result', 'user-mismatch-failure')
Expand Down Expand Up @@ -433,8 +430,7 @@ def test_authenticate_jwt_and_session_match(self, mock_set_custom_attribute):
mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_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_check_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 'jwt_auth_mismatch_session_user_id' not in set_custom_attribute_keys
assert response.status_code == 200

@override_settings(
Expand All @@ -459,8 +455,7 @@ def test_authenticate_jwt_and_no_session(self, mock_set_custom_attribute):
mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_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_check_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 'jwt_auth_mismatch_session_user_id' not in set_custom_attribute_keys
assert response.status_code == 200

@override_settings(
Expand All @@ -486,8 +481,7 @@ def test_authenticate_jwt_and_session_mismatch_disabled(self, mock_set_custom_at
mock_set_custom_attribute.assert_any_call('is_jwt_vs_session_user_check_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_check_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 '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):
Expand Down