Authenticator user group management #3548

thomafred · 2021-07-23T12:27:12Z

Added authenticator hook for synchronizing user groups

Added hook function stub to authenticator base class
Added new argument authenticator_managed_group to base Authenticator class
Call authenticator hook from refresh_auth-function in Base handler class
Added example

welcome · 2021-07-23T12:27:14Z

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

minrk

Great! I think this makes sense in principle. Let's add some tests and make sure we iron out some details.

A few things to cover:

How is group creation handled? (i.e. what happens if a group is not already present in the db?)
Presumably when this flag is set, other group assignment mechanisms should fail (via config, via the API)

Is there any issue with users who have not logged in yet (i.e. have no auth state)?

jupyterhub/user.py

minrk · 2021-12-02T09:06:51Z

@thomafred now that 2.0 is out, I'd like to make authenticator-managed groups my focus for 2.1.

Do you have time to address some of the review to get this ready to merge, or would you prefer us to finish it up?

thomafred · 2021-12-02T10:15:32Z

Awesome!

I will make the changes suggested quickly

minrk · 2021-12-02T13:56:20Z

Great! I never want to assume availability when a PR's been waiting for months.

thomafred · 2021-12-06T11:27:54Z

@minrk - Responding to your questions:

How is group creation handled? (i.e. what happens if a group is not already present in the db?)

The group is automatically created in the database. There is no need for the admin to manually create new groups provided by the authenticator the way things work now. However, I can see this being a bit controversial, so maybe this feature could be optional. What do you think?

Presumably when this flag is set, other group assignment mechanisms should fail (via config, via the API)

I am not sure what would be best here. Ideally, this PR should provide the features needed to offload group management to the authenticator. In this case, having other mechanisms fail would make sense.

minrk · 2021-12-07T10:45:47Z

What do you think?

I think that's fine. If the authenticator is in charge, it can be its responsibility.

this PR should provide the features needed to offload group management to the authenticator.

I think the right thing to do here is to add a check to all endpoints that can modify group membership (includes, optionally, user creation):

if self.authenticator.manage_groups:
    raise web.HTTPError(400, "Group management via API is disabled")

and do the same during startup if JupyterHub.load_groups is defined.

thomafred · 2021-12-15T11:47:09Z

@minrk - I have implemented the changes you suggested. However, the unit-tests test_auth.py appear to be failing stating AttributeError: 'MockPAMAuthenticator' object has no attribute 'load_groups'.

jupyterhub/app.py

jupyterhub/auth.py

minrk · 2021-12-16T11:39:03Z

jupyterhub/orm.py

@@ -222,7 +222,7 @@ class Group(Base):
    __tablename__ = 'groups'
    id = Column(Integer, primary_key=True, autoincrement=True)
    name = Column(Unicode(255), unique=True)
-    users = relationship('User', secondary='user_group_map', backref='groups')
+    users = relationship('User', secondary='user_group_map', back_populates='groups')


Can you explain this change? backref is supposed to be equivalent to two instances of back_populates, so I'm not sure why this is here.

Changed to backref

jupyterhub/handlers/base.py

…s to use back_populates instead of backref

…s back to backref

- Added hook function stub to authenticator base class - Added new config option `manage_groups` to base `Authenticator` class - Call authenticator hook from `refresh_auth`-function in `Base` handler class - Added example

minrk · 2022-01-20T15:20:58Z

@thomafred thanks! I think this is just about ready to go - I rebased the PR and added some tests and docs, but wasn't allowed to update the branch. Is it possible for you to either allow maintainers to update the branch (usually a checkbox on the right), or reset your branch to minrk:authenticator_user_group_management? Then I think this can be merged. If not, I can make a new PR from my own branch.

I mostly added docs and tests, but fixed a few things revealed by the tests. I also updated the implementation to use the groups field returned by authenticate, which I think better matches how other fields are already populated (auth_state, admin), and what's in #3307.

thomafred · 2022-01-24T08:03:59Z

@minrk Was not able to find the "allow maintainers"-toggle, so I added as you a maintainer in the repo instead.

- tests - docs - ensure all group APIs are rejected when auth is in control - use 'groups' field in return value of authenticate/refresh_user, instead of defining new method - log group changes in sync_groups

minrk · 2022-01-25T09:51:59Z

Was not able to find the "allow maintainers"-toggle,

Ah, I didn't realize it was an org-to-org PR. I think the checkbox is only available from personal forks, for some reason.

I think this is all set, I'll merge as soon as I finish making 2.1.1.

welcome · 2022-01-25T13:36:43Z

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️

minrk · 2022-01-25T13:36:59Z

Woo! Landed. Thanks so much.

`manage_groups` was added in 2.2.0 (current version is 4.0.2) jupyterhub/jupyterhub#3548

thomafred force-pushed the authenticator_user_group_management branch from f346b22 to dbc4881 Compare July 23, 2021 12:35

thomafred mentioned this pull request Jul 23, 2021

[AzureAD] Add an implementation for Authenticator.authenticator_managed_groups=true jupyterhub/oauthenticator#448

Closed

minrk reviewed Aug 24, 2021

View reviewed changes

jupyterhub/user.py Outdated Show resolved Hide resolved

minrk mentioned this pull request Oct 29, 2021

Deadlock when using REST API from Authenticator.post_auth_hook #3654

Closed

minrk added the new new features label Dec 8, 2021

