From b7d68ca255717854a1d0a130fd4c53f8f32e21eb Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Tue, 26 Mar 2024 17:51:25 +0000 Subject: [PATCH] Implement `load_managed_roles`, allow to assign scopes and update roles (but not assign them to users/groups) by using `load_roles` when `Authenticator.manage_roles` is on. --- docs/source/reference/authenticators.md | 6 ++- jupyterhub/app.py | 14 +++++- jupyterhub/auth.py | 12 +++++ jupyterhub/tests/test_auth.py | 59 ++++++++++++++++++++++- jupyterhub/tests/test_roles.py | 62 +++++++++++++++++++++---- 5 files changed, 141 insertions(+), 12 deletions(-) diff --git a/docs/source/reference/authenticators.md b/docs/source/reference/authenticators.md index 8a6b90dd..cbd5966f 100644 --- a/docs/source/reference/authenticators.md +++ b/docs/source/reference/authenticators.md @@ -337,7 +337,11 @@ which is a list of roles that user should be assigned to: If authenticator-managed groups are enabled, all group-management via the API is disabled, -and roles cannot be specified with `load_roles` traitlet. +and roles cannot be assigned to groups nor users via `load_roles` traitlet +(roles can still be created via `load_roles` or assigned to services). + +When an authenticator manages roles, the initial roles and role assignments +can be loaded from role specifications returned by the `Authenticator.load_managed_roles()` method. ## pre_spawn_start and post_spawn_stop hooks diff --git a/jupyterhub/app.py b/jupyterhub/app.py index ac8365af..d7c9153a 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -2241,8 +2241,20 @@ class JupyterHub(Application): self.log.info(f"Defining {len(self.custom_scopes)} custom scopes.") scopes.define_custom_scopes(self.custom_scopes) + roles_to_load = self.load_roles + if self.authenticator.manage_roles and self.load_roles: - raise ValueError("Role management has been offloaded to the authenticator") + for role_spec in roles_to_load: + user_role_assignments = role_spec.get('users', []) + group_role_assignments = role_spec.get('groups', []) + if user_role_assignments or group_role_assignments: + raise ValueError( + "When authenticator manages roles, `load_roles` can not" + " be used for assigning roles to users nor groups." + ) + + if self.authenticator.manage_roles: + roles_to_load.extend(await self.authenticator.load_managed_roles()) self.log.debug('Loading roles into database') default_roles = roles.get_default_roles() diff --git a/jupyterhub/auth.py b/jupyterhub/auth.py index 53a44bf9..0f1f96a8 100644 --- a/jupyterhub/auth.py +++ b/jupyterhub/auth.py @@ -661,6 +661,18 @@ class Authenticator(LoggingConfigurable): Raising errors directly allows customizing the message shown to the user. """ + async def load_managed_roles(self): + """Load roles managed by authenticator. + + Returns a list of predefined role dictionaries to load at startup, + following the same format as `JupyterHub.load_roles` + """ + if not self.manage_roles: + raise ValueError( + 'Managed roles can only be loaded when `manage_roles` is True' + ) + return [] + def pre_spawn_start(self, user, spawner): """Hook called before spawning a user's server diff --git a/jupyterhub/tests/test_auth.py b/jupyterhub/tests/test_auth.py index 0a024b27..1958a702 100644 --- a/jupyterhub/tests/test_auth.py +++ b/jupyterhub/tests/test_auth.py @@ -13,7 +13,7 @@ from traitlets.config import Config from jupyterhub import auth, crypto, orm -from .mocking import MockPAMAuthenticator, MockStructGroup, MockStructPasswd +from .mocking import MockHub, MockPAMAuthenticator, MockStructGroup, MockStructPasswd from .utils import add_user, async_requests, get_page, public_url @@ -598,6 +598,7 @@ async def test_auth_managed_groups( class MockRolesAuthenticator(auth.Authenticator): authenticated_roles = Any() refresh_roles = Any() + initial_roles = Any() manage_roles = True def authenticate(self, handler, data): @@ -612,6 +613,62 @@ class MockRolesAuthenticator(auth.Authenticator): "roles": self.refresh_roles, } + async def load_managed_roles(self): + return self.initial_roles + + +def role_to_dict(role): + return {col: getattr(role, col) for col in role.__table__.columns.keys()} + + +@pytest.mark.parametrize( + "initial_roles", + [ + pytest.param( + [{'name': 'test-role'}], + id="should have the same effect as `load_roles` when creating a new role", + ), + pytest.param( + [ + { + 'name': 'server', + 'users': ['test-user'], + } + ], + id="should have the same effect as `load_roles` when assigning a role to a user", + ), + pytest.param( + [ + { + 'name': 'server', + 'groups': ['test-group'], + } + ], + id="should have the same effect as `load_roles` when assigning a role to a group", + ), + ], +) +async def test_auth_load_managed_roles(app, initial_roles): + authenticator = MockRolesAuthenticator( + parent=app, + initial_roles=initial_roles, + ) + + # create the roles using `load_roles` + hub = MockHub(load_roles=initial_roles) + hub.init_db() + await hub.init_role_creation() + expected_roles = [role_to_dict(role) for role in hub.db.query(orm.Role).all()] + + # create the roles using authenticator's `load_managed_roles` + hub = MockHub(load_roles=[], authenticator=authenticator) + hub.init_db() + await hub.init_role_creation() + actual_roles = [role_to_dict(role) for role in hub.db.query(orm.Role).all()] + + # `load_managed_roles` should produce the same set of roles as `load_roles` does + assert expected_roles == actual_roles + @pytest.mark.parametrize( "authenticated_roles", diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index af16d28f..e9a8613b 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -1273,21 +1273,65 @@ async def test_admin_role_membership(in_db, role_users, admin_users, expected_me assert role_members == expected_members -async def test_manage_roles_disallows_load_roles(): - roles_to_load = [ - { - 'name': 'elephant', - 'description': 'pacing about', - 'scopes': ['read:hub'], - }, - ] +@mark.parametrize( + "role_spec", + [ + pytest.param( + { + 'name': 'elephant', + 'users': ['admin'], + }, + id="should not allow assigning a role to a user", + ), + pytest.param( + { + 'name': 'elephant', + 'groups': ['test-group'], + }, + id="should not allow assigning a role to a group", + ), + ], +) +async def test_manage_roles_disallows_role_assignment(role_spec): + roles_to_load = [role_spec] hub = MockHub(load_roles=roles_to_load) hub.init_db() hub.authenticator.manage_roles = True - with pytest.raises(ValueError, match="offloaded to the authenticator"): + with pytest.raises( + ValueError, + match="`load_roles` can not be used for assigning roles to users nor groups", + ): await hub.init_role_creation() +@mark.parametrize( + "role_spec", + [ + pytest.param( + {'name': 'elephant', 'description': 'pacing about'}, + id="should allow creating a new role", + ), + pytest.param( + { + 'name': 'elephant', + 'scopes': ['read:hub'], + }, + id="should allow assigning a scope to a new role", + ), + pytest.param( + {'name': 'user', 'scopes': ['read:hub']}, + id="should allow assigning a scope to a default role", + ), + ], +) +async def test_manage_roles_allows_using_load_roles(role_spec): + roles_to_load = [role_spec] + hub = MockHub(load_roles=roles_to_load) + hub.init_db() + hub.authenticator.manage_roles = True + await hub.init_role_creation() + + async def test_manage_roles_loads_default_roles(): hub = MockHub() hub.init_db()