diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 86b4db89..fa37037a 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -27,7 +27,6 @@ from .utils import api_request from .utils import async_requests from .utils import auth_header from .utils import find_user -from .utils import get_scopes # -------------------- diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 494ec8f9..4e2ed343 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -3,10 +3,13 @@ import pytest from pytest import mark from tornado import web +from .. import orm from ..utils import check_scope from ..utils import needs_scope from ..utils import parse_scopes from ..utils import Scope +from .utils import api_request +from .utils import auth_header def test_scope_constructor(): @@ -74,7 +77,7 @@ def test_scope_parse_server_name(): ) -class MockAPI: +class MockAPIHandler: def __init__(self): self.scopes = ['users'] @@ -152,7 +155,7 @@ class MockAPI: ], ) def test_scope_method_access(scopes, method, arguments, is_allowed): - obj = MockAPI() + obj = MockAPIHandler() obj.scopes = scopes api_call = getattr(obj, method) if is_allowed: @@ -160,3 +163,19 @@ def test_scope_method_access(scopes, method, arguments, is_allowed): else: with pytest.raises(web.HTTPError): api_call(*arguments) + + +async def test_expand_groups(app): + db = app.db + user = orm.User(name='gob') + group = orm.Group(name='bluth') + db.add(group) + db.add(user) + group.users.append(user) + db.commit() + scopes = ['read:users!user=micheal', 'read:users!group=bluth', 'read:groups'] + app.tornado_settings['mock_scopes'] = scopes + r = await api_request(app, 'users', 'micheal', headers=auth_header(db, 'micheal')) + assert r.status_code == 200 + r = await api_request(app, 'users', 'gob', headers=auth_header(db, 'user')) + assert r.status_code == 200 diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py index 1b2ce995..0d54dc31 100644 --- a/jupyterhub/utils.py +++ b/jupyterhub/utils.py @@ -307,7 +307,6 @@ def needs_scope_expansion(filter_, filter_value, sub_scope): """ Check if there is a requirements to expand the `group` scope to individual `user` scopes. Assumptions: - req_scopes in scopes filter_ != Scope.ALL This can be made arbitrarily intelligent but that would make it more complex @@ -334,7 +333,7 @@ def check_user_in_expanded_scope(handler, user_name, scope_group_names): if user is None: raise web.HTTPError(404, 'No such user found') group_names = {group.name for group in user.groups} - return bool(scope_group_names & group_names) + return bool(set(scope_group_names) & group_names) def check_scope(api_handler, req_scope, scopes, **kwargs): @@ -354,12 +353,15 @@ def check_scope(api_handler, req_scope, scopes, **kwargs): filter_, filter_value = list(kwargs.items())[0] sub_scope = scopes[req_scope] if filter_ not in sub_scope: - if needs_scope_expansion(filter_, filter_value, sub_scope): - group_names = sub_scope['groups'] - return check_user_in_expanded_scope(api_handler, filter_value, group_names) - else: - return False - return filter_value in sub_scope[filter_] + valid_scope = False + else: + valid_scope = filter_value in sub_scope[filter_] + if not valid_scope and needs_scope_expansion(filter_, filter_value, sub_scope): + group_names = sub_scope['group'] + valid_scope |= check_user_in_expanded_scope( + api_handler, filter_value, group_names + ) + return valid_scope def parse_scopes(scope_list):