From cde7ae1c53b71ce6820b6511a9e9900733fdd3e2 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 2 Jul 2025 21:30:15 -0400 Subject: [PATCH 1/2] feat: Enhance security of api routes --- packages/api/package.json | 2 + packages/api/src/api-app.ts | 9 ++ packages/api/src/fixtures.ts | 18 +++- .../api/src/middleware/__tests__/csrf.test.ts | 86 +++++++++++++++++++ packages/api/src/middleware/csrf.ts | 58 +++++++++++++ packages/app/src/api.ts | 19 +++- 6 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 packages/api/src/middleware/__tests__/csrf.test.ts create mode 100644 packages/api/src/middleware/csrf.ts diff --git a/packages/api/package.json b/packages/api/package.json index 87c21d842..2d64a0cb6 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -19,7 +19,9 @@ "axios": "^1.6.2", "compression": "^1.7.4", "connect-mongo": "^4.6.0", + "cookie-parser": "^1.4.7", "cors": "^2.8.5", + "csrf-csrf": "^4.0.3", "cron": "^3.5.0", "date-fns": "^2.28.0", "date-fns-tz": "^2.0.0", diff --git a/packages/api/src/api-app.ts b/packages/api/src/api-app.ts index cf4d70950..f7b80f988 100644 --- a/packages/api/src/api-app.ts +++ b/packages/api/src/api-app.ts @@ -1,5 +1,6 @@ import compression from 'compression'; import MongoStore from 'connect-mongo'; +import cookieParser from 'cookie-parser'; import express from 'express'; import session from 'express-session'; import onHeaders from 'on-headers'; @@ -7,6 +8,7 @@ import onHeaders from 'on-headers'; import * as config from './config'; import { isUserAuthenticated } from './middleware/auth'; import defaultCors from './middleware/cors'; +import { csrfProtection, csrfToken } from './middleware/csrf'; import { appErrorHandler } from './middleware/error'; import routers from './routers/api'; import clickhouseProxyRouter from './routers/api/clickhouseProxy'; @@ -27,6 +29,8 @@ const sess: session.SessionOptions & { cookie: session.CookieOptions } = { cookie: { secure: false, maxAge: 1000 * 60 * 60 * 24 * 30, // 30 days + sameSite: 'lax', + httpOnly: true, }, rolling: true, store: new MongoStore({ mongoUrl: config.MONGO_URI }), @@ -46,6 +50,7 @@ app.use(compression()); app.use(express.json({ limit: '32mb' })); app.use(express.text({ limit: '32mb' })); app.use(express.urlencoded({ extended: false, limit: '32mb' })); +app.use(cookieParser()); app.use(session(sess)); if (!config.IS_LOCAL_APP_MODE) { @@ -67,6 +72,10 @@ app.use(function (req, res, next) { }); app.use(defaultCors); +// CSRF Protection +app.use(csrfToken); +app.use(csrfProtection); + // --------------------------------------------------------------------- // ----------------------- Background Jobs ----------------------------- // --------------------------------------------------------------------- diff --git a/packages/api/src/fixtures.ts b/packages/api/src/fixtures.ts index aecdc48cb..5fabe39ba 100644 --- a/packages/api/src/fixtures.ts +++ b/packages/api/src/fixtures.ts @@ -323,8 +323,24 @@ export const getLoggedInAgent = async (server: MockServer) => { // login app await agent.post('/login/password').send(MOCK_USER).expect(302); + // Get CSRF token for subsequent requests + const csrfResponse = await agent.get('/me'); + const csrfToken = csrfResponse.headers['x-csrf-token']; + + // Create enhanced agent with CSRF support + const enhancedAgent = { + ...agent, + post: (url: string) => agent.post(url).set('x-csrf-token', csrfToken || ''), + put: (url: string) => agent.put(url).set('x-csrf-token', csrfToken || ''), + patch: (url: string) => + agent.patch(url).set('x-csrf-token', csrfToken || ''), + delete: (url: string) => + agent.delete(url).set('x-csrf-token', csrfToken || ''), + get: (url: string) => agent.get(url), + }; + return { - agent, + agent: enhancedAgent, team, user, }; diff --git a/packages/api/src/middleware/__tests__/csrf.test.ts b/packages/api/src/middleware/__tests__/csrf.test.ts new file mode 100644 index 000000000..db676f769 --- /dev/null +++ b/packages/api/src/middleware/__tests__/csrf.test.ts @@ -0,0 +1,86 @@ +import cookieParser from 'cookie-parser'; +import express from 'express'; +import session from 'express-session'; +import request from 'supertest'; + +import { csrfProtection, csrfToken } from '../csrf'; + +describe('CSRF middleware', () => { + let app: express.Application; + + beforeEach(() => { + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use( + session({ + secret: 'test-secret', + resave: false, + saveUninitialized: true, + cookie: { secure: false }, + }), + ); + app.use(csrfToken); + app.use(csrfProtection); + + // Test routes + app.get('/test', (req, res) => { + res.json({ + message: 'GET request successful', + sessionId: req.session?.id, + hasSession: !!req.session, + }); + }); + + app.post('/test', (req, res) => { + res.json({ + message: 'POST request successful', + sessionId: req.session?.id, + hasSession: !!req.session, + }); + }); + }); + + it('should allow GET requests without CSRF token', async () => { + const response = await request(app).get('/test').expect(200); + expect(response.body.message).toBe('GET request successful'); + }); + + it('should provide CSRF token in response header on GET requests', async () => { + const response = await request(app).get('/test').expect(200); + expect(response.headers['x-csrf-token']).toBeDefined(); + expect(typeof response.headers['x-csrf-token']).toBe('string'); + }); + + it('should reject POST requests without CSRF token', async () => { + await request(app).post('/test').send({ data: 'test' }).expect(403); + }); + + it('should accept POST requests with valid CSRF token', async () => { + // Use the same agent for both requests to maintain session + const agent = request.agent(app); + + // First get CSRF token (this also establishes the session and sets cookie) + const getResponse = await agent.get('/test').expect(200); + const csrfToken = getResponse.headers['x-csrf-token']; + + expect(csrfToken).toBeDefined(); + expect(typeof csrfToken).toBe('string'); + + // The agent should automatically include cookies from the first request + // Make POST request with the same agent and token + await agent + .post('/test') + .set('x-csrf-token', csrfToken) + .send({ data: 'test' }) + .expect(200); + }); + + it('should reject POST requests with invalid CSRF token', async () => { + await request(app) + .post('/test') + .set('x-csrf-token', 'invalid-token') + .send({ data: 'test' }) + .expect(403); + }); +}); diff --git a/packages/api/src/middleware/csrf.ts b/packages/api/src/middleware/csrf.ts new file mode 100644 index 000000000..72bbf6cf1 --- /dev/null +++ b/packages/api/src/middleware/csrf.ts @@ -0,0 +1,58 @@ +import { doubleCsrf } from 'csrf-csrf'; +import { NextFunction, Request, Response } from 'express'; + +const { generateCsrfToken, doubleCsrfProtection } = doubleCsrf({ + getSecret: () => process.env.EXPRESS_SESSION_SECRET || 'fallback-secret', + getSessionIdentifier: req => req.session?.id || 'anonymous', + cookieName: '__csrf', + cookieOptions: { + httpOnly: true, + secure: process.env.NODE_ENV === 'production', + sameSite: 'lax', + maxAge: 1000 * 60 * 60 * 24, + }, + size: 64, + ignoredMethods: ['GET', 'HEAD', 'OPTIONS'], +}); + +export interface CSRFRequest extends Request { + csrfToken?: () => string; +} + +const shouldSkipCsrf = (path: string): boolean => { + if (path.startsWith('/api/v1')) return true; + if (path.includes('/webhook')) return true; + if (path.startsWith('/heroku')) return true; + + const authRoutes = ['/login', '/logout', '/register', '/password-reset']; + return authRoutes.some(route => path.includes(route)); +}; + +export const csrfToken = ( + req: CSRFRequest, + res: Response, + next: NextFunction, +) => { + req.csrfToken = () => generateCsrfToken(req, res); + + if (!shouldSkipCsrf(req.path)) { + const token = req.csrfToken(); + res.setHeader('X-CSRF-Token', token); + } + + next(); +}; + +export const csrfProtection = ( + req: Request, + res: Response, + next: NextFunction, +) => { + if (shouldSkipCsrf(req.path)) { + return next(); + } + + doubleCsrfProtection(req, res, next); +}; + +export { generateCsrfToken }; diff --git a/packages/app/src/api.ts b/packages/app/src/api.ts index 29c0cdfe7..e39967521 100644 --- a/packages/app/src/api.ts +++ b/packages/app/src/api.ts @@ -47,11 +47,28 @@ export function loginHook(request: Request, options: any, response: Response) { } } +let csrfToken: string | null = null; + export const server = ky.create({ prefixUrl: '/api', credentials: 'include', hooks: { - afterResponse: [loginHook], + beforeRequest: [ + request => { + if (csrfToken) { + request.headers.set('x-csrf-token', csrfToken); + } + }, + ], + afterResponse: [ + loginHook, + (request, options, response) => { + const token = response.headers.get('X-CSRF-Token'); + if (token) { + csrfToken = token; + } + }, + ], }, timeout: false, }); From 97afe94eada541ea3d5cd88e70c4d5ef83f82250 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Thu, 3 Jul 2025 15:16:47 -0400 Subject: [PATCH 2/2] Add CSRF protection middleware and exclude clickhouse-proxy endpoint --- packages/api/package.json | 2 +- packages/api/src/middleware/csrf.ts | 1 + yarn.lock | 30 ++++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/api/package.json b/packages/api/package.json index 2d64a0cb6..b33bf3eba 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -21,8 +21,8 @@ "connect-mongo": "^4.6.0", "cookie-parser": "^1.4.7", "cors": "^2.8.5", - "csrf-csrf": "^4.0.3", "cron": "^3.5.0", + "csrf-csrf": "^4.0.3", "date-fns": "^2.28.0", "date-fns-tz": "^2.0.0", "express": "^4.19.2", diff --git a/packages/api/src/middleware/csrf.ts b/packages/api/src/middleware/csrf.ts index 72bbf6cf1..39b73c605 100644 --- a/packages/api/src/middleware/csrf.ts +++ b/packages/api/src/middleware/csrf.ts @@ -23,6 +23,7 @@ const shouldSkipCsrf = (path: string): boolean => { if (path.startsWith('/api/v1')) return true; if (path.includes('/webhook')) return true; if (path.startsWith('/heroku')) return true; + if (path.startsWith('/clickhouse-proxy')) return true; const authRoutes = ['/login', '/logout', '/register', '/password-reset']; return authRoutes.some(route => path.includes(route)); diff --git a/yarn.lock b/yarn.lock index ce1c2b72b..bc8a973f9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4476,8 +4476,10 @@ __metadata: axios: "npm:^1.6.2" compression: "npm:^1.7.4" connect-mongo: "npm:^4.6.0" + cookie-parser: "npm:^1.4.7" cors: "npm:^2.8.5" cron: "npm:^3.5.0" + csrf-csrf: "npm:^4.0.3" date-fns: "npm:^2.28.0" date-fns-tz: "npm:^2.0.0" esbuild: "npm:^0.25.5" @@ -13746,6 +13748,16 @@ __metadata: languageName: node linkType: hard +"cookie-parser@npm:^1.4.7": + version: 1.4.7 + resolution: "cookie-parser@npm:1.4.7" + dependencies: + cookie: "npm:0.7.2" + cookie-signature: "npm:1.0.6" + checksum: 10c0/46bef553de409031b69a6074ce512d131a98e4fa12612669f1a9c3dd98d56897a31db86a3f4338d4a3a895c6f8d5cfd6fa4d99cdf588e0e8eda655efc3f384dc + languageName: node + linkType: hard + "cookie-signature@npm:1.0.6": version: 1.0.6 resolution: "cookie-signature@npm:1.0.6" @@ -13767,6 +13779,13 @@ __metadata: languageName: node linkType: hard +"cookie@npm:0.7.2": + version: 0.7.2 + resolution: "cookie@npm:0.7.2" + checksum: 10c0/9596e8ccdbf1a3a88ae02cf5ee80c1c50959423e1022e4e60b91dd87c622af1da309253d8abdb258fb5e3eacb4f08e579dc58b4897b8087574eee0fd35dfa5d2 + languageName: node + linkType: hard + "cookie@npm:^0.5.0": version: 0.5.0 resolution: "cookie@npm:0.5.0" @@ -14001,6 +14020,15 @@ __metadata: languageName: node linkType: hard +"csrf-csrf@npm:^4.0.3": + version: 4.0.3 + resolution: "csrf-csrf@npm:4.0.3" + dependencies: + http-errors: "npm:^2.0.0" + checksum: 10c0/29a73ca1448226233dc48669f9b02b19e27d5e2238af284d72ba938cf8619971fc1dac3a2c5f7d6f1a23f9b0ed0fc9941e1328900c498df63ecea9a6b3b93db5 + languageName: node + linkType: hard + "css-functions-list@npm:^3.2.2": version: 3.2.2 resolution: "css-functions-list@npm:3.2.2" @@ -18084,7 +18112,7 @@ __metadata: languageName: node linkType: hard -"http-errors@npm:2.0.0": +"http-errors@npm:2.0.0, http-errors@npm:^2.0.0": version: 2.0.0 resolution: "http-errors@npm:2.0.0" dependencies: