diff --git a/backend/__tests__/routes/hiddenPhotoAccessControl.test.js b/backend/__tests__/routes/hiddenPhotoAccessControl.test.js new file mode 100644 index 00000000..5cb6733f --- /dev/null +++ b/backend/__tests__/routes/hiddenPhotoAccessControl.test.js @@ -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: 'host@example.com', + admin_email: 'admin@example.com', + 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); + }); + }); +}); diff --git a/backend/__tests__/utils/photoVisibility.test.js b/backend/__tests__/utils/photoVisibility.test.js new file mode 100644 index 00000000..287f03ed --- /dev/null +++ b/backend/__tests__/utils/photoVisibility.test.js @@ -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); + }); +}); diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 2d34b24d..7c4ffe87 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -757,6 +757,16 @@ router.patch('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.e .where({ id: photoId, event_id: eventId }) .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 const updatedPhoto = await db('photos') .where({ id: photoId, event_id: eventId }) @@ -905,6 +915,14 @@ router.post('/:eventId/photos/bulk-update', adminAuth, requirePermission('photos .where('event_id', eventId) .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` }); } catch (error) { errorResponse(res, error, 500, 'Failed to update photos'); diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 2b08a304..7263bda8 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -33,6 +33,7 @@ const { toIso } = require('../utils/dateNormalize'); const { NotFoundError } = require('../utils/errors'); const { ensureThumbnail, ensureHeroImage, ensurePreviewImage, withLocalCopy } = require('../services/imageProcessor'); const downloadZipService = require('../services/downloadZipService'); +const { applyPhotoVisibilityFilter, canSeeHiddenPhotos } = require('../utils/photoVisibility'); const { getUseOriginalFilenames, pickRawDownloadName, @@ -1002,6 +1003,10 @@ router.patch('/:slug/photos/:photoId/visibility', verifyGalleryAccess, async (re .where({ id: photoId, event_id: req.event.id }) .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 }); } catch (error) { 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) .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 }); } catch (error) { 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' }); } - // Try to serve pre-generated zip (instant download with Content-Length) - const zipInfo = await downloadZipService.getZipInfo(req.event.id); + // Try to serve pre-generated zip (instant download with Content-Length). + // 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) { const storage = getStorage(); @@ -1240,23 +1262,31 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block return; } - // Fallback: on-the-fly streaming (existing behavior) - // Also trigger background zip generation for next time - downloadZipService.generateZip(req.event.id).catch(err => - logger.warn('Background zip generation failed', { eventId: req.event.id, error: err.message }) - ); + // Fallback: on-the-fly streaming (existing behavior). Only pre-build the + // guest cache when it will actually be served next time — a guest + // download of an event with no hidden photos. Client bypasses and + // 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). // Uncategorised photos are always included; categories without the column // (pre-migration-135) fall through the LEFT JOIN's null and are included. - const photos = await db('photos') - .leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id') - .where('photos.event_id', req.event.id) - .where(function () { - this.whereNull('photos.category_id') - .orWhere('photo_categories.allow_downloads', true) - .orWhereNull('photo_categories.allow_downloads'); - }) + const photos = await applyPhotoVisibilityFilter( + db('photos') + .leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id') + .where('photos.event_id', req.event.id) + .where(function () { + this.whereNull('photos.category_id') + .orWhere('photo_categories.allow_downloads', true) + .orWhereNull('photo_categories.allow_downloads'); + }), + req.accessLevel + ) .select('photos.*') .orderBy('photos.type', 'asc') .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). // Same LEFT JOIN pattern as the download-all endpoint. - const photos = await db('photos') - .leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id') - .where('photos.event_id', req.event.id) - .whereIn('photos.id', photoIds) - .where(function () { - this.whereNull('photos.category_id') - .orWhere('photo_categories.allow_downloads', true) - .orWhereNull('photo_categories.allow_downloads'); - }) + const photos = await applyPhotoVisibilityFilter( + db('photos') + .leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id') + .where('photos.event_id', req.event.id) + .whereIn('photos.id', photoIds) + .where(function () { + this.whereNull('photos.category_id') + .orWhere('photo_categories.allow_downloads', true) + .orWhereNull('photo_categories.allow_downloads'); + }), + req.accessLevel + ) .select('photos.*') .orderBy('photos.uploaded_at', 'desc'); diff --git a/backend/src/routes/protectedImages.js b/backend/src/routes/protectedImages.js index 2069920a..5f48aa59 100644 --- a/backend/src/routes/protectedImages.js +++ b/backend/src/routes/protectedImages.js @@ -8,6 +8,7 @@ const secureImageService = require('../services/secureImageService'); const { getStorage } = require('../services/storage'); const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('../services/photoResolver'); const { withLocalCopy } = require('../services/imageProcessor'); +const { isPhotoHiddenFromViewer, canSeeHiddenPhotos } = require('../utils/photoVisibility'); const crypto = require('crypto'); const logger = require('../utils/logger'); const { timingSafeEqualStr } = require('../utils/timingSafe'); @@ -17,13 +18,16 @@ const router = express.Router(); /** * 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 expires = Date.now() + (expiresIn * 1000); // Third segment (#838): whether the minting context bypasses reveal mode - // (slideshow/client/admin). Old two-segment tokens verify unchanged and - // read as no-bypass. - const data = `${photoId}:${expires}:${revealBypass ? 1 : 0}`; + // (slideshow/client/admin). Fourth segment: whether the minter was a + // PIN-client, allowing the serve route to still deliver a photo that was + // 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'); return `${Buffer.from(data).toString('base64')}.${signature}`; } @@ -36,20 +40,25 @@ function verifyImageToken(token) { const secret = process.env.JWT_SECRET; const [data, signature] = token.split('.'); 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) const expectedSignature = crypto.createHmac('sha256', secret).update(decoded).digest('hex'); if (!timingSafeEqualStr(signature, expectedSignature)) { return null; } - + // Check expiration if (Date.now() > parseInt(expires)) { 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) { return null; } @@ -83,6 +92,12 @@ router.get('/:slug/photo/:photoId/view', verifyGalleryAccess, blockHiddenGallery 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 const isSuspicious = await secureImageService.detectSuspiciousActivity(clientFingerprint, photoId); if (isSuspicious) { @@ -195,15 +210,23 @@ router.post('/:slug/photo/:photoId/generate-secure-token', verifyGalleryAccess, 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 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', { expiresIn, maxUses: protectionLevel === 'maximum' ? 1 : 3, clientFingerprint, - protectionLevel + protectionLevel, + clientBypass: canSeeHiddenPhotos(req.accessLevel) }); res.json({ @@ -242,9 +265,19 @@ router.post('/:slug/photo/:photoId/generate-url', verifyGalleryAccess, async (re if (!photo) { return res.status(404).json({ error: 'Photo not found' }); } - - // Generate signed token - const token = generateImageToken(photoId, 3600, bypassesReveal(req)); + + // Refuse to mint a signed URL for a hidden/client-only photo when the + // 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}`; res.json({ @@ -298,7 +331,14 @@ router.get('/:slug/photo/:photoId/signed/:token', async (req, res) => { if (!photo) { 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 const watermarkSettings = await watermarkService.getWatermarkSettings(); diff --git a/backend/src/routes/secureImages.js b/backend/src/routes/secureImages.js index 91acb476..820251fe 100644 --- a/backend/src/routes/secureImages.js +++ b/backend/src/routes/secureImages.js @@ -14,6 +14,7 @@ const { pickRawDownloadName, } = require('../services/downloadFilenameService'); const { buildContentDisposition } = require('../utils/filenameSanitizer'); +const { isPhotoHiddenFromViewer, canSeeHiddenPhotos } = require('../utils/photoVisibility'); 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' }); } + // 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 const clientFingerprint = secureImageService.createClientFingerprint(req); @@ -55,7 +62,10 @@ router.post('/:slug/generate-token', async (req, res, next) => { protectionLevel, // Reveal mode (#838): recorded in the token so a re-hide invalidates // 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( @@ -184,6 +194,13 @@ router.get('/:slug/secure/:photoId/:token', 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 // path (external reference mode). secureImageService needs a local file, // 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' }); } + // 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). const storageKey = resolvePhotoStorageKey(req.event, photo); diff --git a/backend/src/services/downloadZipService.js b/backend/src/services/downloadZipService.js index c8891023..92503c8a 100644 --- a/backend/src/services/downloadZipService.js +++ b/backend/src/services/downloadZipService.js @@ -112,8 +112,15 @@ class DownloadZipService { const event = await db('events').where({ id: eventId }).first(); 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') .where({ event_id: eventId }) + .where(function () { + this.where('visibility', 'visible').orWhereNull('visibility'); + }) .select('*') .orderBy('type', 'asc') .orderBy('uploaded_at', 'desc'); diff --git a/backend/src/services/secureImageService.js b/backend/src/services/secureImageService.js index 4e7a7ae9..c7224ec9 100644 --- a/backend/src/services/secureImageService.js +++ b/backend/src/services/secureImageService.js @@ -25,7 +25,11 @@ class SecureImageService { // Reveal mode (#838): whether the minting context bypasses the // hidden-gallery gate — re-checked at SERVE time so a re-hide // 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; const tokenData = { @@ -37,6 +41,7 @@ class SecureImageService { usedCount: 0, protectionLevel, revealBypass, + clientBypass, createdAt: Date.now() }; diff --git a/backend/src/utils/photoVisibility.js b/backend/src/utils/photoVisibility.js new file mode 100644 index 00000000..7fa66366 --- /dev/null +++ b/backend/src/utils/photoVisibility.js @@ -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, +};