No longer hash auth tokens + store the token scopes
This partially reverts commit c57aefd48b10ca3cabc9df162bc32efa62a6a21e. The code to check against hashed tokens remains, because existing tokens should still work. The unhashed tokens are necessary for fetching badges from Blender ID.
This commit is contained in:
parent
a753637e70
commit
85eab0c6cb
@ -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',
|
||||
|
@ -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:
|
||||
|
@ -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:
|
||||
|
@ -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
|
||||
|
@ -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',
|
||||
auth.store_token(user_id, 'long-expired',
|
||||
now - datetime.timedelta(days=365), None)
|
||||
tokdat_se = auth.store_token(user_id, 'short-expired',
|
||||
auth.store_token(user_id, 'short-expired',
|
||||
now - datetime.timedelta(seconds=5), None)
|
||||
tokdat_ne = auth.store_token(user_id, 'not-expired',
|
||||
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
|
||||
|
@ -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
|
||||
|
@ -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)
|
||||
|
||||
|
@ -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', [])))
|
||||
|
Loading…
x
Reference in New Issue
Block a user