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.

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

Move allowed_groups and admin_groups to base authenticator #741

Closed
wants to merge 2 commits into from

Conversation

yuvipanda
Copy link
Collaborator

A simplification of #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.

A simplification of jupyterhub#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.
@yuvipanda yuvipanda requested a review from minrk April 25, 2024 19:45
@yuvipanda
Copy link
Collaborator Author

I was wrong, looks like it's not actually easier to move just these two - I thought get_user_groups was common to all authenticators, but not the case.

@yuvipanda yuvipanda closed this Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant