fix: recognize canonicalized login usernames in account-switch guard
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { describe, it, expect } from 'vitest';
|
import { describe, it, expect } from 'vitest';
|
||||||
import { connectedAccountCandidates } from '../auth-store';
|
import { connectedAccountCandidates, classifySessionMatch } from '../auth-store';
|
||||||
import type { Identity } from '@/lib/jmap/types';
|
import type { Identity } from '@/lib/jmap/types';
|
||||||
|
|
||||||
// The account-switch guard force-re-auths only when the connected session
|
// The account-switch guard force-re-auths only when the connected session
|
||||||
@@ -66,3 +66,33 @@ describe('connectedAccountCandidates (account-switch guard)', () => {
|
|||||||
expect(out).toEqual([]);
|
expect(out).toEqual([]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('classifySessionMatch (account-switch verdict)', () => {
|
||||||
|
const CANDIDATES = ['linus@rathblume.de@mail.example.com', 'admin@rbm.systems@mail.example.com'];
|
||||||
|
|
||||||
|
it('accepts when a candidate equals the stored accountId (full-email login)', () => {
|
||||||
|
expect(classifySessionMatch(CANDIDATES, 'linus@rathblume.de@mail.example.com', [])).toBe('accept');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts a short-username accountId via captured server identifiers', () => {
|
||||||
|
// accountId built from the short login `linus`, which the server canonicalizes
|
||||||
|
// to linus@rathblume.de — the id itself never appears among the candidates.
|
||||||
|
expect(classifySessionMatch(CANDIDATES, 'linus@mail.example.com', CANDIDATES)).toBe('accept');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('trusts a legacy account with no captured baseline (TOFU self-heal)', () => {
|
||||||
|
expect(classifySessionMatch(CANDIDATES, 'linus@mail.example.com', undefined)).toBe('trust');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a real desync once a baseline exists', () => {
|
||||||
|
// slot resolves to a genuinely different account; baseline was captured before.
|
||||||
|
expect(
|
||||||
|
classifySessionMatch(['stranger@mail.example.com'], 'linus@mail.example.com', CANDIDATES),
|
||||||
|
).toBe('reject');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts when nothing could be confirmed (empty candidates never bounce)', () => {
|
||||||
|
expect(classifySessionMatch([], 'linus@mail.example.com', undefined)).toBe('accept');
|
||||||
|
expect(classifySessionMatch([], 'linus@mail.example.com', CANDIDATES)).toBe('accept');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -17,6 +17,15 @@ export interface AccountEntry {
|
|||||||
cookieSlot: number;
|
cookieSlot: number;
|
||||||
/** Whether "Remember Me" was checked (basic auth only) */
|
/** Whether "Remember Me" was checked (basic auth only) */
|
||||||
rememberMe: boolean;
|
rememberMe: boolean;
|
||||||
|
/**
|
||||||
|
* Server-confirmed account identifiers captured at login: the account-id form
|
||||||
|
* ({@link generateAccountId}) of the JMAP Session.username and the primary
|
||||||
|
* sending-identity email. The account-switch guard matches a reconnected
|
||||||
|
* session against these so a short login username (e.g. `linus`) that the
|
||||||
|
* server canonicalizes to a full address (`linus@example.com`) is still
|
||||||
|
* recognized as the same account. `undefined` = never captured (legacy entry).
|
||||||
|
*/
|
||||||
|
serverIdentifiers?: string[];
|
||||||
/** Cached display info */
|
/** Cached display info */
|
||||||
displayName: string;
|
displayName: string;
|
||||||
email: string;
|
email: string;
|
||||||
|
|||||||
+66
-8
@@ -423,6 +423,17 @@ function scheduleRefresh(expiresIn: number, refreshFn: () => Promise<string | nu
|
|||||||
* resolves to the wrong account before it surfaces as the wrong mailbox.
|
* 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").
|
* Returns null when it can't determine the identity (treated as "don't block").
|
||||||
*/
|
*/
|
||||||
|
export function buildServerIdentifiers(
|
||||||
|
sessionUsername: string | undefined,
|
||||||
|
primaryEmail: string | undefined,
|
||||||
|
serverUrl: string,
|
||||||
|
): string[] {
|
||||||
|
const ids = new Set<string>();
|
||||||
|
if (sessionUsername) ids.add(generateAccountId(sessionUsername, serverUrl));
|
||||||
|
if (primaryEmail) ids.add(generateAccountId(primaryEmail, serverUrl));
|
||||||
|
return [...ids];
|
||||||
|
}
|
||||||
|
|
||||||
export async function connectedAccountCandidates(client: JMAPClient, serverUrl: string): Promise<string[]> {
|
export async function connectedAccountCandidates(client: JMAPClient, serverUrl: string): Promise<string[]> {
|
||||||
// accountId is generated differently per auth mode: OAuth/SSO registers from
|
// accountId is generated differently per auth mode: OAuth/SSO registers from
|
||||||
// the primary-identity EMAIL, basic auth from the typed login username. A
|
// the primary-identity EMAIL, basic auth from the typed login username. A
|
||||||
@@ -434,16 +445,45 @@ export async function connectedAccountCandidates(client: JMAPClient, serverUrl:
|
|||||||
// username (always the target) and would defeat the desync check. An empty
|
// username (always the target) and would defeat the desync check. An empty
|
||||||
// result means nothing could be confirmed → the caller should NOT force a
|
// result means nothing could be confirmed → the caller should NOT force a
|
||||||
// re-auth.
|
// re-auth.
|
||||||
const ids = new Set<string>();
|
let sessionUser: string | undefined;
|
||||||
try {
|
try {
|
||||||
const sessionUser = client.getSessionUsername();
|
sessionUser = client.getSessionUsername();
|
||||||
if (sessionUser) ids.add(generateAccountId(sessionUser, serverUrl));
|
|
||||||
} catch { /* session unavailable */ }
|
} catch { /* session unavailable */ }
|
||||||
|
let primaryEmail: string | undefined;
|
||||||
try {
|
try {
|
||||||
const { primaryIdentity } = loadIdentities(await client.getIdentities(), client.getUsername());
|
const { primaryIdentity } = loadIdentities(await client.getIdentities(), client.getUsername());
|
||||||
if (primaryIdentity?.email) ids.add(generateAccountId(primaryIdentity.email, serverUrl));
|
primaryEmail = primaryIdentity?.email;
|
||||||
} catch { /* identities unavailable */ }
|
} catch { /* identities unavailable */ }
|
||||||
return [...ids];
|
return buildServerIdentifiers(sessionUser, primaryEmail, serverUrl);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decide whether a freshly connected session may be bound to `accountId`.
|
||||||
|
*
|
||||||
|
* `connectedCandidates` are the server-confirmed identifiers of the session we
|
||||||
|
* just connected ({@link connectedAccountCandidates}). We accept when they
|
||||||
|
* overlap either the stored `accountId` (full-email / OAuth logins, where the
|
||||||
|
* id already IS the canonical address) or `storedIdentifiers` — the identifiers
|
||||||
|
* captured when THIS account last logged in, which cover a short login username
|
||||||
|
* the server canonicalizes to a full address.
|
||||||
|
*
|
||||||
|
* - 'accept' — bind the session (matched, or nothing confirmable to check).
|
||||||
|
* - 'trust' — legacy account with no baseline yet: accept and backfill (TOFU),
|
||||||
|
* so short-username accounts created before this check self-heal
|
||||||
|
* instead of bouncing forever.
|
||||||
|
* - 'reject' — server identity contradicts a known baseline: a real desync;
|
||||||
|
* force a clean re-auth.
|
||||||
|
*/
|
||||||
|
export function classifySessionMatch(
|
||||||
|
connectedCandidates: string[],
|
||||||
|
accountId: string,
|
||||||
|
storedIdentifiers: string[] | undefined,
|
||||||
|
): 'accept' | 'trust' | 'reject' {
|
||||||
|
if (connectedCandidates.length === 0) return 'accept';
|
||||||
|
const accepted = new Set<string>([accountId, ...(storedIdentifiers ?? [])]);
|
||||||
|
if (connectedCandidates.some((c) => accepted.has(c))) return 'accept';
|
||||||
|
if (storedIdentifiers === undefined) return 'trust';
|
||||||
|
return 'reject';
|
||||||
}
|
}
|
||||||
|
|
||||||
function clearRefreshTimer(accountId?: string): void {
|
function clearRefreshTimer(accountId?: string): void {
|
||||||
@@ -670,6 +710,14 @@ export const useAuthStore = create<AuthState>()(
|
|||||||
lastLoginAt: Date.now(),
|
lastLoginAt: Date.now(),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Capture the server-confirmed identity now so a later account switch
|
||||||
|
// recognizes this session even when `username` is a short login name
|
||||||
|
// the server canonicalizes to a different address (see the switch guard).
|
||||||
|
const serverIdentifiers = buildServerIdentifiers(client.getSessionUsername(), primaryIdentity?.email, serverUrl);
|
||||||
|
if (serverIdentifiers.length > 0) {
|
||||||
|
accountStore.updateAccount(accountId, { serverIdentifiers });
|
||||||
|
}
|
||||||
|
|
||||||
set({
|
set({
|
||||||
isAuthenticated: true,
|
isAuthenticated: true,
|
||||||
isLoading: false,
|
isLoading: false,
|
||||||
@@ -1424,10 +1472,15 @@ export const useAuthStore = create<AuthState>()(
|
|||||||
// persisted client state left over from an older build, or any future
|
// persisted client state left over from an older build, or any future
|
||||||
// slot desync) can hand back a *different* account's token; the
|
// slot desync) can hand back a *different* account's token; the
|
||||||
// connection then succeeds and we would silently show the wrong
|
// connection then succeeds and we would silently show the wrong
|
||||||
// mailbox. On mismatch, drop the poisoned cookies for this slot and
|
// mailbox. We accept the session when it matches the stored accountId
|
||||||
// force a clean re-auth instead of surfacing someone else's mail.
|
// OR the server identity captured at this account's login (so a short
|
||||||
|
// login username the server canonicalizes to a full address is still
|
||||||
|
// recognized). On a genuine mismatch, drop the poisoned cookies for
|
||||||
|
// this slot and force a clean re-auth instead of surfacing someone
|
||||||
|
// else's mail.
|
||||||
const connectedCandidates = await connectedAccountCandidates(targetClient, targetAccount.serverUrl);
|
const connectedCandidates = await connectedAccountCandidates(targetClient, targetAccount.serverUrl);
|
||||||
if (connectedCandidates.length > 0 && !connectedCandidates.includes(accountId)) {
|
const verdict = classifySessionMatch(connectedCandidates, accountId, targetAccount.serverIdentifiers);
|
||||||
|
if (verdict === 'reject') {
|
||||||
debug.error(`switchAccount: slot ${targetAccount.cookieSlot} for ${accountId} resolved to [${connectedCandidates.join(", ")}] — forcing re-auth`);
|
debug.error(`switchAccount: slot ${targetAccount.cookieSlot} for ${accountId} resolved to [${connectedCandidates.join(", ")}] — forcing re-auth`);
|
||||||
clients.delete(accountId);
|
clients.delete(accountId);
|
||||||
try { targetClient.disconnect(); } catch { /* noop */ }
|
try { targetClient.disconnect(); } catch { /* noop */ }
|
||||||
@@ -1438,6 +1491,11 @@ export const useAuthStore = create<AuthState>()(
|
|||||||
replaceWindowLocation(getLocaleLoginPath());
|
replaceWindowLocation(getLocaleLoginPath());
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
// Refresh (or, for a legacy 'trust' entry, establish) the identity
|
||||||
|
// baseline now that we've confirmed a good session.
|
||||||
|
if (connectedCandidates.length > 0) {
|
||||||
|
accountStore.updateAccount(accountId, { serverIdentifiers: connectedCandidates });
|
||||||
|
}
|
||||||
|
|
||||||
// Restore cached state or fetch fresh
|
// Restore cached state or fetch fresh
|
||||||
const restored = restoreAccount(accountId);
|
const restored = restoreAccount(accountId);
|
||||||
|
|||||||
Reference in New Issue
Block a user