diff --git a/docs/source/rbac/scopes.md b/docs/source/rbac/scopes.md index b8772324..a3a1aadc 100644 --- a/docs/source/rbac/scopes.md +++ b/docs/source/rbac/scopes.md @@ -72,13 +72,29 @@ Requested resources are filtered based on the filter of the corresponding scope. In case a user resource is being accessed, any scopes with _group_ filters will be expanded to filters for each _user_ in those groups. -### `!user` filter +### Self-referencing filters + +There are some 'shortcut' filters, +which can be applied to all scopes, +that filter based on the entities associated with the request. The `!user` filter is a special horizontal filter that strictly refers to the **"owner only"** scopes, where _owner_ is a user entity. The filter resolves internally into `!user=` ensuring that only the owner's resources may be accessed through the associated scopes. For example, the `server` role assigned by default to server tokens contains `access:servers!user` and `users:activity!user` scopes. This allows the token to access and post activity of only the servers owned by the token owner. -The filter can be applied to any scope. +:::{versionadded} 2.3 +`!service` and `!server` filters. +::: + +In addition to `!user`, _tokens_ may have filters `!service` +or `!server`, which expand similarly to `!service=servicename` +and `!server=servername`. +This only applies to tokens issued via the OAuth flow. +In these cases, the name is the _issuing_ entity (a service or single-user server), +so that access can be restricted to the issuing service, +e.g. `access:servers!server` would grant access only to the server that requested the token. + +These filters can be applied to any scope. (vertical-filtering-target)= diff --git a/jupyterhub/oauth/provider.py b/jupyterhub/oauth/provider.py index 05637142..dc19eb42 100644 --- a/jupyterhub/oauth/provider.py +++ b/jupyterhub/oauth/provider.py @@ -351,7 +351,7 @@ class JupyterHubRequestValidator(RequestValidator): # APIToken.new commits the token to the db orm.APIToken.new( - client_id=client.identifier, + oauth_client=client, expires_in=token['expires_in'], scopes=request.scopes, token=token['access_token'], diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 2536eb70..b30b8040 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -682,7 +682,8 @@ class APIToken(Hashed, Base): generated=True, session_id=None, expires_in=None, - client_id='jupyterhub', + client_id=None, + oauth_client=None, return_orm=False, ): """Generate a new API token for a user or service""" @@ -726,11 +727,20 @@ class APIToken(Hashed, Base): orm_roles.append(role) scopes = roles_to_scopes(orm_roles) + if oauth_client is None: + # lookup oauth client by identifier + if client_id is None: + # default: global 'jupyterhub' client + client_id = "jupyterhub" + oauth_client = db.query(OAuthClient).filter_by(identifier=client_id).one() + if client_id is None: + client_id = oauth_client.identifier + # avoid circular import from .scopes import _check_scopes_exist, _check_token_scopes _check_scopes_exist(scopes, who_for="token") - _check_token_scopes(scopes, owner=user or service) + _check_token_scopes(scopes, owner=user or service, oauth_client=oauth_client) # two stages to ensure orm_token.generated has been set # before token setter is called @@ -760,7 +770,9 @@ class APIToken(Hashed, Base): from .scopes import _check_scopes_exist, _check_token_scopes _check_scopes_exist(new_scopes, who_for="token") - _check_token_scopes(new_scopes, owner=self.owner) + _check_token_scopes( + new_scopes, owner=self.owner, oauth_client=self.oauth_client + ) self.scopes = new_scopes diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 1d83ba60..1d82ab54 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -341,7 +341,13 @@ def get_scopes_for(orm_object): # only thing we miss by short-circuiting here: warning about excluded extra scopes return owner_scopes - token_scopes = set(expand_scopes(token_scopes, owner=owner)) + token_scopes = set( + expand_scopes( + token_scopes, + owner=owner, + oauth_client=orm_object.oauth_client, + ) + ) if orm_object.client_id != "jupyterhub": # oauth tokens can be used to access the service issuing the token, @@ -468,7 +474,7 @@ def _expand_scope(scope): return frozenset(expanded_scopes) -def _expand_scopes_key(scopes, owner=None): +def _expand_scopes_key(scopes, owner=None, oauth_client=None): """Cache key function for expand_scopes scopes is usually a mutable list or set, @@ -484,11 +490,15 @@ def _expand_scopes_key(scopes, owner=None): else: # owner key is the type and name owner_key = (type(owner).__name__, owner.name) - return (frozen_scopes, owner_key) + if oauth_client is None: + oauth_client_key = None + else: + oauth_client_key = oauth_client.identifier + return (frozen_scopes, owner_key, oauth_client_key) @lru_cache_key(_expand_scopes_key) -def expand_scopes(scopes, owner=None): +def expand_scopes(scopes, owner=None, oauth_client=None): """Returns a set of fully expanded scopes for a collection of raw scopes Arguments: @@ -496,28 +506,47 @@ def expand_scopes(scopes, owner=None): owner (obj, optional): orm.User or orm.Service as owner of orm.APIToken Used for expansion of metascopes such as `self` and owner-based filters such as `!user` + oauth_client (obj, optional): orm.OAuthClient + The issuing OAuth client of an API token. Returns: expanded scopes (set): set of all expanded scopes, with filters applied for the owner """ expanded_scopes = set(chain.from_iterable(map(_expand_scope, scopes))) + filter_replacements = { + "user": None, + "service": None, + "server": None, + } + owner_name = None if isinstance(owner, orm.User): owner_name = owner.name - else: - owner_name = None + filter_replacements["user"] = f"user={owner_name}" + elif isinstance(owner, orm.Service): + filter_replacements["service"] = f"service={owner.name}" + + if oauth_client is not None: + if oauth_client.service is not None: + filter_replacements["service"] = f"service={oauth_client.service.name}" + elif oauth_client.spawner is not None: + spawner = oauth_client.spawner + filter_replacements["server"] = f"server={spawner.user.name}/{spawner.name}" for scope in expanded_scopes.copy(): base_scope, _, filter = scope.partition('!') - if filter == 'user': + if filter in filter_replacements: # translate !user into !user={username} + # and !service into !service={servicename} + # and !server into !server={username}/{servername} expanded_scopes.remove(scope) - if owner_name: + expanded_filter = filter_replacements[filter] + if expanded_filter: # translate - expanded_scopes.add(f'{base_scope}!user={owner_name}') + expanded_scopes.add(f'{base_scope}!{expanded_filter}') else: warnings.warn( - f"Not expanding !user filter without owner in {scope}", + f"Not expanding !{filter} filter without target {filter} in {scope}", stacklevel=2, ) @@ -610,7 +639,8 @@ def _check_scopes_exist(scopes, who_for=None): """ allowed_scopes = set(scope_definitions.keys()) - allowed_filters = ('!user=', '!service=', '!group=', '!server=', '!user') + filter_prefixes = ('!user=', '!service=', '!group=', '!server=') + exact_filters = {"!user", "!service", "!server"} if who_for: log_for = f"for {who_for}" @@ -625,13 +655,15 @@ def _check_scopes_exist(scopes, who_for=None): raise KeyError(f"Scope '{scope}' {log_for} does not exist") if filter_: full_filter = f"!{filter_}" - if not full_filter.startswith(allowed_filters): + if full_filter not in exact_filters and not full_filter.startswith( + filter_prefixes + ): raise KeyError( f"Scope filter {filter_} '{full_filter}' in scope '{scope}' {log_for} does not exist" ) -def _check_token_scopes(scopes, owner): +def _check_token_scopes(scopes, owner, oauth_client): """Check that scopes to be assigned to a token are in fact @@ -648,7 +680,7 @@ def _check_token_scopes(scopes, owner): return scopes.discard("inherit") # common short circuit - token_scopes = expand_scopes(scopes, owner=owner) + token_scopes = expand_scopes(scopes, owner=owner, oauth_client=oauth_client) if not token_scopes: return diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 60109bb1..e51533ee 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -285,7 +285,22 @@ class MockServiceSpawner(jupyterhub.services.service._ServiceSpawner): _mock_service_counter = 0 -def _mockservice(request, app, url=False): +def _mockservice(request, app, external=False, url=False): + """ + Add a service to the application + + Args: + request: pytest request fixture + app: MockHub application + external (bool): + If False (default), launch the service. + Otherwise, consider it 'external, + registering a service in the database, + but don't start it. + url (bool): + If True, register the service at a URL + (as opposed to headless, API-only). + """ global _mock_service_counter _mock_service_counter += 1 name = 'mock-service-%i' % _mock_service_counter @@ -296,6 +311,10 @@ def _mockservice(request, app, url=False): else: spec['url'] = 'http://127.0.0.1:%i' % random_port() + if external: + + spec['oauth_redirect_uri'] = 'http://127.0.0.1:%i' % random_port() + io_loop = app.io_loop with mock.patch.object( @@ -313,17 +332,20 @@ def _mockservice(request, app, url=False): await app.proxy.add_all_services(app._service_map) await service.start() - io_loop.run_sync(start) + if not external: + io_loop.run_sync(start) def cleanup(): - asyncio.get_event_loop().run_until_complete(service.stop()) + if not external: + asyncio.get_event_loop().run_until_complete(service.stop()) app.services[:] = [] app._service_map.clear() request.addfinalizer(cleanup) # ensure process finishes starting - with raises(TimeoutExpired): - service.proc.wait(1) + if not external: + with raises(TimeoutExpired): + service.proc.wait(1) if url: io_loop.run_sync(partial(service.server.wait_up, http=True)) return service @@ -335,6 +357,12 @@ def mockservice(request, app): yield _mockservice(request, app, url=False) +@fixture +def mockservice_external(request, app): + """Mock an externally managed service (don't start anything)""" + yield _mockservice(request, app, external=True, url=False) + + @fixture def mockservice_url(request, app): """Mock a service with its own url to test external services""" diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 2f40d949..4e8f7a9f 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -1151,28 +1151,52 @@ async def test_user_filter_expansion(app, create_user_with_scopes): @pytest.mark.parametrize( "scopes, expected", [ - ("read:users:name!user", ["read:users:name!user=$user"]), + ("read:users:name!user", ["read:users:name!user={user}"]), ( "users:activity!user", [ - "read:users:activity!user=$user", - "users:activity!user=$user", + "read:users:activity!user={user}", + "users:activity!user={user}", ], ), ("self", ["*"]), (["access:services", "access:services!service=x"], ["access:services"]), + ("access:services!service", ["access:services!service={service}"]), + ("access:servers!server", ["access:servers!server={server}"]), ], ) -def test_expand_scopes(user, scopes, expected): +def test_expand_scopes(app, user, scopes, expected, mockservice_external): if isinstance(scopes, str): scopes = [scopes] - scopes = {s.replace("$user", user.name) for s in scopes} - expected = {s.replace("$user", user.name) for s in expected} + + db = app.db + service = mockservice_external + spawner_name = "salmon" + server_name = f"{user.name}/{spawner_name}" + if 'server' in str(scopes): + oauth_client = orm.OAuthClient() + db.add(oauth_client) + spawner = user.spawners[spawner_name] + spawner.orm_spawner.oauth_client = oauth_client + db.commit() + assert oauth_client.spawner is spawner.orm_spawner + else: + oauth_client = service.oauth_client + assert oauth_client is not None + + def format_scopes(scopes): + return { + s.format(service=service.name, server=server_name, user=user.name) + for s in scopes + } + + scopes = format_scopes(scopes) + expected = format_scopes(expected) if "*" in expected: expected.remove("*") expected.update(_expand_self_scope(user.name)) - expanded = expand_scopes(scopes, owner=user.orm_user) + expanded = expand_scopes(scopes, owner=user.orm_user, oauth_client=oauth_client) assert isinstance(expanded, frozenset) assert sorted(expanded) == sorted(expected)