diff --git a/docs/source/_static/rest-api.yml b/docs/source/_static/rest-api.yml index 7e1fd0cf..236ddb68 100644 --- a/docs/source/_static/rest-api.yml +++ b/docs/source/_static/rest-api.yml @@ -560,7 +560,19 @@ paths: description: A note attached to the token for future bookkeeping roles: type: array - description: A list of role names that the token should have + description: | + A list of role names from which to derive scopes. + This is a shortcut for assigning collections of scopes; + Tokens do not retain role assignment. + (Changed in 2.3: roles are immediately resolved to scopes + instead of stored on roles.) + items: + type: string + scopes: + type: array + description: | + A list of scopes that the token should have. + (new in JupyterHub 2.3). items: type: string required: false @@ -1314,7 +1326,15 @@ components: description: The service that owns the token (undefined of owned by a user) roles: type: array - description: The names of roles this token has + description: Deprecated in JupyterHub 2.3, always an empty list. + Tokens have 'scopes' starting from JupyterHub 2.3. + items: + type: string + scopes: + type: array + description: List of scopes this token has been assigned. + New in JupyterHub 2.3. In JupyterHub 2.0-2.2, + tokens were assigned 'roles' insead of scopes. items: type: string note: diff --git a/docs/source/rbac/roles.md b/docs/source/rbac/roles.md index 47927c64..7eb683d7 100644 --- a/docs/source/rbac/roles.md +++ b/docs/source/rbac/roles.md @@ -41,7 +41,7 @@ Services do not have a default role. Services without roles have no access to th A group does not require any role, and has no roles by default. If a user is a member of a group, they automatically inherit any of the group's permissions (see {ref}`resolving-roles-scopes-target` for more details). This is useful for assigning a set of common permissions to several users. **Tokens** \ -A token’s permissions are evaluated based on their owning entity. Since a token is always issued for a user or service, it can never have more permissions than its owner. If no specific role is requested for a new token, the token is assigned the `token` role. +A token’s permissions are evaluated based on their owning entity. Since a token is always issued for a user or service, it can never have more permissions than its owner. If no specific scopes are requested for a new token, the token is assigned the `token` role. (define-role-target)= diff --git a/docs/source/rbac/tech-implementation.md b/docs/source/rbac/tech-implementation.md index c396e220..7420842d 100644 --- a/docs/source/rbac/tech-implementation.md +++ b/docs/source/rbac/tech-implementation.md @@ -7,11 +7,11 @@ Roles and scopes utilities can be found in `roles.py` and `scopes.py` modules. S ```{admonition} **Scope variable nomenclature** :class: tip - _scopes_ \ - List of scopes with abbreviations (used in role definitions). E.g., `["users:activity!user"]`. + List of scopes that may contain abbreviations (used in role definitions). E.g., `["users:activity!user", "self"]`. - _expanded scopes_ \ - Set of expanded scopes without abbreviations (i.e., resolved metascopes, filters and subscopes). E.g., `{"users:activity!user=charlie", "read:users:activity!user=charlie"}`. + Set of fully expanded scopes without abbreviations (i.e., resolved metascopes, filters, and subscopes). E.g., `{"users:activity!user=charlie", "read:users:activity!user=charlie"}`. - _parsed scopes_ \ - Dictionary JSON like format of expanded scopes. E.g., `{"users:activity": {"user": ["charlie"]}, "read:users:activity": {"users": ["charlie"]}}`. + Dictionary represenation of expanded scopes. E.g., `{"users:activity": {"user": ["charlie"]}, "read:users:activity": {"users": ["charlie"]}}`. - _intersection_ \ Set of expanded scopes as intersection of 2 expanded scope sets. - _identify scopes_ \ @@ -32,17 +32,29 @@ Roles and scopes are resolved on several occasions, for example when requesting ### Requesting API token with specific roles -API tokens grant access to JupyterHub's APIs. The RBAC framework allows for requesting tokens with specific existing roles. To date, it is only possible to add roles to a token through the _POST /users/:name/tokens_ API where the roles can be specified in the token parameters body (see [](../reference/rest-api.rst)). +:::{versionchanged} 2.3 +API tokens have _scopes_ instead of roles, +so that their permissions cannot be updated. + +You may still request roles for a token, +but those roles will be evaluated to the corresponding _scopes_ immediately. +::: + +API tokens grant access to JupyterHub's APIs. The RBAC framework allows for requesting tokens with specific permissions. +As of JupyterHub 2.3, it is only possible to specify scopes for a token through the _POST /users/:name/tokens_ API where the scopes can be specified in the token parameters body (see [](../reference/rest-api.rst)). RBAC adds several steps into the token issue flow. -If no roles are requested, the token is issued with the default `token` role (providing the requester is allowed to create the token). +If no scopes are requested, the token is issued with the permissions stored on the default `token` role +(providing the requester is allowed to create the token). -If the token is requested with any roles, the permissions of requesting entity are checked against the requested permissions to ensure the token would not grant its owner additional privileges. +If the token is requested with any scopes, the permissions of requesting entity are checked against the requested permissions to ensure the token would not grant its owner additional privileges. -If, due to modifications of roles or entities, at API request time a token has any scopes that its owner does not, those scopes are removed. The API request is resolved without additional errors using the scopes _intersection_, but the Hub logs a warning (see {ref}`Figure 2 `). +If, due to modifications of roles or entities, at API request time a token has any scopes that its owner does not, those scopes are removed. +The API request is resolved without additional errors using the scope _intersection_, +but the Hub logs a warning (see {ref}`Figure 2 `). -Resolving a token's roles (yellow box in {ref}`Figure 1 `) corresponds to resolving all the token's owner roles (including the roles associated with their groups) and the token's requested roles into a set of scopes. The two sets are compared (Resolve the scopes box in orange in {ref}`Figure 1 `), taking into account the scope hierarchy but, solely for role assignment, omitting any {ref}`horizontal filter ` comparison. If the token's scopes are a subset of the token owner's scopes, the token is issued with the requested roles; if not, JupyterHub will raise an error. +Resolving a token's scope (yellow box in {ref}`Figure 1 `) corresponds to resolving all the token's owner roles (including the roles associated with their groups) and the token's requested roles into a set of scopes. The two sets are compared (Resolve the scopes box in orange in {ref}`Figure 1 `), taking into account the scope hierarchy but, solely for role assignment, omitting any {ref}`horizontal filter ` comparison. If the token's scopes are a subset of the token owner's scopes, the token is issued with the requested roles; if not, JupyterHub will raise an error. {ref}`Figure 1 ` below illustrates the steps involved. The orange rectangles highlight where in the process the roles and scopes are resolved. diff --git a/jupyterhub/apihandlers/auth.py b/jupyterhub/apihandlers/auth.py index cd8b1947..aec972df 100644 --- a/jupyterhub/apihandlers/auth.py +++ b/jupyterhub/apihandlers/auth.py @@ -224,7 +224,7 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): else: self.log.debug( f"User {user.name} has authorized {oauth_client.identifier}" - f" for scopes {authorized_scopes}, confirming additonal scopes {requested_scopes}" + f" for scopes {authorized_scopes}, confirming additional scopes {requested_scopes}" ) # default: require confirmation return True @@ -289,13 +289,22 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): user = self.current_user # raw, _not_ expanded scopes user_scopes = roles.roles_to_scopes(roles.get_roles_for(user.orm_user)) + # these are some scopes the user may not have + # in 'raw' form, but definitely have at this point + # make sure they are here, because we are computing the + # 'raw' scope intersection, + # rather than the expanded_scope intersection + + required_scopes = {*scopes.identify_scopes(), *scopes.access_scopes(client)} + user_scopes.update({"inherit", *required_scopes}) + allowed_scopes = requested_scopes.intersection(user_scopes) excluded_scopes = requested_scopes.difference(user_scopes) - # TODO: compute lower-level intersection - # of _expanded_ scopes + # TODO: compute lower-level intersection of remaining _expanded_ scopes + # (e.g. user has admin:users, requesting read:users!group=x) if excluded_scopes: - self.log.info( + self.log.warning( f"Service {client.description} requested scopes {','.join(requested_scopes)}" f" for user {self.current_user.name}," f" granting only {','.join(allowed_scopes) or '[]'}." @@ -311,8 +320,14 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): self._complete_login(uri, headers, allowed_scopes, credentials) return - # resolve roles to scopes for authorization page - if not allowed_scopes: + # discard 'required' scopes from description + # no need to describe the ability to access itself + scopes_to_describe = allowed_scopes.difference(required_scopes) + + if not scopes_to_describe: + # TODO: describe all scopes? + # Not right now, because the no-scope default 'identify' text + # is clearer than what we produce for those scopes individually scope_descriptions = [ { "scope": None, @@ -322,8 +337,8 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): "filter": "", } ] - elif 'inherit' in allowed_scopes: - allowed_scopes = ['inherit'] + elif 'inherit' in scopes_to_describe: + allowed_scopes = scopes_to_describe = ['inherit'] scope_descriptions = [ { "scope": "inherit", @@ -335,7 +350,7 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): ] else: scope_descriptions = scopes.describe_raw_scopes( - allowed_scopes, + scopes_to_describe, username=self.current_user.name, ) # Render oauth 'Authorize application...' page diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 5252ea7d..8c7b4055 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -55,7 +55,6 @@ from traitlets import ( Bool, Any, Tuple, - Type, Set, Instance, Bytes, diff --git a/jupyterhub/oauth/provider.py b/jupyterhub/oauth/provider.py index e034b56a..267f3965 100644 --- a/jupyterhub/oauth/provider.py +++ b/jupyterhub/oauth/provider.py @@ -12,6 +12,8 @@ from tornado.log import app_log from .. import orm from ..roles import roles_to_scopes from ..scopes import _check_scopes_exist +from ..scopes import access_scopes +from ..scopes import identify_scopes from ..utils import compare_token from ..utils import hash_token @@ -154,7 +156,13 @@ class JupyterHubRequestValidator(RequestValidator): ) if orm_client is None: raise ValueError("No such client: %s" % client_id) - return roles_to_scopes(orm_client.allowed_roles) + scopes = roles_to_scopes(orm_client.allowed_roles) + if 'inherit' not in scopes: + # add identify-user scope + scopes.update(identify_scopes()) + # add access-service scope + scopes.update(access_scopes(orm_client)) + return scopes def get_original_scopes(self, refresh_token, request, *args, **kwargs): """Get the list of scopes associated with the refresh token. @@ -254,7 +262,7 @@ class JupyterHubRequestValidator(RequestValidator): code=code['code'], # oauth has 5 minutes to complete expires_at=int(orm.OAuthCode.now() + 300), - scopes=list(request._jupyterhub_scopes), + scopes=list(request.scopes), user=request.user.orm_user, redirect_uri=orm_client.redirect_uri, session_id=request.session_id, @@ -539,7 +547,7 @@ class JupyterHubRequestValidator(RequestValidator): def validate_scopes(self, client_id, scopes, client, request, *args, **kwargs): """Ensure the client is authorized access to requested scopes. :param client_id: Unicode client identifier - :param scopes: List of scopes (defined by you) + :param scopes: List of 'raw' scopes (defined by you) :param client: Client object set by you, see authenticate_client. :param request: The HTTP Request (oauthlib.common.Request) :rtype: True or False @@ -557,30 +565,50 @@ class JupyterHubRequestValidator(RequestValidator): app_log.warning("No such oauth client %s", client_id) return False + requested_scopes = set(scopes) # explicitly allow 'identify', which was the only allowed scope previously # requesting 'identify' gets no actual permissions other than self-identification - scopes = set(scopes) - scopes.discard("identify") + if "identify" in requested_scopes: + app_log.warning( + f"Ignoring deprecated 'identify' scope, requested by {client_id}" + ) + requested_scopes.discard("identify") # TODO: handle roles->scopes transition - # at this point, 'scopes' _may_ be roles + # In 2.0-2.2, `?scopes=` only accepted _role_ names, + # but in 2.3 we accept and prefer scopes. + # For backward-compatibility, we still accept both. + # Should roles be deprecated here, or kept as a convenience? try: - _check_scopes_exist(scopes) + _check_scopes_exist(requested_scopes) except KeyError as e: # scopes don't exist, maybe they are role names requested_roles = list( - self.db.query(orm.Role).filter(orm.Role.name.in_(scopes)) + self.db.query(orm.Role).filter(orm.Role.name.in_(requested_scopes)) ) - if len(requested_roles) != len(scopes): + if len(requested_roles) != len(requested_scopes): # did not find roles - app_log.warning(f"No such scopes: {scopes}") + app_log.warning(f"No such scopes: {requested_scopes}") return False - app_log.info(f"OAuth client {client_id} requesting roles: {scopes}") - scopes = roles_to_scopes(requested_roles) + app_log.info( + f"OAuth client {client_id} requesting roles: {requested_scopes}" + ) + requested_scopes = roles_to_scopes(requested_roles) client_allowed_scopes = roles_to_scopes(orm_client.allowed_roles) - requested_scopes = set(scopes) + # always grant reading the token-owner's name + # and accessing the service itself + required_scopes = {*identify_scopes(), *access_scopes(orm_client)} + requested_scopes.update(required_scopes) + client_allowed_scopes.update(required_scopes) + + # TODO: handle expanded_scopes intersection here? + # e.g. client allowed to request admin:users, + # but requests admin:users!name=x will not be allowed + # This can probably be dealt with in config by listing expected requests + # as explcitly allowed + disallowed_scopes = requested_scopes.difference(client_allowed_scopes) if disallowed_scopes: app_log.error( @@ -588,12 +616,11 @@ class JupyterHubRequestValidator(RequestValidator): ) return False - # store resolved roles on request + # store resolved scopes on request app_log.debug( f"Allowing request for scope(s) for {client_id}: {','.join(requested_scopes) or '[]'}" ) - # these will be stored on the OAuthCode object - request._jupyterhub_scopes = requested_scopes + request.scopes = requested_scopes return True diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 8a44e16e..0021bb75 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -3,11 +3,12 @@ General scope definitions and utilities Scope variable nomenclature --------------------------- -scopes: list of scopes with abbreviations (e.g., in role definition) -expanded scopes: set of expanded scopes without abbreviations (i.e., resolved metascopes, filters and subscopes) -parsed scopes: dictionary JSON like format of expanded scopes +scopes or 'raw' scopes: collection of scopes that may contain abbreviations (e.g., in role definition) +expanded scopes: set of expanded scopes without abbreviations (i.e., resolved metascopes, filters, and subscopes) +parsed scopes: dictionary format of expanded scopes (`read:users!user=name` -> `{'read:users': {user: [name]}`) intersection : set of expanded scopes as intersection of 2 expanded scope sets identify scopes: set of expanded scopes needed for identify (whoami) endpoints +reduced scopes: expanded scopes that have been reduced """ import functools import inspect @@ -300,37 +301,30 @@ def get_scopes_for(orm_object): f"Only allow orm objects or User wrappers, got {orm_object}" ) + owner = None if isinstance(orm_object, orm.APIToken): owner = orm_object.user or orm_object.service - token_scopes = expand_scopes(orm_object.scopes, owner=owner) - if orm_object.client_id != "jupyterhub": - # oauth tokens can be used to access the service issuing the token, - # assuming the owner itself still has permission to do so - spawner = orm_object.oauth_client.spawner - if spawner: - token_scopes.add( - f"access:servers!server={spawner.user.name}/{spawner.name}" - ) - else: - service = orm_object.oauth_client.service - if service: - token_scopes.add(f"access:services!service={service.name}") - else: - app_log.warning( - f"Token {orm_object} has no associated service or spawner!" - ) - owner_roles = roles.get_roles_for(owner) owner_scopes = roles.roles_to_expanded_scopes(owner_roles, owner) - if token_scopes == {'inherit'}: - # token_scopes is only 'inherit', return scopes inherited from owner as-is - # short-circuit common case where we don't need to compute an intersection + token_scopes = set(orm_object.scopes) + if 'inherit' in token_scopes: + # token_scopes includes 'inherit', + # so we know the intersection is exactly the owner's scopes + # only thing we miss by short-circuiting here: warning about excluded extra scopes return owner_scopes - if 'inherit' in token_scopes: - token_scopes.remove('inherit') - token_scopes |= owner_scopes + token_scopes = expand_scopes(token_scopes, owner=owner) + + if orm_object.client_id != "jupyterhub": + # oauth tokens can be used to access the service issuing the token, + # assuming the owner itself still has permission to do so + access = access_scopes(orm_object.oauth_client) + token_scopes.update(access) + + # reduce to collapse multiple filters on the same scope + # to avoid spurious logs about discarded scopes + token_scopes = reduce_scopes(token_scopes) intersection = _intersect_expanded_scopes( token_scopes, @@ -342,8 +336,14 @@ def get_scopes_for(orm_object): # Not taking symmetric difference here because token owner can naturally have more scopes than token if discarded_token_scopes: app_log.warning( - "discarding scopes [%s], not present in owner roles" - % ", ".join(discarded_token_scopes) + f"discarding scopes [{discarded_token_scopes}]," + f" not present in roles of owner {owner}" + ) + app_log.debug( + "Owner %s has scopes: %s\nToken has scopes: %s", + owner, + owner_scopes, + token_scopes, ) expanded_scopes = intersection else: @@ -351,6 +351,12 @@ def get_scopes_for(orm_object): roles.get_roles_for(orm_object), owner=orm_object, ) + if isinstance(orm_object, (orm.User, orm.Service)): + owner = orm_object + + # always include identify scopes + if owner: + expanded_scopes.update(identify_scopes(owner)) return expanded_scopes @@ -473,7 +479,8 @@ def expand_scopes(scopes, owner=None): stacklevel=2, ) - return expanded_scopes + # reduce to minimize + return reduce_scopes(expanded_scopes) def _needs_scope_expansion(filter_, filter_value, sub_scope): @@ -658,6 +665,14 @@ def unparse_scopes(parsed_scopes): return expanded_scopes +def reduce_scopes(expanded_scopes): + """Reduce expanded scopes to minimal set + + Eliminates redundancy, such as access:services and access:services!service=x + """ + return unparse_scopes(parse_scopes(expanded_scopes)) + + def needs_scope(*scopes): """Decorator to restrict access to users or services with the required scope""" @@ -708,16 +723,20 @@ def needs_scope(*scopes): return scope_decorator -def identify_scopes(obj): +def identify_scopes(obj=None): """Return 'identify' scopes for an orm object Arguments: - obj: orm.User or orm.Service + obj (optional): orm.User or orm.Service + If not specified, 'raw' scopes for identifying the current user are returned, + which may need to be expanded, later. Returns: identify scopes (set): set of scopes needed for 'identify' endpoints """ - if isinstance(obj, orm.User): + if obj is None: + return {f"read:users:{field}!user" for field in {"name", "groups"}} + elif isinstance(obj, orm.User): return {f"read:users:{field}!user={obj.name}" for field in {"name", "groups"}} elif isinstance(obj, orm.Service): return {f"read:services:{field}!service={obj.name}" for field in {"name"}} @@ -725,6 +744,25 @@ def identify_scopes(obj): raise TypeError(f"Expected orm.User or orm.Service, got {obj!r}") +def access_scopes(oauth_client): + """Return scope(s) required to access an oauth client""" + scopes = set() + if oauth_client.identifier == "jupyterhub": + return scopes + spawner = oauth_client.spawner + if spawner: + scopes.add(f"access:servers!server={spawner.user.name}/{spawner.name}") + else: + service = oauth_client.service + if service: + scopes.add(f"access:services!service={service.name}") + else: + app_log.warning( + f"OAuth client {oauth_client} has no associated service or spawner!" + ) + return scopes + + def check_scope_filter(sub_scope, orm_resource, kind): """Return whether a sub_scope filter applies to a given resource. diff --git a/jupyterhub/tests/mockservice.py b/jupyterhub/tests/mockservice.py index 92d6501e..b50a6c9b 100644 --- a/jupyterhub/tests/mockservice.py +++ b/jupyterhub/tests/mockservice.py @@ -68,7 +68,7 @@ class WhoAmIHandler(HubAuthenticated, web.RequestHandler): @web.authenticated def get(self): - self.write(self.get_current_user()) + self.write(json.dumps(self.get_current_user())) class OWhoAmIHandler(HubOAuthenticated, web.RequestHandler): @@ -86,7 +86,7 @@ class OWhoAmIHandler(HubOAuthenticated, web.RequestHandler): @web.authenticated def get(self): - self.write(self.get_current_user()) + self.write(json.dumps(self.get_current_user())) def main(): diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 81c159a9..2ef1e69b 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -10,7 +10,6 @@ from tornado.log import app_log from .. import orm from .. import roles -from ..scopes import _expand_self_scope from ..scopes import get_scopes_for from ..scopes import scope_definitions from ..utils import utcnow @@ -810,19 +809,6 @@ async def test_user_filter_expansion(app, scope_list, kind, test_for_token): app.db.delete(test_role) -async def test_large_filter_expansion(app, create_temp_role, create_user_with_scopes): - scope_list = _expand_self_scope('==') - # Mimic the role 'self' based on '!user' filter for tokens - scope_list = [scope.rstrip("=") for scope in scope_list] - filtered_role = create_temp_role(scope_list) - user = create_user_with_scopes('self') - user.new_api_token(roles=[filtered_role.name]) - user.new_api_token(roles=['token']) - manual_scope_set = get_scopes_for(user.api_tokens[0]) - auto_scope_set = get_scopes_for(user.api_tokens[1]) - assert manual_scope_set == auto_scope_set - - @mark.role @mark.parametrize( "name, valid", diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 946821e6..caac1b2c 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -12,7 +12,9 @@ from .. import roles from .. import scopes from ..handlers import BaseHandler from ..scopes import _check_scope_access +from ..scopes import _expand_self_scope from ..scopes import _intersect_expanded_scopes +from ..scopes import expand_scopes from ..scopes import get_scopes_for from ..scopes import needs_scope from ..scopes import parse_scopes @@ -290,7 +292,7 @@ async def test_exceeding_user_permissions( api_token = user.new_api_token() orm_api_token = orm.APIToken.find(app.db, token=api_token) # store scopes user does not have - orm_api_token.scopes = orm_api_token.scopes + ['list:users', 'read:users'] + orm_api_token.update_scopes(orm_api_token.scopes + ['list:users', 'read:users']) headers = {'Authorization': 'token %s' % api_token} r = await api_request(app, 'users', headers=headers) assert r.status_code == 200 @@ -1127,3 +1129,45 @@ def test_custom_scopes_bad(preserve_scopes, custom_scopes): with pytest.raises(ValueError): scopes.define_custom_scopes(custom_scopes) assert scopes.scope_definitions == preserve_scopes + + +async def test_user_filter_expansion(app, create_user_with_scopes): + scope_list = _expand_self_scope('ignored') + # turn !user=ignored into !user + # Mimic the role 'self' based on '!user' filter for tokens + scope_list = [scope.partition("=")[0] for scope in scope_list] + user = create_user_with_scopes('self') + user.new_api_token(scopes=scope_list) + user.new_api_token() + manual_scope_set = get_scopes_for(user.api_tokens[0]) + auto_scope_set = get_scopes_for(user.api_tokens[1]) + assert manual_scope_set == auto_scope_set + + +@pytest.mark.parametrize( + "scopes, expected", + [ + ("read:users:name!user", ["read:users:name!user=$user"]), + ( + "users:activity!user", + [ + "read:users:activity!user=$user", + "users:activity!user=$user", + ], + ), + ("self", ["*"]), + (["access:services", "access:services!service=x"], ["access:services"]), + ], +) +def test_expand_scopes(user, scopes, expected): + 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} + + if "*" in expected: + expected.remove("*") + expected.update(_expand_self_scope(user.name)) + + expanded = expand_scopes(scopes, owner=user.orm_user) + assert sorted(expanded) == sorted(expected) diff --git a/jupyterhub/tests/test_services_auth.py b/jupyterhub/tests/test_services_auth.py index e3fde251..0eb8c783 100644 --- a/jupyterhub/tests/test_services_auth.py +++ b/jupyterhub/tests/test_services_auth.py @@ -12,6 +12,7 @@ import pytest from bs4 import BeautifulSoup from pytest import raises from tornado.httputil import url_concat +from tornado.log import app_log from .. import orm from .. import roles @@ -206,32 +207,50 @@ async def test_hubauth_service_token(request, app, mockservice_url, scopes, allo @pytest.mark.parametrize( - "client_allowed_roles, request_roles, expected_roles", + "client_allowed_roles, request_scopes, expected_scopes", [ - # allow empty roles + # allow empty permissions ([], [], []), # allow original 'identify' scope to map to no role ([], ["identify"], []), # requesting roles outside client list doesn't work ([], ["admin"], None), - ([], ["token"], None), - # requesting nonexistent roles fails in the same way (no server error) - ([], ["nosuchrole"], None), - # requesting exactly client allow list works + ([], ["read:users"], None), + # requesting nonexistent roles or scopes fails in the same way (no server error) + ([], ["nosuchscope"], None), + ([], ["admin:invalid!no=bo!"], None), + # requesting role exactly client allow list works (["user"], ["user"], ["user"]), + # Request individual scope, held by user, not listed in allowed role # no explicit request, defaults to all (["token", "user"], [], ["token", "user"]), - # explicit 'identify' maps to none - (["token", "user"], ["identify"], []), + # explicit 'identify' maps to read:users:name!user + (["token", "user"], ["identify"], ["read:users:name!user=$user"]), # any item outside the list isn't allowed (["token", "user"], ["token", "server"], None), + (["read-only"], ["access:services"], None), # requesting subset (["admin", "user"], ["user"], ["user"]), (["user", "token", "server"], ["token", "user"], ["token", "user"]), (["admin", "user", "read-only"], ["read-only"], ["read-only"]), + # Request individual scopes, listed in allowed role + (["read-only"], ["access:servers"], ["access:servers"]), # requesting valid subset, some not held by user - (["admin", "user"], ["admin", "user"], ["user"]), - (["admin", "user"], ["admin"], []), + ( + ["admin", "user"], + ["admin:users", "access:servers", "self"], + ["access:servers", "user"], + ), + (["other"], ["other"], []), + # custom scopes + (["user"], ["custom:jupyter_server:read:*"], None), + ( + ["read-only"], + ["custom:jupyter_server:read:*"], + ["custom:jupyter_server:read:*"], + ), + # this one _should_ work, but doesn't until we implement expanded_scope filtering + # (["read-only"], ["custom:jupyter_server:read:*!user=$user"], ["custom:jupyter_server:read:*!user=$user"]), ], ) async def test_oauth_service_roles( @@ -239,8 +258,8 @@ async def test_oauth_service_roles( mockservice_url, create_user_with_scopes, client_allowed_roles, - request_roles, - expected_roles, + request_scopes, + expected_scopes, preserve_scopes, ): service = mockservice_url @@ -267,13 +286,24 @@ async def test_oauth_service_roles( ], }, ) + + roles.create_role( + app.db, + { + "name": "other", + "description": "A role not held by our test user", + "scopes": [ + "admin:users", + ], + }, + ) oauth_client.allowed_roles = [ orm.Role.find(app.db, role_name) for role_name in client_allowed_roles ] app.db.commit() url = url_path_join(public_url(app, mockservice_url) + 'owhoami/?arg=x') - if request_roles: - url = url_concat(url, {"request-scope": " ".join(request_roles)}) + if request_scopes: + url = url_concat(url, {"request-scope": " ".join(request_scopes)}) # first request is only going to login and get us to the oauth form page s = AsyncSession() user = create_user_with_scopes("access:services") @@ -283,7 +313,7 @@ async def test_oauth_service_roles( s.cookies = await app.login_user(name) r = await s.get(url) - if expected_roles is None: + if expected_scopes is None: # expected failed auth, stop here # verify expected 'invalid scope' error, not server error dest_url, _, query = r.url.partition("?") @@ -291,6 +321,7 @@ async def test_oauth_service_roles( assert parse_qs(query).get("error") == ["invalid_scope"] assert r.status_code == 400 return + r.raise_for_status() # we should be looking at the oauth confirmation page assert urlparse(r.url).path == app.base_url + 'hub/api/oauth2/authorize' @@ -300,7 +331,7 @@ async def test_oauth_service_roles( page = BeautifulSoup(r.text, "html.parser") scope_inputs = page.find_all("input", {"name": "scopes"}) scope_values = [input["value"] for input in scope_inputs] - print("Submitting request with scope values", scope_values) + app_log.info(f"Submitting request with scope values {scope_values}") # submit the oauth form to complete authorization data = {} if scope_values: @@ -317,10 +348,35 @@ async def test_oauth_service_roles( r = await s.get(url, allow_redirects=False) r.raise_for_status() assert r.status_code == 200 + assert len(r.history) == 0 reply = r.json() sub_reply = {key: reply.get(key, 'missing') for key in ('kind', 'name')} assert sub_reply == {'name': user.name, 'kind': 'user'} + expected_scopes = {s.replace("$user", user.name) for s in expected_scopes} + + # expand roles to scopes (shortcut) + for scope in list(expected_scopes): + role = orm.Role.find(app.db, scope) + if role: + expected_scopes.discard(role.name) + expected_scopes.update( + roles.roles_to_expanded_scopes([role], owner=user.orm_user) + ) + + if 'inherit' in expected_scopes: + expected_scopes = scopes.get_scopes_for(user.orm_user) + + # always expect identify/access scopes + # on successful authentication + expected_scopes.update(scopes.identify_scopes(user.orm_user)) + expected_scopes.update(scopes.access_scopes(oauth_client)) + expected_scopes = scopes.reduce_scopes(expected_scopes) + have_scopes = scopes.reduce_scopes(set(reply['scopes'])) + # pytest is better at reporting list differences + # than set differences, especially with `-vv` + assert sorted(have_scopes) == sorted(expected_scopes) + # token-authenticated request to HubOAuth token = app.users[name].new_api_token() # token in ?token parameter @@ -428,18 +484,23 @@ async def test_oauth_page_hit( user = create_user_with_scopes("access:services", "self") for role in test_roles.values(): roles.grant_role(app.db, user, role) - token_scopes = roles.roles_to_scopes([test_roles[t] for t in token_roles]) - user.new_api_token(scopes=token_scopes) - token = user.api_tokens[0] + # Create a token with the prior authorization oauth_client = ( app.db.query(orm.OAuthClient) .filter_by(identifier=service.oauth_client_id) .one() ) oauth_client.allowed_roles = list(test_roles.values()) + + authorized_scopes = roles.roles_to_scopes([test_roles[t] for t in token_roles]) + authorized_scopes.update(scopes.identify_scopes()) + authorized_scopes.update(scopes.access_scopes(oauth_client)) + user.new_api_token(scopes=authorized_scopes) + token = user.api_tokens[0] token.client_id = service.oauth_client_id app.db.commit() + s = AsyncSession() s.cookies = await app.login_user(user.name) url = url_path_join(public_url(app, service) + 'owhoami/?arg=x') diff --git a/share/jupyterhub/templates/oauth.html b/share/jupyterhub/templates/oauth.html index c726388b..95f73f53 100644 --- a/share/jupyterhub/templates/oauth.html +++ b/share/jupyterhub/templates/oauth.html @@ -14,7 +14,7 @@

{{ oauth_client.description }} (oauth URL: {{ oauth_client.redirect_uri }}) would like permission to identify you. - {% if not role_names %} + {% if scope_descriptions | length == 1 and not scope_descriptions[0].scope %} It will not be able to take actions on your behalf. {% endif %} @@ -24,8 +24,8 @@

{# these are the 'real' inputs to the form -#} - {% for role_name in role_names %} - + {% for scope in allowed_scopes %} + {% endfor %} {% for scope_info in scope_descriptions %}