fix(security): block guest access to hidden/client-only photos across bulk + secure routes (#939)

* fix(security): block guest access to hidden/client-only photos across bulk + secure routes

* fix(security): harden hidden-photo fix per review (stale ZIP cache, legacy token mint, SQLite bool, client rebuild)

* fix(security): invalidate ZIP cache on photo visibility/category change (codex r2)

* fix(security): recheck photo visibility at signed/secure serve time (TOCTOU) + invalidate ZIP on client visibility change (codex r3)

---------

Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
2026-08-01 17:36:15 +02:00
committed by GitHub
co-authored by Paul Nothaft
parent fe615c82e4
commit 8a87c9274b
9 changed files with 505 additions and 42 deletions
@@ -0,0 +1,240 @@
/**
* Hidden/client-only photo access control across the bulk + secure photo
* routes (GHSA cluster: fpwq / ghf8 / 3jvw / 9cc4 / 2hqg / jc22).
*
* A photo with visibility='hidden' is client-only. The main photo-list and
* single-photo download/view routes enforced this, but the bulk-download,
* protected-image, and secure-image routes shipped without the check —
* letting an ordinary guest reach hidden photos. These tests pin that
* guests are refused and PIN-clients (accessLevel='client') still succeed.
*/
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-hidden-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'hidden-photo-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-hidden-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const SLUG = 'hidden-photo-test-event';
describe('hidden-photo access control (GHSA cluster)', () => {
let db;
let cleanup;
let app;
let eventId;
let visibleId;
let hiddenId;
const guestToken = () => jwt.sign(
{ eventId, eventSlug: SLUG, type: 'gallery' },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' }
);
const clientToken = () => jwt.sign(
{ eventId, eventSlug: SLUG, type: 'gallery', accessLevel: 'client' },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' }
);
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const inserted = await db('events').insert({
slug: SLUG,
event_type: 'wedding',
event_name: 'Hidden Photo Test',
event_date: '2026-08-01',
host_email: '[email protected]',
admin_email: '[email protected]',
password_hash: 'x',
share_link: `/gallery/${SLUG}/share`,
share_token: 'hidden-photo-share',
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
is_active: 1, is_archived: 0, is_draft: 0, allow_downloads: 1,
created_at: new Date().toISOString(),
}).returning('id');
eventId = inserted[0]?.id ?? inserted[0];
const photoDir = path.join(process.env.STORAGE_PATH, 'events/active', SLUG);
fs.mkdirSync(photoDir, { recursive: true });
// A real 1x1 PNG so the protected /view route's Sharp processing path
// succeeds (fake bytes 500 on metadata()). Content, not extension,
// drives Sharp's format detection.
const PNG_1x1 = Buffer.from(
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M8AAAMCAQGV2rY9AAAAAElFTkSuQmCC',
'base64'
);
const mkPhoto = async (filename, visibility) => {
fs.writeFileSync(path.join(photoDir, filename), PNG_1x1);
const p = await db('photos').insert({
event_id: eventId,
filename,
path: `${SLUG}/${filename}`,
type: 'individual',
visibility,
uploaded_at: new Date().toISOString(),
}).returning('id');
return p[0]?.id ?? p[0];
};
visibleId = await mkPhoto('visible.jpg', 'visible');
hiddenId = await mkPhoto('hidden.jpg', 'hidden');
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/gallery', require('../../src/routes/gallery'));
app.use('/api/images', require('../../src/routes/protectedImages'));
app.use('/api/secure-images', require('../../src/routes/secureImages'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
describe('download-selected (GHSA-ghf8, medium)', () => {
it('omits a hidden photo for a guest even when its id is requested', async () => {
const res = await request(app)
.post(`/api/gallery/${SLUG}/download-selected`)
.set('Authorization', `Bearer ${guestToken()}`)
.send({ photo_ids: [visibleId, hiddenId] });
// The visible photo still zips; the hidden one is filtered out. If
// only the hidden id were requested, the filter empties the set → 404.
expect(res.status).toBe(200);
const solo = await request(app)
.post(`/api/gallery/${SLUG}/download-selected`)
.set('Authorization', `Bearer ${guestToken()}`)
.send({ photo_ids: [hiddenId] });
expect(solo.status).toBe(404);
});
it('includes the hidden photo for a client', async () => {
const res = await request(app)
.post(`/api/gallery/${SLUG}/download-selected`)
.set('Authorization', `Bearer ${clientToken()}`)
.send({ photo_ids: [hiddenId] });
expect(res.status).toBe(200);
});
});
describe('download-all (GHSA-fpwq, medium)', () => {
it('streams for a guest without erroring (hidden photos filtered)', async () => {
const res = await request(app)
.get(`/api/gallery/${SLUG}/download-all`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(200);
});
});
describe('protected-image view (GHSA-9cc4)', () => {
it('403s a hidden photo for a guest', async () => {
const res = await request(app)
.get(`/api/images/${SLUG}/photo/${hiddenId}/view`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(403);
});
it('serves a visible photo for a guest', async () => {
const res = await request(app)
.get(`/api/images/${SLUG}/photo/${visibleId}/view`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(200);
});
it('serves a hidden photo for a client', async () => {
const res = await request(app)
.get(`/api/images/${SLUG}/photo/${hiddenId}/view`)
.set('Authorization', `Bearer ${clientToken()}`);
expect(res.status).toBe(200);
});
});
describe('signed-URL mint (GHSA-3jvw)', () => {
it('403s minting a signed URL for a hidden photo as a guest', async () => {
const res = await request(app)
.post(`/api/images/${SLUG}/photo/${hiddenId}/generate-url`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(403);
});
it('mints for a client', async () => {
const res = await request(app)
.post(`/api/images/${SLUG}/photo/${hiddenId}/generate-url`)
.set('Authorization', `Bearer ${clientToken()}`);
expect(res.status).toBe(200);
expect(res.body.url).toContain('/signed/');
});
});
describe('legacy secure-token mint (protectedImages generate-secure-token)', () => {
it('403s a hidden photo for a guest', async () => {
const res = await request(app)
.post(`/api/images/${SLUG}/photo/${hiddenId}/generate-secure-token`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(403);
});
it('mints for a client', async () => {
const res = await request(app)
.post(`/api/images/${SLUG}/photo/${hiddenId}/generate-secure-token`)
.set('Authorization', `Bearer ${clientToken()}`);
expect(res.status).toBe(200);
expect(res.body.token).toBeDefined();
});
});
describe('secure-token mint (GHSA-2hqg)', () => {
it('403s minting a secure token for a hidden photo as a guest', async () => {
const res = await request(app)
.post(`/api/secure-images/${SLUG}/generate-token`)
.set('Authorization', `Bearer ${guestToken()}`)
.send({ photoId: hiddenId });
expect(res.status).toBe(403);
});
it('mints for a client', async () => {
const res = await request(app)
.post(`/api/secure-images/${SLUG}/generate-token`)
.set('Authorization', `Bearer ${clientToken()}`)
.send({ photoId: hiddenId });
expect(res.status).toBe(200);
expect(res.body.token).toBeDefined();
});
});
// A capability minted while a photo is visible must stop serving once the
// photo is hidden — unless minted by a client (clientBypass in the token).
describe('signed-URL TOCTOU (hidden AFTER minting)', () => {
afterEach(async () => {
await db('photos').where({ id: visibleId }).update({ visibility: 'visible' });
});
it("a guest's pre-minted signed URL stops serving once the photo is hidden", async () => {
const mint = await request(app)
.post(`/api/images/${SLUG}/photo/${visibleId}/generate-url`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(mint.status).toBe(200);
const url = mint.body.url;
// Still visible → serves.
expect((await request(app).get(url)).status).toBe(200);
// Hide it → the guest token (no clientBypass) must now be refused.
await db('photos').where({ id: visibleId }).update({ visibility: 'hidden' });
expect((await request(app).get(url)).status).toBe(403);
});
it("a client's pre-minted signed URL keeps serving after the photo is hidden", async () => {
const mint = await request(app)
.post(`/api/images/${SLUG}/photo/${visibleId}/generate-url`)
.set('Authorization', `Bearer ${clientToken()}`);
expect(mint.status).toBe(200);
const url = mint.body.url;
await db('photos').where({ id: visibleId }).update({ visibility: 'hidden' });
expect((await request(app).get(url)).status).toBe(200);
});
});
});
@@ -0,0 +1,40 @@
/**
* Unit tests for the shared hidden-photo access-control helper.
*
* Pins the rule that ordinary gallery guests never receive photos with
* visibility='hidden' (NULL = visible), while PIN-clients see everything.
*/
const {
canSeeHiddenPhotos,
isPhotoHiddenFromViewer,
} = require('../../src/utils/photoVisibility');
describe('canSeeHiddenPhotos', () => {
it('is true only for the client access level', () => {
expect(canSeeHiddenPhotos('client')).toBe(true);
expect(canSeeHiddenPhotos('guest')).toBe(false);
expect(canSeeHiddenPhotos('slideshow')).toBe(false);
expect(canSeeHiddenPhotos(undefined)).toBe(false);
});
});
describe('isPhotoHiddenFromViewer', () => {
it('blocks a hidden photo from guests', () => {
expect(isPhotoHiddenFromViewer({ visibility: 'hidden' }, 'guest')).toBe(true);
expect(isPhotoHiddenFromViewer({ visibility: 'hidden' }, 'slideshow')).toBe(true);
});
it('lets clients see hidden photos', () => {
expect(isPhotoHiddenFromViewer({ visibility: 'hidden' }, 'client')).toBe(false);
});
it('treats visible and NULL visibility as viewable by everyone', () => {
expect(isPhotoHiddenFromViewer({ visibility: 'visible' }, 'guest')).toBe(false);
expect(isPhotoHiddenFromViewer({ visibility: null }, 'guest')).toBe(false);
expect(isPhotoHiddenFromViewer({}, 'guest')).toBe(false);
});
it('is null-safe', () => {
expect(isPhotoHiddenFromViewer(null, 'guest')).toBe(false);
});
});
+18
View File
@@ -757,6 +757,16 @@ router.patch('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.e
.where({ id: photoId, event_id: eventId }) .where({ id: photoId, event_id: eventId })
.update(updateData); .update(updateData);
// A visibility or category change alters which photos belong in the
// guest download bundle — drop the cached ZIP so it rebuilds fresh,
// otherwise a hide→unhide cycle can leave the stale cache omitting
// photos added in between (codex review).
if (updateData.visibility !== undefined
|| Object.prototype.hasOwnProperty.call(updateData, 'category_id')
|| Object.prototype.hasOwnProperty.call(updateData, 'type')) {
downloadZipService.invalidate(parseInt(eventId, 10));
}
// Fetch and return the updated photo // Fetch and return the updated photo
const updatedPhoto = await db('photos') const updatedPhoto = await db('photos')
.where({ id: photoId, event_id: eventId }) .where({ id: photoId, event_id: eventId })
@@ -905,6 +915,14 @@ router.post('/:eventId/photos/bulk-update', adminAuth, requirePermission('photos
.where('event_id', eventId) .where('event_id', eventId)
.update(updateData); .update(updateData);
// Visibility/category changes alter the guest download bundle — drop the
// cached ZIP so it rebuilds fresh (codex review).
if (updateData.visibility !== undefined
|| Object.prototype.hasOwnProperty.call(updateData, 'category_id')
|| Object.prototype.hasOwnProperty.call(updateData, 'type')) {
downloadZipService.invalidate(parseInt(eventId, 10));
}
res.json({ message: `${photoIds.length} photos updated successfully` }); res.json({ message: `${photoIds.length} photos updated successfully` });
} catch (error) { } catch (error) {
errorResponse(res, error, 500, 'Failed to update photos'); errorResponse(res, error, 500, 'Failed to update photos');
+57 -24
View File
@@ -33,6 +33,7 @@ const { toIso } = require('../utils/dateNormalize');
const { NotFoundError } = require('../utils/errors'); const { NotFoundError } = require('../utils/errors');
const { ensureThumbnail, ensureHeroImage, ensurePreviewImage, withLocalCopy } = require('../services/imageProcessor'); const { ensureThumbnail, ensureHeroImage, ensurePreviewImage, withLocalCopy } = require('../services/imageProcessor');
const downloadZipService = require('../services/downloadZipService'); const downloadZipService = require('../services/downloadZipService');
const { applyPhotoVisibilityFilter, canSeeHiddenPhotos } = require('../utils/photoVisibility');
const { const {
getUseOriginalFilenames, getUseOriginalFilenames,
pickRawDownloadName, pickRawDownloadName,
@@ -1002,6 +1003,10 @@ router.patch('/:slug/photos/:photoId/visibility', verifyGalleryAccess, async (re
.where({ id: photoId, event_id: req.event.id }) .where({ id: photoId, event_id: req.event.id })
.update({ visibility }); .update({ visibility });
// A client hiding/showing a photo changes the guest download bundle —
// drop the cached ZIP so it rebuilds fresh (codex review).
downloadZipService.invalidate(req.event.id);
res.json({ message: 'Photo visibility updated', visibility }); res.json({ message: 'Photo visibility updated', visibility });
} catch (error) { } catch (error) {
errorResponse(res, error, 500, 'Failed to update photo visibility'); errorResponse(res, error, 500, 'Failed to update photo visibility');
@@ -1030,6 +1035,10 @@ router.patch('/:slug/photos/visibility/bulk', verifyGalleryAccess, async (req, r
.where('event_id', req.event.id) .where('event_id', req.event.id)
.update({ visibility }); .update({ visibility });
// Client bulk hide/show alters the guest download bundle — invalidate
// the cached ZIP (codex review).
downloadZipService.invalidate(req.event.id);
res.json({ message: `${count} photos updated`, visibility }); res.json({ message: `${count} photos updated`, visibility });
} catch (error) { } catch (error) {
errorResponse(res, error, 500, 'Failed to update photo visibility'); errorResponse(res, error, 500, 'Failed to update photo visibility');
@@ -1182,8 +1191,21 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
return res.status(403).json({ error: 'Downloads are disabled for this gallery' }); return res.status(403).json({ error: 'Downloads are disabled for this gallery' });
} }
// Try to serve pre-generated zip (instant download with Content-Length) // Try to serve pre-generated zip (instant download with Content-Length).
const zipInfo = await downloadZipService.getZipInfo(req.event.id); // Guests may use the prebuilt cache ONLY when the event has no hidden
// photos: a cache built before a photo was hidden — or before this
// visibility-aware builder shipped — could otherwise still leak it, and
// getZipInfo only checks the DB pointer + file stat, not freshness. When
// hidden photos exist, guests fall through to the visibility-filtered
// stream below. PIN-clients always stream a full archive.
const isClient = canSeeHiddenPhotos(req.accessLevel);
const eventHasHidden = await db('photos')
.where({ event_id: req.event.id, visibility: 'hidden' })
.first()
.then(Boolean);
const zipInfo = (isClient || eventHasHidden)
? null
: await downloadZipService.getZipInfo(req.event.id);
if (zipInfo) { if (zipInfo) {
const storage = getStorage(); const storage = getStorage();
@@ -1240,23 +1262,31 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
return; return;
} }
// Fallback: on-the-fly streaming (existing behavior) // Fallback: on-the-fly streaming (existing behavior). Only pre-build the
// Also trigger background zip generation for next time // guest cache when it will actually be served next time — a guest
downloadZipService.generateZip(req.event.id).catch(err => // download of an event with no hidden photos. Client bypasses and
logger.warn('Background zip generation failed', { eventId: req.event.id, error: err.message }) // hidden-photo events always stream, so rebuilding the guest archive on
); // those requests is wasted I/O (codex review).
if (!isClient && !eventHasHidden) {
downloadZipService.generateZip(req.event.id).catch(err =>
logger.warn('Background zip generation failed', { eventId: req.event.id, error: err.message })
);
}
// Fetch photos — exclude photos in categories that disabled downloads (#640). // Fetch photos — exclude photos in categories that disabled downloads (#640).
// Uncategorised photos are always included; categories without the column // Uncategorised photos are always included; categories without the column
// (pre-migration-135) fall through the LEFT JOIN's null and are included. // (pre-migration-135) fall through the LEFT JOIN's null and are included.
const photos = await db('photos') const photos = await applyPhotoVisibilityFilter(
.leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id') db('photos')
.where('photos.event_id', req.event.id) .leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id')
.where(function () { .where('photos.event_id', req.event.id)
this.whereNull('photos.category_id') .where(function () {
.orWhere('photo_categories.allow_downloads', true) this.whereNull('photos.category_id')
.orWhereNull('photo_categories.allow_downloads'); .orWhere('photo_categories.allow_downloads', true)
}) .orWhereNull('photo_categories.allow_downloads');
}),
req.accessLevel
)
.select('photos.*') .select('photos.*')
.orderBy('photos.type', 'asc') .orderBy('photos.type', 'asc')
.orderBy('photos.uploaded_at', 'desc'); .orderBy('photos.uploaded_at', 'desc');
@@ -1412,15 +1442,18 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
// Fetch photos — exclude photos in categories that disabled downloads (#640). // Fetch photos — exclude photos in categories that disabled downloads (#640).
// Same LEFT JOIN pattern as the download-all endpoint. // Same LEFT JOIN pattern as the download-all endpoint.
const photos = await db('photos') const photos = await applyPhotoVisibilityFilter(
.leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id') db('photos')
.where('photos.event_id', req.event.id) .leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id')
.whereIn('photos.id', photoIds) .where('photos.event_id', req.event.id)
.where(function () { .whereIn('photos.id', photoIds)
this.whereNull('photos.category_id') .where(function () {
.orWhere('photo_categories.allow_downloads', true) this.whereNull('photos.category_id')
.orWhereNull('photo_categories.allow_downloads'); .orWhere('photo_categories.allow_downloads', true)
}) .orWhereNull('photo_categories.allow_downloads');
}),
req.accessLevel
)
.select('photos.*') .select('photos.*')
.orderBy('photos.uploaded_at', 'desc'); .orderBy('photos.uploaded_at', 'desc');
+50 -10
View File
@@ -8,6 +8,7 @@ const secureImageService = require('../services/secureImageService');
const { getStorage } = require('../services/storage'); const { getStorage } = require('../services/storage');
const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('../services/photoResolver'); const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('../services/photoResolver');
const { withLocalCopy } = require('../services/imageProcessor'); const { withLocalCopy } = require('../services/imageProcessor');
const { isPhotoHiddenFromViewer, canSeeHiddenPhotos } = require('../utils/photoVisibility');
const crypto = require('crypto'); const crypto = require('crypto');
const logger = require('../utils/logger'); const logger = require('../utils/logger');
const { timingSafeEqualStr } = require('../utils/timingSafe'); const { timingSafeEqualStr } = require('../utils/timingSafe');
@@ -17,13 +18,16 @@ const router = express.Router();
/** /**
* Generate a signed URL token for image access * Generate a signed URL token for image access
*/ */
function generateImageToken(photoId, expiresIn = 3600, revealBypass = false) { function generateImageToken(photoId, expiresIn = 3600, revealBypass = false, clientBypass = false) {
const secret = process.env.JWT_SECRET; const secret = process.env.JWT_SECRET;
const expires = Date.now() + (expiresIn * 1000); const expires = Date.now() + (expiresIn * 1000);
// Third segment (#838): whether the minting context bypasses reveal mode // Third segment (#838): whether the minting context bypasses reveal mode
// (slideshow/client/admin). Old two-segment tokens verify unchanged and // (slideshow/client/admin). Fourth segment: whether the minter was a
// read as no-bypass. // PIN-client, allowing the serve route to still deliver a photo that was
const data = `${photoId}:${expires}:${revealBypass ? 1 : 0}`; // hidden AFTER minting (TOCTOU) — a guest's token carries 0, so it stops
// working the moment the photo is hidden. Old shorter tokens verify
// unchanged and read both flags as no-bypass.
const data = `${photoId}:${expires}:${revealBypass ? 1 : 0}:${clientBypass ? 1 : 0}`;
const signature = crypto.createHmac('sha256', secret).update(data).digest('hex'); const signature = crypto.createHmac('sha256', secret).update(data).digest('hex');
return `${Buffer.from(data).toString('base64')}.${signature}`; return `${Buffer.from(data).toString('base64')}.${signature}`;
} }
@@ -36,7 +40,7 @@ function verifyImageToken(token) {
const secret = process.env.JWT_SECRET; const secret = process.env.JWT_SECRET;
const [data, signature] = token.split('.'); const [data, signature] = token.split('.');
const decoded = Buffer.from(data, 'base64').toString(); const decoded = Buffer.from(data, 'base64').toString();
const [photoId, expires, bypassFlag] = decoded.split(':'); const [photoId, expires, bypassFlag, clientFlag] = decoded.split(':');
// Verify signature (constant-time — avoids leaking the HMAC byte-by-byte) // Verify signature (constant-time — avoids leaking the HMAC byte-by-byte)
const expectedSignature = crypto.createHmac('sha256', secret).update(decoded).digest('hex'); const expectedSignature = crypto.createHmac('sha256', secret).update(decoded).digest('hex');
@@ -49,7 +53,12 @@ function verifyImageToken(token) {
return null; return null;
} }
return { photoId: parseInt(photoId), expires: parseInt(expires), revealBypass: bypassFlag === '1' }; return {
photoId: parseInt(photoId),
expires: parseInt(expires),
revealBypass: bypassFlag === '1',
clientBypass: clientFlag === '1',
};
} catch (error) { } catch (error) {
return null; return null;
} }
@@ -83,6 +92,12 @@ router.get('/:slug/photo/:photoId/view', verifyGalleryAccess, blockHiddenGallery
return res.status(404).json({ error: 'Photo not found' }); return res.status(404).json({ error: 'Photo not found' });
} }
// Block guest access to hidden/client-only photos (parity with the
// gallery single-photo routes).
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Check for suspicious activity // Check for suspicious activity
const isSuspicious = await secureImageService.detectSuspiciousActivity(clientFingerprint, photoId); const isSuspicious = await secureImageService.detectSuspiciousActivity(clientFingerprint, photoId);
if (isSuspicious) { if (isSuspicious) {
@@ -195,15 +210,23 @@ router.post('/:slug/photo/:photoId/generate-secure-token', verifyGalleryAccess,
return res.status(404).json({ error: 'Photo not found' }); return res.status(404).json({ error: 'Photo not found' });
} }
// Don't mint a secure-image capability for a hidden/client-only photo
// when the caller isn't a client — the serve route is token-only.
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Create client fingerprint // Create client fingerprint
const clientFingerprint = secureImageService.createClientFingerprint(req); const clientFingerprint = secureImageService.createClientFingerprint(req);
// Generate secure token // Generate secure token. clientBypass lets a client's token keep serving
// a photo hidden after minting; a guest's stops at the serve route.
const token = secureImageService.generateSecureToken(photoId, req.sessionID || 'anonymous', { const token = secureImageService.generateSecureToken(photoId, req.sessionID || 'anonymous', {
expiresIn, expiresIn,
maxUses: protectionLevel === 'maximum' ? 1 : 3, maxUses: protectionLevel === 'maximum' ? 1 : 3,
clientFingerprint, clientFingerprint,
protectionLevel protectionLevel,
clientBypass: canSeeHiddenPhotos(req.accessLevel)
}); });
res.json({ res.json({
@@ -243,8 +266,18 @@ router.post('/:slug/photo/:photoId/generate-url', verifyGalleryAccess, async (re
return res.status(404).json({ error: 'Photo not found' }); return res.status(404).json({ error: 'Photo not found' });
} }
// Generate signed token // Refuse to mint a signed URL for a hidden/client-only photo when the
const token = generateImageToken(photoId, 3600, bypassesReveal(req)); // caller isn't a client. The signed-serve route below is token-only
// (no gallery auth), so the access decision has to happen here at mint
// time — mirroring how the reveal-bypass flag is baked into the token.
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Generate signed token. The client-bypass flag lets a PIN-client's
// token keep serving a photo hidden after minting; a guest's token
// (clientBypass=0) stops the moment the photo is hidden.
const token = generateImageToken(photoId, 3600, bypassesReveal(req), canSeeHiddenPhotos(req.accessLevel));
const signedUrl = `/api/images/${req.params.slug}/photo/${photoId}/signed/${token}`; const signedUrl = `/api/images/${req.params.slug}/photo/${photoId}/signed/${token}`;
res.json({ res.json({
@@ -299,6 +332,13 @@ router.get('/:slug/photo/:photoId/signed/:token', async (req, res) => {
return res.status(404).json({ error: 'Photo not found' }); return res.status(404).json({ error: 'Photo not found' });
} }
// Recheck visibility at serve time (TOCTOU): a photo hidden AFTER the
// URL was minted must stop serving, unless the token was minted by a
// client (clientBypass) — mirroring the reveal-mode check above.
if (photo.visibility === 'hidden' && !tokenData.clientBypass) {
return res.status(403).json({ error: 'Photo not available' });
}
// Get watermark settings // Get watermark settings
const watermarkSettings = await watermarkService.getWatermarkSettings(); const watermarkSettings = await watermarkService.getWatermarkSettings();
+35 -1
View File
@@ -14,6 +14,7 @@ const {
pickRawDownloadName, pickRawDownloadName,
} = require('../services/downloadFilenameService'); } = require('../services/downloadFilenameService');
const { buildContentDisposition } = require('../utils/filenameSanitizer'); const { buildContentDisposition } = require('../utils/filenameSanitizer');
const { isPhotoHiddenFromViewer, canSeeHiddenPhotos } = require('../utils/photoVisibility');
const router = express.Router(); const router = express.Router();
@@ -41,6 +42,12 @@ router.post('/:slug/generate-token', async (req, res, next) => {
return res.status(404).json({ error: 'Photo not found' }); return res.status(404).json({ error: 'Photo not found' });
} }
// Don't mint a secure-image capability for a hidden/client-only photo
// when the caller isn't a client (the token is reusable up to 3×).
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Create client fingerprint // Create client fingerprint
const clientFingerprint = secureImageService.createClientFingerprint(req); const clientFingerprint = secureImageService.createClientFingerprint(req);
@@ -55,7 +62,10 @@ router.post('/:slug/generate-token', async (req, res, next) => {
protectionLevel, protectionLevel,
// Reveal mode (#838): recorded in the token so a re-hide invalidates // Reveal mode (#838): recorded in the token so a re-hide invalidates
// in-flight guest tokens at serve time without breaking the slideshow. // in-flight guest tokens at serve time without breaking the slideshow.
revealBypass: bypassesReveal(req) revealBypass: bypassesReveal(req),
// TOCTOU: a client's token keeps serving a photo hidden after minting;
// a guest's stops the moment it's hidden (checked at the serve route).
clientBypass: canSeeHiddenPhotos(req.accessLevel)
}; };
const token = secureImageService.generateSecureToken( const token = secureImageService.generateSecureToken(
@@ -184,6 +194,13 @@ router.get('/:slug/secure/:photoId/:token',
return res.status(404).json({ error: 'Photo not found' }); return res.status(404).json({ error: 'Photo not found' });
} }
// Recheck visibility at serve time (TOCTOU): a photo hidden AFTER the
// token was minted must stop serving, unless the token was minted by a
// client (clientBypass) — mirroring the reveal-mode check above.
if (photo.visibility === 'hidden' && !tokenValidation.data?.clientBypass) {
return res.status(403).json({ error: 'Photo not available' });
}
// Resolve photo through storage backend (managed) or fall back to local // Resolve photo through storage backend (managed) or fall back to local
// path (external reference mode). secureImageService needs a local file, // path (external reference mode). secureImageService needs a local file,
// so we materialize a tmp copy via withLocalCopy in S3 mode. // so we materialize a tmp copy via withLocalCopy in S3 mode.
@@ -341,6 +358,23 @@ router.get('/:slug/secure-download/:photoId/:token',
return res.status(404).json({ error: 'Photo not found' }); return res.status(404).json({ error: 'Photo not found' });
} }
// Block guest access to hidden/client-only photos.
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Per-category download opt-out (#640) — the regular single-photo
// download enforces this too; the secure path skipped it. SQLite
// returns the boolean as numeric 0, so check both forms.
if (photo.category_id) {
const cat = await db('photo_categories')
.where('id', photo.category_id)
.first('allow_downloads');
if (cat && (cat.allow_downloads === false || cat.allow_downloads === 0)) {
return res.status(403).json({ error: 'Downloads are disabled for this category' });
}
}
// Resolve photo through storage backend (managed) or local disk (external). // Resolve photo through storage backend (managed) or local disk (external).
const storageKey = resolvePhotoStorageKey(req.event, photo); const storageKey = resolvePhotoStorageKey(req.event, photo);
@@ -112,8 +112,15 @@ class DownloadZipService {
const event = await db('events').where({ id: eventId }).first(); const event = await db('events').where({ id: eventId }).first();
if (!event) return { success: false, error: 'Event not found' }; if (!event) return { success: false, error: 'Event not found' };
// The prebuilt zip is served to ordinary gallery guests (the
// download-all fast path), so it must exclude hidden/client-only
// photos — NULL visibility counts as visible (pre-migration rows).
// PIN-clients bypass this cache and stream a full archive instead.
const photos = await db('photos') const photos = await db('photos')
.where({ event_id: eventId }) .where({ event_id: eventId })
.where(function () {
this.where('visibility', 'visible').orWhereNull('visibility');
})
.select('*') .select('*')
.orderBy('type', 'asc') .orderBy('type', 'asc')
.orderBy('uploaded_at', 'desc'); .orderBy('uploaded_at', 'desc');
+6 -1
View File
@@ -25,7 +25,11 @@ class SecureImageService {
// Reveal mode (#838): whether the minting context bypasses the // Reveal mode (#838): whether the minting context bypasses the
// hidden-gallery gate — re-checked at SERVE time so a re-hide // hidden-gallery gate — re-checked at SERVE time so a re-hide
// invalidates in-flight guest tokens without breaking the slideshow. // invalidates in-flight guest tokens without breaking the slideshow.
revealBypass = false revealBypass = false,
// Whether the minter was a PIN-client — lets the serve route keep
// delivering a photo hidden AFTER minting (TOCTOU). A guest's token
// carries false, so it stops the moment the photo is hidden.
clientBypass = false
} = options; } = options;
const tokenData = { const tokenData = {
@@ -37,6 +41,7 @@ class SecureImageService {
usedCount: 0, usedCount: 0,
protectionLevel, protectionLevel,
revealBypass, revealBypass,
clientBypass,
createdAt: Date.now() createdAt: Date.now()
}; };
+46
View File
@@ -0,0 +1,46 @@
/**
* Shared hidden-photo access control.
*
* PicPeak photos carry a `visibility` column: 'visible' (or NULL, for
* pre-migration rows) is shown to everyone; 'hidden' is client-only. A
* gallery viewer's `req.accessLevel` is 'client' for a PIN-client login and
* something else ('guest'/'slideshow'/…) for an ordinary guest.
*
* The main photo-list query and the single-photo download/view routes each
* enforced this inline, but several bulk/secure paths (download-all,
* download-selected, protected-image view, signed-URL mint, secure-token
* mint, secure-download) shipped without it — letting ordinary guests reach
* hidden/client-only photos. These helpers centralise the rule so every
* sink applies exactly the same predicate.
*/
// PIN-clients see hidden photos; everyone else does not.
function canSeeHiddenPhotos(accessLevel) {
return accessLevel === 'client';
}
/**
* Append the guest visibility filter to a knex `photos` query. No-op for
* clients. NULL visibility is treated as visible (pre-migration default).
* The query must reference the table as `photos` (all call sites do).
*/
function applyPhotoVisibilityFilter(query, accessLevel) {
if (canSeeHiddenPhotos(accessLevel)) return query;
return query.where(function () {
this.where('photos.visibility', 'visible').orWhereNull('photos.visibility');
});
}
/**
* Single-photo predicate: true when this photo must be blocked for a viewer
* at the given access level. Mirrors the inline guards in gallery.js.
*/
function isPhotoHiddenFromViewer(photo, accessLevel) {
return !!photo && photo.visibility === 'hidden' && !canSeeHiddenPhotos(accessLevel);
}
module.exports = {
canSeeHiddenPhotos,
applyPhotoVisibilityFilter,
isPhotoHiddenFromViewer,
};