From 63f66422efa2b3b1cc5738c95d25134082ab4b68 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Thu, 25 Apr 2024 12:39:21 -0700 Subject: [PATCH 1/2] Move allowed_groups and admin_groups to base authenticator A simplification of https://github.com/jupyterhub/oauthenticator/pull/735, moving 2 of the 3 traitlets. This is a straight up move, without any functional breaking changes. - `admin_groups` allows setting members of some groups as admins. - `allowed_groups` allows setting what groups should be allowed to login. Both of these are more useful with claim_groups_key, as that allows an *external* party to drive group memberships. Without that, I guess primarily this depends on membership within the JupyterHub admin UI. Splitting this up helps us get this moving faster, as figuring out how to move `claim_groups_key` is going to be slightly more involved. --- oauthenticator/generic.py | 71 --------------------------------------- oauthenticator/oauth2.py | 42 ++++++++++++++++++++++- 2 files changed, 41 insertions(+), 72 deletions(-) diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 2fd07be7..4a6f983f 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -33,34 +33,6 @@ def _login_service_default(self): """, ) - allowed_groups = Set( - Unicode(), - config=True, - help=""" - Allow members of selected groups to sign in. - - When configuring this you may need to configure `claim_groups_key` as - well as it determines the key in the `userdata_url` response that is - assumed to list the groups a user is a member of. - """, - ) - - admin_groups = Set( - Unicode(), - config=True, - help=""" - Allow members of selected groups to sign in and consider them as - JupyterHub admins. - - If this is set and a user isn't part of one of these groups or listed in - `admin_users`, a user signing in will have their admin status revoked. - - When configuring this you may need to configure `claim_groups_key` as - well as it determines the key in the `userdata_url` response that is - assumed to list the groups a user is a member of. - """, - ) - @default("http_client") def _default_http_client(self): return AsyncHTTPClient( @@ -124,49 +96,6 @@ def get_user_groups(self, user_info): ) return set() - 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 populates groups if `manage_groups` is set. - """ - 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"] = sorted(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 - auth_model["admin"] = bool(user_groups & self.admin_groups) - - return auth_model - - async def check_allowed(self, username, auth_model): - """ - Overrides the OAuthenticator.check_allowed to also allow users part of - `allowed_groups`. - """ - if await super().check_allowed(username, auth_model): - return True - - if self.allowed_groups: - user_info = auth_model["auth_state"][self.user_auth_state_key] - user_groups = self.get_user_groups(user_info) - if any(user_groups & self.allowed_groups): - return True - - # users should be explicitly allowed via config, otherwise they aren't - return False - class LocalGenericOAuthenticator(LocalAuthenticator, GenericOAuthenticator): """A version that mixes in local system user creation""" diff --git a/oauthenticator/oauth2.py b/oauthenticator/oauth2.py index 793643a0..b874c426 100644 --- a/oauthenticator/oauth2.py +++ b/oauthenticator/oauth2.py @@ -20,7 +20,7 @@ from tornado.httpclient import AsyncHTTPClient, HTTPClientError, HTTPRequest from tornado.httputil import url_concat from tornado.log import app_log -from traitlets import Any, Bool, Callable, Dict, List, Unicode, Union, default, validate +from traitlets import Any, Bool, Callable, Dict, List, Unicode, Union, default, Set, validate def guess_callback_uri(protocol, host, hub_server_url): @@ -316,6 +316,27 @@ class OAuthenticator(Authenticator): """, ) + allowed_groups = Set( + Unicode(), + config=True, + help=""" + Allow members of selected groups to log in. + """, + ) + + admin_groups = Set( + Unicode(), + config=True, + help=""" + Allow members of selected groups to sign in and consider them as + JupyterHub admins. + + If this is set and a user isn't part of one of these groups or listed in + `admin_users`, a user signing in will have their admin status revoked. + """, + ) + + authorize_url = Unicode( config=True, help=""" @@ -1010,6 +1031,18 @@ async def update_auth_model(self, auth_model): Called by the :meth:`oauthenticator.OAuthenticator.authenticate` """ + 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"] = sorted(user_groups) + + if self.admin_groups: + if not auth_model["admin"]: + # auth_model["admin"] being True means the user was in admin_users + # so their group membership should not affect their admin status + auth_model["admin"] = bool(user_groups & self.admin_groups) return auth_model async def authenticate(self, handler, data=None, **kwargs): @@ -1087,6 +1120,13 @@ async def check_allowed(self, username, auth_model): if username in self.allowed_users: return True + # allow users who are members of allowed_groups + if self.allowed_groups: + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_groups = self.get_user_groups(user_info) + if any(user_groups & self.allowed_groups): + return True + # users should be explicitly allowed via config, otherwise they aren't return False From 4283e14c346c54e354aa912e72a36e2073bec15f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 25 Apr 2024 19:46:41 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- oauthenticator/generic.py | 2 +- oauthenticator/oauth2.py | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 4a6f983f..f2ca7000 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -8,7 +8,7 @@ from jupyterhub.auth import LocalAuthenticator from jupyterhub.traitlets import Callable from tornado.httpclient import AsyncHTTPClient -from traitlets import Bool, Dict, Set, Unicode, Union, default +from traitlets import Bool, Dict, Unicode, Union, default from .oauth2 import OAuthenticator diff --git a/oauthenticator/oauth2.py b/oauthenticator/oauth2.py index b874c426..8e18f8ba 100644 --- a/oauthenticator/oauth2.py +++ b/oauthenticator/oauth2.py @@ -20,7 +20,18 @@ from tornado.httpclient import AsyncHTTPClient, HTTPClientError, HTTPRequest from tornado.httputil import url_concat from tornado.log import app_log -from traitlets import Any, Bool, Callable, Dict, List, Unicode, Union, default, Set, validate +from traitlets import ( + Any, + Bool, + Callable, + Dict, + List, + Set, + Unicode, + Union, + default, + validate, +) def guess_callback_uri(protocol, host, hub_server_url): @@ -336,7 +347,6 @@ class OAuthenticator(Authenticator): """, ) - authorize_url = Unicode( config=True, help="""