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

[fabioxgn]

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:

omniauth-oauth2/lib/omniauth/strategies/oauth2.rb

Line 52 in 8438b89

hash["refresh_token"] = access_token.refresh_token if access_token.expires? && access_token.refresh_token

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 }

[espen]

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]

@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.

[fabioxgn]

@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]

@espen but if this is your fix: espen/omniauth-mailchimp@fd6b007#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.

[espen]

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