fix(events): apply the gallery password policy to publish (#1255)
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 <[email protected]>
This commit is contained in:
co-authored by
Paul Nothaft
parent
261e243070
commit
1d9f0b6c64
@@ -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: '[email protected]',
|
||||||
|
admin_email: '[email protected]',
|
||||||
|
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');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -30,6 +30,29 @@ const { getFrontendBaseUrl } = require('../../utils/frontendUrl');
|
|||||||
const downloadZipService = require('../../services/downloadZipService');
|
const downloadZipService = require('../../services/downloadZipService');
|
||||||
const { validateHeroImageAnchor, getEventFieldRequirements, readBooleanSetting, getDownloadProtectionDefaults, getBrandingDefaults, getCustomerNameFromPayload, getCustomerEmailFromPayload, getCustomerPhoneFromPayload, isPhoneFieldEnabled, mapEventForApi, hasCustomerContactColumns, deleteEventCascade, SLIDESHOW_TRANSITIONS, SLIDESHOW_COLORFILTERS } = require('./helpers');
|
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) => {
|
module.exports = (router) => {
|
||||||
|
|
||||||
|
|
||||||
@@ -854,6 +877,9 @@ module.exports = (router) => {
|
|||||||
// Re-hash so the stored hash matches what the email carries — even if
|
// 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
|
// the admin mistypes vs. what was set at draft creation, the gallery
|
||||||
// password the customer receives is the one that actually works.
|
// 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());
|
publishUpdates.password_hash = await bcrypt.hash(password, getBcryptRounds());
|
||||||
}
|
}
|
||||||
await db('events').where('id', id).update(publishUpdates);
|
await db('events').where('id', id).update(publishUpdates);
|
||||||
|
|||||||
Reference in New Issue
Block a user