search

jupyterhub / oauthenticator

OAuth + JupyterHub Authenticator = OAuthenticator
https://oauthenticator.readthedocs.io
BSD 3-Clause "New" or "Revised" License
415 stars 366 forks source link

Various fixes for allowed_groups and admin_groups #758

Closed consideRatio closed 2 months ago

consideRatio commented 2 months ago

Ensures allowed_groups and admin_groups works as in all authenticator classes by adding tests and resolving some implementation details. The fixes are to unreleased changes, so this is considered a maintenance PR rather than bugfix.

consideRatio commented 2 months ago

@minrk this isn't perfect, but it catches a few issues correctly at least I think.

Do you have work in progress related to actual fixes? This PR is 100% tests and docs currently.

minrk commented 2 months ago

I moved the manage_groups logic outside of update_auth_model so it's unconditional, private, and not overrideable. Classes can implement get_user_groups to govern this behavior. All tests are passing. Left one comment in-line about whether the mock responses should be kept realistic, because right now we are injecting group fields that aren't going to be there. I think that's fine for the purposes of what we want to test.

consideRatio commented 2 months ago

@minrk I pushed a few more commits and think I'm happy with things now - go for merge?

minrk commented 2 months ago

@consideRatio looks great, thank you!