From dd129bb4f1e41ff60fd1dc2248fb251c197a2190 Mon Sep 17 00:00:00 2001 From: Thomas Li Fredriksen Date: Tue, 7 Feb 2023 14:03:29 +0100 Subject: [PATCH 1/7] Added group-management to azuread --- oauthenticator/azuread.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index d40d98be..4ebaab64 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -20,10 +20,11 @@ class AzureAdOAuthenticator(OAuthenticator): tenant_id = Unicode(config=True, help="The Azure Active Directory Tenant ID") user_auth_state_key = "user" + username_claim = "name" - @default("username_claim") - def _username_claim_default(self): - return "name" + user_groups_claim = Unicode( + "", config=True, help="Name of claim containing user group memberships" + ) @default('tenant_id') def _tenant_id_default(self): @@ -37,6 +38,12 @@ def _authorize_url_default(self): def _token_url_default(self): return f"https://login.microsoftonline.com/{self.tenant_id}/oauth2/token" + def build_auth_state_dict(self, token_info, user_info): + auth_state = super().build_auth_state_dict(token_info, user_info) + auth_state["groups"] = token_info.get(self.user_groups_claim) + + return auth_state + async def token_to_user(self, token_info): id_token = token_info['id_token'] decoded = jwt.decode( From 7b068a4df2b1641a7e80074cd13f0ad10669ac85 Mon Sep 17 00:00:00 2001 From: Thomas Li Fredriksen Date: Tue, 7 Feb 2023 14:33:24 +0100 Subject: [PATCH 2/7] Updated azuread. Updated azuread-tests --- oauthenticator/azuread.py | 15 ++++++++---- oauthenticator/tests/test_azuread.py | 35 +++++++++++++++++++++------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 4ebaab64..98c0d949 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -20,12 +20,15 @@ class AzureAdOAuthenticator(OAuthenticator): tenant_id = Unicode(config=True, help="The Azure Active Directory Tenant ID") user_auth_state_key = "user" - username_claim = "name" user_groups_claim = Unicode( "", config=True, help="Name of claim containing user group memberships" ) + @default("username_claim") + def _username_claim_default(self): + return "name" + @default('tenant_id') def _tenant_id_default(self): return os.environ.get('AAD_TENANT_ID', '') @@ -38,11 +41,13 @@ def _authorize_url_default(self): def _token_url_default(self): return f"https://login.microsoftonline.com/{self.tenant_id}/oauth2/token" - def build_auth_state_dict(self, token_info, user_info): - auth_state = super().build_auth_state_dict(token_info, user_info) - auth_state["groups"] = token_info.get(self.user_groups_claim) + async def update_auth_model(self, auth_model, **kwargs): + auth_model = await super().update_auth_model(auth_model, **kwargs) + + user_info = auth_model["auth_state"][self.user_auth_state_key] + auth_model["groups"] = user_info.get(self.user_groups_claim) - return auth_state + return auth_model async def token_to_user(self, token_info): id_token = token_info['id_token'] diff --git a/oauthenticator/tests/test_azuread.py b/oauthenticator/tests/test_azuread.py index d8c9ca77..ada5bd75 100644 --- a/oauthenticator/tests/test_azuread.py +++ b/oauthenticator/tests/test_azuread.py @@ -39,6 +39,16 @@ def user_model(tenant_id, client_id, name): "tid": tenant_id, "nonce": "123523", "aio": "Df2UVXL1ix!lMCWMSOJBcFatzcGfvFGhjKv8q5g0x732dR5MB5BisvGQO7YWByjd8iQDLq!eGbIDakyp5mnOrcdqHeYSnltepQmRp6AIZ8jY", + "groups": [ + "96000b2c-7333-4f6e-a2c3-e7608fa2d131", + "a992b3d5-1966-4af4-abed-6ef021417be4", + "ceb90a42-030f-44f1-a0c7-825b572a3b07", + ], + "grp": [ + "96000b2c-7333-4f6e-a2c3-e7608fa2d131", + "a992b3d5-1966-4af4-abed-6ef021417be4", + "ceb90a42-030f-44f1-a0c7-825b572a3b07", + ], }, os.urandom(5), ) @@ -61,26 +71,32 @@ def azure_client(client): @pytest.mark.parametrize( - 'username_claim', + 'username_claim, user_groups_claim, manage_groups', [ - None, - 'name', - 'oid', - 'preferred_username', + (None, None, False), + ('name', None, False), + ('oid', None, False), + ('preferred_username', None, False), + (None, None, True), + (None, "groups", True), + (None, "grp", True), ], ) -async def test_azuread(username_claim, azure_client): +async def test_azuread(username_claim, user_groups_claim, manage_groups, azure_client): cfg = Config() cfg.AzureAdOAuthenticator = Config( { "tenant_id": str(uuid.uuid1()), "client_id": str(uuid.uuid1()), "client_secret": str(uuid.uuid1()), + "manage_groups": manage_groups, } ) if username_claim: cfg.AzureAdOAuthenticator.username_claim = username_claim + if user_groups_claim: + cfg.AzureAdOAuthenticator.user_groups_claim = user_groups_claim authenticator = AzureAdOAuthenticator(config=cfg) @@ -93,8 +109,7 @@ async def test_azuread(username_claim, azure_client): ) user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] - + assert sorted(user_info) == ['auth_state', 'groups', 'name'] auth_state = user_info['auth_state'] assert 'access_token' in auth_state assert 'user' in auth_state @@ -108,3 +123,7 @@ async def test_azuread(username_claim, azure_client): else: # The default AzureADOAuthenticator `username_claim` is "name" assert username == auth_state_user_info["name"] + + if user_groups_claim: + groups = user_info['groups'] + assert groups == auth_state_user_info[user_groups_claim] From 04ce8ce15be3a022a9097de204155b6e6b863661 Mon Sep 17 00:00:00 2001 From: Thomas Li Fredriksen Date: Sat, 10 Jun 2023 12:38:03 +0200 Subject: [PATCH 3/7] Removed `user_groups_claim` dynamic default Co-authored-by: Georgiana --- oauthenticator/azuread.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 98c0d949..2eaa89bb 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -25,13 +25,12 @@ class AzureAdOAuthenticator(OAuthenticator): "", config=True, help="Name of claim containing user group memberships" ) - @default("username_claim") - def _username_claim_default(self): - return "name" - @default('tenant_id') def _tenant_id_default(self): return os.environ.get('AAD_TENANT_ID', '') + @default('username_claim') + def _username_claim_default(self): + return 'name' @default("authorize_url") def _authorize_url_default(self): From fd7f878e98efc6882860728609fd0880ac8d7c3d Mon Sep 17 00:00:00 2001 From: Thomas Li Fredriksen Date: Sat, 10 Jun 2023 13:12:57 +0200 Subject: [PATCH 4/7] Ran pre-commit hooks --- oauthenticator/azuread.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 2eaa89bb..7ea37965 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -28,6 +28,7 @@ class AzureAdOAuthenticator(OAuthenticator): @default('tenant_id') def _tenant_id_default(self): return os.environ.get('AAD_TENANT_ID', '') + @default('username_claim') def _username_claim_default(self): return 'name' @@ -44,7 +45,8 @@ async def update_auth_model(self, auth_model, **kwargs): auth_model = await super().update_auth_model(auth_model, **kwargs) user_info = auth_model["auth_state"][self.user_auth_state_key] - auth_model["groups"] = user_info.get(self.user_groups_claim) + if self.user_groups_claim: + auth_model["groups"] = user_info.get(self.user_groups_claim) return auth_model From a2c5c1aa0184d3862c344f6a016444cd476ef470 Mon Sep 17 00:00:00 2001 From: Thomas Li Fredriksen Date: Sat, 10 Jun 2023 13:20:19 +0200 Subject: [PATCH 5/7] Updated docs --- .../providers/azuread.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/source/tutorials/provider-specific-setup/providers/azuread.md b/docs/source/tutorials/provider-specific-setup/providers/azuread.md index cc038fe8..e19309de 100644 --- a/docs/source/tutorials/provider-specific-setup/providers/azuread.md +++ b/docs/source/tutorials/provider-specific-setup/providers/azuread.md @@ -53,3 +53,22 @@ 1. See `run.sh` for an [example](https://github.com/jupyterhub/oauthenticator/tree/main/examples/azuread) 1. [Source Code](https://github.com/jupyterhub/oauthenticator/blob/HEAD/oauthenticator/azuread.py) + +## 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 +import os +from oauthenticator.azuread import AzureAdOAuthenticator + +c.JupyterHub.authenticator_class = AzureAdOAuthenticator + +# {...} other settings (see above) + +c.AzureAdOAuthenticator.user_groups_claim = 'groups' +``` + +This requires Azure AD to be configured to include the group-membership in the access token. From 649841c23e7e625a8d981885534a9cbae2293964 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 29 Nov 2023 10:30:17 +0100 Subject: [PATCH 6/7] azuread: populate groups only if manage_groups is true allows user_groups_claim to have a default value --- .../provider-specific-setup/providers/azuread.md | 8 +++----- oauthenticator/azuread.py | 14 ++++++++++---- oauthenticator/tests/test_azuread.py | 12 ++++++++---- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/docs/source/tutorials/provider-specific-setup/providers/azuread.md b/docs/source/tutorials/provider-specific-setup/providers/azuread.md index 59c57e7e..09359d69 100644 --- a/docs/source/tutorials/provider-specific-setup/providers/azuread.md +++ b/docs/source/tutorials/provider-specific-setup/providers/azuread.md @@ -39,14 +39,12 @@ This is done by setting the `AzureAdOAuthenticator.groups_claim` to the name of group-membership. ```python -import os -from oauthenticator.azuread import AzureAdOAuthenticator - -c.JupyterHub.authenticator_class = AzureAdOAuthenticator +c.JupyterHub.authenticator_class = "azuread" # {...} other settings (see above) -c.AzureAdOAuthenticator.user_groups_claim = 'groups' +c.AzureAdOAuthenticator.manage_groups = True +c.AzureAdOAuthenticator.user_groups_claim = 'groups' # this is the default ``` This requires Azure AD to be configured to include the group-membership in the access token. diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 2bbd201f..48359864 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -22,7 +22,13 @@ def _username_claim_default(self): return "name" user_groups_claim = Unicode( - "", config=True, help="Name of claim containing user group memberships" + "groups", + config=True, + help=""" + Name of claim containing user group memberships. + + Will populate JupyterHub groups if Authenticator.manage_groups is True. + """, ) tenant_id = Unicode( @@ -51,9 +57,9 @@ def _token_url_default(self): async def update_auth_model(self, auth_model, **kwargs): auth_model = await super().update_auth_model(auth_model, **kwargs) - user_info = auth_model["auth_state"][self.user_auth_state_key] - if self.user_groups_claim: - auth_model["groups"] = user_info.get(self.user_groups_claim) + 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 diff --git a/oauthenticator/tests/test_azuread.py b/oauthenticator/tests/test_azuread.py index 2b8322c1..aa974b29 100644 --- a/oauthenticator/tests/test_azuread.py +++ b/oauthenticator/tests/test_azuread.py @@ -117,13 +117,17 @@ def user_model(tenant_id, client_id, name): # test user_groups_claim ( "30", - {"allow_all": True, "user_groups_claim": "groups"}, + {"allow_all": True, "manage_groups": True}, True, None, ), ( "31", - {"allow_all": True, "user_groups_claim": "grp"}, + { + "allow_all": True, + "manage_groups": True, + "user_groups_claim": "grp", + }, True, None, ), @@ -155,7 +159,7 @@ async def test_azuread( if expect_allowed: assert auth_model expected_keys = {"name", "admin", "auth_state"} - if authenticator.user_groups_claim: + if authenticator.manage_groups: expected_keys.add("groups") assert set(auth_model) == expected_keys assert auth_model["admin"] == expect_admin @@ -165,7 +169,7 @@ async def test_azuread( user_info = auth_state[authenticator.user_auth_state_key] assert user_info["aud"] == authenticator.client_id assert auth_model["name"] == user_info[authenticator.username_claim] - if authenticator.user_groups_claim: + if authenticator.manage_groups: groups = auth_model['groups'] assert groups == user_info[authenticator.user_groups_claim] else: From e29012d49df8b91e2423f78ad0c649cc006da4d0 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 29 Nov 2023 10:38:21 +0100 Subject: [PATCH 7/7] skip manage_groups tests on too-old jupyterhub --- oauthenticator/tests/test_azuread.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/oauthenticator/tests/test_azuread.py b/oauthenticator/tests/test_azuread.py index aa974b29..b3537dd5 100644 --- a/oauthenticator/tests/test_azuread.py +++ b/oauthenticator/tests/test_azuread.py @@ -7,6 +7,7 @@ from unittest import mock import jwt +import pytest from pytest import fixture, mark from traitlets.config import Config @@ -147,6 +148,12 @@ async def test_azuread( c.AzureAdOAuthenticator.client_id = str(uuid.uuid1()) c.AzureAdOAuthenticator.client_secret = str(uuid.uuid1()) authenticator = AzureAdOAuthenticator(config=c) + manage_groups = False + if "manage_groups" in class_config: + if hasattr(authenticator, "manage_groups"): + manage_groups = authenticator.manage_groups + else: + pytest.skip("manage_groups requires jupyterhub 2.2") handled_user_model = user_model( tenant_id=authenticator.tenant_id, @@ -159,7 +166,7 @@ async def test_azuread( if expect_allowed: assert auth_model expected_keys = {"name", "admin", "auth_state"} - if authenticator.manage_groups: + if manage_groups: expected_keys.add("groups") assert set(auth_model) == expected_keys assert auth_model["admin"] == expect_admin @@ -169,7 +176,7 @@ async def test_azuread( user_info = auth_state[authenticator.user_auth_state_key] assert user_info["aud"] == authenticator.client_id assert auth_model["name"] == user_info[authenticator.username_claim] - if authenticator.manage_groups: + if manage_groups: groups = auth_model['groups'] assert groups == user_info[authenticator.user_groups_claim] else: