mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-05-13 07:46:47 +00:00
🛂 fix: Restrict OpenID JWT Bearer Reuse (#13052)
* fix: Restrict OpenID JWT bearer reuse * fix: Require signed OpenID reuse cookie * fix: Bind OpenID reuse to authenticated user
This commit is contained in:
parent
846eb0aa2c
commit
763fab2e3e
4 changed files with 322 additions and 10 deletions
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue