From 8355bf4ab7ddab7512c198610eb7cbbbc87c9ceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B3n=20Levy?= Date: Tue, 10 Feb 2026 15:21:39 +0000 Subject: [PATCH 1/3] fix(openid): distinguish ID tokens from access tokens in federated auth Fix OpenID Connect token handling to properly distinguish ID tokens from access tokens. ID tokens and access tokens are now stored and propagated separately, preventing token placeholders from resolving to identical values. - AuthService.js: Added idToken field to session storage - openIdJwtStrategy.js: Updated to read idToken from session - openidStrategy.js: Explicitly included id_token in federatedTokens - Test suites: Added comprehensive test coverage for token distinction Co-Authored-By: Claude Opus 4.6 --- api/server/services/AuthService.js | 1 + api/strategies/openIdJwtStrategy.js | 3 ++- api/strategies/openidStrategy.js | 1 + api/strategies/openidStrategy.spec.js | 26 +++++++++++++++++++++++- packages/api/src/utils/oidc.spec.ts | 29 +++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 2 deletions(-) diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index a400bce8b7d..28e811775f0 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -452,6 +452,7 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) = if (req.session) { req.session.openidTokens = { accessToken: tokenset.access_token, + idToken: tokenset.id_token, refreshToken: refreshToken, expiresAt: expirationDate.getTime(), }; diff --git a/api/strategies/openIdJwtStrategy.js b/api/strategies/openIdJwtStrategy.js index df318ca30e3..8cad8272e6a 100644 --- a/api/strategies/openIdJwtStrategy.js +++ b/api/strategies/openIdJwtStrategy.js @@ -84,6 +84,7 @@ const openIdJwtLogin = (openIdConfig) => { /** Read tokens from session (server-side) to avoid large cookie issues */ const sessionTokens = req.session?.openidTokens; let accessToken = sessionTokens?.accessToken; + let idToken = sessionTokens?.idToken; let refreshToken = sessionTokens?.refreshToken; /** Fallback to cookies for backward compatibility */ @@ -96,7 +97,7 @@ const openIdJwtLogin = (openIdConfig) => { user.federatedTokens = { access_token: accessToken || rawToken, - id_token: rawToken, + id_token: idToken, refresh_token: refreshToken, expires_at: payload.exp, }; diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index 84458ce9925..0d8fd83ddd6 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -498,6 +498,7 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { tokenset, federatedTokens: { access_token: tokenset.access_token, + id_token: tokenset.id_token, refresh_token: tokenset.refresh_token, expires_at: tokenset.expires_at, }, diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index ada27cca172..99c2a40d8b0 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -465,10 +465,11 @@ describe('setupOpenId', () => { }); it('should attach federatedTokens to user object for token propagation', async () => { - // Arrange - setup tokenset with access token, refresh token, and expiration + // Arrange - setup tokenset with access token, id token, refresh token, and expiration const tokensetWithTokens = { ...tokenset, access_token: 'mock_access_token_abc123', + id_token: 'mock_id_token_def456', refresh_token: 'mock_refresh_token_xyz789', expires_at: 1234567890, }; @@ -480,16 +481,37 @@ describe('setupOpenId', () => { expect(user.federatedTokens).toBeDefined(); expect(user.federatedTokens).toEqual({ access_token: 'mock_access_token_abc123', + id_token: 'mock_id_token_def456', refresh_token: 'mock_refresh_token_xyz789', expires_at: 1234567890, }); }); + it('should include id_token in federatedTokens distinct from access_token', async () => { + // Arrange - use different values for access_token and id_token + const tokensetWithTokens = { + ...tokenset, + access_token: 'the_access_token', + id_token: 'the_id_token', + refresh_token: 'the_refresh_token', + expires_at: 9999999999, + }; + + // Act + const { user } = await validate(tokensetWithTokens); + + // Assert - id_token and access_token must be different values + expect(user.federatedTokens.access_token).toBe('the_access_token'); + expect(user.federatedTokens.id_token).toBe('the_id_token'); + expect(user.federatedTokens.id_token).not.toBe(user.federatedTokens.access_token); + }); + it('should include tokenset along with federatedTokens', async () => { // Arrange const tokensetWithTokens = { ...tokenset, access_token: 'test_access_token', + id_token: 'test_id_token', refresh_token: 'test_refresh_token', expires_at: 9999999999, }; @@ -501,7 +523,9 @@ describe('setupOpenId', () => { expect(user.tokenset).toBeDefined(); expect(user.federatedTokens).toBeDefined(); expect(user.tokenset.access_token).toBe('test_access_token'); + expect(user.tokenset.id_token).toBe('test_id_token'); expect(user.federatedTokens.access_token).toBe('test_access_token'); + expect(user.federatedTokens.id_token).toBe('test_id_token'); }); it('should set role to "ADMIN" if OPENID_ADMIN_ROLE is set and user has that role', async () => { diff --git a/packages/api/src/utils/oidc.spec.ts b/packages/api/src/utils/oidc.spec.ts index a5312e9c69e..0d7216304b5 100644 --- a/packages/api/src/utils/oidc.spec.ts +++ b/packages/api/src/utils/oidc.spec.ts @@ -427,6 +427,35 @@ describe('OpenID Token Utilities', () => { expect(result).toContain('User:'); }); + it('should resolve LIBRECHAT_OPENID_ID_TOKEN and LIBRECHAT_OPENID_ACCESS_TOKEN to different values', () => { + const user: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + email: 'test@example.com', + name: 'Test User', + federatedTokens: { + access_token: 'my-access-token', + id_token: 'my-id-token', + refresh_token: 'my-refresh-token', + expires_at: Math.floor(Date.now() / 1000) + 3600, + }, + }; + + const tokenInfo = extractOpenIDTokenInfo(user); + expect(tokenInfo).not.toBeNull(); + expect(tokenInfo!.accessToken).toBe('my-access-token'); + expect(tokenInfo!.idToken).toBe('my-id-token'); + expect(tokenInfo!.accessToken).not.toBe(tokenInfo!.idToken); + + const input = 'ACCESS={{LIBRECHAT_OPENID_ACCESS_TOKEN}}, ID={{LIBRECHAT_OPENID_ID_TOKEN}}'; + const result = processOpenIDPlaceholders(input, tokenInfo!); + + expect(result).toBe('ACCESS=my-access-token, ID=my-id-token'); + // Verify they are not the same value (the reported bug) + expect(result).not.toBe('ACCESS=my-access-token, ID=my-access-token'); + }); + it('should handle expired tokens correctly', () => { const user: Partial = { id: 'user-123', From de7710e132384cc0d5d2ce40563e3410f1105283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B3n=20Levy?= Date: Thu, 12 Feb 2026 12:52:35 +0000 Subject: [PATCH 2/3] fix(openid): add separate openid_id_token cookie for ID token storage Store the OIDC ID token in its own cookie rather than relying solely on the access token, ensuring correct token type is used for identity verification vs API authorization. Co-Authored-By: Claude Opus 4.6 --- api/cache/banViolation.js | 1 + api/server/controllers/auth/LogoutController.js | 1 + api/server/services/AuthService.js | 8 ++++++++ api/strategies/openIdJwtStrategy.js | 3 ++- 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/api/cache/banViolation.js b/api/cache/banViolation.js index 122355edb1b..4d321889c10 100644 --- a/api/cache/banViolation.js +++ b/api/cache/banViolation.js @@ -55,6 +55,7 @@ const banViolation = async (req, res, errorMessage) => { res.clearCookie('refreshToken'); res.clearCookie('openid_access_token'); + res.clearCookie('openid_id_token'); res.clearCookie('openid_user_id'); res.clearCookie('token_provider'); diff --git a/api/server/controllers/auth/LogoutController.js b/api/server/controllers/auth/LogoutController.js index ec663162853..0b3cf262b8f 100644 --- a/api/server/controllers/auth/LogoutController.js +++ b/api/server/controllers/auth/LogoutController.js @@ -22,6 +22,7 @@ const logoutController = async (req, res) => { res.clearCookie('refreshToken'); res.clearCookie('openid_access_token'); + res.clearCookie('openid_id_token'); res.clearCookie('openid_user_id'); res.clearCookie('token_provider'); const response = { message }; diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index 28e811775f0..facd052574d 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -470,6 +470,14 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) = secure: isProduction, sameSite: 'strict', }); + if (tokenset.id_token) { + res.cookie('openid_id_token', tokenset.id_token, { + expires: expirationDate, + httpOnly: true, + secure: isProduction, + sameSite: 'strict', + }); + } } /** Small cookie to indicate token provider (required for auth middleware) */ diff --git a/api/strategies/openIdJwtStrategy.js b/api/strategies/openIdJwtStrategy.js index 8cad8272e6a..997dcec397c 100644 --- a/api/strategies/openIdJwtStrategy.js +++ b/api/strategies/openIdJwtStrategy.js @@ -88,10 +88,11 @@ const openIdJwtLogin = (openIdConfig) => { let refreshToken = sessionTokens?.refreshToken; /** Fallback to cookies for backward compatibility */ - if (!accessToken || !refreshToken) { + 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; } From 3a3fa6777aaffca02f6ac2f68bd68de18810e47e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B3n=20Levy?= Date: Thu, 12 Feb 2026 12:59:12 +0000 Subject: [PATCH 3/3] test(openid): add JWT strategy cookie fallback tests Cover the token source resolution logic in openIdJwtStrategy: session-only, cookie-only, partial session fallback, raw Bearer fallback, and distinct id_token/access_token from cookies. Co-Authored-By: Claude Opus 4.6 --- api/strategies/openIdJwtStrategy.spec.js | 183 +++++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 api/strategies/openIdJwtStrategy.spec.js diff --git a/api/strategies/openIdJwtStrategy.spec.js b/api/strategies/openIdJwtStrategy.spec.js new file mode 100644 index 00000000000..566afe5a90f --- /dev/null +++ b/api/strategies/openIdJwtStrategy.spec.js @@ -0,0 +1,183 @@ +const { SystemRoles } = require('librechat-data-provider'); + +// --- Capture the verify callback from JwtStrategy --- +let capturedVerifyCallback; +jest.mock('passport-jwt', () => ({ + Strategy: jest.fn((_opts, verifyCallback) => { + capturedVerifyCallback = verifyCallback; + return { name: 'jwt' }; + }), + ExtractJwt: { + fromAuthHeaderAsBearerToken: jest.fn(() => 'mock-extractor'), + }, +})); +jest.mock('jwks-rsa', () => ({ + passportJwtSecret: jest.fn(() => 'mock-secret-provider'), +})); +jest.mock('https-proxy-agent', () => ({ + HttpsProxyAgent: jest.fn(), +})); +jest.mock('@librechat/data-schemas', () => ({ + logger: { info: jest.fn(), warn: jest.fn(), debug: jest.fn(), error: jest.fn() }, +})); +jest.mock('@librechat/api', () => ({ + isEnabled: jest.fn(() => false), + findOpenIDUser: jest.fn(), + math: jest.fn((val, fallback) => fallback), +})); +jest.mock('~/models', () => ({ + findUser: jest.fn(), + updateUser: jest.fn(), +})); + +const { findOpenIDUser } = require('@librechat/api'); +const { updateUser } = require('~/models'); +const openIdJwtLogin = require('./openIdJwtStrategy'); + +// Helper: build a mock openIdConfig +const mockOpenIdConfig = { + serverMetadata: () => ({ jwks_uri: 'https://example.com/.well-known/jwks.json' }), +}; + +// Helper: invoke the captured verify callback +async function invokeVerify(req, payload) { + return new Promise((resolve, reject) => { + capturedVerifyCallback(req, payload, (err, user, info) => { + if (err) { + return reject(err); + } + resolve({ user, info }); + }); + }); +} + +describe('openIdJwtStrategy – token source handling', () => { + const baseUser = { + _id: { toString: () => 'user-abc' }, + role: SystemRoles.USER, + provider: 'openid', + }; + + const payload = { sub: 'oidc-123', email: 'test@example.com', exp: 9999999999 }; + + beforeEach(() => { + jest.clearAllMocks(); + findOpenIDUser.mockResolvedValue({ user: { ...baseUser }, error: null, migration: false }); + updateUser.mockResolvedValue({}); + + // Initialize the strategy so capturedVerifyCallback is set + openIdJwtLogin(mockOpenIdConfig); + }); + + it('should read all tokens from session when available', async () => { + const req = { + headers: { authorization: 'Bearer raw-bearer-token' }, + session: { + openidTokens: { + accessToken: 'session-access', + idToken: 'session-id', + refreshToken: 'session-refresh', + }, + }, + }; + + const { user } = await invokeVerify(req, payload); + + expect(user.federatedTokens).toEqual({ + access_token: 'session-access', + id_token: 'session-id', + refresh_token: 'session-refresh', + expires_at: payload.exp, + }); + }); + + it('should fall back to cookies when session is absent', async () => { + const req = { + headers: { + authorization: 'Bearer raw-bearer-token', + cookie: + 'openid_access_token=cookie-access; openid_id_token=cookie-id; refreshToken=cookie-refresh', + }, + }; + + const { user } = await invokeVerify(req, payload); + + expect(user.federatedTokens).toEqual({ + access_token: 'cookie-access', + id_token: 'cookie-id', + refresh_token: 'cookie-refresh', + expires_at: payload.exp, + }); + }); + + it('should fall back to cookie for idToken only when session lacks it', async () => { + const req = { + headers: { + authorization: 'Bearer raw-bearer-token', + cookie: 'openid_id_token=cookie-id', + }, + session: { + openidTokens: { + accessToken: 'session-access', + // idToken intentionally missing + refreshToken: 'session-refresh', + }, + }, + }; + + const { user } = await invokeVerify(req, payload); + + expect(user.federatedTokens).toEqual({ + access_token: 'session-access', + id_token: 'cookie-id', + refresh_token: 'session-refresh', + expires_at: payload.exp, + }); + }); + + it('should use raw Bearer token as access_token fallback when neither session nor cookie has one', async () => { + const req = { + headers: { + authorization: 'Bearer raw-bearer-token', + cookie: 'openid_id_token=cookie-id; refreshToken=cookie-refresh', + }, + }; + + const { user } = await invokeVerify(req, payload); + + expect(user.federatedTokens.access_token).toBe('raw-bearer-token'); + expect(user.federatedTokens.id_token).toBe('cookie-id'); + expect(user.federatedTokens.refresh_token).toBe('cookie-refresh'); + }); + + it('should set id_token to undefined when not available in session or cookies', async () => { + const req = { + headers: { + authorization: 'Bearer raw-bearer-token', + cookie: 'openid_access_token=cookie-access; refreshToken=cookie-refresh', + }, + }; + + const { user } = await invokeVerify(req, payload); + + expect(user.federatedTokens.access_token).toBe('cookie-access'); + expect(user.federatedTokens.id_token).toBeUndefined(); + expect(user.federatedTokens.refresh_token).toBe('cookie-refresh'); + }); + + it('should keep id_token and access_token as distinct values from cookies', async () => { + const req = { + headers: { + authorization: 'Bearer raw-bearer-token', + cookie: + 'openid_access_token=the-access-token; openid_id_token=the-id-token; refreshToken=the-refresh', + }, + }; + + const { user } = await invokeVerify(req, payload); + + expect(user.federatedTokens.access_token).toBe('the-access-token'); + expect(user.federatedTokens.id_token).toBe('the-id-token'); + expect(user.federatedTokens.access_token).not.toBe(user.federatedTokens.id_token); + }); +});