From 1d9f0b6c6491cface22f12651131fd5dfba330f0 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:36:41 +0200 Subject: [PATCH] fix(events): apply the gallery password policy to publish (#1255) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stable twin of #1253. This branch has only the publish door — send-gallery-email is #1235, main-only — so the same gap exists here in one place rather than two. /publish re-hashes password_hash from a plaintext the admin re-types in the publish dialog, validated with nothing but express-validator's isLength({min:6}). So the configured complexity — moderate by default, meaning 8 characters plus upper, lower and a digit — governed event creation and password reset while this door accepted 'aaaaaa' and made it the live gallery password. Not an escalation: it needs admin auth plus events.edit. It is a policy gap, the admin UI advertising a complexity level this write path did not enforce. BEHAVIOUR CHANGE: an API-only consumer publishing with a sub-policy password now gets 400, with the same body shape event creation returns (error, details, score, feedback). 3 tests, including that the rejection happens BEFORE the write — the gallery keeps its old hash and stays a draft — and that a publish carrying no password at all is untouched. Co-authored-by: Paul Nothaft --- .../integration/publishPasswordPolicy.test.js | 114 ++++++++++++++++++ backend/src/routes/adminEvents/crud.js | 26 ++++ 2 files changed, 140 insertions(+) create mode 100644 backend/__tests__/integration/publishPasswordPolicy.test.js diff --git a/backend/__tests__/integration/publishPasswordPolicy.test.js b/backend/__tests__/integration/publishPasswordPolicy.test.js new file mode 100644 index 00000000..5281d541 --- /dev/null +++ b/backend/__tests__/integration/publishPasswordPolicy.test.js @@ -0,0 +1,114 @@ +/** + * Publishing must not be a way around the configured gallery password policy. + * + * `POST /:id/publish` (#627) re-hashes `password_hash` from a plaintext the + * admin re-types in the publish dialog, and validated it with nothing but + * express-validator's `isLength({ min: 6 })`. So the configured complexity — + * moderate by default, meaning 8 characters plus upper, lower and a digit — + * governed event creation and password reset, while this door accepted + * `aaaaaa` and made it the live gallery password. + * + * Not an escalation: it needs admin auth plus events.edit, and such an admin + * could already set a weak password elsewhere. It is a policy gap — the admin + * UI advertises a complexity level this write path did not enforce. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-pubpolicy-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'publish-policy-test-secret'; +process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-pubpolicy-storage-')); + +const request = require('supertest'); +const { + bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken, buildRouteApp, +} = require('./helpers/crmDb'); + +describe('publish enforces the gallery password policy', () => { + let db; let cleanup; let app; let token; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + const { adminId } = await seedMinimal(db); + await assignAdminRole(db, adminId, 'admin'); + token = mintAdminToken(adminId); + app = buildRouteApp('/admin/events', require('../../src/routes/adminEvents')); + }, 120000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + async function seedDraft(slug) { + const [row] = await db('events').insert({ + slug, + event_type: 'wedding', + event_name: `Event ${slug}`, + event_date: '2026-09-01', + host_email: 'client@example.com', + admin_email: 'admin@example.com', + password_hash: 'original-hash', + require_password: 1, + share_link: `/gallery/${slug}/share`, + share_token: `${slug}-token`, + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 1, + created_at: new Date().toISOString(), + }).returning('id'); + return typeof row === 'object' ? row.id : row; + } + + it('refuses a password that misses the configured complexity', async () => { + const id = await seedDraft('weak-publish'); + + const res = await request(app) + .post(`/admin/events/${id}/publish`) + .set('Authorization', `Bearer ${token}`) + .send({ password: 'aaaaaa' }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/security requirements/i); + + // Rejected BEFORE the write, not after — the gallery must be untouched, + // and still a draft. + const after = await db('events').where({ id }).first(); + expect(after.password_hash).toBe('original-hash'); + expect(after.is_draft === 1 || after.is_draft === true).toBe(true); + }); + + it('still accepts a password that meets it', async () => { + const id = await seedDraft('strong-publish'); + + const res = await request(app) + .post(`/admin/events/${id}/publish`) + .set('Authorization', `Bearer ${token}`) + .send({ password: 'Sup3r-Secret' }); + + expect(res.status).toBe(200); + + const bcrypt = require('bcrypt'); + const after = await db('events').where({ id }).first(); + expect(after.password_hash).not.toBe('original-hash'); + expect(await bcrypt.compare('Sup3r-Secret', after.password_hash)).toBe(true); + }); + + it('leaves a publish without a password alone', async () => { + // The legacy sentinel path: no password in the body means no rehash, so + // the policy has nothing to check and must not block the publish. + const id = await seedDraft('no-password-publish'); + + const res = await request(app) + .post(`/admin/events/${id}/publish`) + .set('Authorization', `Bearer ${token}`) + .send({}); + + expect(res.status).toBe(200); + const after = await db('events').where({ id }).first(); + expect(after.password_hash).toBe('original-hash'); + }); +}); diff --git a/backend/src/routes/adminEvents/crud.js b/backend/src/routes/adminEvents/crud.js index 6d5df4c2..785a909c 100644 --- a/backend/src/routes/adminEvents/crud.js +++ b/backend/src/routes/adminEvents/crud.js @@ -30,6 +30,29 @@ const { getFrontendBaseUrl } = require('../../utils/frontendUrl'); const downloadZipService = require('../../services/downloadZipService'); const { validateHeroImageAnchor, getEventFieldRequirements, readBooleanSetting, getDownloadProtectionDefaults, getBrandingDefaults, getCustomerNameFromPayload, getCustomerEmailFromPayload, getCustomerPhoneFromPayload, isPhoneFieldEnabled, mapEventForApi, hasCustomerContactColumns, deleteEventCascade, SLIDESHOW_TRANSITIONS, SLIDESHOW_COLORFILTERS } = require('./helpers'); +/** + * Validate a gallery password the admin re-typed, against the SAME policy + * event creation applies. + * + * The publish dialog (#627) re-hashes `password_hash` from a plaintext the + * admin types again, and validated it with nothing but express-validator's + * `isLength({ min: 6 })`. So the configured complexity — moderate by default + * — governed creation and reset while this door accepted `aaaaaa` and made it + * the live gallery password. + * + * Returns null when the password passes; otherwise the response body to send. + */ +async function checkGalleryPasswordPolicy(password, eventName) { + const result = await validatePasswordInContext(password, 'gallery', { eventName }); + if (result.valid) return null; + return { + error: 'Password does not meet security requirements', + details: result.errors, + score: result.score, + feedback: result.feedback, + }; +} + module.exports = (router) => { @@ -854,6 +877,9 @@ module.exports = (router) => { // Re-hash so the stored hash matches what the email carries — even if // the admin mistypes vs. what was set at draft creation, the gallery // password the customer receives is the one that actually works. + const policyError = await checkGalleryPasswordPolicy(password, event.event_name); + if (policyError) return res.status(400).json(policyError); + publishUpdates.password_hash = await bcrypt.hash(password, getBcryptRounds()); } await db('events').where('id', id).update(publishUpdates);