element-hq / element-hq/element-web

A failed IndexedDB token write can silently and permanently lose the session (and leave it orphaned on the server)

Open
#34,866 0 comments 0 reactions 1 assignee Claimed by @hughns View on GitHub
A-OIDC A-Storage O-Uncommon S-Critical T-Defect
Dominant language
TypeScript
Stars
13.5k
Forks
2.8k
PR merge metrics
PR metrics pending

Description

When `persistTokenInStorage()` cannot write a token to IndexedDB, it falls back to writing it to localStorage. However, precedence is given to IndexedDB and the fallback is only used if IndexedDB does not contain a value for the token. Therefore, if IndexedDB contains an older (stale) token it will still be used.

With rotating refresh tokens (any MAS/OAuth 2.0 deployment) that is fatal: the stale refresh token has already been consumed, the server rejects it with a 400, and the client responds by deleting all local data and forcing a re-login with a new device ID.

Neither the write failure nor the fallback is logged, so the moment a session becomes doomed leaves no trace in rageshakes at all. The failure surfaces on a later launch as an unexplained logout.

## Mechanism

The write path: https://github.com/element-hq/element-web/blob/4dacb8b4ca97707f60696e22dd8abe43b390d68d/apps/web/src/utils/tokens/tokens.ts#L148-L173

The read path: https://github.com/element-hq/element-web/blob/4dacb8b4ca97707f60696e22dd8abe43b390d68d/apps/web/src/Lifecycle.ts#L550-L567

Note that there is no logging on either path.

## The problem

Failure sequence:
- Refresh succeeds; the server rotates RT1 → RT2 and invalidates RT1.
- `persistTokens()` → `idbSave()` fails. RT2 goes to localStorage; IndexedDB still holds RT1.
- The current runtime keeps working — it has RT2 in memory — so nothing appears wrong.

If the app stays alive until the next token refresh and that succeeds in being written to IndexedDB then the failure has self rectified.

However, if the app is closed:

- On the next launch, `getStoredToken()` returns RT1 from IndexedDB. The localStorage copy is never consulted.
- `POST /oauth2/token` gets a `400 Bad Request` (because RT1 already consumed).
- matrix-js-sdk raises `TokenRefreshLogoutError` → `HttpApiEvent.SessionLoggedOut`.
- This is treated as a hard failure and calls: `clearStorage({ deleteEverything: true })`.

The user loses their crypto store, has to re-verify, and gets a new device ID. The old device is left behind on the server, because by then the only tokens the client can offer to `/oauth2/revoke` are the dead ones (RFC 7009 returns 200 for an unknown token, so the revocation is a silent no-op).

Secondary problem: a plaintext token is left behind indefinitely

The fallback writes the token to localStorage unencrypted (localStorage only stores strings), and the write path never clears it after a later successful IndexedDB write. Since the read path can no longer reach it, that value simply stays there — an unencrypted access or refresh token, in a profile where the pickle key exists specifically so that tokens are encrypted at rest. Nothing removes it short of clearStorage() on logout.

## Proposed fix
- Write the fallback to a distinct key (e.g. `_fallback`) and prefer it on read. It is only ever written when an IndexedDB write failed, and is cleared as soon as one succeeds, so it is always at least as new as IndexedDB. Keeping it separate from the primary key preserves the existing meaning of a value there — a pre-IndexedDB legacy token, which may be older than IndexedDB — so the migration path is unaffected and the change is safe on upgrade.
- Log the write failure. This is the single highest-value part: it makes this class of report self-diagnosing instead of invisible.
- Clear the stale IndexedDB entry after a failed write, so a client that does not know about the fallback key reads "no token" rather than an outdated one.
- Sweep up any plaintext token still sitting at the primary key once IndexedDB has answered, and log it. Deliberately without using it: the old code did not clear it on a later successful write, so it may be older than the IndexedDB copy and preferring it could demote a working session to a dead token.

**Not covered by the above fix:** Users who are already in the broken state — stale token in IndexedDB, good one in localStorage — are not recovered by this. Preferring the old value is not safe, because coexistence does not tell you which of the two is newer.

Co-Authored-By: Claude Opus 5 (1M context)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.