From 79907e85ad720efcfe0b06a1eb534b74168dce5b Mon Sep 17 00:00:00 2001 From: mathias Date: Wed, 27 Oct 2021 11:17:42 +0200 Subject: [PATCH 1/2] Add azuread allowed_groups, admin_groups and tests for the groups --- oauthenticator/azuread.py | 26 ++++++++ oauthenticator/tests/test_azuread.py | 98 +++++++++++++++++++++------- 2 files changed, 101 insertions(+), 23 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index bd7bb9ad..3e436291 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -10,6 +10,7 @@ from tornado.httpclient import HTTPRequest from traitlets import default from traitlets import Unicode +from traitlets import List from .oauth2 import OAuthenticator @@ -50,6 +51,22 @@ def _token_url_default(self): self.tenant_id ) + allowed_groups = List( + Unicode(), + config=True, + help="Automatically allow members of selected groups", + ) + + admin_groups = List( + Unicode(), + config=True, + help="Groups whose members should have Jupyterhub admin privileges", + ) + + @staticmethod + def check_user_in_groups(member_groups, allowed_groups): + return bool(set(member_groups) & set(allowed_groups)) + async def authenticate(self, handler, data=None): code = handler.get_argument("code") @@ -94,6 +111,15 @@ async def authenticate(self, handler, data=None): # results in a decoded JWT for the user data auth_state['user'] = decoded + groups = self.allowed_groups + self.admin_groups + if groups: + ad_groups = decoded.get('groups') + if self.check_user_in_groups(ad_groups, groups): + userdict['admin'] = self.check_user_in_groups( + ad_groups, self.admin_groups + ) + else: + userdict = None return userdict diff --git a/oauthenticator/tests/test_azuread.py b/oauthenticator/tests/test_azuread.py index 35c8a2bf..88dd30a9 100644 --- a/oauthenticator/tests/test_azuread.py +++ b/oauthenticator/tests/test_azuread.py @@ -3,8 +3,13 @@ import re import time import uuid + +from functools import partial + from unittest import mock +from pytest import fixture + import jwt import pytest @@ -19,26 +24,30 @@ def test_tenant_id_from_env(): assert aad.tenant_id == tenant_id -def user_model(tenant_id, client_id, name): +def user_model(tenant_id, client_id, name, **kwargs): """Return a user model""" # model derived from https://docs.microsoft.com/en-us/azure/active-directory/develop/id-tokens#v20 now = int(time.time()) + + user = { + "ver": "2.0", + "iss": f"https://login.microsoftonline.com/{tenant_id}/v2.0", + "sub": "AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ", + "aud": client_id, + "exp": now + 3600, + "iat": now, + "nbf": now, + "name": name, + "preferred_username": name, + "oid": str(uuid.uuid1()), + "tid": tenant_id, + "nonce": "123523", + "aio": "Df2UVXL1ix!lMCWMSOJBcFatzcGfvFGhjKv8q5g0x732dR5MB5BisvGQO7YWByjd8iQDLq!eGbIDakyp5mnOrcdqHeYSnltepQmRp6AIZ8jY", + } + user.update(kwargs) + id_token = jwt.encode( - { - "ver": "2.0", - "iss": f"https://login.microsoftonline.com/{tenant_id}/v2.0", - "sub": "AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ", - "aud": client_id, - "exp": now + 3600, - "iat": now, - "nbf": now, - "name": name, - "preferred_username": name, - "oid": str(uuid.uuid1()), - "tid": tenant_id, - "nonce": "123523", - "aio": "Df2UVXL1ix!lMCWMSOJBcFatzcGfvFGhjKv8q5g0x732dR5MB5BisvGQO7YWByjd8iQDLq!eGbIDakyp5mnOrcdqHeYSnltepQmRp6AIZ8jY", - }, + user, os.urandom(5), ).decode("ascii") @@ -48,6 +57,15 @@ def user_model(tenant_id, client_id, name): } +def _get_authenticator(**kwargs): + return AzureAdOAuthenticator( + tenant_id=str(uuid.uuid1()), + client_id=str(uuid.uuid1()), + client_secret=str(uuid.uuid1()), + **kwargs + ) + + @pytest.fixture def azure_client(client): setup_oauth_mock( @@ -59,6 +77,11 @@ def azure_client(client): return client +@fixture +def get_authenticator(azure_client, **kwargs): + return partial(_get_authenticator, http_client=azure_client) + + @pytest.mark.parametrize( 'username_claim', [ @@ -68,12 +91,8 @@ def azure_client(client): 'preferred_username', ], ) -async def test_azuread(username_claim, azure_client): - authenticator = AzureAdOAuthenticator( - tenant_id=str(uuid.uuid1()), - client_id=str(uuid.uuid1()), - client_secret=str(uuid.uuid1()), - ) +async def test_azuread(get_authenticator, username_claim, azure_client): + authenticator = get_authenticator() if username_claim: authenticator.username_claim = username_claim @@ -81,7 +100,7 @@ async def test_azuread(username_claim, azure_client): user_model( tenant_id=authenticator.tenant_id, client_id=authenticator.client_id, - name="somebody", + name="somebody" ) ) @@ -95,3 +114,36 @@ async def test_azuread(username_claim, azure_client): name = user_info['name'] assert name == jwt_user[authenticator.username_claim] + + +@pytest.mark.parametrize( + 'allowed_groups,admin_groups,azuread_groups,expected', + [ + ([], ['jupyterhub-admin'], ['jupyterhub-admin'], lambda r: bool(r) and r['admin']), + ([], ['jupyterhub-admin'], ['jupyter-admin'], lambda r: not bool(r)), + (['jupyterhub'], [], ['jupyterhub'], lambda r: bool(r) and not r['admin']), + (['jupyterhub'], [], ['jupyter'], lambda r: not bool(r)), + ([], [], ['jupyterhub'], lambda r: bool(r)), + (['jupyterhub'], ['jupyterhub-admin'], ['jupyterhub', 'jupyterhub-admin'], lambda r: bool(r) and r['admin']), + (['jupyterhub'], [], [], lambda r: not bool(r)), + ([], [], [], lambda r: bool(r) and r.get('admin') is None) + ], +) +async def test_azuread_groups(get_authenticator, azure_client, allowed_groups, admin_groups, azuread_groups, expected): + authenticator = get_authenticator( + scope=['openid', 'profile'], + allowed_groups=allowed_groups, + admin_groups=admin_groups, + ) + + handler = azure_client.handler_for_user( + user_model( + tenant_id=authenticator.tenant_id, + client_id=authenticator.client_id, + name="somebody", + groups=azuread_groups, + ) + ) + + r = await authenticator.authenticate(handler) + assert expected(r) From 10408ee705da526fd5d2e3b6a05834d79fdbd954 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 27 Oct 2021 12:16:13 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- oauthenticator/azuread.py | 2 +- oauthenticator/tests/test_azuread.py | 34 ++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 3e436291..2a223663 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -9,8 +9,8 @@ from jupyterhub.auth import LocalAuthenticator from tornado.httpclient import HTTPRequest from traitlets import default -from traitlets import Unicode from traitlets import List +from traitlets import Unicode from .oauth2 import OAuthenticator diff --git a/oauthenticator/tests/test_azuread.py b/oauthenticator/tests/test_azuread.py index 88dd30a9..5055325e 100644 --- a/oauthenticator/tests/test_azuread.py +++ b/oauthenticator/tests/test_azuread.py @@ -3,15 +3,12 @@ import re import time import uuid - from functools import partial - from unittest import mock -from pytest import fixture - import jwt import pytest +from pytest import fixture from ..azuread import AzureAdOAuthenticator from .mocks import setup_oauth_mock @@ -62,7 +59,7 @@ def _get_authenticator(**kwargs): tenant_id=str(uuid.uuid1()), client_id=str(uuid.uuid1()), client_secret=str(uuid.uuid1()), - **kwargs + **kwargs, ) @@ -100,7 +97,7 @@ async def test_azuread(get_authenticator, username_claim, azure_client): user_model( tenant_id=authenticator.tenant_id, client_id=authenticator.client_id, - name="somebody" + name="somebody", ) ) @@ -119,17 +116,34 @@ async def test_azuread(get_authenticator, username_claim, azure_client): @pytest.mark.parametrize( 'allowed_groups,admin_groups,azuread_groups,expected', [ - ([], ['jupyterhub-admin'], ['jupyterhub-admin'], lambda r: bool(r) and r['admin']), + ( + [], + ['jupyterhub-admin'], + ['jupyterhub-admin'], + lambda r: bool(r) and r['admin'], + ), ([], ['jupyterhub-admin'], ['jupyter-admin'], lambda r: not bool(r)), (['jupyterhub'], [], ['jupyterhub'], lambda r: bool(r) and not r['admin']), (['jupyterhub'], [], ['jupyter'], lambda r: not bool(r)), ([], [], ['jupyterhub'], lambda r: bool(r)), - (['jupyterhub'], ['jupyterhub-admin'], ['jupyterhub', 'jupyterhub-admin'], lambda r: bool(r) and r['admin']), + ( + ['jupyterhub'], + ['jupyterhub-admin'], + ['jupyterhub', 'jupyterhub-admin'], + lambda r: bool(r) and r['admin'], + ), (['jupyterhub'], [], [], lambda r: not bool(r)), - ([], [], [], lambda r: bool(r) and r.get('admin') is None) + ([], [], [], lambda r: bool(r) and r.get('admin') is None), ], ) -async def test_azuread_groups(get_authenticator, azure_client, allowed_groups, admin_groups, azuread_groups, expected): +async def test_azuread_groups( + get_authenticator, + azure_client, + allowed_groups, + admin_groups, + azuread_groups, + expected, +): authenticator = get_authenticator( scope=['openid', 'profile'], allowed_groups=allowed_groups,