diff --git a/api/server/middleware/__tests__/requireJwtAuth.spec.js b/api/server/middleware/__tests__/requireJwtAuth.spec.js index 6e1eba6a82..08f779fac0 100644 --- a/api/server/middleware/__tests__/requireJwtAuth.spec.js +++ b/api/server/middleware/__tests__/requireJwtAuth.spec.js @@ -71,6 +71,19 @@ jest.mock('@librechat/api', () => { recordRumProxyRequest: jest.fn(), getAuthFailureReasonCategory: actualApi.getAuthFailureReasonCategory, buildSafeAuthLogContext: actualApi.buildSafeAuthLogContext, + getValidOpenIdReuseUserId: (token) => { + if (!token || !process.env.JWT_REFRESH_SECRET) { + return null; + } + try { + const payload = require('jsonwebtoken').verify(token, process.env.JWT_REFRESH_SECRET); + return typeof payload === 'object' && payload != null && typeof payload.id === 'string' + ? payload.id + : null; + } catch { + return null; + } + }, maybeRefreshCloudFrontAuthCookiesMiddleware: jest.fn((req, res, next) => next()), tenantContextMiddleware: (req, res, next) => { const context = { diff --git a/api/server/middleware/requireJwtAuth.js b/api/server/middleware/requireJwtAuth.js index 8457018320..251fef9958 100644 --- a/api/server/middleware/requireJwtAuth.js +++ b/api/server/middleware/requireJwtAuth.js @@ -1,5 +1,4 @@ const cookies = require('cookie'); -const jwt = require('jsonwebtoken'); const passport = require('passport'); const { logger } = require('@librechat/data-schemas'); const { @@ -9,27 +8,12 @@ const { buildSafeAuthLogContext, maybeRefreshCloudFrontAuthCookiesMiddleware, recordRumProxyRequest, + getValidOpenIdReuseUserId, } = require('@librechat/api'); const hasPassportStrategy = (strategy) => typeof passport._strategy === 'function' && passport._strategy(strategy) != null; -const getValidOpenIdReuseUserId = (parsedCookies) => { - const openidUserId = parsedCookies.openid_user_id; - if (!openidUserId || !process.env.JWT_REFRESH_SECRET) { - return null; - } - - try { - const payload = jwt.verify(openidUserId, process.env.JWT_REFRESH_SECRET); - return typeof payload === 'object' && payload != null && typeof payload.id === 'string' - ? payload.id - : null; - } catch { - return null; - } -}; - const getAuthenticatedUserId = (user) => user?.id?.toString?.() ?? user?._id?.toString?.(); const refreshCloudFrontCookies = maybeRefreshCloudFrontAuthCookiesMiddleware ?? ((_req, _res, next) => next()); @@ -46,7 +30,7 @@ const getAuthStrategies = (req) => { const tokenProvider = parsedCookies.token_provider; const openidReuseEnabled = isEnabled(process.env.OPENID_REUSE_TOKENS); const openidJwtAvailable = openidReuseEnabled && hasPassportStrategy('openidJwt'); - const openIdReuseUserId = getValidOpenIdReuseUserId(parsedCookies); + const openIdReuseUserId = getValidOpenIdReuseUserId(parsedCookies.openid_user_id); const useOpenIdJwt = tokenProvider === 'openid' && openidJwtAvailable && openIdReuseUserId != null; diff --git a/api/strategies/openIdJwtStrategy.js b/api/strategies/openIdJwtStrategy.js index 0bdf14b8b6..3606fe474b 100644 --- a/api/strategies/openIdJwtStrategy.js +++ b/api/strategies/openIdJwtStrategy.js @@ -1,6 +1,6 @@ const cookies = require('cookie'); const jwksRsa = require('jwks-rsa'); -const { logger } = require('@librechat/data-schemas'); +const { logger, getTenantId } = require('@librechat/data-schemas'); const { CacheKeys, SystemRoles } = require('librechat-data-provider'); const { Strategy: JwtStrategy, ExtractJwt } = require('passport-jwt'); const { @@ -12,6 +12,7 @@ const { buildAuthUserDocCacheKey, getAuthUserDocCacheMode, getCachedAuthUserDoc, + getValidOpenIdReuseUserId, invalidateCachedAuthUserDoc, setCachedAuthUserDoc, getHttpsProxyAgent, @@ -55,6 +56,28 @@ const isOpenIdIssuerAllowed = (payload, openIdConfig) => { const getAuthUserDocCacheStore = () => getLogStores(CacheKeys.AUTH_USER_DOC); +const getUserId = (user) => user?.id?.toString?.() ?? user?._id?.toString?.(); + +const getAuthUserCacheScope = (tenantId, userId) => { + if (tenantId) { + return { tenantId }; + } + if (userId) { + return { userId }; + } + return {}; +}; + +const isUserInAuthCacheScope = (user, { tenantId, userId }) => { + if (tenantId) { + return (user?.tenantId || undefined) === tenantId; + } + if (userId) { + return getUserId(user) === userId; + } + return !user?.tenantId; +}; + /** * @function openIdJwtLogin * @param {import('openid-client').Configuration} openIdConfig - Configuration object for the JWT strategy. @@ -108,10 +131,16 @@ const openIdJwtLogin = (openIdConfig) => { const authHeader = req.headers.authorization; const rawToken = authHeader?.replace('Bearer ', ''); const openidIssuer = getOpenIdIssuer(payload, openIdConfig); + const tenantId = getTenantId(); + const cookieHeader = req.headers.cookie; + const parsedCookies = cookieHeader ? cookies.parse(cookieHeader) : {}; + const openIdReuseUserId = getValidOpenIdReuseUserId(parsedCookies.openid_user_id); + const authUserCacheScope = getAuthUserCacheScope(tenantId, openIdReuseUserId); const authUserCacheKey = buildAuthUserDocCacheKey({ strategy: 'openid-jwt', subject: payload?.sub, issuer: openidIssuer, + ...authUserCacheScope, }); const authUserCacheMode = getAuthUserDocCacheMode(); const authUserCacheStore = @@ -121,7 +150,10 @@ const openIdJwtLogin = (openIdConfig) => { ? await getCachedAuthUserDoc(authUserCacheStore, authUserCacheKey) : undefined; - const servedCachedUser = authUserCacheMode === 'on' && cachedUser; + const servedCachedUser = + authUserCacheMode === 'on' && + cachedUser && + isUserInAuthCacheScope(cachedUser, authUserCacheScope); const lookupResult = servedCachedUser ? { user: cachedUser, error: null, migration: false } : await findOpenIDUser({ @@ -167,7 +199,7 @@ const openIdJwtLogin = (openIdConfig) => { userId: user.id, cacheKey: authUserCacheKey, }); - } else if (!servedCachedUser) { + } else if (!servedCachedUser && isUserInAuthCacheScope(user, authUserCacheScope)) { await setCachedAuthUserDoc(authUserCacheStore, authUserCacheKey, user); } } @@ -180,8 +212,6 @@ const openIdJwtLogin = (openIdConfig) => { /** Fallback to cookies for backward compatibility */ if (!accessToken || !refreshToken || !idToken) { - const cookieHeader = req.headers.cookie; - const parsedCookies = cookieHeader ? cookies.parse(cookieHeader) : {}; accessToken = accessToken || parsedCookies.openid_access_token; idToken = idToken || parsedCookies.openid_id_token; refreshToken = refreshToken || parsedCookies.refreshToken; diff --git a/api/strategies/openIdJwtStrategy.spec.js b/api/strategies/openIdJwtStrategy.spec.js index 076fd99337..a1993aa5c7 100644 --- a/api/strategies/openIdJwtStrategy.spec.js +++ b/api/strategies/openIdJwtStrategy.spec.js @@ -9,6 +9,7 @@ const mockAuthUserDocCacheStore = { delete: jest.fn(), }; const mockGetLogStores = jest.fn(() => mockAuthUserDocCacheStore); +const mockGetTenantId = jest.fn(); jest.mock('passport-jwt', () => ({ Strategy: jest.fn((opts, verifyCallback) => { capturedStrategyOptions = opts; @@ -27,6 +28,7 @@ jest.mock('https-proxy-agent', () => ({ })); jest.mock('@librechat/data-schemas', () => ({ logger: { info: jest.fn(), warn: jest.fn(), debug: jest.fn(), error: jest.fn() }, + getTenantId: mockGetTenantId, })); jest.mock('@librechat/api', () => ({ isEnabled: jest.fn(() => false), @@ -37,6 +39,7 @@ jest.mock('@librechat/api', () => ({ buildAuthUserDocCacheKey: jest.fn(() => 'auth-user-doc-key'), getAuthUserDocCacheMode: jest.fn(() => 'off'), getCachedAuthUserDoc: jest.fn(), + getValidOpenIdReuseUserId: jest.fn(), invalidateCachedAuthUserDoc: jest.fn(), setCachedAuthUserDoc: jest.fn(), getHttpsProxyAgent: jest.fn(() => undefined), @@ -61,6 +64,7 @@ const { findOpenIDUser, getAuthUserDocCacheMode, getCachedAuthUserDoc, + getValidOpenIdReuseUserId, invalidateCachedAuthUserDoc, setCachedAuthUserDoc, } = require('@librechat/api'); @@ -68,6 +72,7 @@ const openIdJwtLogin = require('./openIdJwtStrategy'); const { findUser, updateUser } = require('~/models'); function resetAuthUserDocCacheMocks() { + mockGetTenantId.mockReturnValue(undefined); mockAuthUserDocCacheStore.get.mockResolvedValue(undefined); mockAuthUserDocCacheStore.set.mockResolvedValue(undefined); mockAuthUserDocCacheStore.delete.mockResolvedValue(undefined); @@ -75,6 +80,7 @@ function resetAuthUserDocCacheMocks() { buildAuthUserDocCacheKey.mockReturnValue('auth-user-doc-key'); getAuthUserDocCacheMode.mockReturnValue('off'); getCachedAuthUserDoc.mockResolvedValue(undefined); + getValidOpenIdReuseUserId.mockReturnValue(null); invalidateCachedAuthUserDoc.mockResolvedValue(undefined); setCachedAuthUserDoc.mockResolvedValue(undefined); } @@ -416,11 +422,13 @@ describe('openIdJwtStrategy – auth user document cache', () => { }); it('uses the cached user document in on mode without a database lookup', async () => { + mockGetTenantId.mockReturnValue('tenant-a'); const cachedUser = { _id: 'cached-user', role: SystemRoles.USER, provider: 'openid', email: 'cached@example.com', + tenantId: 'tenant-a', }; getAuthUserDocCacheMode.mockReturnValue('on'); getCachedAuthUserDoc.mockResolvedValue(cachedUser); @@ -431,6 +439,7 @@ describe('openIdJwtStrategy – auth user document cache', () => { strategy: 'openid-jwt', subject: payload.sub, issuer: 'https://issuer.example.com', + tenantId: 'tenant-a', }); expect(findOpenIDUser).not.toHaveBeenCalled(); expect(user).toMatchObject({ @@ -458,6 +467,88 @@ describe('openIdJwtStrategy – auth user document cache', () => { expect(invalidateCachedAuthUserDoc).not.toHaveBeenCalled(); }); + it('rejects a cached user document from another tenant', async () => { + mockGetTenantId.mockReturnValue('tenant-b'); + getAuthUserDocCacheMode.mockReturnValue('on'); + getCachedAuthUserDoc.mockResolvedValue({ + _id: 'tenant-a-user', + role: SystemRoles.ADMIN, + provider: 'openid', + email: 'cached@example.com', + tenantId: 'tenant-a', + }); + findOpenIDUser.mockResolvedValue({ + user: { ...baseUser, _id: { toString: () => 'tenant-b-user' }, tenantId: 'tenant-b' }, + error: null, + migration: false, + }); + + const { user } = await invokeVerify(req, payload); + + expect(buildAuthUserDocCacheKey).toHaveBeenCalledWith({ + strategy: 'openid-jwt', + subject: payload.sub, + issuer: 'https://issuer.example.com', + tenantId: 'tenant-b', + }); + expect(findOpenIDUser).toHaveBeenCalled(); + expect(user).toMatchObject({ id: 'tenant-b-user', tenantId: 'tenant-b' }); + expect(setCachedAuthUserDoc).toHaveBeenCalledWith( + mockAuthUserDocCacheStore, + 'auth-user-doc-key', + expect.objectContaining({ id: 'tenant-b-user', tenantId: 'tenant-b' }), + ); + }); + + it('uses the signed OpenID user id as cache scope before tenant context is available', async () => { + getAuthUserDocCacheMode.mockReturnValue('on'); + getValidOpenIdReuseUserId.mockReturnValue('tenant-a-user'); + getCachedAuthUserDoc.mockResolvedValue({ + _id: 'tenant-a-user', + role: SystemRoles.USER, + provider: 'openid', + email: 'cached@example.com', + tenantId: 'tenant-a', + }); + + const { user } = await invokeVerify( + { + headers: { + authorization: 'Bearer tok', + cookie: 'openid_user_id=signed-user-id', + }, + session: {}, + }, + payload, + ); + + expect(getValidOpenIdReuseUserId).toHaveBeenCalledWith('signed-user-id'); + expect(buildAuthUserDocCacheKey).toHaveBeenCalledWith({ + strategy: 'openid-jwt', + subject: payload.sub, + issuer: 'https://issuer.example.com', + userId: 'tenant-a-user', + }); + expect(findOpenIDUser).not.toHaveBeenCalled(); + expect(user).toMatchObject({ id: 'tenant-a-user', tenantId: 'tenant-a' }); + }); + + it('does not cache a lookup result outside the active tenant scope', async () => { + mockGetTenantId.mockReturnValue('tenant-b'); + getAuthUserDocCacheMode.mockReturnValue('on'); + getCachedAuthUserDoc.mockResolvedValue(undefined); + findOpenIDUser.mockResolvedValue({ + user: { ...baseUser, tenantId: 'tenant-a' }, + error: null, + migration: false, + }); + + await invokeVerify(req, payload); + + expect(findOpenIDUser).toHaveBeenCalled(); + expect(setCachedAuthUserDoc).not.toHaveBeenCalled(); + }); + it('invalidates instead of populating when login mutates the user', async () => { getAuthUserDocCacheMode.mockReturnValue('on'); findOpenIDUser.mockResolvedValue({ diff --git a/packages/api/src/auth/index.ts b/packages/api/src/auth/index.ts index 1932986e3b..f2b4b2adde 100644 --- a/packages/api/src/auth/index.ts +++ b/packages/api/src/auth/index.ts @@ -10,3 +10,4 @@ export * from './invite'; export * from './codeapi'; export * from './openidRoleSync'; export * from './userDocCache'; +export * from './reuse'; diff --git a/packages/api/src/auth/reuse.spec.ts b/packages/api/src/auth/reuse.spec.ts new file mode 100644 index 0000000000..b49b0ec9ba --- /dev/null +++ b/packages/api/src/auth/reuse.spec.ts @@ -0,0 +1,18 @@ +import jwt from 'jsonwebtoken'; +import { getValidOpenIdReuseUserId } from './reuse'; + +const secret = 'test-refresh-secret'; + +describe('getValidOpenIdReuseUserId', () => { + it('returns the signed OpenID user id', () => { + const token = jwt.sign({ id: 'user-a' }, secret); + + expect(getValidOpenIdReuseUserId(token, secret)).toBe('user-a'); + }); + + it('rejects missing or invalid signed user ids', () => { + expect(getValidOpenIdReuseUserId(undefined, secret)).toBeNull(); + expect(getValidOpenIdReuseUserId('invalid-token', secret)).toBeNull(); + expect(getValidOpenIdReuseUserId(jwt.sign({ sub: 'user-a' }, secret), secret)).toBeNull(); + }); +}); diff --git a/packages/api/src/auth/reuse.ts b/packages/api/src/auth/reuse.ts new file mode 100644 index 0000000000..b5315aa23f --- /dev/null +++ b/packages/api/src/auth/reuse.ts @@ -0,0 +1,19 @@ +import jwt from 'jsonwebtoken'; + +export function getValidOpenIdReuseUserId( + openidUserId: string | undefined, + secret: string | undefined = process.env.JWT_REFRESH_SECRET, +): string | null { + if (!openidUserId || !secret) { + return null; + } + + try { + const payload = jwt.verify(openidUserId, secret); + return typeof payload === 'object' && payload != null && typeof payload.id === 'string' + ? payload.id + : null; + } catch { + return null; + } +} diff --git a/packages/api/src/auth/userDocCache.spec.ts b/packages/api/src/auth/userDocCache.spec.ts index e052b77f33..4c70c438ab 100644 --- a/packages/api/src/auth/userDocCache.spec.ts +++ b/packages/api/src/auth/userDocCache.spec.ts @@ -86,28 +86,61 @@ describe('auth user document cache helpers', () => { expect(getAuthUserDocCacheMode()).toBe('off'); }); - it('builds stable keys from strategy, subject, issuer, and scope', () => { + it('builds stable keys from strategy, subject, issuer, tenant, user, and scope', () => { const key = buildAuthUserDocCacheKey({ strategy: ' OpenID-JWT ', subject: 'subject-1', issuer: 'https://issuer.example.com/', + tenantId: 'Tenant-A', + userId: 'User-A', scope: ' Org-A ', }); const equivalent = buildAuthUserDocCacheKey({ strategy: 'openid-jwt', subject: 'subject-1', issuer: 'https://issuer.example.com', + tenantId: 'Tenant-A', + userId: 'User-A', + scope: 'org-a', + }); + const otherTenant = buildAuthUserDocCacheKey({ + strategy: 'openid-jwt', + subject: 'subject-1', + issuer: 'https://issuer.example.com', + tenantId: 'Tenant-B', + userId: 'User-A', + scope: 'org-a', + }); + const caseVariantTenant = buildAuthUserDocCacheKey({ + strategy: 'openid-jwt', + subject: 'subject-1', + issuer: 'https://issuer.example.com', + tenantId: 'tenant-a', + userId: 'User-A', + scope: 'org-a', + }); + const otherUser = buildAuthUserDocCacheKey({ + strategy: 'openid-jwt', + subject: 'subject-1', + issuer: 'https://issuer.example.com', + tenantId: 'Tenant-A', + userId: 'User-B', scope: 'org-a', }); const otherScope = buildAuthUserDocCacheKey({ strategy: 'openid-jwt', subject: 'subject-1', issuer: 'https://issuer.example.com', + tenantId: 'Tenant-A', + userId: 'User-A', scope: 'org-b', }); - expect(key).toMatch(/^auth-user-doc:v1:/); + expect(key).toMatch(/^auth-user-doc:v2:/); expect(key).toBe(equivalent); + expect(key).not.toBe(otherTenant); + expect(key).not.toBe(caseVariantTenant); + expect(key).not.toBe(otherUser); expect(key).not.toBe(otherScope); expect(buildAuthUserDocCacheKey({ strategy: '', subject: 'subject-1' })).toBeUndefined(); expect(buildAuthUserDocCacheKey({ strategy: 'openid-jwt' })).toBeUndefined(); @@ -115,7 +148,7 @@ describe('auth user document cache helpers', () => { it('sanitizes sensitive fields and remembers cache keys by user id', async () => { const store = makeStore(); - const cacheKey = 'auth-user-doc:v1:key'; + const cacheKey = 'auth-user-doc:v2:key'; const userId = new Types.ObjectId(); await setCachedAuthUserDoc(store, cacheKey, { @@ -146,7 +179,7 @@ describe('auth user document cache helpers', () => { expect(store.set).toHaveBeenCalledWith( cacheKey, - expect.objectContaining({ version: 1, user: expect.any(Object) }), + expect.objectContaining({ version: 2, user: expect.any(Object) }), AUTH_USER_DOC_CACHE_TTL_MS, ); expect(store.values.get(buildAuthUserDocReverseIndexKey(userId.toString()))).toEqual([ @@ -187,8 +220,8 @@ describe('auth user document cache helpers', () => { it('returns cached user documents only for the current cache version', async () => { const store = makeStore(); - store.values.set('current', { version: 1, cachedAt: Date.now(), user: { id: 'user-1' } }); - store.values.set('stale', { version: 0, cachedAt: Date.now(), user: { id: 'user-2' } }); + store.values.set('current', { version: 2, cachedAt: Date.now(), user: { id: 'user-1' } }); + store.values.set('stale', { version: 1, cachedAt: Date.now(), user: { id: 'user-2' } }); await expect(getCachedAuthUserDoc(store, 'current')).resolves.toEqual({ id: 'user-1' }); await expect(getCachedAuthUserDoc(store, 'stale')).resolves.toBeUndefined(); diff --git a/packages/api/src/auth/userDocCache.ts b/packages/api/src/auth/userDocCache.ts index 95be55fd64..7a308e3823 100644 --- a/packages/api/src/auth/userDocCache.ts +++ b/packages/api/src/auth/userDocCache.ts @@ -4,7 +4,7 @@ import { AUTH_USER_DOC_BY_ID_PREFIX, CacheKeys } from 'librechat-data-provider'; import type { IUser } from '@librechat/data-schemas'; import { cacheConfig } from '~/cache/cacheConfig'; -const AUTH_USER_DOC_CACHE_VERSION = 1; +const AUTH_USER_DOC_CACHE_VERSION = 2; export const AUTH_USER_DOC_CACHE_TTL_MS = 5000; export type AuthUserDocCacheMode = 'off' | 'on'; @@ -19,6 +19,8 @@ export interface AuthUserDocCacheKeyInput { strategy: string; subject?: string; issuer?: string; + tenantId?: string; + userId?: string; scope?: string; } @@ -71,6 +73,10 @@ function normalizeKeyPart(value: string | undefined): string { return (value ?? '').trim().toLowerCase().replace(/\/+$/, ''); } +function normalizeExactKeyPart(value: string | undefined): string { + return (value ?? '').trim(); +} + export function buildAuthUserDocCacheKey(input: AuthUserDocCacheKeyInput): string | undefined { const strategy = input.strategy.trim(); const subject = input.subject?.trim(); @@ -84,6 +90,8 @@ export function buildAuthUserDocCacheKey(input: AuthUserDocCacheKeyInput): strin normalizeKeyPart(strategy), subject, normalizeKeyPart(input.issuer), + normalizeExactKeyPart(input.tenantId), + normalizeExactKeyPart(input.userId), normalizeKeyPart(input.scope), ].join('\0'), )