From 32fd1c50cdc4bd5e82bdb425c78e56964de2419c Mon Sep 17 00:00:00 2001 From: Christian Oudard Date: Wed, 4 Sep 2024 13:52:17 -0600 Subject: [PATCH 1/4] Implement AUTO_REFRESH_MAX_TTL to limit total token lifetime when AUTO_REFRESH = True. --- docs/settings.md | 8 +++++++- knox/auth.py | 6 ++++++ knox/settings.py | 1 + tests/tests.py | 31 ++++++++++++++++++++++++++++++- 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/docs/settings.md b/docs/settings.md index 0ba2317d..16832452 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -20,6 +20,7 @@ REST_KNOX = { 'USER_SERIALIZER': 'knox.serializers.UserSerializer', 'TOKEN_LIMIT_PER_USER': None, 'AUTO_REFRESH': False, + 'AUTO_REFRESH_MAX_TTL': None, 'MIN_REFRESH_INTERVAL': 60, 'AUTH_HEADER_PREFIX': 'Token', 'EXPIRY_DATETIME_FORMAT': api_settings.DATETIME_FORMAT, @@ -75,9 +76,14 @@ This is the reference to the class used to serialize the `User` objects when successfully returning from `LoginView`. The default is `knox.serializers.UserSerializer` ## AUTO_REFRESH -This defines if the token expiry time is extended by TOKEN_TTL each time the token +This defines if the token expiry time is extended by AUTO_REFRESH_TOKEN_TTL each time the token is used. +## AUTO_REFRESH_MAX_TTL +When automatically extending token expiry time, limit the total token lifetime. If +AUTO_REFRESH_MAX_TTL is set, then the token lifetime since the original creation date cannot +exceed AUTO_REFRESH_MAX_TTL. + ## MIN_REFRESH_INTERVAL This is the minimum time in seconds that needs to pass for the token expiry to be updated in the database. diff --git a/knox/auth.py b/knox/auth.py index 858ee504..323181c6 100644 --- a/knox/auth.py +++ b/knox/auth.py @@ -74,7 +74,13 @@ def authenticate_credentials(self, token): def renew_token(self, auth_token) -> None: current_expiry = auth_token.expiry new_expiry = timezone.now() + knox_settings.TOKEN_TTL + + # Do not auto-renew tokens past AUTO_REFRESH_MAX_TTL. + if knox_settings.AUTO_REFRESH_MAX_TTL is not None: + new_expiry = min(new_expiry, auth_token.created + knox_settings.AUTO_REFRESH_MAX_TTL) + auth_token.expiry = new_expiry + # Throttle refreshing of token to avoid db writes delta = (new_expiry - current_expiry).total_seconds() if delta > knox_settings.MIN_REFRESH_INTERVAL: diff --git a/knox/settings.py b/knox/settings.py index a2c3d9c8..2bb6fa7a 100644 --- a/knox/settings.py +++ b/knox/settings.py @@ -13,6 +13,7 @@ 'USER_SERIALIZER': None, 'TOKEN_LIMIT_PER_USER': None, 'AUTO_REFRESH': False, + 'AUTO_REFRESH_MAX_TTL': None, 'MIN_REFRESH_INTERVAL': 60, 'AUTH_HEADER_PREFIX': 'Token', 'EXPIRY_DATETIME_FORMAT': api_settings.DATETIME_FORMAT, diff --git a/tests/tests.py b/tests/tests.py index 9494db0b..8849341f 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -29,6 +29,9 @@ def get_basic_auth_header(username, password): auto_refresh_knox = knox_settings.defaults.copy() auto_refresh_knox["AUTO_REFRESH"] = True +auto_refresh_max_ttl_knox = auto_refresh_knox.copy() +auto_refresh_max_ttl_knox["AUTO_REFRESH_MAX_TTL"] = timedelta(hours=12) + token_user_limit_knox = knox_settings.defaults.copy() token_user_limit_knox["TOKEN_LIMIT_PER_USER"] = 10 @@ -313,11 +316,37 @@ def test_token_expiry_is_not_extended_within_MIN_REFRESH_INTERVAL(self): reload(auth) # necessary to reload settings in core code with freeze_time(in_min_interval): response = self.client.get(root_url, {}, format='json') - reload(auth) # necessary to reload settings in core code + reload(auth) self.assertEqual(response.status_code, 200) self.assertEqual(original_expiry, AuthToken.objects.get().expiry) + def test_token_expiry_is_not_extended_past_max_ttl(self): + ttl = knox_settings.TOKEN_TTL + self.assertEqual(ttl, timedelta(hours=10)) + original_time = datetime(2018, 7, 25, 0, 0, 0, 0) + + with freeze_time(original_time): + instance, token = AuthToken.objects.create(user=self.user) + + self.client.credentials(HTTP_AUTHORIZATION=('Token %s' % token)) + five_hours_later = original_time + timedelta(hours=5) + with override_settings(REST_KNOX=auto_refresh_max_ttl_knox): + reload(auth) # necessary to reload settings in core code + self.assertEqual(auth.knox_settings.AUTO_REFRESH, True) + self.assertEqual(auth.knox_settings.AUTO_REFRESH_MAX_TTL, timedelta(hours=12)) + with freeze_time(five_hours_later): + response = self.client.get(root_url, {}, format='json') + reload(auth) + self.assertEqual(response.status_code, 200) + + # original expiry date was extended, but not past max_ttl: + new_expiry = AuthToken.objects.get().expiry + expected_expiry = original_time + timedelta(hours=12) + self.assertEqual(new_expiry.replace(tzinfo=None), expected_expiry, + "Expiry time should have been extended to {} but is {}." + .format(expected_expiry, new_expiry)) + def test_expiry_signals(self): self.signal_was_called = False From e81886ab48113dd1753d07a0dde460ba075ee358 Mon Sep 17 00:00:00 2001 From: Christian Oudard Date: Wed, 4 Sep 2024 14:19:18 -0600 Subject: [PATCH 2/4] Review notes. --- docs/settings.md | 2 +- knox/auth.py | 3 ++- tests/tests.py | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/settings.md b/docs/settings.md index 16832452..8bf9db73 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -76,7 +76,7 @@ This is the reference to the class used to serialize the `User` objects when successfully returning from `LoginView`. The default is `knox.serializers.UserSerializer` ## AUTO_REFRESH -This defines if the token expiry time is extended by AUTO_REFRESH_TOKEN_TTL each time the token +This defines if the token expiry time is extended by TOKEN_TTL each time the token is used. ## AUTO_REFRESH_MAX_TTL diff --git a/knox/auth.py b/knox/auth.py index 323181c6..d574c5bb 100644 --- a/knox/auth.py +++ b/knox/auth.py @@ -77,7 +77,8 @@ def renew_token(self, auth_token) -> None: # Do not auto-renew tokens past AUTO_REFRESH_MAX_TTL. if knox_settings.AUTO_REFRESH_MAX_TTL is not None: - new_expiry = min(new_expiry, auth_token.created + knox_settings.AUTO_REFRESH_MAX_TTL) + max_expiry = auth_token.created + knox_settings.AUTO_REFRESH_MAX_TTL + new_expiry = min(new_expiry, max_expiry) auth_token.expiry = new_expiry diff --git a/tests/tests.py b/tests/tests.py index 8849341f..0d6f1515 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -316,7 +316,7 @@ def test_token_expiry_is_not_extended_within_MIN_REFRESH_INTERVAL(self): reload(auth) # necessary to reload settings in core code with freeze_time(in_min_interval): response = self.client.get(root_url, {}, format='json') - reload(auth) + reload(auth) # necessary to reload settings in core code self.assertEqual(response.status_code, 200) self.assertEqual(original_expiry, AuthToken.objects.get().expiry) @@ -337,7 +337,7 @@ def test_token_expiry_is_not_extended_past_max_ttl(self): self.assertEqual(auth.knox_settings.AUTO_REFRESH_MAX_TTL, timedelta(hours=12)) with freeze_time(five_hours_later): response = self.client.get(root_url, {}, format='json') - reload(auth) + reload(auth) # necessary to reload settings in core code self.assertEqual(response.status_code, 200) # original expiry date was extended, but not past max_ttl: From b4ea791410db23b236fd71c6b8d1414fa497f673 Mon Sep 17 00:00:00 2001 From: Christian Oudard Date: Fri, 20 Sep 2024 15:45:37 -0600 Subject: [PATCH 3/4] Review notes. --- knox/auth.py | 8 +++++++- tests/tests.py | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/knox/auth.py b/knox/auth.py index d574c5bb..8dd4c803 100644 --- a/knox/auth.py +++ b/knox/auth.py @@ -1,5 +1,6 @@ import binascii from hmac import compare_digest +import logging from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -14,6 +15,9 @@ from knox.signals import token_expired +logger = logging.getLogger(__name__) + + class TokenAuthentication(BaseAuthentication): ''' This authentication scheme uses Knox AuthTokens for authentication. @@ -78,7 +82,9 @@ def renew_token(self, auth_token) -> None: # Do not auto-renew tokens past AUTO_REFRESH_MAX_TTL. if knox_settings.AUTO_REFRESH_MAX_TTL is not None: max_expiry = auth_token.created + knox_settings.AUTO_REFRESH_MAX_TTL - new_expiry = min(new_expiry, max_expiry) + if new_expiry > max_expiry: + new_expiry = max_expiry + logger.info('Token renewal truncated due to AUTO_REFRESH_MAX_TTL.') auth_token.expiry = new_expiry diff --git a/tests/tests.py b/tests/tests.py index 0d6f1515..b91e8ecf 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -347,6 +347,10 @@ def test_token_expiry_is_not_extended_past_max_ttl(self): "Expiry time should have been extended to {} but is {}." .format(expected_expiry, new_expiry)) + with freeze_time(expected_expiry + timedelta(seconds=1)): + response = self.client.get(root_url, {}, format='json') + self.assertEqual(response.status_code, 401) + def test_expiry_signals(self): self.signal_was_called = False From d13ff2af394d3e3b1e88a04d8ef3a31cfed8ea73 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 20 Sep 2024 21:46:24 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- knox/auth.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/knox/auth.py b/knox/auth.py index 8dd4c803..f02576a3 100644 --- a/knox/auth.py +++ b/knox/auth.py @@ -1,6 +1,6 @@ import binascii -from hmac import compare_digest import logging +from hmac import compare_digest from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -14,7 +14,6 @@ from knox.settings import CONSTANTS, knox_settings from knox.signals import token_expired - logger = logging.getLogger(__name__)