Restrict access to profiles based on GH team membership #1239
Conversation
profile_list is now dynamically generated, based on the GH teams user is a part of. This list of teams is refreshed only during login - so user needs to log out and log back in to see new teams! This also means that users removed from teams on GH will still have access to the profiles until they are logged out from the admin panel too (to be fixed) This approach is taken over customizing options_form to protect against users just bypassing the options form and using the API directly to spawn servers. Deployed to the leap hub, except 'large' & 'huge' is only available to leap-stc:leap-pangeo-research members, not to leap-stc:leap-pangeo-users members - based on 2i2c-org#1050 (comment) Fixes 2i2c-org#1146
yuvipanda
force-pushed
the
oauthenticator
branch
from
5faec8b to
97eff99
Compare
|
I think this is ready to go! |
| need to be a member of any one of the listed teams for access. The list of teams a user | ||
| is part of is fetched at login time - so if the user is added to a GitHub team, they need | ||
| to log out and log back in to the JupyterHub (not necessarily to GitHub!) to see the new | ||
| profiles they have access to. To remove access to a profile from a user, they have to be |
There was a problem hiding this comment.
We should document this in docs.2i2c.org so that others know this is possible. I think we should also flag this as a BETA feature that may change at any moment (e.g. if we want to generalize this to non-github, or if we want to change the behavior around things like the need to delete users)
There was a problem hiding this comment.
@choldgraf opened #1245 to track.
| if spawner.user.name == 'deployment-service-check': | ||
| # For our hub deployer health checker, ignore all this logic | ||
| print("Ignoring allowed_teams check for deployment-service-check") | ||
| return original_profile_list |
There was a problem hiding this comment.
Now that we own https://github.com/deployment-service-check/ maybe we should add that account to our GitHub org and tech team and remove this logic? I wonder if we can close the security backdoor by treating deployment-service-check as an actual member of our team?
There was a problem hiding this comment.
@sgibson91 we couldo that if we can make that user authenticate using the oauth flow - although last i looked that was pretty difficult :(
| @@ -18,6 +18,9 @@ RUN pip install --no-cache git+https://github.com/yuvipanda/jupyterhub-configura | |||
| # z2jh 1.2.x ships with kubespawner 1.1.0, so we just do a little bump | |||
| RUN pip install --no-cache --upgrade jupyterhub-kubespawner==1.1.2 | |||
|
|
|||
| # Brings in https://github.com/jupyterhub/oauthenticator/pull/498 | |||
There was a problem hiding this comment.
@yuvipanda, can we create a follow-up issue to use a released version as soon as it is available?
Deployments to staging hub fail in some cases since 2i2c-org#1239 if they use neither GitHub OAuth nor set any profiles. Users will get a blank server options page that doesn't let users start servers without this PR
Deployments to staging hub fail in some cases since 2i2c-org#1239 if they use neither GitHub OAuth nor set any profiles. Users will get a blank server options page that doesn't let users start servers without this PR
profile_list is now dynamically generated, based on the GH teams
user is a part of. This list of teams is refreshed only during login -
so user needs to log out and log back in to see new teams! This also
means that users removed from teams on GH will still have access to
the profiles until they are logged out from the admin panel too (to
be fixed)
This approach is taken over customizing options_form to protect
against users just bypassing the options form and using the API
directly to spawn servers.
Deployed to the leap hub, except 'large' & 'huge' is only available to
leap-stc:leap-pangeo-research members, not to leap-stc:leap-pangeo-users
members - based on #1050 (comment)
Brings in jupyterhub/oauthenticator#498,
we can update to a newer oauthenticator once that is merged.
Fixes #1146
TODO: