fix(gallery): let an admin preview a draft through its short share URL (stable) (#1407)

* fix(gallery): let an admin preview a draft through its short share URL (stable)

Stable twin of the main-branch fix.

/resolve/:identifier filtered drafts out through ACTIVE_EVENT_FILTER and
/:slug/verify-token/:token repeated the filter inline, so with "use short
gallery URLs" on the admin's own View Gallery link answered "Gallery Not
Found" for an unpublished gallery. With the setting off the link carries the
slug, /info serves it, and the preview worked — which is why this looked like
a short-URL bug rather than a draft one.

The mechanism differs from main by branch: stable identifies an admin preview
by a signed admin JWT in ?preview=, so this uses isAdminPreview, the same
predicate /info already uses for its draft gate. Both routes now match /info
rather than being stricter than the branch they live on.

The draft lookup only runs after isAdminPreview accepts the caller, so the
published path keeps its single query and an unverified caller never learns
the draft exists. GHSA-rh8r is unchanged and pinned by test: a bare slug
lookup still never returns share_token.

Relates to issue 1386

* fix(gallery): carry the admin preview credential to the API on stable

External review found the backend half of the previous commit was unreachable:
`preview=` appeared in exactly two places in the whole frontend — building the
View Gallery link and reading the token — and nothing forwarded it into the API
calls the gallery page then makes. So the new /resolve fallback exited at its
guard for every real browser request, and the /info draft escape that has been
there all along was equally inert. Draft preview on this branch was broken for
both URL forms, not just short ones.

The request interceptor now forwards the credential as x-admin-preview, and
isAdminPreview accepts it there as well as in ?preview=. A header rather than a
query parameter because the credential is the admin's own session JWT, and
query strings reach nginx access logs, browser history and Referer headers.
?preview= stays accepted: the gallery PAGE url is what the browser navigates
to, and hand-built links rely on it.

The tests only exercised ?preview=, which the browser never sends on an API
call — so they passed while the feature stayed broken end to end. They now
cover the header transport across /resolve, /verify-token and /info.

Relates to issue 1386

* fix(gallery): authenticate the draft preview by the admin cookie on stable

The header transport in the previous commit could not work. `admin_token`
appears exactly once in this frontend — the read inside getPreviewToken() —
and nothing ever writes it: AdminAuthContext stores only admin_user and the
JWT lives in an HttpOnly cookie. So getPreviewToken() always returned null,
the View Gallery link was built as `?preview=` with an empty value, and every
transport downstream had nothing to carry. Draft preview on this branch has
never worked from the UI, by either URL form.

The machinery was already there: verifyGalleryAccess drops the is_draft
constraint for a preview in three places. Only delivery was missing.

isAdminPreview now also accepts `admin_preview=1` as an intent flag,
authenticated by the admin_token cookie the browser already sends. That fixes
every caller at once, including the native fetch() in AuthenticatedImage and
AuthenticatedVideo, which bypasses the axios interceptor entirely — without
the flag on the media URL a preview loaded its metadata and then showed no
thumbnails, hero or lightbox media at all. The flag alone authorizes nothing:
with no valid admin token the check fails closed.

`?preview=<jwt>` keeps working for hand-built links, but nothing emits it any
more, so the admin's own session JWT no longer travels in a query string where
nginx access logs, browser history and Referer headers can see it.
getPreviewToken() is deleted along with its now-orphaned import.

Relates to issue 1386

* fix(gallery): carry the preview flag on every non-axios gallery URL

Third review round found the flag still missing on the paths that never touch
the axios interceptor:

- PhotoLightbox renders VideoPlayer, which assigns the photo URL straight to
  <video src>. Draft video playback 404'd. The previous commit had put the flag
  in AuthenticatedVideo, which has no consumers on this branch at all — dead
  code fixing nothing. Reverted; VideoPlayer carries it now, for both src and
  poster.
- savePhotoToDevice builds a native anchor from api.getUri(), and
  downloadAllPhotos uses a direct anchor when a zip is ready. Both downloads
  404'd inside a preview.

The three call sites plus AuthenticatedImage now share utils/adminPreview.ts
rather than repeating the check. It refuses absolute URLs, and the flag is
applied while the URL is still relative — buildResourceUrl can turn it
absolute in split deployments, which would have dropped it silently.

Relates to issue 1386

* fix(gallery): authorize the admin preview against the event, not just the token

isAdminPreview verified the JWT signature and `type === 'admin'` and checked
nothing else — not that the account still exists, not that the token is
unrevoked, and not that this admin may see this event. verifyGalleryAccess
then dropped the is_draft constraint on that basis, so any valid admin token
previewed any draft gallery and its photos, including one created by a
different photographer and including an account whose role grants neither
events.view nor photos.view. main closes this through access.authorize; this
applies the same rule where this branch keeps its checks.

The predicate could not simply be tightened in place: it runs while the event
lookup is being shaped, before there is an event to authorize against. So it
splits in two. previewClaimed() stays synchronous and signature-only, and its
one legitimate use is deciding whether the lookup includes drafts.
verifyAdminPreview(req, event) then applies the real rules — revocation, an
active account, ownership (super_admin, ownerless, or own event) and
events.view + photos.view — and assertDraftPreviewAllowed gates every loaded
event behind it. Both query branches in verifyGalleryAccess converge on one
`if (!event)`, so two gates cover all three lookups.

Fails closed on a transient database fault in the revocation or permission
check, rather than treating an error as a pass. The roles-table fallback
mirrors adminAuth: an install predating that schema has admins but no role to
check, so ownership is the only gate that applies there.

The suite previously carried a test documenting the hole — "accepts any valid
admin token, matching /info on this branch". That is replaced by the three
cases it was standing in for: a non-owning admin, an admin with no gallery
permissions, and a deactivated account, each 404 now and 200 before.

Relates to issue 1411

* fix(gallery): close two gaps in the draft-preview authorization

Found by a fourth review round.

verify-token selected its own columns and omitted created_by, so
verifyAdminPreview saw an ownerless event and allowed any admin holding
events.view and photos.view — including one who does not own the draft, and
while /resolve and /info were correctly refusing them. The ownership check was
running; it just had nothing to check against.

savePhotoToDevice applied the preview flag to the output of api.getUri().
With an absolute VITE_API_URL that is an absolute URL, which withAdminPreview
refuses by design, so the flag was silently dropped and desktop and Android
preview downloads 404'd. Applied to the relative path before getUri expands it.

Relates to issue 1386
Relates to issue 1411

* fix(gallery): keep an admin draft preview out of the guest share-login flow (stable)

Twin of the main-branch fix. Making verify-token pass for a draft preview
opened a path that did not exist before it: the gallery bootstrap then called
shareLinkLogin, whose share lookup excludes drafts, so it 404'd and recorded a
failed login attempt against the caller's IP on the way out. Five preview opens
inside the attempt window locked share-link logins out for that IP — for real
guests too, and after publishing.

An admin preview needs no guest session: the admin cookie plus admin_preview=1
already authorizes every gallery call. The preview path loads the gallery
directly and never touches the login endpoint.

Relates to issue 1386

* test(gallery): move the preview revocation tests onto the new predicates

The revocation hardening that landed in the meantime shipped unit tests
against isAdminPreview, which this branch replaces with previewClaimed plus
verifyAdminPreview. They were asserting the old shape — including which where()
calls the event lookup makes — so they broke on the merge.

Rewritten against the contract that actually matters rather than the query
shape: previewClaimed is signature-only by design and deliberately does not
consult revocation, and verifyAdminPreview refuses a revoked token, fails
closed when the revocation store cannot be read, and refuses when there is no
event to authorize against. The end-to-end case is asserted through
verifyGalleryAccess: a revoked preview token widens the lookup and still gets
404 for the draft.

Found by CI, not locally — these live in backend/src/__tests__, a second test
root that the suites I had been running do not cover.

---------

Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
2026-09-11 12:14:24 +02:00
committed by GitHub
co-authored by Paul Nothaft
parent a5e797e5db
commit 7382e13371
14 changed files with 607 additions and 108 deletions
@@ -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=<jwt> 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: '[email protected]',
admin_email: '[email protected]',
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: '[email protected]', 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);
});
});
});
+2 -2
View File
@@ -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",
@@ -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=<adminJWT>) ------------------
// ---- 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);
});
});
+139 -22
View File
@@ -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=<jwt> — 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
};
+44 -5
View File
@@ -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=<jwt> 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' });
}
+15 -3
View File
@@ -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) {
@@ -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<AuthenticatedImageProps> = ({
// 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<string, string> = {};
// Attach the gallery bearer token ONLY to relative (same-app) image
@@ -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<VideoPlayerProps> = ({
onMouseMove={handleMouseMove}
onMouseLeave={() => isPlaying && setShowControls(false)}
>
{/* A bare <video src> never touches the axios interceptor, so a draft
preview needs the flag on the URL itself (#1386) — otherwise the
gallery renders and the video 404s. */}
<video
ref={videoRef}
src={src}
poster={poster}
src={withAdminPreview(src)}
poster={poster ? withAdminPreview(poster) : poster}
autoPlay={autoPlay}
muted={muted}
loop={loop}
+10
View File
@@ -115,6 +115,16 @@ api.interceptors.request.use(
}
}
}
// Admin draft preview (#1386). The gallery tab was opened with
// ?admin_preview=1; forward that intent flag on every gallery API call
// so the backend applies the draft bypass. The HttpOnly admin_token
// cookie authenticates it server-side — no credential in the URL.
// Harmless for guests: without a valid admin cookie the check fails
// closed and they get exactly what they got before.
if (new URLSearchParams(window.location.search).get('admin_preview') === '1') {
config.params = { ...(config.params as Record<string, unknown> | undefined), admin_preview: 1 };
}
}
}
@@ -249,6 +249,24 @@ export const GalleryAuthProvider: React.FC<GalleryAuthProviderProps> = ({ childr
if (routeInfo.token) {
const verify = await galleryService.verifyToken(currentSlug, routeInfo.token);
if (verify?.valid) {
// An admin preview does not take a guest session (#1386). The admin
// cookie plus admin_preview=1 already authorizes every gallery call,
// and shareLinkLogin refuses drafts AND records a failed attempt
// when it does — so opening a draft preview five times would lock
// share-link logins out for that IP, even after publishing.
const isAdminPreview = typeof window !== 'undefined'
&& new URLSearchParams(window.location.search).get('admin_preview') === '1';
if (isAdminPreview) {
const previewData = await galleryService.getGalleryPhotos(currentSlug);
if (previewData?.event) {
const previewEvent = normalizeEvent(previewData.event);
setEvent(previewEvent);
setActiveGallerySlug(currentSlug);
setIsAuthenticated(true);
return;
}
}
const response = await authService.shareLinkLogin(currentSlug, routeInfo.token);
if (response?.event) {
// Store token and slug BEFORE setting authenticated state to avoid
@@ -19,7 +19,6 @@ import type { Event } from '../../../types';
import { Button, Card } from '../../../components/common';
import { useLocalizedDate } from '../../../hooks/useLocalizedDate';
import { useFeatureFlags } from '../../../contexts/FeatureFlagsContext';
import { eventsService } from '../../../services/events.service';
import { buildShareLinkUrl } from '../../../utils/url';
import { isGalleryPublic } from '../../../utils/accessControl';
import type { FeedbackSettings as FeedbackSettingsType } from '../../../services/feedback.service';
@@ -191,7 +190,7 @@ export const EventDetailsHeader: React.FC<EventDetailsHeaderProps> = ({
{event.share_link && !isEditing && (
<a
href={event.is_draft
? `${buildShareLinkUrl(event.share_link)}${buildShareLinkUrl(event.share_link).includes('?') ? '&' : '?'}preview=${eventsService.getPreviewToken() || ''}`
? `${buildShareLinkUrl(event.share_link)}${buildShareLinkUrl(event.share_link).includes('?') ? '&' : '?'}admin_preview=1`
: buildShareLinkUrl(event.share_link)
}
target="_blank"
-5
View File
@@ -284,11 +284,6 @@ export const eventsService = {
},
// Get admin preview token (uses existing admin session token)
getPreviewToken(): string | null {
const token = sessionStorage.getItem('admin_token') || localStorage.getItem('admin_token');
return token;
},
// Rename event
async renameEvent(eventId: number, newEventName: string, resendEmail: boolean = false): Promise<{
success: boolean;
+8 -2
View File
@@ -2,6 +2,7 @@ import { api } from '../config/api';
import type { GalleryInfo, GalleryData, GalleryStats, ResolvedGalleryIdentifier } from '../types';
import { normalizeRequirePassword } from '../utils/accessControl';
import { parseContentDispositionFilename } from '../utils/contentDisposition';
import { withAdminPreview } from '../utils/adminPreview';
// iOS is the only platform whose system share sheet exposes a
// first-party "Save Image" / "Save to Photos" action for files
@@ -88,7 +89,12 @@ export const galleryService = {
async savePhotoToDevice(slug: string, photoId: number, filename: string): Promise<void> {
if (!isIOS()) {
this.triggerDirectDownload(
api.getUri({ url: `/gallery/${slug}/download/${photoId}` }),
// Native anchor download: bypasses the axios interceptor, so a draft
// preview needs the flag on the URL itself (#1386). Applied to the
// relative path BEFORE getUri: with an absolute VITE_API_URL getUri
// returns an absolute URL, and withAdminPreview refuses those by
// design, which would silently drop the flag.
api.getUri({ url: withAdminPreview(`/gallery/${slug}/download/${photoId}`) }),
filename,
);
return;
@@ -215,7 +221,7 @@ export const galleryService = {
// Native browser download — the server sends Content-Length so
// the browser shows a real progress bar and mobile doesn't crash.
const link = document.createElement('a');
link.href = `/api/gallery/${slug}/download-all`;
link.href = withAdminPreview(`/api/gallery/${slug}/download-all`);
link.setAttribute('download', `${slug}.zip`);
document.body.appendChild(link);
link.click();
+23
View File
@@ -0,0 +1,23 @@
/**
* Admin draft preview (#1386).
*
* The gallery tab is opened with `?admin_preview=1`, and the axios interceptor
* forwards that flag on every gallery API call. Anything that does NOT go
* through axios — a native `fetch`, a `<video src>`, an `<a href>` download —
* has to put the flag on its own URL, or `verifyGalleryAccess` filters the
* unpublished event out and answers 404.
*
* The flag is an intent signal only: the admin's HttpOnly `admin_token` cookie
* is what actually authenticates it, and it rides along on its own because all
* of these are same-origin. No credential is ever placed in a URL.
*/
export function withAdminPreview(url: string | null | undefined): string {
if (!url) return url || '';
// Relative (app-owned) URLs only. Never append to an absolute URL: that
// could point at any origin, and the flag would be a hint to a third party
// about what the admin is doing.
if (!url.startsWith('/')) return url;
if (typeof window === 'undefined') return url;
if (new URLSearchParams(window.location.search).get('admin_preview') !== '1') return url;
return `${url}${url.includes('?') ? '&' : '?'}admin_preview=1`;
}