-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Extend OAuth provider configuration object to include a token refresh handler #4902
Comments
Thank you! Handling this is definitely on our list, but doing it consistently across different providers and strategies (jwt, database) is what makes it hard (I.e. not all providers have the ability to even refresh tokens, should we warn/swallow/throw error then?) |
I understand this is more complicated than it might seem at first glance. What I'm proposing is not to tackle this generically, but to start with a more manageable intermediate step on the road towards doing so. Of course, my proposal could also be more complicated than I personally see, but since I prototyped it for my own use case yesterday, let me show you what I have in mind. 1 — Extend the definition of
|
Luckily, I think we could just defer to the underlying library for some of the logic: https://github.com/panva/node-openid-client/blob/main/docs/README.md#clientrefreshrefreshtoken-extras https://github.com/panva/oauth4webapi/blob/main/docs/functions/refreshTokenGrantRequest.md (in #4299) Maybe we could make this an opt-in flag either on One issue that needs to be solved is when multiple requests hit the backend and all want to update at the same time. See one discussion here: #3940 We will also want to make sure that refreshing tokens update the database's |
It's great that some of the logic can be deferred to the library underneath! For the "multiple requests at the same time" problem and the "update the Of course, having a feature with known bugs /limitation doesn't feel great even if "experimental". But in this case, the status quo situation for the users of this library is that once their tokens have stopped working they have no easy way to refresh them. Personally, I'd rather opt-in into a (temporarily) buggy solution to the issue, since the alternative is that I must write that buggy solution myself anyway, as I've been doing :P FYI, updating the database's |
It looks like this issue did not receive any activity for 60 days. It will be closed in 7 days if no further activity occurs. If you think your issue is still relevant, commenting will keep it open. Thanks! |
To keep things tidy, we are closing this issue for now. If you think your issue is still relevant, leave a comment and we might reopen it. Thanks! |
Description 📓
The OAuth provider configuration object includes options (e.g.
authorization
,token
, anduserinfo
) to configure how each provider handles the various stages of the OAuth flow.While a mechanism for token rotation is explained in the documentation, there is no way to contribute the token refresh code to the set of built-in OAuth providers. I believe allowing this configuration to be shared with other developers through a built-in mechanism would be incredibly beneficial to the users of this library, even if the token rotation is not automatically executed.
In short, my proposal would be to add the following option to the OAuth provider configuration object :
where
RefreshParams
would be something like:This would allow developers to share the code for OAuth token refresh with the community. I don't foresee specific issues with such an extension.
Note that while I likely won't have time to implement a full automatic refresh flow, I'd be willing to contribute this specific extension (and a couple configurations for existing providers) as long as someone can offer help figuring out the right method signature.
How to reproduce ☕️
N/A
Contributing 🙌🏽
Yes, I am willing to help implement this feature in a PR
The text was updated successfully, but these errors were encountered: