diff --git a/pillar/api/eve_settings.py b/pillar/api/eve_settings.py index 0fcb9200..a39caf3b 100644 --- a/pillar/api/eve_settings.py +++ b/pillar/api/eve_settings.py @@ -368,11 +368,11 @@ tokens_schema = { }, 'token': { 'type': 'string', - 'required': False, + 'required': True, }, 'token_hashed': { 'type': 'string', - 'required': True, + 'required': False, }, 'expire_time': { 'type': 'datetime', diff --git a/pillar/api/local_auth.py b/pillar/api/local_auth.py index 8b3fe1bc..a0d8f1c4 100644 --- a/pillar/api/local_auth.py +++ b/pillar/api/local_auth.py @@ -94,17 +94,10 @@ def generate_and_store_token(user_id, days=15, prefix=b'') -> dict: # Use 'xy' as altargs to prevent + and / characters from appearing. # We never have to b64decode the string anyway. - token_bytes = prefix + base64.b64encode(random_bits, altchars=b'xy').strip(b'=') - token = token_bytes.decode('ascii') + token = prefix + base64.b64encode(random_bits, altchars=b'xy').strip(b'=') token_expiry = utcnow() + datetime.timedelta(days=days) - token_data = store_token(user_id, token, token_expiry) - - # Include the token in the returned document so that it can be stored client-side, - # in configuration, etc. - token_data['token'] = token - - return token_data + return store_token(user_id, token.decode('ascii'), token_expiry) def hash_password(password: str, salt: typing.Union[str, bytes]) -> str: diff --git a/pillar/api/utils/authentication.py b/pillar/api/utils/authentication.py index 8ce39240..9d28cf95 100644 --- a/pillar/api/utils/authentication.py +++ b/pillar/api/utils/authentication.py @@ -200,7 +200,7 @@ def remove_token(token: str): tokens_coll = current_app.db('tokens') token_hashed = hash_auth_token(token) - # TODO: remove matching on unhashed tokens once all tokens have been hashed. + # TODO: remove matching on hashed tokens once all hashed tokens have expired. lookup = {'$or': [{'token': token}, {'token_hashed': token_hashed}]} del_res = tokens_coll.delete_many(lookup) log.debug('Removed token %r, matched %d documents', token, del_res.deleted_count) @@ -212,7 +212,7 @@ def find_token(token, is_subclient_token=False, **extra_filters): tokens_coll = current_app.db('tokens') token_hashed = hash_auth_token(token) - # TODO: remove matching on unhashed tokens once all tokens have been hashed. + # TODO: remove matching on hashed tokens once all hashed tokens have expired. lookup = {'$or': [{'token': token}, {'token_hashed': token_hashed}], 'is_subclient_token': True if is_subclient_token else {'$in': [False, None]}, 'expire_time': {"$gt": utcnow()}} @@ -246,7 +246,7 @@ def store_token(user_id, token: str, token_expiry, oauth_subclient_id=False, token_data = { 'user': user_id, - 'token_hashed': hash_auth_token(token), + 'token': token, 'expire_time': token_expiry, } if oauth_subclient_id: diff --git a/pillar/config.py b/pillar/config.py index 7af41916..93e3adad 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -29,6 +29,7 @@ DEBUG = False SECRET_KEY = '' # Authentication token hashing key. If empty falls back to UTF8-encoded SECRET_KEY with a warning. +# Not used to hash new tokens, but it is used to check pre-existing hashed tokens. AUTH_TOKEN_HMAC_KEY = b'' # Authentication settings diff --git a/tests/test_api/test_auth.py b/tests/test_api/test_auth.py index 13b97dfc..eff1286b 100644 --- a/tests/test_api/test_auth.py +++ b/tests/test_api/test_auth.py @@ -114,15 +114,16 @@ class AuthenticationTests(AbstractPillarTest): past = now - datetime.timedelta(days=1) subclient = self.app.config['BLENDER_ID_SUBCLIENT_ID'] + with self.app.test_request_context(): auth.store_token(user_id, 'nonexpired-main', future, None) auth.store_token(user_id, 'nonexpired-sub', future, subclient) token3 = auth.store_token(user_id, 'expired-sub', past, subclient) - # We should not find the given tokens as unhashed tokens. + # We should find the given tokens as unhashed tokens. tokens_coll = self.app.db('tokens') - self.assertIsNone(tokens_coll.find_one({'token': 'nonespired-main'})) - self.assertIsNone(tokens_coll.find_one({'token': 'nonespired-sub'})) - self.assertIsNone(tokens_coll.find_one({'token': 'expired-sub'})) + self.assertIsNotNone(tokens_coll.find_one({'token': 'nonexpired-main'})) + self.assertIsNotNone(tokens_coll.find_one({'token': 'nonexpired-sub'})) + self.assertIsNotNone(tokens_coll.find_one({'token': 'expired-sub'})) with self.app.test_request_context( headers={'Authorization': self.make_header('nonexpired-main')}): @@ -206,19 +207,19 @@ class AuthenticationTests(AbstractPillarTest): with self.app.test_request_context(): from pillar.api.utils import authentication as auth - tokdat_le = auth.store_token(user_id, 'long-expired', - now - datetime.timedelta(days=365), None) - tokdat_se = auth.store_token(user_id, 'short-expired', - now - datetime.timedelta(seconds=5), None) - tokdat_ne = auth.store_token(user_id, 'not-expired', - now + datetime.timedelta(days=1), None) + 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_this_token('je', 'moeder') token_coll = self.app.data.driver.db['tokens'] - self.assertEqual({tokdat_se['token_hashed'], tokdat_ne['token_hashed']}, - {item['token_hashed'] for item in token_coll.find()}) + self.assertEqual({'short-expired', 'not-expired'}, + {item['token'] for item in token_coll.find()}) def test_token_hashing_cli(self): from dateutil.parser import parse diff --git a/tests/test_api/test_blender_id_subclient.py b/tests/test_api/test_blender_id_subclient.py index 6047fcb5..eb465c5c 100644 --- a/tests/test_api/test_blender_id_subclient.py +++ b/tests/test_api/test_blender_id_subclient.py @@ -42,8 +42,6 @@ class BlenderIdSubclientTest(AbstractPillarTest): @responses.activate def test_store_multiple_tokens(self): - from pillar.api.utils.authentication import hash_auth_token - scst1 = '%s-1' % TEST_SUBCLIENT_TOKEN scst2 = '%s-2' % TEST_SUBCLIENT_TOKEN db_user1 = self._common_user_test(201, scst=scst1) @@ -53,10 +51,8 @@ class BlenderIdSubclientTest(AbstractPillarTest): # Now there should be two tokens. with self.app.test_request_context(): tokens = self.app.data.driver.db['tokens'] - self.assertIsNotNone(tokens.find_one( - {'user': db_user1['_id'], 'token_hashed': hash_auth_token(scst1)})) - self.assertIsNotNone(tokens.find_one( - {'user': db_user1['_id'], 'token_hashed': hash_auth_token(scst2)})) + self.assertIsNotNone(tokens.find_one({'user': db_user1['_id'], 'token': scst1})) + self.assertIsNotNone(tokens.find_one({'user': db_user1['_id'], 'token': scst2})) # There should still be only one auth element for blender-id in the user doc. self.assertEqual(1, len(db_user1['auth'])) @@ -80,8 +76,6 @@ class BlenderIdSubclientTest(AbstractPillarTest): def _common_user_test(self, expected_status_code, scst=TEST_SUBCLIENT_TOKEN, expected_full_name=TEST_FULL_NAME, mock_happy_blender_id=True): - from pillar.api.utils.authentication import hash_auth_token - if mock_happy_blender_id: self.mock_blenderid_validate_happy() @@ -110,7 +104,7 @@ class BlenderIdSubclientTest(AbstractPillarTest): # Check that the token was succesfully stored. tokens = self.app.data.driver.db['tokens'] db_token = tokens.find_one({'user': db_user['_id'], - 'token_hashed': hash_auth_token(scst)}) + 'token': scst}) self.assertIsNotNone(db_token) return db_user diff --git a/tests/test_api/test_local_auth.py b/tests/test_api/test_local_auth.py index f327fe0b..5c3e8285 100644 --- a/tests/test_api/test_local_auth.py +++ b/tests/test_api/test_local_auth.py @@ -38,8 +38,6 @@ class LocalAuthTest(AbstractPillarTest): self.assertEqual(200, resp.status_code, resp.data) def test_login_expired_token(self): - from pillar.api.utils.authentication import hash_auth_token - user_id = self.create_test_user() resp = self.client.post('/api/auth/make-token', @@ -54,7 +52,7 @@ class LocalAuthTest(AbstractPillarTest): tokens = self.app.data.driver.db['tokens'] exp = datetime.datetime.now(tz=tz_util.utc) - datetime.timedelta(1) - result = tokens.update_one({'token_hashed': hash_auth_token(token)}, + result = tokens.update_one({'token': token}, {'$set': {'expire_time': exp}}) self.assertEqual(1, result.modified_count) diff --git a/tests/test_api/test_organizations.py b/tests/test_api/test_organizations.py index 77c5b873..36189d74 100644 --- a/tests/test_api/test_organizations.py +++ b/tests/test_api/test_organizations.py @@ -989,8 +989,6 @@ class IPRangeLoginRolesTest(AbstractIPRangeSingleOrgTest): ]}) def _test_api(self, headers: dict, env: dict): - from pillar.api.utils.authentication import hash_auth_token - self.mock_blenderid_validate_happy() # This should check the IP of the user agains the organization IP ranges and update the # user in the database. @@ -1003,7 +1001,7 @@ class IPRangeLoginRolesTest(AbstractIPRangeSingleOrgTest): tokens_coll = self.app.db('tokens') token = tokens_coll.find_one({ 'user': bson.ObjectId(me['_id']), - 'token_hashed': hash_auth_token('usertoken'), + 'token': 'usertoken', }) self.assertEqual(self.org_roles, set(token['org_roles'])) @@ -1033,7 +1031,6 @@ class IPRangeLoginRolesTest(AbstractIPRangeSingleOrgTest): self._test_api_remote_addr('192.168.3.254') def _test_web_forwarded_for(self, ip_addr: str, ip_roles: typing.Set[str]): - from pillar.api.utils.authentication import hash_auth_token from pillar import auth self.mock_blenderid_validate_happy() @@ -1053,7 +1050,7 @@ class IPRangeLoginRolesTest(AbstractIPRangeSingleOrgTest): tokens_coll = self.app.db('tokens') token = tokens_coll.find_one({ 'user': bson.ObjectId(my_id), - 'token_hashed': hash_auth_token('usertoken'), + 'token': 'usertoken', 'expire_time': {'$gt': datetime.datetime.now(tz_util.utc)}, }) self.assertEqual(ip_roles, set(token.get('org_roles', [])))