diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py index 600931d5..2b4a447a 100644 --- a/jupyterhub/apihandlers/base.py +++ b/jupyterhub/apihandlers/base.py @@ -88,9 +88,12 @@ class APIHandler(BaseHandler): if sub_scope == scopes.Scope.ALL: return True else: - found_resource = orm_resource.name in sub_scope[kind] + try: + found_resource = orm_resource.name in sub_scope[kind] + except KeyError: + found_resource = False if not found_resource: # Try group-based access - if kind == 'server': + if kind == 'server' and 'user' in sub_scope: # First check if we have access to user info user_name = orm_resource.user.name found_resource = user_name in sub_scope['user'] diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index ceb53801..56775e34 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -188,11 +188,13 @@ class UserAPIHandler(APIHandler): @needs_scope( 'read:users', 'read:users:name', - 'reda:users:servers', + 'read:users:servers', 'read:users:groups', 'read:users:activity', ) - async def get(self, user_name): + async def get( + self, user_name + ): # Fixme: Does not work when only server filter is selected user = self.find_user(user_name) model = self.user_model(user) # auth state will only be shown if the requester is an admin @@ -263,7 +265,7 @@ class UserAPIHandler(APIHandler): self.set_status(204) - @needs_scope('admin:users') + @needs_scope('admin:users') # Todo: Change to `users`? async def patch(self, user_name): user = self.find_user(user_name) if user is None: @@ -326,6 +328,7 @@ class UserTokenListAPIHandler(APIHandler): oauth_tokens.append(self.token_model(token)) self.write(json.dumps({'api_tokens': api_tokens, 'oauth_tokens': oauth_tokens})) + # Todo: Set to @needs_scope('users:tokens') async def post(self, user_name): body = self.get_json_body() or {} if not isinstance(body, dict): @@ -765,7 +768,7 @@ class ActivityAPIHandler(APIHandler): ) return servers - @needs_scope('users') + @needs_scope('users:activity') def post(self, user_name): user = self.find_user(user_name) if user is None: diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 8131cad7..3f733dd8 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1377,7 +1377,7 @@ class JupyterHub(Application): Can be a Unicode string (e.g. '/hub/home') or a callable based on the handler object: :: - + def default_url_fn(handler): user = handler.current_user if user and user.admin: diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 886354e6..53df7857 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -90,8 +90,8 @@ def _get_scope_hierarchy(): 'read:users:servers', ], 'users:tokens': ['read:users:tokens'], - 'admin:users': ['admin:users:auth_state', 'admin:users:server_state'], - 'admin:users:servers': None, + 'admin:users': ['admin:users:auth_state'], + 'admin:users:servers': ['admin:users:server_state'], 'groups': ['read:groups'], 'admin:groups': None, 'read:services': None, @@ -236,7 +236,7 @@ def assign_default_roles(db, entity): # tokens can have only 'token' role as default # assign the default only for tokens if isinstance(entity, orm.APIToken): - if not entity.roles and entity.user is not None: + if not entity.roles and (entity.user or entity.service) is not None: default_token_role.tokens.append(entity) db.commit() # users and services can have 'user' or 'admin' roles as default diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 0819cc08..58c0e334 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -64,7 +64,7 @@ def _check_user_in_expanded_scope(handler, user_name, scope_group_names): def _check_scope(api_handler, req_scope, **kwargs): """Check if scopes satisfy requirements - Returns True for (restricted) access, False for refused access + Returns True for (potentially restricted) access, False for refused access """ # Parse user name and server name together try: @@ -74,24 +74,23 @@ def _check_scope(api_handler, req_scope, **kwargs): if 'user' in kwargs and 'server' in kwargs: kwargs['server'] = "{}/{}".format(kwargs['user'], kwargs['server']) if req_scope not in api_handler.parsed_scopes: - app_log.debug("No scopes present to access %s" % api_name) + app_log.debug("No access to %s via %s", api_name, req_scope) return False if api_handler.parsed_scopes[req_scope] == Scope.ALL: - app_log.debug("Unrestricted access to %s call", api_name) + app_log.debug("Unrestricted access to %s via %s", api_name, req_scope) return True # Apply filters sub_scope = api_handler.parsed_scopes[req_scope] if not kwargs: app_log.debug( - "Client has restricted access to %s. Internal filtering may apply" - % api_name + "Client has restricted access to %s via %s. Internal filtering may apply", + api_name, + req_scope, ) return True for (filter_, filter_value) in kwargs.items(): if filter_ in sub_scope and filter_value in sub_scope[filter_]: - app_log.debug( - "Restricted client access supported by endpoint %s" % api_name - ) + app_log.debug("Argument-based access to %s via %s", api_name, req_scope) return True if _needs_scope_expansion(filter_, filter_value, sub_scope): group_names = sub_scope['group'] @@ -160,27 +159,26 @@ def needs_scope(*scopes): if resource_name in bound_sig.arguments: resource_value = bound_sig.arguments[resource_name] s_kwargs[resource] = resource_value - has_access = False for scope in scopes: - has_access |= _check_scope(self, scope, **s_kwargs) - if has_access: - return func(self, *args, **kwargs) - else: - try: - end_point = self.request.path - except AttributeError: - end_point = self.__name__ - app_log.warning( - "Not authorizing access to {}. Requires any of [{}], not derived from scopes [{}]".format( - end_point, ", ".join(scopes), ", ".join(self.raw_scopes) - ) - ) - raise web.HTTPError( - 403, - "Action is not authorized with current scopes; requires any of [{}]".format( - ", ".join(scopes) - ), + app_log.debug("Checking access via scope %s", scope) + has_access = _check_scope(self, scope, **s_kwargs) + if has_access: + return func(self, *args, **kwargs) + try: + end_point = self.request.path + except AttributeError: + end_point = self.__name__ + app_log.warning( + "Not authorizing access to {}. Requires any of [{}], not derived from scopes [{}]".format( + end_point, ", ".join(scopes), ", ".join(self.raw_scopes) ) + ) + raise web.HTTPError( + 403, + "Action is not authorized with current scopes; requires any of [{}]".format( + ", ".join(scopes) + ), + ) return _auth_func diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index e929f078..3dc49052 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -340,6 +340,12 @@ async def test_load_roles_tokens(tmpdir, request): 'scopes': ['users:servers', 'admin:servers'], 'tokens': ['another-secret-token'], }, + { + 'name': 'admin', + 'description': 'Admin access', + 'scopes': ['a lot'], + 'users': ['admin'], + }, ] kwargs = { 'load_roles': roles_to_load, diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 9b431f65..0706e8f4 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -540,6 +540,7 @@ async def test_metascope_self_expansion(app, kind, has_user_scopes): assert bool(token_scopes) == has_user_scopes app.db.delete(orm_obj) app.db.delete(test_role) + app.db.commit() async def test_metascope_all_expansion(app): @@ -557,3 +558,77 @@ async def test_metascope_all_expansion(app): app.db.commit() token_scope_set = get_scopes_for(token) assert not token_scope_set + + +@mark.parametrize( + "scopes, can_stop ,num_servers, keys_in, keys_out", + [ + (['read:users:servers!user=almond'], False, 2, {'name'}, {'state'}), + ( + ['admin:users:server_state', 'read:users:servers'], + True, + 2, + {'name', 'state'}, + set(), + ), + (['users:servers', 'read:users:name'], True, 0, set(), set()), + ( + [ + 'read:users:name!user=almond', + 'read:users:servers!server=almond/bianca', # fixme: server-scope not working yet + 'admin:users:server_state!server=almond/bianca', + ], + False, + 1, + {'name', 'state'}, + set(), + ), + ], +) +async def test_server_state_access( + app, scopes, can_stop, num_servers, keys_in, keys_out +): + with mock.patch.dict( + app.tornado_settings, + {'allow_named_servers': True, 'named_server_limit_per_user': 2}, + ): + ## 1. Test a user can access all servers without auth_state + ## 2. Test a service with admin:user but no admin:users:servers gets no access to any server data + ## 3. Test a service with admin:user:server_state gets access to auth_state + ## 4. Test a service with user:servers:server gives access to one server, and the correct server. + username = 'almond' + user = add_user(app.db, app, name=username) + + server_names = ['bianca', 'terry'] + try: + for server_name in server_names: + await api_request( + app, 'users', username, 'servers', server_name, method='post' + ) + role = orm.Role(name=f"{username}-role", scopes=scopes) + app.db.add(role) + app.db.commit() + service_name = 'server_accessor' + service = orm.Service(name=service_name) + app.db.add(service) + service.roles.append(role) + app.db.commit() + api_token = service.new_api_token() + app.init_roles() + headers = {'Authorization': 'token %s' % api_token} + r = await api_request(app, 'users', username, headers=headers) + r.raise_for_status() + user_model = r.json() + if num_servers: + assert 'servers' in user_model + server_models = user_model['servers'] + assert len(server_models) == num_servers + for server, server_model in server_models.items(): + assert keys_in.issubset(server_model) + assert keys_out.isdisjoint(server_model) + else: + assert 'servers' not in user_model + finally: + app.db.delete(role) + app.db.delete(service) + app.db.commit()