diff --git a/backend/__tests__/middleware/securityHardeningBatch2.test.js b/backend/__tests__/middleware/securityHardeningBatch2.test.js new file mode 100644 index 00000000..e255c4a0 --- /dev/null +++ b/backend/__tests__/middleware/securityHardeningBatch2.test.js @@ -0,0 +1,123 @@ +/** + * Second security sweep on the same branch as the password-strength DoS fix. + * Each block pins one gap the audit found: + * + * - maintenance gate classified paths case-sensitively while Express routes + * case-insensitively, so /API/... bypassed maintenance mode + * - the general rate limiter skipped anyone holding ANY verified JWT, + * including a gallery token minted for free on password-less galleries + * - the admin gallery preview trusted a verified signature alone, ignoring + * revocation, deactivation and password changes + * - 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() })); +jest.mock('../../src/utils/tokenRevocation', () => ({ isTokenRevoked: jest.fn(async () => fake.revoked) })); +jest.mock('../../src/utils/sessionCutoff', () => ({ isTokenBeforeCutoff: jest.fn(async () => fake.beforeCutoff) })); +jest.mock('../../src/utils/frontendUrl', () => ({ getFrontendBaseUrlSync: () => 'https://photos.example.com' })); + +const { maintenanceMiddleware, clearMaintenanceCache } = require('../../src/middleware/maintenance'); +const { isAuthenticated } = require('../../src/services/rateLimitService'); +const { verifyAdminPreview, isAdminPreview } = require('../../src/middleware/gallery'); +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('maintenance gate is case-insensitive', () => { + async function run(path) { + clearMaintenanceCache(); + const req = { path, method: 'GET', headers: {} }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn().mockReturnThis() }; + const next = jest.fn(); + await maintenanceMiddleware(req, res, next); + return next.mock.calls.length === 1; + } + it('gates /API/gallery/... exactly like /api/gallery/...', async () => { + expect(await run('/api/gallery/x/download-all')).toBe(false); + expect(await run('/API/gallery/x/download-all')).toBe(false); + expect(await run('/Og/gallery/x')).toBe(false); + }); +}); + +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('admin preview requires a live admin session', () => { + const req = (token) => ({ query: { admin_preview: '1' }, cookies: { admin_token: token }, headers: {} }); + beforeEach(() => { fake.revoked = false; fake.beforeCutoff = false; fake.admin = { id: 1, password_changed_at: null }; }); + + it('passes for a live session and sets req.isAdminPreview', async () => { + const r = req(adminToken()); + expect(isAdminPreview(r)).toBe(true); + expect(await verifyAdminPreview(r)).toBe(true); + expect(r.isAdminPreview).toBe(true); + }); + it('fails for a revoked token', async () => { + fake.revoked = true; + const r = req(adminToken()); + expect(await verifyAdminPreview(r)).toBe(false); + expect(r.isAdminPreview).toBeUndefined(); + }); + it('fails after the restore cutoff', async () => { + fake.beforeCutoff = true; + expect(await verifyAdminPreview(req(adminToken()))).toBe(false); + }); + it('fails for a deactivated or deleted admin', async () => { + fake.admin = null; + expect(await verifyAdminPreview(req(adminToken()))).toBe(false); + }); + it('fails for a token minted before the last password change', async () => { + fake.admin = { id: 1, password_changed_at: new Date((iat + 5) * 1000).toISOString() }; + expect(await verifyAdminPreview(req(adminToken()))).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 efeb35f2..4ceff39c 100644 --- a/backend/server.js +++ b/backend/server.js @@ -218,29 +218,16 @@ 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) { // getFrontendBaseUrlSync() resolves FRONTEND_URL, else the configured // general_site_url (#705) — without it, an install that leaves the // environment untouched and answers the setup wizard instead would have // its own public origin missing from the allowlist. - const allowedOrigins = [ - getFrontendBaseUrlSync() || '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 - ); - } - // 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 @@ -527,8 +514,14 @@ app.use(createApiRateLimitGate(() => generalRateLimiter)); // and why it must stay unmounted. app.use(createAuthRateLimitGate(() => authRateLimiter)); -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 @@ -540,6 +533,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