From 561baf06aa507a52427bdf617eb0b1c8cf0ddea8 Mon Sep 17 00:00:00 2001 From: Ben Lewis Date: Tue, 28 Nov 2023 12:18:55 +0000 Subject: [PATCH 1/5] Support GenericOAuth managed groups --- oauthenticator/generic.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 31945d6a..43de2c2a 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -27,8 +27,8 @@ def _login_service_default(self): that accepts the returned json (as a dict) and returns the groups list. This configures how group membership in the upstream provider is determined - for use by `allowed_groups`, `admin_groups`, etc. - It has no effect on its own, and is not related to users' _JupyterHub_ group membership. + for use by `allowed_groups`, `admin_groups`, etc. If `manage_groups` is True, + this will also determine users' _JupyterHub_ group membership. """, ) @@ -153,16 +153,18 @@ async def update_auth_model(self, auth_model): Sets admin status to True or False if `admin_groups` is configured and the user isn't part of `admin_users` or `admin_groups`. Note that leaving it at None makes users able to retain an admin status while - setting it to False makes it be revoked. + setting it to False makes it be revoked. Also applies groups. """ + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_groups = self.get_user_groups(user_info) + auth_model["groups"] = user_groups + if auth_model["admin"]: # auth_model["admin"] being True means the user was in admin_users return auth_model if self.admin_groups: # admin status should in this case be True or False, not None - user_info = auth_model["auth_state"][self.user_auth_state_key] - user_groups = self.get_user_groups(user_info) auth_model["admin"] = bool(user_groups & self.admin_groups) return auth_model From b3ed2b784cc7dbb12139d64cbac03f2a93a74afa Mon Sep 17 00:00:00 2001 From: Ben Lewis Date: Tue, 28 Nov 2023 22:51:25 +0000 Subject: [PATCH 2/5] Only populate groups if explicitly requested --- oauthenticator/generic.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 43de2c2a..14052d9e 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -153,11 +153,16 @@ async def update_auth_model(self, auth_model): Sets admin status to True or False if `admin_groups` is configured and the user isn't part of `admin_users` or `admin_groups`. Note that leaving it at None makes users able to retain an admin status while - setting it to False makes it be revoked. Also applies groups. + setting it to False makes it be revoked. + + Also populates groups if `manage_groups` is set. """ - user_info = auth_model["auth_state"][self.user_auth_state_key] - user_groups = self.get_user_groups(user_info) - auth_model["groups"] = user_groups + if self.manage_groups or self.admin_groups: + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_groups = self.get_user_groups(user_info) + + if self.manage_groups: + auth_model["groups"] = user_groups if auth_model["admin"]: # auth_model["admin"] being True means the user was in admin_users From 648c623e181bd63f1b536ce611b1dac5996b0453 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 29 Nov 2023 12:10:54 +0100 Subject: [PATCH 3/5] handle manage_groups being unavailable before JupyterHub 2.2 --- oauthenticator/generic.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 14052d9e..2dce3fb3 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -157,11 +157,13 @@ async def update_auth_model(self, auth_model): Also populates groups if `manage_groups` is set. """ - if self.manage_groups or self.admin_groups: + # Authenticator.manage_groups is new in jupyterhub 2.2 + manage_groups = getattr(self, "manage_groups", False) + if manage_groups or self.admin_groups: user_info = auth_model["auth_state"][self.user_auth_state_key] user_groups = self.get_user_groups(user_info) - if self.manage_groups: + if manage_groups: auth_model["groups"] = user_groups if auth_model["admin"]: From 0b37825010398765df917b179b720cc320695d20 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 29 Nov 2023 12:19:05 +0100 Subject: [PATCH 4/5] [generic] test manage_groups --- oauthenticator/generic.py | 2 +- oauthenticator/tests/test_generic.py | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 2dce3fb3..3b4542ae 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -164,7 +164,7 @@ async def update_auth_model(self, auth_model): user_groups = self.get_user_groups(user_info) if manage_groups: - auth_model["groups"] = user_groups + auth_model["groups"] = sorted(user_groups) if auth_model["admin"]: # auth_model["admin"] being True means the user was in admin_users diff --git a/oauthenticator/tests/test_generic.py b/oauthenticator/tests/test_generic.py index 42dc7781..f374d941 100644 --- a/oauthenticator/tests/test_generic.py +++ b/oauthenticator/tests/test_generic.py @@ -1,6 +1,7 @@ import json from functools import partial +import pytest from pytest import fixture, mark from traitlets.config import Config @@ -151,6 +152,15 @@ def get_authenticator(generic_client): False, False, ), + ( + "20", + { + "manage_groups": True, + "allow_all": True, + }, + True, + None, + ), ], ) async def test_generic( @@ -166,6 +176,13 @@ async def test_generic( c.GenericOAuthenticator = Config(class_config) c.GenericOAuthenticator.username_claim = "username" authenticator = get_authenticator(config=c) + manage_groups = False + if "manage_groups" in class_config: + try: + manage_groups = authenticator.manage_groups + except AttributeError: + pytest.skip("manage_groups requires jupyterhub 2.2") + 1 / 0 handled_user_model = user_model("user1") handler = generic_client.handler_for_user(handled_user_model) @@ -173,7 +190,10 @@ async def test_generic( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + expected_keys = {"name", "admin", "auth_state"} + if manage_groups: + expected_keys.add("groups") + assert set(auth_model) == expected_keys assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) @@ -183,6 +203,9 @@ async def test_generic( assert "scope" in auth_state user_info = auth_state[authenticator.user_auth_state_key] assert auth_model["name"] == user_info[authenticator.username_claim] + if manage_groups: + assert auth_model["groups"] == user_info[authenticator.claim_groups_key] + else: assert auth_model == None From f6dab1178dfa0e0a3a1d903ce40e6c5d51072b73 Mon Sep 17 00:00:00 2001 From: Ben Lewis Date: Mon, 22 Jan 2024 23:35:00 +0000 Subject: [PATCH 5/5] Assume jupyterhub >= 2.2 --- oauthenticator/generic.py | 6 ++---- oauthenticator/tests/test_generic.py | 7 +------ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 3b4542ae..4dd88bb6 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -157,13 +157,11 @@ async def update_auth_model(self, auth_model): Also populates groups if `manage_groups` is set. """ - # Authenticator.manage_groups is new in jupyterhub 2.2 - manage_groups = getattr(self, "manage_groups", False) - if manage_groups or self.admin_groups: + if self.manage_groups or self.admin_groups: user_info = auth_model["auth_state"][self.user_auth_state_key] user_groups = self.get_user_groups(user_info) - if manage_groups: + if self.manage_groups: auth_model["groups"] = sorted(user_groups) if auth_model["admin"]: diff --git a/oauthenticator/tests/test_generic.py b/oauthenticator/tests/test_generic.py index f374d941..fcc96652 100644 --- a/oauthenticator/tests/test_generic.py +++ b/oauthenticator/tests/test_generic.py @@ -1,7 +1,6 @@ import json from functools import partial -import pytest from pytest import fixture, mark from traitlets.config import Config @@ -178,11 +177,7 @@ async def test_generic( authenticator = get_authenticator(config=c) manage_groups = False if "manage_groups" in class_config: - try: - manage_groups = authenticator.manage_groups - except AttributeError: - pytest.skip("manage_groups requires jupyterhub 2.2") - 1 / 0 + manage_groups = authenticator.manage_groups handled_user_model = user_model("user1") handler = generic_client.handler_for_user(handled_user_model)