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

Implement AUTO_REFRESH_MAX_TTL to limit total token lifetime when AUTO_REFRESH = True. #366

Merged
merged 4 commits into from
Sep 30, 2024
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
6 changes: 6 additions & 0 deletions docs/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -78,6 +79,11 @@ successfully returning from `LoginView`. The default is `knox.serializers.UserSe
This defines if the token expiry time is extended by 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.
Expand Down
12 changes: 12 additions & 0 deletions knox/auth.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import binascii
import logging
from hmac import compare_digest

from django.utils import timezone
Expand All @@ -13,6 +14,8 @@
from knox.settings import CONSTANTS, knox_settings
from knox.signals import token_expired

logger = logging.getLogger(__name__)


class TokenAuthentication(BaseAuthentication):
'''
Expand Down Expand Up @@ -74,7 +77,16 @@ 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:
max_expiry = auth_token.created + knox_settings.AUTO_REFRESH_MAX_TTL
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

# Throttle refreshing of token to avoid db writes
delta = (new_expiry - current_expiry).total_seconds()
if delta > knox_settings.MIN_REFRESH_INTERVAL:
Expand Down
1 change: 1 addition & 0 deletions knox/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
33 changes: 33 additions & 0 deletions tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -318,6 +321,36 @@ def test_token_expiry_is_not_extended_within_MIN_REFRESH_INTERVAL(self):
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) # necessary to reload settings in core code
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))

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is ok but I think we need to demonstrate the use case from an http standpoint: status code, error message etc in case auto refresh is not allowed anymore

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I wasn’t clear in my previous comment but can you add the call that actually fails due to the expired token ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an http request to the test and showed that it fails.

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

Expand Down
Loading