diff --git a/server/src/middleware/globalMiddleware.ts b/server/src/middleware/globalMiddleware.ts index 647410d7d..e71ac145a 100644 --- a/server/src/middleware/globalMiddleware.ts +++ b/server/src/middleware/globalMiddleware.ts @@ -5,6 +5,7 @@ import helmet from 'helmet'; import cookieParser from 'cookie-parser'; import { readEnv, type AppEnv } from '../app-config'; import { logDebug, logWarn, logError } from '../nest/audit/audit-log.logger'; +import { isSameHostOrigin } from '../nest/common/same-origin'; /** * Field names redacted from request-log query/body dumps (case-insensitive — @@ -162,17 +163,28 @@ export function applyGlobalMiddleware( const allowedOrigins = http.corsOrigins; - let corsOrigin: cors.CorsOptions['origin']; - if (allowedOrigins) { - corsOrigin = (origin: string | undefined, callback: (err: Error | null, allow?: boolean) => void) => { - if (!origin || allowedOrigins.includes(origin)) callback(null, true); - else callback(new Error('Not allowed by CORS')); - }; - } else if (isProduction) { - corsOrigin = false; - } else { - corsOrigin = true; - } + // With ALLOWED_ORIGINS set, a request from anywhere else is refused outright. + // Two things keep that from hitting the instance's own pages (#2543): a request + // whose Origin is the host it was sent to is same-origin and always passes, and + // a refusal is a 403 naming the cause instead of an unhandled error, which the + // exception filter turned into a bare 500 on the login screen. + const warnedOrigins = new Set(); + const corsOptions: cors.CorsOptions | cors.CorsOptionsDelegate = allowedOrigins + ? (req: Request, callback: (err: Error | null, options?: cors.CorsOptions) => void) => { + const origin = req.headers.origin; + if (!origin || allowedOrigins.includes(origin) || isSameHostOrigin(origin, req.headers.host)) { + callback(null, { origin: true, credentials: true }); + return; + } + if (!warnedOrigins.has(origin) && warnedOrigins.size < 50) { + warnedOrigins.add(origin); + logWarn( + `CORS: refused origin ${origin} for host ${req.headers.host ?? '(none)'}; add it to ALLOWED_ORIGINS if it is yours`, + ); + } + callback(Object.assign(new Error('Not allowed by CORS'), { statusCode: 403 })); + } + : { origin: isProduction ? false : true, credentials: true }; const shouldForceHttps = http.forceHttps; // HSTS is worth enabling any time we're serving production traffic, @@ -225,7 +237,7 @@ export function applyGlobalMiddleware( } }, ); - app.use(cors({ origin: corsOrigin, credentials: true })); + app.use(cors(corsOptions)); app.use(helmet({ contentSecurityPolicy: { directives: { diff --git a/server/src/nest/common/same-origin.ts b/server/src/nest/common/same-origin.ts new file mode 100644 index 000000000..33f986a32 --- /dev/null +++ b/server/src/nest/common/same-origin.ts @@ -0,0 +1,40 @@ +/** + * Whether a browser `Origin` names the very host the request was sent to. + * + * `ALLOWED_ORIGINS` exists to let OTHER origins in. A request whose Origin is the + * instance's own host is not cross-origin at all, yet a browser still attaches the + * header to every POST and to every WebSocket upgrade. Checking only the list + * turned an ordinary login into a refusal as soon as the list and the address in + * the address bar differed in any way (another hostname, the LAN IP, http against + * https), which is #2543. + * + * The comparison is against the `Host` header only. The browser sets it from the + * URL it was asked to load, and a page on another site cannot choose it, so a + * foreign Origin never matches. `X-Forwarded-Host` is deliberately not consulted: + * that one is client-controlled unless a proxy overwrites it. + * + * The scheme is compared through the host normalisation only (a default port on + * the Host header is dropped for the Origin's own scheme), not for equality: TLS + * usually ends at a reverse proxy, so the server itself cannot tell which scheme + * the page was loaded over. + */ +export function isSameHostOrigin(origin: string | undefined, hostHeader: string | undefined): boolean { + if (!origin || !hostHeader) return false; + const host = hostHeader.trim(); + // A Host is a hostname with an optional port, nothing that a URL parser would + // read as userinfo, a path or a query. + if (!host || /[\s/\\@?#]/.test(host)) return false; + let originUrl: URL; + try { + originUrl = new URL(origin); + } catch { + return false; // `null` and anything else that is not a URL + } + if (originUrl.protocol !== 'http:' && originUrl.protocol !== 'https:') return false; + try { + const requestHost = new URL(`${originUrl.protocol}//${host}`).host; + return requestHost !== '' && requestHost === originUrl.host; + } catch { + return false; + } +} diff --git a/server/src/nest/realtime/trek-ws.adapter.ts b/server/src/nest/realtime/trek-ws.adapter.ts index ff9a29cf3..75126c793 100644 --- a/server/src/nest/realtime/trek-ws.adapter.ts +++ b/server/src/nest/realtime/trek-ws.adapter.ts @@ -1,12 +1,13 @@ import { WsAdapter } from '@nestjs/platform-ws'; import type { INestApplicationContext } from '@nestjs/common'; -import type { Server as HttpServer } from 'node:http'; +import type { IncomingMessage, Server as HttpServer } from 'node:http'; import type { MessageMappingProperties } from '@nestjs/websockets'; import { WebSocketServer } from 'ws'; import type { Observable } from 'rxjs'; import { readEnv } from '../../app-config'; import { setServer, type TrekWebSocket } from './ws-state'; import { logError } from '../audit/audit-log.logger'; +import { isSameHostOrigin } from '../common/same-origin'; // Per-connection message rate limiting. It lives in the adapter, not in the // gateway's handlers, because the original counted EVERY inbound frame before @@ -122,10 +123,12 @@ export class TrekWsAdapter extends WsAdapter { ...(allowedOrigins ? { verifyClient: ( - { origin }: { origin: string }, + { origin, req }: { origin: string; req: IncomingMessage }, cb: (ok: boolean, code?: number, msg?: string) => void, ) => { - if (!origin || allowedOrigins.includes(origin)) cb(true); + // Same rule as the REST CORS check (#2543): the instance's own host + // is never cross-origin, whatever ALLOWED_ORIGINS lists. + if (!origin || allowedOrigins.includes(origin) || isSameHostOrigin(origin, req?.headers.host)) cb(true); else cb(false, 403, 'Origin not allowed'); }, } diff --git a/server/tests/unit/middleware/globalMiddleware.cors.test.ts b/server/tests/unit/middleware/globalMiddleware.cors.test.ts new file mode 100644 index 000000000..80561025f --- /dev/null +++ b/server/tests/unit/middleware/globalMiddleware.cors.test.ts @@ -0,0 +1,105 @@ +import { describe, it, expect } from 'vitest'; +import express, { type NextFunction, type Request, type Response } from 'express'; +import request from 'supertest'; +import { applyGlobalMiddleware } from '../../../src/middleware/globalMiddleware'; +import { readEnv } from '../../../src/app-config'; +import { isSameHostOrigin } from '../../../src/nest/common/same-origin'; + +/** + * An app with ALLOWED_ORIGINS set, and the error mapping TrekExceptionFilter + * applies to a plain error (statusCode, else 500). + */ +function appWithAllowlist(corsOrigins: string[] | null) { + const app = express(); + applyGlobalMiddleware(app, { http: { ...readEnv().http, corsOrigins } }); + app.post('/api/auth/login', (_req, res) => res.json({ ok: true })); + app.use((err: { statusCode?: number; message?: string }, _req: Request, res: Response, _next: NextFunction) => { + const status = err.statusCode || 500; + res.status(status).json({ error: status < 500 ? err.message : 'Internal server error' }); + }); + return app; +} + +describe('CORS with ALLOWED_ORIGINS set (#2543)', () => { + const app = appWithAllowlist(['https://trek.example.com']); + + it('lets a request through whose Origin is the host it was sent to', async () => { + // The instance reached under another name than the one in ALLOWED_ORIGINS + // (its LAN address, http instead of https): the login POST carries the page's + // own Origin, and refusing it answered the login form with a 500. + const res = await request(app) + .post('/api/auth/login') + .set('Host', '192.168.1.20:30080') + .set('Origin', 'http://192.168.1.20:30080') + .send({}); + expect(res.status).toBe(200); + expect(res.headers['access-control-allow-origin']).toBe('http://192.168.1.20:30080'); + }); + + it('still lets the listed origin through, with credentials', async () => { + const res = await request(app).post('/api/auth/login').set('Origin', 'https://trek.example.com').send({}); + expect(res.status).toBe(200); + expect(res.headers['access-control-allow-origin']).toBe('https://trek.example.com'); + expect(res.headers['access-control-allow-credentials']).toBe('true'); + }); + + it('still lets a request without Origin through', async () => { + const res = await request(app).post('/api/auth/login').send({}); + expect(res.status).toBe(200); + }); + + it('refuses a foreign origin with a 403 that names the cause, not a 500', async () => { + const res = await request(app) + .post('/api/auth/login') + .set('Host', 'trek.example.com') + .set('Origin', 'https://evil.example') + .send({}); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'Not allowed by CORS' }); + expect(res.headers['access-control-allow-origin']).toBeUndefined(); + }); + + it('refuses Origin: null', async () => { + const res = await request(app).post('/api/auth/login').set('Origin', 'null').send({}); + expect(res.status).toBe(403); + }); +}); + +describe('CORS without ALLOWED_ORIGINS', () => { + it('passes everything through as before (the test env is not production)', async () => { + const res = await request(appWithAllowlist(null)) + .post('/api/auth/login') + .set('Origin', 'https://anything.example') + .send({}); + expect(res.status).toBe(200); + expect(res.headers['access-control-allow-origin']).toBe('https://anything.example'); + }); +}); + +describe('isSameHostOrigin', () => { + it('matches host and port', () => { + expect(isSameHostOrigin('http://192.168.1.20:30080', '192.168.1.20:30080')).toBe(true); + expect(isSameHostOrigin('https://trek.example.com', 'trek.example.com')).toBe(true); + expect(isSameHostOrigin('https://Trek.Example.com', 'TREK.example.com')).toBe(true); + }); + + it('drops the default port of the Origin\'s scheme from the Host', () => { + expect(isSameHostOrigin('https://trek.example.com', 'trek.example.com:443')).toBe(true); + expect(isSameHostOrigin('http://trek.example.com', 'trek.example.com:80')).toBe(true); + }); + + it('does not match another host or another port', () => { + expect(isSameHostOrigin('https://evil.example', 'trek.example.com')).toBe(false); + expect(isSameHostOrigin('http://192.168.1.20:3000', '192.168.1.20:30080')).toBe(false); + expect(isSameHostOrigin('https://trek.example.com.evil.example', 'trek.example.com')).toBe(false); + }); + + it('refuses anything that is not an http(s) origin or a plain host', () => { + expect(isSameHostOrigin('null', 'trek.example.com')).toBe(false); + expect(isSameHostOrigin(undefined, 'trek.example.com')).toBe(false); + expect(isSameHostOrigin('https://trek.example.com', undefined)).toBe(false); + expect(isSameHostOrigin('file://trek.example.com', 'trek.example.com')).toBe(false); + expect(isSameHostOrigin('https://evil.example', 'x@evil.example')).toBe(false); + expect(isSameHostOrigin('https://evil.example', 'evil.example/path')).toBe(false); + }); +}); diff --git a/server/tests/unit/nest/trek-ws-adapter.test.ts b/server/tests/unit/nest/trek-ws-adapter.test.ts index 83d10210d..34c0a1860 100644 --- a/server/tests/unit/nest/trek-ws-adapter.test.ts +++ b/server/tests/unit/nest/trek-ws-adapter.test.ts @@ -282,6 +282,28 @@ describe('TrekWsAdapter server creation', () => { } }); + it('WSAD-031d: a same-host origin passes even when the allowlist names another (#2543)', () => { + // A browser always sends Origin on an upgrade. Reached at its LAN address or + // under a second name, the instance is still talking to its own page, and + // refusing that left the planner without live updates. + wsOrigins.value = ['https://trip.example']; + const ad = new TrekWsAdapter(httpServer); + const server = ad.create(0, { path: '/ws' }) as { options: { verifyClient?: unknown } }; + try { + const verify = server.options.verifyClient as ( + info: { origin: string; req: { headers: { host?: string } } }, + cb: (ok: boolean, code?: number, msg?: string) => void, + ) => void; + const seen: unknown[][] = []; + verify({ origin: 'http://192.168.1.20:3000', req: { headers: { host: '192.168.1.20:3000' } } }, (...args) => seen.push(args)); + // A foreign page cannot pick the Host header, so it still gets the 403. + verify({ origin: 'https://evil.example', req: { headers: { host: '192.168.1.20:3000' } } }, (...args) => seen.push(args)); + expect(seen).toEqual([[true], [false, 403, 'Origin not allowed']]); + } finally { + void ad.close(server as never); + } + }); + it('WSAD-030b: falls back to /ws when the gateway declares no path', () => { wsOrigins.value = null; const ad = new TrekWsAdapter(httpServer); diff --git a/wiki/Troubleshooting.md b/wiki/Troubleshooting.md index d7458672e..db4c9bcce 100644 --- a/wiki/Troubleshooting.md +++ b/wiki/Troubleshooting.md @@ -334,7 +334,7 @@ docker logs 2>&1 | grep -E "SMTP test email (sent|failed)|SMTP test ## CORS error — API requests blocked in the browser -**Cause:** If `ALLOWED_ORIGINS` is set, only those origins are permitted. Any request from a different origin is rejected with a CORS error visible in the browser console. +**Cause:** If `ALLOWED_ORIGINS` is set, only those origins are permitted, plus the host the request was sent to (the address in the browser's address bar is never cross-origin to itself). A request from any other origin is refused with `403 Not allowed by CORS`, and the server logs a `CORS: refused origin ...` warning naming the origin to add. **Fix:** Add your origin to the comma-separated list: