diff --git a/backend/__tests__/integration/imageSecurityDefaults.test.js b/backend/__tests__/integration/imageSecurityDefaults.test.js index 32b61ded..d64b0e43 100644 --- a/backend/__tests__/integration/imageSecurityDefaults.test.js +++ b/backend/__tests__/integration/imageSecurityDefaults.test.js @@ -23,10 +23,12 @@ describe('image-security creation defaults', () => { let db; let cleanup; let getImageSecurityDefaults; + let resolveImageSecurityColumns; beforeAll(async () => { ({ db, cleanup } = await bootCrmDb()); - ({ getImageSecurityDefaults } = require('../../src/routes/adminEvents/helpers')); + ({ getImageSecurityDefaults, resolveImageSecurityColumns } = + require('../../src/routes/adminEvents/helpers')); }, 120000); afterAll(async () => { @@ -82,6 +84,14 @@ describe('image-security creation defaults', () => { ['a non-numeric quality', 'default_image_quality', 'high'], ['fragmentation above the range', 'default_fragmentation_level', 99], ['a non-boolean canvas value', 'enable_canvas_rendering', 'yes'], + // parseInt would have rescued each of these into a valid-looking + // integer. The settings PUT stores values without validating them, so + // they can genuinely be in the table. + ['a numeric prefix with trailing junk', 'default_image_quality', '72oops'], + ['a fractional quality', 'default_image_quality', 72.5], + ['a single-element array', 'default_image_quality', [72]], + ['a fractional fragmentation level', 'default_fragmentation_level', 3.7], + ['a fragmentation level with trailing junk', 'default_fragmentation_level', '3x'], ])('ignores %s and falls through to the column default', async (_label, key, value) => { await setSetting(key, value); expect(await getImageSecurityDefaults()).toEqual({}); @@ -96,4 +106,53 @@ describe('image-security creation defaults', () => { await setSetting('default_image_quality', { nonsense: true }); await expect(getImageSecurityDefaults()).resolves.toEqual({}); }); + + describe('resolveImageSecurityColumns', () => { + it('omits every column when neither the request nor the settings supply one', () => { + expect(resolveImageSecurityColumns({}, {})).toEqual({}); + }); + + it('uses the global default when the request says nothing', () => { + expect(resolveImageSecurityColumns({}, { protection_level: 'maximum' })) + .toEqual({ protection_level: 'maximum' }); + }); + + it('lets an explicit request value win over the global default', () => { + expect(resolveImageSecurityColumns( + { protection_level: 'basic' }, + { protection_level: 'maximum' }, + )).toEqual({ protection_level: 'basic' }); + }); + + it('keeps an explicit false canvas value instead of reading it as absent', () => { + const columns = resolveImageSecurityColumns( + { use_canvas_rendering: false }, + { use_canvas_rendering: true }, + ); + expect(columns.use_canvas_rendering).toBeFalsy(); + }); + + it('keeps a zero-ish explicit value rather than falling through', () => { + // 0 is out of range for the column, but the guard is `!== undefined`, + // not truthiness — the validator is what rejects out-of-range input. + expect(resolveImageSecurityColumns({ image_quality: 0 }, { image_quality: 85 })) + .toEqual({ image_quality: 0 }); + }); + + it('resolves each column independently', () => { + expect(resolveImageSecurityColumns( + { image_quality: 60 }, + { protection_level: 'enhanced', fragmentation_level: 4 }, + )).toEqual({ + protection_level: 'enhanced', + image_quality: 60, + fragmentation_level: 4, + }); + }); + + it('tolerates a missing body, which is what an empty API request looks like', () => { + expect(resolveImageSecurityColumns(undefined, { image_quality: 90 })) + .toEqual({ image_quality: 90 }); + }); + }); }); diff --git a/backend/src/routes/adminEvents/crud.js b/backend/src/routes/adminEvents/crud.js index d6a0d500..f91640b5 100644 --- a/backend/src/routes/adminEvents/crud.js +++ b/backend/src/routes/adminEvents/crud.js @@ -31,7 +31,7 @@ const { getFrontendBaseUrl, getAbsoluteFrontendUrl } = require('../../utils/fron const downloadZipService = require('../../services/downloadZipService'); const { resolveEventFeedbackDefaults, applyFeedbackDefaults, KEYBIND_MODES } = require('../../services/feedbackDefaults'); const { validateHeroImageAnchor, getEventFieldRequirements, readBooleanSetting, getDownloadProtectionDefaults, - getImageSecurityDefaults, getBrandingDefaults, getCustomerNameFromPayload, getCustomerEmailFromPayload, getCustomerPhoneFromPayload, isPhoneFieldEnabled, mapEventForApi, hasCustomerContactColumns, deleteEventCascade, SLIDESHOW_TRANSITIONS, SLIDESHOW_COLORFILTERS } = require('./helpers'); + getImageSecurityDefaults, resolveImageSecurityColumns, getBrandingDefaults, getCustomerNameFromPayload, getCustomerEmailFromPayload, getCustomerPhoneFromPayload, isPhoneFieldEnabled, mapEventForApi, hasCustomerContactColumns, deleteEventCascade, SLIDESHOW_TRANSITIONS, SLIDESHOW_COLORFILTERS } = require('./helpers'); /** * `events.slug` is UNIQUE, and both routes that mint one do a read-then-insert @@ -229,6 +229,13 @@ module.exports = (router) => { body('allow_downloads').optional().isBoolean(), body('disable_right_click').optional().isBoolean(), body('enable_devtools_protection').optional().isBoolean(), + // Image security. PUT /:id has validated these all along; create + // accepted none of them, so a value sent here used to be dropped on the + // floor and the column default applied instead (#1296). + body('protection_level').optional().isIn(['basic', 'standard', 'enhanced', 'maximum']), + body('use_canvas_rendering').optional().isBoolean().toBoolean(), + body('image_quality').optional().isInt({ min: 1, max: 100 }).toInt(), + body('fragmentation_level').optional().isInt({ min: 1, max: 10 }).toInt(), body('watermark_downloads').optional().isBoolean(), body('watermark_text').optional().trim(), // #328 follow-up: per-event opt-in for presigned-URL "Download All". @@ -546,9 +553,13 @@ module.exports = (router) => { // but new events still got it ON because the column default is true). const protectionDefaults = await getDownloadProtectionDefaults(); // #1296 — the other four Image-security settings, which were written, - // rendered as controls, and read by nothing. Creation-time only; see + // rendered as controls, and read by nothing. Same inheritance rule as + // the devtools setting below. Creation-time only; see // getImageSecurityDefaults for why existing events are left alone. - const imageSecurityDefaults = await getImageSecurityDefaults(); + const imageSecurityColumns = resolveImageSecurityColumns( + req.body, + await getImageSecurityDefaults(), + ); const effectiveEnableDevtoolsProtection = enableDevtoolsProtectionInput !== undefined ? enableDevtoolsProtectionInput @@ -622,18 +633,9 @@ module.exports = (router) => { allow_downloads: formatBoolean(allow_downloads !== undefined ? allow_downloads : true), disable_right_click: formatBoolean(disable_right_click !== undefined ? disable_right_click : false), enable_devtools_protection: formatBoolean(effectiveEnableDevtoolsProtection), - // Spread AFTER the explicit columns so a value the request supplied - // still wins; each key is present only when the global setting held - // a usable value, so anything unset falls through to the column - // default exactly as before (#1296). - ...(imageSecurityDefaults.protection_level !== undefined - ? { protection_level: imageSecurityDefaults.protection_level } : {}), - ...(imageSecurityDefaults.image_quality !== undefined - ? { image_quality: imageSecurityDefaults.image_quality } : {}), - ...(imageSecurityDefaults.use_canvas_rendering !== undefined - ? { use_canvas_rendering: formatBoolean(imageSecurityDefaults.use_canvas_rendering) } : {}), - ...(imageSecurityDefaults.fragmentation_level !== undefined - ? { fragmentation_level: imageSecurityDefaults.fragmentation_level } : {}), + // Request value, else the global default, else the column default — + // a key absent here is one the database fills in (#1296). + ...imageSecurityColumns, watermark_downloads: formatBoolean(watermark_downloads !== undefined ? watermark_downloads : false), watermark_text, allow_presigned_download: formatBoolean(allow_presigned_download === true || allow_presigned_download === 'true'), diff --git a/backend/src/routes/adminEvents/helpers.js b/backend/src/routes/adminEvents/helpers.js index fd271bee..7f62ecee 100644 --- a/backend/src/routes/adminEvents/helpers.js +++ b/backend/src/routes/adminEvents/helpers.js @@ -122,6 +122,19 @@ const getDownloadProtectionDefaults = async () => { */ const PROTECTION_LEVELS = ['basic', 'standard', 'enhanced', 'maximum']; +// parseInt would rescue malformed settings instead of rejecting them: +// parseInt('72oops') is 72, parseInt(72.5) is 72, parseInt([72]) is 72. +// That matters because the settings PUT stores whatever JSON it is handed +// without validating the value (adminImageSecurity.js writes +// JSON.stringify(value) for any allow-listed key), so those shapes really +// can be sitting in app_settings. Accept only a genuine integer, or a +// string that is exactly one. +const toInteger = (value) => { + if (typeof value === 'number') return Number.isInteger(value) ? value : undefined; + if (typeof value === 'string' && /^[+-]?\d+$/.test(value.trim())) return Number(value.trim()); + return undefined; +}; + const getImageSecurityDefaults = async () => { const defaults = {}; try { @@ -152,8 +165,8 @@ const getImageSecurityDefaults = async () => { // The column is an integer percentage; anything outside 1..100 is a // misconfiguration and falls through rather than being clamped into // something the operator did not choose. - const quality = parseInt(read('default_image_quality'), 10); - if (Number.isInteger(quality) && quality >= 1 && quality <= 100) { + const quality = toInteger(read('default_image_quality')); + if (quality !== undefined && quality >= 1 && quality <= 100) { defaults.image_quality = quality; } @@ -162,8 +175,8 @@ const getImageSecurityDefaults = async () => { defaults.use_canvas_rendering = canvas; } - const fragmentation = parseInt(read('default_fragmentation_level'), 10); - if (Number.isInteger(fragmentation) && fragmentation >= 1 && fragmentation <= 10) { + const fragmentation = toInteger(read('default_fragmentation_level')); + if (fragmentation !== undefined && fragmentation >= 1 && fragmentation <= 10) { defaults.fragmentation_level = fragmentation; } } catch (error) { @@ -174,6 +187,39 @@ const getImageSecurityDefaults = async () => { return defaults; }; +/** + * Build the image-security columns for a NEW event: an explicit request + * value wins, then the global default, then the column default (the key is + * omitted entirely so the database supplies it). + * + * Shared by the admin create route and POST /api/v1/events so the configured + * security level cannot depend on which entry point created the gallery — + * the same split that made #592 (devtools) a separate bug from #317. + * + * `body` values are already validated by the route's express-validator + * chain; `defaults` come from getImageSecurityDefaults(), which validates + * them itself. + */ +const resolveImageSecurityColumns = (body = {}, defaults = {}) => { + const { formatBoolean } = require('../../utils/dbCompat'); + const columns = {}; + const pick = (key) => (body[key] !== undefined ? body[key] : defaults[key]); + + const level = pick('protection_level'); + if (level !== undefined) columns.protection_level = level; + + const quality = pick('image_quality'); + if (quality !== undefined) columns.image_quality = quality; + + const canvas = pick('use_canvas_rendering'); + if (canvas !== undefined) columns.use_canvas_rendering = formatBoolean(canvas); + + const fragmentation = pick('fragmentation_level'); + if (fragmentation !== undefined) columns.fragmentation_level = fragmentation; + + return columns; +}; + // Helper to get branding defaults for new events (Feature 7: Branding Inheritance). // // Note: `branding_logo_position` (header bar — left/center/right) is a @@ -608,6 +654,7 @@ module.exports = { readBooleanSetting, getDownloadProtectionDefaults, getImageSecurityDefaults, + resolveImageSecurityColumns, getBrandingDefaults, getCustomerNameFromPayload, getCustomerEmailFromPayload, diff --git a/backend/src/routes/v1/__tests__/events.create.test.js b/backend/src/routes/v1/__tests__/events.create.test.js index 1e973a86..92b1ad8e 100644 --- a/backend/src/routes/v1/__tests__/events.create.test.js +++ b/backend/src/routes/v1/__tests__/events.create.test.js @@ -126,6 +126,7 @@ const BASE_BODY = { const baseSettingsChains = () => [ buildChain({ firstResult: null }), // feedback default buildChain({ firstResult: null }), // devtools default + buildChain({ selectResult: [] }), // image-security whereIn → empty rows (#1296) buildChain({ selectResult: [] }), // branding whereIn → empty rows ]; @@ -169,19 +170,21 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { it('creates event_feedback_settings row when feedback_enabled=true is sent', async () => { // feedback_enabled provided → feedback probe SKIPPED. Sequence: // 1. devtools probe - // 2. branding probe (whereIn → select) - // 3. slug probe - // 4. events insert - // 5. feedback sub-toggle defaults probe (whereIn → select, #1044) - // 6. event_feedback_settings insert + // 2. image-security probe (whereIn → select, #1296) + // 3. branding probe (whereIn → select) + // 4. slug probe + // 5. events insert + // 6. feedback sub-toggle defaults probe (whereIn → select, #1044) + // 7. event_feedback_settings insert const devtoolsChain = buildChain({ firstResult: null }); + const imageSecurityChain = buildChain({ selectResult: [] }); const brandingChain = buildChain({ selectResult: [] }); const slugChain = buildChain({ firstResult: null }); const insertChain = buildChain({ returningResult: [{ id: 50 }] }); const feedbackDefaultsChain = buildChain({ selectResult: [] }); const feedbackInsertChain = buildChain(); db.__setImplementations( - devtoolsChain, brandingChain, slugChain, insertChain, + devtoolsChain, imageSecurityChain, brandingChain, slugChain, insertChain, feedbackDefaultsChain, feedbackInsertChain, ); @@ -190,7 +193,7 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { .send({ ...BASE_BODY, feedback_enabled: true }) .expect(201); - expect(db).toHaveBeenNthCalledWith(6, 'event_feedback_settings'); + expect(db).toHaveBeenNthCalledWith(7, 'event_feedback_settings'); const feedbackRow = feedbackInsertChain.insert.mock.calls[0][0]; expect(feedbackRow).toMatchObject({ event_id: 50 }); @@ -216,20 +219,21 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { it('honours the event_default_feedback_enabled global when body omits feedback_enabled', async () => { // Feedback probe returns serialized "true" → fallback kicks in and // the feedback insert runs. Sequence: feedback probe, devtools probe, - // branding probe, slug, insert, sub-toggle defaults probe (#1044), - // feedback insert (7 calls total). + // image-security probe (#1296), branding probe, slug, insert, sub-toggle + // defaults probe (#1044), feedback insert (8 calls total). const feedbackProbe = buildChain({ firstResult: { setting_key: 'event_default_feedback_enabled', setting_value: 'true' }, }); const devtoolsChain = buildChain({ firstResult: null }); + const imageSecurityChain = buildChain({ selectResult: [] }); const brandingChain = buildChain({ selectResult: [] }); const slugChain = buildChain({ firstResult: null }); const insertChain = buildChain({ returningResult: [{ id: 51 }] }); const feedbackDefaultsChain = buildChain({ selectResult: [] }); const feedbackInsertChain = buildChain(); db.__setImplementations( - feedbackProbe, devtoolsChain, brandingChain, slugChain, insertChain, - feedbackDefaultsChain, feedbackInsertChain, + feedbackProbe, devtoolsChain, imageSecurityChain, brandingChain, slugChain, + insertChain, feedbackDefaultsChain, feedbackInsertChain, ); await request(buildApp()) @@ -237,7 +241,7 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { .send(BASE_BODY) .expect(201); - expect(db).toHaveBeenNthCalledWith(7, 'event_feedback_settings'); + expect(db).toHaveBeenNthCalledWith(8, 'event_feedback_settings'); expect(feedbackInsertChain.insert).toHaveBeenCalledTimes(1); }); @@ -251,9 +255,9 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { .send(BASE_BODY) .expect(201); - // 5 db() calls: feedback + devtools + branding probes, slug, insert. - // event_feedback_settings is never touched. - expect(db).toHaveBeenCalledTimes(5); + // 6 db() calls: feedback + devtools + image-security + branding probes, + // slug, insert. event_feedback_settings is never touched. + expect(db).toHaveBeenCalledTimes(6); expect(db).not.toHaveBeenCalledWith('event_feedback_settings'); }); diff --git a/backend/src/routes/v1/events.js b/backend/src/routes/v1/events.js index fd4cb2a9..54990a6d 100644 --- a/backend/src/routes/v1/events.js +++ b/backend/src/routes/v1/events.js @@ -37,6 +37,7 @@ const logger = require('../../utils/logger'); const { slugify } = require('../../utils/slug'); const { formatBoolean } = require('../../utils/dbCompat'); const { parseBooleanInput } = require('../../utils/parsers'); +const { getImageSecurityDefaults, resolveImageSecurityColumns } = require('../adminEvents/helpers'); const { isValidEventType } = require('../../services/eventTypeService'); const { replacePhoto } = require('../../services/photoReplacementService'); const { getMaxFileSizeBytes, DEFAULT_MAX_FILE_SIZE_MB } = require('../../services/uploadSettings'); @@ -134,6 +135,10 @@ const photoUpload = async (req, res, next) => { * color_theme: { type: string, nullable: true, description: "Preset name (e.g. 'default') or JSON-encoded ThemeConfig. Persisted as-is on the event row." } * feedback_enabled: { type: boolean, nullable: true, description: "Enable guest feedback for this gallery. When omitted, falls back to the global event_default_feedback_enabled setting." } * enable_devtools_protection: { type: boolean, nullable: true, description: "Block right-click / devtools shortcuts in the gallery. When omitted, falls back to the global enable_devtools_protection setting." } + * protection_level: { type: string, nullable: true, enum: [basic, standard, enhanced, maximum], description: "Image protection level. When omitted, falls back to the global default_protection_level setting." } + * use_canvas_rendering: { type: boolean, nullable: true, description: "Render gallery images to a canvas instead of an img tag. When omitted, falls back to the global enable_canvas_rendering setting." } + * image_quality: { type: integer, minimum: 1, maximum: 100, nullable: true, description: "Served image quality percentage. When omitted, falls back to the global default_image_quality setting." } + * fragmentation_level: { type: integer, minimum: 1, maximum: 10, nullable: true, description: "Stored for future use; no renderer consumes it yet. When omitted, falls back to the global default_fragmentation_level setting." } * hero_logo_visible: { type: boolean, nullable: true, description: "Show event logo in the hero block. When omitted, falls back to the global branding_logo_display_hero setting." } * hero_logo_size: { type: string, nullable: true, enum: [small, medium, large, xlarge], description: "Hero logo size. When omitted, falls back to the global branding_logo_size setting." } * hero_logo_position: { type: string, nullable: true, enum: [top, center, bottom], description: "Hero logo position. Defaults to 'top' (not settings-backed — see migration 084)." } @@ -179,6 +184,10 @@ router.post( body('color_theme').optional({ nullable: true }).isString().trim(), body('feedback_enabled').optional().isBoolean(), body('enable_devtools_protection').optional().isBoolean(), + body('protection_level').optional().isIn(['basic', 'standard', 'enhanced', 'maximum']), + body('use_canvas_rendering').optional().isBoolean().toBoolean(), + body('image_quality').optional().isInt({ min: 1, max: 100 }).toInt(), + body('fragmentation_level').optional().isInt({ min: 1, max: 10 }).toInt(), body('hero_logo_visible').optional().isBoolean(), body('hero_logo_size').optional().isIn(['small', 'medium', 'large', 'xlarge']), body('hero_logo_position').optional().isIn(['top', 'center', 'bottom']) @@ -235,6 +244,15 @@ router.post( } const enable_devtools_protection = parseBooleanInput(devtoolsInput, devtoolsFallback); + // #1296 — same shape again, for the four Image Security settings that + // were stored and applied nowhere. Shared with the admin create route + // so a gallery's security level does not depend on which endpoint made + // it; #592 above is the bug this would otherwise repeat. + const imageSecurityColumns = resolveImageSecurityColumns( + req.body, + await getImageSecurityDefaults(), + ); + // Same shape as the feedback / devtools fallbacks: honour the global // event_default_require_password toggle (#317). Without this an admin // who disabled "require password by default" globally still got @@ -324,6 +342,8 @@ router.post( // Issue #592 — write the resolved devtools setting (input value // or global fallback) so the column default doesn't shadow it. enable_devtools_protection: formatBoolean(enable_devtools_protection), + // Request value, else the global default, else the column default. + ...imageSecurityColumns, // Branding inheritance — resolved value from body or app_settings. hero_logo_visible: formatBoolean(hero_logo_visible), hero_logo_size,