diff --git a/pillar/application/utils/authentication.py b/pillar/application/utils/authentication.py index 254143e9..fb18782a 100644 --- a/pillar/application/utils/authentication.py +++ b/pillar/application/utils/authentication.py @@ -6,9 +6,9 @@ module for Blender ID communication. """ import logging +import datetime from bson import tz_util -from datetime import datetime from flask import g from flask import request from flask import current_app @@ -31,6 +31,8 @@ def validate_token(): # Default to no user at all. g.current_user = None + _delete_expired_tokens() + if not request.authorization: # If no authorization headers are provided, we are getting a request # from a non logged in user. Proceed accordingly. @@ -76,7 +78,7 @@ def find_token(token, is_subclient_token=False, **extra_filters): # TODO: remove expired tokens from collection. lookup = {'token': token, 'is_subclient_token': True if is_subclient_token else {'$in': [False, None]}, - 'expire_time': {"$gt": datetime.now(tz=tz_util.utc)}} + 'expire_time': {"$gt": datetime.datetime.now(tz=tz_util.utc)}} lookup.update(extra_filters) db_token = tokens_collection.find_one(lookup) @@ -170,3 +172,20 @@ def make_unique_username(email): if user_from_username is None: return unique_name suffix += 1 + + +def _delete_expired_tokens(): + """Deletes tokens that have expired. + + For debugging, we keep expired tokens around for a few days, so that we + can determine that a token was expired rather than not created in the + first place. It also grants some leeway in clock synchronisation. + """ + + token_coll = current_app.data.driver.db['tokens'] + + now = datetime.datetime.now(tz_util.utc) + expiry_date = now - datetime.timedelta(days=7) + + result = token_coll.delete_many({'expire_time': {"$lt": expiry_date}}) + log.debug('Deleted %i expired authentication tokens', result.deleted_count) diff --git a/tests/test_auth.py b/tests/test_auth.py index 1bc3b194..a2195c9f 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -159,6 +159,30 @@ class AuthenticationTests(AbstractPillarTest): self.assertEqual([u'subscriber'], db_user['roles']) + def test_token_expiry(self): + """Expired tokens should be deleted from the database.""" + + # Insert long-expired, almost-expired and not-expired token. + user_id = self.create_user() + now = datetime.datetime.now(tz_util.utc) + + with self.app.test_request_context(): + from application.utils import authentication as auth + + auth.store_token(user_id, 'long-expired', + now - datetime.timedelta(days=365), None) + auth.store_token(user_id, 'short-expired', + now - datetime.timedelta(seconds=5), None) + auth.store_token(user_id, 'not-expired', + now + datetime.timedelta(days=1), None) + + # Validation should clean up old tokens. + auth.validate_token() + + token_coll = self.app.data.driver.db['tokens'] + self.assertEqual({'short-expired', 'not-expired'}, + {item['token'] for item in token_coll.find()}) + class UserListTests(AbstractPillarTest): """Security-related tests."""