-
Notifications
You must be signed in to change notification settings - Fork 365
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
[Generic] Implement refresh_user to periodically refresh OAuth tokens #475
base: main
Are you sure you want to change the base?
Conversation
Requirements for token refresh: - OAuth server response includes `expires_in` and `refresh_token` - State is persisted on hub with `c.Authenticator.enable_auth_state = True`
Thanks for submitting your first pull request! You are awesome! 🤗 |
for more information, see https://pre-commit.ci
As in the next release of jupyterhub there will be authenticator user group management jupyterhub/jupyterhub#3548 what happens if we have manage_groups=true and the user's group membership changes after auth and before refresh? Your refresh code is not updating user_info['groups'] as it seem. |
@kkaraivanov1 Thanks for looking at the PR. I agree that group data should be refreshed as well. Is group membership info already being stored on authentication? |
The way I understand the intentions behind the PR I mention is if c.Authenticator.manage_groups is set AND if the GenericOAuthenticator adds the groups to user_info['groups'] they will be populated automatically. |
That makes sense, and thank you for the clarification. Adding support for |
grant_type='refresh_token', | ||
) | ||
|
||
headers = self._get_headers() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one is wrong. It has to be
headers = { "Content-Type": "application/x-www-form-urlencoded" }
…expiration Follows the convention of Let's Encrypt
An attempt at OAuth token refresh, following the pattern in PR #287, without relying on tokens to be JWTs. This should address at least some of issue #398.
Requirements for token refresh:
expires_in
andrefresh_token
c.Authenticator.enable_auth_state = True
(to store therefresh_token
)