From b25517efe813e56e15629f77e4aa7d6e58d5c5eb Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 25 Oct 2021 14:35:52 +0200 Subject: [PATCH] Rename 'all' metascope to more descriptive 'inherit' since it means 'inheriting' the owner's permissions 'all' prompted the question 'all of what, exactly?' Additionally, fix some NameErrors that should have been KeyErrors --- docs/source/rbac/roles.md | 4 ++-- jupyterhub/apihandlers/auth.py | 8 ++++---- jupyterhub/apihandlers/users.py | 6 ++++-- jupyterhub/app.py | 2 +- jupyterhub/roles.py | 18 ++++++++++-------- jupyterhub/scopes.py | 10 +++++----- jupyterhub/tests/test_roles.py | 12 ++++++------ jupyterhub/tests/test_scopes.py | 2 +- 8 files changed, 33 insertions(+), 29 deletions(-) diff --git a/docs/source/rbac/roles.md b/docs/source/rbac/roles.md index 44754a75..47927c64 100644 --- a/docs/source/rbac/roles.md +++ b/docs/source/rbac/roles.md @@ -123,13 +123,13 @@ has, define the `server` role. To restore the JupyterHub 1.x behavior of servers being able to do anything their owners can do, -use the scope `all`: +use the scope `inherit` (for 'inheriting' the owner's permissions): ```python c.JupyterHub.load_roles = [ { 'name': 'server', - 'scopes': ['all'], + 'scopes': ['inherit'], } ] ``` diff --git a/jupyterhub/apihandlers/auth.py b/jupyterhub/apihandlers/auth.py index 7a787928..9c38907f 100644 --- a/jupyterhub/apihandlers/auth.py +++ b/jupyterhub/apihandlers/auth.py @@ -308,12 +308,12 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): "filter": "", } ] - elif 'all' in raw_scopes: - raw_scopes = ['all'] + elif 'inherit' in raw_scopes: + raw_scopes = ['inherit'] scope_descriptions = [ { - "scope": "all", - "description": scopes.scope_definitions['all']['description'], + "scope": "inherit", + "description": scopes.scope_definitions['inherit']['description'], "filter": "", } ] diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index 43905d82..2b977277 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -397,9 +397,11 @@ class UserTokenListAPIHandler(APIHandler): token_roles = body.get('roles') try: api_token = user.new_api_token( - note=note, expires_in=body.get('expires_in', None), roles=token_roles + note=note, + expires_in=body.get('expires_in', None), + roles=token_roles, ) - except NameError: + except KeyError: raise web.HTTPError(404, "Requested roles %r not found" % token_roles) except ValueError: raise web.HTTPError( diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 475ae16a..46501ef4 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -2070,7 +2070,7 @@ class JupyterHub(Application): if role_spec['name'] == 'admin': app_log.warning( "Configuration specifies both admin_users and users in the admin role specification. " - "If admin role is present in config, c.authenticator.admin_users should not be used." + "If admin role is present in config, c.Authenticator.admin_users should not be used." ) app_log.info( "Merging admin_users set with users list in admin role" diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 7ac5382a..df4f95f8 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -57,7 +57,7 @@ def get_default_roles(): { 'name': 'token', 'description': 'Token with same permissions as its owner', - 'scopes': ['all'], + 'scopes': ['inherit'], }, ] return default_roles @@ -214,7 +214,7 @@ def _check_scopes(*args, rolename=None): or scopes (list): list of scopes to check - Raises NameError if scope does not exist + Raises KeyError if scope does not exist """ allowed_scopes = set(scopes.scope_definitions.keys()) @@ -228,11 +228,13 @@ def _check_scopes(*args, rolename=None): for scope in args: scopename, _, filter_ = scope.partition('!') if scopename not in allowed_scopes: - raise NameError(f"Scope '{scope}' {log_role} does not exist") + 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 NameError( + raise KeyError( f"Scope filter '{full_filter}' in scope '{scope}' {log_role} does not exist" ) @@ -322,7 +324,7 @@ def delete_role(db, rolename): db.commit() app_log.info('Role %s has been deleted', rolename) else: - raise NameError('Cannot remove role %r that does not exist', rolename) + raise KeyError('Cannot remove role %r that does not exist', rolename) def existing_only(func): @@ -413,7 +415,7 @@ def _token_allowed_role(db, token, role): expanded_scopes = _get_subscopes(role, owner=owner) - implicit_permissions = {'all', 'read:all'} + implicit_permissions = {'inherit', 'read:inherit'} explicit_scopes = expanded_scopes - implicit_permissions # ignore horizontal filters no_filter_scopes = { @@ -462,7 +464,7 @@ def update_roles(db, entity, roles): """Updates object's roles checking for requested permissions if object is orm.APIToken """ - standard_permissions = {'all', 'read:all'} + standard_permissions = {'inherit', 'read:inherit'} for rolename in roles: if isinstance(entity, orm.APIToken): role = orm.Role.find(db, rolename) @@ -478,7 +480,7 @@ def update_roles(db, entity, roles): f'Requested token role {rolename} of {entity} has more permissions than the token owner' ) else: - raise NameError('Role %r does not exist' % rolename) + raise KeyError(f'Role {rolename} does not exist') else: app_log.debug('Assigning default roles to %s', type(entity).__name__) grant_role(db, entity=entity, rolename=rolename) diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py index 5559f5a4..6212ea95 100644 --- a/jupyterhub/scopes.py +++ b/jupyterhub/scopes.py @@ -30,7 +30,7 @@ scope_definitions = { 'description': 'Your own resources', 'doc_description': 'The user’s own resources _(metascope for users, resolves to (no_scope) for services)_', }, - 'all': { + 'inherit': { 'description': 'Anything you have access to', 'doc_description': 'Everything that the token-owning entity can access _(metascope for tokens)_', }, @@ -317,13 +317,13 @@ def get_scopes_for(orm_object): owner_scopes = roles.expand_roles_to_scopes(owner) - if token_scopes == {'all'}: - # token_scopes is only 'all', return owner scopes as-is + 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 return owner_scopes - if 'all' in token_scopes: - token_scopes.remove('all') + if 'inherit' in token_scopes: + token_scopes.remove('inherit') token_scopes |= owner_scopes intersection = _intersect_expanded_scopes( diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index ed9ede2e..84b96619 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -28,7 +28,7 @@ def test_orm_roles(db): user_role = orm.Role(name='user', scopes=['self']) db.add(user_role) if not token_role: - token_role = orm.Role(name='token', scopes=['all']) + token_role = orm.Role(name='token', scopes=['inherit']) db.add(token_role) if not service_role: service_role = orm.Role(name='service', scopes=[]) @@ -369,7 +369,7 @@ async def test_creating_roles(app, role, role_def, response_type, response): 'info', app_log.info('Role user scopes attribute has been changed'), ), - ('non-existing', 'test-role2', 'error', NameError), + ('non-existing', 'test-role2', 'error', KeyError), ('default', 'user', 'error', ValueError), ], ) @@ -410,9 +410,9 @@ async def test_delete_roles(db, role_type, rolename, response_type, response): }, 'existing', ), - ({'name': 'test-scopes-2', 'scopes': ['uses']}, NameError), - ({'name': 'test-scopes-3', 'scopes': ['users:activities']}, NameError), - ({'name': 'test-scopes-4', 'scopes': ['groups!goup=class-A']}, NameError), + ({'name': 'test-scopes-2', 'scopes': ['uses']}, KeyError), + ({'name': 'test-scopes-3', 'scopes': ['users:activities']}, KeyError), + ({'name': 'test-scopes-4', 'scopes': ['groups!goup=class-A']}, KeyError), ], ) async def test_scope_existence(tmpdir, request, role, response): @@ -431,7 +431,7 @@ async def test_scope_existence(tmpdir, request, role, response): assert added_role is not None assert added_role.scopes == role['scopes'] - elif response == NameError: + elif response == KeyError: with pytest.raises(response): roles.create_role(db, role) added_role = orm.Role.find(db, role['name']) diff --git a/jupyterhub/tests/test_scopes.py b/jupyterhub/tests/test_scopes.py index 3e055611..b54dc67c 100644 --- a/jupyterhub/tests/test_scopes.py +++ b/jupyterhub/tests/test_scopes.py @@ -477,7 +477,7 @@ async def test_metascope_all_expansion(app, create_user_with_scopes): user = create_user_with_scopes('self') user.new_api_token() token = user.api_tokens[0] - # Check 'all' expansion + # Check 'inherit' expansion token_scope_set = get_scopes_for(token) user_scope_set = get_scopes_for(user) assert user_scope_set == token_scope_set