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

add refresh_user_hook #780

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add refresh_user_hook #780

wants to merge 1 commit into from

Conversation

minrk
Copy link
Member

@minrk minrk commented Dec 3, 2024

allows customizing/overriding refresh_user behavior when the default isn't desirable.

testing refresh_user on 2i2c revealed a use case I had not considered, which is reliance on 'fake' users that don't actually exist in the oauth provider, and refresh_user means those users cannot actually do things, because their auth state is never valid. I added what should be the solution for 2i2c's health check accounts as an example in the docs. This makes enabling refresh_user by default a more disruptive change than I realized, if we want to switch the default to a more explicit opt-in. I still think it is the right thing to do in most cases, so in that way makes sense as a default (eventually, if not immediately).

I also hadn't realized that refresh_pre_spawn is overridden to True by default in OAuthenticator, which was surprising since it's been that way for years, despite refresh_user never having been implemented before now.

Maybe I should have done a beta of 17.2. We can yank 17.2, if folks feel that would take some pressure off, and then take a more careful beta process pushing this out again as 17.3.

allows customizing/overriding refresh_user
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant