fix(security): apply image-security defaults on every creation path
Review follow-ups on the #1296 fix. The defaults were resolved only in the admin POST / handler. POST /api/v1/events builds its own insert and resolved just the devtools setting, so an API-created gallery still fell back to the column defaults — the same split that made #592 a separate bug from #317, about to be repeated. Both paths now share resolveImageSecurityColumns(). An explicitly supplied value now wins over the global default. The create routes never accepted these four fields at all, though PUT /:id has validated them all along, so a client sending protection_level on create had it silently dropped. The previous comment claimed the spread ordering preserved a request value; there was no request value to preserve, and a later spread would have overridden one anyway. Settings validation no longer leans on parseInt, which rescues '72oops', 72.5 and [72] into valid-looking integers. The settings PUT stores whatever JSON it is handed without validating values, so those really can reach the resolver. fragmentation_level is still stored and consumed by no renderer — ProtectedImage hardcodes a 4-grid and secureImageService a 3x3. Noted in the API docs rather than silently implied to work. Refs #1296
This commit is contained in:
@@ -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'),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user