security: RBAC cache cross-instance invalidation + force re-login on role/perm change (#57)
- shrink roleDefaults cache TTL from 60s to 10s (safety-net staleness bound) - publish/subscribe on capakraken:rbac-invalidate so peer instances drop their local role-defaults cache on mutation (ioredis pub/sub; lazy init so idle test files don't open connections) - after updateUserRole/setUserPermissions/resetUserPermissions: delete all ActiveSession rows for that user so the next request re-auths via tRPC's jti check, and invalidate the role-defaults cache - tests: peer-instance invalidation via FakeRedis pub/sub fan-out; mutation side-effects assert session deletion + cache invalidation on each path Without this, demoted admins kept their JWT valid until expiry and peer instances kept serving stale role defaults for up to the TTL window. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,131 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
/**
|
||||
* Ticket #57 — verify that:
|
||||
*
|
||||
* 1. Publishing on RBAC_INVALIDATE_CHANNEL from node A causes node B to
|
||||
* drop its local `_roleDefaultsCache`, so its next `loadRoleDefaults()`
|
||||
* call re-reads from the DB (acceptance criterion:
|
||||
* "2nd node sees update within 1 s" — we verify the mechanism, not the
|
||||
* Redis latency).
|
||||
*
|
||||
* 2. `invalidateRoleDefaultsCache()` on the current node publishes on the
|
||||
* same channel so peer instances receive the event.
|
||||
*
|
||||
* Strategy: stub `ioredis` with an EventEmitter-based fake before loading
|
||||
* trpc.ts. The fake captures `publish()` calls and lets the test emit
|
||||
* synthetic "message" events.
|
||||
*/
|
||||
|
||||
// Fake Redis with two separate instances so the test mirrors the multi-node
|
||||
// shape: one as subscriber, one as publisher. Both share the same module-
|
||||
// level event router keyed by channel.
|
||||
const channelSubscribers = new Map<string, Set<FakeRedis>>();
|
||||
const publishCalls: Array<{ channel: string; message: string }> = [];
|
||||
|
||||
class FakeRedis extends EventEmitter {
|
||||
constructor(_url: string, _opts: unknown) {
|
||||
super();
|
||||
}
|
||||
// eslint-disable-next-line @typescript-eslint/require-await
|
||||
async subscribe(channel: string): Promise<number> {
|
||||
let set = channelSubscribers.get(channel);
|
||||
if (!set) {
|
||||
set = new Set();
|
||||
channelSubscribers.set(channel, set);
|
||||
}
|
||||
set.add(this);
|
||||
return set.size;
|
||||
}
|
||||
// eslint-disable-next-line @typescript-eslint/require-await
|
||||
async publish(channel: string, message: string): Promise<number> {
|
||||
publishCalls.push({ channel, message });
|
||||
const subs = channelSubscribers.get(channel);
|
||||
if (!subs) return 0;
|
||||
// Fan out synchronously so the subscriber handler runs before the test
|
||||
// assertion reads the cache — matches real ioredis "message" semantics
|
||||
// from the subscriber's point of view.
|
||||
for (const sub of subs) sub.emit("message", channel, message);
|
||||
return subs.size;
|
||||
}
|
||||
}
|
||||
|
||||
vi.mock("ioredis", () => ({ Redis: FakeRedis, default: FakeRedis }));
|
||||
vi.mock("../lib/logger.js", () => ({
|
||||
logger: { warn: vi.fn(), error: vi.fn(), info: vi.fn(), debug: vi.fn() },
|
||||
}));
|
||||
|
||||
// Prisma client mock — loadRoleDefaults pulls from systemRoleConfig.findMany.
|
||||
const findManyCalls: number[] = [];
|
||||
vi.mock("@capakraken/db", async () => {
|
||||
const actual = await vi.importActual<Record<string, unknown>>("@capakraken/db");
|
||||
return {
|
||||
...actual,
|
||||
prisma: {
|
||||
systemRoleConfig: {
|
||||
findMany: vi.fn().mockImplementation(async () => {
|
||||
findManyCalls.push(Date.now());
|
||||
return [{ role: "ADMIN", defaultPermissions: ["MANAGE_USERS"] }];
|
||||
}),
|
||||
},
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
// REDIS_URL is needed so trpc.ts decides to instantiate the fake Redis.
|
||||
// `trpc.ts` now reads it lazily on first RBAC call, so setting it in
|
||||
// beforeAll is enough; we always restore in afterAll to avoid leaking into
|
||||
// other test files in the same worker.
|
||||
const originalRedisUrl = process.env["REDIS_URL"];
|
||||
|
||||
describe("RBAC cache Redis pub/sub (#57)", () => {
|
||||
beforeAll(() => {
|
||||
process.env["REDIS_URL"] = "redis://fake:6379";
|
||||
});
|
||||
afterAll(() => {
|
||||
if (originalRedisUrl === undefined) delete process.env["REDIS_URL"];
|
||||
else process.env["REDIS_URL"] = originalRedisUrl;
|
||||
});
|
||||
beforeEach(() => {
|
||||
findManyCalls.length = 0;
|
||||
});
|
||||
|
||||
it("peer-instance invalidation: receiving a message clears the local cache", async () => {
|
||||
const { loadRoleDefaults } = await import("../trpc.js");
|
||||
|
||||
// Warm the cache.
|
||||
await loadRoleDefaults();
|
||||
const hitsAfterWarm = findManyCalls.length;
|
||||
expect(hitsAfterWarm).toBe(1);
|
||||
|
||||
// Second call within TTL should be cached — no additional findMany.
|
||||
await loadRoleDefaults();
|
||||
expect(findManyCalls.length).toBe(hitsAfterWarm);
|
||||
|
||||
// Simulate a peer instance publishing an invalidation: grab any
|
||||
// subscriber on the channel and fire the event as if Redis delivered it.
|
||||
const subs = channelSubscribers.get("capakraken:rbac-invalidate");
|
||||
expect(subs).toBeDefined();
|
||||
expect(subs!.size).toBeGreaterThanOrEqual(1);
|
||||
for (const sub of subs!) sub.emit("message", "capakraken:rbac-invalidate", "1");
|
||||
|
||||
// Next load must hit the DB again.
|
||||
await loadRoleDefaults();
|
||||
expect(findManyCalls.length).toBe(hitsAfterWarm + 1);
|
||||
});
|
||||
|
||||
it("local invalidation publishes on the RBAC channel", async () => {
|
||||
const { invalidateRoleDefaultsCache } = await import("../trpc.js");
|
||||
const countBefore = publishCalls.length;
|
||||
|
||||
invalidateRoleDefaultsCache();
|
||||
|
||||
// Give the microtask queue one tick (publish returns a promise).
|
||||
await Promise.resolve();
|
||||
|
||||
const newPublishes = publishCalls.slice(countBefore);
|
||||
expect(newPublishes.length).toBe(1);
|
||||
expect(newPublishes[0]!.channel).toBe("capakraken:rbac-invalidate");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,180 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { SystemRole } from "@capakraken/shared";
|
||||
|
||||
vi.mock("../lib/audit.js", () => ({ createAuditEntry: vi.fn() }));
|
||||
vi.mock("../lib/audit-helpers.js", () => ({
|
||||
makeAuditLogger: () => vi.fn(),
|
||||
}));
|
||||
|
||||
const invalidateRoleDefaultsCache = vi.hoisted(() => vi.fn());
|
||||
vi.mock("../trpc.js", () => ({
|
||||
invalidateRoleDefaultsCache,
|
||||
}));
|
||||
|
||||
import {
|
||||
resetUserPermissions,
|
||||
setUserPermissions,
|
||||
updateUserRole,
|
||||
} from "../router/user-procedure-support.js";
|
||||
|
||||
/**
|
||||
* Ticket #57 — when a privileged-state mutation happens we MUST:
|
||||
* 1. delete every ActiveSession for the affected user (forces next-request
|
||||
* re-auth, because the tRPC route validates `jti` against ActiveSession),
|
||||
* 2. call `invalidateRoleDefaultsCache()` so peer instances drop their
|
||||
* 10 s cache entries via the Redis pub/sub fan-out.
|
||||
*
|
||||
* Without (1), a demoted admin keeps their JWT valid until it expires, so
|
||||
* permissions resolved server-side still reflect the old role. Without (2),
|
||||
* peer instances keep serving the old role defaults for up to the TTL.
|
||||
*/
|
||||
describe("RBAC mutation side effects (#57)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
function makeCtx(dbOverrides: Record<string, unknown> = {}) {
|
||||
const defaultDb = {
|
||||
user: {
|
||||
findUnique: vi.fn(),
|
||||
update: vi.fn(),
|
||||
},
|
||||
activeSession: {
|
||||
deleteMany: vi.fn().mockResolvedValue({ count: 3 }),
|
||||
},
|
||||
...dbOverrides,
|
||||
};
|
||||
return {
|
||||
ctx: {
|
||||
db: defaultDb as never,
|
||||
dbUser: {
|
||||
id: "admin_1",
|
||||
systemRole: SystemRole.ADMIN,
|
||||
permissionOverrides: null,
|
||||
},
|
||||
session: {
|
||||
user: { email: "admin@example.com", name: "Admin", image: null },
|
||||
expires: "2099-01-01T00:00:00.000Z",
|
||||
},
|
||||
},
|
||||
db: defaultDb,
|
||||
};
|
||||
}
|
||||
|
||||
describe("updateUserRole", () => {
|
||||
it("deletes active sessions and invalidates cache when role changes", async () => {
|
||||
const { ctx, db } = makeCtx({
|
||||
user: {
|
||||
findUnique: vi.fn().mockResolvedValue({
|
||||
id: "user_victim",
|
||||
name: "Victim",
|
||||
email: "victim@example.com",
|
||||
systemRole: SystemRole.ADMIN,
|
||||
}),
|
||||
update: vi.fn().mockResolvedValue({
|
||||
id: "user_victim",
|
||||
name: "Victim",
|
||||
email: "victim@example.com",
|
||||
systemRole: SystemRole.USER,
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
await updateUserRole(ctx as never, {
|
||||
id: "user_victim",
|
||||
systemRole: SystemRole.USER,
|
||||
});
|
||||
|
||||
expect(db.activeSession.deleteMany).toHaveBeenCalledWith({
|
||||
where: { userId: "user_victim" },
|
||||
});
|
||||
expect(invalidateRoleDefaultsCache).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does NOT delete sessions or invalidate when role is unchanged", async () => {
|
||||
const { ctx, db } = makeCtx({
|
||||
user: {
|
||||
findUnique: vi.fn().mockResolvedValue({
|
||||
id: "user_1",
|
||||
name: "Alice",
|
||||
email: "alice@example.com",
|
||||
systemRole: SystemRole.MANAGER,
|
||||
}),
|
||||
update: vi.fn().mockResolvedValue({
|
||||
id: "user_1",
|
||||
name: "Alice",
|
||||
email: "alice@example.com",
|
||||
systemRole: SystemRole.MANAGER,
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
await updateUserRole(ctx as never, {
|
||||
id: "user_1",
|
||||
systemRole: SystemRole.MANAGER,
|
||||
});
|
||||
|
||||
expect(db.activeSession.deleteMany).not.toHaveBeenCalled();
|
||||
expect(invalidateRoleDefaultsCache).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("setUserPermissions", () => {
|
||||
it("deletes active sessions and invalidates cache on every call", async () => {
|
||||
const { ctx, db } = makeCtx({
|
||||
user: {
|
||||
findUnique: vi.fn().mockResolvedValue({
|
||||
id: "user_1",
|
||||
name: "Alice",
|
||||
email: "alice@example.com",
|
||||
permissionOverrides: null,
|
||||
}),
|
||||
update: vi.fn().mockResolvedValue({
|
||||
id: "user_1",
|
||||
name: "Alice",
|
||||
email: "alice@example.com",
|
||||
permissionOverrides: { granted: ["x"], denied: [] },
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
await setUserPermissions(ctx as never, {
|
||||
userId: "user_1",
|
||||
overrides: { granted: ["x"], denied: [] },
|
||||
});
|
||||
|
||||
expect(db.activeSession.deleteMany).toHaveBeenCalledWith({
|
||||
where: { userId: "user_1" },
|
||||
});
|
||||
expect(invalidateRoleDefaultsCache).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resetUserPermissions", () => {
|
||||
it("deletes active sessions and invalidates cache", async () => {
|
||||
const { ctx, db } = makeCtx({
|
||||
user: {
|
||||
findUnique: vi.fn().mockResolvedValue({
|
||||
id: "user_1",
|
||||
name: "Alice",
|
||||
email: "alice@example.com",
|
||||
permissionOverrides: { granted: ["x"], denied: [] },
|
||||
}),
|
||||
update: vi.fn().mockResolvedValue({
|
||||
id: "user_1",
|
||||
name: "Alice",
|
||||
email: "alice@example.com",
|
||||
permissionOverrides: null,
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
await resetUserPermissions(ctx as never, { userId: "user_1" });
|
||||
|
||||
expect(db.activeSession.deleteMany).toHaveBeenCalledWith({
|
||||
where: { userId: "user_1" },
|
||||
});
|
||||
expect(invalidateRoleDefaultsCache).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -49,12 +49,20 @@ vi.mock("otpauth", () => {
|
||||
const createCaller = createCallerFactory(userRouter);
|
||||
|
||||
function createAdminCaller(db: Record<string, unknown>) {
|
||||
// Provide a no-op activeSession stub by default — some mutation paths
|
||||
// (setPermissions / resetPermissions / updateRole, see ticket #57) now
|
||||
// invalidate active sessions to force a re-login on privilege changes.
|
||||
// Individual tests can override by passing their own `activeSession` key.
|
||||
const dbWithDefaults = {
|
||||
activeSession: { deleteMany: vi.fn().mockResolvedValue({ count: 0 }) },
|
||||
...db,
|
||||
};
|
||||
return createCaller({
|
||||
session: {
|
||||
user: { email: "admin@example.com", name: "Admin", image: null },
|
||||
expires: "2099-01-01T00:00:00.000Z",
|
||||
},
|
||||
db: db as never,
|
||||
db: dbWithDefaults as never,
|
||||
dbUser: {
|
||||
id: "user_admin",
|
||||
systemRole: SystemRole.ADMIN,
|
||||
|
||||
Reference in New Issue
Block a user