From e36b3309ca66404d189d5b218cc1f0eba925e4c7 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sun, 21 Jun 2026 02:44:12 +0200 Subject: [PATCH] fix(slideshow): deny display-only token on download/upload/feedback (PR #646 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The slideshow JWT reuses type:'gallery', so verifyGalleryAccess accepts it on every gallery route — a leaked projector link could download (single/all/ selected), upload (when allow_user_uploads), or post feedback for up to ~12h, beyond its display-only contract. Add a `denySlideshowToken` middleware (403 when req.accessLevel==='slideshow') after verifyGalleryAccess on those 5 routes. The photo-display routes (/photos, photo/thumbnail/preview/hero) stay open — the kiosk needs them. +4 tests mint a real slideshow JWT and assert 403. Docs note that Regenerate/Disable isn't instant revocation (~12h) and the feature flag is the hard cut-off. --- .../__tests__/routes/slideshowPublic.test.js | 51 ++++++++++++++++++- backend/src/middleware/gallery.js | 18 +++++++ backend/src/routes/gallery.js | 10 ++-- backend/src/routes/galleryFeedback.js | 3 +- docs/live-slideshow.md | 3 +- 5 files changed, 76 insertions(+), 9 deletions(-) diff --git a/backend/__tests__/routes/slideshowPublic.test.js b/backend/__tests__/routes/slideshowPublic.test.js index 5d13fbe5..1d3e8666 100644 --- a/backend/__tests__/routes/slideshowPublic.test.js +++ b/backend/__tests__/routes/slideshowPublic.test.js @@ -20,8 +20,10 @@ process.env.TEST_DATABASE_PATH = path.join( ); process.env.JWT_SECRET = process.env.JWT_SECRET || 'slideshow-test-secret'; +const express = require('express'); +const cookieParser = require('cookie-parser'); const request = require('supertest'); -const { bootCrmDb, seedMinimal, buildRouteApp } = require('../integration/helpers/crmDb'); +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); const { invalidateFeatureFlagCache } = require('../../src/middleware/requireFeatureFlag'); const SLUG = 'wedding-test'; @@ -67,7 +69,17 @@ describe('public Live Slideshow routes', () => { beforeAll(async () => { ({ db, cleanup } = await bootCrmDb()); await seedMinimal(db); - app = buildRouteApp('/api/gallery', require('../../src/routes/gallery')); + app = express(); + app.use(express.json()); + app.use(cookieParser()); + // Both routers mount under /api/gallery in production; the display-only + // guard lives on download routes (gallery) + the feedback POST (galleryFeedback). + app.use('/api/gallery', require('../../src/routes/gallery')); + app.use('/api/gallery', require('../../src/routes/galleryFeedback')); + // eslint-disable-next-line no-unused-vars + app.use((err, req, res, next) => { + res.status(err.statusCode || err.status || 500).json({ error: err.message, code: err.code }); + }); }); afterAll(async () => { await cleanup(); }); @@ -209,6 +221,41 @@ describe('public Live Slideshow routes', () => { }); }); + describe('display-only token guards (#646 review concern 1)', () => { + // Mint a real slideshow JWT, then prove it is denied on the + // download / upload / feedback routes (display-only contract). + async function slideshowJwt() { + await insertEvent(db); + const res = await request(app).get(`/api/gallery/${SLUG}/show/${TOKEN}/session`); + expect(res.status).toBe(200); + return res.body.token; + } + + it('403 on whole-gallery download', async () => { + const jwt = await slideshowJwt(); + const res = await request(app).get(`/api/gallery/${SLUG}/download-all`).set('Authorization', `Bearer ${jwt}`); + expect(res.status).toBe(403); + }); + + it('403 on single-photo download', async () => { + const jwt = await slideshowJwt(); + const res = await request(app).get(`/api/gallery/${SLUG}/download/1`).set('Authorization', `Bearer ${jwt}`); + expect(res.status).toBe(403); + }); + + it('403 on bulk download-selected', async () => { + const jwt = await slideshowJwt(); + const res = await request(app).post(`/api/gallery/${SLUG}/download-selected`).set('Authorization', `Bearer ${jwt}`).send({ photoIds: [1] }); + expect(res.status).toBe(403); + }); + + it('403 on feedback POST', async () => { + const jwt = await slideshowJwt(); + const res = await request(app).post(`/api/gallery/${SLUG}/photos/1/feedback`).set('Authorization', `Bearer ${jwt}`).send({ feedback_type: 'like' }); + expect(res.status).toBe(403); + }); + }); + describe('GET /session', () => { it('mints a token + sets the gallery cookie on a valid link', async () => { await insertEvent(db); diff --git a/backend/src/middleware/gallery.js b/backend/src/middleware/gallery.js index f22051ab..8d4ccf56 100644 --- a/backend/src/middleware/gallery.js +++ b/backend/src/middleware/gallery.js @@ -171,7 +171,25 @@ async function verifyGalleryAccess(req, res, next) { } } +/** + * Deny a slideshow-scoped JWT. The Live Slideshow token (accessLevel + * 'slideshow') is reused as a `type:'gallery'` token so it can read photos for + * the kiosk, which means every verifyGalleryAccess-protected route would + * otherwise accept it. A projector URL is meant to be display-only and is + * comparatively easy to leak (browser history, venue laptop, USB), so this + * gate is placed AFTER verifyGalleryAccess on the write/bulk-download routes to + * keep a leaked slideshow link from downloading, uploading, or posting + * feedback. (#646 review) + */ +function denySlideshowToken(req, res, next) { + if (req.accessLevel === 'slideshow') { + return res.status(403).json({ error: 'Slideshow tokens are display-only' }); + } + next(); +} + module.exports = { verifyGalleryAccess, + denySlideshowToken, isAdminPreview }; diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 89533e4d..a678ec22 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -7,7 +7,7 @@ const path = require('path'); const router = express.Router(); const watermarkService = require('../services/watermarkService'); const watermarkGeneratorService = require('../services/watermarkGeneratorService'); -const { verifyGalleryAccess, isAdminPreview } = require('../middleware/gallery'); +const { verifyGalleryAccess, denySlideshowToken, isAdminPreview } = require('../middleware/gallery'); const { resolveGuest } = require('../middleware/guestAuth'); const { generateGuestIdentifier } = require('../middleware/feedbackRateLimit'); const secureImageService = require('../services/secureImageService'); @@ -807,7 +807,7 @@ router.patch('/:slug/photos/visibility/bulk', verifyGalleryAccess, async (req, r }); // Download single photo -router.get('/:slug/download/:photoId', verifyGalleryAccess, async (req, res) => { +router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken, async (req, res) => { try { const { photoId } = req.params; @@ -927,7 +927,7 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, async (req, res) => }); // Download all photos as ZIP -router.get('/:slug/download-all', verifyGalleryAccess, async (req, res) => { +router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async (req, res) => { try { // Check if downloads are allowed for this event if (req.event.allow_downloads === false) { @@ -1103,7 +1103,7 @@ router.get('/:slug/download-all', verifyGalleryAccess, async (req, res) => { }); // Download selected photos as ZIP -router.post('/:slug/download-selected', verifyGalleryAccess, async (req, res) => { +router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken, async (req, res) => { try { // Check if downloads are allowed for this event if (req.event.allow_downloads === false) { @@ -1804,7 +1804,7 @@ router.get('/:slug/stats', verifyGalleryAccess, async (req, res) => { }); // User photo upload endpoint -router.post('/:eventId/upload', verifyGalleryAccess, async (req, res) => { +router.post('/:eventId/upload', verifyGalleryAccess, denySlideshowToken, async (req, res) => { try { const eventId = parseInt(req.params.eventId); diff --git a/backend/src/routes/galleryFeedback.js b/backend/src/routes/galleryFeedback.js index b8028509..10600999 100644 --- a/backend/src/routes/galleryFeedback.js +++ b/backend/src/routes/galleryFeedback.js @@ -1,7 +1,7 @@ const express = require('express'); const router = express.Router(); const { photoAuth } = require('../middleware/photoAuth'); -const { verifyGalleryAccess } = require('../middleware/gallery'); +const { verifyGalleryAccess, denySlideshowToken } = require('../middleware/gallery'); const { feedbackRateLimit, generateGuestIdentifier } = require('../middleware/feedbackRateLimit'); const { resolveGuest } = require('../middleware/guestAuth'); const feedbackService = require('../services/feedbackService'); @@ -140,6 +140,7 @@ router.get('/:slug/photos/:photoId/feedback', // Submit feedback for a photo router.post('/:slug/photos/:photoId/feedback', verifyGalleryAccess, + denySlideshowToken, resolveGuest, validatePhotoId, validateFeedbackSubmission, diff --git a/docs/live-slideshow.md b/docs/live-slideshow.md index c8584c5c..5cafad07 100644 --- a/docs/live-slideshow.md +++ b/docs/live-slideshow.md @@ -67,7 +67,8 @@ While a slideshow is running it polls a lightweight endpoint every few seconds: ## Good to know -- **The token is the secret.** Anyone with the link can view the slideshow (published photos only). Rotate it with **Regenerate** if it leaks; **Disable** removes it entirely. +- **The token is the secret.** Anyone with the link can view the slideshow — **published photos only**: a slideshow link is display-only and cannot download, upload, or post feedback. Rotate it with **Regenerate** if it leaks; **Disable** removes it entirely. +- **Regenerate / Disable is not instant revocation.** A running projector stops within one poll, but the browser session it already opened keeps working for **up to 12 hours** on the old token (there's no token-revocation list — same as gallery passwords). For a hard cut-off, also turn the **Live Slideshow** feature flag off, which denies every link immediately. - **Fullscreen needs the first click.** The ▶ splash exists because browsers require a user gesture to enter fullscreen — unavoidable, and harmless for a projector. - **Slideshow views don't pollute analytics.** The projector is excluded from your event's visitor view/download counts. - **Turning the feature off suspends, doesn't destroy.** Existing links stop working while the flag is off and resume when you turn it back on — the token isn't deleted.