Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Various fixes for allowed_groups and admin_groups #758

Merged
merged 12 commits into from
Sep 3, 2024
Merged

Various fixes for allowed_groups and admin_groups #758

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e7c2e09
Systematically test allowed_groups, admin_groups, and manage_groups
consideRatio Sep 2, 2024
3d79615
docs: update example for Generic to use non-deprecated config
consideRatio Sep 3, 2024
79ef403
tests: verify deprecation message for GenericOAuthenticator.claim_gro…
consideRatio Sep 3, 2024
da158fe
tests: iterate on allowed_groups and admin_groups tests
consideRatio Sep 3, 2024
d9ac90e
apply manage_groups behavior outside `update_auth_model`
minrk Sep 3, 2024
baca8a4
fix auth_state_groups_key example in docs
minrk Sep 3, 2024
115ceee
openshift: rely on base class implementation for allowed_groups and a…
consideRatio Sep 3, 2024
6618811
openshift: test equivalent of default auth_state_groups_key provided …
consideRatio Sep 3, 2024
c16bb8c
tests: verify deprecation message for AzureAdOAuthenticator.user_grou…
consideRatio Sep 3, 2024
f92b511
docs: update azuread docs about auth_state_groups_key
consideRatio Sep 3, 2024
05778cd
azuread: retain default of deprecated user_groups_claim within auth_s…
consideRatio Sep 3, 2024
9e5520d
docs: mention allowed_groups and admin_groups in general setup
consideRatio Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/source/tutorials/provider-specific-setup/providers/generic.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "groups"

# Authorization
# -------------
Expand Down
47 changes: 46 additions & 1 deletion oauthenticator/tests/test_auth0.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def user_model():
return {
"email": "[email protected]",
"name": "user1",
"groups": ["group1"],
}


Expand Down Expand Up @@ -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(
Expand All @@ -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)
Expand Down
42 changes: 42 additions & 0 deletions oauthenticator/tests/test_azuread.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,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": [
Expand Down Expand Up @@ -132,6 +133,47 @@ def user_model(tenant_id, client_id, name):
True,
None,
),
# common tests with allowed_groups and manage_groups
(
"40",
{
"allowed_groups": {"group1"},
"auth_state_groups_key": "user.groups",
"manage_groups": True,
},
True,
None,
),
(
"41",
{
"allowed_groups": {"test-user-not-in-group"},
"auth_state_groups_key": "user.groups",
"manage_groups": True,
},
False,
None,
),
(
"42",
{
"admin_groups": {"group1"},
"auth_state_groups_key": "user.groups",
"manage_groups": True,
},
True,
True,
),
(
"43",
{
"admin_groups": {"test-user-not-in-group"},
"auth_state_groups_key": "user.groups",
"manage_groups": True,
},
False,
False,
),
],
)
async def test_azuread(
Expand Down
47 changes: 46 additions & 1 deletion oauthenticator/tests/test_bitbucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def user_model(username):
"""
return {
"username": username,
"groups": ["group1"],
}


Expand Down Expand Up @@ -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(
Expand All @@ -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)
Expand Down
52 changes: 50 additions & 2 deletions oauthenticator/tests/test_cilogon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
74 changes: 60 additions & 14 deletions oauthenticator/tests/test_generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import logging
import re
from functools import partial

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Loading
Loading