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.

Sign up for GitHub

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

Jump to bottom

Upgrade to z2jh 3.1.0 with oauthenticator 16.1 #3144

Merged
merged 24 commits into from
Oct 4, 2023
Merged

Upgrade to z2jh 3.1.0 with oauthenticator 16.1 #3144

merged 24 commits into from
Oct 4, 2023

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Sep 15, 2023 •
edited
Loading

Summary

This PR updates from our use of z2jh 3.0.0-beta.1 (where we manually had upgraded KubeSpawner) to z2jh 3.1.0, and with it we get oauthenticator 16.1.0, up from 15.1.0. With this, we have significant amount of changes to review manually as the authentication configuration is not something we test automatically (its very hard).

Pre-merge review

  • Ensure we have all newly added hubs get their configs updated as well, for example HHMI hub #3136
    • Ensured initially
    • Ensured before merge
  • Go though all CILogonOAuthenticator config manually
    • Verified to seem OK by Erik
    • Verified to seem OK by at least another engineer (edit: or by Erik again at a later time?)
  • Go though all GitHubOAuthenticator config manually
    • Verified by Erik
    • Verified by at least another engineer (edit: or by Erik again at a later time?)
  • Deploy to cluster's staging hubs (Saturday 2023-09-30, 15:00 UTC+2)
    • utoronto (cilogon idp https://idpz.utorauth.utoronto.ca/shibboleth)
    • ubc-eoas (cilogon idp https://authentication.ubc.ca)
    • openscapes (github, allow_existing_users)
    • jupyter-meets-the-earth (github, allow_existing_users)
    • gridsst (github, allow_existing_users)
    • cloudbank (a lot of different hubs)
    • carbonplan (cilogon with github idp, allow_existing_users)
    • callysto (cilogon with special authenticator customization)
  • Check ourselves the access in staging hubs, or ask for community to check if needed
    • utoronto (cilogon idp https://idpz.utorauth.utoronto.ca/shibboleth)
      I checked with two utoronto accounts
    • ubc-eoas (cilogon idp https://authentication.ubc.ca)
      Asked representatives in Upgrade to z2jh 3.1.0 with oauthenticator 16.1 #3144 (comment)
    • openscapes (github, allow_existing_users)
      I checked with consideratio-demo, first not getting access, then addig that user and getting access, then deleting the user and no longer having access. This was the intented behavior.
    • jupyter-meets-the-earth (github, allow_existing_users)
      I performed a check like for openscapes
    • gridsst (github, allow_existing_users)
      I performed a check like for openscapes
    • cloudbank (a lot of different hubs)
      I checked with community admin via Cloudbank followup to review auth config #3196
    • carbonplan (cilogon with github idp, allow_existing_users)
      I performed a check like for openscapes
    • callysto (cilogon with special authenticator customization)
      I performed a check that a google user from a unknown domain wasn't allowed, and one from an allowed domain was. I also checked that I could authenticate with a microsoft account, but could only check I got denied as I had no allowed microsoft account.

Help on how to review

These are key changes in OAuthenticator 16 to be aware about:

  • OAuthenticator.allow_all and CILogonOAuthenticator.allowed_idps[].allow_all is new config that needs to be explicitly set to True if all authenticated users by the authenticator or specific CILogon identity provider are to be authorized. Previously all users could be authorized automatically if no config existed to indicate only some should be authorized access.
  • OAuthenticator.allow_existing_users is introduced to explicitly toggle a behavior that previousy was decided based on if Authenticator.allowed_users was declared or not. In v15 when allowed_users was set, any user in JupyterHub's user database (as updatable via /hub/admin panel) was allowed access as well. In v16, this needs to be explicitly configured via the new config option.
  • In OAuthenticator v15, there was config like allowed_users that together with GitHubOAuthenticator.allowed_organizations required authenticated users to be part of both the list of allowed_user and alowed_organizations. That led to us never configuring both, only one or the other. In v16 a user can be authorized by being part of either.

Reference links

Related

@consideRatio consideRatio requested a review from a team as a code owner September 15, 2023 01:05
@consideRatio consideRatio removed the request for review from a team September 15, 2023 01:05
@consideRatio consideRatio marked this pull request as draft September 15, 2023 01:05
@github-actions

This comment was marked as resolved.

Sign in to view
@consideRatio consideRatio mentioned this pull request Sep 15, 2023
Merged
@consideRatio consideRatio changed the title (draft) Upgrade to oauthenticator 16 Upgrade to z2jh 3.1.0 with oauthenticator 16.1 Sep 30, 2023
This was referenced Sep 30, 2023
Closed
Closed
@consideRatio
Copy link
Member Author

@ckrzysik @lheagy @hmodzelewski could one of you make a quick verification that you have access to https://staging.ubc-eoas.2i2c.cloud by logging in (if needed logout first) and reply if it worked out or not?

@consideRatio consideRatio marked this pull request as ready for review September 30, 2023 13:59
@consideRatio consideRatio requested a review from a team September 30, 2023 13:59
@lheagy
Copy link
Member

lheagy commented Sep 30, 2023

This worked for me!

@jmunroe
Copy link
Contributor

jmunroe commented Oct 3, 2023 •
edited
Loading

This change is expected to rolled out 2023-10-04. I'm planning on sending the following email to our hub champions for their information:

2i2c will be doing routine maintenance and an upgrade to the authentication code on your JupyterHub deployment. This update will be rolled out on 2023-10-04. While we don't anticipate any action required on your part, please let us know through [email protected] if there are any reported issues with accessing or logging in to your hub from your users. Please see PR #3144 for additional information.

We don't expect any issues but we are using this PR to iterate on how we notify our hub communities of changes that may impact them. Depending on how this goes, I'll make a correspond change in the Team Compass for a procedure to adopt in these cases.

Update: Email sent. See https://2i2c.freshdesk.com/a/tickets/101

@jmunroe jmunroe mentioned this pull request Oct 3, 2023
Open
2 tasks
@consideRatio consideRatio mentioned this pull request Oct 4, 2023
Merged
@consideRatio
oauthenticator 16: remove shown_idps, allowed_idps now provides that …
ca4e736 
…effect
@consideRatio
oauthenticator 16: remove explicit scope, profile is included anyhow
7c6c7c5 
The default scope in oauthenticator 16 includes what we need. Let's rely
on the default for simplicity.
@consideRatio
oauthenticator 16: add allow_existing_users where allowed_users was c…
3663194 
…onfigured
@consideRatio
oauthenticator 16: remove outdated comment about allowed_users
adb6b9a
@consideRatio
auth config: remove outdated workaround setting empty admin_users
93e2e25 
This was fixed in 2i2c-org#2299
@consideRatio
oauthenticator 16: remove redundant spec of allowed_users, add warnings
641d8bc
@consideRatio
auth config: remove temporary config addition
6e51de5
@consideRatio
basehub: tweak values to avoid formatting conflicts
3496083
@consideRatio
basehub: refactor, simplify chowning container's command
93d1466
@consideRatio
basehub: upgrade z2jh from 3.0.0-beta.1 to 3.1.0
d4bc156
@consideRatio
hub images: use kubespawner 6.1.0 from base image, add fixme notes
0b082a5
@consideRatio
utoronto: re-order config entries for consistency
81d66df
@consideRatio
utoronto: remove duplicated specification
edf1500
@consideRatio
docs: remove no longer correct warning
5685328
@consideRatio
oauthenticator 16.1: add allow_all config to individual idps
b5e1f1e
@consideRatio
docs: refine inline comment about admin_users
4c9be70
@consideRatio
docs: refresh cilogon docs about config
a430c9e
@consideRatio
hhmi: stop allowing all users, allow only existing users
03aece5
@consideRatio
hub image: configure use of newly built image based on z2jh 3.1.0
789cc81
@consideRatio
refactor: make ordering of config entries consistent
2cd299e
@consideRatio
deployer: remove no longer relevant github auth validation
f4bc4ce 
In oauthenticator 16 its possible to allow a github organization/team
and separately allow individual users. Instead of the intersection of
allowed_users and users allowed via allowed_organization, oauthenticator
16 allows the union of these users. This makes this validation check no
longer relevant.
@consideRatio
docs: remove a comment
37a86a3
@consideRatio
refactor: reorder config for consistency
bc68128
@consideRatio
cloudbank, fresno: fix auth config mistake in v15 to v16 migration
25d52b7
@consideRatio
Copy link
Member Author

I rebased and force pushed to ensure this PR is easier to revert if needed, and I caught one config mistake for a cloudbank hub thanks to the second review, as fixed via 25d52b7.

With nasa-ghg and nasa-veda credentials refreshed, this is good to go I think. I'm going for a merge once tests pass.

@consideRatio consideRatio merged commit bf47d1a into 2i2c-org:master Oct 4, 2023
32 checks passed
@github-actions
Copy link

github-actions bot commented Oct 4, 2023

🎉🎉🎉🎉

Monitor the deployment of the hubs here 👉 https://github.com/2i2c-org/infrastructure/actions/runs/6404219319

This was referenced Oct 5, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@consideRatio consideRatio

Labels
None yet
Projects
No open projects
Sprint Board
Status: Done 🎉
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@consideRatio @lheagy @jmunroe