feat(portal-bff): helmet + env-driven CORS allowlist + double-submit CSRF
phase-2 security baseline the main.ts placeholder has been
advertising since the auth track started. three independent
middlewares + their spa counterparts shipped together because
they only become meaningful as a set.
helmet on the bff:
helmet() with three overrides matching our specific shape:
- HSTS only in production (dev runs on plain http)
- crossOriginResourcePolicy: 'cross-origin' (SPA needs to read
bff json from a different origin)
- CSP disabled in non-prod (bff doesn't render html; default CSP
just adds devtools noise)
cors allowlist env-driven:
CORS_ALLOWED_ORIGINS is now mandatory at boot. readCorsAllowlist()
parses + validates (http(s) only, bare origins, no path/query),
same boot-time validator family as assertSessionSecret etc. the
previous hardcoded localhost fallback is removed — silent
cors misconfiguration is exactly the "works in dev breaks in
prod" trap this guard catches.
double-submit CSRF (session-bound):
- bff mints a 256-bit csrfToken at /auth/callback, stored on
req.session.csrfToken AND mirrored to a js-readable cookie
(__Host-portal_csrf prod / portal_csrf dev). cookie is the
spa's read-only view; session is the source of truth.
- createCsrfMiddleware compares X-CSRF-Token header to the
session token via crypto.timingSafeEqual. skips: safe methods,
anonymous requests, /auth/login + /auth/callback.
- mismatch → 403 {"error":"csrf"} + structured pino warn.
- spa csrfInterceptor reads the cookie via document.cookie and
copies into X-CSRF-Token on mutating bff requests. omitted on
GET/HEAD/OPTIONS and on non-bff origins.
- logout + absolute-timeout middleware now clear the csrf cookie
alongside the session cookie.
notable choices:
- session-bound double-submit, not pure cookie-vs-header. an
attacker who plants a cookie via subdomain takeover would
still need to know the server-side session token. tying the
check to req.session.csrfToken instead of the cookie itself
is what makes this stronger than the canonical recipe.
- csrfInterceptor sits between bffCredentialsInterceptor (which
sets withCredentials) and bffUnauthorizedInterceptor (which
handles 401s). forward order, no surprises.
- no CSRF for anonymous mutating routes in v1 — none exist today
and generating tokens for anonymous sessions conflicts with
express-session's saveUninitialized: false.
specs:
- 239 / 239 pass under the clean-env repro (env -u every config
var including the new CORS_ALLOWED_ORIGINS).
- +42 specs across check-cors-allowlist, csrf-cookie, csrf
middleware, csrf interceptor, and extended auth.controller /
absolute-timeout coverage for the csrf cookie mirror/clear.
out of scope, landing in follow-ups:
- rate limiting + structured error filter (remaining phase-2
todos)
- CSP fine-tuning for the portal-shell + portal-admin static
bundles
- csrf token rotation on idle-extension (current token lives
the session lifetime)
This commit is contained in:
@@ -0,0 +1,177 @@
|
||||
import type { NextFunction, Request, Response } from 'express';
|
||||
import type { Logger } from 'nestjs-pino';
|
||||
import { createCsrfMiddleware } from './csrf.middleware';
|
||||
|
||||
interface SessionStub {
|
||||
user?: { oid: string };
|
||||
csrfToken?: string;
|
||||
}
|
||||
|
||||
function makeReq(opts: {
|
||||
method: string;
|
||||
path: string;
|
||||
headers?: Record<string, string>;
|
||||
session?: SessionStub;
|
||||
}): Request {
|
||||
const headers = opts.headers ?? {};
|
||||
return {
|
||||
method: opts.method,
|
||||
path: opts.path,
|
||||
session: opts.session,
|
||||
get: (name: string) => headers[name],
|
||||
} as unknown as Request;
|
||||
}
|
||||
|
||||
function makeRes() {
|
||||
const res = {
|
||||
status: jest.fn(),
|
||||
json: jest.fn(),
|
||||
};
|
||||
res.status.mockReturnValue(res);
|
||||
res.json.mockReturnValue(res);
|
||||
return res as unknown as Response & { status: jest.Mock; json: jest.Mock };
|
||||
}
|
||||
|
||||
function makeLogger() {
|
||||
return {
|
||||
log: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
debug: jest.fn(),
|
||||
} as unknown as Logger & { warn: jest.Mock };
|
||||
}
|
||||
|
||||
describe('csrf middleware', () => {
|
||||
const next: NextFunction = jest.fn();
|
||||
beforeEach(() => (next as jest.Mock).mockReset());
|
||||
|
||||
describe('skip cases', () => {
|
||||
it.each(['GET', 'HEAD', 'OPTIONS'])('passes through safe method %s', (method) => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({ method, path: '/api/anything' });
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(next).toHaveBeenCalledTimes(1);
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('passes through anonymous mutating requests (no req.session.user)', () => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({ method: 'POST', path: '/api/public', session: {} });
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(next).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('passes through /api/auth/login (CSRF-exempt: session is born here)', () => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({
|
||||
method: 'POST',
|
||||
path: '/api/auth/login',
|
||||
session: { user: { oid: 'u' }, csrfToken: 'irrelevant' },
|
||||
});
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(next).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('passes through /api/auth/callback (CSRF-exempt)', () => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({
|
||||
method: 'POST',
|
||||
path: '/api/auth/callback',
|
||||
session: { user: { oid: 'u' }, csrfToken: 'irrelevant' },
|
||||
});
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(next).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('enforcement', () => {
|
||||
it('rejects with 403 when an authenticated mutation lacks X-CSRF-Token', () => {
|
||||
const logger = makeLogger();
|
||||
const csrf = createCsrfMiddleware(logger);
|
||||
const req = makeReq({
|
||||
method: 'POST',
|
||||
path: '/api/profile',
|
||||
session: { user: { oid: 'u' }, csrfToken: 'expected-token' },
|
||||
});
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(res.json).toHaveBeenCalledWith({ error: 'csrf' });
|
||||
expect(next).not.toHaveBeenCalled();
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ event: 'csrf.reject', hasHeaderToken: false }),
|
||||
'Csrf',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects with 403 when the header value mismatches the session token', () => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({
|
||||
method: 'POST',
|
||||
path: '/api/profile',
|
||||
headers: { 'X-CSRF-Token': 'wrong-token' },
|
||||
session: { user: { oid: 'u' }, csrfToken: 'expected-token' },
|
||||
});
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
});
|
||||
|
||||
it('rejects with 403 when the session has no csrfToken (auth without CSRF — should never happen)', () => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({
|
||||
method: 'POST',
|
||||
path: '/api/profile',
|
||||
headers: { 'X-CSRF-Token': 'anything' },
|
||||
session: { user: { oid: 'u' } },
|
||||
});
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
});
|
||||
|
||||
it('passes when header matches the session token exactly', () => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({
|
||||
method: 'POST',
|
||||
path: '/api/profile',
|
||||
headers: { 'X-CSRF-Token': 'expected-token' },
|
||||
session: { user: { oid: 'u' }, csrfToken: 'expected-token' },
|
||||
});
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(next).toHaveBeenCalledTimes(1);
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(['PUT', 'PATCH', 'DELETE'])('enforces on mutation method %s like POST', (method) => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({
|
||||
method,
|
||||
path: '/api/profile',
|
||||
headers: { 'X-CSRF-Token': 'tok' },
|
||||
session: { user: { oid: 'u' }, csrfToken: 'tok' },
|
||||
});
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(next).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('uses constant-time comparison (different lengths short-circuit as unequal)', () => {
|
||||
const csrf = createCsrfMiddleware(makeLogger());
|
||||
const req = makeReq({
|
||||
method: 'POST',
|
||||
path: '/api/profile',
|
||||
headers: { 'X-CSRF-Token': 'short' },
|
||||
session: { user: { oid: 'u' }, csrfToken: 'much-longer-token-value' },
|
||||
});
|
||||
const res = makeRes();
|
||||
csrf(req, res, next);
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user