From 5e8864f29df3054cfe6af4c77a05430c2bd1450c Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Wed, 16 Dec 2020 11:17:43 +0100 Subject: [PATCH 01/10] fixed default roles for mocked services --- jupyterhub/apihandlers/base.py | 11 +++- jupyterhub/app.py | 18 ++++-- jupyterhub/orm.py | 9 +++ jupyterhub/roles.py | 36 +++++++++++- jupyterhub/tests/conftest.py | 2 + jupyterhub/tests/test_api.py | 13 +++-- jupyterhub/tests/test_roles.py | 102 ++++++++++++++++++++++++++++++++- 7 files changed, 175 insertions(+), 16 deletions(-) diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py index a416e6c8..e6524cff 100644 --- a/jupyterhub/apihandlers/base.py +++ b/jupyterhub/apihandlers/base.py @@ -222,6 +222,7 @@ class APIHandler(BaseHandler): 'kind': 'group', 'name': group.name, 'users': [u.name for u in group.users], + 'roles': [r.name for r in group.roles], } def service_model(self, service): @@ -233,9 +234,15 @@ class APIHandler(BaseHandler): 'roles': [r.name for r in service.roles], } - _user_model_types = {'name': str, 'admin': bool, 'groups': list, 'auth_state': dict} + _user_model_types = { + 'name': str, + 'admin': bool, + 'groups': list, + 'roles': list, + 'auth_state': dict, + } - _group_model_types = {'name': str, 'users': list} + _group_model_types = {'name': str, 'users': list, 'roles': list} def _check_model(self, model, model_types, name): """Check a model provided by a REST API request diff --git a/jupyterhub/app.py b/jupyterhub/app.py index c7571453..28c70e29 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -326,7 +326,8 @@ class JupyterHub(Application): 'scopes': ['users', 'groups'], 'users': ['cyclops', 'gandalf'], 'services': [], - 'tokens': [] + 'tokens': [], + 'groups': [] } ] @@ -1827,7 +1828,7 @@ class JupyterHub(Application): async def init_roles(self): """Load default and predefined roles into the database""" db = self.db - role_bearers = ['users', 'services', 'tokens'] + role_bearers = ['groups', 'users', 'services', 'tokens'] # load default roles default_roles = roles.get_default_roles() @@ -1857,11 +1858,18 @@ class JupyterHub(Application): ) # make sure all users, services and tokens have at least one role (update with default) + # groups can be without a role but all group members should have the same role(s) as the group for bearer in role_bearers: Class = roles.get_orm_class(bearer) - for obj in db.query(Class): - if len(obj.roles) < 1: - roles.update_roles(db, obj=obj, kind=bearer) + if bearer == 'groups': + for group in db.query(Class): + for user in group.users: + rolenames = roles.get_rolenames(group.roles) + roles.update_roles(db, obj=user, kind='users', roles=rolenames) + else: + for obj in db.query(Class): + if len(obj.roles) < 1: + roles.update_roles(db, obj=obj, kind=bearer) db.commit() async def _add_tokens(self, token_dict, kind): diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 90ca63f7..42089c10 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -166,6 +166,14 @@ api_token_role_map = Table( Column('role_id', ForeignKey('roles.id', ondelete='CASCADE'), primary_key=True), ) +# group:role many:many mapping table +group_role_map = Table( + 'group_role_map', + Base.metadata, + Column('group_id', ForeignKey('groups.id', ondelete='CASCADE'), primary_key=True), + Column('role_id', ForeignKey('roles.id', ondelete='CASCADE'), primary_key=True), +) + class Role(Base): """User Roles""" @@ -178,6 +186,7 @@ class Role(Base): 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): return "<%s %s (%s) - scopes: %s>" % ( diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 544aa08f..6291a3ce 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -147,12 +147,26 @@ def get_orm_class(kind): Class = orm.Service elif kind == 'tokens': Class = orm.APIToken + elif kind == 'groups': + Class = orm.Group else: - raise ValueError("kind must be users, services or tokens, not %r" % kind) + raise ValueError( + "kind must be users, services, tokens or groups, not %r" % kind + ) return Class +def get_rolenames(role_list): + + """Return a list of rolenames from a list of roles""" + + rolenames = [] + for role in role_list: + rolenames.append(role.name) + return rolenames + + def existing_only(func): """Decorator for checking if objects and roles exist""" @@ -176,7 +190,7 @@ def existing_only(func): @existing_only def add_obj(db, objname, kind, rolename): - """Adds a role for users, services or tokens""" + """Adds a role for users, services, tokens or groups""" if rolename not in objname.roles: objname.roles.append(rolename) @@ -250,12 +264,28 @@ def update_roles(db, obj, kind, roles=None): else: add_obj(db, objname=obj.name, kind=kind, rolename=rolename) else: + # groups can be without a role + if Class == orm.Group: + pass # tokens can have only 'user' role as default # assign the default only for user tokens - if Class == orm.APIToken: + elif Class == orm.APIToken: if len(obj.roles) < 1 and obj.user is not None: user_role.tokens.append(obj) db.commit() # users and services can have 'user' or 'admin' roles as default else: switch_default_role(db, obj, kind, obj.admin) + + +def mock_roles(app, name, kind): + + """Loads and assigns default roles for mocked objects""" + + Class = get_orm_class(kind) + + obj = Class.find(app.db, name=name) + default_roles = get_default_roles() + for role in default_roles: + add_role(app.db, role) + update_roles(db=app.db, obj=obj, kind=kind) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 104ca635..45bfd203 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -44,6 +44,7 @@ import jupyterhub.services.service from . import mocking from .. import crypto from .. import orm +from ..roles import mock_roles from ..utils import random_port from .mocking import MockHub from .test_services import mockservice_cmd @@ -245,6 +246,7 @@ def _mockservice(request, app, url=False): ): app.services = [spec] app.init_services() + mock_roles(app, name, 'services') assert name in app._service_map service = app._service_map[name] diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index d2ccbba4..df2dd302 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -1446,7 +1446,7 @@ async def test_groups_list(app): r = await api_request(app, 'groups') r.raise_for_status() reply = r.json() - assert reply == [{'kind': 'group', 'name': 'alphaflight', 'users': []}] + assert reply == [{'kind': 'group', 'name': 'alphaflight', 'users': [], 'roles': []}] @mark.group @@ -1481,7 +1481,12 @@ async def test_group_get(app): r = await api_request(app, 'groups/alphaflight') r.raise_for_status() reply = r.json() - assert reply == {'kind': 'group', 'name': 'alphaflight', 'users': ['sasquatch']} + assert reply == { + 'kind': 'group', + 'name': 'alphaflight', + 'users': ['sasquatch'], + 'roles': [], + } @mark.group @@ -1594,7 +1599,7 @@ async def test_get_services(app, mockservice_url): mockservice.name: { 'name': mockservice.name, 'admin': True, - 'roles': [], + 'roles': ['admin'], 'command': mockservice.command, 'pid': mockservice.proc.pid, 'prefix': mockservice.server.base_url, @@ -1620,7 +1625,7 @@ async def test_get_service(app, mockservice_url): assert service == { 'name': mockservice.name, 'admin': True, - 'roles': [], + 'roles': ['admin'], 'command': mockservice.command, 'pid': mockservice.proc.pid, 'prefix': mockservice.server.base_url, diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index bdc8b98a..90c108fd 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -26,6 +26,18 @@ def test_orm_roles(db): db.add(service_role) db.commit() + group_role = orm.Role(name='group', scopes=['read:users']) + db.add(group_role) + db.commit() + + group_role = orm.Role(name='group', scopes=['read:users']) + db.add(group_role) + db.commit() + + group_role = orm.Role(name='group', scopes=['read:users']) + db.add(group_role) + db.commit() + user = orm.User(name='falafel') db.add(user) db.commit() @@ -34,20 +46,30 @@ def test_orm_roles(db): db.add(service) db.commit() + group = orm.Group(name='fast-food') + db.add(group) + db.commit() + assert user_role.users == [] assert user_role.services == [] + assert user_role.groups == [] assert service_role.users == [] assert service_role.services == [] + assert service_role.groups == [] assert user.roles == [] assert service.roles == [] + assert group.roles == [] user_role.users.append(user) service_role.services.append(service) + group_role.groups.append(group) db.commit() assert user_role.users == [user] assert user.roles == [user_role] assert service_role.services == [service] assert service.roles == [service_role] + assert group_role.groups == [group] + assert group.roles == [group_role] # check token creation without specifying its role # assigns it the default 'user' role @@ -67,16 +89,22 @@ def test_orm_roles(db): db.commit() assert user_role.users == [] assert user_token not in user_role.tokens - # check deleting the service token removes it from 'service' role + # 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 + # check deleting the service_role removes it from service.roles db.delete(service_role) db.commit() assert service.roles == [] + # check deleting the group removes it from group_roles + db.delete(group) + db.commit() + assert group_role.groups == [] + # clean up db db.delete(service) + db.delete(group_role) db.commit() @@ -310,6 +338,76 @@ async def test_load_roles_services(tmpdir, request): db.commit() +@mark.role +async def test_load_roles_groups(tmpdir, request): + """Test loading predefined roles for groups in app.py""" + groups_to_load = { + 'group1': ['gandalf'], + 'group2': ['bilbo', 'gargamel'], + 'group3': ['cyclops'], + } + roles_to_load = [ + { + 'name': 'assistant', + 'description': 'Access users information only', + 'scopes': ['read:users'], + 'groups': ['group2'], + }, + { + 'name': 'head', + 'description': 'Whole user access', + 'scopes': ['users', 'admin:users'], + 'groups': ['group3'], + }, + ] + kwargs = {'load_groups': groups_to_load, 'load_roles': roles_to_load} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) + hub.init_db() + db = hub.db + hub.authenticator.allowed_users = ['cyclops', 'gandalf', 'bilbo', 'gargamel'] + await hub.init_users() + await hub.init_groups() + await hub.init_roles() + + assist_role = orm.Role.find(db, name='assistant') + head_role = orm.Role.find(db, name='head') + user_role = orm.Role.find(db, name='user') + + group1 = orm.Group.find(db, name='group1') + group2 = orm.Group.find(db, name='group2') + group3 = orm.Group.find(db, name='group3') + + gandalf = orm.User.find(db, name='gandalf') + bilbo = orm.User.find(db, name='bilbo') + gargamel = orm.User.find(db, name='gargamel') + cyclops = orm.User.find(db, name='cyclops') + + # test group roles + assert group1.roles == [] + assert group2 in assist_role.groups + assert group3 in head_role.groups + + # test group members' roles + assert assist_role in bilbo.roles + assert assist_role in gargamel.roles + assert head_role in cyclops.roles + + # check the default user_role assignment + # FIXME - should users with group roles still have default? + assert user_role in gandalf.roles + assert user_role not in bilbo.roles + assert user_role not in gargamel.roles + assert user_role not in cyclops.roles + + +# FIXME +# @mark.role +# async def test_group_roles_delete_cascade(tmpdir, request): + + @mark.role async def test_load_roles_tokens(tmpdir, request): services = [ From 1a513f8dd966873b8e4495e5121c5da41db45087 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Tue, 5 Jan 2021 15:50:01 +0100 Subject: [PATCH 02/10] added roles to groups --- docs/rest-api.yml | 9 ++++ jupyterhub/app.py | 19 ++----- jupyterhub/roles.py | 88 ++++++++++++++++++++----------- jupyterhub/tests/test_roles.py | 64 +++++++++------------- jupyterhub/tests/test_services.py | 1 + 5 files changed, 98 insertions(+), 83 deletions(-) diff --git a/docs/rest-api.yml b/docs/rest-api.yml index 869fa85b..22127456 100644 --- a/docs/rest-api.yml +++ b/docs/rest-api.yml @@ -41,6 +41,7 @@ securityDefinitions: read:groups: Read-only access to groups admin:groups: Grants access to create/delete groups read:services: Read-only access to services + read:hub: Read-only access to detailed information about JupyterHub proxy: Grants access to proxy's routing table, syncing and notifying about a new proxy shutdown: Grants access to shutdown the Hub security: # global security, do we want to keep only the apiKey (token: []), change to only oauth2 (with scope all) or have both (either can be used)? @@ -71,6 +72,9 @@ paths: /info: get: summary: Get detailed info about JupyterHub + security: + - oauth2: + - read:hub description: | Detailed JupyterHub information, including Python version, JupyterHub's version and executable path, @@ -983,6 +987,11 @@ definitions: description: The names of users who are members of this group items: type: string + roles: + type: array + description: The names of roles this group has + items: + type: string Service: type: object properties: diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 28c70e29..98565f92 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1828,7 +1828,7 @@ class JupyterHub(Application): async def init_roles(self): """Load default and predefined roles into the database""" db = self.db - role_bearers = ['groups', 'users', 'services', 'tokens'] + role_bearers = ['users', 'services', 'tokens', 'groups'] # load default roles default_roles = roles.get_default_roles() @@ -1838,7 +1838,7 @@ class JupyterHub(Application): # load predefined roles from config file for predef_role in self.load_roles: roles.add_role(db, predef_role) - # add users, services and/or tokens + # add users, services, tokens and/or groups for bearer in role_bearers: if bearer in predef_role.keys(): for bname in predef_role[bearer]: @@ -1857,20 +1857,9 @@ class JupyterHub(Application): db, objname=bname, kind=bearer, rolename=predef_role['name'] ) - # make sure all users, services and tokens have at least one role (update with default) - # groups can be without a role but all group members should have the same role(s) as the group + # make sure role bearers have at least a default role for bearer in role_bearers: - Class = roles.get_orm_class(bearer) - if bearer == 'groups': - for group in db.query(Class): - for user in group.users: - rolenames = roles.get_rolenames(group.roles) - roles.update_roles(db, obj=user, kind='users', roles=rolenames) - else: - for obj in db.query(Class): - if len(obj.roles) < 1: - roles.update_roles(db, obj=obj, kind=bearer) - db.commit() + roles.check_for_default_roles(db, bearer) async def _add_tokens(self, token_dict, kind): """Add tokens for users or services to the database""" diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 6291a3ce..f8943558 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -3,6 +3,8 @@ # Distributed under the terms of the Modified BSD License. from itertools import chain +from sqlalchemy import func + from . import orm @@ -36,7 +38,7 @@ def get_default_roles(): { 'name': 'server', 'description': 'Post activity only', - 'scopes': ['users:activity!user=username'], + 'scopes': ['users:activity'], }, ] return default_roles @@ -157,16 +159,6 @@ def get_orm_class(kind): return Class -def get_rolenames(role_list): - - """Return a list of rolenames from a list of roles""" - - rolenames = [] - for role in role_list: - rolenames.append(role.name) - return rolenames - - def existing_only(func): """Decorator for checking if objects and roles exist""" @@ -228,6 +220,32 @@ def switch_default_role(db, obj, kind, admin): add_and_remove(db, obj, kind, admin_role, user_role) +def check_token_roles(db, token, role): + + """Returns a set of token scopes from its roles and a set of + token's owner scopes from their roles and their group roles""" + + token_scopes = get_subscopes(role) + owner = None + roles_to_check = [] + + # find the owner and their roles + if token.user_id: + owner = db.query(orm.User).get(token.user_id) + roles_to_check.extend(owner.roles) + # if user is a member of any groups, include the groups' roles as well + for group in owner.groups: + roles_to_check.extend(group.roles) + + elif token.service_id: + owner = db.query(orm.Service).get(token.service_id) + roles_to_check = owner.roles + + owner_scopes = get_subscopes(*roles_to_check) + + return token_scopes, owner_scopes + + def update_roles(db, obj, kind, roles=None): """Updates object's roles if specified, @@ -235,30 +253,21 @@ def update_roles(db, obj, kind, roles=None): Class = get_orm_class(kind) user_role = orm.Role.find(db, 'user') + admin_role = orm.Role.find(db, 'admin') if roles: for rolename in roles: if Class == orm.APIToken: - role = orm.Role.find(db, rolename) if role: - # compare the requested role permissions with the owner's permissions (scopes) - token_scopes = get_subscopes(role) - # find the owner and their roles - owner = None - if obj.user_id: - owner = db.query(orm.User).get(obj.user_id) - elif obj.service_id: - owner = db.query(orm.Service).get(obj.service_id) - if owner: - owner_scopes = get_subscopes(*owner.roles) - if token_scopes.issubset(owner_scopes): - role.tokens.append(obj) - else: - raise ValueError( - 'Requested token role %r has higher permissions than the token owner' - % rolename - ) + token_scopes, owner_scopes = check_token_roles(db, obj, role) + if token_scopes.issubset(owner_scopes): + role.tokens.append(obj) + else: + raise ValueError( + 'Requested token role %r has higher permissions than the token owner' + % rolename + ) else: raise NameError('Role %r does not exist' % rolename) else: @@ -272,12 +281,31 @@ def update_roles(db, obj, kind, roles=None): elif Class == orm.APIToken: if len(obj.roles) < 1 and obj.user is not None: user_role.tokens.append(obj) - db.commit() + db.commit() # users and services can have 'user' or 'admin' roles as default else: switch_default_role(db, obj, kind, obj.admin) +def check_for_default_roles(db, bearer): + + """Checks that all role bearers have at least one role (default if none). + Groups can be without a role""" + + Class = get_orm_class(bearer) + if Class == orm.Group: + pass + else: + for obj in ( + db.query(Class) + .outerjoin(orm.Role, Class.roles) + .group_by(Class.id) + .having(func.count(orm.Role.id) == 0) + ): + update_roles(db, obj=obj, kind=bearer) + db.commit() + + def mock_roles(app, name, kind): """Loads and assigns default roles for mocked objects""" diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 90c108fd..06e9fff1 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -30,14 +30,6 @@ def test_orm_roles(db): db.add(group_role) db.commit() - group_role = orm.Role(name='group', scopes=['read:users']) - db.add(group_role) - db.commit() - - group_role = orm.Role(name='group', scopes=['read:users']) - db.add(group_role) - db.commit() - user = orm.User(name='falafel') db.add(user) db.commit() @@ -367,46 +359,21 @@ async def test_load_roles_groups(tmpdir, request): hub = MockHub(**kwargs) hub.init_db() db = hub.db - hub.authenticator.allowed_users = ['cyclops', 'gandalf', 'bilbo', 'gargamel'] - await hub.init_users() await hub.init_groups() await hub.init_roles() assist_role = orm.Role.find(db, name='assistant') head_role = orm.Role.find(db, name='head') - user_role = orm.Role.find(db, name='user') group1 = orm.Group.find(db, name='group1') group2 = orm.Group.find(db, name='group2') group3 = orm.Group.find(db, name='group3') - gandalf = orm.User.find(db, name='gandalf') - bilbo = orm.User.find(db, name='bilbo') - gargamel = orm.User.find(db, name='gargamel') - cyclops = orm.User.find(db, name='cyclops') - # test group roles assert group1.roles == [] assert group2 in assist_role.groups assert group3 in head_role.groups - # test group members' roles - assert assist_role in bilbo.roles - assert assist_role in gargamel.roles - assert head_role in cyclops.roles - - # check the default user_role assignment - # FIXME - should users with group roles still have default? - assert user_role in gandalf.roles - assert user_role not in bilbo.roles - assert user_role not in gargamel.roles - assert user_role not in cyclops.roles - - -# FIXME -# @mark.role -# async def test_group_roles_delete_cascade(tmpdir, request): - @mark.role async def test_load_roles_tokens(tmpdir, request): @@ -465,16 +432,37 @@ async def test_load_roles_tokens(tmpdir, request): @mark.parametrize( "headers, role_list, status", [ + # no role requested - gets default 'user' role ({}, None, 200), - ({}, ['reader'], 200), + # role scopes within the user's default 'user' role + ({}, ['self-reader'], 200), + # role scopes outside of the user's role but within the group's role scopes of which the user is a member + ({}, ['users-reader'], 200), + # non-existing role request ({}, ['non-existing'], 404), - ({}, ['user_creator'], 403), + # role scopes outside of both user's role and group's role scopes + ({}, ['users-creator'], 403), ], ) async def test_get_new_token_via_api(app, headers, role_list, status): user = add_user(app.db, app, name='user') - roles.add_role(app.db, {'name': 'reader', 'scopes': ['read:all']}) - roles.add_role(app.db, {'name': 'user_creator', 'scopes': ['admin:users']}) + + roles.add_role(app.db, {'name': 'self-reader', 'scopes': ['read:all']}) + roles.add_role(app.db, {'name': 'users-reader', 'scopes': ['read:users']}) + roles.add_role(app.db, {'name': 'users-creator', 'scopes': ['admin:users']}) + # add role for a group + roles.add_role(app.db, {'name': 'group_role', 'scopes': ['read:users']}) + + # create a group and add the user and group_role + group = orm.Group.find(app.db, 'test_group') + if not group: + group = orm.Group(name='test_group') + app.db.add(group) + group.users.append(user) + group_role = orm.Role.find(app.db, 'group_role') + group.roles.append(group_role) + app.db.commit() + if role_list: body = json.dumps({'roles': role_list}) else: @@ -493,7 +481,7 @@ async def test_get_new_token_via_api(app, headers, role_list, status): if not role_list: assert reply['roles'] == ['user'] else: - assert reply['roles'] == ['reader'] + assert reply['roles'] == role_list token_id = reply['id'] # delete the token diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py index fd46dd0f..9f42d0dd 100644 --- a/jupyterhub/tests/test_services.py +++ b/jupyterhub/tests/test_services.py @@ -90,6 +90,7 @@ async def test_external_service(app): await maybe_future(app.init_services()) await app.init_api_tokens() await app.proxy.add_all_services(app._service_map) + await app.init_roles() service = app._service_map[name] url = public_url(app, service) + '/api/users' From f90b4e13df61a1f318ad6378221bda570bb0333c Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Fri, 15 Jan 2021 15:32:58 +0100 Subject: [PATCH 03/10] added token role check during loading config file and logs for role creation/changes/assignements --- jupyterhub/app.py | 13 +- jupyterhub/roles.py | 75 ++++++++++-- jupyterhub/tests/conftest.py | 2 +- jupyterhub/tests/test_roles.py | 210 ++++++++++++++++++++++++++++----- 4 files changed, 255 insertions(+), 45 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 98565f92..51b081cb 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1828,17 +1828,20 @@ class JupyterHub(Application): async def init_roles(self): """Load default and predefined roles into the database""" db = self.db - role_bearers = ['users', 'services', 'tokens', 'groups'] + role_bearers = ['users', 'services', 'groups'] # load default roles + self.log.debug('Loading default roles to database') default_roles = roles.get_default_roles() for role in default_roles: roles.add_role(db, role) # load predefined roles from config file + self.log.debug('Loading predefined roles from config file to database') for predef_role in self.load_roles: roles.add_role(db, predef_role) - # add users, services, tokens and/or groups + # add users, services, and/or groups, + # tokens need to be checked for permissions for bearer in role_bearers: if bearer in predef_role.keys(): for bname in predef_role[bearer]: @@ -1861,6 +1864,12 @@ class JupyterHub(Application): for bearer in role_bearers: roles.check_for_default_roles(db, bearer) + # now add roles to tokens if their owner's permissions allow + roles.add_predef_roles_tokens(db, self.load_roles) + + # 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/roles.py b/jupyterhub/roles.py index f8943558..b943749e 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -4,6 +4,7 @@ from itertools import chain from sqlalchemy import func +from tornado.log import app_log from . import orm @@ -122,8 +123,10 @@ def add_role(db, role_dict): """Adds a new role to database or modifies an existing one""" + default_roles = get_default_roles() + if 'name' not in role_dict.keys(): - raise ValueError('Role must have a name') + raise ValueError('Role definition in config file must have a name!') else: name = role_dict['name'] role = orm.Role.find(db, name) @@ -134,11 +137,17 @@ def add_role(db, role_dict): if role is None: role = orm.Role(name=name, description=description, scopes=scopes) db.add(role) + if role_dict not in default_roles: + app_log.info('Adding role %s to database', name) else: if description: - role.description = description + if role.description != description: + app_log.info('Changing role %s description to %s', name, description) + role.description = description if scopes: - role.scopes = scopes + if role.scopes != scopes: + app_log.info('Changing role %s scopes to %s', name, scopes) + role.scopes = scopes db.commit() @@ -184,9 +193,15 @@ def add_obj(db, objname, kind, rolename): """Adds a role for users, services, tokens or groups""" + if kind == 'tokens': + log_objname = objname + else: + log_objname = objname.name + if rolename not in objname.roles: objname.roles.append(rolename) db.commit() + app_log.info('Adding role %s for %s: %s', rolename.name, kind[:-1], log_objname) @existing_only @@ -194,9 +209,17 @@ def remove_obj(db, objname, kind, rolename): """Removes a role for users, services or tokens""" + if kind == 'tokens': + log_objname = objname + else: + log_objname = objname.name + if rolename in objname.roles: objname.roles.remove(rolename) db.commit() + app_log.info( + 'Removing role %s for %s: %s', rolename.name, kind[:-1], log_objname + ) def switch_default_role(db, obj, kind, admin): @@ -260,36 +283,67 @@ def update_roles(db, obj, kind, roles=None): if Class == orm.APIToken: role = orm.Role.find(db, rolename) if role: + app_log.debug( + 'Checking token permissions against requested role %s', rolename + ) token_scopes, owner_scopes = check_token_roles(db, obj, role) if token_scopes.issubset(owner_scopes): role.tokens.append(obj) + app_log.info( + 'Adding role %s for %s: %s', role.name, kind[:-1], obj + ) else: raise ValueError( - 'Requested token role %r has higher permissions than the token owner' - % rolename + 'Requested token role %r of %r with scopes %r has higher permissions than the owner scopes %r' + % (rolename, obj, token_scopes, owner_scopes) ) else: raise NameError('Role %r does not exist' % rolename) else: add_obj(db, objname=obj.name, kind=kind, rolename=rolename) else: + # CHECK ME - Does the default role assignment here make sense? + # groups can be without a role if Class == orm.Group: pass # tokens can have only 'user' role as default # assign the default only for user tokens + # service tokens with no specified role remain without any role (no default) elif Class == orm.APIToken: + app_log.debug('Assigning default roles to tokens') if len(obj.roles) < 1 and obj.user is not None: user_role.tokens.append(obj) db.commit() + app_log.info('Adding role %s to token %s', 'user', obj) # users and services can have 'user' or 'admin' roles as default else: + app_log.debug('Assigning default roles to %s', kind) switch_default_role(db, obj, kind, obj.admin) +def add_predef_roles_tokens(db, predef_roles): + + """Adds tokens to predefined roles in config file + if their permissions allow""" + + for predef_role in predef_roles: + if 'tokens' in predef_role.keys(): + token_role = orm.Role.find(db, name=predef_role['name']) + for token_name in predef_role['tokens']: + token = orm.APIToken.find(db, token_name) + if token is None: + raise ValueError( + "Token %r does not exist and cannot assign it to role %r" + % (token_name, token_role.name) + ) + else: + update_roles(db, obj=token, kind='tokens', roles=[token_role.name]) + + def check_for_default_roles(db, bearer): - """Checks that all role bearers have at least one role (default if none). + """Checks that role bearers have at least one role (default if none). Groups can be without a role""" Class = get_orm_class(bearer) @@ -306,14 +360,15 @@ def check_for_default_roles(db, bearer): db.commit() -def mock_roles(app, name, kind): +def mock_roles(db, name, kind): """Loads and assigns default roles for mocked objects""" Class = get_orm_class(kind) - obj = Class.find(app.db, name=name) + obj = Class.find(db, name=name) default_roles = get_default_roles() for role in default_roles: - add_role(app.db, role) - update_roles(db=app.db, obj=obj, kind=kind) + add_role(db, role) + app_log.info('Assigning default roles to mocked %s: %s', kind[:-1], name) + update_roles(db=db, obj=obj, kind=kind) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 45bfd203..93b6b77c 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -246,7 +246,7 @@ def _mockservice(request, app, url=False): ): app.services = [spec] app.init_services() - mock_roles(app, name, 'services') + mock_roles(app.db, name, 'services') assert name in app._service_map service = app._service_map[name] diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 06e9fff1..3082cd24 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -277,8 +277,13 @@ async def test_load_roles_services(tmpdir, request): services = [ {'name': 'cull_idle', 'api_token': 'some-token'}, {'name': 'user_service', 'api_token': 'some-other-token'}, - {'name': 'admin_service', 'api_token': 'secret-token', 'admin': True}, + {'name': 'admin_service', 'api_token': 'secret-token'}, ] + service_tokens = { + 'some-token': 'cull_idle', + 'some-other-token': 'user_service', + 'secret-token': 'admin_service', + } roles_to_load = [ { 'name': 'culler', @@ -287,24 +292,21 @@ async def test_load_roles_services(tmpdir, request): 'services': ['cull_idle'], }, ] - kwargs = {'load_roles': roles_to_load} + kwargs = { + 'load_roles': roles_to_load, + 'services': services, + 'service_tokens': service_tokens, + } ssl_enabled = getattr(request.module, "ssl_enabled", False) if ssl_enabled: kwargs['internal_certs_location'] = str(tmpdir) hub = MockHub(**kwargs) hub.init_db() db = hub.db - # clean db of previous services and add testing ones - for service in db.query(orm.Service): - db.delete(service) - db.commit() - for service in services: - orm_service = orm.Service.find(db, name=service['name']) - if orm_service is None: - # not found, create a new one - orm_service = orm.Service(name=service['name']) - db.add(orm_service) - orm_service.admin = service.get('admin', False) + await hub.init_api_tokens() + # make 'admin_service' admin + admin_service = orm.Service.find(db, 'admin_service') + admin_service.admin = True db.commit() await hub.init_roles() @@ -329,6 +331,11 @@ async def test_load_roles_services(tmpdir, request): db.delete(service) db.commit() + # delete the test tokens + for token in db.query(orm.APIToken): + db.delete(token) + db.commit() + @mark.role async def test_load_roles_groups(tmpdir, request): @@ -376,30 +383,23 @@ async def test_load_roles_groups(tmpdir, request): @mark.role -async def test_load_roles_tokens(tmpdir, request): - services = [ - {'name': 'cull_idle', 'admin': True, 'api_token': 'another-secret-token'} - ] +async def test_load_roles_user_tokens(tmpdir, request): user_tokens = { 'secret-token': 'cyclops', + 'secrety-token': 'gandalf', 'super-secret-token': 'admin', } - service_tokens = { - 'another-secret-token': 'cull_idle', - } roles_to_load = [ { - 'name': 'culler', - 'description': 'Cull idle servers', - 'scopes': ['users:servers', 'admin:servers'], - 'tokens': ['another-secret-token'], + 'name': 'reader', + 'description': 'Read-only own model', + 'scopes': ['read:all'], + 'tokens': ['secrety-token'], }, ] kwargs = { 'load_roles': roles_to_load, - 'services': services, 'api_tokens': user_tokens, - 'service_tokens': service_tokens, } ssl_enabled = getattr(request.module, "ssl_enabled", False) if ssl_enabled: @@ -413,12 +413,10 @@ async def test_load_roles_tokens(tmpdir, request): await hub.init_api_tokens() await hub.init_roles() - # test if another-secret-token has culler role - service = orm.Service.find(db, 'cull_idle') - culler_role = orm.Role.find(db, 'culler') - token = orm.APIToken.find(db, 'another-secret-token') - assert len(token.roles) == 1 - assert culler_role in token.roles + # test if gandalf's token has the 'reader' role + reader_role = orm.Role.find(db, 'reader') + token = orm.APIToken.find(db, 'secrety-token') + assert reader_role in token.roles # test if all other tokens have default 'user' role user_role = orm.Role.find(db, 'user') @@ -427,6 +425,154 @@ async def test_load_roles_tokens(tmpdir, request): s_sec_token = orm.APIToken.find(db, 'super-secret-token') assert user_role in s_sec_token.roles + # delete the test tokens + for token in db.query(orm.APIToken): + db.delete(token) + db.commit() + + +@mark.role +async def test_load_roles_user_tokens_not_allowed(tmpdir, request): + user_tokens = { + 'secret-token': 'bilbo', + } + roles_to_load = [ + { + 'name': 'user-reader', + 'description': 'Read-only any user model', + 'scopes': ['read:users'], + 'tokens': ['secret-token'], + }, + ] + kwargs = { + 'load_roles': roles_to_load, + 'api_tokens': user_tokens, + } + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) + hub.init_db() + db = hub.db + hub.authenticator.allowed_users = ['bilbo'] + await hub.init_users() + await hub.init_api_tokens() + + response = 'allowed' + # bilbo has only default 'user' role + # while bilbo's token is requesting role with higher permissions + try: + await hub.init_roles() + except ValueError: + response = 'denied' + + assert response == 'denied' + + # delete the test tokens + for token in db.query(orm.APIToken): + db.delete(token) + db.commit() + + +@mark.role +async def test_load_roles_service_tokens(tmpdir, request): + services = [{'name': 'cull_idle', 'api_token': 'another-secret-token'}] + service_tokens = { + 'another-secret-token': 'cull_idle', + } + roles_to_load = [ + { + 'name': 'culler', + 'description': 'Cull idle servers', + 'scopes': ['users:servers', 'admin:users:servers'], + 'tokens': ['another-secret-token'], + }, + ] + kwargs = { + 'load_roles': roles_to_load, + 'services': services, + 'service_tokens': service_tokens, + } + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) + hub.init_db() + db = hub.db + await hub.init_api_tokens() + # make the service admin + service = orm.Service.find(db, 'cull_idle') + service.admin = True + await hub.init_roles() + + # test if another-secret-token has culler role + culler_role = orm.Role.find(db, 'culler') + token = orm.APIToken.find(db, 'another-secret-token') + assert len(token.roles) == 1 + assert culler_role in token.roles + + # delete the test services + for service in db.query(orm.Service): + db.delete(service) + db.commit() + + # delete the test tokens + for token in db.query(orm.APIToken): + db.delete(token) + db.commit() + + +@mark.role +async def test_load_roles_service_tokens_not_allowed(tmpdir, request): + services = [{'name': 'some-service', 'api_token': 'secret-token'}] + service_tokens = { + 'secret-token': 'some-service', + } + roles_to_load = [ + { + 'name': 'user-reader', + 'description': 'Read-only user models', + 'scopes': ['read:users'], + 'services': ['some-service'], + }, + # 'culler' role has higher permissions that the token's owner 'some-service' + { + 'name': 'culler', + 'description': 'Cull idle servers', + 'scopes': ['users:servers', 'admin:users:servers'], + 'tokens': ['secret-token'], + }, + ] + kwargs = { + 'load_roles': roles_to_load, + 'services': services, + 'service_tokens': service_tokens, + } + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) + hub.init_db() + db = hub.db + await hub.init_api_tokens() + response = 'allowed' + try: + await hub.init_roles() + except ValueError: + response = 'denied' + + assert response == 'denied' + + # delete the test services + for service in db.query(orm.Service): + db.delete(service) + db.commit() + + # delete the test tokens + for token in db.query(orm.APIToken): + db.delete(token) + db.commit() + @mark.role @mark.parametrize( From 39fc501d5018264c64cc1a38bf316d02c0d2fb61 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Tue, 9 Mar 2021 16:38:43 +0100 Subject: [PATCH 04/10] Add warnings and errors when creating new roles --- jupyterhub/roles.py | 30 ++++++++++++++++++++----- jupyterhub/tests/test_roles.py | 41 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index c46f7e19..4f024249 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -3,6 +3,8 @@ # Distributed under the terms of the Modified BSD License. from itertools import chain +from tornado.log import app_log + from . import orm @@ -35,7 +37,7 @@ def get_default_roles(): { 'name': 'server', 'description': 'Post activity only', - 'scopes': ['users:activity!user=username'], + 'scopes': ['users:activity'], }, ] return default_roles @@ -114,9 +116,8 @@ def get_subscopes(*args): def add_role(db, role_dict): """Adds a new role to database or modifies an existing one""" - if 'name' not in role_dict.keys(): - raise ValueError('Role must have a name') + raise KeyError('Role definition must have a name') else: name = role_dict['name'] role = orm.Role.find(db, name) @@ -126,15 +127,34 @@ def add_role(db, role_dict): if role is None: role = orm.Role(name=name, description=description, scopes=scopes) + if not scopes: + app_log.warning('Warning: New defined role %s has no scopes', name) db.add(role) else: if description: - role.description = description + if role.name == 'admin' and description != role.description: + raise AttributeError('admin role description cannot be overwritten') + else: + role.description = description if scopes: - role.scopes = scopes + if role.name == 'admin' and scopes != role.scopes: + raise AttributeError('admin role scopes cannot be overwritten') + else: + role.scopes = scopes db.commit() +def remove_role(db, rolename): + """Removes a role from database""" + + role = orm.Role.find(db, rolename) + if role: + db.delete(role) + db.commit() + else: + raise NameError('Cannot remove role %r that does not exist', rolename) + + def existing_only(func): """Decorator for checking if objects and roles exist""" diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index bdc8b98a..854578b1 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -3,7 +3,9 @@ # Distributed under the terms of the Modified BSD License. import json +import pytest from pytest import mark +from tornado.log import app_log from .. import orm from .. import roles @@ -183,6 +185,7 @@ def test_get_subscopes(db, scopes, subscopes): db.delete(role) +@mark.role async def test_load_default_roles(tmpdir, request): """Test loading default roles in app.py""" kwargs = {} @@ -199,6 +202,44 @@ async def test_load_default_roles(tmpdir, request): assert orm.Role.find(db, 'server') is not None +@mark.role +@mark.parametrize( + "role, role_def, response_type, response", + [ + ('no_name', {'scopes': ['users']}, 'error', KeyError), + ( + 'no_scopes', + {'name': 'no-permissions'}, + 'warning', + app_log.warning('Warning: New defined role no-permissions has no scopes'), + ), + ( + 'admin', + {'name': 'admin', 'scopes': ['admin:users']}, + 'error', + AttributeError, + ), + ], +) +async def test_adding_new_roles_raise_errors( + tmpdir, request, role, role_def, response_type, response +): + """Test raising errors and warnings when creating new roles""" + kwargs = {'load_roles': [role_def]} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) + hub.init_db() + db = hub.db + if response_type == 'error': + with pytest.raises(response): + await hub.init_roles() + elif response_type == 'warning': + with pytest.warns(response): + await hub.init_roles() + + @mark.role async def test_load_roles_users(tmpdir, request): """Test loading predefined roles for users in app.py""" From 01f32866204e712bf2117c6d3dc2c3af82e5681e Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Thu, 11 Mar 2021 15:30:11 +0100 Subject: [PATCH 05/10] Add check that scopes exists when adding new/modifying existing role --- jupyterhub/roles.py | 59 ++++++++++++++---- jupyterhub/tests/test_roles.py | 106 +++++++++++++++++++++++++++------ 2 files changed, 137 insertions(+), 28 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 4f024249..556c343c 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -114,8 +114,48 @@ def get_subscopes(*args): return scopes +def check_scopes(*args): + """Check if provided scopes exist""" + + allowed_scopes = get_scopes() + allowed_filters = ['!user=', '!group=', '!server='] + subscopes = set( + chain.from_iterable([x for x in allowed_scopes.values() if x is not None]) + ) + + for scope in args: + # check the ! filters + if '!' in scope: + if any(filter in scope for filter in allowed_filters): + scope = scope.split('!', 1)[0] + else: + raise NameError( + 'Scope filter %r in scope %r does not exist', + scope.split('!', 1)[1], + scope, + ) + # check if the actual scope syntax exists + if scope not in allowed_scopes.keys() and scope not in subscopes: + raise NameError('Scope %r does not exist', scope) + + +def overwrite_role(role, role_dict): + """Overwrites role's description and/or scopes with role_dict if role not 'admin'""" + + for attr in role_dict.keys(): + if attr == 'description' or attr == 'scopes': + if role.name == 'admin' and role_dict[attr] != getattr(role, attr): + raise ValueError( + 'admin role description or scopes cannot be overwritten' + ) + else: + setattr(role, attr, role_dict[attr]) + app_log.info('Role %r %r attribute has been changed', role.name, attr) + + def add_role(db, role_dict): """Adds a new role to database or modifies an existing one""" + if 'name' not in role_dict.keys(): raise KeyError('Role definition must have a name') else: @@ -125,22 +165,19 @@ def add_role(db, role_dict): description = role_dict.get('description') scopes = role_dict.get('scopes') + # check if the provided scopes exist + if scopes: + check_scopes(*scopes) + if role is None: - role = orm.Role(name=name, description=description, scopes=scopes) if not scopes: app_log.warning('Warning: New defined role %s has no scopes', name) + + role = orm.Role(name=name, description=description, scopes=scopes) db.add(role) else: - if description: - if role.name == 'admin' and description != role.description: - raise AttributeError('admin role description cannot be overwritten') - else: - role.description = description - if scopes: - if role.name == 'admin' and scopes != role.scopes: - raise AttributeError('admin role scopes cannot be overwritten') - else: - role.scopes = scopes + overwrite_role(role, role_dict) + db.commit() diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 854578b1..1d2f60a1 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -2,6 +2,7 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import json +from itertools import chain import pytest from pytest import mark @@ -217,14 +218,27 @@ async def test_load_default_roles(tmpdir, request): 'admin', {'name': 'admin', 'scopes': ['admin:users']}, 'error', - AttributeError, + ValueError, + ), + ( + 'admin', + {'name': 'admin', 'description': 'New description'}, + 'error', + ValueError, + ), + ( + 'user', + {'name': 'user', 'scopes': ['read:users:name']}, + 'info', + app_log.info('Role user scopes attribute has been changed'), ), ], ) -async def test_adding_new_roles_raise_errors( +async def test_adding_new_roles( tmpdir, request, role, role_def, response_type, response ): """Test raising errors and warnings when creating new roles""" + kwargs = {'load_roles': [role_def]} ssl_enabled = getattr(request.module, "ssl_enabled", False) if ssl_enabled: @@ -232,14 +246,64 @@ async def test_adding_new_roles_raise_errors( hub = MockHub(**kwargs) hub.init_db() db = hub.db + if response_type == 'error': with pytest.raises(response): await hub.init_roles() - elif response_type == 'warning': + + elif response_type == 'warning' or response_type == 'info': with pytest.warns(response): await hub.init_roles() +@mark.role +@mark.parametrize( + "role, response", + [ + ( + { + 'name': 'test-scopes-1', + 'scopes': [ + 'users', + 'users!user=charlie', + 'admin:groups', + 'read:users:tokens', + ], + }, + '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), + ], +) +async def test_scope_existence(tmpdir, request, role, response): + """Test checking of scopes provided in role definitions""" + kwargs = {'load_roles': [role]} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) + hub.init_db() + db = hub.db + + if response == 'existing': + roles.add_role(db, role) + added_role = orm.Role.find(db, role['name']) + assert added_role is not None + assert added_role.scopes == role['scopes'] + + elif response == NameError: + with pytest.raises(response): + roles.add_role(db, role) + added_role = orm.Role.find(db, role['name']) + assert added_role is None + + # delete the tested roles + if added_role: + roles.remove_role(db, added_role.name) + + @mark.role async def test_load_roles_users(tmpdir, request): """Test loading predefined roles for users in app.py""" @@ -296,16 +360,21 @@ async def test_load_roles_users(tmpdir, request): @mark.role async def test_load_roles_services(tmpdir, request): services = [ - {'name': 'cull_idle', 'api_token': 'some-token'}, + {'name': 'idle-culler', 'api_token': 'some-token'}, {'name': 'user_service', 'api_token': 'some-other-token'}, {'name': 'admin_service', 'api_token': 'secret-token', 'admin': True}, ] roles_to_load = [ { - 'name': 'culler', + 'name': 'idle-culler', 'description': 'Cull idle servers', - 'scopes': ['users:servers', 'admin:servers'], - 'services': ['cull_idle'], + 'scopes': [ + 'read:users:name', + 'read:users:activity', + 'read:users:servers', + 'users:servers', + ], + 'services': ['idle-culler'], }, ] kwargs = {'load_roles': roles_to_load} @@ -340,8 +409,8 @@ async def test_load_roles_services(tmpdir, request): assert user_role not in service.roles # test if predefined roles loaded and assigned - culler_role = orm.Role.find(db, name='culler') - cull_idle = orm.Service.find(db, name='cull_idle') + culler_role = orm.Role.find(db, name='idle-culler') + cull_idle = orm.Service.find(db, name='idle-culler') assert culler_role in cull_idle.roles assert user_role not in cull_idle.roles @@ -353,21 +422,24 @@ async def test_load_roles_services(tmpdir, request): @mark.role async def test_load_roles_tokens(tmpdir, request): - services = [ - {'name': 'cull_idle', 'admin': True, 'api_token': 'another-secret-token'} - ] + services = [{'name': 'idle-culler', 'api_token': 'another-secret-token'}] user_tokens = { 'secret-token': 'cyclops', 'super-secret-token': 'admin', } service_tokens = { - 'another-secret-token': 'cull_idle', + 'another-secret-token': 'idle-culler', } roles_to_load = [ { - 'name': 'culler', + 'name': 'idle-culler', 'description': 'Cull idle servers', - 'scopes': ['users:servers', 'admin:servers'], + 'scopes': [ + 'read:users:name', + 'read:users:activity', + 'read:users:servers', + 'users:servers', + ], 'tokens': ['another-secret-token'], }, ] @@ -390,8 +462,8 @@ async def test_load_roles_tokens(tmpdir, request): await hub.init_roles() # test if another-secret-token has culler role - service = orm.Service.find(db, 'cull_idle') - culler_role = orm.Role.find(db, 'culler') + service = orm.Service.find(db, 'idle-culler') + culler_role = orm.Role.find(db, 'idle-culler') token = orm.APIToken.find(db, 'another-secret-token') assert len(token.roles) == 1 assert culler_role in token.roles From bdc7b3ab8d9d9653ce8e8b50b48d51d014297102 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Fri, 12 Mar 2021 16:09:23 +0100 Subject: [PATCH 06/10] Account for horizontal filtering in get_subscopes() --- jupyterhub/roles.py | 28 +++++++++++++++++++++++++++- jupyterhub/tests/test_roles.py | 21 +++++++++++++-------- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 556c343c..49a30b50 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -73,6 +73,22 @@ def get_scopes(): return scopes +def horizontal_filter(func): + """Decorator to account for horizontal filtering in scope syntax""" + + 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} + + return full_expanded_scope + + return ignore + + +@horizontal_filter def expand_scope(scopename): """Returns a set of all subscopes""" @@ -264,6 +280,11 @@ def update_roles(db, obj, kind, roles=None): if role: # compare the requested role permissions with the owner's permissions (scopes) token_scopes = get_subscopes(role) + # ignore horizontal filters for comparison + t_scopes = { + scope.split('!', 1)[0] if '!' in scope else scope + for scope in token_scopes + } # find the owner and their roles owner = None if obj.user_id: @@ -272,7 +293,12 @@ def update_roles(db, obj, kind, roles=None): owner = db.query(orm.Service).get(obj.service_id) if owner: owner_scopes = get_subscopes(*owner.roles) - if token_scopes.issubset(owner_scopes): + # ignore horizontal filters for comparison + o_scopes = { + scope.split('!', 1)[0] if '!' in scope else scope + for scope in owner_scopes + } + if t_scopes.issubset(o_scopes): role.tokens.append(obj) else: raise ValueError( diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 1d2f60a1..f00cdb61 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -21,7 +21,7 @@ def test_orm_roles(db): """Test orm roles setup""" user_role = orm.Role.find(db, name='user') if not user_role: - user_role = orm.Role(name='user', scopes=['all', 'read:all']) + user_role = orm.Role(name='user', scopes=['self']) db.add(user_role) db.commit() @@ -175,6 +175,10 @@ def test_orm_roles_delete_cascade(db): ), (['read:users:servers'], {'read:users:servers'}), (['admin:groups'], {'admin:groups'}), + ( + ['users:tokens!group=hobbits'], + {'users:tokens!group=hobbits', 'read:users:tokens!group=hobbits'}, + ), ], ) def test_get_subscopes(db, scopes, subscopes): @@ -316,9 +320,8 @@ async def test_load_roles_users(tmpdir, request): }, { 'name': 'user', - 'description': 'Only read access', - 'scopes': ['read:all'], - 'users': ['bilbo'], + 'description': 'Read access to users names', + 'scopes': ['read:users:name'], }, ] kwargs = {'load_roles': roles_to_load} @@ -333,12 +336,13 @@ async def test_load_roles_users(tmpdir, request): await hub.init_users() await hub.init_roles() - # test if the 'user' role has been overwritten and assigned + # test if the 'user' role has been overwritten user_role = orm.Role.find(db, 'user') - admin_role = orm.Role.find(db, 'admin') assert user_role is not None - assert user_role.scopes == ['read:all'] + assert user_role.description == roles_to_load[1]['description'] + assert user_role.scopes == roles_to_load[1]['scopes'] + admin_role = orm.Role.find(db, 'admin') # test if every user has a role (and no duplicates) # and admins have admin role for user in db.query(orm.User): @@ -504,8 +508,9 @@ async def test_get_new_token_via_api(app, headers, role_list, status): # check the new-token reply for roles reply = r.json() assert 'token' in reply - assert reply['user'] == 'user' + assert reply['user'] == user.name if not role_list: + # token should have a default role assert reply['roles'] == ['user'] else: assert reply['roles'] == ['reader'] From b6221f6cb11699af93cdab7ee373c1416fc12c9c Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Fri, 12 Mar 2021 17:40:38 +0100 Subject: [PATCH 07/10] Fix tests --- jupyterhub/roles.py | 2 +- jupyterhub/tests/test_roles.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index b30624ea..d008903d 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -135,7 +135,7 @@ def check_scopes(*args): """Check if provided scopes exist""" allowed_scopes = get_scopes() - allowed_filters = ['!user=', '!group=', '!server='] + allowed_filters = ['!user=', '!service=', '!group=', '!server='] subscopes = set( chain.from_iterable([x for x in allowed_scopes.values() if x is not None]) ) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 7d143f24..fed9418e 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -389,7 +389,7 @@ async def test_load_roles_services(tmpdir, request): {'name': 'admin_service', 'api_token': 'secret-token'}, ] service_tokens = { - 'some-token': 'cull_idle', + 'some-token': 'idle-culler', 'some-other-token': 'user_service', 'secret-token': 'admin_service', } @@ -620,9 +620,6 @@ async def test_load_roles_service_tokens(tmpdir, request): hub.init_db() db = hub.db await hub.init_api_tokens() - # make the service admin - service = orm.Service.find(db, 'cull_idle') - service.admin = True await hub.init_roles() # test if another-secret-token has culler role From 7d5fc27f7ceafcfb100869045a5372ae08afa7a7 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Tue, 16 Mar 2021 11:03:18 +0100 Subject: [PATCH 08/10] Make some funcs in roles.py private --- jupyterhub/roles.py | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index d008903d..23c2f5ae 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -90,13 +90,13 @@ def horizontal_filter(func): @horizontal_filter -def expand_scope(scopename): +def _expand_scope(scopename): """Returns a set of all subscopes""" scopes = get_scopes() subscopes = [scopename] - def expand_subscopes(index): + def _expand_subscopes(index): more_subscopes = list( filter(lambda scope: scope in scopes.keys(), subscopes[index:]) @@ -109,9 +109,9 @@ def expand_scope(scopename): # record the index from where it should check for "subscopes of sub-subscopes" index_for_sssc = len(subscopes) # check for "subscopes of subscopes" - expand_subscopes(index=1) + _expand_subscopes(index=1) # check for "subscopes of sub-subscopes" - expand_subscopes(index=index_for_sssc) + _expand_subscopes(index=index_for_sssc) expanded_scope = set(subscopes) @@ -126,12 +126,12 @@ def get_subscopes(*args): for role in args: scope_list.extend(role.scopes) - scopes = set(chain.from_iterable(list(map(expand_scope, scope_list)))) + scopes = set(chain.from_iterable(list(map(_expand_scope, scope_list)))) return scopes -def check_scopes(*args): +def _check_scopes(*args): """Check if provided scopes exist""" allowed_scopes = get_scopes() @@ -156,7 +156,7 @@ def check_scopes(*args): raise NameError('Scope %r does not exist', scope) -def overwrite_role(role, role_dict): +def _overwrite_role(role, role_dict): """Overwrites role's description and/or scopes with role_dict if role not 'admin'""" for attr in role_dict.keys(): @@ -186,7 +186,7 @@ def add_role(db, role_dict): # check if the provided scopes exist if scopes: - check_scopes(*scopes) + _check_scopes(*scopes) if role is None: if not scopes: @@ -197,7 +197,7 @@ def add_role(db, role_dict): if role_dict not in default_roles: app_log.info('Adding role %s to database', name) else: - overwrite_role(role, role_dict) + _overwrite_role(role, role_dict) db.commit() @@ -216,7 +216,7 @@ def remove_role(db, rolename): def existing_only(func): """Decorator for checking if objects and roles exist""" - def check_existence(db, objname, kind, rolename): + def _check_existence(db, objname, kind, rolename): Class = orm.get_class(kind) obj = Class.find(db, objname) @@ -229,7 +229,7 @@ def existing_only(func): else: func(db, obj, kind, role) - return check_existence + return _check_existence @existing_only @@ -264,13 +264,13 @@ def remove_obj(db, objname, kind, rolename): ) -def switch_default_role(db, obj, kind, admin): +def _switch_default_role(db, obj, kind, admin): """Switch between default user and admin roles for users/services""" user_role = orm.Role.find(db, 'user') admin_role = orm.Role.find(db, 'admin') - def add_and_remove(db, obj, kind, current_role, new_role): + def _add_and_remove(db, obj, kind, current_role, new_role): if current_role in obj.roles: remove_obj(db, objname=obj.name, kind=kind, rolename=current_role.name) @@ -279,12 +279,12 @@ def switch_default_role(db, obj, kind, admin): add_obj(db, objname=obj.name, kind=kind, rolename=new_role.name) if admin: - add_and_remove(db, obj, kind, user_role, admin_role) + _add_and_remove(db, obj, kind, user_role, admin_role) else: - add_and_remove(db, obj, kind, admin_role, user_role) + _add_and_remove(db, obj, kind, admin_role, user_role) -def token_allowed_role(db, token, role): +def _token_allowed_role(db, token, role): """Returns True if token allowed to have requested role through comparing the requested scopes with the set of token's owner scopes @@ -339,7 +339,7 @@ def update_roles(db, obj, kind, roles=None): 'Checking token permissions against requested role %s', rolename ) - if token_allowed_role(db, obj, role): + if _token_allowed_role(db, obj, role): role.tokens.append(obj) app_log.info( 'Adding role %s for %s: %s', role.name, kind[:-1], obj @@ -369,7 +369,7 @@ def update_roles(db, obj, kind, roles=None): # users and services can have 'user' or 'admin' roles as default else: app_log.debug('Assigning default roles to %s', kind) - switch_default_role(db, obj, kind, obj.admin) + _switch_default_role(db, obj, kind, obj.admin) def add_predef_roles_tokens(db, predef_roles): From e26e8f9c36657921c2f782aedca6ae303e0c8bd2 Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Tue, 23 Mar 2021 11:47:50 +0100 Subject: [PATCH 09/10] Prevent deleting default roles --- jupyterhub/roles.py | 6 ++++++ jupyterhub/tests/test_roles.py | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 23c2f5ae..1782afdd 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -205,10 +205,16 @@ def add_role(db, role_dict): def remove_role(db, rolename): """Removes a role from database""" + # default roles are not removable + default_roles = get_default_roles() + if any(role['name'] == rolename for role in default_roles): + raise ValueError('Default role %r cannot be removed', rolename) + role = orm.Role.find(db, rolename) if role: db.delete(role) db.commit() + app_log.info('Role %s has been deleted', rolename) else: raise NameError('Cannot remove role %r that does not exist', rolename) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index fed9418e..1423fd88 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -280,6 +280,41 @@ async def test_adding_new_roles( await hub.init_roles() +@mark.role +@mark.parametrize( + "role_type, rolename, response_type, response", + [ + ( + 'existing', + 'test-role1', + 'info', + app_log.info('Role user scopes attribute has been changed'), + ), + ('non-existing', 'test-role2', 'error', NameError), + ('default', 'user', 'error', ValueError), + ], +) +async def test_delete_roles(db, role_type, rolename, response_type, response): + """Test raising errors and info when deleting roles""" + + if response_type == 'info': + # add the role to db + test_role = orm.Role(name=rolename) + db.add(test_role) + db.commit() + check_role = orm.Role.find(db, rolename) + assert check_role is not None + # check the role is deleted and info raised + with pytest.warns(response): + roles.remove_role(db, rolename) + check_role = orm.Role.find(db, rolename) + assert check_role is None + + elif response_type == 'error': + with pytest.raises(response): + roles.remove_role(db, rolename) + + @mark.role @mark.parametrize( "role, response", From 933e4d555b5bcd8938a5d804df45a0add3b8b97d Mon Sep 17 00:00:00 2001 From: IvanaH8 Date: Tue, 6 Apr 2021 10:39:50 +0200 Subject: [PATCH 10/10] Add TO DO flag for users:activity scope in server role --- jupyterhub/roles.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jupyterhub/roles.py b/jupyterhub/roles.py index 4b5c2f2f..ec92d769 100644 --- a/jupyterhub/roles.py +++ b/jupyterhub/roles.py @@ -38,7 +38,9 @@ def get_default_roles(): { 'name': 'server', 'description': 'Post activity only', - 'scopes': ['users:activity'], + 'scopes': [ + 'users:activity' + ], # TO DO - fix scope to refer to only self once implemented }, { 'name': 'token',