Speed up authentication by trusting g.current_user if set.

This commit is contained in:
Sybren A. Stüvel 2018-01-30 12:40:19 +01:00
parent ed1e348d67
commit 4b5a961e14
5 changed files with 124 additions and 70 deletions

View File

@ -113,7 +113,7 @@ def create_home_project(user_id, write_access):
# Re-validate the authentication token, so that the put_internal call sees the # Re-validate the authentication token, so that the put_internal call sees the
# new group created for the project. # new group created for the project.
authentication.validate_token() authentication.validate_token(force=True)
# There are a few things in the on_insert_projects hook we need to adjust. # There are a few things in the on_insert_projects hook we need to adjust.

View File

@ -16,7 +16,6 @@ import bson
from bson import tz_util from bson import tz_util
from flask import g, current_app from flask import g, current_app
from flask import request from flask import request
from flask import current_app
from werkzeug import exceptions as wz_exceptions from werkzeug import exceptions as wz_exceptions
from pillar.api.utils import remove_private_keys from pillar.api.utils import remove_private_keys
@ -105,7 +104,7 @@ def find_user_in_db(user_info: dict, provider='blender-id') -> dict:
return db_user return db_user
def validate_token(): def validate_token(*, force=False):
"""Validate the token provided in the request and populate the current_user """Validate the token provided in the request and populate the current_user
flask.g object, so that permissions and access to a resource can be defined flask.g object, so that permissions and access to a resource can be defined
from it. from it.
@ -113,11 +112,19 @@ def validate_token():
When the token is successfully validated, sets `g.current_user` to contain When the token is successfully validated, sets `g.current_user` to contain
the user information, otherwise it is set to None. the user information, otherwise it is set to None.
@returns True iff the user is logged in with a valid Blender ID token. :param force: don't trust g.current_user and force a re-check.
:returns: True iff the user is logged in with a valid Blender ID token.
""" """
from pillar.auth import AnonymousUser from pillar.auth import AnonymousUser
# Trust a pre-existing g.current_user
if not force:
cur = getattr(g, 'current_user', None)
if cur is not None and cur.is_authenticated:
log.debug('skipping token check because current user is already set to %s', cur)
return True
auth_header = request.headers.get('Authorization') or '' auth_header = request.headers.get('Authorization') or ''
if request.authorization: if request.authorization:
token = request.authorization.username token = request.authorization.username
@ -359,7 +366,6 @@ def setup_app(app):
@app.before_request @app.before_request
def validate_token_at_each_request(): def validate_token_at_each_request():
validate_token() validate_token()
return None
def upsert_user(db_user): def upsert_user(db_user):

View File

@ -138,7 +138,8 @@ class AbstractPillarTest(TestMinimal):
self.app.process_extensions() self.app.process_extensions()
assert self.app.config['MONGO_DBNAME'] == 'pillar_test' assert self.app.config['MONGO_DBNAME'] == 'pillar_test'
self.client = self.app.test_client() self.app.testing = True
self.client = self.app.test_client(use_cookies=False)
assert isinstance(self.client, FlaskClient) assert isinstance(self.client, FlaskClient)
def tearDown(self): def tearDown(self):
@ -157,9 +158,14 @@ class AbstractPillarTest(TestMinimal):
The app context is automatically exited upon testcase teardown. The app context is automatically exited upon testcase teardown.
""" """
from flask import g
self._app_ctx: flask.ctx.AppContext = self.app.app_context() self._app_ctx: flask.ctx.AppContext = self.app.app_context()
self._app_ctx.__enter__() self._app_ctx.__enter__()
if hasattr(g, 'current_user'):
g.current_user = None
def unload_modules(self, module_name): def unload_modules(self, module_name):
"""Uploads the named module, and all submodules.""" """Uploads the named module, and all submodules."""

View File

