Introduce expiration on refresh tokens - an implementation proposal

[vjt]

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!

[michaelstingl]

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

[vjt]

@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

[michaelstingl]

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/

[vjt]

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!

[michaelstingl]

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

@pmaier1 @DeepDiver1975 @micbar ?

[T0mWz]

@vjt , we have this already for years. You find here the needed code; #182