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

Support JupyterHub 2's fine grained RBAC permissions, and test against Py3.10 and JH2 #160

Merged
merged 3 commits into from
Oct 29, 2021

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Oct 28, 2021

I cherry picked a commit from #159 that did the change required for JupyterHub 2 support, then added tests to verify it and made it backward compatible as well.

Testing against Python 3.10 as well was a feature creep in this PR while I was at it.


Closes #159 as it includes it.
Closes #157 by supporting JupyterHub 2's new scope decorator.

@csears-hg and @lambdaTotoro what do you think?

csears-hg and others added 2 commits October 28, 2021 19:21
JupyterHub 2.0 deprecated the admin_only decorator in jupyterhub/jupyterhub#3659. Per the deprecation warning, I'm replacing admin_only with the new needs_scope decorator, using the scope 'admin:users'. In testing, this produced 403 errors for non-admin users as expected.
@consideRatio consideRatio changed the title ci: test against py 3.10 and jupyterhub 2 Support JupyterHub 2's fine grained RBAC permissions, and test against Py3.10 and JH2 Oct 28, 2021
@csears-hg
Copy link
Contributor

This looks like a solid approach!

@lambdaTotoro
Copy link
Collaborator

This works as-is for JupyterHub < and > 2?

@consideRatio
Copy link
Member Author

consideRatio commented Oct 29, 2021

This works as-is for JupyterHub < and > 2?

Yepp, or hmmm.. I've actually not installed jupyterhub 2 and tried this fully, I've just run the pytest and made it work to that extent against jupyterhub 1 and 2.

I'll go ahead and try it out manually as well against jupyterhub 2 because perhaps we haven't requested sufficient permissions or similar for the things we do.


Action points

  • Manually test against JupyterHub 2

@consideRatio consideRatio marked this pull request as draft October 29, 2021 10:04
@lambdaTotoro
Copy link
Collaborator

Yes, that would be great. If that runs through, I'm happy to merge it. But I would like to have it actually tested at least once. Thanks again.

@consideRatio consideRatio marked this pull request as ready for review October 29, 2021 15:20
@consideRatio
Copy link
Member Author

@lambdaTotoro I've verified that all the decorators worked well with JupyterHub 2.0.0b3 with the new decorators alongside the old decorator on all endpoints where they are used to restrict access.

I've also verified that the normal non-admin users can't access such endpoints.

image

@lambdaTotoro
Copy link
Collaborator

Wonderful, thank you for going the extra mile. Merging now.

@lambdaTotoro lambdaTotoro merged commit f425546 into jupyterhub:main Oct 29, 2021
@consideRatio
Copy link
Member Author

@lambdaTotoro thanks for being so attentive and providing concrete and kind feedback to changes to this repo, it is a breeze to contribute to it!

@consideRatio
Copy link
Member Author

@csears-hg thank you for raising this issue and coming up with the change needed to make this function for JupyterHub 2! ❤️ 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

admin_only decorator deprecated in jupyterhub 2.0
3 participants