From 307684592711a7e5ca997707a2d83295057077e2 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 11 Mar 2022 10:56:56 +0100 Subject: [PATCH 1/7] tokens have scopes instead of roles, which allow tokens to change permissions over time This is mostly a low-level change, with little outward-facing effects. - on upgrade, evaluate all token role assignments to their current scopes, and store those scopes on the tokens - assigning roles to tokens still works, but scopes are evaluated and validated immediately, rather than lazily stored as roles - no longer need to check for role permission changes on startup, because token permissions aren't affected - move a few scope utilities from roles to scopes - oauth allows specifying scopes, not just roles. But these are still at the level specified in roles, not fully-resolved scopes. - more granular APIs for working with scopes and roles --- ci/init-db.sh | 2 +- jupyterhub/alembic/env.py | 28 +- .../versions/651f5419b74d_api_token_scopes.py | 82 ++++++ jupyterhub/apihandlers/auth.py | 65 ++--- jupyterhub/apihandlers/base.py | 5 +- jupyterhub/apihandlers/users.py | 15 +- jupyterhub/app.py | 20 -- jupyterhub/oauth/provider.py | 52 ++-- jupyterhub/orm.py | 81 ++++-- jupyterhub/roles.py | 274 +++--------------- jupyterhub/scopes.py | 201 ++++++++++++- jupyterhub/tests/conftest.py | 1 - jupyterhub/tests/populate_db.py | 31 ++ jupyterhub/tests/test_db.py | 13 +- jupyterhub/tests/test_roles.py | 121 ++------ jupyterhub/tests/test_scopes.py | 18 +- jupyterhub/tests/test_services.py | 2 - jupyterhub/tests/test_services_auth.py | 4 +- 18 files changed, 550 insertions(+), 465 deletions(-) create mode 100644 jupyterhub/alembic/versions/651f5419b74d_api_token_scopes.py diff --git a/ci/init-db.sh b/ci/init-db.sh index 32a73d29..46672cf6 100755 --- a/ci/init-db.sh +++ b/ci/init-db.sh @@ -20,7 +20,7 @@ fi # Configure a set of databases in the database server for upgrade tests set -x -for SUFFIX in '' _upgrade_100 _upgrade_122 _upgrade_130; do +for SUFFIX in '' _upgrade_100 _upgrade_122 _upgrade_130 _upgrade_150 _upgrade_211; do $SQL_CLIENT "DROP DATABASE jupyterhub${SUFFIX};" 2>/dev/null || true $SQL_CLIENT "CREATE DATABASE jupyterhub${SUFFIX} ${EXTRA_CREATE_DATABASE_ARGS:-};" done diff --git a/jupyterhub/alembic/env.py b/jupyterhub/alembic/env.py index bf944f79..2d135290 100644 --- a/jupyterhub/alembic/env.py +++ b/jupyterhub/alembic/env.py @@ -9,7 +9,6 @@ from sqlalchemy import pool # this is the Alembic Config object, which provides # access to the values within the .ini file in use. config = context.config - # Interpret the config file for Python logging. # This line sets up loggers basically. if 'jupyterhub' in sys.modules: @@ -42,6 +41,16 @@ target_metadata = orm.Base.metadata # my_important_option = config.get_main_option("my_important_option") # ... etc. +# pass these to context.configure(**config_opts) +common_config_opts = dict( + # target_metadata for autogenerate + target_metadata=target_metadata, + # transaction per migration to ensure + # each migration is 'complete' before running the next one + # (e.g. dropped tables) + transaction_per_migration=True, +) + def run_migrations_offline(): """Run migrations in 'offline' mode. @@ -56,14 +65,14 @@ def run_migrations_offline(): """ connectable = config.attributes.get('connection', None) + config_opts = dict(literal_binds=True) + config_opts.update(common_config_opts) if connectable is None: - url = config.get_main_option("sqlalchemy.url") - context.configure(url=url, target_metadata=target_metadata, literal_binds=True) + config_opts["url"] = config.get_main_option("sqlalchemy.url") else: - context.configure( - connection=connectable, target_metadata=target_metadata, literal_binds=True - ) + config_opts["connection"] = connectable + context.configure(**config_opts) with context.begin_transaction(): context.run_migrations() @@ -77,6 +86,8 @@ def run_migrations_online(): """ connectable = config.attributes.get('connection', None) + config_opts = {} + config_opts.update(common_config_opts) if connectable is None: connectable = engine_from_config( @@ -86,7 +97,10 @@ def run_migrations_online(): ) with connectable.connect() as connection: - context.configure(connection=connection, target_metadata=target_metadata) + context.configure( + connection=connection, + **common_config_opts, + ) with context.begin_transaction(): context.run_migrations() diff --git a/jupyterhub/alembic/versions/651f5419b74d_api_token_scopes.py b/jupyterhub/alembic/versions/651f5419b74d_api_token_scopes.py new file mode 100644 index 00000000..bda81a2b --- /dev/null +++ b/jupyterhub/alembic/versions/651f5419b74d_api_token_scopes.py @@ -0,0 +1,82 @@ +"""api_token_scopes + +Revision ID: 651f5419b74d +Revises: 833da8570507 +Create Date: 2022-02-28 12:42:55.149046 + +""" +# revision identifiers, used by Alembic. +revision = '651f5419b74d' +down_revision = '833da8570507' +branch_labels = None +depends_on = None + +import sqlalchemy as sa +from alembic import op +from sqlalchemy import Column +from sqlalchemy import ForeignKey +from sqlalchemy import Integer +from sqlalchemy import Table +from sqlalchemy import Unicode +from sqlalchemy.ext.declarative import declarative_base +from sqlalchemy.orm import relationship +from sqlalchemy.orm.session import Session + +from jupyterhub import orm +from jupyterhub import roles + + +def upgrade(): + c = op.get_bind() + + tables = sa.inspect(c.engine).get_table_names() + + # oauth codes are short lived, no need to upgrade them + if 'oauth_code_role_map' in tables: + op.drop_table('oauth_code_role_map') + + if 'oauth_codes' in tables: + op.add_column('oauth_codes', sa.Column('scopes', orm.JSONList(), nullable=True)) + + if 'api_tokens' not in tables: + # e.g. upgrade from 1.x, token table dropped + # no migration to do + return + + # define new scopes column on API tokens + op.add_column('api_tokens', sa.Column('scopes', orm.JSONList(), nullable=True)) + + if 'api_token_role_map' in tables: + # redefine the to-be-removed api_token->role relationship + # so we can run a query on it for the migration + token_role_map = Table( + "api_token_role_map", + orm.Base.metadata, + Column( + 'api_token_id', + ForeignKey('api_tokens.id', ondelete='CASCADE'), + primary_key=True, + ), + Column( + 'role_id', + ForeignKey('roles.id', ondelete='CASCADE'), + primary_key=True, + ), + extend_existing=True, + ) + orm.APIToken.roles = relationship('Role', secondary='api_token_role_map') + + # tokens have roles, evaluate to scopes + db = Session(bind=c) + for token in db.query(orm.APIToken): + token.scopes = list(roles.roles_to_scopes(token.roles)) + db.commit() + # drop token-role relationship + op.drop_table('api_token_role_map') + + +def downgrade(): + # cannot map permissions from scopes back to roles + # drop whole api token table (revokes all tokens), which will be recreated on hub start + op.drop_table('api_tokens') + op.drop_table('oauth_codes') diff --git a/jupyterhub/apihandlers/auth.py b/jupyterhub/apihandlers/auth.py index 9d05f1cb..cd8b1947 100644 --- a/jupyterhub/apihandlers/auth.py +++ b/jupyterhub/apihandlers/auth.py @@ -180,7 +180,7 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): raise self.send_oauth_response(headers, body, status) - def needs_oauth_confirm(self, user, oauth_client, roles): + def needs_oauth_confirm(self, user, oauth_client, requested_scopes): """Return whether the given oauth client needs to prompt for access for the given user Checks list for oauth clients that don't need confirmation @@ -211,20 +211,20 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): user_id=user.id, client_id=oauth_client.identifier, ) - authorized_roles = set() + authorized_scopes = set() for token in existing_tokens: - authorized_roles.update({role.name for role in token.roles}) + authorized_scopes.update(token.scopes) - if authorized_roles: - if set(roles).issubset(authorized_roles): + if authorized_scopes: + if set(requested_scopes).issubset(authorized_scopes): self.log.debug( - f"User {user.name} has already authorized {oauth_client.identifier} for roles {roles}" + f"User {user.name} has already authorized {oauth_client.identifier} for scopes {requested_scopes}" ) return False else: self.log.debug( f"User {user.name} has authorized {oauth_client.identifier}" - f" for roles {authorized_roles}, confirming additonal roles {roles}" + f" for scopes {authorized_scopes}, confirming additonal scopes {requested_scopes}" ) # default: require confirmation return True @@ -251,7 +251,7 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): uri, http_method, body, headers = self.extract_oauth_params() try: ( - role_names, + requested_scopes, credentials, ) = self.oauth_provider.validate_authorization_request( uri, http_method, body, headers @@ -284,40 +284,35 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): 403, f"You do not have permission to access {client.description}" ) - # subset role names to those held by authenticating user - requested_role_names = set(role_names) + # subset 'raw scopes' to those held by authenticating user + requested_scopes = set(requested_scopes) user = self.current_user - user_role_names = {role.name for role in user.roles} - allowed_role_names = requested_role_names.intersection(user_role_names) - excluded_role_names = requested_role_names.difference(allowed_role_names) - if excluded_role_names: - self.log.info( - f"Service {client.description} requested roles {','.join(role_names)}" - f" for user {self.current_user.name}," - f" granting only {','.join(allowed_role_names) or '[]'}." - ) - role_names = list(allowed_role_names) + # raw, _not_ expanded scopes + user_scopes = roles.roles_to_scopes(roles.get_roles_for(user.orm_user)) + allowed_scopes = requested_scopes.intersection(user_scopes) + excluded_scopes = requested_scopes.difference(user_scopes) + # TODO: compute lower-level intersection + # of _expanded_ scopes - if not self.needs_oauth_confirm(self.current_user, client, role_names): + if excluded_scopes: + self.log.info( + f"Service {client.description} requested scopes {','.join(requested_scopes)}" + f" for user {self.current_user.name}," + f" granting only {','.join(allowed_scopes) or '[]'}." + ) + + if not self.needs_oauth_confirm(self.current_user, client, allowed_scopes): self.log.debug( "Skipping oauth confirmation for %s accessing %s", self.current_user, client.description, ) # this is the pre-1.0 behavior for all oauth - self._complete_login(uri, headers, role_names, credentials) + self._complete_login(uri, headers, allowed_scopes, credentials) return # resolve roles to scopes for authorization page - raw_scopes = set() - if role_names: - role_objects = ( - self.db.query(orm.Role).filter(orm.Role.name.in_(role_names)).all() - ) - raw_scopes = set( - itertools.chain(*(role.scopes for role in role_objects)) - ) - if not raw_scopes: + if not allowed_scopes: scope_descriptions = [ { "scope": None, @@ -327,8 +322,8 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): "filter": "", } ] - elif 'inherit' in raw_scopes: - raw_scopes = ['inherit'] + elif 'inherit' in allowed_scopes: + allowed_scopes = ['inherit'] scope_descriptions = [ { "scope": "inherit", @@ -340,7 +335,7 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): ] else: scope_descriptions = scopes.describe_raw_scopes( - raw_scopes, + allowed_scopes, username=self.current_user.name, ) # Render oauth 'Authorize application...' page @@ -349,7 +344,7 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): await self.render_template( "oauth.html", auth_state=auth_state, - role_names=role_names, + allowed_scopes=allowed_scopes, scope_descriptions=scope_descriptions, oauth_client=client, ) diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py index 6cbb7659..d46eedff 100644 --- a/jupyterhub/apihandlers/base.py +++ b/jupyterhub/apihandlers/base.py @@ -14,6 +14,7 @@ from tornado import web from .. import orm from ..handlers import BaseHandler +from ..scopes import get_scopes_for from ..utils import get_browser_protocol from ..utils import isoformat from ..utils import url_path_join @@ -224,7 +225,9 @@ class APIHandler(BaseHandler): owner_key: owner, 'id': token.api_id, 'kind': 'api_token', - 'roles': [r.name for r in token.roles], + # deprecated field, but leave it present. + 'roles': [], + 'scopes': list(get_scopes_for(token)), 'created': isoformat(token.created), 'last_activity': isoformat(token.last_activity), 'expires_at': isoformat(token.expires_at), diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 109ad32a..9b0bd34a 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -406,21 +406,18 @@ class UserTokenListAPIHandler(APIHandler): if requester is not user: note += f" by {kind} {requester.name}" - token_roles = body.get('roles') + token_roles = body.get("roles") + token_scopes = body.get("scopes") + try: api_token = user.new_api_token( note=note, expires_in=body.get('expires_in', None), roles=token_roles, + scopes=token_scopes, ) - except KeyError: - raise web.HTTPError(404, "Requested roles %r not found" % token_roles) - except ValueError: - raise web.HTTPError( - 403, - "Requested roles %r cannot have higher permissions than the token owner" - % token_roles, - ) + except ValueError as e: + raise web.HTTPError(400, str(e)) if requester is not user: self.log.info( "%s %s requested API token for %s", diff --git a/jupyterhub/app.py b/jupyterhub/app.py index ddb9649a..5252ea7d 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -2079,23 +2079,6 @@ class JupyterHub(Application): # make sure we load any default roles not overridden init_roles = list(default_roles_dict.values()) + init_roles - if roles_with_new_permissions: - unauthorized_oauth_tokens = ( - self.db.query(orm.APIToken) - .filter( - orm.APIToken.roles.any( - orm.Role.name.in_(roles_with_new_permissions) - ) - ) - .filter(orm.APIToken.client_id != 'jupyterhub') - ) - for token in unauthorized_oauth_tokens: - self.log.warning( - "Deleting OAuth token %s; one of its roles obtained new permissions that were not authorized by user" - % token - ) - self.db.delete(token) - self.db.commit() init_role_names = [r['name'] for r in init_roles] if ( @@ -2231,9 +2214,6 @@ class JupyterHub(Application): for kind in kinds: roles.check_for_default_roles(db, kind) - # check tokens for default roles - roles.check_for_default_roles(db, bearer='tokens') - async def _add_tokens(self, token_dict, kind): """Add tokens for users or services to the database""" if kind == 'user': diff --git a/jupyterhub/oauth/provider.py b/jupyterhub/oauth/provider.py index 7594131a..e034b56a 100644 --- a/jupyterhub/oauth/provider.py +++ b/jupyterhub/oauth/provider.py @@ -10,6 +10,8 @@ from oauthlib.oauth2.rfc6749.grant_types import base from tornado.log import app_log from .. import orm +from ..roles import roles_to_scopes +from ..scopes import _check_scopes_exist from ..utils import compare_token from ..utils import hash_token @@ -152,7 +154,7 @@ class JupyterHubRequestValidator(RequestValidator): ) if orm_client is None: raise ValueError("No such client: %s" % client_id) - return [role.name for role in orm_client.allowed_roles] + return roles_to_scopes(orm_client.allowed_roles) def get_original_scopes(self, refresh_token, request, *args, **kwargs): """Get the list of scopes associated with the refresh token. @@ -252,7 +254,7 @@ class JupyterHubRequestValidator(RequestValidator): code=code['code'], # oauth has 5 minutes to complete expires_at=int(orm.OAuthCode.now() + 300), - roles=request._jupyterhub_roles, + scopes=list(request._jupyterhub_scopes), user=request.user.orm_user, redirect_uri=orm_client.redirect_uri, session_id=request.session_id, @@ -349,7 +351,7 @@ class JupyterHubRequestValidator(RequestValidator): orm.APIToken.new( client_id=client.identifier, expires_in=token['expires_in'], - roles=[rolename for rolename in request.scopes], + scopes=request.scopes, token=token['access_token'], session_id=request.session_id, user=request.user, @@ -451,7 +453,7 @@ class JupyterHubRequestValidator(RequestValidator): return False request.user = orm_code.user request.session_id = orm_code.session_id - request.scopes = [role.name for role in orm_code.roles] + request.scopes = orm_code.scopes return True def validate_grant_type( @@ -547,35 +549,51 @@ class JupyterHubRequestValidator(RequestValidator): - Resource Owner Password Credentials Grant - Client Credentials Grant """ + orm_client = ( self.db.query(orm.OAuthClient).filter_by(identifier=client_id).one_or_none() ) if orm_client is None: app_log.warning("No such oauth client %s", client_id) return False - client_allowed_roles = {role.name: role for role in orm_client.allowed_roles} + # explicitly allow 'identify', which was the only allowed scope previously # requesting 'identify' gets no actual permissions other than self-identification - client_allowed_roles.setdefault('identify', None) - roles = [] - requested_roles = set(scopes) - disallowed_roles = requested_roles.difference(client_allowed_roles) - if disallowed_roles: + scopes = set(scopes) + scopes.discard("identify") + + # TODO: handle roles->scopes transition + # at this point, 'scopes' _may_ be roles + try: + _check_scopes_exist(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)) + ) + if len(requested_roles) != len(scopes): + # did not find roles + app_log.warning(f"No such scopes: {scopes}") + return False + app_log.info(f"OAuth client {client_id} requesting roles: {scopes}") + scopes = roles_to_scopes(requested_roles) + + client_allowed_scopes = roles_to_scopes(orm_client.allowed_roles) + + requested_scopes = set(scopes) + disallowed_scopes = requested_scopes.difference(client_allowed_scopes) + if disallowed_scopes: app_log.error( - f"Role(s) not allowed for {client_id}: {','.join(disallowed_roles)}" + f"Scope(s) not allowed for {client_id}: {', '.join(disallowed_scopes)}" ) return False # store resolved roles on request app_log.debug( - f"Allowing request for role(s) for {client_id}: {','.join(requested_roles) or '[]'}" + f"Allowing request for scope(s) for {client_id}: {','.join(requested_scopes) or '[]'}" ) # these will be stored on the OAuthCode object - request._jupyterhub_roles = [ - client_allowed_roles[name] - for name in requested_roles - if client_allowed_roles[name] is not None - ] + request._jupyterhub_scopes = requested_scopes return True diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index c368bce4..39e7984b 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -3,7 +3,6 @@ # Distributed under the terms of the Modified BSD License. import enum import json -import warnings from base64 import decodebytes from base64 import encodebytes from datetime import datetime @@ -40,10 +39,7 @@ from sqlalchemy.types import Text from sqlalchemy.types import TypeDecorator from tornado.log import app_log -from .roles import assign_default_roles -from .roles import create_role -from .roles import get_default_roles -from .roles import update_roles +from .roles import roles_to_scopes from .utils import compare_token from .utils import hash_token from .utils import new_token @@ -110,7 +106,9 @@ class JSONList(JSONDict): return value def process_result_value(self, value, dialect): - if value is not None: + if value is None: + return [] + else: value = json.loads(value) return value @@ -157,9 +155,7 @@ for has_role in ( 'user', 'group', 'service', - 'api_token', 'oauth_client', - 'oauth_code', ): role_map = Table( f'{has_role}_role_map', @@ -185,10 +181,9 @@ class Role(Base): id = Column(Integer, primary_key=True, autoincrement=True) name = Column(Unicode(255), unique=True) description = Column(Unicode(1023)) - scopes = Column(JSONList) + scopes = Column(JSONList, default=[]) users = relationship('User', secondary='user_role_map', backref='roles') services = relationship('Service', secondary='service_role_map', backref='roles') - tokens = relationship('APIToken', secondary='api_token_role_map', backref='roles') groups = relationship('Group', secondary='group_role_map', backref='roles') def __repr__(self): @@ -597,6 +592,10 @@ class APIToken(Hashed, Base): def api_id(self): return 'a%i' % self.id + @property + def owner(self): + return self.user or self.service + # added in 2.0 client_id = Column( Unicode(255), @@ -624,6 +623,7 @@ class APIToken(Hashed, Base): expires_at = Column(DateTime, default=None, nullable=True) last_activity = Column(DateTime) note = Column(Unicode(1023)) + scopes = Column(JSONList, default=[]) def __repr__(self): if self.user is not None: @@ -676,9 +676,11 @@ class APIToken(Hashed, Base): def new( cls, token=None, + *, user=None, service=None, roles=None, + scopes=None, note='', generated=True, session_id=None, @@ -697,6 +699,42 @@ class APIToken(Hashed, Base): generated = True else: cls.check_token(db, token) + + if scopes is not None and roles is not None: + raise ValueError( + "Can only assign one of scopes or roles when creating tokens." + ) + + elif scopes is None and roles is None: + # this is the default branch + # use the default 'token' role to specify default permissions for API tokens + default_token_role = Role.find(db, 'token') + if not default_token_role: + scopes = ["inherit"] + else: + scopes = roles_to_scopes([default_token_role]) + elif roles is not None: + # evaluate roles to scopes immediately + # TODO: should this be deprecated, or not? + # warnings.warn( + # "Setting roles on tokens is deprecated in JupyterHub 2.2. Use scopes.", + # DeprecationWarning, + # stacklevel=3, + # ) + orm_roles = [] + for rolename in roles: + role = Role.find(db, name=rolename) + if role is None: + raise ValueError(f"No such role: {rolename}") + orm_roles.append(role) + scopes = roles_to_scopes(orm_roles) + + # 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) + # two stages to ensure orm_token.generated has been set # before token setter is called orm_token = cls( @@ -704,6 +742,7 @@ class APIToken(Hashed, Base): note=note or '', client_id=client_id, session_id=session_id, + scopes=list(scopes), ) orm_token.token = token if user: @@ -716,21 +755,17 @@ class APIToken(Hashed, Base): orm_token.expires_at = cls.now() + timedelta(seconds=expires_in) db.add(orm_token) - if not Role.find(db, 'token'): - raise RuntimeError("Default token role has not been created") - try: - if roles is not None: - update_roles(db, entity=orm_token, roles=roles) - else: - assign_default_roles(db, entity=orm_token) - except Exception: - db.delete(orm_token) - db.commit() - raise - db.commit() return token + def update_scopes(self, new_scopes): + """Set new scopes, checking that they are allowed""" + 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) + self.scopes = new_scopes + class OAuthCode(Expiring, Base): __tablename__ = 'oauth_codes' @@ -746,7 +781,7 @@ class OAuthCode(Expiring, Base): # state = Column(Unicode(1023)) user_id = Column(Integer, ForeignKey('users.id', ondelete='CASCADE')) - roles = relationship('Role', secondary='oauth_code_role_map') + scopes = Column(JSONList, default=[]) @staticmethod def now(): diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 5bba4d45..c892e5f2 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -3,7 +3,6 @@ # Distributed under the terms of the Modified BSD License. import re from functools import wraps -from itertools import chain from sqlalchemy import func from tornado.log import app_log @@ -65,180 +64,51 @@ def get_default_roles(): return default_roles -def expand_self_scope(name): - """ - Users have a metascope 'self' that should be expanded to standard user privileges. - At the moment that is a user-filtered version (optional read) access to - users - users:name - users:groups - users:activity - tokens - servers - access:servers +def get_roles_for(orm_object): + """Get roles for a given User/Group/etc. - - Arguments: - name (str): user name - - Returns: - expanded scopes (set): set of expanded scopes covering standard user privileges - """ - scope_list = [ - 'read:users', - 'read:users:name', - 'read:users:groups', - 'users:activity', - 'read:users:activity', - 'servers', - 'delete:servers', - 'read:servers', - 'tokens', - 'read:tokens', - 'access:servers', - ] - return {f"{scope}!user={name}" for scope in scope_list} - - -def horizontal_filter(func): - """Decorator to account for horizontal filtering in scope syntax""" - - def expand_server_filter(hor_filter): - resource, mark, value = hor_filter.partition('=') - if resource == 'server': - user, mark, server = value.partition('/') - return f'read:users:name!user={user}' - - def ignore(scopename): - # temporarily remove horizontal filtering if present - scopename, mark, hor_filter = scopename.partition('!') - expanded_scope = func(scopename) - # add the filter back - full_expanded_scope = {scope + mark + hor_filter for scope in expanded_scope} - server_filter = expand_server_filter(hor_filter) - if server_filter: - full_expanded_scope.add(server_filter) - return full_expanded_scope - - return ignore - - -@horizontal_filter -def _expand_scope(scopename): - """Returns a set of all subscopes - Arguments: - scopename (str): name of the scope to expand - - Returns: - expanded scope (set): set of all scope's subscopes including the scope itself - """ - expanded_scope = [] - - def _add_subscopes(scopename): - expanded_scope.append(scopename) - if scopes.scope_definitions[scopename].get('subscopes'): - for subscope in scopes.scope_definitions[scopename].get('subscopes'): - _add_subscopes(subscope) - - _add_subscopes(scopename) - - return set(expanded_scope) - - -def expand_roles_to_scopes(orm_object): - """Get the scopes listed in the roles of the User/Service/Group/Token If User, take into account the user's groups roles as well Arguments: - orm_object: orm.User, orm.Service, orm.Group or orm.APIToken + orm_object: orm.User, orm.Service, orm.Group + Any role-having entity Returns: - expanded scopes (set): set of all expanded scopes for the orm object + roles (list): list of orm.Role objects assigned to the object. """ if not isinstance(orm_object, orm.Base): raise TypeError(f"Only orm objects allowed, got {orm_object}") - pass_roles = [] - pass_roles.extend(orm_object.roles) + roles = [] + roles.extend(orm_object.roles) if isinstance(orm_object, orm.User): for group in orm_object.groups: - pass_roles.extend(group.roles) - - expanded_scopes = _get_subscopes(*pass_roles, owner=orm_object) - - return expanded_scopes + roles.extend(group.roles) + return roles -def _get_subscopes(*roles, owner=None): - """Returns a set of all available subscopes for a specified role or list of roles +def roles_to_scopes(roles): + """Returns set of raw (not expanded) scopes for a collection of roles""" + raw_scopes = set() + + for role in roles: + raw_scopes.update(role.scopes) + return raw_scopes + + +def roles_to_expanded_scopes(roles, owner): + """Returns a set of fully expanded scopes for a specified role or list of roles Arguments: - roles (obj): orm.Roles - owner (obj, optional): orm.User or orm.Service as owner of orm.APIToken + roles (list(orm.Role): orm.Role objects to expand + owner (obj): orm.User or orm.Service which holds the role(s) + Used for expanding filters and metascopes such as !user. Returns: expanded scopes (set): set of all expanded scopes for the role(s) """ - scopes = set() - - for role in roles: - scopes.update(role.scopes) - - expanded_scopes = set(chain.from_iterable(list(map(_expand_scope, scopes)))) - # transform !user filter to !user=ownername - for scope in expanded_scopes.copy(): - base_scope, _, filter = scope.partition('!') - if filter == 'user': - expanded_scopes.remove(scope) - if isinstance(owner, orm.APIToken): - token_owner = owner.user - if token_owner is None: - token_owner = owner.service - name = token_owner.name - else: - name = owner.name - trans_scope = f'{base_scope}!user={name}' - expanded_scopes.add(trans_scope) - if 'self' in expanded_scopes: - expanded_scopes.remove('self') - if owner and isinstance(owner, orm.User): - expanded_scopes |= expand_self_scope(owner.name) - - return expanded_scopes - - -def _check_scopes(*args, rolename=None): - """Check if provided scopes exist - - Arguments: - scope (str): name of the scope to check - or - scopes (list): list of scopes to check - - Raises KeyError if scope does not exist - """ - - allowed_scopes = set(scopes.scope_definitions.keys()) - allowed_filters = ['!user=', '!service=', '!group=', '!server=', '!user'] - - if rolename: - log_role = f"for role {rolename}" - else: - log_role = "" - - for scope in args: - scopename, _, filter_ = scope.partition('!') - if scopename not in allowed_scopes: - if scopename == "all": - raise KeyError("Draft scope 'all' is now called 'inherit'") - raise KeyError(f"Scope '{scope}' {log_role} does not exist") - if filter_: - full_filter = f"!{filter_}" - if not any(f in scope for f in allowed_filters): - raise KeyError( - f"Scope filter '{full_filter}' in scope '{scope}' {log_role} does not exist" - ) + return scopes.expand_scopes(roles_to_scopes(roles), owner=owner) _role_name_pattern = re.compile(r'^[a-z][a-z0-9\-_~\.]{1,253}[a-z0-9]$') @@ -288,7 +158,10 @@ def create_role(db, role_dict): # check if the provided scopes exist if scopes: - _check_scopes(*scopes, rolename=role_dict['name']) + # avoid circular import + from .scopes import _check_scopes_exist + + _check_scopes_exist(scopes, who_for=f"role {role_dict['name']}") if role is None: if not scopes: @@ -362,13 +235,13 @@ def grant_role(db, entity, role): if role not in entity.roles: entity.roles.append(role) - db.commit() app_log.info( 'Adding role %s for %s: %s', role.name, type(entity).__name__, entity_repr, ) + db.commit() @_existing_only @@ -389,45 +262,6 @@ def strip_role(db, entity, role): ) -def _token_allowed_role(db, token, role): - """Checks if requested role for token does not grant the token - higher permissions than the token's owner has - - Returns: - True if requested permissions are within the owner's permissions, False otherwise - """ - owner = token.user - if owner is None: - owner = token.service - - if owner is None: - raise ValueError(f"Owner not found for {token}") - - if role in owner.roles: - # shortcut: token is assigned an exact role the owner has - return True - - expanded_scopes = _get_subscopes(role, owner=owner) - - implicit_permissions = {'inherit', 'read:inherit'} - explicit_scopes = expanded_scopes - implicit_permissions - # find the owner's scopes - expanded_owner_scopes = expand_roles_to_scopes(owner) - allowed_scopes = scopes._intersect_expanded_scopes( - explicit_scopes, expanded_owner_scopes, db - ) - disallowed_scopes = explicit_scopes.difference(allowed_scopes) - - if not disallowed_scopes: - # no scopes requested outside owner's own scopes - return True - else: - app_log.warning( - f"Token requesting role {role.name} with scopes not held by owner {owner.name}: {disallowed_scopes}" - ) - return False - - def assign_default_roles(db, entity): """Assigns default role(s) to an entity: @@ -440,54 +274,28 @@ def assign_default_roles(db, entity): if isinstance(entity, orm.Group): return - if isinstance(entity, orm.APIToken): - app_log.debug('Assigning default role to token') - default_token_role = orm.Role.find(db, 'token') - if not entity.roles and (entity.user or entity.service) is not None: - default_token_role.tokens.append(entity) - app_log.info('Added role %s to token %s', default_token_role.name, entity) - db.commit() # users and services all have 'user' role by default # and optionally 'admin' as well + + kind = type(entity).__name__ + app_log.debug(f'Assigning default role to {kind} {entity.name}') + if entity.admin: + grant_role(db, entity=entity, rolename="admin") else: - kind = type(entity).__name__ - app_log.debug(f'Assigning default role to {kind} {entity.name}') - if entity.admin: - grant_role(db, entity=entity, rolename="admin") - else: - admin_role = orm.Role.find(db, 'admin') - if admin_role in entity.roles: - strip_role(db, entity=entity, rolename="admin") - if kind == "User": - grant_role(db, entity=entity, rolename="user") + admin_role = orm.Role.find(db, 'admin') + if admin_role in entity.roles: + strip_role(db, entity=entity, rolename="admin") + if kind == "User": + grant_role(db, entity=entity, rolename="user") def update_roles(db, entity, roles): """Add roles to an entity (token, user, etc.) - If it is an API token, check role permissions against token owner - prior to assignment to avoid permission expansion. - - Otherwise, it just calls `grant_role` for each role. + Calls `grant_role` for each role. """ for rolename in roles: - if isinstance(entity, orm.APIToken): - role = orm.Role.find(db, rolename) - if role: - app_log.debug( - 'Checking token permissions against requested role %s', rolename - ) - if _token_allowed_role(db, entity, role): - role.tokens.append(entity) - app_log.info('Adding role %s to token: %s', role.name, entity) - else: - raise ValueError( - f'Requested token role {rolename} for {entity} has more permissions than the token owner' - ) - else: - raise KeyError(f'Role {rolename} does not exist') - else: - grant_role(db, entity=entity, rolename=rolename) + grant_role(db, entity=entity, rolename=rolename) def check_for_default_roles(db, bearer): diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 7a34f122..8a44e16e 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -15,6 +15,7 @@ import re import warnings from enum import Enum from functools import lru_cache +from itertools import chain from textwrap import indent import sqlalchemy as sa @@ -300,9 +301,8 @@ def get_scopes_for(orm_object): ) if isinstance(orm_object, orm.APIToken): - app_log.debug(f"Authenticated with token {orm_object}") owner = orm_object.user or orm_object.service - token_scopes = roles.expand_roles_to_scopes(orm_object) + 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 @@ -320,7 +320,8 @@ def get_scopes_for(orm_object): f"Token {orm_object} has no associated service or spawner!" ) - owner_scopes = roles.expand_roles_to_scopes(owner) + 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 @@ -346,7 +347,132 @@ def get_scopes_for(orm_object): ) expanded_scopes = intersection else: - expanded_scopes = roles.expand_roles_to_scopes(orm_object) + expanded_scopes = roles.roles_to_expanded_scopes( + roles.get_roles_for(orm_object), + owner=orm_object, + ) + return expanded_scopes + + +def _expand_self_scope(username): + """ + Users have a metascope 'self' that should be expanded to standard user privileges. + At the moment that is a user-filtered version (optional read) access to + users + users:name + users:groups + users:activity + tokens + servers + access:servers + + + Arguments: + username (str): user name + + Returns: + expanded scopes (set): set of expanded scopes covering standard user privileges + """ + scope_list = [ + 'read:users', + 'read:users:name', + 'read:users:groups', + 'users:activity', + 'read:users:activity', + 'servers', + 'delete:servers', + 'read:servers', + 'tokens', + 'read:tokens', + 'access:servers', + ] + return {f"{scope}!user={username}" for scope in scope_list} + + +def _expand_scope(scope): + """Returns a scope and all all subscopes + + Arguments: + scope (str): the scope to expand + + Returns: + expanded scope (set): set of all scope's subscopes including the scope itself + """ + + # remove filter, save for later + scope_name, sep, filter_ = scope.partition('!') + + # expand scope and subscopes + expanded_scope_names = set() + + def _add_subscopes(scope_name): + expanded_scope_names.add(scope_name) + if scope_definitions[scope_name].get('subscopes'): + for subscope in scope_definitions[scope_name].get('subscopes'): + _add_subscopes(subscope) + + _add_subscopes(scope_name) + + # reapply !filter + if filter_: + expanded_scopes = { + f"{scope_name}!{filter_}" for scope_name in expanded_scope_names + } + # special handling of server filter + # any access via server filter includes permission to read the user's name + resource, _, value = filter_.partition('=') + if resource == 'server': + username, _, server = value.partition('/') + expanded_scopes.add(f'read:users:name!user={username}') + else: + expanded_scopes = expanded_scope_names + + return expanded_scopes + + +def expand_scopes(scopes, owner=None): + """Returns a set of fully expanded scopes for a collection of raw scopes + + Arguments: + scopes (collection(str)): collection of raw scopes + 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` + + 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))) + + if isinstance(owner, orm.User): + owner_name = owner.name + else: + owner_name = None + + for scope in expanded_scopes.copy(): + base_scope, _, filter = scope.partition('!') + if filter == 'user': + # translate !user into !user={username} + expanded_scopes.remove(scope) + if owner_name: + # translate + expanded_scopes.add(f'{base_scope}!user={owner_name}') + else: + warnings.warn( + f"Not expanding !user filter without owner in {scope}", + stacklevel=2, + ) + + if 'self' in expanded_scopes: + expanded_scopes.remove('self') + if owner_name: + expanded_scopes |= _expand_self_scope(owner_name) + else: + warnings.warn( + "Not expanding 'self' scope without owner", + stacklevel=2, + ) + return expanded_scopes @@ -414,6 +540,73 @@ def _check_scope_access(api_handler, req_scope, **kwargs): raise web.HTTPError(404, "No access to resources or resources not found") +def _check_scopes_exist(scopes, who_for=None): + """Check if provided scopes exist + + Arguments: + scopes (list): list of scopes to check + + Raises KeyError if scope does not exist + """ + + allowed_scopes = set(scope_definitions.keys()) + allowed_filters = ('!user=', '!service=', '!group=', '!server=', '!user') + + if who_for: + log_for = f"for {who_for}" + else: + log_for = "" + + for scope in scopes: + scopename, _, filter_ = scope.partition('!') + if scopename not in allowed_scopes: + if scopename == "all": + raise KeyError("Draft scope 'all' is now called 'inherit'") + raise KeyError(f"Scope '{scope}' {log_for} does not exist") + if filter_: + full_filter = f"!{filter_}" + if not full_filter.startswith(allowed_filters): + raise KeyError( + f"Scope filter {filter_} '{full_filter}' in scope '{scope}' {log_for} does not exist" + ) + + +def _check_token_scopes(scopes, owner): + """Check that scopes to be assigned to a token + are in fact + + Arguments: + scopes: raw or expanded scopes + owner: orm.User or orm.Service + + raises: + ValueError: if requested scopes exceed owner's assigned scopes + """ + scopes = set(scopes) + if scopes.issubset({"inherit"}): + # nothing to check for simple 'inherit' scopes + return + scopes.discard("inherit") + # common short circuit + token_scopes = expand_scopes(scopes, owner=owner) + + if not token_scopes: + return + + owner_scopes = get_scopes_for(owner) + intersection = _intersect_expanded_scopes( + token_scopes, + owner_scopes, + db=sa.inspect(owner).session, + ) + excess_scopes = token_scopes - intersection + + if excess_scopes: + raise ValueError( + f"Not assigning requested scopes {','.join(excess_scopes)} not held by {owner.__class__.__name__} {owner.name}" + ) + + def parse_scopes(scope_list): """ Parses scopes and filters in something akin to JSON style diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 22ae94b4..8e540429 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -303,7 +303,6 @@ def _mockservice(request, app, url=False): assert name in app._service_map service = app._service_map[name] token = service.orm.api_tokens[0] - update_roles(app.db, token, roles=['token']) async def start(): # wait for proxy to be updated before starting the service diff --git a/jupyterhub/tests/populate_db.py b/jupyterhub/tests/populate_db.py index 4504a13c..c85e8c57 100644 --- a/jupyterhub/tests/populate_db.py +++ b/jupyterhub/tests/populate_db.py @@ -18,6 +18,32 @@ def populate_db(url): if 'mysql' in url: connect_args['auth_plugin'] = 'mysql_native_password' db = orm.new_session_factory(url, connect_args=connect_args)() + + if jupyterhub.version_info >= (2,): + if ( + not db.query(orm.OAuthClient) + .filter_by(identifier="jupyterhub") + .one_or_none() + ): + # create the oauth client for jupyterhub itself + # this allows us to distinguish between orphaned tokens + # (failed cascade deletion) and tokens issued by the hub + # it has no client_secret, which means it cannot be used + # to make requests + client = orm.OAuthClient( + identifier="jupyterhub", + secret="", + redirect_uri="", + description="JupyterHub", + ) + db.add(client) + db.commit() + + from jupyterhub import roles + + for role in roles.get_default_roles(): + roles.create_role(db, role) + # create some users admin = orm.User(name='admin', admin=True) db.add(admin) @@ -80,6 +106,11 @@ def populate_db(url): client_id=client.identifier, user_id=user.id, ) + if jupyterhub.version_info >= (2,): + if jupyterhub.version_info < (2, 2): + access_token.roles = [db.query(orm.Role).filter_by(name="server").one()] + else: + access_token.scopes = [f"read:users!user={user.name}"] db.add(access_token) db.commit() diff --git a/jupyterhub/tests/test_db.py b/jupyterhub/tests/test_db.py index 77231f97..a4a39f12 100644 --- a/jupyterhub/tests/test_db.py +++ b/jupyterhub/tests/test_db.py @@ -8,10 +8,10 @@ import pytest from pytest import raises from traitlets.config import Config -from ..app import JupyterHub +from .. import orm from ..app import NewToken from ..app import UpgradeDB - +from ..scopes import _check_scopes_exist here = os.path.abspath(os.path.dirname(__file__)) populate_db = os.path.join(here, 'populate_db.py') @@ -36,7 +36,7 @@ def generate_old_db(env_dir, hub_version, db_url): check_call([env_py, populate_db, db_url]) -@pytest.mark.parametrize('hub_version', ['1.0.0', "1.2.2", "1.3.0"]) +@pytest.mark.parametrize('hub_version', ['1.0.0', "1.2.2", "1.3.0", "1.5.0", "2.1.1"]) async def test_upgrade(tmpdir, hub_version): db_url = os.getenv('JUPYTERHUB_TEST_DB_URL') if db_url: @@ -75,3 +75,10 @@ async def test_upgrade(tmpdir, hub_version): # run tokenapp again, it should work tokenapp.start() + + db = orm.new_session_factory(db_url)() + query = db.query(orm.APIToken) + assert query.count() >= 1 + for token in query: + assert token.scopes, f"Upgraded token {token} has no scopes" + _check_scopes_exist(token.scopes) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 3476058f..81c159a9 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -10,6 +10,7 @@ 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 @@ -76,24 +77,17 @@ def test_orm_roles(db): # assigns it the default 'token' role token = user.new_api_token() user_token = orm.APIToken.find(db, token=token) - assert user_token in token_role.tokens - assert token_role in user_token.roles + assert set(user_token.scopes) == set(token_role.scopes) # check creating token with a specific role token = service.new_api_token(roles=['service']) service_token = orm.APIToken.find(db, token=token) - assert service_token in service_role.tokens - assert service_role in service_token.roles + assert set(service_token.scopes) == set(service_role.scopes) - # check deleting user removes the user and the token from roles + # check deleting user removes the user from roles db.delete(user) db.commit() assert user_role.users == [] - assert user_token not in token_role.tokens - # check deleting the service token removes it from 'service' role - db.delete(service_token) - db.commit() - assert service_token not in service_role.tokens # check deleting the service_role removes it from service.roles db.delete(service_role) db.commit() @@ -174,7 +168,7 @@ def test_orm_roles_delete_cascade(db): @mark.role @mark.parametrize( - "scopes, subscopes", + "scopes, expected_scopes", [ ( ['admin:users'], @@ -246,12 +240,12 @@ def test_orm_roles_delete_cascade(db): ), ], ) -def test_get_subscopes(db, scopes, subscopes): - """Test role scopes expansion into their subscopes""" +def test_get_expanded_scopes(db, scopes, expected_scopes): + """Test role scopes expansion into their fully expanded scopes""" roles.create_role(db, {'name': 'testing_scopes', 'scopes': scopes}) role = orm.Role.find(db, name='testing_scopes') - response = roles._get_subscopes(role) - assert response == subscopes + expanded_scopes = roles.roles_to_expanded_scopes([role], owner=None) + assert expanded_scopes == expected_scopes db.delete(role) @@ -659,9 +653,9 @@ async def test_load_roles_user_tokens(tmpdir, request): # test if all other tokens have default 'user' role token_role = orm.Role.find(db, 'token') secret_token = orm.APIToken.find(db, 'secret-token') - assert token_role in secret_token.roles + assert set(secret_token.scopes) == set(token_role.scopes) secrety_token = orm.APIToken.find(db, 'secrety-token') - assert token_role in secrety_token.roles + assert set(secrety_token.scopes) == set(token_role.scopes) # delete the test tokens for token in db.query(orm.APIToken): @@ -682,15 +676,15 @@ async def test_load_roles_user_tokens(tmpdir, request): # role scopes within the user's default 'user' role ({}, 'self-reader', ['read:users!user'], 201), # role scopes within the user's default 'user' role, but with disjoint filter - ({}, 'other-reader', ['read:users!user=other'], 403), + ({}, 'other-reader', ['read:users!user=other'], 400), # role scopes within the user's default 'user' role, without filter - ({}, 'other-reader', ['read:users'], 403), + ({}, 'other-reader', ['read:users'], 400), # role scopes outside of the user's role but within the group's role scopes of which the user is a member ({}, 'groups-reader', ['read:groups'], 201), # non-existing role request - ({}, 'non-existing', [], 404), + ({}, 'non-existing', [], 400), # role scopes outside of both user's role and group's role scopes - ({}, 'users-creator', ['admin:users'], 403), + ({}, 'users-creator', ['admin:users'], 400), ], ) async def test_get_new_token_via_api(app, headers, rolename, scopes, status): @@ -756,7 +750,7 @@ async def test_self_expansion(app, kind, has_user_scopes): test_role = orm.Role(name='test_role', scopes=['self']) orm_obj.roles.append(test_role) # test expansion of user/service scopes - scopes = roles.expand_roles_to_scopes(orm_obj) + scopes = get_scopes_for(orm_obj) assert bool(scopes) == has_user_scopes if kind == 'users': for scope in scopes: @@ -797,9 +791,9 @@ async def test_user_filter_expansion(app, scope_list, kind, test_for_token): if test_for_token: token = orm_obj.new_api_token(roles=['test_role']) orm_token = orm.APIToken.find(app.db, token) - expanded_scopes = roles.expand_roles_to_scopes(orm_token) + expanded_scopes = get_scopes_for(orm_token) else: - expanded_scopes = roles.expand_roles_to_scopes(orm_obj) + expanded_scopes = get_scopes_for(orm_obj) for scope in scope_list: base, _, filter = scope.partition('!') @@ -817,7 +811,7 @@ async def test_user_filter_expansion(app, scope_list, kind, test_for_token): async def test_large_filter_expansion(app, create_temp_role, create_user_with_scopes): - scope_list = roles.expand_self_scope('==') + 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) @@ -864,9 +858,7 @@ async def test_server_token_role(app): assert orm_server_token server_role = orm.Role.find(app.db, 'server') - token_role = orm.Role.find(app.db, 'token') - assert server_role in orm_server_token.roles - assert token_role not in orm_server_token.roles + assert set(server_role.scopes) == set(orm_server_token.scopes) assert orm_server_token.user.name == user.name assert user.api_tokens == [orm_server_token] @@ -891,7 +883,7 @@ async def test_server_role_api_calls( user = add_user(app.db, app, name='test_user') roles.grant_role(app.db, user, 'user') app_log.debug(user.roles) - app_log.debug(roles.expand_roles_to_scopes(user.orm_user)) + app_log.debug(get_scopes_for(user.orm_user)) if token_role == 'no_role': api_token = user.new_api_token(roles=[]) else: @@ -963,7 +955,7 @@ async def test_user_group_roles(app, create_temp_role): # regression test for #3472 roles_before = list(user.roles) for i in range(3): - roles.expand_roles_to_scopes(user.orm_user) + get_scopes_for(user.orm_user) user_roles = list(user.roles) assert user_roles == roles_before @@ -1065,33 +1057,6 @@ async def test_config_role_users(): assert role not in user.roles -async def test_scope_expansion_revokes_tokens(app, mockservice_url): - role_name = 'morpheus' - roles_to_load = [ - { - 'name': role_name, - 'description': 'wears sunglasses', - 'scopes': ['users', 'groups'], - }, - ] - app.load_roles = roles_to_load - await app.init_role_creation() - user = add_user(app.db, name='laurence') - for _ in range(2): - user.new_api_token() - red_token, blue_token = user.api_tokens - roles.grant_role(app.db, red_token, role_name) - service = mockservice_url - red_token.client_id = service.oauth_client_id - app.db.commit() - # Restart hub and see if token is revoked - app.load_roles[0]['scopes'].append('proxy') - await app.init_role_creation() - user = orm.User.find(app.db, name='laurence') - assert red_token not in user.api_tokens - assert blue_token in user.api_tokens - - async def test_duplicate_role_users(): role_name = 'painter' user_name = 'benny' @@ -1352,7 +1317,7 @@ async def test_hub_upgrade_detection(tmpdir): for name in user_names: user = orm.User.find(hub.db, name) assert user_role in user.roles - assert token_role in user.api_tokens[0].roles + assert set(user.api_tokens[0].scopes) == set(token_role.scopes) # Strip all roles and see if it sticks user_role.users = [] token_role.tokens = [] @@ -1371,50 +1336,10 @@ async def test_hub_upgrade_detection(tmpdir): allowed_user = orm.User.find(hub.db, 'patricia') rem_user = orm.User.find(hub.db, 'quentin') assert user_role in allowed_user.roles - assert token_role not in allowed_user.api_tokens[0].roles assert user_role not in rem_user.roles assert token_role not in rem_user.roles -async def test_token_keep_roles_on_restart(): - role_spec = [ - { - 'name': 'bloop', - 'scopes': ['read:users'], - } - ] - hub = MockHub(load_roles=role_spec) - hub.init_db() - hub.authenticator.admin_users = ['ben'] - await hub.init_role_creation() - await hub.init_users() - await hub.init_role_assignment() - user = orm.User.find(hub.db, name='ben') - for _ in range(3): - user.new_api_token() - happy_token, content_token, sad_token = user.api_tokens - roles.grant_role(hub.db, happy_token, 'bloop') - roles.strip_role(hub.db, sad_token, 'token') - assert len(happy_token.roles) == 2 - assert len(content_token.roles) == 1 - assert len(sad_token.roles) == 0 - # Restart hub and see if roles are as expected - hub.load_roles = [] - await hub.init_role_creation() - await hub.init_users() - await hub.init_api_tokens() - await hub.init_role_assignment() - user = orm.User.find(hub.db, name='ben') - happy_token, content_token, sad_token = user.api_tokens - assert len(happy_token.roles) == 1 - assert len(content_token.roles) == 1 - print(sad_token.roles) - assert len(sad_token.roles) == 0 - for token in user.api_tokens: - hub.db.delete(token) - hub.db.commit() - - async def test_login_default_role(app, username): cookies = await app.login_user(username) user = app.users[username] diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index fc42a89f..946821e6 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -278,19 +278,19 @@ async def test_refuse_exceeding_token_permissions( ): user = create_user_with_scopes('self') user.new_api_token() - create_temp_role(['admin:users'], 'exceeding_role') with pytest.raises(ValueError): - roles.update_roles(app.db, entity=user.api_tokens[0], roles=['exceeding_role']) + user.api_tokens[0].update_scopes(["admin:users"]) async def test_exceeding_user_permissions( - app, create_user_with_scopes, create_temp_role + app, + create_user_with_scopes, ): user = create_user_with_scopes('list:users', 'read:users:groups') api_token = user.new_api_token() orm_api_token = orm.APIToken.find(app.db, token=api_token) - create_temp_role(['list:users', 'read:users'], 'reader_role') - roles.grant_role(app.db, orm_api_token, rolename='reader_role') + # store scopes user does not have + orm_api_token.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 @@ -465,7 +465,7 @@ async def test_metascope_self_expansion( else: orm_obj = create_service_with_scopes('self') # test expansion of user/service scopes - scopes = roles.expand_roles_to_scopes(orm_obj) + scopes = get_scopes_for(orm_obj) assert bool(scopes) == has_user_scopes # test expansion of token scopes @@ -484,7 +484,7 @@ async def test_metascope_all_expansion(app, create_user_with_scopes): assert user_scope_set == token_scope_set # Check no roles means no permissions - token.roles.clear() + token.scopes.clear() app.db.commit() token_scope_set = get_scopes_for(token) assert not token_scope_set @@ -688,8 +688,8 @@ async def test_resolve_token_permissions( roles.strip_role(app.db, orm_user, "admin") # get expanded !user filter scopes for check - user_scopes = roles.expand_roles_to_scopes(orm_user) - token_scopes = roles.expand_roles_to_scopes(orm_api_token) + user_scopes = get_scopes_for(orm_user) + token_scopes = get_scopes_for(orm_api_token) token_retained_scopes = get_scopes_for(orm_api_token) diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py index af0d45a6..55f2b6bb 100644 --- a/jupyterhub/tests/test_services.py +++ b/jupyterhub/tests/test_services.py @@ -1,5 +1,4 @@ """Tests for services""" -import asyncio import os import sys from binascii import hexlify @@ -99,7 +98,6 @@ async def test_external_service(app): assert service.oauth_client is not None assert service.oauth_client.allowed_roles == [orm.Role.find(app.db, "user")] api_token = service.orm.api_tokens[0] - update_roles(app.db, api_token, roles=['token']) url = public_url(app, service) + '/api/users' r = await async_requests.get(url, allow_redirects=False) r.raise_for_status() diff --git a/jupyterhub/tests/test_services_auth.py b/jupyterhub/tests/test_services_auth.py index 8e2054fa..e3fde251 100644 --- a/jupyterhub/tests/test_services_auth.py +++ b/jupyterhub/tests/test_services_auth.py @@ -428,9 +428,9 @@ 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) - user.new_api_token() + 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] - token.roles = [test_roles[t] for t in token_roles] oauth_client = ( app.db.query(orm.OAuthClient) From a08aa3398c13c482bd2dcb2176fb76d0e6c4211e Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 18 Mar 2022 15:25:46 +0100 Subject: [PATCH 2/7] ensure literal_binds is set in order Co-authored-by: Erik Sundell --- jupyterhub/alembic/env.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jupyterhub/alembic/env.py b/jupyterhub/alembic/env.py index 2d135290..30dfbd92 100644 --- a/jupyterhub/alembic/env.py +++ b/jupyterhub/alembic/env.py @@ -65,8 +65,9 @@ def run_migrations_offline(): """ connectable = config.attributes.get('connection', None) - config_opts = dict(literal_binds=True) + config_opts = {} config_opts.update(common_config_opts) + config_opts["literal_binds"] = True if connectable is None: config_opts["url"] = config.get_main_option("sqlalchemy.url") From 7e22614a4ec2a68a1b8d84f09fce988ab4b292a0 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 24 Mar 2022 15:05:50 +0100 Subject: [PATCH 3/7] [squash me] token progress tokens have scopes instead of roles, which allow tokens to change permissions over time This is mostly a low-level change, with little outward-facing effects. - on upgrade, evaluate all token role assignments to their current scopes, and store those scopes on the tokens - assigning roles to tokens still works, but scopes are evaluated and validated immediately, rather than lazily stored as roles - no longer need to check for role permission changes on startup, because token permissions aren't affected - move a few scope utilities from roles to scopes - oauth allows specifying scopes, not just roles. But these are still at the level specified in roles, not fully-resolved scopes. - more granular APIs for working with scopes and roles Still to do later: - expose scopes config for Spawner/service - compute 'full' intersection of requested scopes, rather than on the 'raw' scope list in roles --- docs/source/_static/rest-api.yml | 24 +++++- docs/source/rbac/roles.md | 2 +- docs/source/rbac/tech-implementation.md | 28 +++++-- jupyterhub/apihandlers/auth.py | 33 ++++++-- jupyterhub/app.py | 1 - jupyterhub/oauth/provider.py | 59 ++++++++++---- jupyterhub/scopes.py | 104 ++++++++++++++++-------- jupyterhub/tests/mockservice.py | 4 +- jupyterhub/tests/test_roles.py | 14 ---- jupyterhub/tests/test_scopes.py | 46 ++++++++++- jupyterhub/tests/test_services_auth.py | 99 +++++++++++++++++----- share/jupyterhub/templates/oauth.html | 6 +- 12 files changed, 311 insertions(+), 109 deletions(-) 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 %} From 3cfb14b9e5ae272be85fa1ca75ee53e4654716e6 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 24 Mar 2022 15:16:21 +0100 Subject: [PATCH 4/7] rerender rest-api --- docs/source/_static/rest-api.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/source/_static/rest-api.yml b/docs/source/_static/rest-api.yml index 236ddb68..a4a5390e 100644 --- a/docs/source/_static/rest-api.yml +++ b/docs/source/_static/rest-api.yml @@ -1326,15 +1326,16 @@ components: description: The service that owns the token (undefined of owned by a user) roles: type: array - description: Deprecated in JupyterHub 2.3, always an empty list. - Tokens have 'scopes' starting from JupyterHub 2.3. + 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. + 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: From 78b5aa150c027e8f198637976ba83638700885af Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 24 Mar 2022 15:36:56 +0100 Subject: [PATCH 5/7] avoid always-adding identify scope to everything add it to token permissions _before_ intersecting with owner --- jupyterhub/scopes.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 0021bb75..25196354 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -319,11 +319,11 @@ def get_scopes_for(orm_object): 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) + token_scopes.update(access_scopes(orm_object.oauth_client)) # reduce to collapse multiple filters on the same scope # to avoid spurious logs about discarded scopes + token_scopes.update(identify_scopes(owner)) token_scopes = reduce_scopes(token_scopes) intersection = _intersect_expanded_scopes( @@ -346,6 +346,8 @@ def get_scopes_for(orm_object): token_scopes, ) expanded_scopes = intersection + # always include identify scopes + expanded_scopes else: expanded_scopes = roles.roles_to_expanded_scopes( roles.get_roles_for(orm_object), @@ -354,9 +356,6 @@ def get_scopes_for(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 From 0f4258d00c44917736d795dc54029bd1d773ab8d Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 24 Mar 2022 15:47:02 +0100 Subject: [PATCH 6/7] update more test expectations --- jupyterhub/tests/test_scopes.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index caac1b2c..3548b54c 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -16,6 +16,7 @@ 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 identify_scopes from ..scopes import needs_scope from ..scopes import parse_scopes from ..scopes import Scope @@ -292,7 +293,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.update_scopes(orm_api_token.scopes + ['list:users', 'read:users']) + orm_api_token.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 @@ -476,9 +477,9 @@ async def test_metascope_self_expansion( assert bool(token_scopes) == has_user_scopes -async def test_metascope_all_expansion(app, create_user_with_scopes): +async def test_metascope_inherit_expansion(app, create_user_with_scopes): user = create_user_with_scopes('self') - user.new_api_token() + user.new_api_token(scopes=["inherit"]) token = user.api_tokens[0] # Check 'inherit' expansion token_scope_set = get_scopes_for(token) @@ -489,7 +490,7 @@ async def test_metascope_all_expansion(app, create_user_with_scopes): token.scopes.clear() app.db.commit() token_scope_set = get_scopes_for(token) - assert not token_scope_set + assert token_scope_set.issubset(identify_scopes(user.orm_user)) @mark.parametrize( From 310d9621e5cb0941a8d69edd5403874da6b465ad Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 24 Mar 2022 16:13:57 +0100 Subject: [PATCH 7/7] limit server->read:users:name filter to read: scopes it shouldn't be included in _access_ scopes, for instance --- jupyterhub/scopes.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 25196354..8af2263b 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -424,9 +424,11 @@ def _expand_scope(scope): f"{scope_name}!{filter_}" for scope_name in expanded_scope_names } # special handling of server filter - # any access via server filter includes permission to read the user's name + # any read access via server filter includes permission to read the user's name resource, _, value = filter_.partition('=') - if resource == 'server': + if resource == 'server' and any( + scope_name.startswith("read:") for scope_name in expanded_scope_names + ): username, _, server = value.partition('/') expanded_scopes.add(f'read:users:name!user={username}') else: