🪙 refactor: Distinguish ID Tokens from Access Tokens in OIDC Federated Auth (#11711)

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Jón Levy 2026-02-13 16:07:39 +00:00 committed by GitHub
parent 8e3b717e99
commit dc89e00039
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 253 additions and 3 deletions

View file

@ -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');

View file

@ -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 };

View file

@ -466,6 +466,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(),
};
@ -483,6 +484,14 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) =
secure: shouldUseSecureCookie(),
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) */

View file

@ -84,19 +84,21 @@ 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 */
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;
}
user.federatedTokens = {
access_token: accessToken || rawToken,
id_token: rawToken,
id_token: idToken,
refresh_token: refreshToken,
expires_at: payload.exp,
};

View file

@ -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);
});
});

View file

@ -590,6 +590,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,
},

View file

@ -775,10 +775,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,
};
@ -790,16 +791,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,
};
@ -811,7 +833,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 () => {

View file

@ -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<TUser> = {
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<TUser> = {
id: 'user-123',