Merge pull request #44 from James1345/develop

Release 3.0.0: Big performance improve

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

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

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):
@@ -57,7 +53,12 @@ def authenticate_credentials(self, token):
         Tokens that have expired will be deleted and skipped
         '''
         msg = _('Invalid token.')
-        for auth_token in AuthToken.objects.all():
+        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()

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,
+        ),
+    ]

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(

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.2.2',
+    version='3.0.0',
 
     description='Authentication for django rest framework',
     long_description=long_description,

tests/tests.py

@@ -37,13 +37,14 @@ def test_logout_deletes_keys(self):
         username, password = 'root', 'toor'
         user = User.objects.create_user(
             username, '[email protected]', password)
-        token = AuthToken.objects.create(user=user)
-        self.assertEqual(AuthToken.objects.count(), 1)
+        for _ in range(2):
+            token = AuthToken.objects.create(user=user)
+        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)
@@ -59,6 +60,36 @@ 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, '[email protected]', password)
+        user2 = User.objects.create_user(
+            'user2', '[email protected]', 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'
+        user = User.objects.create_user(
+            username, '[email protected]', 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'
@@ -83,14 +114,19 @@ def test_update_token_key(self):
         user = User.objects.create_user(
             username, '[email protected]', 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)
+
+    def test_invalid_token_length_returns_401_code(self):
+        invalid_token = "1" * (CONSTANTS.TOKEN_KEY_LENGTH - 1)
+        url = reverse('api-root')
+        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."})
+