feat: Implement oauthenticator.OAuthenticator.refresh_user method
#579
Conversation
|
|
||
| def _access_token_expiration_default(self): | ||
| return os.getenv(self.access_token_expiration_env, "3600") | ||
|
|
There was a problem hiding this comment.
May be replaced by Authenticator.auth_refresh_age ?
|
|
||
| Called by the :meth:`oauthenticator.OAuthenticator.build_auth_state_dict` | ||
| """ | ||
| return token_info.get("expires_in", self.access_token_expiration) |
There was a problem hiding this comment.
Need specific methods to retrieve refresh-related informations, as other providers may need custom logic.
But what if the required fields are provider via response headers ? May need provider implementation to see if that's relevant.
|
|
||
| created_at = auth_state.get('created_at', 0) | ||
| expires_in = auth_state.get('expires_in', 0) | ||
| is_expired = created_at + expires_in - time.time() < 0 |
There was a problem hiding this comment.
May need to refresh before delta reaches 0, to avoid disruption while working.
| """ | ||
| params = { | ||
| "grant_type": "refresh_token", | ||
| "refresh_token": refresh_token, |
There was a problem hiding this comment.
need to include 'scope' here too - the same one used on initial login, to ensure you get back an equivalent id_token/access_token
There was a problem hiding this comment.
apologies, looks like this is only required on the original authorization code request. the scopes for the token_info and refresh_token requests do not affect the responses
|
I'll be super excited to try get this to eventually land :) |
|
@yuvipanda I don't have bandwidth to work on it anymore, so feel free to take over the code ! |
|
@Wykiki I'll poke around and try to find someone! |
|
And thank you for your contribution :) |
|
Hello! Was any progress made here? This would be useful for my work, and this looks like a fairly complete implementation (thanks!). |
|
I've been using this with gitlab with a couple of compatibility changes (adding the redirect_url and the client info) and it seems to work. Are there any other particular changes that would need to be done here for this to be merged? |
This MR aims to implement the
refresh_user()'sAuthenticatormethod inOAuthenticatorclass.Following #398 comment.
At the time of writing, nothing has been tested, and provider specific code would be needed, as the OAuth2 spec does not force the
expires_infield in theaccess_tokenresponse body.I may not have bandwidth soon to work again on this, feel free to take the lead !