From 6938bad107335af54dd8bfe42822219a03921b8b Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:30:17 +0200 Subject: [PATCH] fix(events): apply the gallery password policy to publish and send-later (#1253) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both routes re-hash password_hash from a plaintext the admin re-types, and both validated it with nothing but express-validator's isLength({min:6}). So the configured complexity — moderate by default, meaning 8 chars plus upper, lower and a digit — governed creation and reset while these two doors accepted 'aaaaaa' and made it the live gallery password. Fixed for both at once, deliberately. Fixing only the newer send-later route would have made a quiet-publish password valid at publish time and rejected by send-later, leaving the admin unable to mail a gallery that is already live under exactly that password. Not an escalation — it needs admin auth plus events.edit, and such an admin could already set the same weak password through /publish. It is a policy gap: the UI promised a complexity level these two endpoints 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) instead of silently weakening the gallery. Two existing test fixtures had to change for the same reason — their intent was that the supplied password is carried and persisted, not that a weak one is accepted. Co-authored-by: Paul Nothaft --- .../integration/publishQuietly.test.js | 58 +++++++++++++++++-- backend/src/routes/adminEvents/crud.js | 34 +++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/backend/__tests__/integration/publishQuietly.test.js b/backend/__tests__/integration/publishQuietly.test.js index 8f2851e6..cf10023a 100644 --- a/backend/__tests__/integration/publishQuietly.test.js +++ b/backend/__tests__/integration/publishQuietly.test.js @@ -225,11 +225,11 @@ describe('publish quietly (#1235)', () => { const res = await request(app) .post(`/admin/events/${id}/send-gallery-email`) - .send({ password: 'sup3r-secret' }); + .send({ password: 'Sup3r-Secret' }); expect(res.status).toBe(200); const [queued] = await queuedFor(id); - expect(JSON.parse(queued.email_data).gallery_password).toBe('sup3r-secret'); + expect(JSON.parse(queued.email_data).gallery_password).toBe('Sup3r-Secret'); }); it('persists a changed password so the emailed one actually works', async () => { @@ -241,16 +241,16 @@ describe('publish quietly (#1235)', () => { const res = await request(app) .post(`/admin/events/${id}/send-gallery-email`) - .send({ password: 'brand-new-pass' }); + .send({ password: 'Brand-New-Pass1' }); expect(res.status).toBe(200); const bcrypt = require('bcrypt'); const row = await db('events').where({ id }).first(); expect(row.password_hash).not.toBe('stale-hash'); - expect(await bcrypt.compare('brand-new-pass', row.password_hash)).toBe(true); + expect(await bcrypt.compare('Brand-New-Pass1', row.password_hash)).toBe(true); const [queued] = await queuedFor(id); - expect(JSON.parse(queued.email_data).gallery_password).toBe('brand-new-pass'); + expect(JSON.parse(queued.email_data).gallery_password).toBe('Brand-New-Pass1'); }); it('does NOT touch the gallery password when only an account notice goes out', async () => { @@ -336,6 +336,54 @@ describe('publish quietly (#1235)', () => { expect(res.body.error).toMatch(/no customer email/i); }); + it('applies the configured gallery policy before rehashing, on both doors', async () => { + // Both endpoints re-hash a plaintext the admin re-types, and both used to + // validate it with nothing but isLength({min:6}) — so the configured + // complexity governed creation and reset while these two accepted + // 'aaaaaa' and made it the live gallery password. + const draftId = await seedDraft({ slug: 'weak-publish' }); + await db('events').where({ id: draftId }).update({ require_password: 1 }); + + const publishRes = await request(app) + .post(`/admin/events/${draftId}/publish`) + .send({ password: 'aaaaaa' }); + + expect(publishRes.status).toBe(400); + expect(publishRes.body.error).toMatch(/security requirements/i); + + // And the same password must not sneak in through send-later, or a + // gallery published quietly could still be weakened afterwards. + const [row] = await db('events').insert({ + slug: 'weak-send', + event_type: 'wedding', + event_name: 'Weak Send', + event_date: '2026-09-01', + host_email: '', + admin_email: 'admin@example.com', + customer_email: 'client@example.com', + password_hash: 'original-hash', + require_password: 1, + share_link: '/gallery/weak-send/share', + share_token: 'weak-send-token', + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 0, + created_at: new Date().toISOString(), + }).returning('id'); + const sendId = typeof row === 'object' ? row.id : row; + + const sendRes = await request(app) + .post(`/admin/events/${sendId}/send-gallery-email`) + .send({ password: 'aaaaaa' }); + + expect(sendRes.status).toBe(400); + expect(sendRes.body.error).toMatch(/security requirements/i); + // Rejected means untouched — not rejected after the write. + const after = await db('events').where({ id: sendId }).first(); + expect(after.password_hash).toBe('original-hash'); + }); + it('re-sending is allowed — a lost email should not need an unpublish/republish', async () => { const id = await seedDraft({ slug: 'resend' }); await request(app).post(`/admin/events/${id}/publish`).send({}); diff --git a/backend/src/routes/adminEvents/crud.js b/backend/src/routes/adminEvents/crud.js index 720370c6..6186ea8a 100644 --- a/backend/src/routes/adminEvents/crud.js +++ b/backend/src/routes/adminEvents/crud.js @@ -31,6 +31,34 @@ const downloadZipService = require('../../services/downloadZipService'); const { resolveEventFeedbackDefaults, applyFeedbackDefaults, KEYBIND_MODES } = require('../../services/feedbackDefaults'); 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. + * + * Both the publish dialog (#627) and the send-later route (#1235) re-hash + * `password_hash` from a plaintext the admin types again, and both validated + * it with nothing but express-validator's `isLength({ min: 6 })`. So the + * configured complexity — moderate by default — governed creation and reset + * while these two doors accepted `aaaaaa` and made it the live password. + * + * Fixed in one place and for both, deliberately. Fixing only the newer route + * would have made a quiet-publish password valid at publish time and rejected + * by send-later, leaving the admin unable to mail a gallery that is already + * live under that exact 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, + }; +} + /** * Can this assigned customer account actually receive — and act on — the * gallery notice? (#1235) @@ -1036,6 +1064,9 @@ module.exports = (router) => { // the live gallery password and lock out everyone holding the old one, // in exchange for nothing. if (hasInlineRecipient && requirePassword && password) { + const policyError = await checkGalleryPasswordPolicy(password, event.event_name); + if (policyError) return res.status(400).json(policyError); + await db('events').where('id', id).update({ password_hash: await bcrypt.hash(password, getBcryptRounds()), }); @@ -1132,6 +1163,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);