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.
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