From b869de33a5b5cf927d18776dfaece432480a5483 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:22:20 +0200 Subject: [PATCH] fix(backend): check the revocation store on gallery access (stable) (#1388) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(backend): check the revocation store on gallery access Gallery logout wrote to revoked_tokens correctly, but verifyGalleryAccess never read it back — a logged-out gallery JWT kept working until natural expiry. Admin auth already calls isTokenRevoked(); gallery was the outlier. Note: this gap was independently closed on main via a broader gallery- access refactor (PR #1357), so no main-branch fix is needed there — this is a stable-only backport of the same protection. * fix(backend): check revocation on the admin-preview gallery token too isAdminPreview() decoded the ?preview= admin JWT but never checked isTokenRevoked — a revoked admin session kept working via a preview link indefinitely. Same gap this branch already closed for the main gallery-token path (GHSA-q7f7-gjx8-mf6h), just in the sibling admin-preview check within the same file. --------- Co-authored-by: Paul Nothaft --- ...verifyGalleryAccess.customerRevoke.test.js | 153 +++++++++++++++++- backend/src/middleware/gallery.js | 31 +++- backend/src/routes/gallery.js | 2 +- 3 files changed, 179 insertions(+), 7 deletions(-) diff --git a/backend/src/__tests__/verifyGalleryAccess.customerRevoke.test.js b/backend/src/__tests__/verifyGalleryAccess.customerRevoke.test.js index 55302bbb..7129669e 100644 --- a/backend/src/__tests__/verifyGalleryAccess.customerRevoke.test.js +++ b/backend/src/__tests__/verifyGalleryAccess.customerRevoke.test.js @@ -38,6 +38,10 @@ jest.mock('../utils/tokenUtils', () => ({ getGalleryTokenFromRequest: jest.fn(), })); +jest.mock('../utils/tokenRevocation', () => ({ + isTokenRevoked: jest.fn().mockResolvedValue(false), +})); + jest.mock('../utils/dbCompat', () => ({ formatBoolean: (v) => (v ? 1 : 0), })); @@ -45,7 +49,8 @@ jest.mock('../utils/dbCompat', () => ({ const jwt = require('jsonwebtoken'); const { db } = require('../database/db'); const { getGalleryTokenFromRequest } = require('../utils/tokenUtils'); -const { verifyGalleryAccess } = require('../middleware/gallery'); +const { isTokenRevoked } = require('../utils/tokenRevocation'); +const { verifyGalleryAccess, isAdminPreview } = require('../middleware/gallery'); function makeRes() { const res = {}; @@ -93,6 +98,152 @@ beforeEach(() => { db.mockReset(); jwt.verify.mockReset(); getGalleryTokenFromRequest.mockReset(); + isTokenRevoked.mockReset(); + isTokenRevoked.mockResolvedValue(false); +}); + +// ---- revoked gallery token (GHSA-q7f7-gjx8-mf6h) ----------------------- + +describe('verifyGalleryAccess — revoked token', () => { + it('returns 401 TOKEN_REVOKED and never reaches the events query when revoked', async () => { + getGalleryTokenFromRequest.mockReturnValue('tkn'); + jwt.verify.mockReturnValue({ type: 'gallery', eventId: 42 }); + isTokenRevoked.mockResolvedValue(true); + + const req = makeReq(); + const res = makeRes(); + const next = jest.fn(); + await verifyGalleryAccess(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ code: 'TOKEN_REVOKED' }), + ); + expect(db).not.toHaveBeenCalled(); + }); + + it('proceeds normally when the token is not revoked', async () => { + getGalleryTokenFromRequest.mockReturnValue('tkn'); + jwt.verify.mockReturnValue({ type: 'gallery', eventId: 42 }); + isTokenRevoked.mockResolvedValue(false); + + const eventsChain = {}; + eventsChain.where = jest.fn().mockReturnValue(eventsChain); + eventsChain.select = jest.fn().mockReturnValue(eventsChain); + eventsChain.first = jest.fn().mockResolvedValue({ + id: 42, slug: 'test-event', is_active: true, is_archived: false, + }); + db.mockImplementationOnce(() => eventsChain); + + const req = makeReq(); + const res = makeRes(); + const next = jest.fn(); + await verifyGalleryAccess(req, res, next); + + expect(isTokenRevoked).toHaveBeenCalledWith( + expect.objectContaining({ type: 'gallery', eventId: 42 }), + ); + expect(next).toHaveBeenCalledTimes(1); + expect(res.status).not.toHaveBeenCalled(); + }); +}); + +// ---- revoked admin-preview token (?preview=) ------------------ +// +// isAdminPreview() decodes the ?preview= admin JWT independently of the +// main gallery-token flow above, and previously never checked +// isTokenRevoked — a revoked admin session kept granting preview access +// via a bookmarked/shared preview link indefinitely. Same gap as +// GHSA-q7f7-gjx8-mf6h, just in this sibling code path. + +describe('isAdminPreview — token revocation', () => { + it('returns false for a revoked admin token', async () => { + jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); + isTokenRevoked.mockResolvedValue(true); + + const result = await isAdminPreview({ query: { preview: 'revoked-admin-jwt' } }); + + expect(result).toBe(false); + expect(isTokenRevoked).toHaveBeenCalledWith( + expect.objectContaining({ type: 'admin' }), + ); + }); + + it('returns true for a valid, non-revoked admin token', async () => { + jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); + isTokenRevoked.mockResolvedValue(false); + + const result = await isAdminPreview({ query: { preview: 'valid-admin-jwt' } }); + + expect(result).toBe(true); + }); +}); + +describe('verifyGalleryAccess — revoked admin preview token', () => { + it('does not grant preview access; falls through to the normal flow and 401s', async () => { + getGalleryTokenFromRequest.mockReturnValue(undefined); // no gallery-scoped token + jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); // decoded ?preview= token + isTokenRevoked.mockResolvedValue(true); + + const eventsChain = {}; + eventsChain.where = jest.fn().mockReturnValue(eventsChain); + eventsChain.select = jest.fn().mockReturnValue(eventsChain); + eventsChain.first = jest.fn().mockResolvedValue({ + id: 42, slug: 'test-event', is_active: true, is_archived: false, + is_draft: false, require_password: true, + }); + db.mockImplementationOnce(() => eventsChain); + + const req = makeReq(); + req.query = { preview: 'revoked-admin-jwt' }; + const res = makeRes(); + const next = jest.fn(); + await verifyGalleryAccess(req, res, next); + + expect(isTokenRevoked).toHaveBeenCalledWith( + expect.objectContaining({ type: 'admin' }), + ); + // adminPreview resolved to false, so the code took the extra + // is_draft:false where() call it only skips for a real preview. + expect(eventsChain.where).toHaveBeenCalledTimes(2); + expect(eventsChain.where).toHaveBeenNthCalledWith(2, { is_draft: 0 }); + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'No token provided' }), + ); + }); + + it('a non-revoked admin preview token still works', async () => { + getGalleryTokenFromRequest.mockReturnValue(undefined); + jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); + isTokenRevoked.mockResolvedValue(false); + + const eventsChain = {}; + eventsChain.where = jest.fn().mockReturnValue(eventsChain); + eventsChain.select = jest.fn().mockReturnValue(eventsChain); + eventsChain.first = jest.fn().mockResolvedValue({ + id: 42, slug: 'test-event', is_active: true, is_archived: false, + is_draft: true, require_password: false, + }); + db.mockImplementationOnce(() => eventsChain); + + const req = makeReq(); + req.query = { preview: 'valid-admin-jwt' }; + const res = makeRes(); + const next = jest.fn(); + await verifyGalleryAccess(req, res, next); + + expect(isTokenRevoked).toHaveBeenCalledWith( + expect.objectContaining({ type: 'admin' }), + ); + // adminPreview resolved to true, so no is_draft filter was applied. + expect(eventsChain.where).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledTimes(1); + expect(res.status).not.toHaveBeenCalled(); + expect(req.event).toEqual(expect.objectContaining({ id: 42 })); + }); }); // ---- customer-minted JWT, assignment intact ---------------------------- diff --git a/backend/src/middleware/gallery.js b/backend/src/middleware/gallery.js index a1ee968a..0c78ba4b 100644 --- a/backend/src/middleware/gallery.js +++ b/backend/src/middleware/gallery.js @@ -2,15 +2,28 @@ const jwt = require('jsonwebtoken'); const { db, withRetry } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { getGalleryTokenFromRequest } = require('../utils/tokenUtils'); +const { isTokenRevoked } = require('../utils/tokenRevocation'); const logger = require('../utils/logger'); // Check if the request carries a valid admin preview token (Feature 3) -function isAdminPreview(req) { +async 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'; + if (decoded.type !== 'admin') return false; + + // Same gap this file already closed for the main gallery-token path + // (GHSA-q7f7-gjx8-mf6h): a revoked admin session must not keep + // granting preview access via a bookmarked/shared ?preview= link. + // Treat a revoked token the same as an invalid one -- fail the + // preview check, don't throw, so callers fall through to the normal + // gallery-token flow. + if (await isTokenRevoked(decoded)) { + return false; + } + + return true; } catch { return false; } @@ -28,7 +41,7 @@ async function verifyGalleryAccess(req, res, next) { return res.status(401).json({ error: 'No token provided' }); } - const adminPreview = isAdminPreview(req); + const adminPreview = await isAdminPreview(req); event = await withRetry(async () => { const q = db('events') .where({ @@ -89,10 +102,18 @@ async function verifyGalleryAccess(req, res, next) { return res.status(403).json({ error: 'Invalid token type for gallery access' }); } + // Gallery logout writes to the revocation store (see routes/auth.js), + // but nothing on this path ever read it back (GHSA-q7f7-gjx8-mf6h) — a + // logged-out gallery JWT kept working until natural expiry. + if (await isTokenRevoked(decoded)) { + logger.warn('[verifyGalleryAccess] Revoked token used', { eventId: decoded.eventId }); + return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' }); + } + // If we have a slug in the URL params or from pre-middleware, verify it matches if (requestedSlug) { // Verify by slug and ensure it matches the token's event - const adminPreviewToken = isAdminPreview(req); + const adminPreviewToken = await isAdminPreview(req); event = await withRetry(async () => { const q = db('events') .where({ @@ -112,7 +133,7 @@ async function verifyGalleryAccess(req, res, next) { } } else { // Fallback to using eventId from token - const adminPreviewFallback = isAdminPreview(req); + const adminPreviewFallback = await isAdminPreview(req); event = await withRetry(async () => { const q = db('events') .where({ diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 1fde4b86..4040b566 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -238,7 +238,7 @@ router.get('/:slug/info', async (req, res) => { } // Check if event is a draft (allow admin preview) - if (event.is_draft && !isAdminPreview(req)) { + if (event.is_draft && !(await isAdminPreview(req))) { return res.status(404).json({ error: 'Gallery is not yet published' }); }