From 6fb9d94b37e00bd2d5304bb735fa314d7a30ad21 Mon Sep 17 00:00:00 2001 From: Raphael Jasjukaitis Date: Thu, 18 Aug 2016 11:40:54 +0200 Subject: [PATCH 1/9] Made token_key not nullable. Huge performance improvements: Iterating only over token_key (mostly just one element). --- knox/auth.py | 7 ++---- knox/migrations/0006_auto_20160818_0932.py | 27 ++++++++++++++++++++++ knox/models.py | 3 +-- setup.py | 2 +- tests/tests.py | 22 +----------------- 5 files changed, 32 insertions(+), 29 deletions(-) create mode 100644 knox/migrations/0006_auto_20160818_0932.py diff --git a/knox/auth.py b/knox/auth.py index c6526234..88cd63c9 100644 --- a/knox/auth.py +++ b/knox/auth.py @@ -43,10 +43,6 @@ def authenticate(self, request): raise exceptions.AuthenticationFailed(msg) user, auth_token = self.authenticate_credentials(auth[1]) - # For a smooth migration to enforce the token_key - if not auth_token.token_key: - auth_token.token_key = auth[1][:CONSTANTS.TOKEN_KEY_LENGTH] - auth_token.save() return (user, auth_token) def authenticate_credentials(self, token): @@ -56,7 +52,8 @@ def authenticate_credentials(self, token): Tokens that have expired will be deleted and skipped ''' - for auth_token in AuthToken.objects.all(): + for auth_token in AuthToken.objects.filter( + token_key=token[:CONSTANTS.TOKEN_KEY_LENGTH]): if auth_token.expires is not None: if auth_token.expires < timezone.now(): auth_token.delete() diff --git a/knox/migrations/0006_auto_20160818_0932.py b/knox/migrations/0006_auto_20160818_0932.py new file mode 100644 index 00000000..b8540905 --- /dev/null +++ b/knox/migrations/0006_auto_20160818_0932.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10 on 2016-08-18 09:32 +from __future__ import unicode_literals + +from django.db import migrations, models + + +def cleanup_tokens(apps, schema_editor): + AuthToken = apps.get_model('knox', 'AuthToken') + AuthToken.objects.filter(token_key__isnull=True).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('knox', '0005_authtoken_token_key'), + ] + + operations = [ + migrations.RunPython(cleanup_tokens), + migrations.AlterField( + model_name='authtoken', + name='token_key', + field=models.CharField(db_index=True, default='', max_length=8), + preserve_default=False, + ), + ] diff --git a/knox/models.py b/knox/models.py index f1525e1d..ba676be8 100644 --- a/knox/models.py +++ b/knox/models.py @@ -31,8 +31,7 @@ class AuthToken(models.Model): digest = models.CharField( max_length=CONSTANTS.DIGEST_LENGTH, primary_key=True) token_key = models.CharField( - max_length=CONSTANTS.TOKEN_KEY_LENGTH, db_index=True, - null=True, blank=True) + max_length=CONSTANTS.TOKEN_KEY_LENGTH, db_index=True) salt = models.CharField( max_length=CONSTANTS.SALT_LENGTH, unique=True) user = models.ForeignKey( diff --git a/setup.py b/setup.py index 1da6890d..da678486 100644 --- a/setup.py +++ b/setup.py @@ -16,7 +16,7 @@ # Versions should comply with PEP440. For a discussion on single-sourcing # the version across setup.py and the project code, see # https://packaging.python.org/en/latest/single_source_version.html - version='2.3.0', + version='3.0.0', description='Authentication for django rest framework', long_description=long_description, diff --git a/tests/tests.py b/tests/tests.py index a6516d17..5fea9896 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -4,11 +4,9 @@ from django.contrib.auth import get_user_model from django.core.urlresolvers import reverse -from rest_framework.test import APIRequestFactory, APITestCase as TestCase +from rest_framework.test import APITestCase as TestCase -from knox.auth import TokenAuthentication from knox.models import AuthToken -from knox.settings import CONSTANTS User = get_user_model() @@ -77,21 +75,3 @@ def test_expired_tokens_deleted(self): self.client.credentials(HTTP_AUTHORIZATION=('Token %s' % token)) self.client.post(url, {}, format='json') self.assertEqual(AuthToken.objects.count(), 0) - - def test_update_token_key(self): - self.assertEqual(AuthToken.objects.count(), 0) - username, password = 'root', 'toor' - user = User.objects.create_user( - username, 'root@localhost.com', password) - token = AuthToken.objects.create(user) - auth_token = AuthToken.objects.first() - auth_token.token_key = None - auth_token.save() - rf = APIRequestFactory() - request = rf.get('/') - request.META = {'HTTP_AUTHORIZATION': 'Token {}'.format(token)} - TokenAuthentication().authenticate(request) - auth_token = AuthToken.objects.get(digest=auth_token.digest) - self.assertEqual( - token[:CONSTANTS.TOKEN_KEY_LENGTH], - auth_token.token_key) From f4e72b7f915bbcaaaab39ddb753ba4e4a49934b8 Mon Sep 17 00:00:00 2001 From: belugame Date: Mon, 19 Dec 2016 09:30:52 -0500 Subject: [PATCH 2/9] Forgot import --- tests/tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/tests.py b/tests/tests.py index 67ebc5bb..d0b34fb9 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -7,6 +7,7 @@ from knox.auth import TokenAuthentication from knox.models import AuthToken +from knox.settings import CONSTANTS User = get_user_model() @@ -100,4 +101,5 @@ def test_invalid_token_length_returns_401_code(self): self.client.credentials(HTTP_AUTHORIZATION=('Token %s' % invalid_token)) response = self.client.post(url, {}, format='json') self.assertEqual(response.status_code, 401) - self.assertEqual(response.data, {"detail": "Invalid token."}) \ No newline at end of file + self.assertEqual(response.data, {"detail": "Invalid token."}) + From 968c77af7861d40e8d8427d63a1ede5eede7aa51 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 11 Jan 2017 10:26:24 -0800 Subject: [PATCH 3/9] Update `test_update_token_key` test so it passes. * No need to set `token_key` to None, it's never cleared anymore. * Use returned AuthToken from `authenticate` rather than trying to find the token in the DB. --- tests/tests.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/tests.py b/tests/tests.py index a77097ec..37e28607 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -83,14 +83,10 @@ def test_update_token_key(self): user = User.objects.create_user( username, 'root@localhost.com', password) token = AuthToken.objects.create(user) - auth_token = AuthToken.objects.first() - auth_token.token_key = None - auth_token.save() rf = APIRequestFactory() request = rf.get('/') request.META = {'HTTP_AUTHORIZATION': 'Token {}'.format(token)} - TokenAuthentication().authenticate(request) - auth_token = AuthToken.objects.get(digest=auth_token.digest) + (user, auth_token) = TokenAuthentication().authenticate(request) self.assertEqual( token[:CONSTANTS.TOKEN_KEY_LENGTH], auth_token.token_key) From 9a6ba16b448b6379c9ead0deec171539c9dc167a Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 11 Jan 2017 10:49:11 -0800 Subject: [PATCH 4/9] Update the `test_logout_deletes_keys` to create multiple tokens for a user to ensure all are deleted. --- tests/tests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/tests.py b/tests/tests.py index 37e28607..ecf6d2ba 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -37,8 +37,9 @@ def test_logout_deletes_keys(self): username, password = 'root', 'toor' user = User.objects.create_user( username, 'root@localhost.com', password) - token = AuthToken.objects.create(user=user) - self.assertEqual(AuthToken.objects.count(), 1) + for _ in range(10): + token = AuthToken.objects.create(user=user) + self.assertEqual(AuthToken.objects.count(), 10) url = reverse('knox_logout') self.client.credentials(HTTP_AUTHORIZATION=('Token %s' % token)) From a7bef68c8cda490fe0803b2aeb2b42746d90db01 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 11 Jan 2017 11:39:29 -0800 Subject: [PATCH 5/9] My mistake, logout should only remove the current token. --- tests/tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/tests.py b/tests/tests.py index ecf6d2ba..885073bc 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -37,14 +37,14 @@ def test_logout_deletes_keys(self): username, password = 'root', 'toor' user = User.objects.create_user( username, 'root@localhost.com', password) - for _ in range(10): + for _ in range(2): token = AuthToken.objects.create(user=user) - self.assertEqual(AuthToken.objects.count(), 10) + self.assertEqual(AuthToken.objects.count(), 2) url = reverse('knox_logout') self.client.credentials(HTTP_AUTHORIZATION=('Token %s' % token)) self.client.post(url, {}, format='json') - self.assertEqual(AuthToken.objects.count(), 0) + self.assertEqual(AuthToken.objects.count(), 1, 'other tokens should remain after logout') def test_logout_all_deletes_keys(self): self.assertEqual(AuthToken.objects.count(), 0) From de89e5de73f63cec49045d3bdbac6db75b225404 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 11 Jan 2017 11:40:27 -0800 Subject: [PATCH 6/9] Add an explicit test that logs in with an expired token to ensure it fails. --- tests/tests.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/tests.py b/tests/tests.py index 885073bc..4dc6d319 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -60,6 +60,19 @@ def test_logout_all_deletes_keys(self): self.client.post(url, {}, format='json') self.assertEqual(AuthToken.objects.count(), 0) + def test_expired_tokens_login_fails(self): + self.assertEqual(AuthToken.objects.count(), 0) + username, password = 'root', 'toor' + user = User.objects.create_user( + username, 'root@localhost.com', password) + token = AuthToken.objects.create( + user=user, expires=datetime.timedelta(seconds=0)) + url = reverse('api-root') + self.client.credentials(HTTP_AUTHORIZATION=('Token %s' % token)) + response = self.client.post(url, {}, format='json') + self.assertEqual(response.status_code, 401) + self.assertEqual(response.data, {"detail": "Invalid token."}) + def test_expired_tokens_deleted(self): self.assertEqual(AuthToken.objects.count(), 0) username, password = 'root', 'toor' From a902c6984149637153271b70dede0d9ddb7fac8e Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 11 Jan 2017 11:40:56 -0800 Subject: [PATCH 7/9] Add another test to ensure logging out doesn't affect other users. --- tests/tests.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/tests.py b/tests/tests.py index 4dc6d319..409aba0b 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -60,6 +60,23 @@ def test_logout_all_deletes_keys(self): self.client.post(url, {}, format='json') self.assertEqual(AuthToken.objects.count(), 0) + def test_logout_all_deletes_only_targets_keys(self): + self.assertEqual(AuthToken.objects.count(), 0) + username, password = 'root', 'toor' + user = User.objects.create_user( + username, 'root@localhost.com', password) + user2 = User.objects.create_user( + 'user2', 'user2@localhost.com', password) + for _ in range(10): + token = AuthToken.objects.create(user=user) + token2 = AuthToken.objects.create(user=user2) + self.assertEqual(AuthToken.objects.count(), 20) + + url = reverse('knox_logoutall') + self.client.credentials(HTTP_AUTHORIZATION=('Token %s' % token)) + self.client.post(url, {}, format='json') + self.assertEqual(AuthToken.objects.count(), 10, 'tokens from other users should not be affected by logout all') + def test_expired_tokens_login_fails(self): self.assertEqual(AuthToken.objects.count(), 0) username, password = 'root', 'toor' From 67bd3793b2786879f7b1e3795ff4e714c8e34dcb Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 11 Jan 2017 11:41:34 -0800 Subject: [PATCH 8/9] Check a users' other tokens for expiration when they attempt to authenticate. --- knox/auth.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/knox/auth.py b/knox/auth.py index 37a4a5cd..8bb91607 100644 --- a/knox/auth.py +++ b/knox/auth.py @@ -55,6 +55,10 @@ def authenticate_credentials(self, token): msg = _('Invalid token.') for auth_token in AuthToken.objects.filter( token_key=token[:CONSTANTS.TOKEN_KEY_LENGTH]): + for other_token in auth_token.user.auth_token_set.all(): + if other_token.digest != auth_token.digest and other_token.expires is not None: + if other_token.expires < timezone.now(): + other_token.delete() if auth_token.expires is not None: if auth_token.expires < timezone.now(): auth_token.delete() From ba9b4c7721d5aab05552fe50ebd585d84f567c50 Mon Sep 17 00:00:00 2001 From: Martin Bauer Date: Wed, 8 Feb 2017 01:16:48 -0200 Subject: [PATCH 9/9] Update changelogs for release 3.0.0 --- CHANGELOG.rst | 10 +++++++++- docs/changes.md | 6 ++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 43791050..36836165 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,3 +1,11 @@ +###### +3.0.0 +###### +**Please be aware: updating to this version requires applying a database migration. All clients will need to reauthenticate.** + +- Big performance fix: Introduction of token_key field to avoid having to compare a login request's token against each and every token in the database (issue #21) +- increased test coverage + ###### 2.2.2 ###### @@ -18,4 +26,4 @@ 2.2.0 ###### -- Change to support python 2.7 +- Change to support python 2.7 diff --git a/docs/changes.md b/docs/changes.md index 2d34ea65..f9da74b0 100644 --- a/docs/changes.md +++ b/docs/changes.md @@ -1,5 +1,11 @@ #Changelog +## 3.0.0 +**Please be aware: updating to this version requires applying a database migration. All clients will need to reauthenticate.** + +- Big performance fix: Introduction of token_key field to avoid having to compare a login request's token against each and every token in the database (issue #21) +- increased test coverage + ## 2.2.2 - Bugfix: invalid token length does no longer trigger a server error - Extending documentation