@ -50,6 +50,34 @@ class AuthenticationTests(AbstractPillarTest):
with self.app.test_request_context(): with self.app.test_request_context():
self.assertFalse(auth.validate_token()) self.assertFalse(auth.validate_token())
@responses.activate
def test_validate_token__force_logged_in(self):
from pillar.api.utils import authentication as auth
from pillar.auth import UserClass
self.mock_blenderid_validate_happy()
with self.app.test_request_context(
headers={'Authorization': self.make_header('knowntoken')}):
from flask import g
g.current_user = UserClass('12345')
self.assertTrue(g.current_user.is_authenticated)
self.assertTrue(auth.validate_token(force=True))
self.assertTrue(g.current_user.is_authenticated)
@responses.activate
def test_validate_token__force_not_logged_in(self):
from pillar.api.utils import authentication as auth
from pillar.auth import UserClass, current_user
with self.app.test_request_context():
from flask import g
g.current_user = UserClass('12345')
self.assertTrue(g.current_user.is_authenticated)
self.assertFalse(auth.validate_token(force=True))
self.assertFalse(g.current_user.is_authenticated)
@responses.activate @responses.activate
def test_validate_token__unknown_token(self): def test_validate_token__unknown_token(self):
"""Test validating of invalid token, unknown both to us and Blender ID.""" """Test validating of invalid token, unknown both to us and Blender ID."""
@ -78,24 +106,23 @@ class AuthenticationTests(AbstractPillarTest):
from pillar.api.utils import authentication as auth from pillar.api.utils import authentication as auth
self.enter_app_context()
user_id = self.create_user() user_id = self.create_user()
now = datetime.datetime.now(tz_util.utc) with self.app.app_context():
future = now + datetime.timedelta(days=1) now = datetime.datetime.now(tz_util.utc)
past = now - datetime.timedelta(days=1) future = now + datetime.timedelta(days=1)
subclient = self.app.config['BLENDER_ID_SUBCLIENT_ID'] past = now - datetime.timedelta(days=1)
subclient = self.app.config['BLENDER_ID_SUBCLIENT_ID']
auth.store_token(user_id, 'nonexpired-main', future, None) auth.store_token(user_id, 'nonexpired-main', future, None)
auth.store_token(user_id, 'nonexpired-sub', future, subclient) auth.store_token(user_id, 'nonexpired-sub', future, subclient)
token3 = auth.store_token(user_id, 'expired-sub', past, subclient) token3 = auth.store_token(user_id, 'expired-sub', past, subclient)
# We should not find the given tokens as unhashed tokens. # We should not find the given tokens as unhashed tokens.
tokens_coll = self.app.db('tokens') tokens_coll = self.app.db('tokens')
self.assertIsNone(tokens_coll.find_one({'token': 'nonespired-main'})) 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': 'nonespired-sub'}))
self.assertIsNone(tokens_coll.find_one({'token': 'expired-sub'})) self.assertIsNone(tokens_coll.find_one({'token': 'expired-sub'}))
with self.app.test_request_context( with self.app.test_request_context(
headers={'Authorization': self.make_header('nonexpired-main')}): headers={'Authorization': self.make_header('nonexpired-main')}):
@ -195,32 +222,32 @@ class AuthenticationTests(AbstractPillarTest):
def test_token_hashing_cli(self): def test_token_hashing_cli(self):
from dateutil.parser import parse from dateutil.parser import parse
self.enter_app_context()
user_id1 = self.create_user(24 * 'a') with self.app.app_context():
user_id2 = self.create_user(24 * 'b') user_id1 = self.create_user(24 * 'a')
now = datetime.datetime.now(tz_util.utc) user_id2 = self.create_user(24 * 'b')
now = datetime.datetime.now(tz_util.utc)
# Force unhashed tokens into our database. # Force unhashed tokens into our database.
tokens_coll = self.app.db('tokens') tokens_coll = self.app.db('tokens')
tokens_coll.insert_one({ tokens_coll.insert_one({
'_id': ObjectId('59c0f8ef98377327e1525cd1'), '_id': ObjectId('59c0f8ef98377327e1525cd1'),
'_etag': '3b8fffa5177e87555acd95e49e6764cb81de5d70', '_etag': '3b8fffa5177e87555acd95e49e6764cb81de5d70',
'_created': parse('2017-09-19T13:01:03.000+0200'), '_created': parse('2017-09-19T13:01:03.000+0200'),
'_updated': parse('2017-09-19T13:01:03.000+0200'), '_updated': parse('2017-09-19T13:01:03.000+0200'),
'user': user_id1, 'user': user_id1,
'token': 'unhashed-token', 'token': 'unhashed-token',
'expire_time': now + datetime.timedelta(hours=1), 'expire_time': now + datetime.timedelta(hours=1),
}) })
tokens_coll.insert_one({ tokens_coll.insert_one({
'_id': ObjectId(24 * 'c'), '_id': ObjectId(24 * 'c'),
'_etag': '3b8fffa5177e87555acd95e49e6764cb81de5d70', '_etag': '3b8fffa5177e87555acd95e49e6764cb81de5d70',
'_created': parse('2017-09-20T13:01:03.000+0200'), '_created': parse('2017-09-20T13:01:03.000+0200'),
'_updated': parse('2017-09-20T13:01:03.000+0200'), '_updated': parse('2017-09-20T13:01:03.000+0200'),
'user': user_id2, 'user': user_id2,
'token': 'some-other-token', 'token': 'some-other-token',
'expire_time': now + datetime.timedelta(hours=1), 'expire_time': now + datetime.timedelta(hours=1),
}) })
# This token should work. # This token should work.
me1 = self.get('/api/users/me', auth_token='unhashed-token').get_json() me1 = self.get('/api/users/me', auth_token='unhashed-token').get_json()
@ -228,8 +255,9 @@ class AuthenticationTests(AbstractPillarTest):
me2 = self.get('/api/users/me', auth_token='some-other-token').get_json() me2 = self.get('/api/users/me', auth_token='some-other-token').get_json()
self.assertEqual(str(user_id2), me2['_id']) self.assertEqual(str(user_id2), me2['_id'])
from pillar.cli.operations import hash_auth_tokens with self.app.app_context():
hash_auth_tokens() from pillar.cli.operations import hash_auth_tokens
hash_auth_tokens()
# The same token should still work, but be hashed in the DB. # The same token should still work, but be hashed in the DB.
me1 = self.get('/api/users/me', auth_token='unhashed-token').get_json() me1 = self.get('/api/users/me', auth_token='unhashed-token').get_json()
@ -750,9 +778,9 @@ class UserCreationTest(AbstractPillarTest):
@responses.activate @responses.activate
def test_update_by_auth_no_full_name(self): def test_update_by_auth_no_full_name(self):
"""Blender ID does not require full name, we do.""" """Blender ID does not require full name, we do."""
self.enter_app_context() with self.app.app_context():
users_coll = self.app.db().users users_coll = self.app.db().users
self.assertEqual(0, users_coll.count()) self.assertEqual(0, users_coll.count())
# First request will create the user, the 2nd request will update. # First request will create the user, the 2nd request will update.
self.mock_blenderid_validate_happy() self.mock_blenderid_validate_happy()
@ -769,21 +797,23 @@ class UserCreationTest(AbstractPillarTest):
token = 'this is my life now' token = 'this is my life now'
self.get('/api/users/me', auth_token=token) self.get('/api/users/me', auth_token=token)
# Clear out the full name of the user. This could happen for some with self.app.app_context():
# reason, and it shouldn't break the login flow. # Clear out the full name of the user. This could happen for some
users_coll.update_many({}, {'$set': {'full_name': ''}}) # reason, and it shouldn't break the login flow.
users_coll.update_many({}, {'$set': {'full_name': ''}})
# Delete all tokens to force a re-check with Blender ID # Delete all tokens to force a re-check with Blender ID
tokens_coll = self.app.db('tokens') tokens_coll = self.app.db('tokens')
tokens_coll.delete_many({}) tokens_coll.delete_many({})
self.get('/api/users/me', auth_token=token) self.get('/api/users/me', auth_token=token)
self.assertEqual(1, users_coll.count()) with self.app.app_context():
self.assertEqual(1, users_coll.count())
db_user = users_coll.find()[0] db_user = users_coll.find()[0]
self.assertEqual(db_user['email'], TEST_EMAIL_ADDRESS) self.assertEqual(db_user['email'], TEST_EMAIL_ADDRESS)
self.assertNotEqual('', db_user['full_name']) self.assertNotEqual('', db_user['full_name'])
def test_user_without_email_address(self): def test_user_without_email_address(self):
"""Regular users should always have an email address. """Regular users should always have an email address.

