fix(auth): guard account switch against slot→token desync
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.
This commit is contained in:
@@ -0,0 +1,52 @@
|
|||||||
|
import { describe, it, expect, beforeEach } from 'vitest';
|
||||||
|
import { connectedAccountId } from '../auth-store';
|
||||||
|
import { useIdentityStore } from '../identity-store';
|
||||||
|
import type { Identity } from '@/lib/jmap/types';
|
||||||
|
|
||||||
|
// Covers the make-or-break bit of the account-switch identity guard: the
|
||||||
|
// connected session's accountId must be derived with the SAME canonicalisation
|
||||||
|
// as login (primary-identity email for OAuth, where the JMAP session username
|
||||||
|
// is often a preferred_username claim, not the address). Comparing the raw
|
||||||
|
// JMAP username would both false-positive on OAuth and miss real desyncs.
|
||||||
|
|
||||||
|
const SERVER = 'https://mail.example.com';
|
||||||
|
|
||||||
|
const fakeClient = (jmapUsername: string, identities: Identity[] | Error) =>
|
||||||
|
({
|
||||||
|
getUsername: () => jmapUsername,
|
||||||
|
getIdentities: async () => {
|
||||||
|
if (identities instanceof Error) throw identities;
|
||||||
|
return identities;
|
||||||
|
},
|
||||||
|
}) as never;
|
||||||
|
|
||||||
|
const id = (over: Partial<Identity> = {}): Identity => ({
|
||||||
|
id: 'id-1',
|
||||||
|
name: 'Real User',
|
||||||
|
email: 'real@example.com',
|
||||||
|
mayDelete: true,
|
||||||
|
...over,
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('connectedAccountId (account-switch guard)', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
useIdentityStore.setState({ identities: [], preferredPrimaryId: null } as never);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('derives from the primary-identity EMAIL, not the JMAP session username', async () => {
|
||||||
|
// OAuth: JMAP username is a preferred_username claim, the real address lives
|
||||||
|
// on the identity. The id must be built from the email.
|
||||||
|
const result = await connectedAccountId(fakeClient('preferred_user', [id()]), SERVER);
|
||||||
|
expect(result).toBe('real@example.com@mail.example.com');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to the JMAP username when identities cannot be fetched', async () => {
|
||||||
|
const result = await connectedAccountId(fakeClient('basic@example.com', new Error('no idents')), SERVER);
|
||||||
|
expect(result).toBe('basic@example.com@mail.example.com');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns null when the session identity cannot be determined at all', async () => {
|
||||||
|
const broken = { getUsername: () => { throw new Error('disconnected'); } } as never;
|
||||||
|
expect(await connectedAccountId(broken, SERVER)).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -415,6 +415,29 @@ function scheduleRefresh(expiresIn: number, refreshFn: () => Promise<string | nu
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Derive the accountId a *connected* client actually belongs to, using the
|
||||||
|
* same canonicalisation as login (primary-identity email for OAuth, else the
|
||||||
|
* JMAP session username). Lets a caller detect a slot->token mapping that
|
||||||
|
* resolves to the wrong account before it surfaces as the wrong mailbox.
|
||||||
|
* Returns null when it can't determine the identity (treated as "don't block").
|
||||||
|
*/
|
||||||
|
export async function connectedAccountId(client: JMAPClient, serverUrl: string): Promise<string | null> {
|
||||||
|
try {
|
||||||
|
const jmapUsername = client.getUsername();
|
||||||
|
let canonical = jmapUsername;
|
||||||
|
try {
|
||||||
|
const { primaryIdentity } = loadIdentities(await client.getIdentities(), jmapUsername);
|
||||||
|
canonical = primaryIdentity?.email || jmapUsername;
|
||||||
|
} catch {
|
||||||
|
/* identities unavailable — fall back to the JMAP session username */
|
||||||
|
}
|
||||||
|
return generateAccountId(canonical, serverUrl);
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function clearRefreshTimer(accountId?: string): void {
|
function clearRefreshTimer(accountId?: string): void {
|
||||||
if (accountId) {
|
if (accountId) {
|
||||||
const timer = refreshTimers.get(accountId);
|
const timer = refreshTimers.get(accountId);
|
||||||
@@ -1363,6 +1386,26 @@ export const useAuthStore = create<AuthState>()(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GUARD: verify the connected session actually belongs to the target
|
||||||
|
// account before we bind it. A corrupted slot->token mapping (e.g.
|
||||||
|
// persisted client state left over from an older build, or any future
|
||||||
|
// slot desync) can hand back a *different* account's token; the
|
||||||
|
// connection then succeeds and we would silently show the wrong
|
||||||
|
// mailbox. On mismatch, drop the poisoned cookies for this slot and
|
||||||
|
// force a clean re-auth instead of surfacing someone else's mail.
|
||||||
|
const connectedId = await connectedAccountId(targetClient, targetAccount.serverUrl);
|
||||||
|
if (connectedId && connectedId !== accountId) {
|
||||||
|
debug.error(`switchAccount: slot ${targetAccount.cookieSlot} for ${accountId} resolved to ${connectedId} — forcing re-auth`);
|
||||||
|
clients.delete(accountId);
|
||||||
|
try { targetClient.disconnect(); } catch { /* noop */ }
|
||||||
|
apiFetch(`/api/auth/token?slot=${targetAccount.cookieSlot}`, { method: 'DELETE' }).catch(() => {});
|
||||||
|
apiFetch(`/api/auth/session?slot=${targetAccount.cookieSlot}`, { method: 'DELETE' }).catch(() => {});
|
||||||
|
accountStore.updateAccount(accountId, { isConnected: false, hasError: true, errorMessage: 'session_mismatch' });
|
||||||
|
set({ isLoading: false, error: 'connection_failed', activeAccountId: state.activeAccountId });
|
||||||
|
replaceWindowLocation(getLocaleLoginPath());
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Restore cached state or fetch fresh
|
// Restore cached state or fetch fresh
|
||||||
const restored = restoreAccount(accountId);
|
const restored = restoreAccount(accountId);
|
||||||
accountStore.setActiveAccount(accountId);
|
accountStore.setActiveAccount(accountId);
|
||||||
|
|||||||
Reference in New Issue
Block a user