Fixed scopes and added more specific logs/errors

This commit is contained in:
Omar Richardson
2020-11-23 13:26:36 +01:00
parent d5e7a42135
commit d7d27ad97a
4 changed files with 31 additions and 29 deletions

View File

@@ -29,7 +29,7 @@ def service_model(service):
class ServiceListAPIHandler(APIHandler): class ServiceListAPIHandler(APIHandler):
@needs_scope('read:services') # Todo: allow filter here? @needs_scope('read:services')
def get(self): def get(self):
data = {name: service_model(service) for name, service in self.services.items()} data = {name: service_model(service) for name, service in self.services.items()}
self.write(json.dumps(data)) self.write(json.dumps(data))

View File

@@ -28,7 +28,6 @@ class SelfAPIHandler(APIHandler):
Based on the authentication info. Acts as a 'whoami' for auth tokens. Based on the authentication info. Acts as a 'whoami' for auth tokens.
""" """
# @needs_scope('read:users') # Should be read:users:user=username
async def get(self): async def get(self):
user = self.current_user user = self.current_user
if user is None: if user is None:
@@ -36,6 +35,8 @@ class SelfAPIHandler(APIHandler):
user = self.get_current_user_oauth_token() user = self.get_current_user_oauth_token()
if user is None: if user is None:
raise web.HTTPError(403) raise web.HTTPError(403)
# Later: filter based on scopes.
# Perhaps user
self.write(json.dumps(self.user_model(user))) self.write(json.dumps(self.user_model(user)))
@@ -48,7 +49,7 @@ class UserListAPIHandler(APIHandler):
] ]
self.write(json.dumps(data)) self.write(json.dumps(data))
@needs_scope('users') @needs_scope('admin:users')
async def post(self): async def post(self):
data = self.get_json_body() data = self.get_json_body()
if not data or not isinstance(data, dict) or not data.get('usernames'): if not data or not isinstance(data, dict) or not data.get('usernames'):

View File

@@ -8,6 +8,7 @@ from ..utils import check_scope
from ..utils import needs_scope from ..utils import needs_scope
from ..utils import parse_scopes from ..utils import parse_scopes
from ..utils import Scope from ..utils import Scope
from .utils import add_user
from .utils import api_request from .utils import api_request
from .utils import auth_header from .utils import auth_header
@@ -166,30 +167,29 @@ def test_scope_method_access(scopes, method, arguments, is_allowed):
@mark.parametrize( @mark.parametrize(
"name, status_code", "user_name, in_group, status_code",
[ [
('martha', 200), ('martha', False, 200),
('michael', 200), ('michael', True, 200),
('gob', 200), ('gob', True, 200),
('tobias', 403), ('tobias', False, 403),
('ann', 404), # this leaks a bit of information, what do we think about that? ('ann', False, 403),
], ],
) )
async def test_expand_groups(app, name, status_code): async def test_expand_groups(app, user_name, in_group, status_code):
app.init_db() user = add_user(app.db, name=user_name)
group = orm.Group(name='bluth') group_name = 'bluth'
group = orm.Group.find(app.db, name=group_name)
if not group:
group = orm.Group(name=group_name)
app.db.add(group) app.db.add(group)
for user_name in ['gob', 'michael']: if in_group and user not in group.users:
user = orm.User(name=user_name)
app.db.add(user)
group.users.append(user) group.users.append(user)
app.db.add(orm.User(name='tobias'))
app.db.add(orm.User(name='martha'))
app.db.commit() app.db.commit()
app.tornado_settings['mock_scopes'] = [ app.tornado_settings['mock_scopes'] = [
'read:users!user=martha', 'read:users!user=martha',
'read:users!group=bluth', 'read:users!group=bluth',
'read:groups', 'read:groups',
] ]
r = await api_request(app, 'users', name) r = await api_request(app, 'users', user_name)
assert r.status_code == status_code assert r.status_code == status_code

View File

@@ -319,15 +319,6 @@ def needs_scope_expansion(filter_, filter_value, sub_scope):
return True return True
# def expand_groups_to_users(db, filter_scope):
# """Update the group filters to account for the individual users"""
# if 'group' in filter_scope:
# groups = db.query(orm.Group)
# user_set = orm.User.query.filter(orm.User.group.in_(groups))
# return [user.name for user in user_set]
#
def check_user_in_expanded_scope(handler, user_name, scope_group_names): def check_user_in_expanded_scope(handler, user_name, scope_group_names):
user = handler.find_user(user_name) user = handler.find_user(user_name)
if user is None: if user is None:
@@ -419,7 +410,17 @@ def needs_scope(scope):
if check_scope(self, scope, parsed_scopes, **s_kwargs): if check_scope(self, scope, parsed_scopes, **s_kwargs):
return func(self, *args, **kwargs) return func(self, *args, **kwargs)
else: else:
raise web.HTTPError(403, "Action is not authorized with current scopes") self.log.warning(
"Not authorizing access to {}. Requires scope {}, not derived from scopes {}".format(
self.request.path, scope, ", ".join(self.scopes)
)
)
raise web.HTTPError(
403,
"Action is not authorized with current scopes; requires {}".format(
scope
),
)
return _auth_func return _auth_func