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 openscapes hub to GitHub Auth with Teams #1222

Closed
1 of 5 tasks
yuvipanda opened this issue Apr 20, 2022 · 20 comments
Closed
1 of 5 tasks

Move openscapes hub to GitHub Auth with Teams #1222

yuvipanda opened this issue Apr 20, 2022 · 20 comments
Assignees

Comments

@yuvipanda
Copy link
Member

yuvipanda commented Apr 20, 2022

Context

We currently use auth0 for the Openscapes hub, and the admins have to manage users manually. In #1213, @erinmr asked for features to make that process easier. One way to do that is to just switch to GitHub auth and provide access via teams - so admins can manage access outside of the JupyterHub!

Proposal

Updates and actions

@sgibson91
Copy link
Member

sgibson91 commented Apr 28, 2022

Important Validate that removing a user from a GitHub team actually removes their access on JupyterHub

What I did

  1. Setup @2i2c-org/test-team to test GitHub Teams-based authentication for our hubs
  2. Created an OAuth app in the 2i2c GitHub org: https://github.com/organizations/2i2c-org/settings/applications/1892494
  3. Created a dummy hub to test against. Config can be found in this draft PR (which shouldn't be merged):
  4. I added @GeorgianaElena to the @2i2c-org/test-team. She confirmed that she can login to the hub.
  5. Georgiana was then removed from the team, she logged out of the hub and when she tried to log back in, received a 403.

Conclusion

Removing a user from a GitHub Team does remove their access to a JupyterHub, but not necessarily immediately. The user needs to logout of the hub and so they will retain access to the hub until they logout/an admin revokes all tokens from the OAuth app/a token expires.

Two courses of action to ensure this period of access after removal from the GitHub Team doesn't remain are:

  1. User refresh_user (user needs to login in when refresh_user returns False) or Authenticator.refresh_pre_spawn (user needs to login on launch). These aren't perfect solutions, but implementable now. https://jupyterhub.readthedocs.io/en/stable/api/auth.html#jupyterhub.auth.Authenticator.refresh_pre_spawn
  2. Open a PR to GitHubOAuthenticator forcing refresh_user to periodically refresh. There are related PRs in the following locations:

1 is probably ok short-term, but slightly annoying from a UX POV. We should do 2 for longer-term sustainability.

cc: @yuvipanda

@yuvipanda
Copy link
Member Author

@sgibson91 I think deleting the user from the hub admin interface also logs them out explicitly - so we can document that as the thing for people to do when they remove them from a GitHub team?

@sgibson91
Copy link
Member

sgibson91 commented Apr 29, 2022

admins have to manage users manually.... asked for features to make that process easier. One way to do that is to just switch to GitHub auth and provide access via teams - so admins can manage access outside of the JupyterHub!

I think if do that, that means that admins have to remove a user from both the GitHub Team and the JupyterHub admin panel, which seems to be adding complexity rather than simplifying the process as originally requested quoted above?

@yuvipanda
Copy link
Member Author

@sgibson91 ah, true. But that seems better than making everyone log in each time they want to start a server no? Or we could use this as a way to prioritize adding refresh_user functionality for GitHubOAuthenticator, and hold off until that lands.

@sgibson91
Copy link
Member

we could use this as a way to prioritize adding refresh_user functionality for GitHubOAuthenticator

This would be my preferred long-term solution as I think most hub admins would prefer to only need to carry out admin tasks in one place, whether that's the admin panel or GitHub Teams interface.

I already had a chat with Min in Gitter and I'm happy to put some cycles into the upstream work providing I have some guidance (I've not worked with the OAuthenticators before!)

@sgibson91
Copy link
Member

sgibson91 commented Apr 29, 2022

(while this looks like I'm doing work on my day off, I'm actually comatose in a hotel after eating a lot of delicious Indian food 😋)

@yuvipanda
Copy link
Member Author

