fix(security): verify the signature before writing a token to the revocation list
revokeToken() base64-decoded the payload without checking the signature and
inserted a row keyed on id-iat-type, the same key isTokenRevoked() matches
for real sessions. The logout endpoints are unauthenticated, so anyone could
forge a payload naming another user's id, type and login second and log them
out remotely; a far-future exp also left rows that cleanup never swept.
Expiry is still ignored so logging out an expired session stays idempotent.
(cherry picked from commit 0ca0e4a922)
This commit is contained in:
@@ -0,0 +1,69 @@
|
||||
/**
|
||||
* revokeToken() is reachable from the unauthenticated logout endpoints
|
||||
* (POST /api/auth/logout, /gallery/logout, /customer-auth/logout). It used
|
||||
* to base64-decode the payload without checking the signature and insert a
|
||||
* row keyed on `${id}-${iat}-${type}` -- the same key isTokenRevoked()
|
||||
* matches for real sessions. Anyone could therefore forge a payload naming
|
||||
* another user's id, type and login second and log them out remotely, and
|
||||
* with a far-future `exp` the row was never swept.
|
||||
*
|
||||
* The contract pinned here: only a token whose signature verifies under
|
||||
* JWT_SECRET is written to revoked_tokens. Expired-but-genuine tokens are
|
||||
* still accepted (logout must stay idempotent).
|
||||
*/
|
||||
const jwt = require('jsonwebtoken');
|
||||
|
||||
process.env.JWT_SECRET = 'revocation-forgery-test-secret';
|
||||
|
||||
const inserted = [];
|
||||
jest.mock('../../src/database/db', () => {
|
||||
const dbFn = () => ({
|
||||
insert(row) {
|
||||
inserted.push(row);
|
||||
return { onConflict: () => ({ ignore: async () => undefined }) };
|
||||
},
|
||||
});
|
||||
return { db: dbFn };
|
||||
});
|
||||
jest.mock('../../src/utils/logger', () => ({
|
||||
info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn(),
|
||||
}));
|
||||
|
||||
const { revokeToken } = require('../../src/utils/tokenRevocation');
|
||||
|
||||
const iat = Math.floor(Date.now() / 1000) - 60;
|
||||
|
||||
describe('revokeToken signature check', () => {
|
||||
beforeEach(() => { inserted.length = 0; });
|
||||
|
||||
it('refuses a forged three-part token and writes nothing', async () => {
|
||||
const forgedPayload = Buffer.from(JSON.stringify({
|
||||
id: 1, iat, type: 'admin', exp: 9e9,
|
||||
})).toString('base64');
|
||||
const forged = `eyJhbGciOiJIUzI1NiJ9.${forgedPayload}.notasignature`;
|
||||
|
||||
const result = await revokeToken(forged, 'user_logout');
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(inserted).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('refuses a token signed with a different secret', async () => {
|
||||
const other = jwt.sign({ id: 1, iat, type: 'admin' }, 'some-other-secret', { expiresIn: '1h' });
|
||||
expect(await revokeToken(other, 'user_logout')).toBe(false);
|
||||
expect(inserted).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('revokes a genuine token', async () => {
|
||||
const genuine = jwt.sign({ id: 1, iat, type: 'admin' }, process.env.JWT_SECRET, { expiresIn: '1h' });
|
||||
expect(await revokeToken(genuine, 'user_logout')).toBe(true);
|
||||
expect(inserted).toHaveLength(1);
|
||||
expect(inserted[0].token_id).toBe(`1-${iat}-admin`);
|
||||
});
|
||||
|
||||
it('still revokes a genuine token that has already expired', async () => {
|
||||
const expired = jwt.sign({ id: 1, iat, type: 'admin', exp: iat + 1 }, process.env.JWT_SECRET);
|
||||
expect(await revokeToken(expired, 'user_logout')).toBe(true);
|
||||
expect(inserted).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
@@ -3,6 +3,7 @@
|
||||
* Provides ability to invalidate tokens before expiration
|
||||
*/
|
||||
|
||||
const jwt = require('jsonwebtoken');
|
||||
const { db } = require('../database/db');
|
||||
const logger = require('./logger');
|
||||
|
||||
@@ -28,15 +29,23 @@ function buildTokenId(payload) {
|
||||
|
||||
async function revokeToken(token, reason, metadata = {}) {
|
||||
try {
|
||||
// Extract token info without full verification (it might be compromised)
|
||||
const parts = token.split('.');
|
||||
if (parts.length !== 3) {
|
||||
throw new Error('Invalid token format');
|
||||
// The signature MUST be verified before anything is written. The
|
||||
// revocation key is `${id}-${iat}-${type}` (buildTokenId), and the
|
||||
// logout endpoints are unauthenticated, so a raw base64 decode let
|
||||
// anyone forge a three-part string naming another user's id, type and
|
||||
// login second and insert a row that isTokenRevoked() then matched for
|
||||
// that user's real session -- a remote forced logout of any admin,
|
||||
// customer or gallery session, plus never-swept rows when `exp` was set
|
||||
// far in the future. Expiry is ignored on purpose: revoking an already
|
||||
// expired token is harmless and keeps logout idempotent.
|
||||
const payload = jwt.verify(token, process.env.JWT_SECRET, {
|
||||
algorithms: ['HS256'],
|
||||
ignoreExpiration: true,
|
||||
});
|
||||
if (!payload || typeof payload !== 'object') {
|
||||
throw new Error('Invalid token payload');
|
||||
}
|
||||
|
||||
// Decode payload
|
||||
const payload = JSON.parse(Buffer.from(parts[1], 'base64').toString());
|
||||
|
||||
// user_id is integer-typed in revoked_tokens; for non-admin tokens
|
||||
// we may not have an integer (customer) or any id at all (gallery
|
||||
// tokens use eventId). Coerce to null instead of letting an
|
||||
|
||||
Reference in New Issue
Block a user