thomafred force-pushed the authenticator_user_group_management branch from dbc4881 to 75c64b7 Compare December 16, 2021 08:53

thomafred added a commit to C4IROcean/jupyterhub that referenced this pull request Dec 16, 2021

jupyterhub#3548 - Implemented suggested fix

6387f29

minrk mentioned this pull request Dec 16, 2021

When is the default 'user' role applied or reapplied? #3721

Open

minrk reviewed Dec 16, 2021

View reviewed changes

thomafred added a commit to C4IROcean/jupyterhub that referenced this pull request Jan 14, 2022

Updated auth group handling according to PR jupyterhub#3548 comments

c7437af

thomafred added a commit to C4IROcean/jupyterhub that referenced this pull request Jan 14, 2022

Updated auth group handling according to PR jupyterhub#3548 comments

d459c9d

thomafred added a commit to C4IROcean/jupyterhub that referenced this pull request Jan 14, 2022

PR jupyterhub#3548 - Reverted ORM change

14debcb

minrk reviewed Jan 14, 2022

View reviewed changes

jupyterhub/handlers/base.py Outdated Show resolved Hide resolved

jupyterhub/handlers/base.py Outdated Show resolved Hide resolved

thomafred added a commit to C4IROcean/jupyterhub that referenced this pull request Jan 14, 2022

PR jupyterhub#3548 - Reverted ORM change

49d0304

thomafred added a commit to C4IROcean/jupyterhub that referenced this pull request Jan 14, 2022

PR jupyterhub#3548 - Changed User.groups and Group.users relationship…

8bedf00

…s to use back_populates instead of backref

thomafred added a commit to C4IROcean/jupyterhub that referenced this pull request Jan 14, 2022

PR jupyterhub#3548 - Changed User.groups and Group.users relationship…

d45d514

…s back to backref

thomafred added a commit to C4IROcean/jupyterhub that referenced this pull request Jan 18, 2022

PR jupyterhub#3548 - Changed some function signatures

3cdfa0e

Added authenticator hook for synchronizing user groups

144abcb

- Added hook function stub to authenticator base class - Added new config option `manage_groups` to base `Authenticator` class - Call authenticator hook from `refresh_auth`-function in `Base` handler class - Added example

minrk force-pushed the authenticator_user_group_management branch from 3cdfa0e to b17ded3 Compare January 24, 2022 12:43

minrk approved these changes Jan 24, 2022

View reviewed changes

test coverage for Authenticator.managed_groups

88be7a9

- tests - docs - ensure all group APIs are rejected when auth is in control - use 'groups' field in return value of authenticate/refresh_user, instead of defining new method - log group changes in sync_groups

minrk force-pushed the authenticator_user_group_management branch from b17ded3 to 88be7a9 Compare January 24, 2022 12:45

minrk merged commit bb5ec39 into jupyterhub:main Jan 25, 2022

minrk deleted the authenticator_user_group_management branch January 25, 2022 13:36

toms-place mentioned this pull request Feb 2, 2022

Automatic group synchronization with groups obtained through authenticator #3785

Closed

eliaswimmer mentioned this pull request Feb 3, 2022

get groups from authenticator; improve group mgmt on admin page #3307

Closed

kkaraivanov1 mentioned this pull request Feb 16, 2022

[Generic] Implement refresh_user to periodically refresh OAuth tokens jupyterhub/oauthenticator#475

Open

thomafred mentioned this pull request Feb 7, 2023

[AzureAD] Add an implementation for Authenticator.manage_groups=True jupyterhub/oauthenticator#573

Merged

yuvipanda mentioned this pull request Sep 29, 2023

[Feature Request] Allow authenticators to control group membership #2022

Closed

benjimin added a commit to benjimin/oauthenticator that referenced this pull request Nov 29, 2023

Bump minimum jupyterhub version

8f7bd64

`manage_groups` was added in 2.2.0 (current version is 4.0.2) jupyterhub/jupyterhub#3548

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Authenticator user group management #3548

Authenticator user group management #3548

thomafred commented Jul 23, 2021

welcome bot commented Jul 23, 2021

minrk left a comment

minrk commented Dec 2, 2021

thomafred commented Dec 2, 2021

minrk commented Dec 2, 2021

thomafred commented Dec 6, 2021

minrk commented Dec 7, 2021

thomafred commented Dec 15, 2021

minrk Dec 16, 2021

thomafred Jan 14, 2022

minrk commented Jan 20, 2022

thomafred commented Jan 24, 2022 •

edited

minrk commented Jan 25, 2022

welcome bot commented Jan 25, 2022

minrk commented Jan 25, 2022

Authenticator user group management #3548

Authenticator user group management #3548

Conversation

thomafred commented Jul 23, 2021

welcome bot commented Jul 23, 2021

minrk left a comment

Choose a reason for hiding this comment

minrk commented Dec 2, 2021

thomafred commented Dec 2, 2021

minrk commented Dec 2, 2021

thomafred commented Dec 6, 2021

minrk commented Dec 7, 2021

thomafred commented Dec 15, 2021

minrk Dec 16, 2021

Choose a reason for hiding this comment

thomafred Jan 14, 2022

Choose a reason for hiding this comment

minrk commented Jan 20, 2022

thomafred commented Jan 24, 2022 • edited

minrk commented Jan 25, 2022

welcome bot commented Jan 25, 2022

minrk commented Jan 25, 2022

thomafred commented Jan 24, 2022 •

edited