mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-03 22:32:42 +00:00
👑 refactor: Scope Role Cache Keys by Isolation Context (#13454)
The ROLES cache is a single process-wide store, but role documents are
per-tenant (unique index { name, tenantId }). getRoleByName checked the cache
by role name BEFORE the tenant-scoped DB read, so a warm entry written under
one tenant's context was served to another tenant — leaking that tenant's
permission bits into the other's authorization decisions.
Scope every ROLES cache key with scopedCacheKey(), which appends the active
tenantId from the AsyncLocalStorage tenant context. It is a no-op when no
tenant context is set (or under runAsSystem), so single-tenant deployments
behave exactly as before.
Adds role.cache.spec.ts with a real Map-backed cache: two tenants sharing a
role name receive their own permissions, the cache key is tenant-scoped, the
same-tenant fast path still avoids a second DB read, and single-tenant mode
still uses the unscoped key.
This commit is contained in:
parent
7dba640c9f
commit
b483feae8b
2 changed files with 154 additions and 9 deletions
141
packages/data-schemas/src/methods/role.cache.spec.ts
Normal file
141
packages/data-schemas/src/methods/role.cache.spec.ts
Normal file
|
|
@ -0,0 +1,141 @@
|
|||
import mongoose from 'mongoose';
|
||||
import { MongoMemoryServer } from 'mongodb-memory-server';
|
||||
import { PermissionTypes, Permissions } from 'librechat-data-provider';
|
||||
import type { IRole } from '..';
|
||||
import { createRoleMethods } from './role';
|
||||
import { createModels } from '../models';
|
||||
import { tenantStorage } from '../config/tenantContext';
|
||||
|
||||
jest.mock('~/config/winston', () => ({
|
||||
error: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
info: jest.fn(),
|
||||
debug: jest.fn(),
|
||||
}));
|
||||
|
||||
/**
|
||||
* Real Map-backed cache. A jest.fn mock can only assert which key was passed;
|
||||
* a real store reproduces the actual collision: when two tenants write under the
|
||||
* same key, the second read serves the first tenant's value.
|
||||
*/
|
||||
function createMapCache() {
|
||||
const store = new Map<string, unknown>();
|
||||
return {
|
||||
store,
|
||||
get: async (k: string): Promise<unknown> => store.get(k),
|
||||
set: async (k: string, v: unknown): Promise<void> => {
|
||||
store.set(k, v);
|
||||
},
|
||||
del: async (k: string): Promise<void> => {
|
||||
store.delete(k);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const TENANT_A = 'tenant-aaaaaaaaaaaaaaaaaaaa';
|
||||
const TENANT_B = 'tenant-bbbbbbbbbbbbbbbbbbbb';
|
||||
const ROLE_NAME = 'EDITOR';
|
||||
|
||||
let mongoServer: MongoMemoryServer;
|
||||
let Role: mongoose.Model<IRole>;
|
||||
let cache: ReturnType<typeof createMapCache>;
|
||||
let getRoleByName: ReturnType<typeof createRoleMethods>['getRoleByName'];
|
||||
|
||||
function runAs<T>(tenantId: string, fn: () => Promise<T>): Promise<T> {
|
||||
return tenantStorage.run({ tenantId }, fn);
|
||||
}
|
||||
|
||||
function usePromptsPermission(role: IRole | null | undefined): boolean | undefined {
|
||||
const permissions = (role as unknown as { permissions?: Record<string, Record<string, boolean>> })
|
||||
?.permissions;
|
||||
return permissions?.[PermissionTypes.PROMPTS]?.[Permissions.USE];
|
||||
}
|
||||
|
||||
async function seedRole(tenantId: string, useValue: boolean): Promise<void> {
|
||||
await runAs(tenantId, async () => {
|
||||
await new Role({
|
||||
name: ROLE_NAME,
|
||||
permissions: { [PermissionTypes.PROMPTS]: { [Permissions.USE]: useValue } },
|
||||
}).save();
|
||||
});
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
mongoServer = await MongoMemoryServer.create();
|
||||
await mongoose.connect(mongoServer.getUri());
|
||||
createModels(mongoose);
|
||||
Role = mongoose.models.Role as mongoose.Model<IRole>;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await mongoose.disconnect();
|
||||
await mongoServer.stop();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await Role.deleteMany({});
|
||||
cache = createMapCache();
|
||||
const methods = createRoleMethods(mongoose, { getCache: () => cache });
|
||||
getRoleByName = methods.getRoleByName;
|
||||
});
|
||||
|
||||
describe('getRoleByName cache is scoped to the active tenant', () => {
|
||||
it('does not serve one tenant a cached role belonging to another tenant', async () => {
|
||||
await seedRole(TENANT_A, true);
|
||||
await seedRole(TENANT_B, false);
|
||||
|
||||
const roleA = await runAs(TENANT_A, () => getRoleByName(ROLE_NAME));
|
||||
expect(usePromptsPermission(roleA)).toBe(true);
|
||||
expect(cache.store.size).toBeGreaterThan(0);
|
||||
|
||||
const roleB = await runAs(TENANT_B, () => getRoleByName(ROLE_NAME));
|
||||
expect(usePromptsPermission(roleB)).toBe(false);
|
||||
|
||||
const roleAAgain = await runAs(TENANT_A, () => getRoleByName(ROLE_NAME));
|
||||
expect(usePromptsPermission(roleAAgain)).toBe(true);
|
||||
});
|
||||
|
||||
it('appends the tenant id to the cache key', async () => {
|
||||
await seedRole(TENANT_A, true);
|
||||
await seedRole(TENANT_B, false);
|
||||
|
||||
await runAs(TENANT_A, () => getRoleByName(ROLE_NAME));
|
||||
await runAs(TENANT_B, () => getRoleByName(ROLE_NAME));
|
||||
|
||||
expect(cache.store.has(`${ROLE_NAME}:${TENANT_A}`)).toBe(true);
|
||||
expect(cache.store.has(`${ROLE_NAME}:${TENANT_B}`)).toBe(true);
|
||||
expect(cache.store.has(ROLE_NAME)).toBe(false);
|
||||
});
|
||||
|
||||
it('serves the cached value within the same tenant without a second DB read', async () => {
|
||||
await seedRole(TENANT_A, true);
|
||||
|
||||
const first = await runAs(TENANT_A, () => getRoleByName(ROLE_NAME));
|
||||
expect(usePromptsPermission(first)).toBe(true);
|
||||
|
||||
const findOneSpy = jest.spyOn(Role, 'findOne');
|
||||
const second = await runAs(TENANT_A, () => getRoleByName(ROLE_NAME));
|
||||
expect(usePromptsPermission(second)).toBe(true);
|
||||
expect(findOneSpy).not.toHaveBeenCalled();
|
||||
findOneSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('uses the unscoped key when no tenant context is active (single-tenant)', async () => {
|
||||
const previousDefault = process.env.DEFAULT_TENANT_ID;
|
||||
delete process.env.DEFAULT_TENANT_ID;
|
||||
try {
|
||||
await seedRole(TENANT_A, true);
|
||||
|
||||
const role = await getRoleByName(ROLE_NAME);
|
||||
expect(usePromptsPermission(role)).toBe(true);
|
||||
expect(cache.store.has(ROLE_NAME)).toBe(true);
|
||||
expect(cache.store.has(`${ROLE_NAME}:${TENANT_A}`)).toBe(false);
|
||||
} finally {
|
||||
if (previousDefault === undefined) {
|
||||
delete process.env.DEFAULT_TENANT_ID;
|
||||
} else {
|
||||
process.env.DEFAULT_TENANT_ID = previousDefault;
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -7,6 +7,7 @@ import {
|
|||
} from 'librechat-data-provider';
|
||||
import type { Model } from 'mongoose';
|
||||
import type { IRole, IUser } from '~/types';
|
||||
import { scopedCacheKey } from '~/config/tenantContext';
|
||||
import logger from '~/config/winston';
|
||||
|
||||
const systemRoleValues = new Set<string>(Object.values(SystemRoles));
|
||||
|
|
@ -94,7 +95,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
|
|||
const cache = deps.getCache?.(CacheKeys.ROLES);
|
||||
try {
|
||||
if (cache) {
|
||||
const cachedRole = await cache.get(roleName);
|
||||
const cachedRole = await cache.get(scopedCacheKey(roleName));
|
||||
if (cachedRole) {
|
||||
return cachedRole as IRole;
|
||||
}
|
||||
|
|
@ -109,12 +110,12 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
|
|||
if (!role && systemRoleValues.has(roleName)) {
|
||||
const newRole = await new Role(roleDefaults[roleName as keyof typeof roleDefaults]).save();
|
||||
if (cache) {
|
||||
await cache.set(roleName, newRole);
|
||||
await cache.set(scopedCacheKey(roleName), newRole);
|
||||
}
|
||||
return newRole.toObject() as IRole;
|
||||
}
|
||||
if (cache) {
|
||||
await cache.set(roleName, role);
|
||||
await cache.set(scopedCacheKey(roleName), role);
|
||||
}
|
||||
return role as unknown as IRole;
|
||||
} catch (error) {
|
||||
|
|
@ -135,9 +136,12 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
|
|||
.exec();
|
||||
if (cache) {
|
||||
if (updates.name && updates.name !== roleName) {
|
||||
await Promise.all([cache.set(roleName, null), cache.set(updates.name, role)]);
|
||||
await Promise.all([
|
||||
cache.set(scopedCacheKey(roleName), null),
|
||||
cache.set(scopedCacheKey(updates.name), role),
|
||||
]);
|
||||
} else {
|
||||
await cache.set(roleName, role);
|
||||
await cache.set(scopedCacheKey(roleName), role);
|
||||
}
|
||||
}
|
||||
return role as unknown as IRole;
|
||||
|
|
@ -296,7 +300,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
|
|||
const cache = deps.getCache?.(CacheKeys.ROLES);
|
||||
const updatedRole = await Role.findOne({ name: roleName }).select('-__v').lean().exec();
|
||||
if (cache) {
|
||||
await cache.set(roleName, updatedRole);
|
||||
await cache.set(scopedCacheKey(roleName), updatedRole);
|
||||
}
|
||||
|
||||
logger.info(`Updated role '${roleName}' and removed old schema fields`);
|
||||
|
|
@ -366,7 +370,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
|
|||
const cache = deps.getCache?.(CacheKeys.ROLES);
|
||||
if (cache) {
|
||||
const updatedRole = await Role.findById(role._id).lean().exec();
|
||||
await cache.set(role.name, updatedRole);
|
||||
await cache.set(scopedCacheKey(role.name), updatedRole);
|
||||
}
|
||||
|
||||
migratedCount++;
|
||||
|
|
@ -418,7 +422,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
|
|||
try {
|
||||
const cache = deps.getCache?.(CacheKeys.ROLES);
|
||||
if (cache) {
|
||||
await cache.set(role.name, role.toObject());
|
||||
await cache.set(scopedCacheKey(role.name), role.toObject());
|
||||
}
|
||||
} catch (cacheError) {
|
||||
logger.error(`[createRoleByName] cache set failed for "${role.name}":`, cacheError);
|
||||
|
|
@ -454,7 +458,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
|
|||
// Setting null evicts the stale document. getRoleByName treats falsy cached
|
||||
// values as a miss and falls through to the DB, so this does not provide
|
||||
// negative caching — it only prevents serving the pre-deletion document.
|
||||
await cache.set(roleName, null);
|
||||
await cache.set(scopedCacheKey(roleName), null);
|
||||
}
|
||||
} catch (cacheError) {
|
||||
logger.error(`[deleteRoleByName] cache invalidation failed for "${roleName}":`, cacheError);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue