From 9e38ab1249da685282f7928f5cea776c9d821be6 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Fri, 26 Jul 2024 18:40:44 -0300 Subject: [PATCH 01/19] Include shared group directory mouting role --- .../files/jupyterhub/03-profiles.py | 48 +++++++++++++++---- .../jupyterhub/files/jupyterhub/04-auth.py | 35 ++++++++++++-- .../kubernetes/services/jupyterhub/main.tf | 11 +++++ 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py index 22193e79d..e74ba623c 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py @@ -82,11 +82,11 @@ def base_profile_home_mounts(username): } -def base_profile_shared_mounts(groups): +def base_profile_shared_mounts(groups, group_roles): """Configure the group directory mounts for user. - Ensure that {shared}/{group} directory exists and user has - permissions to read/write/execute. Kubernetes does not allow the + Ensure that {shared}/{group} directory exists based on the scope availability + and if user has permissions to read/write/execute. Kubernetes does not allow the same pvc to be a volume thus we must check that the home and share pvc are not the same for some operation. @@ -103,6 +103,14 @@ def base_profile_shared_mounts(groups): {"name": "shared", "persistentVolumeClaim": {"claimName": shared_pvc_name}} ) + relevant_groups = [] + for group in groups: + for roles in group_roles.get(group, []): + # Check if the group has a role that has a shared-directory scope + if "shared-directory" in roles.get("attributes", {}).get("component", []): + relevant_groups.append(group) + break + extra_container_config = { "volumeMounts": [ { @@ -110,7 +118,7 @@ def base_profile_shared_mounts(groups): "name": "shared" if home_pvc_name != shared_pvc_name else "home", "subPath": pvc_shared_mount_path.format(group=group), } - for group in groups + for group in relevant_groups ] } @@ -118,7 +126,7 @@ def base_profile_shared_mounts(groups): command = " && ".join( [ MKDIR_OWN_DIRECTORY.format(path=pvc_shared_mount_path.format(group=group)) - for group in groups + for group in relevant_groups ] ) init_containers = [ @@ -133,7 +141,7 @@ def base_profile_shared_mounts(groups): "name": "shared" if home_pvc_name != shared_pvc_name else "home", "subPath": pvc_shared_mount_path.format(group=group), } - for group in groups + for group in relevant_groups ], } ] @@ -475,7 +483,7 @@ def profile_conda_store_viewer_token(): } -def render_profile(profile, username, groups, keycloak_profilenames): +def render_profile(profile, username, groups, keycloak_profilenames, group_roles): """Render each profile for user. If profile is not available for given username, groups returns @@ -513,7 +521,7 @@ def render_profile(profile, username, groups, keycloak_profilenames): deep_merge, [ base_profile_home_mounts(username), - base_profile_shared_mounts(groups), + base_profile_shared_mounts(groups, group_roles), profile_conda_store_mounts(username, groups), base_profile_extra_mounts(), configure_user(username, groups), @@ -543,6 +551,27 @@ def preserve_envvars(spawner): return profile +def parse_roles(data): + parsed_roles = {} + + for role in data: + for group in role["groups"]: + # group = str(group).replace("/", "") + group_name = Path(group).name + if group_name not in parsed_roles: + parsed_roles[group_name] = [] + + role_info = { + "description": role["description"], + "name": role["name"], + "attributes": role["attributes"], + } + + parsed_roles[group_name].append(role_info) + + return parsed_roles + + @gen.coroutine def render_profiles(spawner): # jupyterhub does not yet manage groups but it will soon @@ -550,6 +579,7 @@ def render_profiles(spawner): # userinfo request to have the groups in the key # "auth_state.oauth_user.groups" auth_state = yield spawner.user.get_auth_state() + group_roles = parse_roles(spawner.authenticator.group_roles_map) username = auth_state["oauth_user"]["preferred_username"] # only return the lowest level group name @@ -566,7 +596,7 @@ def render_profiles(spawner): filter( None, [ - render_profile(p, username, groups, keycloak_profilenames) + render_profile(p, username, groups, keycloak_profilenames, group_roles) for p in profile_list ], ) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index cbd20a441..263f8ccc1 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -7,7 +7,7 @@ from jupyterhub import scopes from jupyterhub.traitlets import Callable from oauthenticator.generic import GenericOAuthenticator -from traitlets import Bool, Unicode, Union +from traitlets import Bool, Unicode, Union, List class KeyCloakOAuthenticator(GenericOAuthenticator): @@ -27,6 +27,11 @@ class KeyCloakOAuthenticator(GenericOAuthenticator): config=True, help="""The keycloak REST API URL for the realm.""" ) + group_roles_map = List( + config=False, + help="""A mapping of roles to groups based on the user's assigned roles.""", + ) + reset_managed_roles_on_startup = Bool(True) async def update_auth_model(self, auth_model): @@ -55,6 +60,11 @@ async def update_auth_model(self, auth_model): user_roles_rich = await self._get_roles_with_attributes( roles=user_roles, client_id=jupyterhub_client_id, token=token ) + self.group_roles_map = await self._fetch_and_map_roles( + roles=user_roles_rich, + token=token, + url=f"clients/{jupyterhub_client_id}/roles/{{role_name}}", + ) keycloak_api_call_time_taken = time.time() - keycloak_api_call_start user_roles_rich_names = {role["name"] for role in user_roles_rich} user_roles_non_jhub_client = [ @@ -154,17 +164,36 @@ async def load_managed_roles(self): return list(roles.values()) - def _get_scope_from_role(self, role): + def _get_scope_from_role(self, role, component_filter=["jupyterhub"]): """Return scopes from role if the component is jupyterhub""" role_scopes = role.get("attributes", {}).get("scopes", []) component = role.get("attributes", {}).get("component") # Attributes are returned as a single-element array, unless `##` delimiter is used in Keycloak # See this: https://stackoverflow.com/questions/68954733/keycloak-client-role-attribute-array - if component == ["jupyterhub"] and role_scopes: + if component == component_filter and role_scopes: return self.validate_scopes(role_scopes[0].split(",")) + elif component_filter is None: + return self.validate_scopes(role_scopes) else: return [] + async def _fetch_and_map_roles(self, roles, token, url, group_name_key="path"): + # we could use either `name` (e.g. "developer") or `path` ("/developer"); + # since the default claim key returns `path`, it seems preferable. + + self.log.info("Mapping roles with groups and users..") + + for role in roles: + role_name = role["name"] + # fetch role assignments to groups + base_url = url.format(role_name=role_name) + groups = await self._fetch_api(f"{base_url}/groups", token=token) + role["groups"] = [group[group_name_key] for group in groups] + users = await self._fetch_api(f"{base_url}/users", token=token) + role["users"] = [user["username"] for user in users] + + return roles + def validate_scopes(self, role_scopes): """Validate role scopes to sanity check user provided scopes from keycloak""" self.log.info(f"Validating role scopes: {role_scopes}") diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/main.tf b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/main.tf index 8c310c5ed..bde0d03fa 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/main.tf +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/main.tf @@ -304,6 +304,17 @@ module "jupyterhub-openid-client" { "component" : "jupyterhub" } }, + { + "name" : "allow-group-directory-creation-role", + "description" : "Grants a group the ability to manage the creation of its corresponding mounted directory.", + # Adding it to analyst group such that it's applied to every user. + "groups" : ["admin", "analyst", "developer"], + "attributes" : { + # grants permissions to read services + "scopes" : "write:shared-mount", + "component" : "shared-directory" + } + }, ] callback-url-paths = [ "https://${var.external-url}/hub/oauth_callback", From ff74a0d3fa13ec0e24962b8a788873e469e4806c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 29 Jul 2024 16:34:58 +0000 Subject: [PATCH 02/19] [pre-commit.ci] Apply automatic pre-commit fixes --- .../kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index 263f8ccc1..f455007bb 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -7,7 +7,7 @@ from jupyterhub import scopes from jupyterhub.traitlets import Callable from oauthenticator.generic import GenericOAuthenticator -from traitlets import Bool, Unicode, Union, List +from traitlets import Bool, List, Unicode, Union class KeyCloakOAuthenticator(GenericOAuthenticator): From 5ca8dbcdc8967d1744081629612867126358bdfe Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Wed, 31 Jul 2024 12:27:39 -0300 Subject: [PATCH 03/19] Add review commits & refactor container configs --- .../files/jupyterhub/03-profiles.py | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py index e74ba623c..0ad72a69b 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py @@ -103,48 +103,50 @@ def base_profile_shared_mounts(groups, group_roles): {"name": "shared", "persistentVolumeClaim": {"claimName": shared_pvc_name}} ) - relevant_groups = [] + groups_to_volume_mount = [] for group in groups: for roles in group_roles.get(group, []): # Check if the group has a role that has a shared-directory scope if "shared-directory" in roles.get("attributes", {}).get("component", []): - relevant_groups.append(group) + groups_to_volume_mount.append(group) break - extra_container_config = { - "volumeMounts": [ - { - "mountPath": pod_shared_mount_path.format(group=group), - "name": "shared" if home_pvc_name != shared_pvc_name else "home", - "subPath": pvc_shared_mount_path.format(group=group), - } - for group in relevant_groups - ] - } + extra_container_config = {"volumeMounts": []} MKDIR_OWN_DIRECTORY = "mkdir -p /mnt/{path} && chmod 777 /mnt/{path}" command = " && ".join( [ MKDIR_OWN_DIRECTORY.format(path=pvc_shared_mount_path.format(group=group)) - for group in relevant_groups + for group in groups_to_volume_mount ] ) + init_containers = [ { "name": "initialize-shared-mounts", "image": "busybox:1.31", "command": ["sh", "-c", command], "securityContext": {"runAsUser": 0}, - "volumeMounts": [ - { - "mountPath": f"/mnt/{pvc_shared_mount_path.format(group=group)}", - "name": "shared" if home_pvc_name != shared_pvc_name else "home", - "subPath": pvc_shared_mount_path.format(group=group), - } - for group in relevant_groups - ], + "volumeMounts": [], } ] + + for groups in groups_to_volume_mount: + extra_container_config["volumeMounts"].append( + { + "mountPath": pod_shared_mount_path.format(group=groups), + "name": "shared" if home_pvc_name != shared_pvc_name else "home", + "subPath": pvc_shared_mount_path.format(group=groups), + } + ) + init_containers[0]["volumeMounts"].append( + { + "mountPath": f"/mnt/{pvc_shared_mount_path.format(group=groups)}", + "name": "shared" if home_pvc_name != shared_pvc_name else "home", + "subPath": pvc_shared_mount_path.format(group=groups), + } + ) + return { "extra_pod_config": extra_pod_config, "extra_container_config": extra_container_config, From 6a8e000aa66930129ac9a8a84825b7c5947a7a4e Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Wed, 31 Jul 2024 16:13:03 -0300 Subject: [PATCH 04/19] Include testing for new allow-group-directory-creation-role --- tests/tests_deployment/keycloak_utils.py | 10 ++++++++++ tests/tests_deployment/test_jupyterhub_api.py | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/tests/tests_deployment/keycloak_utils.py b/tests/tests_deployment/keycloak_utils.py index b11c64b93..1201a2e06 100644 --- a/tests/tests_deployment/keycloak_utils.py +++ b/tests/tests_deployment/keycloak_utils.py @@ -81,6 +81,16 @@ def create_keycloak_role(client_name: str, role_name: str, scopes: str, componen ) +def get_keycloak_client_role(client_name, role_name): + keycloak_admin = get_keycloak_admin() + client_details = get_keycloak_client_details_by_name( + client_name=client_name, keycloak_admin=keycloak_admin + ) + return keycloak_admin.get_client_role( + client_id=client_details["id"], role_name=role_name + ) + + def get_keycloak_client_roles(client_name): keycloak_admin = get_keycloak_admin() client_details = get_keycloak_client_details_by_name( diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index 4144fd4fe..32b468469 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -5,6 +5,7 @@ assign_keycloak_client_role_to_user, create_keycloak_role, get_keycloak_client_roles, + get_keycloak_client_role, ) from tests.tests_deployment.utils import create_jupyterhub_token, get_jupyterhub_session @@ -33,6 +34,7 @@ def test_jupyterhub_loads_roles_from_keycloak(): "view-profile", # default roles "allow-read-access-to-services-role", + "allow-group-directory-creation-role", } @@ -52,6 +54,23 @@ def test_check_default_roles_added_in_keycloak(): role_names = [role["name"] for role in client_roles] assert "allow-app-sharing-role" in role_names assert "allow-read-access-to-services-role" in role_names + assert "allow-group-directory-creation-role" in role_names + + +@pytest.mark.parametrize( + "component,scopes", + (["shared-directory", "create:shared"],), +) +@pytest.mark.filterwarnings( + "ignore:.*auto_refresh_token is deprecated:DeprecationWarning" +) +@pytest.mark.filterwarnings("ignore::urllib3.exceptions.InsecureRequestWarning") +def test_check_default_groups_receive_directory_creation_scope(component, scopes): + client_role = get_keycloak_client_role( + client_name="jupyterhub", role_name="allow-group-directory-creation-role" + ) + assert client_role["attributes"]["component"] == component + assert client_role["attributes"]["scopes"] == scopes @pytest.mark.parametrize( From bb7a50925a4605d578c0ea322a0fa08c698961ae Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 31 Jul 2024 19:13:32 +0000 Subject: [PATCH 05/19] [pre-commit.ci] Apply automatic pre-commit fixes --- tests/tests_deployment/test_jupyterhub_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index 32b468469..493e7f03b 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -4,8 +4,8 @@ from tests.tests_deployment.keycloak_utils import ( assign_keycloak_client_role_to_user, create_keycloak_role, - get_keycloak_client_roles, get_keycloak_client_role, + get_keycloak_client_roles, ) from tests.tests_deployment.utils import create_jupyterhub_token, get_jupyterhub_session From fcbd4b67c063dffe45b37cb57502bbff64e59596 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Wed, 31 Jul 2024 16:31:25 -0300 Subject: [PATCH 06/19] Fix incorrect naming attribute --- tests/tests_deployment/test_jupyterhub_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index 32b468469..892c0bf25 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -69,8 +69,8 @@ def test_check_default_groups_receive_directory_creation_scope(component, scopes client_role = get_keycloak_client_role( client_name="jupyterhub", role_name="allow-group-directory-creation-role" ) - assert client_role["attributes"]["component"] == component - assert client_role["attributes"]["scopes"] == scopes + assert client_role["attributes"]["resource"][0] == component + assert client_role["attributes"]["scopes"][0] == scopes @pytest.mark.parametrize( From afca6326bf34e6d5cc52e3ea1579c1e0b9f98ad5 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Wed, 31 Jul 2024 17:32:04 -0300 Subject: [PATCH 07/19] Fix incorrect naming attribute --- tests/tests_deployment/test_jupyterhub_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index 002b2d4f7..e467c3ddf 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -59,7 +59,7 @@ def test_check_default_roles_added_in_keycloak(): @pytest.mark.parametrize( "component,scopes", - (["shared-directory", "create:shared"],), + (["shared-directory", "write:shared-mount"],), ) @pytest.mark.filterwarnings( "ignore:.*auto_refresh_token is deprecated:DeprecationWarning" @@ -69,7 +69,7 @@ def test_check_default_groups_receive_directory_creation_scope(component, scopes client_role = get_keycloak_client_role( client_name="jupyterhub", role_name="allow-group-directory-creation-role" ) - assert client_role["attributes"]["resource"][0] == component + assert client_role["attributes"]["component"][0] == component assert client_role["attributes"]["scopes"][0] == scopes From c5f651c62a6a0253dea207eba41dfd313c82a446 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Fri, 2 Aug 2024 16:27:02 -0300 Subject: [PATCH 08/19] Refactor mount dir role permission gathering logic --- .../files/jupyterhub/03-profiles.py | 12 +- .../jupyterhub/files/jupyterhub/04-auth.py | 146 +++++++++++++----- 2 files changed, 115 insertions(+), 43 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py index 0ad72a69b..973244513 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py @@ -581,7 +581,9 @@ def render_profiles(spawner): # userinfo request to have the groups in the key # "auth_state.oauth_user.groups" auth_state = yield spawner.user.get_auth_state() - group_roles = parse_roles(spawner.authenticator.group_roles_map) + groups_with_permission_to_mount = auth_state.get( + "groups_with_permission_to_mount", [] + ) username = auth_state["oauth_user"]["preferred_username"] # only return the lowest level group name @@ -598,7 +600,13 @@ def render_profiles(spawner): filter( None, [ - render_profile(p, username, groups, keycloak_profilenames, group_roles) + render_profile( + p, + username, + groups, + keycloak_profilenames, + groups_with_permission_to_mount, + ) for p in profile_list ], ) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index f455007bb..78f6d3e7e 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -1,3 +1,4 @@ +import asyncio import json import os import time @@ -27,10 +28,10 @@ class KeyCloakOAuthenticator(GenericOAuthenticator): config=True, help="""The keycloak REST API URL for the realm.""" ) - group_roles_map = List( - config=False, - help="""A mapping of roles to groups based on the user's assigned roles.""", - ) + # group_roles_map = List( + # config=False, + # help="""A mapping of roles to groups based on the user's assigned roles.""", + # ) reset_managed_roles_on_startup = Bool(True) @@ -60,18 +61,16 @@ async def update_auth_model(self, auth_model): user_roles_rich = await self._get_roles_with_attributes( roles=user_roles, client_id=jupyterhub_client_id, token=token ) - self.group_roles_map = await self._fetch_and_map_roles( - roles=user_roles_rich, - token=token, - url=f"clients/{jupyterhub_client_id}/roles/{{role_name}}", - ) + keycloak_api_call_time_taken = time.time() - keycloak_api_call_start user_roles_rich_names = {role["name"] for role in user_roles_rich} + user_roles_non_jhub_client = [ {"name": role} for role in user_roles_from_claims if role in (user_roles_from_claims - user_roles_rich_names) ] + auth_model["roles"] = [ { "name": role["name"], @@ -80,12 +79,25 @@ async def update_auth_model(self, auth_model): } for role in [*user_roles_rich, *user_roles_non_jhub_client] ] + + # Include which groups have permission to mount shared directories (user by profiles.py) + auth_model["auth_state"]["groups_with_permission_to_mount"] = ( + await self.get_client_groups_with_mount_permissions( + user_groups=auth_model["auth_state"]["oauth_user"]["groups"], + user_roles=user_roles_rich, + token=token, + ) + ) + # note: because the roles check is comprehensive, we need to re-add the admin and user roles if auth_model["admin"]: auth_model["roles"].append({"name": "admin"}) + if await self.check_allowed(auth_model["name"], auth_model): auth_model["roles"].append({"name": "user"}) + execution_time = time.time() - start + self.log.info( f"Auth model update complete, time taken: {execution_time}s " f"time taken for keycloak api call: {keycloak_api_call_time_taken}s " @@ -138,32 +150,101 @@ async def load_managed_roles(self): } # we could use either `name` (e.g. "developer") or `path` ("/developer"); # since the default claim key returns `path`, it seems preferable. - group_name_key = "path" for realm_role in realm_roles: role_name = realm_role["name"] role = roles[role_name] - # fetch role assignments to groups - groups = await self._fetch_api(f"roles/{role_name}/groups", token=token) - role["groups"] = [group[group_name_key] for group in groups] - # fetch role assignments to users - users = await self._fetch_api(f"roles/{role_name}/users", token=token) - role["users"] = [user["username"] for user in users] + # # fetch role assignments to groups + # groups = await self._fetch_api(f"roles/{role_name}/groups", token=token) + # role["groups"] = [group[group_name_key] for group in groups] + # # fetch role assignments to users + # users = await self._fetch_api(f"roles/{role_name}/users", token=token) + # role["users"] = [user["username"] for user in users] + role.update( + await self._map_users_and_groups_to_role( + role_name, + token=token, + ) + ) + for client_role in client_roles_rich: role_name = client_role["name"] role = roles[role_name] # fetch role assignments to groups - groups = await self._fetch_api( - f"clients/{jupyterhub_client_id}/roles/{role_name}/groups", token=token - ) - role["groups"] = [group[group_name_key] for group in groups] - # fetch role assignments to users - users = await self._fetch_api( - f"clients/{jupyterhub_client_id}/roles/{role_name}/users", token=token + # groups = await self._fetch_api( + # f"clients/{jupyterhub_client_id}/roles/{role_name}/groups", token=token + # ) + # role["groups"] = [group[group_name_key] for group in groups] + # # fetch role assignments to users + # users = await self._fetch_api( + # f"clients/{jupyterhub_client_id}/roles/{role_name}/users", token=token + # ) + # role["users"] = [user["username"] for user in users] + role.update( + await self._map_users_and_groups_to_role( + role_name, + token=token, + ) ) - role["users"] = [user["username"] for user in users] return list(roles.values()) + async def get_client_groups_with_mount_permissions( + self, user_groups, user_roles, token + ): + """ + Asynchronously retrieves the list of client groups with mount permissions + that the user belongs to. + """ + groups_with_permission_to_mount = set() + + # filter user roles by scope=shared-directory + for role in user_roles: + role_component = role["attributes"].get("component", [None])[0] + role_scopes = role["attributes"].get("scopes", [None])[0] + if (role_component == "shared-directory") and ( + role_scopes == "write:shared-mount" + ): + role_groups = await self._fetch_api( + endpoint=f"roles/{role['name']}/groups", + token=token, + ) + # using name here, as the auth_state groups also does not have the path prefix + groups_with_permission_to_mount |= set( + [group["name"] for group in role_groups] + ) + + groups_with_permission_to_mount &= set(user_groups) + return list(groups_with_permission_to_mount) + + async def _map_users_and_groups_to_role( + self, role_name, token, group_name_key="path" + ): + """ + Asynchronously fetches and maps groups and users to a specified role. + + Returns: + dict: A dictionary with groups (path or name) and users mapped to the role. + { + "groups": ["/group1", "/group2"], + "users": ["user1", "user2"], + }, + """ + # Prepare endpoints + group_endpoint = f"roles/{role_name}/groups" + user_endpoint = f"roles/{role_name}/users" + + # fetch role assignments to groups (Fetch data concurrently) + groups, users = await asyncio.gather( + self._fetch_api(endpoint=group_endpoint, token=token), + self._fetch_api(endpoint=user_endpoint, token=token), + ) + + # Process results + return { + "groups": [group[group_name_key] for group in groups], + "users": [user["username"] for user in users], + } + def _get_scope_from_role(self, role, component_filter=["jupyterhub"]): """Return scopes from role if the component is jupyterhub""" role_scopes = role.get("attributes", {}).get("scopes", []) @@ -177,23 +258,6 @@ def _get_scope_from_role(self, role, component_filter=["jupyterhub"]): else: return [] - async def _fetch_and_map_roles(self, roles, token, url, group_name_key="path"): - # we could use either `name` (e.g. "developer") or `path` ("/developer"); - # since the default claim key returns `path`, it seems preferable. - - self.log.info("Mapping roles with groups and users..") - - for role in roles: - role_name = role["name"] - # fetch role assignments to groups - base_url = url.format(role_name=role_name) - groups = await self._fetch_api(f"{base_url}/groups", token=token) - role["groups"] = [group[group_name_key] for group in groups] - users = await self._fetch_api(f"{base_url}/users", token=token) - role["users"] = [user["username"] for user in users] - - return roles - def validate_scopes(self, role_scopes): """Validate role scopes to sanity check user provided scopes from keycloak""" self.log.info(f"Validating role scopes: {role_scopes}") From 7bc8a0423947349f04d081d2fd95d9494188647e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 2 Aug 2024 19:27:20 +0000 Subject: [PATCH 09/19] [pre-commit.ci] Apply automatic pre-commit fixes --- .../kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index 78f6d3e7e..3a5e83e47 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -8,7 +8,7 @@ from jupyterhub import scopes from jupyterhub.traitlets import Callable from oauthenticator.generic import GenericOAuthenticator -from traitlets import Bool, List, Unicode, Union +from traitlets import Bool, Unicode, Union class KeyCloakOAuthenticator(GenericOAuthenticator): From b1336f86f6289fac9255b8803ef02c9253a359ef Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Fri, 2 Aug 2024 18:14:19 -0300 Subject: [PATCH 10/19] fix request URL for client redirects & fix set logic --- .../files/jupyterhub/03-profiles.py | 51 +++++------ .../jupyterhub/files/jupyterhub/04-auth.py | 91 +++++++++++++------ 2 files changed, 88 insertions(+), 54 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py index 973244513..4af184fe9 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py @@ -82,7 +82,7 @@ def base_profile_home_mounts(username): } -def base_profile_shared_mounts(groups, group_roles): +def base_profile_shared_mounts(groups, groups_to_volume_mount): """Configure the group directory mounts for user. Ensure that {shared}/{group} directory exists based on the scope availability @@ -103,14 +103,6 @@ def base_profile_shared_mounts(groups, group_roles): {"name": "shared", "persistentVolumeClaim": {"claimName": shared_pvc_name}} ) - groups_to_volume_mount = [] - for group in groups: - for roles in group_roles.get(group, []): - # Check if the group has a role that has a shared-directory scope - if "shared-directory" in roles.get("attributes", {}).get("component", []): - groups_to_volume_mount.append(group) - break - extra_container_config = {"volumeMounts": []} MKDIR_OWN_DIRECTORY = "mkdir -p /mnt/{path} && chmod 777 /mnt/{path}" @@ -485,7 +477,9 @@ def profile_conda_store_viewer_token(): } -def render_profile(profile, username, groups, keycloak_profilenames, group_roles): +def render_profile( + profile, username, groups, keycloak_profilenames, groups_to_volume_mount +): """Render each profile for user. If profile is not available for given username, groups returns @@ -523,7 +517,7 @@ def render_profile(profile, username, groups, keycloak_profilenames, group_roles deep_merge, [ base_profile_home_mounts(username), - base_profile_shared_mounts(groups, group_roles), + base_profile_shared_mounts(groups, groups_to_volume_mount), profile_conda_store_mounts(username, groups), base_profile_extra_mounts(), configure_user(username, groups), @@ -553,25 +547,25 @@ def preserve_envvars(spawner): return profile -def parse_roles(data): - parsed_roles = {} +# def parse_roles(data): +# parsed_roles = {} - for role in data: - for group in role["groups"]: - # group = str(group).replace("/", "") - group_name = Path(group).name - if group_name not in parsed_roles: - parsed_roles[group_name] = [] +# for role in data: +# for group in role["groups"]: +# # group = str(group).replace("/", "") +# group_name = Path(group).name +# if group_name not in parsed_roles: +# parsed_roles[group_name] = [] - role_info = { - "description": role["description"], - "name": role["name"], - "attributes": role["attributes"], - } +# role_info = { +# "description": role["description"], +# "name": role["name"], +# "attributes": role["attributes"], +# } - parsed_roles[group_name].append(role_info) +# parsed_roles[group_name].append(role_info) - return parsed_roles +# return parsed_roles @gen.coroutine @@ -585,7 +579,12 @@ def render_profiles(spawner): "groups_with_permission_to_mount", [] ) + spawner.log.info( + f"groups_with_permission_to_mount: {groups_with_permission_to_mount}" + ) + username = auth_state["oauth_user"]["preferred_username"] + spawner.log.info(f"username: {username}") # only return the lowest level group name # e.g. /projects/myproj -> myproj # and /developers -> developers diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index 78f6d3e7e..e9c3adf82 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -62,6 +62,21 @@ async def update_auth_model(self, auth_model): roles=user_roles, client_id=jupyterhub_client_id, token=token ) + # Include which groups have permission to mount shared directories (user by + # profiles.py) + self.log.info(f"User roles: {user_roles_rich}") + self.log.info( + f"User groups: {auth_model['auth_state']['oauth_user']['groups']}" + ) + auth_model["auth_state"]["groups_with_permission_to_mount"] = ( + await self.get_client_groups_with_mount_permissions( + user_groups=auth_model["auth_state"]["oauth_user"]["groups"], + user_roles=user_roles_rich, + client_id=jupyterhub_client_id, + token=token, + ) + ) + keycloak_api_call_time_taken = time.time() - keycloak_api_call_start user_roles_rich_names = {role["name"] for role in user_roles_rich} @@ -80,15 +95,6 @@ async def update_auth_model(self, auth_model): for role in [*user_roles_rich, *user_roles_non_jhub_client] ] - # Include which groups have permission to mount shared directories (user by profiles.py) - auth_model["auth_state"]["groups_with_permission_to_mount"] = ( - await self.get_client_groups_with_mount_permissions( - user_groups=auth_model["auth_state"]["oauth_user"]["groups"], - user_roles=user_roles_rich, - token=token, - ) - ) - # note: because the roles check is comprehensive, we need to re-add the admin and user roles if auth_model["admin"]: auth_model["roles"].append({"name": "admin"}) @@ -183,41 +189,64 @@ async def load_managed_roles(self): await self._map_users_and_groups_to_role( role_name, token=token, + client_id=jupyterhub_client_id, ) ) return list(roles.values()) async def get_client_groups_with_mount_permissions( - self, user_groups, user_roles, token + self, user_groups, user_roles, client_id, token ): """ Asynchronously retrieves the list of client groups with mount permissions that the user belongs to. """ + + roles_with_permission = [] groups_with_permission_to_mount = set() - # filter user roles by scope=shared-directory + # Filter roles with the shared-directory component and scope + # This should only happen in case there are user defined/custom ones for role in user_roles: - role_component = role["attributes"].get("component", [None])[0] - role_scopes = role["attributes"].get("scopes", [None])[0] - if (role_component == "shared-directory") and ( - role_scopes == "write:shared-mount" + attributes = role.get("attributes", {}) + + role_component = attributes.get("component", [None])[0] + role_scopes = attributes.get("scopes", [None])[0] + + if ( + role_component == "shared-directory" + and role_scopes == "write:shared-mount" ): - role_groups = await self._fetch_api( - endpoint=f"roles/{role['name']}/groups", - token=token, - ) - # using name here, as the auth_state groups also does not have the path prefix - groups_with_permission_to_mount |= set( - [group["name"] for group in role_groups] - ) + role_name = role.get("name") + roles_with_permission.append(role_name) + self.log.info(f"Roles with permission to mount: {roles_with_permission}") + + # Fetch groups for all relevant roles concurrently + group_fetch_tasks = [ + self._fetch_api( + endpoint=f"clients/{client_id}/roles/{role_name}/groups", + token=token, + ) + for role_name in roles_with_permission + ] + all_role_groups = await asyncio.gather(*group_fetch_tasks) + self.log.info(f"Groups with permission to mount: {all_role_groups}") + + # Collect group names with permissions + for role_groups in all_role_groups: + groups_with_permission_to_mount |= set( + [group["path"] for group in role_groups] + ) + self.log.info( + f"Groups with permission to mount: {groups_with_permission_to_mount}" + ) + self.log.info(f"User groups: {user_groups}") - groups_with_permission_to_mount &= set(user_groups) - return list(groups_with_permission_to_mount) + return list(groups_with_permission_to_mount & set(user_groups)) async def _map_users_and_groups_to_role( - self, role_name, token, group_name_key="path" + self, role_name, token, client_id=None, group_name_key="path" ): """ Asynchronously fetches and maps groups and users to a specified role. @@ -233,10 +262,16 @@ async def _map_users_and_groups_to_role( group_endpoint = f"roles/{role_name}/groups" user_endpoint = f"roles/{role_name}/users" + if client_id: + group_endpoint = f"clients/{client_id}/roles/{role_name}/groups" + user_endpoint = f"clients/{client_id}/roles/{role_name}/users" + # fetch role assignments to groups (Fetch data concurrently) groups, users = await asyncio.gather( - self._fetch_api(endpoint=group_endpoint, token=token), - self._fetch_api(endpoint=user_endpoint, token=token), + *[ + self._fetch_api(endpoint=group_endpoint, token=token), + self._fetch_api(endpoint=user_endpoint, token=token), + ] ) # Process results From 8f05abba18a5fa9fad6575b86542a2632fdc7ff1 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Mon, 5 Aug 2024 16:20:47 -0300 Subject: [PATCH 11/19] remove debugging comments & add another test for checking default groups --- .../files/jupyterhub/03-profiles.py | 35 +++---------------- .../jupyterhub/files/jupyterhub/04-auth.py | 26 +++----------- tests/tests_deployment/keycloak_utils.py | 7 ++++ tests/tests_deployment/test_jupyterhub_api.py | 25 ++++++++++++- 4 files changed, 40 insertions(+), 53 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py index 4af184fe9..d9e426857 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py @@ -547,27 +547,6 @@ def preserve_envvars(spawner): return profile -# def parse_roles(data): -# parsed_roles = {} - -# for role in data: -# for group in role["groups"]: -# # group = str(group).replace("/", "") -# group_name = Path(group).name -# if group_name not in parsed_roles: -# parsed_roles[group_name] = [] - -# role_info = { -# "description": role["description"], -# "name": role["name"], -# "attributes": role["attributes"], -# } - -# parsed_roles[group_name].append(role_info) - -# return parsed_roles - - @gen.coroutine def render_profiles(spawner): # jupyterhub does not yet manage groups but it will soon @@ -575,24 +554,20 @@ def render_profiles(spawner): # userinfo request to have the groups in the key # "auth_state.oauth_user.groups" auth_state = yield spawner.user.get_auth_state() - groups_with_permission_to_mount = auth_state.get( - "groups_with_permission_to_mount", [] - ) - - spawner.log.info( - f"groups_with_permission_to_mount: {groups_with_permission_to_mount}" - ) username = auth_state["oauth_user"]["preferred_username"] - spawner.log.info(f"username: {username}") + # only return the lowest level group name # e.g. /projects/myproj -> myproj # and /developers -> developers groups = [Path(group).name for group in auth_state["oauth_user"]["groups"]] - spawner.log.info(f"user info: {username} {groups}") keycloak_profilenames = auth_state["oauth_user"].get("jupyterlab_profiles", []) + groups_with_permission_to_mount = auth_state.get( + "groups_with_permission_to_mount", [] + ) + # fetch available profiles and render additional attributes profile_list = z2jh.get_config("custom.profiles") return list( diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index b5be76687..ec1add7ce 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -144,6 +144,7 @@ async def load_managed_roles(self): client_roles_rich = await self._get_jupyterhub_client_roles( jupyterhub_client_id=jupyterhub_client_id, token=token ) + # Includes roles like "default-roles-nebari", "offline_access", "uma_authorization" realm_roles = await self._fetch_api(endpoint="roles", token=token) roles = { @@ -154,17 +155,13 @@ async def load_managed_roles(self): } for role in [*realm_roles, *client_roles_rich] } + # we could use either `name` (e.g. "developer") or `path` ("/developer"); # since the default claim key returns `path`, it seems preferable. for realm_role in realm_roles: role_name = realm_role["name"] role = roles[role_name] - # # fetch role assignments to groups - # groups = await self._fetch_api(f"roles/{role_name}/groups", token=token) - # role["groups"] = [group[group_name_key] for group in groups] - # # fetch role assignments to users - # users = await self._fetch_api(f"roles/{role_name}/users", token=token) - # role["users"] = [user["username"] for user in users] + # fetch role assignments to groups role.update( await self._map_users_and_groups_to_role( role_name, @@ -176,15 +173,6 @@ async def load_managed_roles(self): role_name = client_role["name"] role = roles[role_name] # fetch role assignments to groups - # groups = await self._fetch_api( - # f"clients/{jupyterhub_client_id}/roles/{role_name}/groups", token=token - # ) - # role["groups"] = [group[group_name_key] for group in groups] - # # fetch role assignments to users - # users = await self._fetch_api( - # f"clients/{jupyterhub_client_id}/roles/{role_name}/users", token=token - # ) - # role["users"] = [user["username"] for user in users] role.update( await self._map_users_and_groups_to_role( role_name, @@ -207,7 +195,6 @@ async def get_client_groups_with_mount_permissions( groups_with_permission_to_mount = set() # Filter roles with the shared-directory component and scope - # This should only happen in case there are user defined/custom ones for role in user_roles: attributes = role.get("attributes", {}) @@ -220,7 +207,6 @@ async def get_client_groups_with_mount_permissions( ): role_name = role.get("name") roles_with_permission.append(role_name) - self.log.info(f"Roles with permission to mount: {roles_with_permission}") # Fetch groups for all relevant roles concurrently group_fetch_tasks = [ @@ -230,18 +216,14 @@ async def get_client_groups_with_mount_permissions( ) for role_name in roles_with_permission ] + all_role_groups = await asyncio.gather(*group_fetch_tasks) - self.log.info(f"Groups with permission to mount: {all_role_groups}") # Collect group names with permissions for role_groups in all_role_groups: groups_with_permission_to_mount |= set( [group["path"] for group in role_groups] ) - self.log.info( - f"Groups with permission to mount: {groups_with_permission_to_mount}" - ) - self.log.info(f"User groups: {user_groups}") return list(groups_with_permission_to_mount & set(user_groups)) diff --git a/tests/tests_deployment/keycloak_utils.py b/tests/tests_deployment/keycloak_utils.py index 1201a2e06..1c6e4c2d3 100644 --- a/tests/tests_deployment/keycloak_utils.py +++ b/tests/tests_deployment/keycloak_utils.py @@ -99,6 +99,13 @@ def get_keycloak_client_roles(client_name): return keycloak_admin.get_client_roles(client_id=client_details["id"]) +def get_keycloak_role_groups(client_id, role_name): + keycloak_admin = get_keycloak_admin() + return keycloak_admin.get_client_role_groups( + client_id=client_id, role_name=role_name + ) + + def delete_client_keycloak_test_roles(client_name): keycloak_admin = get_keycloak_admin() client_details = get_keycloak_client_details_by_name( diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index e467c3ddf..ac74c71b7 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -2,10 +2,12 @@ from tests.tests_deployment import constants from tests.tests_deployment.keycloak_utils import ( + get_keycloak_client_details_by_name, assign_keycloak_client_role_to_user, create_keycloak_role, get_keycloak_client_role, get_keycloak_client_roles, + get_keycloak_role_groups, ) from tests.tests_deployment.utils import create_jupyterhub_token, get_jupyterhub_session @@ -65,7 +67,7 @@ def test_check_default_roles_added_in_keycloak(): "ignore:.*auto_refresh_token is deprecated:DeprecationWarning" ) @pytest.mark.filterwarnings("ignore::urllib3.exceptions.InsecureRequestWarning") -def test_check_default_groups_receive_directory_creation_scope(component, scopes): +def test_check_directory_creation_scope_attributes(component, scopes): client_role = get_keycloak_client_role( client_name="jupyterhub", role_name="allow-group-directory-creation-role" ) @@ -73,6 +75,27 @@ def test_check_default_groups_receive_directory_creation_scope(component, scopes assert client_role["attributes"]["scopes"][0] == scopes +@pytest.mark.parametrize( + "groups_to_volume_mount", + (["/developer", "/admin", "/analyst"],), +) +@pytest.mark.filterwarnings( + "ignore:.*auto_refresh_token is deprecated:DeprecationWarning" +) +@pytest.mark.filterwarnings("ignore::urllib3.exceptions.InsecureRequestWarning") +def test_groups_with_mount_permissions( + groups_to_volume_mount, groups_to_not_volume_mount +): + client_role = get_keycloak_client_role( + client_name="jupyterhub", role_name="allow-group-directory-creation-role" + ) + client_details = get_keycloak_client_details_by_name(client_name="jupyterhub") + role_groups = get_keycloak_role_groups( + client_id=client_details["id"], role_name=client_role["name"] + ) + assert list([group["path"] for group in role_groups]) == groups_to_volume_mount + + @pytest.mark.parametrize( "component,scopes,expected_scopes_difference", ( From 7dd72cdac62da2304dfb6c0128c473cfa67f138c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 5 Aug 2024 19:21:09 +0000 Subject: [PATCH 12/19] [pre-commit.ci] Apply automatic pre-commit fixes --- tests/tests_deployment/test_jupyterhub_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index ac74c71b7..f636af418 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -2,9 +2,9 @@ from tests.tests_deployment import constants from tests.tests_deployment.keycloak_utils import ( - get_keycloak_client_details_by_name, assign_keycloak_client_role_to_user, create_keycloak_role, + get_keycloak_client_details_by_name, get_keycloak_client_role, get_keycloak_client_roles, get_keycloak_role_groups, From 57137835a01f0897ece17f2736a48376d6beacb7 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Mon, 5 Aug 2024 16:23:50 -0300 Subject: [PATCH 13/19] clean up left legacy code --- .../services/jupyterhub/files/jupyterhub/04-auth.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index ec1add7ce..f4eac98f6 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -28,11 +28,6 @@ class KeyCloakOAuthenticator(GenericOAuthenticator): config=True, help="""The keycloak REST API URL for the realm.""" ) - # group_roles_map = List( - # config=False, - # help="""A mapping of roles to groups based on the user's assigned roles.""", - # ) - reset_managed_roles_on_startup = Bool(True) async def update_auth_model(self, auth_model): From 742f7fe16e5dde3638af4ccd515ffd8af3f54620 Mon Sep 17 00:00:00 2001 From: viniciusdc Date: Mon, 5 Aug 2024 16:25:30 -0300 Subject: [PATCH 14/19] rm debugging self.log statements --- .../services/jupyterhub/files/jupyterhub/04-auth.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index f4eac98f6..9880aa290 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -59,10 +59,6 @@ async def update_auth_model(self, auth_model): # Include which groups have permission to mount shared directories (user by # profiles.py) - self.log.info(f"User roles: {user_roles_rich}") - self.log.info( - f"User groups: {auth_model['auth_state']['oauth_user']['groups']}" - ) auth_model["auth_state"]["groups_with_permission_to_mount"] = ( await self.get_client_groups_with_mount_permissions( user_groups=auth_model["auth_state"]["oauth_user"]["groups"], From fc349d96306b3f6013f2ed56219b1a685d91881c Mon Sep 17 00:00:00 2001 From: vinicius douglas cerutti Date: Mon, 19 Aug 2024 22:08:56 -0300 Subject: [PATCH 15/19] apply suggestions --- .../files/jupyterhub/03-profiles.py | 14 +++++------ .../jupyterhub/files/jupyterhub/04-auth.py | 12 ++++------ tests/tests_deployment/test_jupyterhub_api.py | 24 +++++++------------ 3 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py index d9e426857..26e10c648 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py @@ -82,7 +82,7 @@ def base_profile_home_mounts(username): } -def base_profile_shared_mounts(groups, groups_to_volume_mount): +def base_profile_shared_mounts(groups_to_volume_mount): """Configure the group directory mounts for user. Ensure that {shared}/{group} directory exists based on the scope availability @@ -123,19 +123,19 @@ def base_profile_shared_mounts(groups, groups_to_volume_mount): } ] - for groups in groups_to_volume_mount: + for group in groups_to_volume_mount: extra_container_config["volumeMounts"].append( { - "mountPath": pod_shared_mount_path.format(group=groups), + "mountPath": pod_shared_mount_path.format(group=group), "name": "shared" if home_pvc_name != shared_pvc_name else "home", - "subPath": pvc_shared_mount_path.format(group=groups), + "subPath": pvc_shared_mount_path.format(group=group), } ) init_containers[0]["volumeMounts"].append( { - "mountPath": f"/mnt/{pvc_shared_mount_path.format(group=groups)}", + "mountPath": f"/mnt/{pvc_shared_mount_path.format(group=group)}", "name": "shared" if home_pvc_name != shared_pvc_name else "home", - "subPath": pvc_shared_mount_path.format(group=groups), + "subPath": pvc_shared_mount_path.format(group=group), } ) @@ -517,7 +517,7 @@ def render_profile( deep_merge, [ base_profile_home_mounts(username), - base_profile_shared_mounts(groups, groups_to_volume_mount), + base_profile_shared_mounts(groups_to_volume_mount), profile_conda_store_mounts(username, groups), base_profile_extra_mounts(), configure_user(username, groups), diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index 9880aa290..a97d66a23 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -154,7 +154,7 @@ async def load_managed_roles(self): role = roles[role_name] # fetch role assignments to groups role.update( - await self._map_users_and_groups_to_role( + await self._get_users_and_groups_for_role( role_name, token=token, ) @@ -165,7 +165,7 @@ async def load_managed_roles(self): role = roles[role_name] # fetch role assignments to groups role.update( - await self._map_users_and_groups_to_role( + await self._get_users_and_groups_for_role( role_name, token=token, client_id=jupyterhub_client_id, @@ -218,7 +218,7 @@ async def get_client_groups_with_mount_permissions( return list(groups_with_permission_to_mount & set(user_groups)) - async def _map_users_and_groups_to_role( + async def _get_users_and_groups_for_role( self, role_name, token, client_id=None, group_name_key="path" ): """ @@ -253,16 +253,14 @@ async def _map_users_and_groups_to_role( "users": [user["username"] for user in users], } - def _get_scope_from_role(self, role, component_filter=["jupyterhub"]): + def _get_scope_from_role(self, role): """Return scopes from role if the component is jupyterhub""" role_scopes = role.get("attributes", {}).get("scopes", []) component = role.get("attributes", {}).get("component") # Attributes are returned as a single-element array, unless `##` delimiter is used in Keycloak # See this: https://stackoverflow.com/questions/68954733/keycloak-client-role-attribute-array - if component == component_filter and role_scopes: + if component == "jupyterhub" and role_scopes: return self.validate_scopes(role_scopes[0].split(",")) - elif component_filter is None: - return self.validate_scopes(role_scopes) else: return [] diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index f636af418..f67303b3e 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -59,33 +59,23 @@ def test_check_default_roles_added_in_keycloak(): assert "allow-group-directory-creation-role" in role_names -@pytest.mark.parametrize( - "component,scopes", - (["shared-directory", "write:shared-mount"],), -) @pytest.mark.filterwarnings( "ignore:.*auto_refresh_token is deprecated:DeprecationWarning" ) @pytest.mark.filterwarnings("ignore::urllib3.exceptions.InsecureRequestWarning") -def test_check_directory_creation_scope_attributes(component, scopes): +def test_check_directory_creation_scope_attributes(): client_role = get_keycloak_client_role( client_name="jupyterhub", role_name="allow-group-directory-creation-role" ) - assert client_role["attributes"]["component"][0] == component - assert client_role["attributes"]["scopes"][0] == scopes + assert client_role["attributes"]["component"][0] == "shared-directory" + assert client_role["attributes"]["scopes"][0] == "write:shared-mount" -@pytest.mark.parametrize( - "groups_to_volume_mount", - (["/developer", "/admin", "/analyst"],), -) @pytest.mark.filterwarnings( "ignore:.*auto_refresh_token is deprecated:DeprecationWarning" ) @pytest.mark.filterwarnings("ignore::urllib3.exceptions.InsecureRequestWarning") -def test_groups_with_mount_permissions( - groups_to_volume_mount, groups_to_not_volume_mount -): +def test_groups_with_mount_permissions(): client_role = get_keycloak_client_role( client_name="jupyterhub", role_name="allow-group-directory-creation-role" ) @@ -93,7 +83,11 @@ def test_groups_with_mount_permissions( role_groups = get_keycloak_role_groups( client_id=client_details["id"], role_name=client_role["name"] ) - assert list([group["path"] for group in role_groups]) == groups_to_volume_mount + assert list([group["path"] for group in role_groups]) == [ + "/developer", + "/admin", + "/analyst", + ] @pytest.mark.parametrize( From 53cfcd39d54352e33b69b911c2d823f1fc91db0a Mon Sep 17 00:00:00 2001 From: "Vinicius D. Cerutti" <51954708+viniciusdc@users.noreply.github.com> Date: Mon, 19 Aug 2024 22:12:01 -0300 Subject: [PATCH 16/19] Apply suggestions from code review --- .../template/modules/kubernetes/services/jupyterhub/main.tf | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/main.tf b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/main.tf index bde0d03fa..30dd974c6 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/main.tf +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/main.tf @@ -307,10 +307,9 @@ module "jupyterhub-openid-client" { { "name" : "allow-group-directory-creation-role", "description" : "Grants a group the ability to manage the creation of its corresponding mounted directory.", - # Adding it to analyst group such that it's applied to every user. "groups" : ["admin", "analyst", "developer"], "attributes" : { - # grants permissions to read services + # grants permissions to mount group folder to shared dir "scopes" : "write:shared-mount", "component" : "shared-directory" } From 655b743e1959a575f83cab5fc475ccc4ec29daa9 Mon Sep 17 00:00:00 2001 From: "Vinicius D. Cerutti" <51954708+viniciusdc@users.noreply.github.com> Date: Tue, 20 Aug 2024 14:40:00 -0300 Subject: [PATCH 17/19] Update tests/tests_deployment/test_jupyterhub_api.py --- tests/tests_deployment/test_jupyterhub_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index f67303b3e..68d609ce0 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -83,11 +83,11 @@ def test_groups_with_mount_permissions(): role_groups = get_keycloak_role_groups( client_id=client_details["id"], role_name=client_role["name"] ) - assert list([group["path"] for group in role_groups]) == [ + assert set([group["path"] for group in role_groups]) == set([ "/developer", "/admin", "/analyst", - ] + ]) @pytest.mark.parametrize( From 6f0ad66a0cf259071d73dddaaeb1dbacb3cd0dd4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 20 Aug 2024 17:40:11 +0000 Subject: [PATCH 18/19] [pre-commit.ci] Apply automatic pre-commit fixes --- tests/tests_deployment/test_jupyterhub_api.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/tests_deployment/test_jupyterhub_api.py b/tests/tests_deployment/test_jupyterhub_api.py index 68d609ce0..242122800 100644 --- a/tests/tests_deployment/test_jupyterhub_api.py +++ b/tests/tests_deployment/test_jupyterhub_api.py @@ -83,11 +83,13 @@ def test_groups_with_mount_permissions(): role_groups = get_keycloak_role_groups( client_id=client_details["id"], role_name=client_role["name"] ) - assert set([group["path"] for group in role_groups]) == set([ - "/developer", - "/admin", - "/analyst", - ]) + assert set([group["path"] for group in role_groups]) == set( + [ + "/developer", + "/admin", + "/analyst", + ] + ) @pytest.mark.parametrize( From 0c5d9aaf129b77aa4685ca58bf45afa12bb65e98 Mon Sep 17 00:00:00 2001 From: "Vinicius D. Cerutti" <51954708+viniciusdc@users.noreply.github.com> Date: Fri, 30 Aug 2024 17:34:53 -0300 Subject: [PATCH 19/19] Missing list object for role components --- .../services/jupyterhub/files/jupyterhub/04-auth.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py index a97d66a23..2694b2a34 100644 --- a/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py +++ b/src/_nebari/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/files/jupyterhub/04-auth.py @@ -256,10 +256,10 @@ async def _get_users_and_groups_for_role( def _get_scope_from_role(self, role): """Return scopes from role if the component is jupyterhub""" role_scopes = role.get("attributes", {}).get("scopes", []) - component = role.get("attributes", {}).get("component") + component = role.get("attributes", {}).get("component", []) # Attributes are returned as a single-element array, unless `##` delimiter is used in Keycloak # See this: https://stackoverflow.com/questions/68954733/keycloak-client-role-attribute-array - if component == "jupyterhub" and role_scopes: + if component == ["jupyterhub"] and role_scopes: return self.validate_scopes(role_scopes[0].split(",")) else: return []