fix(backend): check the revocation store on gallery access (stable) (#1388)
* 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 <[email protected]>
This commit is contained in:
co-authored by
Paul Nothaft
parent
b663f2a994
commit
b869de33a5
@@ -38,6 +38,10 @@ jest.mock('../utils/tokenUtils', () => ({
|
|||||||
getGalleryTokenFromRequest: jest.fn(),
|
getGalleryTokenFromRequest: jest.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
jest.mock('../utils/tokenRevocation', () => ({
|
||||||
|
isTokenRevoked: jest.fn().mockResolvedValue(false),
|
||||||
|
}));
|
||||||
|
|
||||||
jest.mock('../utils/dbCompat', () => ({
|
jest.mock('../utils/dbCompat', () => ({
|
||||||
formatBoolean: (v) => (v ? 1 : 0),
|
formatBoolean: (v) => (v ? 1 : 0),
|
||||||
}));
|
}));
|
||||||
@@ -45,7 +49,8 @@ jest.mock('../utils/dbCompat', () => ({
|
|||||||
const jwt = require('jsonwebtoken');
|
const jwt = require('jsonwebtoken');
|
||||||
const { db } = require('../database/db');
|
const { db } = require('../database/db');
|
||||||
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
|
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
|
||||||
const { verifyGalleryAccess } = require('../middleware/gallery');
|
const { isTokenRevoked } = require('../utils/tokenRevocation');
|
||||||
|
const { verifyGalleryAccess, isAdminPreview } = require('../middleware/gallery');
|
||||||
|
|
||||||
function makeRes() {
|
function makeRes() {
|
||||||
const res = {};
|
const res = {};
|
||||||
@@ -93,6 +98,152 @@ beforeEach(() => {
|
|||||||
db.mockReset();
|
db.mockReset();
|
||||||
jwt.verify.mockReset();
|
jwt.verify.mockReset();
|
||||||
getGalleryTokenFromRequest.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=<adminJWT>) ------------------
|
||||||
|
//
|
||||||
|
// 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 ----------------------------
|
// ---- customer-minted JWT, assignment intact ----------------------------
|
||||||
|
|||||||
@@ -2,15 +2,28 @@ const jwt = require('jsonwebtoken');
|
|||||||
const { db, withRetry } = require('../database/db');
|
const { db, withRetry } = require('../database/db');
|
||||||
const { formatBoolean } = require('../utils/dbCompat');
|
const { formatBoolean } = require('../utils/dbCompat');
|
||||||
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
|
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
|
||||||
|
const { isTokenRevoked } = require('../utils/tokenRevocation');
|
||||||
const logger = require('../utils/logger');
|
const logger = require('../utils/logger');
|
||||||
|
|
||||||
// Check if the request carries a valid admin preview token (Feature 3)
|
// Check if the request carries a valid admin preview token (Feature 3)
|
||||||
function isAdminPreview(req) {
|
async function isAdminPreview(req) {
|
||||||
const previewToken = req.query?.preview;
|
const previewToken = req.query?.preview;
|
||||||
if (!previewToken) return false;
|
if (!previewToken) return false;
|
||||||
try {
|
try {
|
||||||
const decoded = jwt.verify(previewToken, process.env.JWT_SECRET, { issuer: 'picpeak-auth' });
|
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 {
|
} catch {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
@@ -28,7 +41,7 @@ async function verifyGalleryAccess(req, res, next) {
|
|||||||
return res.status(401).json({ error: 'No token provided' });
|
return res.status(401).json({ error: 'No token provided' });
|
||||||
}
|
}
|
||||||
|
|
||||||
const adminPreview = isAdminPreview(req);
|
const adminPreview = await isAdminPreview(req);
|
||||||
event = await withRetry(async () => {
|
event = await withRetry(async () => {
|
||||||
const q = db('events')
|
const q = db('events')
|
||||||
.where({
|
.where({
|
||||||
@@ -89,10 +102,18 @@ async function verifyGalleryAccess(req, res, next) {
|
|||||||
return res.status(403).json({ error: 'Invalid token type for gallery access' });
|
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 we have a slug in the URL params or from pre-middleware, verify it matches
|
||||||
if (requestedSlug) {
|
if (requestedSlug) {
|
||||||
// Verify by slug and ensure it matches the token's event
|
// Verify by slug and ensure it matches the token's event
|
||||||
const adminPreviewToken = isAdminPreview(req);
|
const adminPreviewToken = await isAdminPreview(req);
|
||||||
event = await withRetry(async () => {
|
event = await withRetry(async () => {
|
||||||
const q = db('events')
|
const q = db('events')
|
||||||
.where({
|
.where({
|
||||||
@@ -112,7 +133,7 @@ async function verifyGalleryAccess(req, res, next) {
|
|||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Fallback to using eventId from token
|
// Fallback to using eventId from token
|
||||||
const adminPreviewFallback = isAdminPreview(req);
|
const adminPreviewFallback = await isAdminPreview(req);
|
||||||
event = await withRetry(async () => {
|
event = await withRetry(async () => {
|
||||||
const q = db('events')
|
const q = db('events')
|
||||||
.where({
|
.where({
|
||||||
|
|||||||
@@ -238,7 +238,7 @@ router.get('/:slug/info', async (req, res) => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Check if event is a draft (allow admin preview)
|
// 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' });
|
return res.status(404).json({ error: 'Gallery is not yet published' });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user