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

[Okpy] Remove the authenticator as it is no longer used #691

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

yuvipanda
Copy link
Collaborator

@yuvipanda yuvipanda commented Oct 5, 2023

There is exactly one installation of okpy in the world - at https://okpy.org/. It's used only by UC Berkeley, and this authenticator was used for a short period of time at one of the hubs run by UC Berkeley. However, it has not been used there for many years, and there are no ongoing plans for it to be used ever again. I can speak reasonably authoritatively on this, given I helped run those hubs from inception until very recently.

Given there are not really any other okpy installations in the world, I think it's actually safe to remove this one.

There is exactly one installation of okpy in the world - at
https://okpy.org/. It's primarily used only by UC Berkeley,
and this authenticator was used for a short period of time at
one of the hubs run by UC Berkeley. However, it has *not* been
used there for many years, and there are no ongoing plans for it
to be used ever again. I can speak reasonably authoritatively
on this, given I helped run those hubs from inception until
very recently.

Given there are not really any other okpy installations in the
world, I think it's actually safe to remove this one.
@GeorgianaElena GeorgianaElena changed the title Get rid of the okpy authenticator Remove the okpy authenticator as it is no longer used Oct 9, 2023
Copy link
Member

@GeorgianaElena GeorgianaElena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @yuvipanda and @minrk!

@GeorgianaElena GeorgianaElena merged commit 2fa1c48 into jupyterhub:main Oct 9, 2023
8 checks passed
@consideRatio
Copy link
Member

@yuvipanda @manics @minrk @GeorgianaElena I'd like to conclude how we should version next release of oauthenticator that includes this PR, and also how z2jh and tljh should be versioned as soon as they have the this oauthenticator PR in it.

I think in a way this should be a 17.0.0 release, but if we feel confident this is fine to remove, I'd much rather we don't do that and go for 16.1.1.

I think z2jh / tljh should major version bump if we bump dependencies directly configured by admins (such as oauthenticator, but not kubernetes-asyncio) to a new major version that can break their config.

I have two ideas on how to go about this, what do you think?

  1. To release 16.1.1 or 16.1.0 with this change, considering as maintenance not impacting users, and not mandating z2jh / tljh to major version bump along with it.
  2. To release 17.0.0 with this change, but to document how to backport releases in our team-compass docs and start doing that until we have more things that merits the 17.0.0 release that also makes us need to cut z2jh 4 and tljh 2.

@manics
Copy link
Member

manics commented Oct 9, 2023

Okpy seems to be open-source https://github.com/okpy . How certain are we that no one is using it privately?

@yuvipanda
Copy link
Collaborator Author

@manics 99.999% certain! https://github.com/okpy/ok is the serverside code, but I also know it's not all the serverside code (there are non-public components, but mostly because there are secrets embedded in that repo, not for any other reason). So purely based on that, I'd think there aren't any other installations.

@consideRatio I would not consider this a breaking change, so (1) seems simplest. I also don't think it necessarily needs released right away - can just be rolled in whenever the next release rolls out.

@consideRatio
Copy link
Member

👍 on considering this as not breaking then, and release in any kind of next version release. If there is someone using this, they can transition to configuration like this.

c.JupyterHub.authenticator_class = "generic"
c.GenericOAuthenticator.authorize_url = "https://okpy.org/oauth/authorize"
c.GenericOAuthenticator.token_url = "https://okpy.org/oauth/token"
c.GenericOAuthenticator.userdata_url = "https://okpy.org/api/v3/user"
c.GenericOAuthenticator.scope = ["email"]
c.GenericOAuthenticator.username_claim = "email"
# userdata_params is configured because otherwise all responses from the API are
# wrapped in an envelope that contains metadata about the response.
# ref: https://okpy.github.io/documentation/ok-api.html
c.GenericOAuthenticator.userdata_params = {"envelope": "false"}

# and other common config
c.OAuthenticator.oauth_callback_url = "https://[your-domain]/hub/oauth_callback"
c.OAuthenticator.client_id = "[your oauth2 application id]"
c.OAuthenticator.client_secret = "[your oauth2 application secret]"

yuvipanda added a commit to yuvipanda/jupyterhub that referenced this pull request Oct 16, 2023
These were removed in
jupyterhub/oauthenticator#691,
and now the link checker is not happy.
@consideRatio consideRatio changed the title Remove the okpy authenticator as it is no longer used [Okpy] Remove the authenticator as it is no longer used Oct 18, 2023
minrk pushed a commit to minrk/jupyterhub that referenced this pull request Jan 31, 2024
These were removed in
jupyterhub/oauthenticator#691,
and now the link checker is not happy.
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.

5 participants