diff --git a/lib/__tests__/auth-routes-credential-check.test.ts b/lib/__tests__/auth-routes-credential-check.test.ts index 222f549e9..3c49cbe32 100644 --- a/lib/__tests__/auth-routes-credential-check.test.ts +++ b/lib/__tests__/auth-routes-credential-check.test.ts @@ -303,12 +303,15 @@ describe('POST /api/auth/stalwart-context (GHSA-wxcm-j4jc-9fxq)', () => { expect(setStalwartAuthContext).toHaveBeenCalledWith(0, expect.objectContaining({ username: 'linus@example.org' })); }); - it('accepts the bare local part of the session username (Stalwart canonicalizes it)', async () => { + it('refuses an address on a domain the bare session username does not name', async () => { + // `lrath` holds no address at corp.example; lrath@corp.example may be + // another account's login, with its own synced settings. const { status } = await postContext({ serverUrl: SERVER, username: 'lrath@corp.example', authHeader: 'Bearer tok-linus', slot: 0, }); - expect(status).toBe(200); - expect(identityGetCalls).toBe(0); + expect(status).toBe(403); + expect(identityGetCalls).toBe(1); + expect(setStalwartAuthContext).not.toHaveBeenCalled(); }); it('still mints the context for a Basic credential the server accepts', async () => { diff --git a/lib/__tests__/verify-jmap-identity.test.ts b/lib/__tests__/verify-jmap-identity.test.ts index 0c2e3d6c4..82ab4616a 100644 --- a/lib/__tests__/verify-jmap-identity.test.ts +++ b/lib/__tests__/verify-jmap-identity.test.ts @@ -47,10 +47,15 @@ describe('usernameMatchesSession', () => { expect(usernameMatchesSession('UserA@Example.org', 'usera@example.org')).toBe(true); }); - it('matches a bare local part against the canonical e-mail login on either side', async () => { + it('matches a bare local part against the canonical e-mail login', async () => { const { usernameMatchesSession } = await load(); expect(usernameMatchesSession('usera', 'usera@example.org')).toBe(true); - expect(usernameMatchesSession('linus@example.org', 'linus')).toBe(true); + }); + + it('does not let a bare login name vouch for an address on any domain', async () => { + // `linus` says nothing about which domain is his; his identities do. + const { usernameMatchesSession } = await load(); + expect(usernameMatchesSession('linus@example.org', 'linus')).toBe(false); }); it('rejects different users, different domains and missing session usernames', async () => { @@ -206,6 +211,24 @@ describe('resolveJmapIdentity account name', () => { .resolves.toEqual({ serverUrl: 'https://mail.internal', accountName: 'John@B.example' }); }); + it('does not key a bare login on an address of another domain', async () => { + // `info` (info@a.example) claiming info@b.example, another account's + // login, would read and write that account's synced settings. + const session = { + apiUrl: 'https://mail.internal/jmap/', + username: 'info', + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'c' }, + accounts: {}, + }; + const identities = { methodResponses: [['Identity/get', { accountId: 'c', list: [{ id: 'i0', email: 'info@a.example' }] }, '0']] }; + fetchSpy.mockImplementation(async (url: RequestInfo | URL) => json(String(url).endsWith('/.well-known/jmap') ? session : identities)); + const { resolveJmapIdentity } = await load(); + await expect(resolveJmapIdentity('https://mail.internal', 'Bearer tok', 'info@b.example', { trusted: true })) + .rejects.toMatchObject({ status: 403 }); + await expect(resolveJmapIdentity('https://mail.internal', 'Bearer tok', 'info@a.example', { trusted: true })) + .resolves.toEqual({ serverUrl: 'https://mail.internal', accountName: 'info@a.example' }); + }); + it('keeps a Basic claim as it is: the server accepted exactly that login', async () => { fetchSpy.mockResolvedValueOnce(json({ apiUrl: 'https://mail.internal/jmap/', username: 'john@a.example', accounts: {} })); const { resolveJmapIdentity } = await load(); diff --git a/lib/auth/verify-jmap-auth.ts b/lib/auth/verify-jmap-auth.ts index 5ddc0cc42..05f9fb86a 100644 --- a/lib/auth/verify-jmap-auth.ts +++ b/lib/auth/verify-jmap-auth.ts @@ -189,19 +189,18 @@ function usernamesEqual(a: string, b: string): boolean { * Does the claimed login name denote the server-reported one? Stalwart * canonicalizes the login it echoes in `Session.username` (`UserA@x` and the * bare local part `usera` both come back as `usera@x`), so besides a - * case-insensitive match we accept a bare local part on either side. + * case-insensitive match we accept a bare local part as the claim. Not the + * other way round: a bare `Session.username` names no domain, so `info` + * would vouch for `info@` any domain, another account's login among them. + * Such a claim has to be one of the account's identities instead. */ export function usernameMatchesSession(claimed: string, sessionUsername: string | undefined): boolean { if (!sessionUsername) return false; if (usernamesEqual(claimed, sessionUsername)) return true; - const claimedAt = claimed.indexOf('@'); const sessionAt = sessionUsername.indexOf('@'); - if (claimedAt < 0 && sessionAt > 0) { + if (!claimed.includes('@') && sessionAt > 0) { return usernamesEqual(claimed, sessionUsername.slice(0, sessionAt)); } - if (sessionAt < 0 && claimedAt > 0) { - return usernamesEqual(claimed.slice(0, claimedAt), sessionUsername); - } return false; }