fix(auth): guard account switch against slot→token desync#506
Open
shukiv wants to merge 2 commits into
Open
Conversation
When switching accounts, the target client connects with the token at the account's stored cookieSlot. If that slot→token mapping is ever wrong — e.g. corrupted client state persisted by an older build, or any future slot desync — the connection succeeds as a *different* account and the UI silently shows the wrong mailbox. Add a post-connect identity guard: derive the connected session's accountId (primary-identity email for OAuth, else the JMAP session username) and compare it to the account being switched to. On mismatch, drop the poisoned slot cookies and force a clean re-auth instead of binding the wrong session. This is belt-and-suspenders on top of 8b164c5, which fixed the slot allocation that caused such a desync: that prevents new corruption, this catches any residual/leftover mapping at switch time. Adds a unit test for the canonicalisation (email vs JMAP username), the make-or-break detail that avoids OAuth false-positives.
Accounts whose primary sending identity differs from their login (basic auth registers accountId from the typed login; OAuth from the identity email) were force-re-authed on switch because the guard derived the connected id only from the primary-identity email. Collect every server-confirmed identifier (JMAP Session.username + primary-identity email) and only re-auth when the target matches none. Excludes the constructor username so a real desync still trips. Adds JMAPClient.getSessionUsername().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On account switch, the target client connects with the token at the account's stored
cookieSlot. If that slot→token mapping is ever wrong — corrupted client state persisted by an older build, or any future slot desync — the connection succeeds as a different account and the UI silently shows the wrong mailbox. Observed in the wild: switching repeatedly kept showing another account's mail; only a full site-data wipe + re-login cleared it.Fix
Post-connect identity guard in
switchAccount: derive the connected session'saccountId(primary-identity email for OAuth, else the JMAP session username) and compare to the account being switched to. On mismatch: drop the poisoned slot cookies, force a clean re-auth — never bind a session whose identity doesn't match.Belt-and-suspenders on top of
8b164c5(per-account cookie slot through OAuth flows): that prevents new corruption at allocation time; this catches any residual/leftover mapping at switch time and converts "silently wrong mailbox" into "re-login".Notes
null(non-blocking) when identity can't be determined — never wedges a switch on a transient identities-fetch failure.preferred_username≠ email). Unit test added for exactly that.Test
stores/__tests__/auth-store-switch-guard.test.ts— 3 cases (email-derived id, JMAP-username fallback, null when undeterminable). Typecheck clean.