diff --git a/backend/__tests__/routes/galleryDraftPreviewResolve.test.js b/backend/__tests__/routes/galleryDraftPreviewResolve.test.js new file mode 100644 index 00000000..2803c3e6 --- /dev/null +++ b/backend/__tests__/routes/galleryDraftPreviewResolve.test.js @@ -0,0 +1,268 @@ +/** + * Previewing an unpublished gallery through its SHORT share URL (#1386). + * + * /info has honoured admin_preview since #868, but two sibling routes never + * did, and both sit on the short-URL path: + * + * GET /resolve/:identifier — filtered drafts out via ACTIVE_EVENT_FILTER + * GET /:slug/verify-token/:token — same, inline + * + * With "use short gallery URLs" OFF the admin's View Gallery link carries the + * slug, GalleryPage never calls /resolve, and the preview worked. With it ON + * the link is the token form, GalleryPage resolves it first, and the draft + * 404'd as "Gallery Not Found" — which is exactly what was reported. + * + * The relaxation is admin-preview-only, so the other half of these tests is + * the part that must NOT move: anonymous callers still get 404 for a draft, + * and GHSA-rh8r's rule (never hand a share_token back on a bare slug lookup) + * has to survive the new path too. + */ +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-draft-preview-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'draft-preview-test-secret'; +process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-draft-preview-storage-')); + +const request = require('supertest'); +const express = require('express'); +const cookieParser = require('cookie-parser'); +const { bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken } = require('../integration/helpers/crmDb'); + +// Share-token fixtures, deliberately low-entropy and obviously fake. They +// have to satisfy SHARE_TOKEN_REGEX (32 hex chars), and random-looking hex of +// that shape is exactly what secret scanners flag — GitGuardian raised two +// "Generic High Entropy Secret" findings on the first version of this file. +const DRAFT_SLUG = 'draft-preview-event'; +const DRAFT_TOKEN = 'deadbeefdeadbeefdeadbeefdeadbeef'; +const LIVE_SLUG = 'published-event'; +const LIVE_TOKEN = 'feedfacefeedfacefeedfacefeedface'; + +describe('draft preview through the short share URL (#1386)', () => { + let db; let cleanup; let app; let adminId; let foreignId; + + // Two transports. admin_preview=1 is an intent flag authenticated by the + // admin cookie — what the frontend sends. ?preview= is the legacy + // hand-built-link form, kept working. + const preview = (id = adminId) => `preview=${mintAdminToken(id)}`; + const asAdmin = (req, id = adminId) => req.set('Cookie', `admin_token=${mintAdminToken(id)}`); + + async function insertEvent({ slug, token, isDraft }) { + await db('events').insert({ + slug, + event_type: 'wedding', + event_name: slug, + event_date: '2026-09-01', + host_email: 'h@example.com', + admin_email: 'a@example.com', + password_hash: 'x', + share_link: `/gallery/${slug}/${token}`, + share_token: token, + require_password: 0, + expires_at: new Date(Date.now() + 7 * 864e5).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: isDraft ? 1 : 0, + created_by: adminId, + created_at: new Date().toISOString(), + }); + } + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + ({ adminId } = await seedMinimal(db)); + await assignAdminRole(db, adminId); + const [row] = await db('admin_users').insert({ + username: 'foreign', email: 'foreign@example.test', password_hash: 'unused', is_active: 1, + }).returning('id'); + foreignId = row?.id ?? row; + await assignAdminRole(db, foreignId, 'viewer'); + + await insertEvent({ slug: DRAFT_SLUG, token: DRAFT_TOKEN, isDraft: true }); + await insertEvent({ slug: LIVE_SLUG, token: LIVE_TOKEN, isDraft: false }); + + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use('/api/gallery', require('../../src/routes/gallery')); + }, 120000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + describe('the reported case — admin previewing a draft', () => { + it('resolves the draft by share token (was 404 "Gallery Not Found")', async () => { + const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?${preview()}`); + expect(res.status).toBe(200); + expect(res.body.slug).toBe(DRAFT_SLUG); + expect(res.body.matchType).toBe('token'); + }); + + it('resolves the draft by full share link', async () => { + const identifier = encodeURIComponent(`/gallery/${DRAFT_SLUG}/${DRAFT_TOKEN}`); + const res = await request(app).get(`/api/gallery/resolve/${identifier}?${preview()}`); + expect(res.status).toBe(200); + expect(res.body.slug).toBe(DRAFT_SLUG); + }); + + it('clears verify-token for the draft, the next step of the same flow', async () => { + const res = await request(app) + .get(`/api/gallery/${DRAFT_SLUG}/verify-token/${DRAFT_TOKEN}?${preview()}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + }); + }); + + // The transport the SHIPPED frontend uses. The first cut of this fix only + // tested ?preview=, which the browser never sends on an API call — so the + // suite passed while the feature stayed broken end to end. Caught in review. + describe('admin_preview=1 authenticated by the admin cookie', () => { + it('resolves the draft', async () => { + const res = await asAdmin( + request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`), + ); + expect(res.status).toBe(200); + expect(res.body.slug).toBe(DRAFT_SLUG); + }); + + it('clears verify-token', async () => { + const res = await asAdmin( + request(app).get(`/api/gallery/${DRAFT_SLUG}/verify-token/${DRAFT_TOKEN}?admin_preview=1`), + ); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + }); + + it('serves /info for the draft', async () => { + const res = await asAdmin( + request(app).get(`/api/gallery/${DRAFT_SLUG}/info?admin_preview=1`), + ); + expect(res.status).toBe(200); + }); + + it('serves draft MEDIA, which is what the flag on the URL is for', async () => { + // AuthenticatedImage/Video use native fetch and never see the axios + // interceptor, so the flag has to travel on the media URL itself. Without + // it the preview loaded metadata and showed no images at all. + const res = await asAdmin( + request(app).get(`/api/gallery/${DRAFT_SLUG}/photos?admin_preview=1`), + ); + expect(res.status).toBe(200); + }); + + it('404s with the flag but no admin cookie — the flag authorizes nothing', async () => { + const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`); + expect(res.status).toBe(404); + }); + + it('404s with the flag and a cookie that is not an admin JWT', async () => { + const res = await request(app) + .get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`) + .set('Cookie', 'admin_token=not-a-jwt'); + expect(res.status).toBe(404); + }); + }); + + describe('what must not move', () => { + it('404s an anonymous resolve of the draft token', async () => { + const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}`); + expect(res.status).toBe(404); + }); + + it('404s when ?preview= carries a token that is not a valid admin JWT', async () => { + const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?preview=not-a-jwt`); + expect(res.status).toBe(404); + }); + + it('404s when ?preview= is absent entirely', async () => { + const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?preview=`); + expect(res.status).toBe(404); + }); + + it('404s a non-owning admin on verify-token too (#1411)', async () => { + // This route selected its own columns and omitted created_by, so the + // ownership check saw an ownerless event and waved the caller through + // while /resolve and /info refused them. + const res = await asAdmin( + request(app).get(`/api/gallery/${DRAFT_SLUG}/verify-token/${DRAFT_TOKEN}?admin_preview=1`), + foreignId, + ); + expect(res.status).toBe(404); + }); + + it('404s an admin who does not own the event (#1411)', async () => { + // Was 200: a valid signature was the whole check, so any admin previewed + // any draft, including another photographer's. Now ownership applies — + // the same rule requireEventOwnership enforces everywhere else. + const res = await asAdmin( + request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`), + foreignId, + ); + expect(res.status).toBe(404); + + const info = await asAdmin( + request(app).get(`/api/gallery/${DRAFT_SLUG}/info?admin_preview=1`), + foreignId, + ); + expect(info.status).toBe(404); + }); + + it('404s an admin whose role grants no gallery permissions (#1411)', async () => { + // The owner, but stripped of events.view/photos.view. + const original = (await db('admin_users').where({ id: adminId }).first()).role_id; + await db('admin_users').where({ id: adminId }).update({ role_id: null }); + try { + const res = await asAdmin( + request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`), + ); + expect(res.status).toBe(404); + } finally { + await db('admin_users').where({ id: adminId }).update({ role_id: original }); + } + }); + + it('404s an admin whose account has been deactivated (#1411)', async () => { + await db('admin_users').where({ id: adminId }).update({ is_active: 0 }); + try { + const res = await asAdmin( + request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`), + ); + expect(res.status).toBe(404); + } finally { + await db('admin_users').where({ id: adminId }).update({ is_active: 1 }); + } + }); + + it('404s an anonymous verify-token for the draft', async () => { + const res = await request(app) + .get(`/api/gallery/${DRAFT_SLUG}/verify-token/${DRAFT_TOKEN}`); + expect(res.status).toBe(404); + }); + + it('still withholds the share_token on a bare slug lookup (GHSA-rh8r)', async () => { + // The draft path must not become a way around the token-withholding rule. + const res = await request(app).get(`/api/gallery/resolve/${DRAFT_SLUG}?${preview()}`); + expect(res.status).toBe(200); + expect(res.body.matchType).toBe('slug'); + expect(res.body.token).toBeUndefined(); + expect(res.body.share_link).toBeUndefined(); + expect(res.body.share_url).toBeUndefined(); + expect(JSON.stringify(res.body)).not.toContain(DRAFT_TOKEN); + }); + + it('leaves the published gallery resolving anonymously, as before', async () => { + const res = await request(app).get(`/api/gallery/resolve/${LIVE_TOKEN}`); + expect(res.status).toBe(200); + expect(res.body.slug).toBe(LIVE_SLUG); + expect(res.body.token).toBe(LIVE_TOKEN); + }); + + it('still 404s an identifier that matches nothing', async () => { + const res = await request(app).get(`/api/gallery/resolve/no-such-gallery?${preview()}`); + expect(res.status).toBe(404); + }); + }); +}); diff --git a/backend/package-lock.json b/backend/package-lock.json index 7beea481..663f5f46 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -1,12 +1,12 @@ { "name": "picpeak-backend", - "version": "3.46.11", + "version": "3.46.12", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "picpeak-backend", - "version": "3.46.11", + "version": "3.46.12", "dependencies": { "@aws-sdk/client-s3": "^3.850.0", "@aws-sdk/lib-storage": "^3.850.0", diff --git a/backend/src/__tests__/verifyGalleryAccess.customerRevoke.test.js b/backend/src/__tests__/verifyGalleryAccess.customerRevoke.test.js index 7129669e..13f04c80 100644 --- a/backend/src/__tests__/verifyGalleryAccess.customerRevoke.test.js +++ b/backend/src/__tests__/verifyGalleryAccess.customerRevoke.test.js @@ -50,7 +50,7 @@ const jwt = require('jsonwebtoken'); const { db } = require('../database/db'); const { getGalleryTokenFromRequest } = require('../utils/tokenUtils'); const { isTokenRevoked } = require('../utils/tokenRevocation'); -const { verifyGalleryAccess, isAdminPreview } = require('../middleware/gallery'); +const { verifyGalleryAccess, previewClaimed, verifyAdminPreview } = require('../middleware/gallery'); function makeRes() { const res = {}; @@ -149,20 +149,50 @@ describe('verifyGalleryAccess — revoked token', () => { }); }); -// ---- revoked admin-preview token (?preview=) ------------------ +// ---- revoked admin-preview token -------------------------------------- // -// isAdminPreview() decodes the ?preview= admin JWT independently of the -// main gallery-token flow above, and previously never checked -// isTokenRevoked — a revoked admin session kept granting preview access -// via a bookmarked/shared preview link indefinitely. Same gap as -// GHSA-q7f7-gjx8-mf6h, just in this sibling code path. +// The preview credential is decoded independently of the main gallery-token +// flow above, and once never checked isTokenRevoked — a revoked admin session +// kept granting preview access through a bookmarked or shared link +// indefinitely (same gap as GHSA-q7f7-gjx8-mf6h, in a sibling path). +// +// That check now lives in verifyAdminPreview rather than in the predicate the +// event lookup is shaped with. previewClaimed stays deliberately cheap and +// signature-only — it decides whether drafts are INCLUDED in the query, never +// whether they are served — and every lookup it shapes is gated behind +// verifyAdminPreview before anything reaches the caller. So a revoked token +// can still widen a query and still cannot preview anything. -describe('isAdminPreview — token revocation', () => { - it('returns false for a revoked admin token', async () => { +describe('previewClaimed — signature only, by design', () => { + it('accepts a syntactically valid admin token without consulting revocation', () => { jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); isTokenRevoked.mockResolvedValue(true); - const result = await isAdminPreview({ query: { preview: 'revoked-admin-jwt' } }); + expect(previewClaimed({ query: { preview: 'revoked-admin-jwt' } })).toBe(true); + // Deliberately NOT consulted here: this predicate is synchronous and only + // shapes the lookup. Authorization happens in verifyAdminPreview. + expect(isTokenRevoked).not.toHaveBeenCalled(); + }); + + it('rejects a non-admin token', () => { + jwt.verify.mockReturnValue({ type: 'gallery', eventId: 42 }); + expect(previewClaimed({ query: { preview: 'not-an-admin-jwt' } })).toBe(false); + }); + + it('rejects a request carrying no preview credential at all', () => { + expect(previewClaimed({ query: {} })).toBe(false); + }); +}); + +describe('verifyAdminPreview — token revocation', () => { + it('refuses a revoked admin token', async () => { + jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); + isTokenRevoked.mockResolvedValue(true); + + const result = await verifyAdminPreview( + { query: { preview: 'revoked-admin-jwt' }, headers: {} }, + { id: 42, created_by: 1 }, + ); expect(result).toBe(false); expect(isTokenRevoked).toHaveBeenCalledWith( @@ -170,56 +200,33 @@ describe('isAdminPreview — token revocation', () => { ); }); - it('returns true for a valid, non-revoked admin token', async () => { + it('fails closed when the revocation store cannot be read', async () => { + jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); + isTokenRevoked.mockRejectedValue(new Error('db down')); + + const result = await verifyAdminPreview( + { query: { preview: 'valid-admin-jwt' }, headers: {} }, + { id: 42, created_by: 1 }, + ); + + // A transient fault must not become a free preview. + expect(result).toBe(false); + }); + + it('refuses when there is no event to authorize against', async () => { jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); isTokenRevoked.mockResolvedValue(false); - const result = await isAdminPreview({ query: { preview: 'valid-admin-jwt' } }); - - expect(result).toBe(true); + expect(await verifyAdminPreview({ query: { preview: 'jwt' }, headers: {} }, null)).toBe(false); }); }); -describe('verifyGalleryAccess — revoked admin preview token', () => { - it('does not grant preview access; falls through to the normal flow and 401s', async () => { +describe('verifyGalleryAccess — a revoked preview token cannot open a draft', () => { + it('answers 404 for the draft instead of granting access', async () => { getGalleryTokenFromRequest.mockReturnValue(undefined); // no gallery-scoped token - jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); // decoded ?preview= token + jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); // decoded preview token isTokenRevoked.mockResolvedValue(true); - const eventsChain = {}; - eventsChain.where = jest.fn().mockReturnValue(eventsChain); - eventsChain.select = jest.fn().mockReturnValue(eventsChain); - eventsChain.first = jest.fn().mockResolvedValue({ - id: 42, slug: 'test-event', is_active: true, is_archived: false, - is_draft: false, require_password: true, - }); - db.mockImplementationOnce(() => eventsChain); - - const req = makeReq(); - req.query = { preview: 'revoked-admin-jwt' }; - const res = makeRes(); - const next = jest.fn(); - await verifyGalleryAccess(req, res, next); - - expect(isTokenRevoked).toHaveBeenCalledWith( - expect.objectContaining({ type: 'admin' }), - ); - // adminPreview resolved to false, so the code took the extra - // is_draft:false where() call it only skips for a real preview. - expect(eventsChain.where).toHaveBeenCalledTimes(2); - expect(eventsChain.where).toHaveBeenNthCalledWith(2, { is_draft: 0 }); - expect(next).not.toHaveBeenCalled(); - expect(res.status).toHaveBeenCalledWith(401); - expect(res.json).toHaveBeenCalledWith( - expect.objectContaining({ error: 'No token provided' }), - ); - }); - - it('a non-revoked admin preview token still works', async () => { - getGalleryTokenFromRequest.mockReturnValue(undefined); - jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); - isTokenRevoked.mockResolvedValue(false); - const eventsChain = {}; eventsChain.where = jest.fn().mockReturnValue(eventsChain); eventsChain.select = jest.fn().mockReturnValue(eventsChain); @@ -227,22 +234,18 @@ describe('verifyGalleryAccess — revoked admin preview token', () => { id: 42, slug: 'test-event', is_active: true, is_archived: false, is_draft: true, require_password: false, }); - db.mockImplementationOnce(() => eventsChain); + db.mockImplementation(() => eventsChain); const req = makeReq(); - req.query = { preview: 'valid-admin-jwt' }; + req.query = { preview: 'revoked-admin-jwt' }; const res = makeRes(); const next = jest.fn(); await verifyGalleryAccess(req, res, next); - expect(isTokenRevoked).toHaveBeenCalledWith( - expect.objectContaining({ type: 'admin' }), - ); - // adminPreview resolved to true, so no is_draft filter was applied. - expect(eventsChain.where).toHaveBeenCalledTimes(1); - expect(next).toHaveBeenCalledTimes(1); - expect(res.status).not.toHaveBeenCalled(); - expect(req.event).toEqual(expect.objectContaining({ id: 42 })); + // The lookup was widened (previewClaimed is signature-only), but the draft + // is refused at the gate — which is the contract that actually matters. + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(404); }); }); diff --git a/backend/src/middleware/gallery.js b/backend/src/middleware/gallery.js index 0c78ba4b..3f985ad9 100644 --- a/backend/src/middleware/gallery.js +++ b/backend/src/middleware/gallery.js @@ -2,31 +2,134 @@ const jwt = require('jsonwebtoken'); const { db, withRetry } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { getGalleryTokenFromRequest } = require('../utils/tokenUtils'); +const { userHasAllPermissions } = require('./permissions'); const { isTokenRevoked } = require('../utils/tokenRevocation'); const logger = require('../utils/logger'); -// Check if the request carries a valid admin preview token (Feature 3) -async function isAdminPreview(req) { - const previewToken = req.query?.preview; - if (!previewToken) return false; +// Admin preview of an unpublished gallery. +// +// Two transports (#1386): +// +// admin_preview=1 — an INTENT flag, authenticated by the admin's existing +// HttpOnly admin_token cookie (or an Authorization +// bearer). This is the one the frontend uses. The cookie +// rides along on same-origin requests automatically, +// including the native fetch() that AuthenticatedImage +// uses, so media works too — and no credential ever +// appears in a URL. +// +// preview= — the original transport, kept so existing hand-built +// links keep working. It puts an admin JWT in the query +// string, which reaches nginx access logs, browser +// history and Referer headers, so nothing emits it any +// more. +function previewTokenFrom(req) { + if (req.query?.admin_preview === '1') { + const header = req.headers?.authorization; + const bearer = header && header.startsWith('Bearer ') ? header.substring(7) : null; + const candidate = req.cookies?.admin_token || bearer; + if (candidate) return candidate; + } + return req.query?.preview || null; +} + +function decodeAdminToken(token) { + if (!token) return null; try { - const decoded = jwt.verify(previewToken, process.env.JWT_SECRET, { issuer: 'picpeak-auth' }); - if (decoded.type !== 'admin') return false; - - // Same gap this file already closed for the main gallery-token path - // (GHSA-q7f7-gjx8-mf6h): a revoked admin session must not keep - // granting preview access via a bookmarked/shared ?preview= link. - // Treat a revoked token the same as an invalid one -- fail the - // preview check, don't throw, so callers fall through to the normal - // gallery-token flow. - if (await isTokenRevoked(decoded)) { - return false; - } - - return true; + const decoded = jwt.verify(token, process.env.JWT_SECRET, { + algorithms: ['HS256'], issuer: 'picpeak-auth', + }); + return decoded.type === 'admin' ? decoded : null; } catch { + return null; + } +} + +/** + * Signature-only predicate. It proves the caller holds SOME valid admin token + * and nothing else — not that the account still exists, not that the token is + * unrevoked, and not that this admin may see this event. + * + * Its only legitimate use is shaping the event lookup, which has to decide + * whether to include drafts BEFORE there is an event to authorize against. + * Every such lookup must be followed by assertDraftPreviewAllowed (#1411). + */ +function previewClaimed(req) { + return decodeAdminToken(previewTokenFrom(req)) !== null; +} + +/** + * Full authorization for previewing a specific event (#1411). + * + * The signature check above used to be the whole story, so any valid admin + * token previewed any draft — including one created by a different admin, and + * including an account whose role grants neither events.view nor photos.view. + * `main` closes this via access.authorize; this is the same rule applied where + * this branch keeps its checks. + */ +async function verifyAdminPreview(req, event) { + const decoded = decodeAdminToken(previewTokenFrom(req)); + if (!decoded || !event) return false; + + // A signed-out or rotated session must stop previewing, same as it stops + // reaching every other admin surface. This is the check isAdminPreview + // carried for GHSA-q7f7-gjx8-mf6h — a revoked admin session must not keep + // granting preview access through a bookmarked or shared link — kept here, + // at the point where preview is actually authorized rather than where the + // event lookup is merely shaped. + try { + if (await isTokenRevoked(decoded)) return false; + } catch (error) { + // Fail closed: a transient DB fault must not become a free preview. + logger.warn('Admin preview revocation check failed', { error: error.message }); return false; } + + let admin; + try { + admin = await db('admin_users') + .leftJoin('roles', 'roles.id', 'admin_users.role_id') + .where({ 'admin_users.id': decoded.id, 'admin_users.is_active': formatBoolean(true) }) + .select('admin_users.id', 'roles.name as role_name') + .first(); + } catch (error) { + // Same posture as adminAuth's join fallback: an install whose roles table + // predates the schema still has admins, but it has no role to check, so + // ownership below is the only gate that applies. + logger.debug('Admin preview role lookup failed', { error: error.message }); + admin = await db('admin_users') + .where({ id: decoded.id, is_active: formatBoolean(true) }) + .select('id').first(); + if (admin) admin.role_name = null; + } + if (!admin) return false; + + // Ownership: super_admin sees everything, everyone else sees ownerless + // (legacy/system) events plus their own — the rule requireEventOwnership + // and scopeEventsQuery already enforce elsewhere. + const owns = admin.role_name === 'super_admin' + || !event.created_by + || Number(event.created_by) === Number(admin.id); + if (!owns) return false; + + try { + return await userHasAllPermissions(admin.id, ['events.view', 'photos.view']); + } catch (error) { + logger.warn('Admin preview permission check failed', { error: error.message }); + return false; + } +} + +/** + * Gate a loaded event behind the preview rules. Published events pass through + * untouched; a draft is visible only to an authorized admin preview. Returns + * false when the caller must be told the gallery does not exist. + */ +async function assertDraftPreviewAllowed(req, event) { + if (!event) return true; + const isDraft = event.is_draft === true || event.is_draft === 1 || event.is_draft === '1'; + if (!isDraft) return true; + return verifyAdminPreview(req, event); } // Middleware to verify gallery access @@ -41,7 +144,7 @@ async function verifyGalleryAccess(req, res, next) { return res.status(401).json({ error: 'No token provided' }); } - const adminPreview = await isAdminPreview(req); + const adminPreview = previewClaimed(req); event = await withRetry(async () => { const q = db('events') .where({ @@ -59,6 +162,12 @@ async function verifyGalleryAccess(req, res, next) { return res.status(404).json({ error: 'Gallery not found or expired' }); } + // The lookup above included drafts on a signature-only check. Authorize + // the draft now that there is an event to authorize against (#1411). + if (!await assertDraftPreviewAllowed(req, event)) { + return res.status(404).json({ error: 'Gallery not found or expired' }); + } + const requiresPassword = !(event.require_password === false || event.require_password === 0 || event.require_password === '0'); if (!requiresPassword) { req.event = event; @@ -113,7 +222,7 @@ async function verifyGalleryAccess(req, res, next) { // If we have a slug in the URL params or from pre-middleware, verify it matches if (requestedSlug) { // Verify by slug and ensure it matches the token's event - const adminPreviewToken = await isAdminPreview(req); + const adminPreviewToken = previewClaimed(req); event = await withRetry(async () => { const q = db('events') .where({ @@ -133,7 +242,7 @@ async function verifyGalleryAccess(req, res, next) { } } else { // Fallback to using eventId from token - const adminPreviewFallback = await isAdminPreview(req); + const adminPreviewFallback = previewClaimed(req); event = await withRetry(async () => { const q = db('events') .where({ @@ -153,6 +262,12 @@ async function verifyGalleryAccess(req, res, next) { return res.status(404).json({ error: 'Gallery not found or expired' }); } + // Same gate as the public branch above (#1411): the draft was included in + // the lookup on a signature-only check and has to be authorized here. + if (!await assertDraftPreviewAllowed(req, event)) { + return res.status(404).json({ error: 'Gallery not found or expired' }); + } + // Customer-minted gallery JWTs (#354): when the customer obtained // this token via /api/customer/events/:slug/access-token, the // payload carries `via:'customer'` and `customerId`. The admin @@ -223,5 +338,7 @@ function denySlideshowToken(req, res, next) { module.exports = { verifyGalleryAccess, denySlideshowToken, - isAdminPreview + previewClaimed, + verifyAdminPreview, + assertDraftPreviewAllowed }; diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 82b4c1e7..67240db1 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -26,7 +26,7 @@ function resolveHeroLogoVisible(perEvent, globalDefault) { } const watermarkService = require('../services/watermarkService'); const watermarkGeneratorService = require('../services/watermarkGeneratorService'); -const { verifyGalleryAccess, denySlideshowToken, isAdminPreview } = require('../middleware/gallery'); +const { verifyGalleryAccess, denySlideshowToken, verifyAdminPreview } = require('../middleware/gallery'); const { resolveGuest } = require('../middleware/guestAuth'); const { generateGuestIdentifier } = require('../middleware/feedbackRateLimit'); const secureImageService = require('../services/secureImageService'); @@ -111,11 +111,36 @@ async function checkSlugRedirect(slug) { } } +// Admin preview of an unpublished gallery (#1386). The /info route below has +// honoured ?preview= for drafts for a while; this route never did, so the +// short-URL form of a draft's share link 404'd with "Gallery Not Found" while +// the long slug form worked. +// +// Deliberately a second lookup on the miss path rather than a widened filter: +// the published case keeps its single query and cannot start returning drafts +// however this evolves, and an unverified caller never gets so far as knowing +// the draft exists. +async function resolveDraftForAdminPreview(req, identifier) { + // A preview credential is required either way, so checking up front costs + // nothing and keeps an unknown identifier from paying for a second set of + // lookups on the public 404 path. admin_preview=1 is what the frontend + // sends; the bare ?preview= is the legacy hand-built-link form. + if (req.query?.admin_preview !== '1' && !req.query?.preview) return null; + const result = await resolveShareIdentifier(identifier, { includeDrafts: true }); + if (!result) return null; + // Authorized against THIS event, not just against a valid signature (#1411). + return await verifyAdminPreview(req, result.event) ? result : null; +} + // Resolve gallery identifier (slug or token) to canonical data router.get('/resolve/:identifier', handleAsync(async (req, res) => { const { identifier } = req.params; let result = await resolveShareIdentifier(identifier); + if (!result) { + result = await resolveDraftForAdminPreview(req, identifier); + } + // If not found, check for redirect if (!result) { const newSlug = await checkSlugRedirect(identifier); @@ -161,14 +186,24 @@ router.get('/:slug/verify-token/:token', handleAsync(async (req, res) => { const { slug, token } = req.params; const event = await db('events') - .where({ slug, is_active: formatBoolean(true), is_archived: formatBoolean(false), is_draft: formatBoolean(false) }) - .select('id', 'share_link', 'share_token') + .where({ slug, is_active: formatBoolean(true), is_archived: formatBoolean(false) }) + // created_by is the ownership input for verifyAdminPreview (#1411). + // Omitting it made every draft look ownerless here, so a non-owning admin + // holding events.view/photos.view validated another photographer's share + // link while /resolve and /info correctly refused them. + .select('id', 'share_link', 'share_token', 'is_draft', 'created_by') .first(); if (!event) { throw new NotFoundError('Gallery'); } + // Drafts are visible to an authorized admin preview only (#1386, #1411). + // Without this the preview clears /resolve and then 404s one step later. + if (event.is_draft && !await verifyAdminPreview(req, event)) { + throw new NotFoundError('Gallery'); + } + const expectedToken = getEventShareToken(event); if (!expectedToken || !timingSafeEqualStr(String(token), expectedToken)) { throw new NotFoundError('Gallery', 'Invalid gallery link'); @@ -211,6 +246,8 @@ router.get('/:slug/info', async (req, res) => { 'hero_divider_style', 'hero_image_anchor', 'is_draft', + // Ownership input for the preview check (#1411). + 'created_by', 'default_photo_sort', // Per-event promotional override (#440). Resolution into a // ready-to-render markdown string happens below so the @@ -238,8 +275,10 @@ router.get('/:slug/info', async (req, res) => { return res.status(404).json({ error: 'Gallery has been archived and is no longer available' }); } - // Check if event is a draft (allow admin preview) - if (event.is_draft && !(await isAdminPreview(req))) { + // Check if event is a draft (allow an AUTHORIZED admin preview — #1411: + // a valid signature alone used to be enough, so any admin previewed any + // draft, including one belonging to a different photographer). + if (event.is_draft && !await verifyAdminPreview(req, event)) { return res.status(404).json({ error: 'Gallery is not yet published' }); } diff --git a/backend/src/services/shareLinkService.js b/backend/src/services/shareLinkService.js index d5ffe602..b08b6263 100644 --- a/backend/src/services/shareLinkService.js +++ b/backend/src/services/shareLinkService.js @@ -118,7 +118,15 @@ const ACTIVE_EVENT_FILTER = { is_draft: formatBoolean(false) }; -const resolveShareIdentifier = async (identifier) => { +// Same filter minus the draft gate, for admin preview only (#1386). Callers +// MUST authorize before returning anything it matched — see the /resolve +// route, which only reaches for it after isAdminPreview accepts the caller. +const UNPUBLISHED_EVENT_FILTER = { + is_active: formatBoolean(true), + is_archived: formatBoolean(false) +}; + +const resolveShareIdentifier = async (identifier, { includeDrafts = false } = {}) => { if (!identifier) { return null; } @@ -140,9 +148,13 @@ const resolveShareIdentifier = async (identifier) => { 'event_date', 'expires_at', 'is_active', - 'is_archived' + 'is_archived', + 'is_draft', + // Ownership input for the preview check (#1411) — a draft is only + // previewable by an admin who may see this event. + 'created_by' ) - .where(ACTIVE_EVENT_FILTER); + .where(includeDrafts ? UNPUBLISHED_EVENT_FILTER : ACTIVE_EVENT_FILTER); let event = await baseQuery.clone().where({ slug: trimmed }).first(); if (event) { diff --git a/frontend/src/components/common/AuthenticatedImage.tsx b/frontend/src/components/common/AuthenticatedImage.tsx index ea862703..8be14df5 100644 --- a/frontend/src/components/common/AuthenticatedImage.tsx +++ b/frontend/src/components/common/AuthenticatedImage.tsx @@ -1,5 +1,6 @@ import React, { useState, useEffect, useRef, useCallback } from 'react'; import { buildResourceUrl } from '../../utils/url'; +import { withAdminPreview } from '../../utils/adminPreview'; import { getActiveGallerySlug, getGalleryToken, @@ -140,11 +141,15 @@ export const AuthenticatedImage: React.FC = ({ // Build full URL for the image. Only relative paths are app-owned; // an absolute URL is passed through untouched. const isRelative = rawUrl.startsWith('/'); - const fullImageUrl = rawUrl.startsWith('/admin') - ? buildResourceUrl(`/api${rawUrl}`) + // Flag goes on while the URL is still relative: buildResourceUrl can + // return an absolute URL in split deployments, and withAdminPreview + // deliberately refuses those (#1386). + const previewUrl = isRelative ? withAdminPreview(rawUrl) : rawUrl; + const fullImageUrl = previewUrl.startsWith('/admin') + ? buildResourceUrl(`/api${previewUrl}`) : isRelative - ? buildResourceUrl(rawUrl) - : rawUrl; + ? buildResourceUrl(previewUrl) + : previewUrl; const headers: Record = {}; // Attach the gallery bearer token ONLY to relative (same-app) image diff --git a/frontend/src/components/gallery/VideoPlayer.tsx b/frontend/src/components/gallery/VideoPlayer.tsx index 209a8285..b2bfaa8e 100644 --- a/frontend/src/components/gallery/VideoPlayer.tsx +++ b/frontend/src/components/gallery/VideoPlayer.tsx @@ -1,6 +1,7 @@ import React, { useRef, useState, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; import { Play, Pause, Volume2, VolumeX, Maximize, Minimize, AlertTriangle } from 'lucide-react'; +import { withAdminPreview } from '../../utils/adminPreview'; interface VideoPlayerProps { src: string; @@ -176,10 +177,13 @@ export const VideoPlayer: React.FC = ({ onMouseMove={handleMouseMove} onMouseLeave={() => isPlaying && setShowControls(false)} > + {/* A bare