paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

feat(gallery): admin preview skips the password on protected galleries (#981)

Browse Source 
Closes #868.

A logged-in admin opening a published, password-protected gallery is let
straight in, mirroring the existing draft-visibility bypass.

Mechanism: an explicit ?admin_preview=1 intent flag AND a verified admin session
read from the httpOnly admin_token cookie (or an admin-typed Bearer) — never a
token from the URL. This retires the old ?preview=<raw-admin-JWT> scheme, which
leaked a 24h admin token into the address bar, referrers and proxy logs.

Per-request bypass only: no gallery JWT is minted, the password endpoint is
never reached so the login_attempts lockout buckets stay clean, and admin
previews are excluded from guest analytics (access_logs, download counts,
per-photo view_count, notification bells).

Review (two rounds) closed three blockers and two concerns:

- Transport: verifyGalleryAccess now resolves admin preview before any gallery
  credential, and isAdminPreview reads the admin cookie first and type-checks
  every candidate — so an admin Bearer no longer 403s on the type gate, and a
  coexisting gallery session can no longer shadow the admin cookie.
- Reveal mode (#838) is a second consumer of isAdminPreview; its bypass is
  unchanged, only the transport moves. revealMode.test.js updated off the
  retired scheme and now carries a coexisting gallery Bearer.
- Admin previews no longer inflate per-photo view counts, and the internal photo
  redirects preserve the flag via withPreview() so they still authorise.
- Happy path: GalleryPage renders GalleryView directly for a preview instead of
  attempting the public empty-password auto-login, which 401'd against a
  genuinely protected gallery and stranded the page on the skeleton.

The backend job timed out once at the 10-minute CI limit; a re-run completed in
2m02s, in line with main's ~2m10s baseline, so that was a runner flake rather
than a hang.
This commit is contained in:
Luca
2026-08-04 16:48:07 +02:00
committed by GitHub
parent 137a42f259
commit f00661511c
9 changed files with 343 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/middleware/gallery.js
+90 -50
View File
@@ -4,22 +4,75 @@ const { formatBoolean } = require('../utils/dbCompat');
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
const logger = require('../utils/logger');
// Check if the request carries a valid admin preview token (Feature 3)
/**
* True when a logged-in admin is explicitly previewing this gallery (#868).
*
* Two conditions, both required:
* 1. The explicit intent flag `?admin_preview=1` is present. The plain share
* link stays byte-identical to a guest's, so the password gate is still
* testable as a guest while logged in as admin — and the bypass is visible
* in the URL without being reusable (it carries no secret).
* 2. A VERIFIED admin session — the httpOnly `admin_token` cookie (rides along
* on same-origin API calls) or an Authorization: Bearer header, never the
* URL. Must decode as `type: 'admin'`, issuer `picpeak-auth`.
*
* The cookie is tried FIRST and the Bearer is accepted only when it is itself an
* admin token (#981 review): the frontend attaches a gallery Bearer to gallery
* endpoints, and a header-first, type-blind read would let a coexisting gallery
* session shadow the admin cookie and wrongly disable the preview.
*
* Fails closed on any verification error. Replaces the old `?preview=<raw-JWT>`
* scheme, which leaked a 24h admin token into the address bar.
*/
function isAdminPreview(req) {
const previewToken = req.query?.preview;
if (!previewToken) return false;
try {
const decoded = jwt.verify(previewToken, process.env.JWT_SECRET, { issuer: 'picpeak-auth' });
return decoded.type === 'admin';
} catch {
return false;
if (req.query?.admin_preview !== '1') return false;
// Cookie first, then a Bearer — but only an admin-typed token satisfies it.
const candidates = [];
if (req.cookies?.admin_token) candidates.push(req.cookies.admin_token);
const header = req.headers?.authorization;
if (header && header.startsWith('Bearer ')) candidates.push(header.slice(7));
for (const token of candidates) {
try {
const decoded = jwt.verify(token, process.env.JWT_SECRET, { issuer: 'picpeak-auth' });
if (decoded.type === 'admin') return true;
} catch { /* try the next candidate */ }
}
return false;
}
// Middleware to verify gallery access
async function verifyGalleryAccess(req, res, next) {
try {
const requestedSlug = req.params.slug || req.requestedSlug;
// Admin preview (#868) is resolved BEFORE any gallery credential (#981
// review): a coexisting gallery token/Bearer must not shadow it, and the
// admin session must never fall into the `type !== 'gallery'` reject path
// below. Per-request bypass — draft + password relaxed, NO gallery JWT
// minted (a lingering guest cookie would muddy the coexisting-cookies case).
// req.isAdminPreview flags downstream logging to keep it out of guest stats.
if (isAdminPreview(req)) {
if (!requestedSlug) {
return res.status(401).json({ error: 'No token provided' });
}
const previewEvent = await withRetry(async () => db('events')
.where({ slug: requestedSlug, is_active: formatBoolean(true), is_archived: formatBoolean(false) })
.select('*').first());
if (!previewEvent) {
return res.status(404).json({ error: 'Gallery not found or expired' });
}
req.event = previewEvent;
req.isAdminPreview = true;
req.sessionID = `gallery_admin_preview_${previewEvent.id}`;
req.clientInfo = {
ip: req.ip || req.connection.remoteAddress || 'unknown',
userAgent: req.get('User-Agent') || 'unknown',
fingerprint: `${req.ip}-${req.get('User-Agent')}`.substring(0, 32),
timestamp: Date.now()
};
return next();
}
const token = getGalleryTokenFromRequest(req, requestedSlug);
let event;
@@ -28,19 +81,14 @@ async function verifyGalleryAccess(req, res, next) {
return res.status(401).json({ error: 'No token provided' });
}
const adminPreview = isAdminPreview(req);
event = await withRetry(async () => {
const q = db('events')
.where({
slug: requestedSlug,
is_active: formatBoolean(true),
is_archived: formatBoolean(false)
});
if (!adminPreview) {
q.where({ is_draft: formatBoolean(false) });
}
return await q.select('*').first();
});
event = await withRetry(async () => db('events')
.where({
slug: requestedSlug,
is_active: formatBoolean(true),
is_archived: formatBoolean(false),
is_draft: formatBoolean(false)
})
.select('*').first());
if (!event) {
return res.status(404).json({ error: 'Gallery not found or expired' });
@@ -61,7 +109,7 @@ async function verifyGalleryAccess(req, res, next) {
return res.status(401).json({ error: 'No token provided' });
}
// Try to verify with issuer first, fallback to no issuer for backward compatibility
let decoded;
try {
@@ -89,42 +137,34 @@ async function verifyGalleryAccess(req, res, next) {
return res.status(403).json({ error: 'Invalid token type for gallery access' });
}
// If we have a slug in the URL params or from pre-middleware, verify it matches
// If we have a slug in the URL params or from pre-middleware, verify it matches.
// (Admin preview never reaches here — it returns above — so drafts stay
// filtered for every real gallery-token request.)
if (requestedSlug) {
// Verify by slug and ensure it matches the token's event
const adminPreviewToken = isAdminPreview(req);
event = await withRetry(async () => {
const q = db('events')
.where({
slug: requestedSlug,
is_active: formatBoolean(true),
is_archived: formatBoolean(false)
});
if (!adminPreviewToken) {
q.where({ is_draft: formatBoolean(false) });
}
return await q.select('*').first();
});
event = await withRetry(async () => db('events')
.where({
slug: requestedSlug,
is_active: formatBoolean(true),
is_archived: formatBoolean(false),
is_draft: formatBoolean(false)
})
.select('*').first());
// Verify the token's eventId matches
if (event && event.id !== decoded.eventId) {
return res.status(403).json({ error: 'Token does not match requested gallery' });
}
} else {
// Fallback to using eventId from token
const adminPreviewFallback = isAdminPreview(req);
event = await withRetry(async () => {
const q = db('events')
.where({
id: decoded.eventId,
is_active: formatBoolean(true),
is_archived: formatBoolean(false)
});
if (!adminPreviewFallback) {
q.where({ is_draft: formatBoolean(false) });
}
return await q.select('*').first();
});
event = await withRetry(async () => db('events')
.where({
id: decoded.eventId,
is_active: formatBoolean(true),
is_archived: formatBoolean(false),
is_draft: formatBoolean(false)
})
.select('*').first());
}
if (!event) {
backend/src/routes/gallery.js
+118 -78
View File
@@ -20,6 +20,10 @@ function resolveHeroLogoVisible(perEvent, globalDefault) {
const watermarkService = require('../services/watermarkService');
const watermarkGeneratorService = require('../services/watermarkGeneratorService');
const { verifyGalleryAccess, denySlideshowToken, isAdminPreview } = require('../middleware/gallery');
// Preserve the admin-preview flag across internal photo redirects (#981 review).
// The redirected request carries no gallery JWT, so without the flag it would
// fall back to the draft/password gate and 404 the derivative.
const withPreview = (req, url) => (req.isAdminPreview ? `${url}${url.includes('?') ? '&' : '?'}admin_preview=1` : url);
const { resolveGuest } = require('../middleware/guestAuth');
const { generateGuestIdentifier } = require('../middleware/feedbackRateLimit');
const secureImageService = require('../services/secureImageService');
@@ -265,8 +269,11 @@ router.get('/:slug/info', async (req, res) => {
return res.status(404).json({ error: 'Gallery has been archived and is no longer available' });
}
// Admin preview (#868) bypasses both the draft gate and — below — the
// password gate. Computed once and reused.
const adminPreview = isAdminPreview(req);
// Check if event is a draft (allow admin preview)
if (event.is_draft && !isAdminPreview(req)) {
if (event.is_draft && !adminPreview) {
return res.status(404).json({ error: 'Gallery is not yet published' });
}
@@ -278,7 +285,11 @@ router.get('/:slug/info', async (req, res) => {
}
}
const requiresPassword = !(event.require_password === false || event.require_password === 0 || event.require_password === '0');
// Admin preview skips the guest password on published, protected galleries
// (#868) — the admin already sees every photo through the admin routes.
const requiresPassword = adminPreview
? false
: !(event.require_password === false || event.require_password === 0 || event.require_password === '0');
const globalHeroLogoVisible = await getAppSetting('branding_logo_display_hero', true);
const globalLogoSize = await getAppSetting('branding_logo_size', 'medium');
@@ -820,7 +831,9 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
// refetches this list on every new-upload poll, which would massively
// inflate total_views / unique_visitors. The slideshow is explicitly
// excluded from real visitor analytics (migration 138 design).
if (req.accessLevel !== 'slideshow') {
// Admin preview (#868) is excluded from guest analytics + the "gallery
// opened" bell — it's the photographer looking at their own gallery.
if (req.accessLevel !== 'slideshow' && !req.isAdminPreview) {
await db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
@@ -914,8 +927,13 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
categories: categories,
photos: photos.map(photo => {
const useJwtUrl = (protectionSettings.protection_level === 'basic' || protectionSettings.protection_level === 'standard');
// Add watermark version to URLs for cache busting when settings change
const wmQuery = wmVersion ? `?${wmVersion}` : '';
// Watermark version (cache-busting) + admin-preview flag (#868). In
// preview mode no gallery cookie is minted, so each <img> request must
// re-assert the admin session — thread the flag onto every /api/gallery
// image URL so the browser sends it (the admin_token cookie rides along
// same-origin).
const imgQuery = [wmVersion, req.isAdminPreview ? 'admin_preview=1' : ''].filter(Boolean).join('&');
const wmQuery = imgQuery ? `?${imgQuery}` : '';
const photoUrl = useJwtUrl ?
`/api/gallery/${req.params.slug}/photo/${photo.id}${wmQuery}` :
`/api/secure-images/${req.params.slug}/secure/${photo.id}/{{token}}`;
@@ -1092,23 +1110,27 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,
}
}
// Update download count
await db('photos').where('id', photoId).increment('download_count', 1);
// Log download
await db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download',
photo_id: photoId
});
// Admin preview (#868) downloads are excluded from the download count +
// guest analytics — kept out of client-facing stats.
if (!req.isAdminPreview) {
// Update download count
await db('photos').where('id', photoId).increment('download_count', 1);
// Log download
await db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download',
photo_id: photoId
});
}
// Surface in the admin notification bell (#746) — debounced, and only
// once the response actually finished: notifying up-front would log a
// download that then 404s/fails and the debounce would suppress the
// next real one for an hour (codex review of #849).
res.on('finish', () => {
if (res.statusCode < 400) notifySinglePhotoDownload(req.event, req);
if (res.statusCode < 400 && !req.isAdminPreview) notifySinglePhotoDownload(req.event, req);
});
let filePath;
@@ -1231,15 +1253,18 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
if (wantsPresigned && storage.kind() === 's3' && !watermarkOnEvent) {
try {
const url = await storage.signedUrl(zipInfo.key, 300); // 5 min
db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download_all_presigned'
}).catch(() => {});
bumpEventDownloadCounts(req.event.id).catch(() => {});
// Surface in the admin notification bell (#746).
logActivity('gallery_downloaded', { scope: 'all' }, req.event.id, galleryActor(req));
// Admin preview (#868): stream the ZIP but keep it out of stats.
if (!req.isAdminPreview) {
db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download_all_presigned'
}).catch(() => {});
bumpEventDownloadCounts(req.event.id).catch(() => {});
// Surface in the admin notification bell (#746).
logActivity('gallery_downloaded', { scope: 'all' }, req.event.id, galleryActor(req));
}
res.redirect(302, url);
return;
} catch (err) {
@@ -1256,20 +1281,22 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
const stream = await storage.get(zipInfo.key);
stream.pipe(res);
// Log bulk download
db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download_all'
}).catch(() => {});
bumpEventDownloadCounts(req.event.id).catch(() => {});
// Surface in the admin notification bell (#746) — only once the
// stream actually finished; logging at pipe-time would report
// downloads that then broke mid-transfer (codex review of #849).
res.on('finish', () => {
if (res.statusCode < 400) logActivity('gallery_downloaded', { scope: 'all' }, req.event.id, galleryActor(req));
});
// Log bulk download (admin preview #868 excluded — stats stay client-only).
if (!req.isAdminPreview) {
db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download_all'
}).catch(() => {});
bumpEventDownloadCounts(req.event.id).catch(() => {});
// Surface in the admin notification bell (#746) — only once the
// stream actually finished; logging at pipe-time would report
// downloads that then broke mid-transfer (codex review of #849).
res.on('finish', () => {
if (res.statusCode < 400) logActivity('gallery_downloaded', { scope: 'all' }, req.event.id, galleryActor(req));
});
}
return;
}
@@ -1405,23 +1432,28 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
// Notification only after the response actually finished — finalize()
// ends Archiver's input, not the HTTP transfer (codex review of #849,
// confirmation round). Registered before finalize so it can't be missed.
res.on('finish', () => {
if (res.statusCode < 400) logActivity('gallery_downloaded', { scope: 'all' }, req.event.id, galleryActor(req));
});
// Admin preview (#868) streams the archive but is excluded from stats.
if (!req.isAdminPreview) {
res.on('finish', () => {
if (res.statusCode < 400) logActivity('gallery_downloaded', { scope: 'all' }, req.event.id, galleryActor(req));
});
}
await archive.finalize();
// Log bulk download
await db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download_all'
});
// Exactly the photos that made it into this archive (#895) — skipped
// (missing/corrupt) sources don't count.
if (appendedIds.length > 0) {
db('photos').whereIn('id', appendedIds)
.increment('download_count', 1).catch(() => {});
if (!req.isAdminPreview) {
// Log bulk download
await db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download_all'
});
// Exactly the photos that made it into this archive (#895) — skipped
// (missing/corrupt) sources don't count.
if (appendedIds.length > 0) {
db('photos').whereIn('id', appendedIds)
.increment('download_count', 1).catch(() => {});
}
}
} catch (error) {
errorResponse(res, error, 500, 'Failed to create download archive');
@@ -1552,22 +1584,27 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
}
// See download-all: notify only on response 'finish'.
res.on('finish', () => {
if (res.statusCode < 400) logActivity('gallery_downloaded', { scope: 'selected', photo_count: photoIds.length }, req.event.id, galleryActor(req));
});
// Admin preview (#868) streams the archive but is excluded from stats.
if (!req.isAdminPreview) {
res.on('finish', () => {
if (res.statusCode < 400) logActivity('gallery_downloaded', { scope: 'selected', photo_count: photoIds.length }, req.event.id, galleryActor(req));
});
}
await archive.finalize();
await db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download_selected'
});
// Exactly the photos that made it into this archive (#895) — skipped
// (missing/corrupt) sources don't count.
if (appendedIds.length > 0) {
db('photos').whereIn('id', appendedIds)
.increment('download_count', 1).catch(() => {});
if (!req.isAdminPreview) {
await db('access_logs').insert({
event_id: req.event.id,
ip_address: req.ip,
user_agent: req.headers['user-agent'],
action: 'download_selected'
});
// Exactly the photos that made it into this archive (#895) — skipped
// (missing/corrupt) sources don't count.
if (appendedIds.length > 0) {
db('photos').whereIn('id', appendedIds)
.increment('download_count', 1).catch(() => {});
}
}
} catch (error) {
errorResponse(res, error, 500, 'Failed to download selected photos');
@@ -1600,7 +1637,10 @@ router.post('/:slug/photo/:photoId/view',
if (photo.visibility === 'hidden' && req.accessLevel !== 'client') {
return res.status(403).json({ error: 'Photo not available' });
}
await db('photos').where('id', photo.id).increment('view_count', 1);
// Admin preview (#981 review) is excluded from per-photo view analytics.
if (!req.isAdminPreview) {
await db('photos').where('id', photo.id).increment('view_count', 1);
}
res.status(204).end();
} catch (error) {
errorResponse(res, error, 500, 'Failed to record view');
@@ -1959,7 +1999,7 @@ router.get('/:slug/hero/:photoId',
const isVideo = photo.media_type === 'video' || (photo.mime_type && photo.mime_type.startsWith('video/'));
if (isVideo) {
// For videos, redirect to the regular photo endpoint
return res.redirect(`/api/gallery/${req.params.slug}/photo/${photoId}`);
return res.redirect(withPreview(req, `/api/gallery/${req.params.slug}/photo/${photoId}`));
}
// Ensure hero image exists and is valid, regenerate if needed
@@ -1968,7 +2008,7 @@ router.get('/:slug/hero/:photoId',
if (!heroPath) {
// If hero generation fails, fall back to original photo
logger.warn(`Failed to generate hero image for photo ${photoId}, falling back to original`);
return res.redirect(`/api/gallery/${req.params.slug}/photo/${photoId}`);
return res.redirect(withPreview(req, `/api/gallery/${req.params.slug}/photo/${photoId}`));
}
// Hero images are always written via the storage abstraction (see
@@ -1983,7 +2023,7 @@ router.get('/:slug/hero/:photoId',
eventId: req.event.id,
heroPath
});
return res.redirect(`/api/gallery/${req.params.slug}/photo/${photoId}`);
return res.redirect(withPreview(req, `/api/gallery/${req.params.slug}/photo/${photoId}`));
}
const mtimeMs = stat.mtime ? stat.mtime.getTime() : 0;
@@ -2022,7 +2062,7 @@ router.get('/:slug/hero/:photoId',
eventId: req.event?.id
});
// Fall back to original photo on any error
res.redirect(`/api/gallery/${req.params.slug}/photo/${req.params.photoId}`);
res.redirect(withPreview(req, `/api/gallery/${req.params.slug}/photo/${req.params.photoId}`));
}
}
);
@@ -2059,7 +2099,7 @@ router.get('/:slug/preview/:photoId',
// belt-and-braces in case a stale tab does.
const isVideo = photo.media_type === 'video' || (photo.mime_type && photo.mime_type.startsWith('video/'));
if (isVideo) {
return res.redirect(`/api/gallery/${req.params.slug}/photo/${photoId}`);
return res.redirect(withPreview(req, `/api/gallery/${req.params.slug}/photo/${photoId}`));
}
// Lazy generation: ensurePreviewImage returns null on any
@@ -2068,7 +2108,7 @@ router.get('/:slug/preview/:photoId',
const previewPath = await ensurePreviewImage(photo);
if (!previewPath) {
logger.warn(`Failed to generate preview for photo ${photoId}, falling back to original`);
return res.redirect(`/api/gallery/${req.params.slug}/photo/${photoId}`);
return res.redirect(withPreview(req, `/api/gallery/${req.params.slug}/photo/${photoId}`));
}
const storage = getStorage();
@@ -2077,7 +2117,7 @@ router.get('/:slug/preview/:photoId',
logger.error('Preview file does not exist in storage backend', {
slug: req.params.slug, photoId, eventId: req.event.id, previewPath,
});
return res.redirect(`/api/gallery/${req.params.slug}/photo/${photoId}`);
return res.redirect(withPreview(req, `/api/gallery/${req.params.slug}/photo/${photoId}`));
}
const mtimeMs = stat.mtime ? stat.mtime.getTime() : 0;
@@ -2118,7 +2158,7 @@ router.get('/:slug/preview/:photoId',
photoId: req.params.photoId,
eventId: req.event?.id,
});
res.redirect(`/api/gallery/${req.params.slug}/photo/${req.params.photoId}`);
res.redirect(withPreview(req, `/api/gallery/${req.params.slug}/photo/${req.params.photoId}`));
}
}
);