View File

@ -12,7 +12,7 @@ class AbstractOrgTest(AbstractPillarTest):
def setUp(self, **kwargs): def setUp(self, **kwargs):
super().setUp(**kwargs) super().setUp(**kwargs)
self.enter_app_context() # self.enter_app_context()
self.om = self.app.org_manager self.om = self.app.org_manager
@ -492,10 +492,11 @@ class OrganizationResourceEveTest(AbstractOrgTest):
self.admin1_uid = self.create_user(24 * 'a', token='admin1-token') self.admin1_uid = self.create_user(24 * 'a', token='admin1-token')
self.admin2_uid = self.create_user(24 * 'b', token='admin2-token') self.admin2_uid = self.create_user(24 * 'b', token='admin2-token')
org_doc = self.om.create_new_org('Хакеры 1', self.admin1_uid, 25) with self.app.app_context():
self.org1_id = org_doc['_id'] org_doc = self.om.create_new_org('Хакеры 1', self.admin1_uid, 25)
org_doc = self.om.create_new_org('Хакеры 2', self.admin2_uid, 10) self.org1_id = org_doc['_id']
self.org2_id = org_doc['_id'] org_doc = self.om.create_new_org('Хакеры 2', self.admin2_uid, 10)
self.org2_id = org_doc['_id']
# Create members of the organizations. # Create members of the organizations.
self.member1_uid = self.create_user(24 * 'd', self.member1_uid = self.create_user(24 * 'd',
@ -504,8 +505,10 @@ class OrganizationResourceEveTest(AbstractOrgTest):
self.member2_uid = self.create_user(24 * 'e', self.member2_uid = self.create_user(24 * 'e',
email='member2@example.com', email='member2@example.com',
token='member2-token') token='member2-token')
self.om.assign_users(self.org1_id, ['member1@example.com', 'member2@example.com'])
self.om.assign_users(self.org2_id, ['member2@example.com']) with self.app.app_context():
self.om.assign_users(self.org1_id, ['member1@example.com', 'member2@example.com'])
self.om.assign_users(self.org2_id, ['member2@example.com'])
# Create an outside user. # Create an outside user.
self.outsider_uid = self.create_user(24 * '0', token='outsider-token') self.outsider_uid = self.create_user(24 * '0', token='outsider-token')
@ -515,7 +518,8 @@ class OrganizationResourceEveTest(AbstractOrgTest):
self.org2_doc = self._from_db(self.org2_id) self.org2_doc = self._from_db(self.org2_id)
def _from_db(self, org_id) -> dict: def _from_db(self, org_id) -> dict:
return self.om._get_org(org_id) with self.app.app_context():
return self.om._get_org(org_id)
def test_list_as_pillar_admin(self): def test_list_as_pillar_admin(self):
"""Pillar Admins should see all orgs""" """Pillar Admins should see all orgs"""
@ -778,8 +782,10 @@ class AbstractIPRangeSingleOrgTest(AbstractOrgTest):
super().setUp(**kwargs) super().setUp(**kwargs)
self.uid = self.create_user(24 * 'a', roles={'subscriber'}, token='token') self.uid = self.create_user(24 * 'a', roles={'subscriber'}, token='token')
self.org_roles = {'org-subscriber', 'org-phabricator'} self.org_roles = {'org-subscriber', 'org-phabricator'}
self.org = self.app.org_manager.create_new_org('Хакеры', self.uid, 25,
org_roles=self.org_roles) with self.app.app_context():
self.org = self.app.org_manager.create_new_org('Хакеры', self.uid, 25,
org_roles=self.org_roles)
self.org_id = self.org['_id'] self.org_id = self.org['_id']
def _patch(self, payload: dict, expected_status=204) -> dict: def _patch(self, payload: dict, expected_status=204) -> dict:
@ -791,7 +797,9 @@ class AbstractIPRangeSingleOrgTest(AbstractOrgTest):
}, },
auth_token='token', auth_token='token',
expected_status=expected_status) expected_status=expected_status)
db_org = self.om._get_org(self.org_id)
with self.app.app_context():
db_org = self.om._get_org(self.org_id)
return db_org return db_org
@ -927,6 +935,8 @@ class IPRangeQueryTest(AbstractOrgTest):
return db_org return db_org
def test_query(self): def test_query(self):
self.enter_app_context()
# Set up a few organisations. A and B have overlapping IPv4 ranges, B and C on IPv6. # Set up a few organisations. A and B have overlapping IPv4 ranges, B and C on IPv6.
org_roles_a = {'org-roleA1', 'org-roleA2'} org_roles_a = {'org-roleA1', 'org-roleA2'}
org_a = self.app.org_manager.create_new_org('Хакеры', self.uid, 25, org_roles=org_roles_a) org_a = self.app.org_manager.create_new_org('Хакеры', self.uid, 25, org_roles=org_roles_a)
@ -989,6 +999,7 @@ class IPRangeLoginRolesTest(AbstractIPRangeSingleOrgTest):
me = resp.json() me = resp.json()
# The IP-based roles should be stored in the token document. # The IP-based roles should be stored in the token document.
self.enter_app_context()
tokens_coll = self.app.db('tokens') tokens_coll = self.app.db('tokens')
token = tokens_coll.find_one({ token = tokens_coll.find_one({
'user': bson.ObjectId(me['_id']), 'user': bson.ObjectId(me['_id']),
@ -1038,6 +1049,7 @@ class IPRangeLoginRolesTest(AbstractIPRangeSingleOrgTest):
self.assertEqual(expect_roles, set(auth.current_user.roles)) self.assertEqual(expect_roles, set(auth.current_user.roles))
# The IP-based roles should be stored in the token document. # The IP-based roles should be stored in the token document.
self.enter_app_context()
tokens_coll = self.app.db('tokens') tokens_coll = self.app.db('tokens')
token = tokens_coll.find_one({ token = tokens_coll.find_one({
'user': bson.ObjectId(my_id), 'user': bson.ObjectId(my_id),