diff --git a/api/server/middleware/__tests__/requireJwtAuth.spec.js b/api/server/middleware/__tests__/requireJwtAuth.spec.js index a884cb5b3f..317e8ae6b4 100644 --- a/api/server/middleware/__tests__/requireJwtAuth.spec.js +++ b/api/server/middleware/__tests__/requireJwtAuth.spec.js @@ -8,7 +8,7 @@ * If the chaining is removed, these tests fail. */ -const { getTenantId } = require('@librechat/data-schemas'); +const jwt = require('jsonwebtoken'); // ── Mocks ────────────────────────────────────────────────────────────── @@ -36,6 +36,15 @@ jest.mock('passport', () => ({ }), })); +jest.mock('@librechat/data-schemas', () => { + const { AsyncLocalStorage } = require('async_hooks'); + const tenantStorage = new AsyncLocalStorage(); + return { + getTenantId: () => tenantStorage.getStore()?.tenantId, + tenantStorage, + }; +}); + // Mock @librechat/api — the real tenantContextMiddleware is TS and cannot be // required directly from CJS tests. This thin wrapper mirrors the real logic // (read req.user.tenantId, call tenantStorage.run) using the same data-schemas @@ -57,13 +66,20 @@ jest.mock('@librechat/api', () => { // ── Helpers ───────────────────────────────────────────────────────────── const requireJwtAuth = require('../requireJwtAuth'); +const { getTenantId } = require('@librechat/data-schemas'); const { isEnabled } = require('@librechat/api'); const passport = require('passport'); +const jwtSecret = 'test-refresh-secret'; + function mockReq(user, extra = {}) { return { headers: {}, _mockUser: user, ...extra }; } +function signedOpenIdUserCookie(userId = 'user-openid') { + return jwt.sign({ id: userId }, jwtSecret); +} + function mockRes() { return { status: jest.fn().mockReturnThis(), json: jest.fn().mockReturnThis() }; } @@ -82,12 +98,23 @@ function runAuth(user) { // ── Tests ────────────────────────────────────────────────────────────── describe('requireJwtAuth tenant context chaining', () => { + const originalJwtSecret = process.env.JWT_REFRESH_SECRET; + + beforeEach(() => { + process.env.JWT_REFRESH_SECRET = jwtSecret; + }); + afterEach(() => { mockPassportError = null; mockRegisteredStrategies = new Set(['jwt']); isEnabled.mockReturnValue(false); passport.authenticate.mockClear(); passport._strategy.mockClear(); + if (originalJwtSecret === undefined) { + delete process.env.JWT_REFRESH_SECRET; + } else { + process.env.JWT_REFRESH_SECRET = originalJwtSecret; + } }); it('forwards passport errors to next() without entering tenant middleware', async () => { @@ -124,7 +151,7 @@ describe('requireJwtAuth tenant context chaining', () => { expect(getTenantId()).toBeUndefined(); }); - it('falls back to OpenID JWT for bearer-only reuse requests', async () => { + it('does not fall back to OpenID JWT for bearer-only reuse requests', () => { isEnabled.mockReturnValue(true); mockRegisteredStrategies.add('openidJwt'); const req = mockReq(undefined, { @@ -134,6 +161,32 @@ describe('requireJwtAuth tenant context chaining', () => { }, }); const res = mockRes(); + const next = jest.fn(); + + requireJwtAuth(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(req.authStrategy).toBeUndefined(); + expect(passport.authenticate).toHaveBeenCalledTimes(1); + expect(passport.authenticate).toHaveBeenCalledWith( + 'jwt', + { session: false }, + expect.any(Function), + ); + }); + + it('uses OpenID JWT before LibreChat JWT when the OpenID cookie is present', async () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const req = mockReq(undefined, { + headers: { cookie: `token_provider=openid; openid_user_id=${signedOpenIdUserCookie()}` }, + _mockStrategies: { + openidJwt: { user: { id: 'user-openid', tenantId: 'tenant-openid', role: 'user' } }, + jwt: { user: false, info: { message: 'invalid signature' }, status: 401 }, + }, + }); + const res = mockRes(); const tenantId = await new Promise((resolve) => { requireJwtAuth(req, res, () => { resolve(getTenantId()); @@ -143,6 +196,98 @@ describe('requireJwtAuth tenant context chaining', () => { expect(tenantId).toBe('tenant-openid'); expect(req.authStrategy).toBe('openidJwt'); expect(res.status).not.toHaveBeenCalled(); + expect(passport.authenticate).toHaveBeenCalledWith( + 'openidJwt', + { session: false }, + expect.any(Function), + ); + }); + + it('does not authenticate OpenID JWT when the reuse cookie belongs to another user', () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const req = mockReq(undefined, { + headers: { + cookie: `token_provider=openid; openid_user_id=${signedOpenIdUserCookie('user-a')}`, + }, + _mockStrategies: { + openidJwt: { user: { id: 'user-b', tenantId: 'tenant-openid', role: 'user' } }, + jwt: { user: false, info: { message: 'invalid signature' }, status: 401 }, + }, + }); + const res = mockRes(); + const next = jest.fn(); + + requireJwtAuth(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(req.authStrategy).toBeUndefined(); + expect(passport.authenticate).toHaveBeenCalledTimes(2); + expect(passport.authenticate).toHaveBeenNthCalledWith( + 1, + 'openidJwt', + { session: false }, + expect.any(Function), + ); + expect(passport.authenticate).toHaveBeenNthCalledWith( + 2, + 'jwt', + { session: false }, + expect.any(Function), + ); + }); + + it('does not use OpenID JWT when the signed OpenID reuse cookie is missing', () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const req = mockReq(undefined, { + headers: { cookie: 'token_provider=openid' }, + _mockStrategies: { + jwt: { user: false, info: { message: 'invalid signature' }, status: 401 }, + openidJwt: { user: { tenantId: 'tenant-openid', role: 'user' } }, + }, + }); + const res = mockRes(); + const next = jest.fn(); + + requireJwtAuth(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(req.authStrategy).toBeUndefined(); + expect(passport.authenticate).toHaveBeenCalledTimes(1); + expect(passport.authenticate).toHaveBeenCalledWith( + 'jwt', + { session: false }, + expect.any(Function), + ); + }); + + it('does not use OpenID JWT when the OpenID reuse cookie is invalid', () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const req = mockReq(undefined, { + headers: { cookie: 'token_provider=openid; openid_user_id=invalid-jwt' }, + _mockStrategies: { + jwt: { user: false, info: { message: 'invalid signature' }, status: 401 }, + openidJwt: { user: { tenantId: 'tenant-openid', role: 'user' } }, + }, + }); + const res = mockRes(); + const next = jest.fn(); + + requireJwtAuth(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(req.authStrategy).toBeUndefined(); + expect(passport.authenticate).toHaveBeenCalledTimes(1); + expect(passport.authenticate).toHaveBeenCalledWith( + 'jwt', + { session: false }, + expect.any(Function), + ); }); it('skips OpenID JWT fallback when the strategy was not registered', async () => { diff --git a/api/server/middleware/requireJwtAuth.js b/api/server/middleware/requireJwtAuth.js index 5a9e21b6d3..2820de5359 100644 --- a/api/server/middleware/requireJwtAuth.js +++ b/api/server/middleware/requireJwtAuth.js @@ -1,10 +1,29 @@ const cookies = require('cookie'); +const jwt = require('jsonwebtoken'); const passport = require('passport'); const { isEnabled, tenantContextMiddleware } = 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?.(); + /** * Custom Middleware to handle JWT authentication, with support for OpenID token reuse. * Switches between JWT and OpenID authentication based on cookies and environment settings. @@ -15,13 +34,14 @@ const hasPassportStrategy = (strategy) => */ const requireJwtAuth = (req, res, next) => { const cookieHeader = req.headers.cookie; - const tokenProvider = cookieHeader ? cookies.parse(cookieHeader).token_provider : null; + const parsedCookies = cookieHeader ? cookies.parse(cookieHeader) : {}; + const tokenProvider = parsedCookies.token_provider; const openidReuseEnabled = isEnabled(process.env.OPENID_REUSE_TOKENS); const openidJwtAvailable = openidReuseEnabled && hasPassportStrategy('openidJwt'); - const strategies = - tokenProvider === 'openid' && openidJwtAvailable - ? ['openidJwt', 'jwt'] - : ['jwt', ...(openidJwtAvailable ? ['openidJwt'] : [])]; + const openIdReuseUserId = getValidOpenIdReuseUserId(parsedCookies); + const useOpenIdJwt = + tokenProvider === 'openid' && openidJwtAvailable && openIdReuseUserId != null; + const strategies = useOpenIdJwt ? ['openidJwt', 'jwt'] : ['jwt']; const authenticateWithStrategy = (index) => { const strategy = strategies[index]; @@ -37,6 +57,12 @@ const requireJwtAuth = (req, res, next) => { message: info?.message || 'Unauthorized', }); } + if (strategy === 'openidJwt' && getAuthenticatedUserId(user) !== openIdReuseUserId) { + if (index + 1 < strategies.length) { + return authenticateWithStrategy(index + 1); + } + return res.status(401).json({ message: 'Unauthorized' }); + } req.user = user; req.authStrategy = strategy; // req.user is now populated by passport — set up tenant ALS context diff --git a/api/strategies/openIdJwtStrategy.js b/api/strategies/openIdJwtStrategy.js index 8af11bf99c..78145f1109 100644 --- a/api/strategies/openIdJwtStrategy.js +++ b/api/strategies/openIdJwtStrategy.js @@ -9,10 +9,40 @@ const { findOpenIDUser, getOpenIdEmail, getOpenIdIssuer, + normalizeOpenIdIssuer, math, } = require('@librechat/api'); const { updateUser, findUser } = require('~/models'); +const getOpenIdJwtAudience = () => { + const audiences = [process.env.OPENID_CLIENT_ID, process.env.OPENID_AUDIENCE].filter(Boolean); + const uniqueAudiences = [...new Set(audiences)]; + + return uniqueAudiences.length > 1 ? uniqueAudiences : uniqueAudiences[0]; +}; + +const escapeRegExp = (value) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +const issuerMatchesTemplate = (expectedIssuer, actualIssuer) => { + if (!expectedIssuer.includes('{tenantid}')) { + return false; + } + + const escapedTemplate = expectedIssuer.split('{tenantid}').map(escapeRegExp).join('[^/]+'); + return new RegExp(`^${escapedTemplate}$`).test(actualIssuer); +}; + +const isOpenIdIssuerAllowed = (payload, openIdConfig) => { + const actualIssuer = normalizeOpenIdIssuer(payload?.iss); + const expectedIssuer = normalizeOpenIdIssuer(openIdConfig.serverMetadata().issuer); + + if (!actualIssuer || !expectedIssuer) { + return false; + } + + return actualIssuer === expectedIssuer || issuerMatchesTemplate(expectedIssuer, actualIssuer); +}; + /** * @function openIdJwtLogin * @param {import('openid-client').Configuration} openIdConfig - Configuration object for the JWT strategy. @@ -47,6 +77,7 @@ const openIdJwtLogin = (openIdConfig) => { { jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), secretOrKeyProvider: jwksRsa.passportJwtSecret(jwksRsaOptions), + audience: getOpenIdJwtAudience(), passReqToCallback: true, }, /** @@ -56,6 +87,11 @@ const openIdJwtLogin = (openIdConfig) => { */ async (req, payload, done) => { try { + if (!isOpenIdIssuerAllowed(payload, openIdConfig)) { + done(null, false, { message: 'Invalid issuer' }); + return; + } + const authHeader = req.headers.authorization; const rawToken = authHeader?.replace('Bearer ', ''); const openidIssuer = getOpenIdIssuer(payload, openIdConfig); diff --git a/api/strategies/openIdJwtStrategy.spec.js b/api/strategies/openIdJwtStrategy.spec.js index e3bc9f6e28..59229a3d15 100644 --- a/api/strategies/openIdJwtStrategy.spec.js +++ b/api/strategies/openIdJwtStrategy.spec.js @@ -1,9 +1,11 @@ const { SystemRoles } = require('librechat-data-provider'); -// --- Capture the verify callback from JwtStrategy --- +// --- Capture JwtStrategy inputs --- +let capturedStrategyOptions; let capturedVerifyCallback; jest.mock('passport-jwt', () => ({ - Strategy: jest.fn((_opts, verifyCallback) => { + Strategy: jest.fn((opts, verifyCallback) => { + capturedStrategyOptions = opts; capturedVerifyCallback = verifyCallback; return { name: 'jwt' }; }), @@ -25,6 +27,7 @@ jest.mock('@librechat/api', () => ({ findOpenIDUser: jest.fn(), getOpenIdEmail: jest.requireActual('@librechat/api').getOpenIdEmail, getOpenIdIssuer: jest.fn(() => 'https://issuer.example.com'), + normalizeOpenIdIssuer: jest.requireActual('@librechat/api').normalizeOpenIdIssuer, math: jest.fn((val, fallback) => fallback), })); jest.mock('~/models', () => ({ @@ -47,6 +50,28 @@ const { findOpenIDUser } = require('@librechat/api'); const openIdJwtLogin = require('./openIdJwtStrategy'); const { findUser, updateUser } = require('~/models'); +function withEnv(env, callback) { + const previous = Object.fromEntries(Object.keys(env).map((key) => [key, process.env[key]])); + Object.entries(env).forEach(([key, value]) => { + if (value === undefined) { + delete process.env[key]; + return; + } + process.env[key] = value; + }); + try { + callback(); + } finally { + Object.entries(previous).forEach(([key, value]) => { + if (value === undefined) { + delete process.env[key]; + return; + } + process.env[key] = value; + }); + } +} + // Helper: build a mock openIdConfig const mockOpenIdConfig = { serverMetadata: () => ({ @@ -67,6 +92,79 @@ async function invokeVerify(req, payload) { }); } +describe('openIdJwtStrategy – token validation', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('requires OpenID JWTs to match the configured client audience and issuer', () => { + withEnv({ OPENID_CLIENT_ID: 'librechat-client-id', OPENID_AUDIENCE: undefined }, () => { + openIdJwtLogin(mockOpenIdConfig); + }); + + expect(capturedStrategyOptions).toMatchObject({ + audience: 'librechat-client-id', + passReqToCallback: true, + }); + expect(capturedStrategyOptions).not.toHaveProperty('issuer'); + }); + + it('also accepts OPENID_AUDIENCE for providers that mint resource-bound JWTs', () => { + withEnv({ OPENID_CLIENT_ID: 'librechat-client-id', OPENID_AUDIENCE: 'api://librechat' }, () => { + openIdJwtLogin(mockOpenIdConfig); + }); + + expect(capturedStrategyOptions).toMatchObject({ + audience: ['librechat-client-id', 'api://librechat'], + }); + }); + + it('rejects OpenID JWTs whose issuer does not match the configured issuer', async () => { + findOpenIDUser.mockResolvedValue({ user: null, error: null, migration: false }); + openIdJwtLogin(mockOpenIdConfig); + + const req = { headers: { authorization: 'Bearer tok' }, session: {} }; + const { user, info } = await invokeVerify(req, { + sub: 'oidc-123', + email: 'test@example.com', + iss: 'https://other-issuer.example.com', + exp: 9999999999, + }); + + expect(user).toBe(false); + expect(info).toEqual({ message: 'Invalid issuer' }); + expect(findOpenIDUser).not.toHaveBeenCalled(); + }); + + it('allows Microsoft Entra tenant issuer values for tenant-independent metadata', async () => { + const entraConfig = { + serverMetadata: () => ({ + issuer: 'https://login.microsoftonline.com/{tenantid}/v2.0', + jwks_uri: 'https://login.microsoftonline.com/common/discovery/v2.0/keys', + }), + }; + const user = { + _id: { toString: () => 'user-abc' }, + role: SystemRoles.USER, + provider: 'openid', + }; + findOpenIDUser.mockResolvedValue({ user, error: null, migration: false }); + updateUser.mockResolvedValue({}); + openIdJwtLogin(entraConfig); + + const req = { headers: { authorization: 'Bearer tok' }, session: {} }; + const { user: result } = await invokeVerify(req, { + sub: 'oidc-123', + email: 'test@example.com', + iss: 'https://login.microsoftonline.com/11111111-2222-3333-4444-555555555555/v2.0', + exp: 9999999999, + }); + + expect(result).toBeTruthy(); + expect(findOpenIDUser).toHaveBeenCalled(); + }); +}); + describe('openIdJwtStrategy – token source handling', () => { const baseUser = { _id: { toString: () => 'user-abc' }, @@ -74,7 +172,12 @@ describe('openIdJwtStrategy – token source handling', () => { provider: 'openid', }; - const payload = { sub: 'oidc-123', email: 'test@example.com', exp: 9999999999 }; + const payload = { + sub: 'oidc-123', + email: 'test@example.com', + iss: 'https://issuer.example.com', + exp: 9999999999, + }; beforeEach(() => { jest.clearAllMocks(); @@ -204,6 +307,7 @@ describe('openIdJwtStrategy – OPENID_EMAIL_CLAIM', () => { email: 'test@example.com', preferred_username: 'testuser', upn: 'test@corp.example.com', + iss: 'https://issuer.example.com', exp: 9999999999, }; @@ -349,6 +453,7 @@ describe('openIdJwtStrategy – OPENID_EMAIL_CLAIM', () => { sub: 'oidc-new-sub', preferred_username: 'legacy@corp.com', upn: 'legacy@corp.com', + iss: 'https://issuer.example.com', exp: 9999999999, };