Support expiry of refresh tokens and expiry of the overall session when refresh tokens are in use.

[reivilibre]

This is part of #11372.

The general idea is that a refresh token can be given a bounded lifetime, so that when your access tokens also have a bounded lifetime,
the client must refresh frequently enough or else get logged out for inactivity.

Refresh token expiry

Useful for providing auto-logout on inactivity.

If you set your refresh token lifetime to R and your access token lifetime to A (and assuming the refresh token lifetime is longer than that of the access tokens), then a client:

• may be logged out for inactivity if its inactivity exceeds R - A.
• will always be logged out for inactivity if its inactivity exceeds R.

(here, by 'logged out' I mean logged out in such a way that the client can't use a refresh token to get access again).

(Ultimate) session lifetime

This is an upper bound on the length of the session as a whole; no matter how much the client uses refresh tokens, it will have to reauthenticate after this time.

Previously, the refresh token mechanism was incompatible with session_lifetime.

[DMRobertson]

Seems reasonable.

I wasn't 100% clear on what refreshing did to access tokens. I think they extend their lifetime, but I didn't see a test that checked this by trying to use an access token after it had been refreshed.

I think you're also forbidden from using the same refresh token twice. Don't understand the security behind this, but that sounds like an important enough point to merit a test case.

[DMRobertson]

Shouldn't it be valid for a fresh batch of 2 minutes? Otherwise, why are you waiting 121 seconds below rather than 2 seconds?

[DMRobertson]

Also, if I understand correctly:

• at this point the access token has expired, so you shouldn't be able to use it
• using the refresh token will allow us to use the access token again(?)
  • If so, do we test this?

[reivilibre]

You're right in saying there should be more tests.

using the refresh token will allow us to use the access token again(?)

It creates a new access token :). It's actually pretty tricky to invalidate the caches on other Synapse workers to extend a token's lifetime (I looked into this before we switched track to refresh tokens for a customer), it's actually easier to create a fresh token. (Also, creating a new token is kind of how refresh tokens protect against access token disclosure, since the leaked access token dies out and never is valid again.)

[reivilibre]

Shouldn't it be valid for a fresh batch of 2 minutes? Otherwise, why are you waiting 121 seconds below rather than 2 seconds?

Yes. I think there must be something unclear about the test since I think the test is right, though I'm not sure I understand what you're asking.

Here, we first test that a refresh (of R1) works within 119 seconds of issuance (less than 2 minutes; this produces R2) but a refresh (of R2) fails after 121 seconds of issuance (of R2).

Perhaps one thing that would help would be to not reuse the same variable name.

I will tweak some comments / names and see if it pleases you more

[DMRobertson]

Thanks, I think those changes are good. What I hadn't groked was that on line +659 we now have a second (new) access token.

The phrase "refresh our access token" on +671 makes me think of keeping the same access token around, but delaying its expiry. Perhaps it would be clearer to speak of "requesting a new access token" or "refreshing/extending the session" here and in #11427?

[reivilibre]

Thanks, that is clearer!

[sandhose]

I think you're also forbidden from using the same refresh token twice. Don't understand the security behind this, but that sounds like an important enough point to merit a test case.

@DMRobertson This helps to reduce the impact of a refresh token leakage. See the last section of the MSC: https://github.com/matrix-org/matrix-doc/blob/main/proposals/2918-refreshtokens.md#detailed-rationale

[reivilibre]

for what it's worth, #11427 is a draft of the Synapse documentation I want to add for this feature. It might or might not help your understanding — it's also a good chance to tell me if the doc is missing anything critical before you understand it too well to notice :)

[reivilibre]

I think you're also forbidden from using the same refresh token twice. Don't understand the security behind this, but that sounds like an important enough point to merit a test case.

My predecessor already thought of this: test_refresh_token_invalidation (Refresh tokens are invalidated after first use of the next token.) seems like it handles this.

Please do let me know if you spot any other tests I should include!

[DMRobertson]

LGTM. I've suggested a potentially useful rephrasing in https://github.com/matrix-org/synapse/pull/11425/files#r757509947 , but I leave that at your discretion---I might be splitting hairs there.