diff --git a/jupyterhub/apihandlers/services.py b/jupyterhub/apihandlers/services.py index 13e46724..0b4cced1 100644 --- a/jupyterhub/apihandlers/services.py +++ b/jupyterhub/apihandlers/services.py @@ -39,6 +39,7 @@ class ServiceListAPIHandler(APIHandler): def admin_or_self(method): """Decorator for restricting access to either the target service or admin""" + """***Deprecated in favor of RBAC, use scope-based decorator***""" def decorated_method(self, name): current = self.current_user diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index a06b8910..c06e6f59 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -53,7 +53,7 @@ def _check_user_in_expanded_scope(handler, user_name, scope_group_names): """Check if username is present in set of allowed groups""" user = handler.find_user(user_name) if user is None: - raise web.HTTPError(404, 'No such user found') + raise web.HTTPError(404, "No access to resources or resources not found") group_names = {group.name for group in user.groups} # Todo: Replace with SQL query return bool(set(scope_group_names) & group_names) @@ -66,7 +66,7 @@ def _get_scope_filter(db, req_scope, sub_scope): 'read:groups': 'groups', } if req_scope not in scope_translator: - raise AttributeError("Scope not found; scope filter not constructed") + raise AttributeError("Internal error: inconsistent scope situation") kind = scope_translator[req_scope] Resource = orm.get_class(kind) sub_scope_values = next(iter(sub_scope.values())) @@ -97,6 +97,8 @@ def _check_scope(api_handler, req_scope, scopes, **kwargs): scope_filter = _get_scope_filter(api_handler.db, req_scope, sub_scope) return scope_filter else: + if not kwargs: + return False # Separated from 404 error below because in this case we don't leak information # Interface change: Now can have multiple filters for (filter_, filter_value) in kwargs.items(): if filter_ in sub_scope and filter_value in sub_scope[filter_]: @@ -107,7 +109,7 @@ def _check_scope(api_handler, req_scope, scopes, **kwargs): api_handler, filter_value, group_names ): return True - return False + raise web.HTTPError(404, "No access to resources or resources not found") def _parse_scopes(scope_list): diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index a6883149..501aed62 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -1395,7 +1395,7 @@ async def test_token_authenticator_dict_noauth(app): [ ('admin', 'other', 200), ('admin', 'missing', 404), - ('user', 'other', 403), + ('user', 'other', 404), ('user', 'user', 200), ], ) @@ -1687,7 +1687,7 @@ async def test_update_activity_403(app, user, admin_user): data="{}", method="post", ) - assert r.status_code == 403 + assert r.status_code == 404 async def test_update_activity_admin(app, user, admin_user): diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index e918cebc..13041883 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -54,10 +54,10 @@ def test_scope_check_not_present(): scope_list = ['read:users!user=maeby'] parsed_scopes = _parse_scopes(scope_list) assert not _check_scope(handler, 'read:users', parsed_scopes) - assert not _check_scope(handler, 'read:users', parsed_scopes, user='gob') - assert not _check_scope( - handler, 'read:users', parsed_scopes, user='gob', server='server' - ) + with pytest.raises(web.HTTPError): + _check_scope(handler, 'read:users', parsed_scopes, user='gob') + with pytest.raises(web.HTTPError): + _check_scope(handler, 'read:users', parsed_scopes, user='gob', server='server') def test_scope_filters(): @@ -217,8 +217,8 @@ def generate_test_role(user_name, scopes, role_name='test'): ('martha', False, 200), ('michael', True, 200), ('gob', True, 200), - ('tobias', False, 403), - ('ann', False, 403), + ('tobias', False, 404), + ('ann', False, 404), ], ) async def test_expand_groups(app, user_name, in_group, status_code): @@ -251,7 +251,7 @@ async def test_expand_groups(app, user_name, in_group, status_code): assert r.status_code == status_code -async def test_non_existing_user(app): +async def test_by_fake_user(app): user_name = 'shade' user = add_user(app.db, name=user_name) auth_ = auth_header(app.db, user_name) @@ -261,6 +261,44 @@ async def test_non_existing_user(app): assert r.status_code == 403 +err_message = "No access to resources or resources not found" + + +async def test_request_fake_user(app): + user_name = 'buster' + fake_user = 'annyong' + add_user(app.db, name=user_name) + test_role = generate_test_role(user_name, ['read:users!group=stuff']) + roles.add_role(app.db, test_role) + roles.add_obj(app.db, objname=user_name, kind='users', rolename='test') + app.db.commit() + r = await api_request( + app, 'users', fake_user, headers=auth_header(app.db, user_name) + ) + assert r.status_code == 404 + # Consistency between no user and user not accessible + assert r.json()['message'] == err_message + + +async def test_request_user_outside_group(app): + user_name = 'buster' + fake_user = 'hello' + add_user(app.db, name=user_name) + add_user(app.db, name=fake_user) + test_role = generate_test_role(user_name, ['read:users!group=stuff']) + roles.add_role(app.db, test_role) + roles.add_obj(app.db, objname=user_name, kind='users', rolename='test') + roles.remove_obj(app.db, objname=user_name, kind='users', rolename='user') + app.db.commit() + print(orm.User.find(db=app.db, name=user_name).roles) + r = await api_request( + app, 'users', fake_user, headers=auth_header(app.db, user_name) + ) + assert r.status_code == 404 + # Consistency between no user and user not accessible + assert r.json()['message'] == err_message + + async def test_user_filter(app): user_name = 'rita' user = add_user(app.db, name=user_name)