oauth2 icon indicating copy to clipboard operation
oauth2 copied to clipboard

Introduce expiration on refresh tokens - an implementation proposal

Open vjt opened this issue 4 years ago • 6 comments

Greetings!

By reading the refresh token mapper code it seems to me that refresh tokens do not have an expiration time, as such they are potentially valid forever, as long as the underlying user is still active it its backend.

In some high-security contexts, users are requested to re-login periodically, to reduce the risk of token stealing.

What do you think of:

  • Add new "refresh token expiration days" setting
  • Add a new expires_at timestamp column on the refresh tokens table
  • When issuing a new refresh token, store time() + $refresh_token_expiration_days in the expires_at
  • When refreshTokenMapper#findByToken() is called, it would select * from refreshtokens where token = ? and expires_at > NOW()
  • Add a new cron job to delete from refreshtokens where expires_at < NOW()

Thank you!

vjt avatar Apr 21 '21 14:04 vjt

@vjt refresh tokens get renewed too and get invalidated after single use. Token stealing not possible from my pov.

michaelstingl avatar Apr 21 '21 15:04 michaelstingl

@michaelstingl thanks. The stealing is indeed difficult, as that would require getting control of the state of an active client.

Still, being refresh tokens valid forever, they pose some concern for our context, as our policies require users to re-authenticate periodically.

What do you think?

Thank you

vjt avatar Apr 21 '21 15:04 vjt

For more control, policies etc, better use OpenID Connect: https://doc.owncloud.com/server/10.7/admin_manual/configuration/user/oidc/oidc.html

Policies configured on your IdP are used to control not only the oC server, but also desktop & mobile clients behave according those policies. OpenID Connect also prepares you to migrate to the next generation ownCloud based on Golang microservice architecture: https://owncloud.dev/ocis/migration/

michaelstingl avatar Apr 21 '21 16:04 michaelstingl

Ouch, that migration would not be straightforward at this time, but I understand one can be more focused on the upcoming technology rather than extending the current.

Anyway - would a PR implementing what I describe be considered?

Thank you!

vjt avatar Apr 21 '21 16:04 vjt

Anyway - would a PR implementing what I describe be considered?

@pmaier1 @DeepDiver1975 @micbar ?

michaelstingl avatar Apr 21 '21 16:04 michaelstingl

@vjt , we have this already for years. You find here the needed code; https://github.com/owncloud/oauth2/pull/182

T0mWz avatar Oct 12 '23 07:10 T0mWz