matrix-org / matrix-authentication-service

OAuth2.0 + OpenID Provider for Matrix Homeservers
https://matrix-org.github.io/matrix-authentication-service/
Apache License 2.0
112 stars 32 forks source link

Refresh token request should be idempotent #2795

Open sandhose opened 5 months ago

sandhose commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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: https://github.com/element-hq/element-x-android/issues/2999#issue-2342111151

jacotec commented 2 months ago

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 commented 2 months ago

related to https://github.com/element-hq/element-web/issues/27914

sandhose commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

@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 https://github.com/element-hq/element-x-android/issues/3028)

sandhose commented 2 months ago

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 commented 1 month ago

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