fix: retain revocations for tokens without expiry

2026-09-08 15:54:01 +02:00
parent a31a2e25e2
commit 662516a5ad
9 changed files with 234 additions and 17 deletions
@@ -0,0 +1,96 @@
+const knex = require('knex');
+const jwt = require('jsonwebtoken');
+const { randomUUID } = require('crypto');
+const migration = require('../../migrations/core/211_revocations_without_expiry');
+
+for (const client of ['sqlite3', 'pg']) {
+  const enabled = client !== 'pg' || process.env.PICPEAK_PG_TEST_URL;
+  (enabled ? describe : describe.skip)(`token revocation expiry (${client})`, () => {
+    let db, owner, schema, revocation;
+    const sign = claims => jwt.sign({ id: 1, type: 'admin', jti: randomUUID(), ...claims }, process.env.JWT_SECRET);
+
+    beforeAll(async () => {
+      if (client === 'pg') {
+        schema = `revocation_${randomUUID().replace(/-/g, '')}`;
+        owner = knex({ client, connection: process.env.PICPEAK_PG_TEST_URL });
+        await owner.schema.createSchema(schema);
+        db = knex({ client, connection: process.env.PICPEAK_PG_TEST_URL, searchPath: [schema] });
+      } else {
+        db = knex({ client, connection: { filename: ':memory:' }, useNullAsDefault: true });
+      }
+      // Exercise the upgrade from the real legacy NOT NULL schema as well as
+      // repeated migration runs, without sharing another test's database.
+      await require('../../migrations/legacy/017_add_token_revocation_tables').up(db);
+      await db('revoked_tokens').insert({ token_id: 'existing', expires_at: '2099-01-01T00:00:00.000Z' });
+      await migration.up(db);
+      await migration.up(db);
+      jest.resetModules();
+      jest.doMock('../../src/database/db', () => ({ db }));
+      revocation = require('../../src/utils/tokenRevocation');
+    });
+
+    afterAll(async () => {
+      await db?.destroy();
+      if (owner) { await owner.schema.dropSchema(schema, true); await owner.destroy(); }
+      jest.dontMock('../../src/database/db');
+    });
+
+    it('preserves existing revocations and their unique key during upgrade', async () => {
+      expect(await db('revoked_tokens').where({ token_id: 'existing' }).first()).toBeTruthy();
+      await expect(db('revoked_tokens').insert({ token_id: 'existing', expires_at: null })).rejects.toThrow();
+    });
+
+    it.each([true, false])('permanently revokes a token without exp (jti: %s)', async withJti => {
+      const token = sign(withJti ? {} : { jti: undefined });
+      const payload = jwt.verify(token, process.env.JWT_SECRET);
+      expect(await revocation.isTokenRevoked(payload)).toBe(false);
+      expect(await revocation.revokeToken(token, 'logout')).toBe(true);
+      expect(await revocation.revokeToken(token, 'logout')).toBe(true);
+      await revocation.cleanupExpiredRevocations();
+      expect(await revocation.isTokenRevoked(payload)).toBe(true);
+      const rows = await db('revoked_tokens').where({ token_id: revocation.buildTokenId(payload) });
+      expect(rows).toHaveLength(1);
+      expect(rows[0].expires_at).toBeNull();
+    });
+
+    it('cleans up expired revocations and retains future ones', async () => {
+      const expired = sign({ exp: Math.floor(Date.now() / 1000) - 60 });
+      const future = sign({ exp: Math.floor(Date.now() / 1000) + 3600 });
+      expect(await revocation.revokeToken(expired, 'logout')).toBe(true);
+      expect(await revocation.revokeToken(future, 'logout')).toBe(true);
+      await revocation.cleanupExpiredRevocations();
+      expect(await revocation.isTokenRevoked(jwt.decode(expired))).toBe(false);
+      expect(await revocation.isTokenRevoked(jwt.decode(future))).toBe(true);
+    });
+
+    it.each([true, false])('upgrades an expiring entry with the same key permanently (jti: %s)', async withJti => {
+      const claims = { id: 99, iat: Math.floor(Date.now() / 1000), jti: withJti ? randomUUID() : undefined };
+      const expiring = sign({ ...claims, exp: claims.iat - 60 });
+      const permanent = sign(claims);
+      expect(await revocation.revokeToken(expiring, 'logout')).toBe(true);
+      expect(await revocation.revokeToken(permanent, 'logout')).toBe(true);
+      expect(await revocation.revokeToken(expiring, 'logout')).toBe(true);
+      await revocation.cleanupExpiredRevocations();
+      expect(await revocation.isTokenRevoked(jwt.decode(permanent))).toBe(true);
+    });
+
+    it('retains a signed token whose numeric expiry cannot fit a database timestamp', async () => {
+      const token = sign({ exp: 1e100 });
+      expect(await revocation.revokeToken(token, 'logout')).toBe(true);
+      await revocation.cleanupExpiredRevocations();
+      expect(await revocation.isTokenRevoked(jwt.decode(token))).toBe(true);
+    });
+
+    it('refuses a rollback that would remove permanent revocations', async () => {
+      await expect(migration.down(db)).rejects.toThrow('permanent token revocations');
+      expect((await db('revoked_tokens').columnInfo('expires_at')).nullable).toBe(true);
+      // A rollback with only expiring records remains supported and reversible.
+      await db('revoked_tokens').whereNull('expires_at').delete();
+      await migration.down(db);
+      await migration.down(db);
+      expect((await db('revoked_tokens').columnInfo('expires_at')).nullable).toBe(false);
+      await migration.up(db);
+      expect(await db('revoked_tokens').where({ token_id: 'existing' }).first()).toBeTruthy();
+    });
+  });
+}
@@ -0,0 +1,75 @@
+const request = require('supertest');
+const jwt = require('jsonwebtoken');
+const { randomUUID } = require('crypto');
+const { bootCrmDb, seedMinimal, assignAdminRole, buildRouteApp } = require('../integration/helpers/crmDb');
+
+let db, cleanup, adminId, customerId, eventId, apps, revocation;
+const slug = 'logout-revocation';
+const cases = [
+  ['auth', '/logout', 'admin', 'admin_token'],
+  ['auth', '/gallery/logout', 'gallery', `gallery_token_${slug}`],
+  ['customerAuth', '/logout', 'customer', 'customer_token'],
+  ['adminAuth', '/logout', 'admin', 'admin_token'],
+];
+const sign = type => jwt.sign({
+  type, ...(type === 'admin' ? { id: adminId } : type === 'customer' ? { customerId } : { eventId, eventSlug: slug }),
+  jti: randomUUID(),
+}, process.env.JWT_SECRET, { issuer: 'picpeak-auth' });
+
+beforeAll(async () => {
+  ({ db, cleanup } = await bootCrmDb());
+  ({ adminId, customerId } = await seedMinimal(db));
+  await assignAdminRole(db, adminId);
+  const event = await require('../../src/services/eventCreationService').createEvent({
+    event_type: 'wedding', event_name: 'Logout revocation', event_date: '2026-10-01',
+    slug, password: 'Logout-Strong-Password-924!', expiration_days: 30,
+    customer_email: '[email protected]', admin_email: '[email protected]',
+  }, { actor: { id: adminId }, source: 'v1' });
+  eventId = event.id;
+  await db('events').where({ id: eventId }).update({ slug });
+  revocation = require('../../src/utils/tokenRevocation');
+  apps = Object.fromEntries(['auth', 'adminAuth', 'customerAuth'].map(name => [
+    name, buildRouteApp('/', require(`../../src/routes/${name}`)),
+  ]));
+});
+afterEach(() => jest.restoreAllMocks());
+afterAll(async () => {
+  await require('../../src/services/serviceShutdown').stopServices();
+  if (cleanup) await cleanup();
+});
+
+it.each(cases)('%s%s revokes a no-expiry %s cookie session', async (route, path, type, cookie) => {
+  const token = sign(type);
+  const sessionApp = type === 'customer' ? apps.customerAuth : apps.auth;
+  const sessionPath = type === 'gallery' ? `/session?slug=${slug}` : '/session';
+  const session = () => request(sessionApp).get(sessionPath).set('Cookie', `${cookie}=${token}`);
+  const before = await session();
+  expect(before.status).toBe(200);
+  if (type !== 'customer') expect(before.body.valid).toBe(true);
+
+  const res = await request(apps[route]).post(path).set('Cookie', `${cookie}=${token}`).send({ slug });
+  expect(res.status).toBe(200);
+  expect(res.headers['set-cookie'].some(value => value.startsWith(`${cookie}=;`))).toBe(true);
+  await revocation.cleanupExpiredRevocations();
+  expect(await revocation.isTokenRevoked(jwt.decode(token))).toBe(true);
+  const after = await session();
+  if (type === 'customer') expect(after.status).toBe(401);
+  else expect(after.body.valid).toBe(false);
+});
+
+it.each(cases)('%s%s reports failed persistence for %s logout and clears its cookie', async (route, path, type, cookie) => {
+  const token = sign(type);
+  const realQuery = db.client.query;
+  jest.spyOn(db.client, 'query').mockImplementation(function (connection, query) {
+    if (/^insert into [`"]revoked_tokens[`"]/.test(query.sql)) {
+      return Promise.reject(new Error('simulated revocation write failure'));
+    }
+    return realQuery.call(this, connection, query);
+  });
+  const res = await request(apps[route]).post(path).set('Cookie', `${cookie}=${token}`).send({ slug });
+  expect(res.status).toBe(500);
+  expect(res.body.error).toBeTruthy();
+  expect(res.body.message).not.toBe('Logged out successfully');
+  expect(res.headers['set-cookie'].some(value => value.startsWith(`${cookie}=;`))).toBe(true);
+  expect(await revocation.isTokenRevoked(jwt.decode(token))).toBe(false);
+});
@@ -20,7 +20,7 @@ jest.mock('../../src/database/db', () => {
  const dbFn = () => ({
    insert(row) {
      inserted.push(row);
-      return { onConflict: () => ({ ignore: async () => undefined }) };
+      return { onConflict: () => ({ ignore: async () => undefined, merge: async () => undefined }) };
    },
  });
  return { db: dbFn };