diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 923456fc..5f392554 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,3 +1,8 @@ +###### +3.1.3 +###### +- Avoid 500 error response for invalid-length token requests + ###### 3.1.2 ###### @@ -65,7 +70,7 @@ ###### 2.2.1 ###### -**Please be aware: updating to his version requires applying a database migration** +**Please be aware: updating to this version requires applying a database migration** - Introducing token_key to avoid loop over all tokens on login-requests - Signals are sent on login/logout diff --git a/docs/changes.md b/docs/changes.md index 40aa11db..6761a772 100644 --- a/docs/changes.md +++ b/docs/changes.md @@ -1,5 +1,8 @@ #Changelog +## 3.1.3 +- Avoid 500 error response for invalid-length token requests + ## 3.1.2 - restore compability with Python <2.7.7 diff --git a/knox/auth.py b/knox/auth.py index b4fba714..a59e3f9a 100644 --- a/knox/auth.py +++ b/knox/auth.py @@ -4,6 +4,8 @@ def compare_digest(a, b): return a == b +import binascii + from django.conf import settings from django.utils.translation import ugettext_lazy as _ from django.utils import timezone @@ -72,7 +74,7 @@ def authenticate_credentials(self, token): continue try: digest = hash_token(token, auth_token.salt) - except TypeError: + except (TypeError, binascii.Error): raise exceptions.AuthenticationFailed(msg) if compare_digest(digest, auth_token.digest): return self.validate_user(auth_token) diff --git a/knox/crypto.py b/knox/crypto.py index 34bd9279..3c857493 100644 --- a/knox/crypto.py +++ b/knox/crypto.py @@ -24,6 +24,9 @@ def hash_token(token, salt): ''' Calculates the hash of a token and salt. input is unhexlified + + token and salt must contain an even number of hex digits or + a binascii.Error exception will be raised ''' digest = hashes.Hash(sha(), backend=default_backend()) digest.update(binascii.unhexlify(token)) diff --git a/setup.py b/setup.py index bfb2616f..c1fdd151 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='3.1.2', + version='3.1.3', description='Authentication for django rest framework', long_description=long_description, diff --git a/tests/tests.py b/tests/tests.py index 42eb6352..cf56c3cf 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -26,13 +26,18 @@ def get_basic_auth_header(username, password): class AuthTestCase(TestCase): + def setUp(self): + self.username, self.email, self.password = 'john.doe', 'john.doe@example.com', 'hunter2' + self.user = User.objects.create_user(self.username, self.email, self.password) + + self.username2, self.email2, self.password2 = 'jane.doe', 'jane.doe@example.com', 'hunter2' + self.user2 = User.objects.create_user(self.username2, self.email2, self.password2) + def test_login_creates_keys(self): self.assertEqual(AuthToken.objects.count(), 0) - username, password = 'root', 'toor' - User.objects.create_user(username, 'root@localhost.com', password) url = reverse('knox_login') self.client.credentials( - HTTP_AUTHORIZATION=get_basic_auth_header(username, password)) + HTTP_AUTHORIZATION=get_basic_auth_header(self.username, self.password)) for _ in range(5): self.client.post(url, {}, format='json') @@ -41,11 +46,8 @@ def test_login_creates_keys(self): def test_logout_deletes_keys(self): self.assertEqual(AuthToken.objects.count(), 0) - username, password = 'root', 'toor' - user = User.objects.create_user( - username, 'root@localhost.com', password) for _ in range(2): - token = AuthToken.objects.create(user=user) + token = AuthToken.objects.create(user=self.user) self.assertEqual(AuthToken.objects.count(), 2) url = reverse('knox_logout') @@ -55,11 +57,8 @@ def test_logout_deletes_keys(self): def test_logout_all_deletes_keys(self): self.assertEqual(AuthToken.objects.count(), 0) - username, password = 'root', 'toor' - user = User.objects.create_user( - username, 'root@localhost.com', password) for _ in range(10): - token = AuthToken.objects.create(user=user) + token = AuthToken.objects.create(user=self.user) self.assertEqual(AuthToken.objects.count(), 10) url = reverse('knox_logoutall') @@ -69,14 +68,9 @@ def test_logout_all_deletes_keys(self): 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) + token = AuthToken.objects.create(user=self.user) + token2 = AuthToken.objects.create(user=self.user2) self.assertEqual(AuthToken.objects.count(), 20) url = reverse('knox_logoutall') @@ -86,11 +80,8 @@ def test_logout_all_deletes_only_targets_keys(self): 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)) + user=self.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') @@ -99,13 +90,10 @@ def test_expired_tokens_login_fails(self): def test_expired_tokens_deleted(self): self.assertEqual(AuthToken.objects.count(), 0) - username, password = 'root', 'toor' - user = User.objects.create_user( - username, 'root@localhost.com', password) for _ in range(10): # 0 TTL gives an expired token token = AuthToken.objects.create( - user=user, expires=datetime.timedelta(seconds=0)) + user=self.user, expires=datetime.timedelta(seconds=0)) self.assertEqual(AuthToken.objects.count(), 10) # Attempting a single logout should delete all tokens @@ -117,14 +105,11 @@ def test_expired_tokens_deleted(self): 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) + token = AuthToken.objects.create(self.user) rf = APIRequestFactory() request = rf.get('/') request.META = {'HTTP_AUTHORIZATION': 'Token {}'.format(token)} - (user, auth_token) = TokenAuthentication().authenticate(request) + (self.user, auth_token) = TokenAuthentication().authenticate(request) self.assertEqual( token[:CONSTANTS.TOKEN_KEY_LENGTH], auth_token.token_key) @@ -136,3 +121,12 @@ def test_invalid_token_length_returns_401_code(self): response = self.client.post(url, {}, format='json') self.assertEqual(response.status_code, 401) self.assertEqual(response.data, {"detail": "Invalid token."}) + + def test_invalid_odd_length_token_returns_401_code(self): + token = AuthToken.objects.create(self.user) + odd_length_token = token + '1' + url = reverse('api-root') + self.client.credentials(HTTP_AUTHORIZATION=('Token %s' % odd_length_token)) + response = self.client.post(url, {}, format='json') + self.assertEqual(response.status_code, 401) + self.assertEqual(response.data, {"detail": "Invalid token."})