diff --git a/backend/__tests__/routes/passwordStrengthDos.test.js b/backend/__tests__/routes/passwordStrengthDos.test.js index 7e60320d..ab568497 100644 --- a/backend/__tests__/routes/passwordStrengthDos.test.js +++ b/backend/__tests__/routes/passwordStrengthDos.test.js @@ -65,8 +65,10 @@ describe('password validation length cap (zxcvbn DoS)', () => { // No route may hand errors.array() straight to the response. expect(src).not.toMatch(/errors:\s*errors\.array\(\)/); - // ...and the helper that replaces it must drop `value`. - expect(src).toMatch(/safeValidationErrors\s*=\s*\(errors\)\s*=>\s*errors\.array\(\)\.map\(\(\{ value, \.\.\.rest \}\)/); + // ...and the shared helper that replaces it must drop `value`. + const helper = require('fs').readFileSync( + require('path').join(__dirname, '../../src/utils/routeHelpers.js'), 'utf8'); + expect(helper).toMatch(/safeValidationErrors\s*=\s*\(errors\)\s*=>\s*errors\.array\(\)\.map\(\(\{ value, \.\.\.rest \}\)/); }); it('applies the cap through the context wrapper too', async () => { diff --git a/backend/src/routes/adminApiTokens.js b/backend/src/routes/adminApiTokens.js index 7c942df3..eba22006 100644 --- a/backend/src/routes/adminApiTokens.js +++ b/backend/src/routes/adminApiTokens.js @@ -7,6 +7,7 @@ const express = require('express'); const { body, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); const { db, logActivity } = require('../database/db'); const { adminAuth } = require('./../middleware/auth'); const { requirePermission } = require('./../middleware/permissions'); @@ -74,7 +75,7 @@ router.post( try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { name, scopes, expires_at } = req.body; const { plaintext, hashed, preview } = generateApiToken(); diff --git a/backend/src/routes/adminCMS.js b/backend/src/routes/adminCMS.js index db4bcc9f..0df5902b 100644 --- a/backend/src/routes/adminCMS.js +++ b/backend/src/routes/adminCMS.js @@ -3,6 +3,7 @@ const path = require('path'); const fs = require('fs').promises; const multer = require('multer'); const { body, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); const { db, logActivity } = require('../database/db'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); @@ -80,7 +81,7 @@ router.put('/pages/:slug', adminAuth, requirePermission('cms.edit'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { slug } = req.params; diff --git a/backend/src/routes/adminCategories.js b/backend/src/routes/adminCategories.js index e7bddada..4461a76d 100644 --- a/backend/src/routes/adminCategories.js +++ b/backend/src/routes/adminCategories.js @@ -1,5 +1,6 @@ const express = require('express'); const { body, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); const { db, logActivity } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { parseBooleanInput } = require('../utils/parsers'); @@ -53,7 +54,7 @@ router.post('/', adminAuth, requirePermission('settings.edit'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { name, slug, is_global = true, event_id = null, is_folder = false } = req.body; @@ -143,7 +144,7 @@ router.put('/:id', adminAuth, requirePermission('settings.edit'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -222,7 +223,7 @@ router.put('/:id/hero', adminAuth, requirePermission('settings.edit'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -312,7 +313,7 @@ router.post('/reorder', adminAuth, requirePermission('settings.edit'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const eventId = parseInt(req.body.event_id, 10); @@ -394,7 +395,7 @@ router.post('/reorder-global', adminAuth, requirePermission('settings.edit'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const orderedIds = req.body.orderedIds.map((id) => parseInt(id, 10)); diff --git a/backend/src/routes/adminCssTemplates.js b/backend/src/routes/adminCssTemplates.js index 4bb4b288..3909f4fc 100644 --- a/backend/src/routes/adminCssTemplates.js +++ b/backend/src/routes/adminCssTemplates.js @@ -6,6 +6,7 @@ const express = require('express'); const router = express.Router(); const { body, param, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); const { db, withRetry } = require('../database/db'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); @@ -58,7 +59,7 @@ router.get('/:slotNumber', adminAuth, requirePermission('branding.view'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { slotNumber } = req.params; @@ -92,7 +93,7 @@ router.put('/:slotNumber', adminAuth, requirePermission('branding.edit'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { slotNumber } = req.params; @@ -166,7 +167,7 @@ router.post('/:slotNumber/reset', adminAuth, requirePermission('branding.edit'), try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } await withRetry(() => diff --git a/backend/src/routes/adminEmail.js b/backend/src/routes/adminEmail.js index de448785..ecbd3eb0 100644 --- a/backend/src/routes/adminEmail.js +++ b/backend/src/routes/adminEmail.js @@ -10,7 +10,7 @@ const { requireFeatureFlag } = require('../middleware/requireFeatureFlag'); const messagingGate = requireFeatureFlag('messaging'); const { wrapEmailHtml, processEmailQueue, resolveFromIdentity } = require('../services/emailProcessor'); const emailWebhookTransport = require('../services/emailWebhookTransport'); -const { errorResponse } = require('../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../utils/routeHelpers'); const logger = require('../utils/logger'); const router = express.Router(); @@ -53,7 +53,7 @@ router.post('/config', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { @@ -153,7 +153,7 @@ router.post('/incoming-config', [ ], async (req, res) => { try { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); const { imap_host, imap_port, imap_secure, imap_user, imap_pass, imap_folder } = req.body; const { isHostAllowed } = require('../utils/networkValidation'); if (!(await isHostAllowed(imap_host))) { @@ -680,7 +680,7 @@ router.get('/queue', adminAuth, requirePermission('email.view'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const page = req.query.page ? parseInt(req.query.page, 10) : 1; diff --git a/backend/src/routes/adminEventRename.js b/backend/src/routes/adminEventRename.js index 8acdbcf0..bc1089ba 100644 --- a/backend/src/routes/adminEventRename.js +++ b/backend/src/routes/adminEventRename.js @@ -5,6 +5,7 @@ const express = require('express'); const { body, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const { requireEventOwnership } = require('../middleware/ownership'); @@ -29,7 +30,7 @@ router.post('/:eventId/rename', adminAuth, requirePermission('events.edit'), req try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ success: false, errors: errors.array() }); + return res.status(400).json({ success: false, errors: safeValidationErrors(errors) }); } const { eventId } = req.params; @@ -70,7 +71,7 @@ router.post('/:eventId/validate-rename', adminAuth, requirePermission('events.ed try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ valid: false, errors: errors.array() }); + return res.status(400).json({ valid: false, errors: safeValidationErrors(errors) }); } const { eventId } = req.params; diff --git a/backend/src/routes/adminEventTypes.js b/backend/src/routes/adminEventTypes.js index 786de49c..23a5a2fd 100644 --- a/backend/src/routes/adminEventTypes.js +++ b/backend/src/routes/adminEventTypes.js @@ -7,6 +7,7 @@ const express = require('express'); const { body, param, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); const { logActivity } = require('../database/db'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); @@ -57,7 +58,7 @@ router.get('/:id', adminAuth, requirePermission(['settings.view', 'event_types.v try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -94,7 +95,7 @@ router.post('/', adminAuth, requirePermission('event_types.manage'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { @@ -156,7 +157,7 @@ router.put('/:id', adminAuth, requirePermission('event_types.manage'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -196,7 +197,7 @@ router.delete('/:id', adminAuth, requirePermission('event_types.manage'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -235,7 +236,7 @@ router.post('/reorder', adminAuth, requirePermission('event_types.manage'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { orderedIds } = req.body; diff --git a/backend/src/routes/adminEvents/archiveBulk.js b/backend/src/routes/adminEvents/archiveBulk.js index 75533ef4..72c4ecde 100644 --- a/backend/src/routes/adminEvents/archiveBulk.js +++ b/backend/src/routes/adminEvents/archiveBulk.js @@ -9,7 +9,7 @@ const { adminAuth } = require('../../middleware/auth'); const { requirePermission } = require('../../middleware/permissions'); const { archiveEvent } = require('../../services/archiveService'); const logger = require('../../utils/logger'); -const { errorResponse } = require('../../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../../utils/routeHelpers'); const { requireEventOwnership, filterOwnedEventIds } = require('../../middleware/ownership'); const { deleteEventCascade } = require('./helpers'); @@ -70,7 +70,7 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { eventIds } = req.body; @@ -160,7 +160,7 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { eventIds } = req.body; diff --git a/backend/src/routes/adminEvents/crud.js b/backend/src/routes/adminEvents/crud.js index 5317c60f..fc2dce8e 100644 --- a/backend/src/routes/adminEvents/crud.js +++ b/backend/src/routes/adminEvents/crud.js @@ -17,7 +17,7 @@ const { escapeLikePattern, likeWithEscape } = require('../../utils/sqlSecurity') const { validatePasswordInContext, getBcryptRounds } = require('../../utils/passwordValidation'); const logger = require('../../utils/logger'); const { sanitizeForLog, sanitizeValidationErrors } = require('../../utils/sanitizeForLog'); -const { errorResponse } = require('../../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../../utils/routeHelpers'); const { isUniqueViolation } = require('../../utils/dbErrors'); const { buildShareLinkVariants } = require('../../services/shareLinkService'); const { parseBooleanInput } = require('../../utils/parsers'); @@ -287,7 +287,7 @@ module.exports = (router) => { // errors.array() embeds the SUBMITTED value per field — including a // rejected plaintext password (GHSA-r794). logger.error('Validation errors:', sanitizeValidationErrors(errors.array())); - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } // Get field requirements from settings @@ -1053,7 +1053,7 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -1169,7 +1169,7 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -1307,7 +1307,7 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -1624,7 +1624,7 @@ module.exports = (router) => { if (!errors.isEmpty()) { // Redact credentials — an invalid update still logs the whole body (GHSA-pgmp). logger.debug('Update event validation errors', { errors: sanitizeValidationErrors(errors.array()), body: sanitizeForLog(req.body) }); - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; @@ -2150,7 +2150,7 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { id } = req.params; diff --git a/backend/src/routes/adminEvents/downloadResolutions.js b/backend/src/routes/adminEvents/downloadResolutions.js index 9ad25f91..b8b9284c 100644 --- a/backend/src/routes/adminEvents/downloadResolutions.js +++ b/backend/src/routes/adminEvents/downloadResolutions.js @@ -11,7 +11,7 @@ const { db, logActivity } = require('../../database/db'); const { formatBoolean } = require('../../utils/dbCompat'); const { adminAuth } = require('../../middleware/auth'); const { requirePermission } = require('../../middleware/permissions'); -const { errorResponse } = require('../../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../../utils/routeHelpers'); const { parseBooleanInput } = require('../../utils/parsers'); const { requireEventOwnership } = require('../../middleware/ownership'); const { @@ -66,7 +66,7 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ error: 'Invalid download settings', details: errors.array() }); + return res.status(400).json({ error: 'Invalid download settings', details: safeValidationErrors(errors) }); } const event = await loadOwnedEvent(req); diff --git a/backend/src/routes/adminEvents/faces.js b/backend/src/routes/adminEvents/faces.js index 42aa7281..4f313f65 100644 --- a/backend/src/routes/adminEvents/faces.js +++ b/backend/src/routes/adminEvents/faces.js @@ -13,7 +13,7 @@ const { adminAuth } = require('../../middleware/auth'); const { requirePermission } = require('../../middleware/permissions'); const { requireFeatureFlag } = require('../../middleware/requireFeatureFlag'); const { requireEventOwnership } = require('../../middleware/ownership'); -const { errorResponse } = require('../../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../../utils/routeHelpers'); const { parseBooleanInput } = require('../../utils/parsers'); const logger = require('../../utils/logger'); @@ -133,7 +133,7 @@ module.exports = (router) => { ], async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const event = await loadOwnedEvent(req); @@ -236,7 +236,7 @@ module.exports = (router) => { [body('person_a_id').isInt(), body('person_b_id').isInt()], async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const event = await loadOwnedEvent(req); @@ -278,7 +278,7 @@ module.exports = (router) => { ], async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const event = await loadOwnedEvent(req); @@ -326,7 +326,7 @@ module.exports = (router) => { [body('source_ids').isArray({ min: 1 }), body('target_id').isInt()], async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const event = await loadOwnedEvent(req); @@ -356,7 +356,7 @@ module.exports = (router) => { [body('face_ids').isArray({ min: 1 })], async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const event = await loadOwnedEvent(req); @@ -471,7 +471,7 @@ module.exports = (router) => { [body('enabled').isBoolean()], async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const enabled = parseBooleanInput(req.body.enabled, false); diff --git a/backend/src/routes/adminEvents/slideshow.js b/backend/src/routes/adminEvents/slideshow.js index 58eb1b7f..9837f3fa 100644 --- a/backend/src/routes/adminEvents/slideshow.js +++ b/backend/src/routes/adminEvents/slideshow.js @@ -8,7 +8,7 @@ const { formatBoolean } = require('../../utils/dbCompat'); const { adminAuth } = require('../../middleware/auth'); const { requirePermission } = require('../../middleware/permissions'); const crypto = require('crypto'); -const { errorResponse } = require('../../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../../utils/routeHelpers'); const { parseBooleanInput } = require('../../utils/parsers'); const { requireEventOwnership } = require('../../middleware/ownership'); const { requireFeatureFlag } = require('../../middleware/requireFeatureFlag'); @@ -113,7 +113,7 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ error: 'Invalid slideshow settings', details: errors.array() }); + return res.status(400).json({ error: 'Invalid slideshow settings', details: safeValidationErrors(errors) }); } const event = await loadOwnedEvent(req); diff --git a/backend/src/routes/adminPhotoExport.js b/backend/src/routes/adminPhotoExport.js index 343785f7..ff5a0554 100644 --- a/backend/src/routes/adminPhotoExport.js +++ b/backend/src/routes/adminPhotoExport.js @@ -11,7 +11,7 @@ const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const { requireEventOwnership } = require('../middleware/ownership'); const { PhotoFilterBuilder } = require('../utils/photoFilterBuilder'); -const { getPagination } = require('../utils/routeHelpers'); +const { getPagination, safeValidationErrors } = require('../utils/routeHelpers'); const { PhotoExportService } = require('../services/photoExportService'); const photoAdminMarksService = require('../services/photoAdminMarksService'); const feedbackService = require('../services/feedbackService'); @@ -43,7 +43,7 @@ router.get('/:eventId/filtered', adminAuth, requirePermission('photos.view'), re try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const eventId = parseInt(req.params.eventId); @@ -189,7 +189,7 @@ router.post('/:eventId/export', adminAuth, requirePermission('photos.download'), try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const eventId = parseInt(req.params.eventId); diff --git a/backend/src/routes/adminRestore.js b/backend/src/routes/adminRestore.js index 66111b2d..c0188d03 100644 --- a/backend/src/routes/adminRestore.js +++ b/backend/src/routes/adminRestore.js @@ -5,7 +5,7 @@ const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const { body, validationResult } = require('express-validator'); const logger = require('../utils/logger'); -const { getPagination } = require('../utils/routeHelpers'); +const { getPagination, safeValidationErrors } = require('../utils/routeHelpers'); const { db } = require('../database/db'); const path = require('path'); const fs = require('fs').promises; @@ -86,7 +86,7 @@ router.post('/validate', requirePermission('backup.restore'), [ if (!errors.isEmpty()) { return res.status(400).json({ success: false, - errors: errors.array() + errors: safeValidationErrors(errors) }); } @@ -158,7 +158,7 @@ router.post('/start', requirePermission('backup.restore'), [ if (!errors.isEmpty()) { return res.status(400).json({ success: false, - errors: errors.array() + errors: safeValidationErrors(errors) }); } @@ -690,7 +690,7 @@ router.put('/settings', requirePermission('backup.restore'), [ if (!errors.isEmpty()) { return res.status(400).json({ success: false, - errors: errors.array() + errors: safeValidationErrors(errors) }); } diff --git a/backend/src/routes/adminSettings.js b/backend/src/routes/adminSettings.js index d08ad826..ded78ed7 100644 --- a/backend/src/routes/adminSettings.js +++ b/backend/src/routes/adminSettings.js @@ -25,7 +25,7 @@ const { upsertAppSetting } = require('../utils/appSettings'); const { clearShareLinkSettingsCache } = require('../services/shareLinkService'); const { invalidateSiteUrlCache, isEnvPinned, envPinnedBase } = require('../utils/frontendUrl'); const { resetSecurityConfigCache } = require('../utils/authSecurity'); -const { errorResponse } = require('../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../utils/routeHelpers'); const { measureLocalStorageUsage } = require('../services/localStorageUsage'); const logger = require('../utils/logger'); const router = express.Router(); @@ -705,7 +705,7 @@ router.put('/sso', adminAuth, requirePermission('settings.security'), [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const oidcService = require('../services/oidcService'); @@ -2045,7 +2045,7 @@ router.put('/security/rate-limit', adminAuth, requirePermission('settings.securi try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { diff --git a/backend/src/routes/adminShortUrls.js b/backend/src/routes/adminShortUrls.js index 6a3d2759..518d4779 100644 --- a/backend/src/routes/adminShortUrls.js +++ b/backend/src/routes/adminShortUrls.js @@ -11,6 +11,7 @@ */ const express = require('express'); const { body, param, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const { requireEventOwnership } = require('../middleware/ownership'); @@ -32,7 +33,7 @@ router.get( requireEventOwnership, async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const rows = await galleryShortUrlService.listForEvent(parseInt(req.params.eventId, 10)); res.json({ shortUrls: rows }); @@ -56,7 +57,7 @@ router.post( requireEventOwnership, async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const row = await galleryShortUrlService.createShortUrl({ eventId: parseInt(req.params.eventId, 10), @@ -96,7 +97,7 @@ router.delete( param('id').isInt({ min: 1 }), async (req, res) => { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); try { const ok = await galleryShortUrlService.softDelete( parseInt(req.params.id, 10), diff --git a/backend/src/routes/adminWebhooks.js b/backend/src/routes/adminWebhooks.js index ad83ce92..a7770bd3 100644 --- a/backend/src/routes/adminWebhooks.js +++ b/backend/src/routes/adminWebhooks.js @@ -17,6 +17,7 @@ const express = require('express'); const { body, query, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); const { db, logActivity } = require('../database/db'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); @@ -110,7 +111,7 @@ router.post( async (req, res) => { try { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); const { name, url, events, active = true, filter, template } = req.body; const { plaintext, preview } = webhookService.generateSecret(); @@ -192,7 +193,7 @@ router.put( async (req, res) => { try { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); const row = await db('webhooks').where({ id: req.params.id }).first(); if (!row) return res.status(404).json({ error: 'Webhook not found' }); @@ -245,7 +246,7 @@ router.post( async (req, res) => { try { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); const row = await db('webhooks').where({ id: req.params.id }).first(); if (!row) return res.status(404).json({ error: 'Webhook not found' }); @@ -297,7 +298,7 @@ router.get( async (req, res) => { try { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); const webhookId = req.params.id; const exists = await db('webhooks').where({ id: webhookId }).first(); diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index f028e862..6eb4b704 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -3,15 +3,6 @@ const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); const { body, validationResult } = require('express-validator'); -/** - * express-validator's errors.array() carries `value` -- the submitted input -- - * so returning it verbatim reflects the caller's password back in the 400 body. - * Five routes in this file validate a password field, and the strength endpoint - * is unauthenticated behind a 50mb JSON limit, which also made the rejection - * itself an allocation amplifier. Everything except `value` is kept, so the - * response shape both frontend consumers rely on (`msg`, `path`) is unchanged. - */ -const safeValidationErrors = (errors) => errors.array().map(({ value, ...rest }) => rest); const { db, logActivity } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { verifyRecaptcha } = require('../services/recaptcha'); @@ -26,8 +17,11 @@ const { const { endSession } = require('../middleware/sessionTimeout'); const { revokeToken } = require('../utils/tokenRevocation'); const { timingSafeEqualStr } = require('../utils/timingSafe'); +// Well-formed bcrypt hash that matches nothing; compared against when there is +// no account so the unknown-user path costs the same as a wrong password. +const DUMMY_BCRYPT_HASH = '$2b$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ01234'; const logger = require('../utils/logger'); -const { errorResponse } = require('../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../utils/routeHelpers'); const { setAdminAuthCookie, clearAdminAuthCookie, @@ -123,8 +117,10 @@ async function completeAdminLogin(req, res, admin, ipAddress, userAgent, lockout // Admin login with enhanced security router.post('/admin/login', [ - body('username').notEmpty().trim(), - body('password').notEmpty(), + // Length caps: an unbounded username reached the lockout lookup, bcrypt, + // the failed-attempt log line and login_attempts.identifier as sent. + body('username').isString().trim().notEmpty().isLength({ max: 255 }), + body('password').isString().notEmpty().isLength({ max: MAX_PASSWORD_LENGTH }), // Optional and boolean-coerced: an absent or malformed value means "no", // so a client that never sends it keeps the 24h session it always had. body('remember_me').optional().isBoolean().toBoolean() @@ -190,7 +186,13 @@ router.post('/admin/login', [ // (#798) never authenticate locally — their random hash is unusable by // design, and the explicit check keeps that true even if a hash ever // gets set through some other path. - if (!admin || admin.auth_provider === 'oidc' || !await bcrypt.compare(password, admin.password_hash)) { + // Always run one bcrypt compare so an unknown username costs the same + // ~100ms as a wrong password; short-circuiting here was a timing oracle + // for username enumeration despite the generic message. + const passwordMatches = (admin && admin.auth_provider !== 'oidc') + ? await bcrypt.compare(password, admin.password_hash) + : await bcrypt.compare(password, DUMMY_BCRYPT_HASH).then(() => false); + if (!passwordMatches) { await trackFailedAttempt(username, ipAddress, userAgent); return res.status(401).json({ error: getGenericAuthError() }); } @@ -402,8 +404,8 @@ router.post('/logout', async (req, res) => { // Gallery password verification with enhanced security router.post('/gallery/verify', [ - body('slug').notEmpty().trim(), - body('password').optional().isString() + body('slug').isString().trim().notEmpty().isLength({ max: 255 }), + body('password').optional().isString().isLength({ max: MAX_PASSWORD_LENGTH }) ], async (req, res) => { try { const errors = validationResult(req); @@ -420,7 +422,7 @@ router.post('/gallery/verify', [ if (!event) { // Perform a dummy bcrypt compare to prevent timing-based slug enumeration - await bcrypt.compare(password || '', '$2b$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ01234'); + await bcrypt.compare(password || '', DUMMY_BCRYPT_HASH); await trackFailedAttempt(`gallery:${slug}`, ipAddress, userAgent); return res.status(401).json({ error: 'Invalid gallery or password' }); } diff --git a/backend/src/routes/customer.js b/backend/src/routes/customer.js index 663a4377..185e7278 100644 --- a/backend/src/routes/customer.js +++ b/backend/src/routes/customer.js @@ -16,9 +16,9 @@ const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); const { body, param, validationResult } = require('express-validator'); const { db, logActivity } = require('../database/db'); -const { getBcryptRounds } = require('../utils/passwordValidation'); +const { getBcryptRounds, MAX_PASSWORD_LENGTH } = require('../utils/passwordValidation'); const logger = require('../utils/logger'); -const { errorResponse } = require('../utils/routeHelpers'); +const { errorResponse, safeValidationErrors } = require('../utils/routeHelpers'); const { getClientIp } = require('../utils/requestIp'); const { customerAuth } = require('../middleware/customerAuth'); const { setGalleryAuthCookies } = require('../utils/tokenUtils'); @@ -146,7 +146,7 @@ router.get('/events/:slug/access-token', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { slug } = req.params; @@ -283,7 +283,7 @@ router.put('/profile', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } // Normalise incoming values: trim strings, drop empty → null so the DB @@ -328,14 +328,14 @@ router.put('/profile', [ */ router.post('/profile/password', [ customerAuth, - body('currentPassword').isString().isLength({ min: 1 }), - body('newPassword').isString().isLength({ min: 8 }) + body('currentPassword').isString().isLength({ min: 1, max: MAX_PASSWORD_LENGTH }), + body('newPassword').isString().isLength({ min: 8, max: MAX_PASSWORD_LENGTH }) .withMessage('Password must be at least 8 characters'), ], async (req, res) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { currentPassword, newPassword } = req.body; diff --git a/backend/src/routes/customerAuth.js b/backend/src/routes/customerAuth.js index eaaefb8c..05a589a7 100644 --- a/backend/src/routes/customerAuth.js +++ b/backend/src/routes/customerAuth.js @@ -16,6 +16,9 @@ const express = require('express'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); const { body, param, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); +const { MAX_PASSWORD_LENGTH } = require('../utils/passwordValidation'); +const DUMMY_BCRYPT_HASH = '$2b$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ01234'; const { db, logActivity } = require('../database/db'); const { verifyRecaptcha } = require('../services/recaptcha'); const { @@ -64,12 +67,12 @@ const TOKEN_TTL_SECONDS = 24 * 60 * 60; // mirrors admin tokens // gallery JWTs (instant per-gallery revocation). router.post('/login', [ body('email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL).withMessage('Valid email is required'), - body('password').isString().notEmpty(), + body('password').isString().notEmpty().isLength({ max: MAX_PASSWORD_LENGTH }), ], async (req, res) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { email, password, recaptchaToken } = req.body; @@ -98,7 +101,12 @@ router.post('/login', [ const customer = await db('customer_accounts').where('email', email).first(); // Generic error to prevent user enumeration — same wording as admin login. - if (!customer || !customer.password_hash || !await bcrypt.compare(password, customer.password_hash)) { + // One bcrypt compare on every path so an unknown email is not a timing + // oracle (the dummy hash matches nothing). + const passwordMatches = customer && customer.password_hash + ? await bcrypt.compare(password, customer.password_hash) + : await bcrypt.compare(password, DUMMY_BCRYPT_HASH).then(() => false); + if (!passwordMatches) { await trackFailedAttempt(lockoutKey, ipAddress, userAgent); return res.status(401).json({ error: getGenericAuthError() }); } @@ -272,7 +280,7 @@ router.post('/accept-invite', [ // Length floor enforced again here for an early reject; the full // policy (uppercase + digit) is checked below so we can surface a // specific message rather than a generic validator error. - body('password').isString().isLength({ min: 8 }) + body('password').isString().isLength({ min: 8, max: MAX_PASSWORD_LENGTH }) .withMessage('Password must be at least 8 characters'), // Optional structured profile from the accept-invite form. Mirrors // the admin prefill shape — anything the customer types here wins @@ -295,7 +303,7 @@ router.post('/accept-invite', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { token, name, password, profile } = req.body; @@ -354,11 +362,11 @@ router.get('/password-reset/:token', [ */ router.post('/password-reset', [ body('token').isLength({ min: 64, max: 64 }).matches(/^[a-f0-9]+$/i), - body('password').isString().isLength({ min: 8 }).withMessage('Password must be at least 8 characters'), + body('password').isString().isLength({ min: 8, max: MAX_PASSWORD_LENGTH }).withMessage('Password must be at least 8 characters'), ], async (req, res) => { try { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); const policyError = validateCustomerPassword(req.body.password); if (policyError) { return res.status(400).json({ diff --git a/backend/src/routes/setup.js b/backend/src/routes/setup.js index 36f9ec24..baf77ba5 100644 --- a/backend/src/routes/setup.js +++ b/backend/src/routes/setup.js @@ -7,6 +7,8 @@ // rate-limited at the mount point in server.js (authRateLimiter). const express = require('express'); const { body, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../utils/routeHelpers'); +const { MAX_PASSWORD_LENGTH } = require('../utils/passwordValidation'); const setupService = require('../services/setupService'); const { getClientIp } = require('../utils/requestIp'); const { setAdminAuthCookie } = require('../utils/tokenUtils'); @@ -32,7 +34,7 @@ router.post('/verify-token', [ ], async (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } try { const valid = await setupService.verifySetupToken(req.body.token); @@ -52,11 +54,11 @@ router.post('/verify-token', [ router.post('/admin', [ body('token').notEmpty().withMessage('Setup token is required'), body('email').isEmail().withMessage('A valid email is required'), - body('password').notEmpty().withMessage('Password is required'), + body('password').isString().notEmpty().isLength({ max: MAX_PASSWORD_LENGTH }).withMessage('Password is required'), ], async (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } try { const { token, email, password } = req.body; diff --git a/backend/src/routes/v1/events.js b/backend/src/routes/v1/events.js index 7f1d3e63..fd4cb2a9 100644 --- a/backend/src/routes/v1/events.js +++ b/backend/src/routes/v1/events.js @@ -18,6 +18,7 @@ const crypto = require('crypto'); const multer = require('multer'); const sharp = require('sharp'); const { body, query, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('../../utils/routeHelpers'); const { db, logActivity } = require('../../database/db'); const { apiTokenAuth, requireApiScope } = require('../../middleware/apiTokenAuth'); const { requireEventOwnership, scopeEventsQuery } = require('../../middleware/ownership'); @@ -185,7 +186,7 @@ router.post( async (req, res) => { try { const errors = validationResult(req); - if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); + if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) }); const { event_name, event_type, event_date, customer_name = null, customer_email = null, customer_phone = null, @@ -1008,7 +1009,7 @@ router.get( try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const eventId = parseInt(req.params.id, 10); diff --git a/backend/src/utils/feedbackValidation.js b/backend/src/utils/feedbackValidation.js index 04800852..ba94673e 100644 --- a/backend/src/utils/feedbackValidation.js +++ b/backend/src/utils/feedbackValidation.js @@ -1,4 +1,5 @@ const { body, param, validationResult } = require('express-validator'); +const { safeValidationErrors } = require('./routeHelpers'); const validator = require('validator'); const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('./emailNormalization'); const { REACTION_EMOJIS } = require('../constants/reactions'); @@ -254,7 +255,7 @@ const checkValidation = (req, res, next) => { if (!errors.isEmpty()) { return res.status(400).json({ error: 'Validation failed', - errors: errors.array() + errors: safeValidationErrors(errors) }); } next(); diff --git a/backend/src/utils/routeHelpers.js b/backend/src/utils/routeHelpers.js index 28d7ce31..43f3b798 100644 --- a/backend/src/utils/routeHelpers.js +++ b/backend/src/utils/routeHelpers.js @@ -42,6 +42,15 @@ const handleAsync = (fn) => { * // ... rest of handler * })); */ + +/** + * express-validator's errors.array() carries `value` -- the submitted input. + * Returning it verbatim reflects whatever the caller sent (a rejected + * password, a 2mb string) back in the 400 body. Everything except `value` is + * kept, so consumers that read `msg` / `path` see no change. + */ +const safeValidationErrors = (errors) => errors.array().map(({ value, ...rest }) => rest); + const validateRequest = (req) => { const errors = validationResult(req); if (!errors.isEmpty()) { @@ -174,6 +183,7 @@ const paginatedResponse = (data, total, page, limit) => { module.exports = { handleAsync, validateRequest, + safeValidationErrors, successResponse, errorResponse, withValidation,