From b73d0dfabfc66a93478d06cbdbeb44a9c0a06d6c Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 15:08:35 +0200 Subject: [PATCH] fix: stop reflecting a user-chosen server's error body from the TOTP login A failed TOTP login returned the first 500 bytes of the upstream error body. With custom JMAP endpoints allowed and no server configured, the upstream is any address the caller names, so the route read other servers' responses on the caller's behalf. The detail is now only passed on from an admin-configured server. --- app/api/auth/totp-token-exchange/route.ts | 15 ++++- .../totp-token-exchange-detail.test.ts | 67 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 lib/__tests__/totp-token-exchange-detail.test.ts diff --git a/app/api/auth/totp-token-exchange/route.ts b/app/api/auth/totp-token-exchange/route.ts index 5afe0f5ab..254e9641e 100644 --- a/app/api/auth/totp-token-exchange/route.ts +++ b/app/api/auth/totp-token-exchange/route.ts @@ -100,7 +100,7 @@ async function attemptLogin( }); if (!loginResponse.ok) { - const detail = (await loginResponse.text()).substring(0, 500); + const detail = await upstreamDetail(loginResponse, upstreamTrusted); logger.warn('TOTP login: /api/auth rejected request', { status: loginResponse.status }); // A 404 means the server predates the structured login endpoint; let the // caller fall back to the legacy basic-auth path. @@ -155,7 +155,7 @@ async function attemptLogin( }); if (!tokenResponse.ok) { - const detail = (await tokenResponse.text()).substring(0, 500); + const detail = await upstreamDetail(tokenResponse, upstreamTrusted); logger.warn('TOTP login: token exchange failed', { status: tokenResponse.status, detail }); return NextResponse.json({ error: 'token_exchange_failed', detail }, { status: 502 }); } @@ -183,6 +183,17 @@ async function attemptLogin( ); } +/** + * The start of an upstream error body, for the login screen. Only from a + * server the admin configured: a user-chosen server URL makes this route a + * way to read what any public address answers, and together with a gap in + * the address guard, internal ones too. + */ +async function upstreamDetail(response: Response, trusted: boolean): Promise { + if (!trusted) return undefined; + return (await response.text()).substring(0, 500); +} + export async function POST(request: NextRequest) { // CSRF gate (GHSA-qvr9-m8cq-7wvg): cookies written here are SameSite=Lax, // so a cross-site top-level POST would otherwise reach this handler. diff --git a/lib/__tests__/totp-token-exchange-detail.test.ts b/lib/__tests__/totp-token-exchange-detail.test.ts new file mode 100644 index 000000000..19dc16714 --- /dev/null +++ b/lib/__tests__/totp-token-exchange-detail.test.ts @@ -0,0 +1,67 @@ +// @vitest-environment node +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { NextRequest } from 'next/server'; + +// On a failed login the route handed back the first 500 bytes of the +// upstream error body. With custom JMAP endpoints on and no configured +// server, the upstream is any address the caller names, so the route read +// other servers' responses for them. + +vi.mock('@/lib/logger', () => ({ + logger: { warn: () => {}, error: () => {}, info: () => {}, debug: () => {} }, +})); +vi.mock('next/headers', () => ({ + cookies: async () => ({ get: () => undefined, set: () => {}, delete: () => {} }), +})); +vi.mock('@/lib/telemetry/login-tracker', () => ({ recordLogin: () => {} })); + +const config: Record = {}; +vi.mock('@/lib/admin/config-manager', () => ({ + configManager: { + ensureLoaded: async () => {}, + get: (key: string, fallback: unknown) => (key in config ? config[key] : fallback), + }, +})); + +const upstream = vi.fn(); +vi.mock('@/lib/security/url-guard', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + isPublicHttpUrl: async () => true, + fetchPublicUrl: (...args: unknown[]) => upstream(...args), + }; +}); + +const SECRET_BODY = 'internal admin page: api-key=sk_live_123'; + +function login(serverUrl: string) { + return new NextRequest('https://webmail.example/api/auth/totp-token-exchange', { + method: 'POST', + headers: { 'content-type': 'application/json', 'sec-fetch-site': 'same-origin' }, + body: JSON.stringify({ serverUrl, username: 'u', password: 'p', totp: '123456' }), + }); +} + +beforeEach(() => { + for (const key of Object.keys(config)) delete config[key]; + upstream.mockReset().mockResolvedValue(new Response(SECRET_BODY, { status: 403 })); + vi.stubGlobal('fetch', vi.fn(async () => new Response(SECRET_BODY, { status: 403 }))); +}); + +describe('totp-token-exchange error detail', () => { + it('does not reflect the body of a user-chosen server', async () => { + config.allowCustomJmapEndpoint = true; + const { POST } = await import('@/app/api/auth/totp-token-exchange/route'); + const res = await POST(login('https://somewhere.example')); + expect(res.status).toBe(502); + expect(JSON.stringify(await res.json())).not.toContain('sk_live_123'); + }); + + it('still shows the start of the error from a configured server', async () => { + config.jmapServerUrl = 'https://mail.example.org'; + const { POST } = await import('@/app/api/auth/totp-token-exchange/route'); + const res = await POST(login('https://mail.example.org')); + expect((await res.json()).detail).toBe(SECRET_BODY); + }); +});