diff --git a/backend/__tests__/middleware/securityHardeningBatch2.test.js b/backend/__tests__/middleware/securityHardeningBatch2.test.js new file mode 100644 index 00000000..8d414afc --- /dev/null +++ b/backend/__tests__/middleware/securityHardeningBatch2.test.js @@ -0,0 +1,70 @@ +/** + * Second security sweep on the same branch as the password-strength DoS fix + * (stable port: the maintenance and admin-preview cases do not apply here). + * Each block pins one gap the audit found: + * + * - the general rate limiter skipped anyone holding ANY verified JWT, + * including a gallery token minted for free on password-less galleries + * - the multipart branch of the CSRF Content-Type gate accepted cross-site + * form posts + */ +const jwt = require('jsonwebtoken'); + +process.env.JWT_SECRET = 'hardening-batch2-secret'; + +const fake = { maintenance: 'true', revoked: false, beforeCutoff: false, admin: { id: 1, password_changed_at: null } }; + +jest.mock('../../src/database/db', () => { + const db = jest.fn((table) => { + const q = { + where: jest.fn().mockReturnThis(), + select: jest.fn().mockReturnThis(), + first: jest.fn(async () => { + if (table === 'app_settings') { + return { setting_key: 'general_maintenance_mode', setting_value: fake.maintenance }; + } + if (table === 'admin_users') return fake.admin; + return null; + }), + }; + return q; + }); + return { db, withRetry: (fn) => fn() }; +}); +jest.mock('../../src/utils/logger', () => ({ error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() })); +process.env.FRONTEND_URL = 'https://photos.example.com'; + +const { isAuthenticated } = require('../../src/services/rateLimitService'); +const { multipartOriginAllowed } = require('../../src/utils/requestOrigin'); + +const iat = Math.floor(Date.now() / 1000) - 10; +const adminToken = (extra = {}) => jwt.sign({ type: 'admin', id: 1, iat, ...extra }, process.env.JWT_SECRET, { issuer: 'picpeak-auth' }); +const galleryToken = () => jwt.sign({ type: 'gallery', eventId: 1, iat }, process.env.JWT_SECRET, { issuer: 'picpeak-auth' }); + +describe('general rate limiter skip', () => { + const req = (token) => ({ path: '/api/gallery/x/photos', headers: { authorization: `Bearer ${token}` }, cookies: {} }); + it('is granted to an admin session', () => { + expect(isAuthenticated(req(adminToken()))).toBe(true); + }); + it('is NOT granted to a gallery token', () => { + expect(isAuthenticated(req(galleryToken()))).toBe(false); + }); +}); + +describe('multipart origin gate', () => { + const req = (headers) => ({ headers: { host: 'photos.example.com', ...headers } }); + it('accepts same-origin, same-site and non-browser requests', () => { + expect(multipartOriginAllowed(req({ 'sec-fetch-site': 'same-origin' }))).toBe(true); + expect(multipartOriginAllowed(req({ 'sec-fetch-site': 'same-site' }))).toBe(true); + expect(multipartOriginAllowed(req({ 'sec-fetch-site': 'none' }))).toBe(true); + expect(multipartOriginAllowed(req({}))).toBe(true); + expect(multipartOriginAllowed(req({ origin: 'https://photos.example.com' }))).toBe(true); + // Same-origin install without FRONTEND_URL: Origin matches the Host. + expect(multipartOriginAllowed({ headers: { host: 'gallery.local', origin: 'http://gallery.local' } })).toBe(true); + }); + it('rejects cross-site form posts', () => { + expect(multipartOriginAllowed(req({ 'sec-fetch-site': 'cross-site' }))).toBe(false); + expect(multipartOriginAllowed(req({ origin: 'https://evil.example' }))).toBe(false); + expect(multipartOriginAllowed(req({ origin: 'null' }))).toBe(false); + }); +}); diff --git a/backend/server.js b/backend/server.js index c7e6bd09..adfc0e40 100644 --- a/backend/server.js +++ b/backend/server.js @@ -206,25 +206,13 @@ app.use((req, res, next) => { }); // CORS configuration (apply only to API routes) +const { isAllowedOrigin, multipartOriginAllowed } = require('./src/utils/requestOrigin'); + const corsOptions = { origin: function (origin, callback) { - const allowedOrigins = [ - process.env.FRONTEND_URL || 'http://localhost:3005', - process.env.ADMIN_URL || 'http://localhost:3005' - ]; - - // In development, also allow localhost origins - if (process.env.NODE_ENV === 'development') { - allowedOrigins.push( - 'http://localhost:5173', // Vite dev server - 'http://localhost:3002', // Backend server - 'http://localhost:3001', // For API testing - 'http://localhost:3000' // Direct backend access - ); - } - + // Allowlist lives in utils/requestOrigin, shared with the multipart gate. // Allow requests with no origin (like curl) and allow-listed origins - if (!origin || allowedOrigins.indexOf(origin) !== -1) { + if (!origin || isAllowedOrigin(origin)) { callback(null, true); } else { // Do not error globally; just omit CORS headers on disallowed origins @@ -452,8 +440,14 @@ async function initializeRateLimiters() { } // Note: Rate limiters will be initialized after database connection -app.use(express.json({ limit: '50mb' })); -app.use(express.urlencoded({ extended: true, limit: '50mb' })); +// Body limits. 50mb is only needed by the authenticated admin and API-token +// surfaces (restore manifests, CMS and email templates, bulk operations); +// applied globally it let any unauthenticated caller hand JSON.parse a 50mb +// body and block the event loop. express.json skips a request whose body +// is already parsed, so the scoped parser must run first. +app.use(['/api/admin', '/api/v1'], express.json({ limit: '50mb' })); +app.use(express.json({ limit: '2mb' })); +app.use(express.urlencoded({ extended: true, limit: '2mb' })); // CSRF protection: require JSON Content-Type on mutating API requests // This blocks cross-origin form submissions which cannot set Content-Type: application/json @@ -465,6 +459,14 @@ app.use('/api', (req, res, next) => { if (contentLength > 0 && !contentType.includes('application/json') && !contentType.includes('multipart/form-data')) { return res.status(415).json({ error: 'Unsupported Content-Type. Use application/json or multipart/form-data.' }); } + // multipart is exactly what a cross-site
can send without a + // preflight, and in a split-origin deployment (SameSite=None) the admin + // cookie rides along to the upload routes. Browsers label such a + // submission Sec-Fetch-Site: cross-site (and always send Origin on a + // cross-origin POST); non-browser clients send neither header and pass. + if (contentType.includes('multipart/form-data') && !multipartOriginAllowed(req)) { + return res.status(403).json({ error: 'Cross-site multipart request rejected' }); + } } next(); }); diff --git a/backend/src/services/rateLimitService.js b/backend/src/services/rateLimitService.js index 028fb8a6..eb40ab8e 100644 --- a/backend/src/services/rateLimitService.js +++ b/backend/src/services/rateLimitService.js @@ -106,8 +106,13 @@ function isAuthenticated(req) { return false; } - // Valid token found - check type - req.tokenType = decoded.type; // 'admin' or 'gallery' + // Only an admin session earns the skip. A gallery token is minted for + // free on password-less galleries and slideshow links, so treating it as + // "authenticated" handed anyone an unlimited budget on every /api route. + if (decoded.type !== 'admin') { + return false; + } + req.tokenType = decoded.type; req.tokenPayload = decoded; return true; diff --git a/backend/src/utils/requestOrigin.js b/backend/src/utils/requestOrigin.js new file mode 100644 index 00000000..2e9a1313 --- /dev/null +++ b/backend/src/utils/requestOrigin.js @@ -0,0 +1,40 @@ +/** + * Origin allow-listing shared by the CORS options and the multipart CSRF gate + * in server.js. Kept apart from server.js so it can be unit-tested without + * booting the app. + */ +function isAllowedOrigin(origin) { + const allowedOrigins = [ + process.env.FRONTEND_URL || 'http://localhost:3005', + process.env.ADMIN_URL || 'http://localhost:3005' + ]; + if (process.env.NODE_ENV === 'development') { + allowedOrigins.push( + 'http://localhost:5173', // Vite dev server + 'http://localhost:3002', // Backend server + 'http://localhost:3001', // For API testing + 'http://localhost:3000' // Direct backend access + ); + } + return allowedOrigins.indexOf(origin) !== -1; +} + +// Origin check for multipart bodies (see the Content-Type gate below). +// Same-origin installs proxy /api through nginx and may not have FRONTEND_URL +// set, so an Origin matching the request Host is accepted alongside the CORS +// allowlist; Sec-Fetch-Site is authoritative when a browser sends it. +function multipartOriginAllowed(req) { + const site = req.headers['sec-fetch-site']; + if (site) return site !== 'cross-site'; + const origin = req.headers.origin; + if (!origin) return true; + if (isAllowedOrigin(origin)) return true; + try { + return new URL(origin).host === req.headers.host; + } catch { + return false; + } +} + + +module.exports = { isAllowedOrigin, multipartOriginAllowed };