Skip to content
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

OAuth2 refresh on short-lived tokens #8147

Closed
jtuta opened this issue Jul 27, 2021 · 2 comments
Closed

OAuth2 refresh on short-lived tokens #8147

jtuta opened this issue Jul 27, 2021 · 2 comments
Assignees
Milestone

Comments

@jtuta
Copy link

jtuta commented Jul 27, 2021

I have been testing OAuth functionality in 1.5rc. While testing I have noted, that token gets refreshed on every request. Upon further examination why this happens I have come across Line 432 in rcmail_oauth.php.

As we (due to security) use short-lived access tokens (5min = 300s), therefore $data['expires'] is always in the past and every time check_token_validity gets called, token gets refreshed.

Why is 600s (10min) subtracted from $data['expires_in']? As refresh token should have it's expiration date bigger than access token, this should not be required and therefore should not be an issue in short-lived tokens.

EDIT: can we at least make this 600s constant configurable, so I can lower it to 30s or similar?

@thomascube
Copy link
Member

Why is 600s (10min) subtracted from $data['expires_in']?

This is a safety margin to avoid token expiration due to date/time offsets between systems. But maybe the margin is too big and should be reduced or at least checked against the overall lifetime of a token.

@alecpl
Copy link
Member

alecpl commented Oct 30, 2021

According to RFC6749 expires_in is: "The lifetime in seconds of the access token. For example, the value "3600" denotes that the access token will expire in one hour from the time the response was generated." and "If omitted, the authorization server SHOULD provide the expiration time via other means or document the default value."

So, it's time from when the response was generated. For me it sounds like we don't need any margin. The later sentence also suggests that we should have a configuration option to force a expires_in value (and use it if not specified in the response).

thomascube added a commit that referenced this issue Dec 2, 2021
Just add a small margin of 10s to consider the transfer
and processing time between oauth server and roundcube.
@thomascube thomascube modified the milestones: later, 1.5.2 Dec 2, 2021
thomascube added a commit that referenced this issue Dec 2, 2021
Just add a small margin of 10s to consider the transfer
and processing time between oauth server and roundcube.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants