omniauth-oauth2 icon indicating copy to clipboard operation
omniauth-oauth2 copied to clipboard

Refresh token is not read if expires_in or expires_at is not present

Open fabioxgn opened this issue 4 years ago • 5 comments

I'm implementing a custom strategy for one provider I need to integrate with, and they do not return the expiration of the token, I know that this is a bad practice but it's their implementation. They do return a refresh_token so I can update the token if I want, but because of this code, the refresh_token is only read if the expiration is set: https://github.com/omniauth/omniauth-oauth2/blob/8438b89b712e03337932a0d95096258c892ea0f3/lib/omniauth/strategies/oauth2.rb#L52

I was reading the OAuth RFC, and noticed that the expiration is recommended, but not required: https://datatracker.ietf.org/doc/html/rfc6749#section-4.2.2

I can send a patch to change this, but I'm wondering if this might break something or if I should add an option for this behavior, any advice?

For now, I did a hack on my strategy to set the expiration to an arbitrary value and it solved the issue for me:

option :auth_token_params, { expires_at: 100.years.from_now }

fabioxgn avatar Oct 20 '21 17:10 fabioxgn

It would be great if it was possible to define an option on the strategy that no refresh_token is available to avoid setting a custom expiry.

espen avatar Sep 05 '22 18:09 espen

@fabioxgn is this working for you? It works on my computer (™) but I see some of our users are getting an error due to expiry.

espen avatar Sep 08 '22 15:09 espen

@fabioxgn is this working for you? It works on my computer (™) but I see some of our users are getting an error due to expiry.

Hi @espen yes, it's working for me, I've been running this in production for almost one year now without any issues.

fabioxgn avatar Sep 08 '22 15:09 fabioxgn

@espen but if this is your fix: https://github.com/espen/omniauth-mailchimp/commit/fd6b007f591404176c1bbba07c2e6ec78ef00632#diff-5e16132f58f644df626f2f761b5b96c649409b57815657367667a0f804d9329eR18 it might be because you are using a short time period, I'm actually using 100.years.from_now since as the token has no expiration I'm setting a really high value that should never expire.

fabioxgn avatar Sep 08 '22 15:09 fabioxgn

@fabioxgn I had it updated to 60 minutes but with the same error. Will try with 100 years, thanks.

espen avatar Sep 08 '22 16:09 espen