diff --git a/docs/source/tutorials/general-setup.md b/docs/source/tutorials/general-setup.md index 772b4e8f..920a3128 100644 --- a/docs/source/tutorials/general-setup.md +++ b/docs/source/tutorials/general-setup.md @@ -60,7 +60,9 @@ projects' authenticator classes. - {attr}`.OAuthenticator.allow_all` - {attr}`.OAuthenticator.allow_existing_users` - {attr}`.OAuthenticator.allowed_users` + - {attr}`.OAuthenticator.allowed_groups` - {attr}`.OAuthenticator.admin_users` + - {attr}`.OAuthenticator.admin_groups` Your authenticator class may have unique config, so in the end it can look something like this: diff --git a/docs/source/tutorials/provider-specific-setup/providers/azuread.md b/docs/source/tutorials/provider-specific-setup/providers/azuread.md index 09359d69..7df9217e 100644 --- a/docs/source/tutorials/provider-specific-setup/providers/azuread.md +++ b/docs/source/tutorials/provider-specific-setup/providers/azuread.md @@ -35,8 +35,6 @@ be relevant to read more about in the configuration reference: ## Loading user groups The `AzureAdOAuthenticator` can load the group-membership of users from the access token. -This is done by setting the `AzureAdOAuthenticator.groups_claim` to the name of the claim that contains the -group-membership. ```python c.JupyterHub.authenticator_class = "azuread" @@ -44,7 +42,7 @@ c.JupyterHub.authenticator_class = "azuread" # {...} other settings (see above) c.AzureAdOAuthenticator.manage_groups = True -c.AzureAdOAuthenticator.user_groups_claim = 'groups' # this is the default +c.AzureAdOAuthenticator.auth_state_groups_key = "user.groups" # this is the default ``` This requires Azure AD to be configured to include the group-membership in the access token. diff --git a/docs/source/tutorials/provider-specific-setup/providers/generic.md b/docs/source/tutorials/provider-specific-setup/providers/generic.md index 386116af..fc8fe390 100644 --- a/docs/source/tutorials/provider-specific-setup/providers/generic.md +++ b/docs/source/tutorials/provider-specific-setup/providers/generic.md @@ -35,7 +35,7 @@ c.GenericOAuthenticator.userdata_url = "https://accounts.example.com/auth/realms # c.GenericOAuthenticator.scope = ["openid", "email", "groups"] c.GenericOAuthenticator.username_claim = "email" -c.GenericOAuthenticator.claim_groups_key = "groups" +c.GenericOAuthenticator.auth_state_groups_key = "oauth_user.groups" # Authorization # ------------- diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index e4e6682b..dec2c2b9 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -23,15 +23,26 @@ def _username_claim_default(self): return "name" user_groups_claim = Unicode( - "groups", + "", config=True, help=""" - Name of claim containing user group memberships. + .. deprecated:: 17.0 - Will populate JupyterHub groups if Authenticator.manage_groups is True. + Use :attr:`auth_state_groups_key` instead. """, ) + @default('auth_state_groups_key') + def _auth_state_groups_key_default(self): + key = "user.groups" + if self.user_groups_claim: + key = f"{self.user_auth_state_key}.{self.user_groups_claim}" + cls = self.__class__.__name__ + self.log.warning( + f"{cls}.user_groups_claim is deprecated in OAuthenticator 17. Use {cls}.auth_state_groups_key = {key!r}" + ) + return key + tenant_id = Unicode( config=True, help=""" @@ -55,15 +66,6 @@ def _authorize_url_default(self): def _token_url_default(self): return f"https://login.microsoftonline.com/{self.tenant_id}/oauth2/token" - async def update_auth_model(self, auth_model, **kwargs): - auth_model = await super().update_auth_model(auth_model, **kwargs) - - if getattr(self, "manage_groups", False): - user_info = auth_model["auth_state"][self.user_auth_state_key] - auth_model["groups"] = user_info[self.user_groups_claim] - - return auth_model - async def token_to_user(self, token_info): id_token = token_info['id_token'] decoded = jwt.decode( diff --git a/oauthenticator/globus.py b/oauthenticator/globus.py index 266789e9..30a49484 100644 --- a/oauthenticator/globus.py +++ b/oauthenticator/globus.py @@ -342,7 +342,7 @@ async def update_auth_model(self, auth_model): to False makes it be revoked. """ user_groups = set() - if self.allowed_globus_groups or self.admin_globus_groups: + if self.allowed_globus_groups or self.admin_globus_groups or self.manage_groups: tokens = self.get_globus_tokens(auth_model["auth_state"]["token_response"]) user_groups = await self._fetch_users_groups(tokens) # sets are not JSONable, cast to list for auth_state diff --git a/oauthenticator/oauth2.py b/oauthenticator/oauth2.py index 5daa3574..8c62a67c 100644 --- a/oauthenticator/oauth2.py +++ b/oauthenticator/oauth2.py @@ -1103,7 +1103,7 @@ def build_auth_state_dict(self, token_info, user_info): Called by the :meth:`oauthenticator.OAuthenticator.authenticate` .. versionchanged:: 17.0 - This method be async. + This method may be async. """ # We know for sure the `access_token` key exists, oterwise we would have errored out already @@ -1133,6 +1133,8 @@ async def get_user_groups(self, auth_state: dict): Returns a set of groups the user belongs to based on auth_state_groups_key and provided auth_state. + Only called when :attr:`manage_groups` is True. + - If auth_state_groups_key is a callable, it returns the list of groups directly. Callable may be async. - If auth_state_groups_key is a nested dictionary key like @@ -1168,11 +1170,23 @@ async def update_auth_model(self, auth_model): - `name`: the normalized username - `admin`: the admin status (True/False/None), where None means it should be unchanged. - - `auth_state`: the dictionary of of auth state - returned by :meth:`oauthenticator.OAuthenticator.build_auth_state_dict` + - `auth_state`: the auth state dictionary, + returned by :meth:`oauthenticator.OAuthenticator.build_auth_state_dict` Called by the :meth:`oauthenticator.OAuthenticator.authenticate` """ + # NOTE: this base implementation should _not_ be updated to do anything + # subclasses should have full control without calling super() + return auth_model + + async def _apply_managed_groups(self, auth_model): + """Applies managed_groups logic + + Called after `update_auth_model` to populate the `groups` field. + Only called if `manage_groups` is True. + + The public method for subclasses to override is `.get_user_groups`. + """ if self.manage_groups: auth_state = auth_model["auth_state"] user_groups = self.get_user_groups(auth_state) @@ -1244,7 +1258,10 @@ async def authenticate(self, handler, data=None, **kwargs): # update the auth_model with info to later authorize the user in # check_allowed, such as admin status and group memberships - return await self.update_auth_model(auth_model) + auth_model = await self.update_auth_model(auth_model) + if self.manage_groups: + auth_model = await self._apply_managed_groups(auth_model) + return auth_model async def check_allowed(self, username, auth_model): """ @@ -1289,12 +1306,8 @@ async def check_allowed(self, username, auth_model): return True # allow users who are members of allowed_groups - if self.manage_groups and self.allowed_groups: - auth_state = auth_model["auth_state"] - user_groups = self.get_user_groups(auth_state) - if isawaitable(user_groups): - user_groups = await user_groups - if any(user_groups & self.allowed_groups): + if self.manage_groups and self.allowed_groups and auth_model.get("groups"): + if set(auth_model["groups"]) & self.allowed_groups: return True # users should be explicitly allowed via config, otherwise they aren't diff --git a/oauthenticator/openshift.py b/oauthenticator/openshift.py index 6b016b0d..3f0c4646 100644 --- a/oauthenticator/openshift.py +++ b/oauthenticator/openshift.py @@ -8,7 +8,7 @@ from jupyterhub.auth import LocalAuthenticator from tornado.httpclient import HTTPClient, HTTPRequest -from traitlets import Bool, Set, Unicode, default +from traitlets import Bool, Unicode, default from oauthenticator.oauth2 import OAuthenticator @@ -16,6 +16,10 @@ class OpenShiftOAuthenticator(OAuthenticator): user_auth_state_key = "openshift_user" + @default("auth_state_groups_key") + def _auth_state_groups_key_default(self): + return "openshift_user.groups" + @default("scope") def _scope_default(self): return ["user:info"] @@ -45,24 +49,6 @@ def _http_request_kwargs_default(self): """, ) - allowed_groups = Set( - config=True, - help=""" - Allow members of selected OpenShift groups to sign in. - """, - ) - - admin_groups = Set( - config=True, - help=""" - Allow members of selected OpenShift 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. - """, - ) - openshift_auth_api_url = Unicode( config=True, help=""" @@ -158,42 +144,6 @@ def user_info_to_username(self, user_info): """ return user_info['metadata']['name'] - 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`. Note that leaving it at None makes - users able to retain an admin status while setting it to False makes it - be revoked. - """ - 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 = set(user_info["groups"]) - auth_model["admin"] = bool(user_groups & self.admin_groups) - - return auth_model - - async def check_allowed(self, username, auth_model): - """ - Overrides 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 = set(user_info["groups"]) - if user_groups & self.allowed_groups: - return True - - # users should be explicitly allowed via config, otherwise they aren't - return False - class LocalOpenShiftOAuthenticator(LocalAuthenticator, OpenShiftOAuthenticator): """A version that mixes in local system user creation""" diff --git a/oauthenticator/tests/test_auth0.py b/oauthenticator/tests/test_auth0.py index e4ae3be9..78a9b42f 100644 --- a/oauthenticator/tests/test_auth0.py +++ b/oauthenticator/tests/test_auth0.py @@ -29,6 +29,7 @@ def user_model(): return { "email": "user1@example.com", "name": "user1", + "groups": ["group1"], } @@ -62,6 +63,47 @@ def user_model(): True, True, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "auth_state_groups_key": "auth0_user.groups", + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "auth0_user.groups", + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "auth_state_groups_key": "auth0_user.groups", + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "auth0_user.groups", + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_auth0( @@ -84,7 +126,10 @@ async def test_auth0( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) diff --git a/oauthenticator/tests/test_azuread.py b/oauthenticator/tests/test_azuread.py index 2bf7606c..77a7c217 100644 --- a/oauthenticator/tests/test_azuread.py +++ b/oauthenticator/tests/test_azuread.py @@ -1,6 +1,7 @@ """test azure ad""" import json +import logging import os import re import time @@ -9,7 +10,7 @@ import jwt import pytest -from pytest import fixture, mark +from pytest import fixture, mark, raises from traitlets.config import Config from ..azuread import AzureAdOAuthenticator @@ -49,6 +50,7 @@ def user_model(tenant_id, client_id, name): "96000b2c-7333-4f6e-a2c3-e7608fa2d131", "a992b3d5-1966-4af4-abed-6ef021417be4", "ceb90a42-030f-44f1-a0c7-825b572a3b07", + "group1", ], # different from 'groups' for tests "grp": [ @@ -115,10 +117,14 @@ def user_model(tenant_id, client_id, name): True, None, ), - # test user_groups_claim + # test user_groups_claim (deprecated) ( "30", - {"allow_all": True, "manage_groups": True}, + { + "allow_all": True, + "user_groups_claim": "groups", + "manage_groups": True, + }, True, None, ), @@ -132,6 +138,43 @@ def user_model(tenant_id, client_id, name): True, None, ), + # common tests with allowed_groups and manage_groups + ( + "40", + { + "allowed_groups": {"group1"}, + "manage_groups": True, + }, + True, + None, + ), + ( + "41", + { + "allowed_groups": {"test-user-not-in-group"}, + "manage_groups": True, + }, + False, + None, + ), + ( + "42", + { + "admin_groups": {"group1"}, + "manage_groups": True, + }, + True, + True, + ), + ( + "43", + { + "admin_groups": {"test-user-not-in-group"}, + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_azuread( @@ -178,9 +221,11 @@ async def test_azuread( assert auth_model["name"] == user_info[authenticator.username_claim] if manage_groups: groups = auth_model['groups'] - assert groups == user_info[authenticator.user_groups_claim] + assert ( + groups == user_info[authenticator.auth_state_groups_key.rsplit(".")[-1]] + ) else: - assert auth_model == None + assert auth_model is None async def test_tenant_id_from_env(): @@ -188,3 +233,47 @@ async def test_tenant_id_from_env(): with mock.patch.dict(os.environ, {"AAD_TENANT_ID": tenant_id}): aad = AzureAdOAuthenticator() assert aad.tenant_id == tenant_id + + +@mark.parametrize( + "test_variation_id,class_config,expect_config,expect_loglevel,expect_message", + [ + ( + "user_groups_claim", + {"user_groups_claim": "groups", "manage_groups": True}, + {"auth_state_groups_key": "user.groups"}, + logging.WARNING, + "AzureAdOAuthenticator.user_groups_claim is deprecated in OAuthenticator 17. Use AzureAdOAuthenticator.auth_state_groups_key = 'user.groups'", + ), + ], +) +async def test_deprecated_config( + caplog, + test_variation_id, + class_config, + expect_config, + expect_loglevel, + expect_message, +): + """ + Tests that a warning is emitted when using a deprecated config and that + configuring the old config ends up configuring the new config. + """ + print(f"Running test variation id {test_variation_id}") + c = Config() + c.AzureAdOAuthenticator = Config(class_config) + + test_logger = logging.getLogger('testlog') + if expect_loglevel == logging.ERROR: + with raises(ValueError, match=expect_message): + AzureAdOAuthenticator(config=c, log=test_logger) + else: + authenticator = AzureAdOAuthenticator(config=c, log=test_logger) + for key, value in expect_config.items(): + assert getattr(authenticator, key) == value + + captured_log_tuples = caplog.record_tuples + print(captured_log_tuples) + + expected_log_tuple = (test_logger.name, expect_loglevel, expect_message) + assert expected_log_tuple in captured_log_tuples diff --git a/oauthenticator/tests/test_bitbucket.py b/oauthenticator/tests/test_bitbucket.py index 512e8030..37d776ce 100644 --- a/oauthenticator/tests/test_bitbucket.py +++ b/oauthenticator/tests/test_bitbucket.py @@ -45,6 +45,7 @@ def user_model(username): """ return { "username": username, + "groups": ["group1"], } @@ -80,6 +81,47 @@ def user_model(username): True, True, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "auth_state_groups_key": "bitbucket_user.groups", + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "bitbucket_user.groups", + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "auth_state_groups_key": "bitbucket_user.groups", + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "bitbucket_user.groups", + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_bitbucket( @@ -101,7 +143,10 @@ async def test_bitbucket( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) diff --git a/oauthenticator/tests/test_cilogon.py b/oauthenticator/tests/test_cilogon.py index 4dac4927..2b830a43 100644 --- a/oauthenticator/tests/test_cilogon.py +++ b/oauthenticator/tests/test_cilogon.py @@ -27,6 +27,7 @@ def user_model(username, username_claim, **kwargs): return { username_claim: username, "idp": "https://some-idp.com/login/oauth/authorize", + "groups": ["group1"], **kwargs, } @@ -61,6 +62,47 @@ def user_model(username, username_claim, **kwargs): True, True, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "auth_state_groups_key": "cilogon_user.groups", + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "cilogon_user.groups", + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "auth_state_groups_key": "cilogon_user.groups", + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "cilogon_user.groups", + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_cilogon( @@ -88,7 +130,10 @@ async def test_cilogon( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) @@ -416,7 +461,10 @@ async def test_cilogon_allowed_idps( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) diff --git a/oauthenticator/tests/test_generic.py b/oauthenticator/tests/test_generic.py index 11da1026..e3225039 100644 --- a/oauthenticator/tests/test_generic.py +++ b/oauthenticator/tests/test_generic.py @@ -1,4 +1,5 @@ import json +import logging import re from functools import partial @@ -104,20 +105,6 @@ def get_authenticator_variant(generic_client, userdata_from_id_token): ("03", {"allowed_users": {"not-test-user"}}, False, None), ("04", {"admin_users": {"user1"}}, True, True), ("05", {"admin_users": {"not-test-user"}}, False, None), - ("06", {"allowed_groups": {"group1"}, "manage_groups": True}, True, None), - ( - "07", - {"allowed_groups": {"test-user-not-in-group"}, "manage_groups": True}, - False, - None, - ), - ("08", {"admin_groups": {"group1"}, "manage_groups": True}, True, True), - ( - "09", - {"admin_groups": {"test-user-not-in-group"}, "manage_groups": True}, - False, - False, - ), # allow config, some combinations of two tested ( "10", @@ -228,6 +215,21 @@ def get_authenticator_variant(generic_client, userdata_from_id_token): True, None, ), + # common tests with allowed_groups and manage_groups + ("20", {"allowed_groups": {"group1"}, "manage_groups": True}, True, None), + ( + "21", + {"allowed_groups": {"test-user-not-in-group"}, "manage_groups": True}, + False, + None, + ), + ("22", {"admin_groups": {"group1"}, "manage_groups": True}, True, True), + ( + "23", + {"admin_groups": {"test-user-not-in-group"}, "manage_groups": True}, + False, + False, + ), ], ) async def test_generic( @@ -501,3 +503,47 @@ async def test_check_allowed_no_auth_state(get_authenticator, name, allowed): # these are previously-allowed users who should pass until subsequent # this check is removed in JupyterHub 5 assert await authenticator.check_allowed(name, None) + + +@mark.parametrize( + "test_variation_id,class_config,expect_config,expect_loglevel,expect_message", + [ + ( + "claim_groups_key", + {"claim_groups_key": "groups", "manage_groups": True}, + {"auth_state_groups_key": "oauth_user.groups"}, + logging.WARNING, + "GenericOAuthenticator.claim_groups_key is deprecated since OAuthenticator 17.0, use GenericOAuthenticator.auth_state_groups_key instead", + ), + ], +) +async def test_deprecated_config( + caplog, + test_variation_id, + class_config, + expect_config, + expect_loglevel, + expect_message, +): + """ + Tests that a warning is emitted when using a deprecated config and that + configuring the old config ends up configuring the new config. + """ + print(f"Running test variation id {test_variation_id}") + c = Config() + c.GenericOAuthenticator = Config(class_config) + + test_logger = logging.getLogger('testlog') + if expect_loglevel == logging.ERROR: + with raises(ValueError, match=expect_message): + GenericOAuthenticator(config=c, log=test_logger) + else: + authenticator = GenericOAuthenticator(config=c, log=test_logger) + for key, value in expect_config.items(): + assert getattr(authenticator, key) == value + + captured_log_tuples = caplog.record_tuples + print(captured_log_tuples) + + expected_log_tuple = (test_logger.name, expect_loglevel, expect_message) + assert expected_log_tuple in captured_log_tuples diff --git a/oauthenticator/tests/test_github.py b/oauthenticator/tests/test_github.py index 35b64a46..e49fe064 100644 --- a/oauthenticator/tests/test_github.py +++ b/oauthenticator/tests/test_github.py @@ -21,6 +21,7 @@ def user_model(username): 'id': 5, 'login': username, 'name': 'Hoban Washburn', + "groups": ["group1"], } @@ -66,6 +67,47 @@ def github_client(client): True, True, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "auth_state_groups_key": "github_user.groups", + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "github_user.groups", + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "auth_state_groups_key": "github_user.groups", + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "github_user.groups", + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_github( @@ -87,7 +129,10 @@ async def test_github( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) diff --git a/oauthenticator/tests/test_gitlab.py b/oauthenticator/tests/test_gitlab.py index f8d7ad3e..86d0c891 100644 --- a/oauthenticator/tests/test_gitlab.py +++ b/oauthenticator/tests/test_gitlab.py @@ -28,6 +28,7 @@ def user_model(username): return { "username": username, "id": id, + "groups": ["group1"], } @@ -86,6 +87,47 @@ def mock_version_response(request): True, True, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "auth_state_groups_key": "gitlab_user.groups", + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "gitlab_user.groups", + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "auth_state_groups_key": "gitlab_user.groups", + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "gitlab_user.groups", + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_gitlab( @@ -107,7 +149,10 @@ async def test_gitlab( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) diff --git a/oauthenticator/tests/test_globus.py b/oauthenticator/tests/test_globus.py index 95ade113..906de7c2 100644 --- a/oauthenticator/tests/test_globus.py +++ b/oauthenticator/tests/test_globus.py @@ -17,6 +17,7 @@ def user_model(username, **kwargs): """Return a user model""" return { "preferred_username": username, + "groups": ["group1"], **kwargs, } @@ -279,6 +280,47 @@ async def save_auth_state(self, state): False, False, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "auth_state_groups_key": "globus_user.groups", + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "globus_user.groups", + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "auth_state_groups_key": "globus_user.groups", + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "globus_user.groups", + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_globus( @@ -300,7 +342,10 @@ async def test_globus( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py index 3aeb652d..7dc7c35f 100644 --- a/oauthenticator/tests/test_google.py +++ b/oauthenticator/tests/test_google.py @@ -18,6 +18,7 @@ def user_model(email, username="user1", hd=None): 'email': email, 'custom': username, 'verified_email': True, + 'groups': ['group1'], } if hd: model['hd'] = hd @@ -151,6 +152,47 @@ def google_client(client): False, False, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "auth_state_groups_key": "google_user.groups", + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "google_user.groups", + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "auth_state_groups_key": "google_user.groups", + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "google_user.groups", + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_google( @@ -175,7 +217,10 @@ async def test_google( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) diff --git a/oauthenticator/tests/test_mediawiki.py b/oauthenticator/tests/test_mediawiki.py index 3510b74d..794ba377 100644 --- a/oauthenticator/tests/test_mediawiki.py +++ b/oauthenticator/tests/test_mediawiki.py @@ -25,6 +25,7 @@ def post_token(request, context): 'iss': 'https://meta.wikimedia.org', 'iat': time.time(), 'nonce': request_nonce, + 'groups': ['group1'], }, 'client_secret', ).encode() @@ -74,6 +75,47 @@ def post_token(request, context): True, True, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "auth_state_groups_key": "MEDIAWIKI_USER_IDENTITY.groups", + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "MEDIAWIKI_USER_IDENTITY.groups", + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "auth_state_groups_key": "MEDIAWIKI_USER_IDENTITY.groups", + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "auth_state_groups_key": "MEDIAWIKI_USER_IDENTITY.groups", + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_mediawiki( @@ -101,7 +143,10 @@ async def test_mediawiki( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"] assert json.dumps(auth_state) @@ -110,7 +155,7 @@ async def test_mediawiki( user_info = auth_state[authenticator.user_auth_state_key] assert auth_model["name"] == user_info[authenticator.username_claim] else: - assert auth_model == None + assert auth_model is None async def test_login_redirect(mediawiki): diff --git a/oauthenticator/tests/test_openshift.py b/oauthenticator/tests/test_openshift.py index 8421cf06..8b87f0e5 100644 --- a/oauthenticator/tests/test_openshift.py +++ b/oauthenticator/tests/test_openshift.py @@ -37,20 +37,6 @@ def user_model(): ("03", {"allowed_users": {"not-test-user"}}, False, None), ("04", {"admin_users": {"user1"}}, True, True), ("05", {"admin_users": {"not-test-user"}}, False, None), - ("06", {"allowed_groups": {"group1"}, "manage_groups": True}, True, None), - ( - "07", - {"allowed_groups": {"test-user-not-in-group"}, "manage_groups": True}, - False, - None, - ), - ("08", {"admin_groups": {"group1"}, "manage_groups": True}, True, True), - ( - "09", - {"admin_groups": {"test-user-not-in-group"}, "manage_groups": True}, - False, - False, - ), # allow config, some combinations of two tested ( "10", @@ -150,6 +136,43 @@ def user_model(): False, False, ), + # common tests with allowed_groups and manage_groups + ( + "20", + { + "allowed_groups": {"group1"}, + "manage_groups": True, + }, + True, + None, + ), + ( + "21", + { + "allowed_groups": {"test-user-not-in-group"}, + "manage_groups": True, + }, + False, + None, + ), + ( + "22", + { + "admin_groups": {"group1"}, + "manage_groups": True, + }, + True, + True, + ), + ( + "23", + { + "admin_groups": {"test-user-not-in-group"}, + "manage_groups": True, + }, + False, + False, + ), ], ) async def test_openshift( @@ -173,7 +196,10 @@ async def test_openshift( if expect_allowed: assert auth_model - assert set(auth_model) == {"name", "admin", "auth_state"} + if authenticator.manage_groups: + assert set(auth_model) == {"name", "admin", "auth_state", "groups"} + else: + assert set(auth_model) == {"name", "admin", "auth_state"} assert auth_model["name"] == handled_user_model["metadata"]["name"] assert auth_model["admin"] == expect_admin auth_state = auth_model["auth_state"]