Skip to content
This repository has been archived by the owner on Sep 10, 2024. It is now read-only.

Refresh token request should be idempotent #2795

Open
sandhose opened this issue May 24, 2024 · 11 comments
Open

Refresh token request should be idempotent #2795

sandhose opened this issue May 24, 2024 · 11 comments

Comments

@sandhose
Copy link
Member

sandhose commented May 24, 2024

It can happen that a refresh token use works on the server side, but then the response never comes back to the client.

This usually means that the client gets logged out, as their old refresh token isn't valid anymore, and they don't have the new one.

One way clients could work around this would be to redo a authz grant with the same device ID, but this impractical in many situations.

To avoid this problem, we should make the refresh token grant idempotent. This means we should track whether access tokens are used or not, and allow re-using an access token if neither the new access token nor the new refresh token were used.

@kegsay
Copy link
Member

kegsay commented May 24, 2024

This is particularly bad on mobile devices / devices which can be suspended (e.g laptops overnight) because often what happens is any in-flight network requests are frozen, timing out when the device is unsuspended. If the refresh request reached the server before the request was frozen, then this will result in a logout due to this issue.

@wrenix
Copy link

wrenix commented Jun 8, 2024

Edit/ Final: my problem is solved on client side.


Oh this is the reason why my phone get logout every night
Edit: Hmm is just an element-x (Android) Problem not an element-web
Edit2: or it has just that client this Problem:

2024-06-14T19:54:39.649643Z  WARN elementx: didRefreshTokens() | 
2024-06-14T19:54:39.655009Z DEBUG elementx: Saving new session data with token: '...budj'. Was token valid: true | 
2024-06-14T19:54:39.666580Z DEBUG elementx: Saved new session data with token: '...budj'. | 
2024-06-14T19:54:39.675595Z DEBUG matrix_sdk::http_client: Error while sending request: Api(Server(ClientApi(Error { status_code: 401, body: Standard { kind: UnknownToken { soft_logout: false }, message: "Token is not active" } }))) | crates/matrix-sdk/src/http_client/mod.rs:196 | spans: send{server_versions=[V1_0, V1_1, V1_2, V1_3, V1_4, V1_5] config=RequestConfig { timeout: 30s } request_id="REQ-0" method=GET uri="https://matrix.private.org/_matrix/client/v3/user/@wrenix:private.org/account_data/m.secret_storage.default_key" status=401 response_size="79 B"}
2024-06-14T19:54:40.098556Z ERROR matrix_sdk::client::futures: Token refresh: OIDC refresh_token rejected with invalid grant | crates/matrix-sdk/src/client/futures.rs:146
2024-06-14T19:54:40.098691Z ERROR matrix_sdk::encryption: Couldn't setup and resume recovery Sdk(Http(RefreshToken(Oidc(Oidc(TokenRefresh(Token(Http(HttpError { status: 400, body: Some(ErrorBody { error: InvalidGrant, error_description: Some("The provided access grant is invalid, expired, or revoked.") }) })))))))) | crates/matrix-sdk/src/encryption/mod.rs:1379
2024-06-14T19:54:40.103757Z  WARN elementx: didReceiveAuthError(isSoftLogout=false) | 

I open an issue also there: element-hq/element-x-android#2999 (comment)

@jacotec
Copy link

jacotec commented Aug 16, 2024

I seem to run into this issue randomly with Element-X-IOS. From time to time we are presented with the welcome screen on our iPhones, EX-IOS simply did throw us out.

As I want to migrate all to Element-X now, this issue is more or less a showstopper. I don't think any "normal" user remembers server domain and this backup passphrase somwhere on the road. A user should never be logged out in a smartphone messenger without explicitely wanting this.

Unfortunately I don't see a trace this will be fixed in the upcoming 0.10.0? @sandhose

@wrenix How did you fix this at the client side?

@n0emis
Copy link

n0emis commented Aug 19, 2024

related to element-hq/element-web#27914

@sandhose
Copy link
Member Author

@jacotec There is indeed nothing in 0.10.0 for that.

One temporary workaround to help with that is to bump the lifetime of access tokens, from 5 min to something like half an hour:

experimental:
  access_token_ttl: 1800

Refresh token idempotency is a tricky problem to solve, and we want to make sure we implement it right, without hiding potential client implementation problems

@jacotec
Copy link

jacotec commented Aug 19, 2024

@jacotec There is indeed nothing in 0.10.0 for that.

One temporary workaround to help with that is to bump the lifetime of access tokens, from 5 min to something like half an hour:

experimental:
  access_token_ttl: 1800

Refresh token idempotency is a tricky problem to solve, and we want to make sure we implement it right, without hiding potential client implementation problems

@sandhose This workaround needs to be applied exactly where? MAS? Sxnapse? What is the impact of increasing the lifetime?
If I open Element-X after 4 hours, is there a lower chance to be logged out? Or would I need to set the token to something in a days range?

My issue is: We use it also for family, and there are our 80yrs+ parents in. No way to explain them on the phone how to re-login again. I do the app istallation and I need to be absolutely sure they are not logged out by random (which never happens in Element-Legacy).

@sandhose
Copy link
Member Author

sandhose commented Aug 19, 2024

This workaround needs to be applied exactly where? MAS? Sxnapse?

It's in the MAS config

What is the impact of increasing the lifetime?

One of the reason for short-lifetime like that is to lower the impact of an access token leak. If your access token leaks, the potential attacker would be able to impersonate you for at most the lifetime of the token.
That being said, I wouldn't worry too much about that for now.

If I open Element-X after 4 hours, is there a lower chance to be logged out? Or would I need to set the token to something in a days range?

There is a very much lower chance, yes. In EX, this issue happens if the token refreshes just before the background process gets closed. With a longer token TTL, there is a higher chance that refresh happens when you just opened the app, not when you're closing it

Note that the max value for this is 24h (86400)

@jacotec
Copy link

jacotec commented Aug 19, 2024

@sandhose Sounds good, I've set it to 86400 now to minimize any logout chance.

What's the compat_token_ttl doing, which is also in the experimental section and defaults to 300?

@wrenix
Copy link

wrenix commented Aug 20, 2024

@jacotec my problem was a bug in element-x-android (if received an push notification a subroutine was started where the refresh token is received but not stored correct, or so - for more see element-hq/element-x-android#3028)

@sandhose
Copy link
Member Author

What's the compat_token_ttl doing, which is also in the experimental section and defaults to 300?

@jacotec MAS supports MSC2918 style refresh tokens, which are refresh tokens but with legacy auth. This was added a while back, but no client use it, so that setting likely won't change anything in your case

@matrixbot
Copy link
Member

For your information, this issue has been copied over to the Element fork of matrix-authentication-service: element-hq/matrix-authentication-service#2795

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants