diff --git a/jupyterhub/apihandlers/services.py b/jupyterhub/apihandlers/services.py index 914483c9..068fd645 100644 --- a/jupyterhub/apihandlers/services.py +++ b/jupyterhub/apihandlers/services.py @@ -29,7 +29,7 @@ def service_model(service): class ServiceListAPIHandler(APIHandler): - @needs_scope('read:services') # Todo: allow filter here? + @needs_scope('read:services') def get(self): data = {name: service_model(service) for name, service in self.services.items()} self.write(json.dumps(data)) diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 2d59e092..e82fa5ae 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -28,7 +28,6 @@ class SelfAPIHandler(APIHandler): 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): user = self.current_user if user is None: @@ -36,6 +35,8 @@ class SelfAPIHandler(APIHandler): user = self.get_current_user_oauth_token() if user is None: raise web.HTTPError(403) + # Later: filter based on scopes. + # Perhaps user self.write(json.dumps(self.user_model(user))) @@ -48,7 +49,7 @@ class UserListAPIHandler(APIHandler): ] self.write(json.dumps(data)) - @needs_scope('users') + @needs_scope('admin:users') async def post(self): data = self.get_json_body() if not data or not isinstance(data, dict) or not data.get('usernames'): diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 52bd0d64..858d2e63 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -8,6 +8,7 @@ from ..utils import check_scope from ..utils import needs_scope from ..utils import parse_scopes from ..utils import Scope +from .utils import add_user from .utils import api_request from .utils import auth_header @@ -166,30 +167,29 @@ def test_scope_method_access(scopes, method, arguments, is_allowed): @mark.parametrize( - "name, status_code", + "user_name, in_group, status_code", [ - ('martha', 200), - ('michael', 200), - ('gob', 200), - ('tobias', 403), - ('ann', 404), # this leaks a bit of information, what do we think about that? + ('martha', False, 200), + ('michael', True, 200), + ('gob', True, 200), + ('tobias', False, 403), + ('ann', False, 403), ], ) -async def test_expand_groups(app, name, status_code): - app.init_db() - group = orm.Group(name='bluth') - app.db.add(group) - for user_name in ['gob', 'michael']: - user = orm.User(name=user_name) - app.db.add(user) +async def test_expand_groups(app, user_name, in_group, status_code): + user = add_user(app.db, name=user_name) + 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) + if in_group and user not in group.users: group.users.append(user) - app.db.add(orm.User(name='tobias')) - app.db.add(orm.User(name='martha')) app.db.commit() app.tornado_settings['mock_scopes'] = [ 'read:users!user=martha', 'read:users!group=bluth', 'read:groups', ] - r = await api_request(app, 'users', name) + r = await api_request(app, 'users', user_name) assert r.status_code == status_code diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py index 0d54dc31..e4ec0944 100644 --- a/jupyterhub/utils.py +++ b/jupyterhub/utils.py @@ -319,15 +319,6 @@ def needs_scope_expansion(filter_, filter_value, sub_scope): 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): user = handler.find_user(user_name) if user is None: @@ -419,7 +410,17 @@ def needs_scope(scope): if check_scope(self, scope, parsed_scopes, **s_kwargs): return func(self, *args, **kwargs) 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