mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-03 08:15:38 +00:00
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.
This commit is contained in:
@@ -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<string | undefined> {
|
||||
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.
|
||||
|
||||
@@ -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<string, unknown> = {};
|
||||
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<typeof import('@/lib/security/url-guard')>();
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user