@sgibson91 yeah, I think doing this in OAuthenticator is the 'right' way to do it, and I think we should prioritize that now. Especially with auth / security stuff, doing it the right way is probably going to save us headaches in the long and short term.

@sgibson91
Copy link
Member

cc: @damianavila FYI for planning purposes: the above will likely become a project for me since I don't know anything about the OAuthenticator and it'll likely not be something I can complete quickly.

@damianavila
Copy link
Contributor

I do agree with both of you about prioritizing the upstream work. Users will be mad if we force them to log in each time.
Btw, let's use this issue to reference/link any further work.
@sgibson91, feel free to create a dedicated board if you feel this one grows enough.

@sgibson91
Copy link
Member

@yuvipanda asked me if he could take over the upstream oauthenticator work for this issue and I agreed since I am quite swamped by admin stuff atm

@sgibson91 sgibson91 assigned yuvipanda and unassigned sgibson91 May 11, 2022
@choldgraf
Copy link
Member

Can somebody clarify what our next steps are here? In this comment it seems like there are two things proposed, a short-term thing and a long-term thing. Can we update the top comment with the steps that need to be taken to close this issue, and if there's a long-term thing we can open a new issue about it?

@sgibson91
Copy link
Member

sgibson91 commented Jun 22, 2022

I think we could just enable GitHub Auth on the openscapes hub for now and tackle refresh_user separately?

I'd be interested in @yuvipanda's estimation of time/work for implementing refresh_user and then @GeorgianaElena agreed to help me with this if no one else can take the lead.

@GeorgianaElena
Copy link
Member

Since I'm currently blocked waiting for more info about the Callysto hub I spent a bit of today documenting about the upstream side of this task, i.e. implementing refresh_user in oauthenticator.

So, I was planning to self-assign this and come up with an initial implementation for the upstream issue or at least a plan to push this forward hopefully tomorrow.

A short summary of the current upstream state

There is support for refresh_user in JupyterHub and there are currently two opened PRs that strive to implement this for GitLab and Generic OAuthenticators. More about this in @sgibson91's comment above #1222 (comment)
That is valuable work that needs reviewing.
However, the longer term solution would be to come up with a generic implementation that each authenticator could extend, as proposed in jupyterhub/oauthenticator#398. I believe we should pursue that, while valuing the existing work proposed in the two PRs.

P.S. I've updated the top comment with the main two sub-tasks remaining of this issue.

@GeorgianaElena GeorgianaElena self-assigned this Jul 5, 2022
@damianavila
Copy link
Contributor

Thanks for taking the lead on this one, @GeorgianaElena!
The plan LGTM.

@yuvipanda
Copy link
Member Author

Thanks a lot for taking this on, @GeorgianaElena!

@yuvipanda yuvipanda removed their assignment Nov 9, 2022
@yuvipanda
Copy link
Member Author

Currently blocked on jupyterhub/oauthenticator#526 :(

@damianavila
Copy link
Contributor

We briefly discussed in the sprint meeting the problem of "visibilizing" what we do upstream since @GeorgianaElena is doing a lot of work upstream of us.

@GeorgianaElena
Copy link
Member

The next big step towards wrapping up this task this is jupyterhub/oauthenticator#398.

But before fixing this issue, some work has been done in order to build a more solid base for it and to improve the maintainability of the jupyterhub/oauthenticator project. I will link some relevant PRs below for visibility and as an update (I'm very grateful for @consideRatio's work and other's on the JupyterHub team on reviewing these)

My plan towards jupyterhub/oauthenticator#398 is to:

And after this, start implementing jupyterhub/oauthenticator#398 because I don't think the release should get blocked by that necessarily. But let's see how things go.

@damianavila
Copy link
Contributor

Pause for now waiting for feedback from the upstream team.

@yuvipanda
Copy link
Member Author

Closing in favor of #3240 which is much more tightly scoped

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

No branches or pull requests

5 participants