From a33b128c47e334a6864a88a80ffbfa7baedbd245 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Tue, 18 Aug 2026 19:02:33 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=AA=20fix:=20Preserve=20Stored=20Acces?= =?UTF-8?q?s=20Token=20Expiry=20Over=20ID=20Token=20Exp=20(#14982)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🪪 fix: Preserve Stored Access Token Expiry Over ID Token Exp extractOpenIDTokenInfo let the ID token exp claim overwrite the token set's stored expires_at. The ID token is minted at login and never refreshed, so once a session outlives the ID token TTL, isOpenIDTokenValid reports the access token as expired even when expires_at is hours in the future, and OpenID placeholder substitution silently stops: MCP headers configured with {{LIBRECHAT_OPENID_ACCESS_TOKEN}} ship the literal placeholder string as the bearer credential and the receiving server rejects every connection with an unparseable JWT until the user fully logs out and back in. The ID token exp now only fills a missing expiresAt instead of overriding a stored one. Identity claim enrichment from the ID token is unchanged, and the exp fallback for token sets without expires_at is preserved. * 🪪 fix: Validate ID Token Expiry Before ID Token Placeholder Substitution The precedence fix made isOpenIDTokenValid track only the access token expiry, so an MCP header using {{LIBRECHAT_OPENID_ID_TOKEN}} could substitute an ID token that had already expired. The ID token exp is now preserved separately as idTokenExpiresAt and checked at the ID token substitution site, so an expired ID token substitutes empty rather than a stale credential while access token substitution is unaffected. * 🪪 fix: Address OpenID Expiry Review Round Fix expires_at at the source in the OpenID JWT strategy. The stored value described the incoming bearer's exp even when access_token came from the session or a cookie, so it could describe a different credential entirely. A new decodeJwtExpiry helper reads the exp of the token actually stored, and payload.exp is kept only when the raw bearer is the resolved access token. Opaque session or cookie tokens now store no expiry rather than a wrong one. Apply a 30 second clock skew buffer in isOpenIDTokenValid and isIdTokenCurrent via a new exported OPENID_EXPIRY_BUFFER_SECONDS, mirroring OPENID_REUSE_EXPIRY_BUFFER_SECONDS in AuthController. Tokens that would expire in transit are treated as already expired. Make isIdTokenCurrent fail closed when idTokenExpiresAt is absent. exp is REQUIRED in an ID token, so a missing value means the token is malformed or the claims parse threw. The check uses == null so an exp of 0 counts as present and therefore expired. Read the ID token exp with a numeric type check so an exp of 0 records idTokenExpiresAt and fails closed downstream while a non-numeric exp is ignored, and compare the stored expiry with != null so a gap filled expiry of 0 reads as expired instead of as no expiry at all. Raise an actionable re authentication error for the ID token placeholder instead of substituting an empty string. An empty substitution produced a malformed Authorization header and a 400 downstream rather than a clean signal that the user must re authenticate. Raise the same re authentication error from processSingleValue when a user has an OpenID identity, the stored token set is no longer valid, and the value still contains a credential bearing OpenID placeholder, so the expired access token case that motivated this PR signals re auth instead of silently shipping or stripping the placeholder. Only the access token, ID token, and generic token names raise: identity metadata resolves from the user document and an expiry hint never needed a token, so those keep their existing literal then strip behaviour. Unknown placeholder names also stay literal and diagnosable, matching the existing resolvable placeholder policy. Add the comments the review asked for on the exp fallback heuristic, the EXPIRES_AT placeholder semantics, why stale ID token claims stay usable for identity fields, and the advisory nature of the freshness check. * 🪪 fix: Honour Opaque Access Tokens And Type The OpenID Re-Auth Error Drop the ID token exp fallback in extractOpenIDTokenInfo. Storing the access token expiry honestly means an opaque access token now records no expiry, and the fallback then handed the ID token exp authority over a credential it does not describe. A deployment issuing opaque access tokens alongside a short lived ID token saw isOpenIDTokenValid go false and the credential guard reject a perfectly good access token, which worked before this branch. An unknown access token expiry is now treated as no expiry, and the ID token exp only ever gates ID token substitution through idTokenExpiresAt. Give the re-authentication signal a type. OpenIDReauthRequiredError is raised at both the ID token placeholder and the credential placeholder guard, ErrorController maps it to a 401 carrying the actionable message, and the class exposes statusCode so the agent generation path answers 401 instead of a bare 500 for the same condition. Omit rather than blank a header whose credential placeholder is still unresolved on a final resolution pass, since an empty bearer credential is malformed under RFC 6750 while an absent header lets the upstream answer its own challenge. Identity placeholders keep stripping to an empty string. Move the resolvable placeholder docblock onto the pattern it describes, resolve an EXPIRES_AT of 0 as the string 0 for consistency with the neighbouring null checks, and let AuthController consume the exported OPENID_EXPIRY_BUFFER_SECONDS so the 30 second skew allowance has a single definition. --- api/server/controllers/AuthController.js | 4 +- api/server/controllers/AuthController.spec.js | 1 + api/strategies/openIdJwtStrategy.js | 15 +- api/strategies/openIdJwtStrategy.spec.js | 52 +++- packages/api/src/middleware/error.spec.ts | 29 ++ packages/api/src/middleware/error.ts | 6 + packages/api/src/utils/env.spec.ts | 218 +++++++++++++- packages/api/src/utils/env.ts | 44 ++- packages/api/src/utils/oidc.spec.ts | 283 +++++++++++++++++- packages/api/src/utils/oidc.ts | 48 ++- 10 files changed, 670 insertions(+), 30 deletions(-) diff --git a/api/server/controllers/AuthController.js b/api/server/controllers/AuthController.js index b3743df828..47c7f2df0e 100644 --- a/api/server/controllers/AuthController.js +++ b/api/server/controllers/AuthController.js @@ -8,6 +8,7 @@ const { findOpenIDUser, getOpenIdIssuer, buildOpenIDRefreshParams, + OPENID_EXPIRY_BUFFER_SECONDS, } = require('@librechat/api'); const { requestPasswordReset, @@ -28,7 +29,6 @@ const { getGraphApiToken } = require('~/server/services/GraphTokenService'); const { getOpenIdConfig, getOpenIdEmail } = require('~/strategies'); const AUTH_REFRESH_USER_PROJECTION = '-password -__v -totpSecret -backupCodes -federatedTokens'; -const OPENID_REUSE_EXPIRY_BUFFER_SECONDS = 30; /** * Max age (ms) LibreChat reuses a cached OpenID session token before forcing an IdP refresh. * Env-overridable (accepts an arithmetic expression, e.g. `60 * 60 * 24 * 1000`, like @@ -110,7 +110,7 @@ const getReusableOpenIDSessionToken = (openidTokens) => { if ( decoded && typeof decoded === 'object' && - decoded.exp > now + OPENID_REUSE_EXPIRY_BUFFER_SECONDS + decoded.exp > now + OPENID_EXPIRY_BUFFER_SECONDS ) { return candidate; } diff --git a/api/server/controllers/AuthController.spec.js b/api/server/controllers/AuthController.spec.js index 40c20bbbe1..607d4b1b7d 100644 --- a/api/server/controllers/AuthController.spec.js +++ b/api/server/controllers/AuthController.spec.js @@ -22,6 +22,7 @@ jest.mock('~/models', () => ({ findUser: jest.fn(), })); jest.mock('@librechat/api', () => ({ + OPENID_EXPIRY_BUFFER_SECONDS: 30, math: jest.fn((value, fallback) => fallback), isEnabled: jest.fn(), findOpenIDUser: jest.fn(), diff --git a/api/strategies/openIdJwtStrategy.js b/api/strategies/openIdJwtStrategy.js index 5cd55020f1..3aefa03cbb 100644 --- a/api/strategies/openIdJwtStrategy.js +++ b/api/strategies/openIdJwtStrategy.js @@ -21,6 +21,15 @@ const { const { updateUser, findUser, isAgentTriggerPrincipalActive } = require('~/models'); const getLogStores = require('~/cache/getLogStores'); +function decodeJwtExpiry(token) { + try { + const payload = JSON.parse(Buffer.from(token.split('.')[1], 'base64').toString()); + return typeof payload.exp === 'number' ? payload.exp : undefined; + } catch { + return undefined; + } +} + const getOpenIdJwtAudience = () => { const parsedAudience = (process.env.OPENID_AUDIENCE ?? '') .split(',') @@ -224,11 +233,13 @@ const openIdJwtLogin = (openIdConfig) => { refreshToken = refreshToken || parsedCookies.refreshToken; } + const resolvedAccessToken = accessToken || rawToken; user.federatedTokens = { - access_token: accessToken || rawToken, + access_token: resolvedAccessToken, id_token: idToken, refresh_token: refreshToken, - expires_at: payload.exp, + expires_at: + resolvedAccessToken === rawToken ? payload.exp : decodeJwtExpiry(resolvedAccessToken), }; done(null, user); diff --git a/api/strategies/openIdJwtStrategy.spec.js b/api/strategies/openIdJwtStrategy.spec.js index e893628edb..a9ade4b160 100644 --- a/api/strategies/openIdJwtStrategy.spec.js +++ b/api/strategies/openIdJwtStrategy.spec.js @@ -296,7 +296,7 @@ describe('openIdJwtStrategy – token source handling', () => { access_token: 'session-access', id_token: 'session-id', refresh_token: 'session-refresh', - expires_at: payload.exp, + expires_at: undefined, }); }); @@ -315,7 +315,7 @@ describe('openIdJwtStrategy – token source handling', () => { access_token: 'cookie-access', id_token: 'cookie-id', refresh_token: 'cookie-refresh', - expires_at: payload.exp, + expires_at: undefined, }); }); @@ -340,7 +340,7 @@ describe('openIdJwtStrategy – token source handling', () => { access_token: 'session-access', id_token: 'cookie-id', refresh_token: 'session-refresh', - expires_at: payload.exp, + expires_at: undefined, }); }); @@ -357,6 +357,52 @@ describe('openIdJwtStrategy – token source handling', () => { 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'); + expect(user.federatedTokens.expires_at).toBe(payload.exp); + }); + + it('should decode expires_at from a session access token that is itself a JWT', async () => { + const sessionAccessExp = 1234567890; + const sessionAccessToken = `header.${Buffer.from( + JSON.stringify({ sub: 'oidc-123', exp: sessionAccessExp }), + ).toString('base64')}.signature`; + const req = { + headers: { authorization: 'Bearer raw-bearer-token' }, + session: { + openidTokens: { + accessToken: sessionAccessToken, + idToken: 'session-id', + refreshToken: 'session-refresh', + }, + }, + }; + + const { user } = await invokeVerify(req, payload); + + expect(user.federatedTokens.access_token).toBe(sessionAccessToken); + expect(user.federatedTokens.expires_at).toBe(sessionAccessExp); + expect(user.federatedTokens.expires_at).not.toBe(payload.exp); + }); + + it('should store an opaque session access token with no expiry alongside a decodable stale ID token', async () => { + const staleIdToken = `header.${Buffer.from( + JSON.stringify({ sub: 'oidc-123', exp: Math.floor(Date.now() / 1000) - 3600 }), + ).toString('base64')}.signature`; + const req = { + headers: { authorization: 'Bearer raw-bearer-token' }, + session: { + openidTokens: { + accessToken: 'opaque-session-access', + idToken: staleIdToken, + refreshToken: 'session-refresh', + }, + }, + }; + + const { user } = await invokeVerify(req, payload); + + expect(user.federatedTokens.access_token).toBe('opaque-session-access'); + expect(user.federatedTokens.id_token).toBe(staleIdToken); + expect(user.federatedTokens.expires_at).toBeUndefined(); }); it('should set id_token to undefined when not available in session or cookies', async () => { diff --git a/packages/api/src/middleware/error.spec.ts b/packages/api/src/middleware/error.spec.ts index eb636007db..99a13771f3 100644 --- a/packages/api/src/middleware/error.spec.ts +++ b/packages/api/src/middleware/error.spec.ts @@ -2,6 +2,7 @@ import { logger, tenantStorage } from '@librechat/data-schemas'; import type { Request, Response } from 'express'; import type { ValidationError, MongoServerError, CustomError } from '~/types'; import { ErrorController, createCustomError } from './error'; +import { OpenIDReauthRequiredError } from '~/utils/oidc'; // Mock the logger jest.mock('@librechat/data-schemas', () => ({ @@ -223,6 +224,34 @@ describe('ErrorController', () => { }); }); + describe('OpenIDReauthRequiredError handling', () => { + it('should map a re-auth error to a 401 carrying the actionable message', () => { + const error = new OpenIDReauthRequiredError( + 'OpenID token is expired or unavailable; re-authentication is required to resolve {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + ); + + ErrorController(error, mockReq, mockRes, mockNext); + + expect(mockRes.status).toHaveBeenCalledWith(401); + expect(mockRes.send).toHaveBeenCalledWith({ + error: 'invalid_token', + message: + 'OpenID token is expired or unavailable; re-authentication is required to resolve {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }); + }); + + it('should carry a 401 statusCode for callers that read the status directly', () => { + expect(new OpenIDReauthRequiredError('re-auth').statusCode).toBe(401); + }); + + it('should not fall through to the bare 500 path', () => { + ErrorController(new OpenIDReauthRequiredError('re-auth'), mockReq, mockRes, mockNext); + + expect(mockRes.status).not.toHaveBeenCalledWith(500); + expect(mockRes.send).not.toHaveBeenCalledWith('An unknown error occurred.'); + }); + }); + describe('Unknown error handling', () => { it('should handle unknown errors', () => { const unknownError = new Error('Some unknown error'); diff --git a/packages/api/src/middleware/error.ts b/packages/api/src/middleware/error.ts index b39e5e8a12..ad8afedf22 100644 --- a/packages/api/src/middleware/error.ts +++ b/packages/api/src/middleware/error.ts @@ -3,6 +3,7 @@ import { logger, tenantStorage } from '@librechat/data-schemas'; import type { NextFunction, Request, Response } from 'express'; import type { MongoServerError, ValidationError, CustomError } from '~/types'; import { buildTenantIsolationErrorLogContext } from './auth'; +import { OpenIDReauthRequiredError } from '~/utils/oidc'; const handleDuplicateKeyError = (err: MongoServerError, res: Response) => { logger.warn('Duplicate key error: ' + (err.errmsg || err.message)); @@ -83,6 +84,11 @@ export const ErrorController = ( return handleDuplicateKeyError(error, res); } + if (err instanceof OpenIDReauthRequiredError) { + logger.warn('OpenID re-authentication required: ' + err.message); + return res.status(401).send({ error: 'invalid_token', message: err.message }); + } + if (isCustomError(error) && error.statusCode && error.body) { return res.status(error.statusCode).send(error.body); } diff --git a/packages/api/src/utils/env.spec.ts b/packages/api/src/utils/env.spec.ts index 395f5e70ed..a34adb0fcb 100644 --- a/packages/api/src/utils/env.spec.ts +++ b/packages/api/src/utils/env.spec.ts @@ -2257,7 +2257,7 @@ describe('resolveHeaders stripUnresolved', () => { expect(result['X-Convo']).toBe(''); }); - it('strips OpenID token placeholders when no valid token is available', () => { + it('omits OpenID credential headers when no valid token is available', () => { const result = resolveHeaders({ headers: { 'X-Access': '{{LIBRECHAT_OPENID_ACCESS_TOKEN}}', @@ -2267,8 +2267,35 @@ describe('resolveHeaders stripUnresolved', () => { stripUnresolved: true, }); - expect(result['X-Access']).toBe(''); - expect(result['X-Token']).toBe(''); + expect(result).not.toHaveProperty('X-Access'); + expect(result).not.toHaveProperty('X-Token'); + }); + + it('omits the credential header but strips identity placeholders to empty', () => { + const result = resolveHeaders({ + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + 'X-Org': '{{LIBRECHAT_OPENID_USER_ID}}', + }, + user: createTestUser({ id: 'user-123' }), + stripUnresolved: true, + }); + + expect(result).not.toHaveProperty('Authorization'); + expect(result['X-Org']).toBe(''); + }); + + it('preserves credential placeholders literally when stripUnresolved is false', () => { + const result = resolveHeaders({ + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + 'X-Org': '{{LIBRECHAT_OPENID_USER_ID}}', + }, + user: createTestUser({ id: 'user-123' }), + }); + + expect(result.Authorization).toBe('Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}'); + expect(result['X-Org']).toBe('{{LIBRECHAT_OPENID_USER_ID}}'); }); it('leaves unknown and non-resolvable placeholders untouched', () => { @@ -2293,3 +2320,188 @@ describe('resolveHeaders stripUnresolved', () => { expect(result['X-User-Id']).toBe('{{LIBRECHAT_USER_ID}}'); }); }); + +describe('processMCPEnv OpenID re-authentication signalling', () => { + function createOpenIDUser(expiresAt: number): IUser { + return { + ...createTestUser({ id: 'user-123', provider: 'openid' }), + openidId: 'oidc-sub-456', + federatedTokens: { + access_token: 'stored-access-token', + id_token: 'stored-id-token', + refresh_token: 'stored-refresh-token', + expires_at: expiresAt, + }, + } as IUser; + } + + function tokenlessOpenIDUser(): IUser { + return { + ...createTestUser({ id: 'user-123', provider: 'openid' }), + openidId: 'oidc-sub-456', + } as IUser; + } + + const expiredSeconds = Math.floor(Date.now() / 1000) - 3600; + const validSeconds = Math.floor(Date.now() / 1000) + 3600; + + it('should throw an actionable re-auth error when the token set is expired and a credential placeholder is present', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }, + }; + + expect(() => processMCPEnv({ options, user: createOpenIDUser(expiredSeconds) })).toThrow( + 'OpenID token is expired or unavailable; re-authentication is required to resolve {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + ); + }); + + it('should still resolve other placeholders when the token set is expired but no OpenID placeholder is present', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + }, + }; + + const result = processMCPEnv({ options, user: createOpenIDUser(expiredSeconds) }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.['X-User-Id']).toBe('user-123'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should leave an unknown OpenID placeholder name literal instead of raising re-auth', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCES_TOKEN}}', + }, + }; + + const result = processMCPEnv({ options, user: createOpenIDUser(expiredSeconds) }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer {{LIBRECHAT_OPENID_ACCES_TOKEN}}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should leave OpenID placeholders untouched for a user with no OpenID identity', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }, + }; + + const result = processMCPEnv({ options, user: createTestUser({ id: 'user-123' }) }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should substitute the access token when the token set is still valid', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }, + }; + + const result = processMCPEnv({ options, user: createOpenIDUser(validSeconds) }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer stored-access-token'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should raise re-auth from resolveHeaders when the token set is expired', () => { + expect(() => + resolveHeaders({ + headers: { Authorization: 'Bearer {{LIBRECHAT_OPENID_ID_TOKEN}}' }, + user: createOpenIDUser(expiredSeconds), + stripUnresolved: true, + }), + ).toThrow( + 'OpenID token is expired or unavailable; re-authentication is required to resolve {{LIBRECHAT_OPENID_ID_TOKEN}}', + ); + }); + + it('should leave identity metadata placeholders literal for an OpenID user with no stored tokens', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-User-Id': '{{LIBRECHAT_OPENID_USER_ID}}', + }, + }; + + const result = processMCPEnv({ options, user: tokenlessOpenIDUser() }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.['X-User-Id']).toBe('{{LIBRECHAT_OPENID_USER_ID}}'); + } else { + throw new Error('Expected streamable-http options'); + } + + expect( + resolveHeaders({ + headers: { 'X-User-Id': '{{LIBRECHAT_OPENID_USER_ID}}' }, + user: tokenlessOpenIDUser(), + stripUnresolved: true, + })['X-User-Id'], + ).toBe(''); + }); + + it('should still raise re-auth for a credential placeholder when no tokens are stored', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }, + }; + + expect(() => processMCPEnv({ options, user: tokenlessOpenIDUser() })).toThrow( + 'OpenID token is expired or unavailable; re-authentication is required to resolve {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + ); + }); + + it('should not raise re-auth for a metadata-only template when the token set is expired', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-User-Id': '{{LIBRECHAT_OPENID_USER_ID}}', + 'X-User-Email': '{{LIBRECHAT_OPENID_USER_EMAIL}}', + 'X-User-Name': '{{LIBRECHAT_OPENID_USER_NAME}}', + 'X-Expires': '{{LIBRECHAT_OPENID_EXPIRES_AT}}', + }, + }; + + const result = processMCPEnv({ options, user: createOpenIDUser(expiredSeconds) }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.['X-User-Id']).toBe('{{LIBRECHAT_OPENID_USER_ID}}'); + expect(result.headers?.['X-Expires']).toBe('{{LIBRECHAT_OPENID_EXPIRES_AT}}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); +}); diff --git a/packages/api/src/utils/env.ts b/packages/api/src/utils/env.ts index 1c39d0b9dd..39a044ed68 100644 --- a/packages/api/src/utils/env.ts +++ b/packages/api/src/utils/env.ts @@ -1,3 +1,4 @@ +import { logger } from '@librechat/data-schemas'; import { extractEnvVariable } from 'librechat-data-provider'; import type { MCPOptions } from 'librechat-data-provider'; import type { IUser } from '@librechat/data-schemas'; @@ -7,6 +8,7 @@ import { isOpenIDTokenValid, extractOpenIDTokenInfo, processOpenIDPlaceholders, + OpenIDReauthRequiredError, } from './oidc'; /** @@ -144,6 +146,8 @@ export function createSafeUser( */ export const ALLOWED_BODY_FIELDS = ['conversationId', 'parentMessageId', 'messageId'] as const; +const OPENID_PLACEHOLDER_NAMES = `LIBRECHAT_OPENID_(?:${OPENID_TOKEN_FIELDS.join('|')}|TOKEN)`; + /** * Matches every placeholder this module knows how to resolve: the enumerated * `{{LIBRECHAT_USER_*}}`, `{{LIBRECHAT_BODY_*}}`, and `{{LIBRECHAT_OPENID_*}}` @@ -155,13 +159,24 @@ const RESOLVABLE_PLACEHOLDER_PATTERN = new RegExp( [ `LIBRECHAT_USER_(?:${ALLOWED_USER_FIELDS.map((field) => field.toUpperCase()).join('|')})`, `LIBRECHAT_BODY_(?:${ALLOWED_BODY_FIELDS.map((field) => field.toUpperCase()).join('|')})`, - `LIBRECHAT_OPENID_(?:${OPENID_TOKEN_FIELDS.join('|')}|TOKEN)`, + OPENID_PLACEHOLDER_NAMES, ] .map((names) => `\\{\\{(?:${names})\\}\\}`) .join('|'), 'g', ); +/** + * The subset of OpenID placeholders that cannot resolve without a usable token + * set. Identity metadata (`USER_ID`, `USER_EMAIL`, `USER_NAME`) comes from the + * user document and `EXPIRES_AT` is only ever a hint, so those must keep their + * pre-existing literal-then-strip behaviour when the token set is invalid + * rather than raising re-auth. Non-global so `exec` stays stateless, and + * unknown names are excluded so a typo stays literal and diagnosable. + */ +const OPENID_CREDENTIAL_PLACEHOLDER_PATTERN = + /\{\{LIBRECHAT_OPENID_(?:ACCESS_TOKEN|ID_TOKEN|TOKEN)\}\}/; + /** * Replaces resolvable-but-unresolved placeholders with an empty string so * LibreChat's internal template syntax is never sent upstream as if it were @@ -320,6 +335,16 @@ function processSingleValue({ const openidTokenInfo = extractOpenIDTokenInfo(user); if (openidTokenInfo && isOpenIDTokenValid(openidTokenInfo)) { value = processOpenIDPlaceholders(value, openidTokenInfo); + } else if (openidTokenInfo) { + const unresolvable = OPENID_CREDENTIAL_PLACEHOLDER_PATTERN.exec(value); + if (unresolvable) { + logger.warn( + `OpenID token is expired or unavailable; cannot resolve ${unresolvable[0]} for the current request`, + ); + throw new OpenIDReauthRequiredError( + `OpenID token is expired or unavailable; re-authentication is required to resolve ${unresolvable[0]}`, + ); + } } if (body) { @@ -610,7 +635,22 @@ export function resolveHeaders(options?: { body, isHeader: true, // Important: Enable header encoding }); - resolvedHeaders[key] = stripUnresolved ? stripUnresolvedPlaceholders(processed) : processed; + if (!stripUnresolved) { + resolvedHeaders[key] = processed; + return; + } + + /** Reached only when the credential guard did not fire, i.e. the user has no OpenID identity at all: blanking the credential would emit `Authorization: Bearer `, which RFC 6750 rejects for a missing b64token */ + const unresolvedCredential = OPENID_CREDENTIAL_PLACEHOLDER_PATTERN.exec(processed); + if (unresolvedCredential) { + logger.warn( + `Omitting header "${key}": ${unresolvedCredential[0]} could not be resolved for the current request`, + ); + delete resolvedHeaders[key]; + return; + } + + resolvedHeaders[key] = stripUnresolvedPlaceholders(processed); }); } diff --git a/packages/api/src/utils/oidc.spec.ts b/packages/api/src/utils/oidc.spec.ts index e7088d9897..0cefdf6068 100644 --- a/packages/api/src/utils/oidc.spec.ts +++ b/packages/api/src/utils/oidc.spec.ts @@ -1,5 +1,10 @@ -import { extractOpenIDTokenInfo, isOpenIDTokenValid, processOpenIDPlaceholders } from './oidc'; import type { IUser } from '@librechat/data-schemas'; +import { + OpenIDReauthRequiredError, + extractOpenIDTokenInfo, + isOpenIDTokenValid, + processOpenIDPlaceholders, +} from './oidc'; describe('OpenID Token Utilities', () => { describe('extractOpenIDTokenInfo', () => { @@ -116,6 +121,156 @@ describe('OpenID Token Utilities', () => { expect(result?.userId).toBe('user-123'); }); + + it('should keep the stored access token expiry when the ID token carries an older exp', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const staleIdTokenPayload = Buffer.from( + JSON.stringify({ sub: 'oidc-sub-456', exp: nowSeconds - 3600 }), + ).toString('base64'); + const user: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + federatedTokens: { + access_token: 'fresh-access-token', + id_token: `header.${staleIdTokenPayload}.signature`, + expires_at: nowSeconds + 3600, + }, + }; + + const result = extractOpenIDTokenInfo(user); + + expect(result?.expiresAt).toBe(nowSeconds + 3600); + expect(isOpenIDTokenValid(result)).toBe(true); + expect(processOpenIDPlaceholders('Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', result)).toBe( + 'Bearer fresh-access-token', + ); + }); + + it('should leave expiresAt unset when no expires_at is stored, regardless of the ID token exp', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const idTokenPayload = Buffer.from( + JSON.stringify({ sub: 'oidc-sub-456', exp: nowSeconds + 1800 }), + ).toString('base64'); + const user: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + federatedTokens: { + access_token: 'access-token-value', + id_token: `header.${idTokenPayload}.signature`, + }, + }; + + const result = extractOpenIDTokenInfo(user); + + expect(result?.expiresAt).toBeUndefined(); + expect(result?.idTokenExpiresAt).toBe(nowSeconds + 1800); + expect(isOpenIDTokenValid(result)).toBe(true); + }); + + it('should keep an opaque access token valid when the stored ID token is already expired', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const staleIdTokenPayload = Buffer.from( + JSON.stringify({ sub: 'oidc-sub-456', exp: nowSeconds - 3600 }), + ).toString('base64'); + const user: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + federatedTokens: { + access_token: 'opaque-access-token', + id_token: `header.${staleIdTokenPayload}.signature`, + }, + }; + + const result = extractOpenIDTokenInfo(user); + + expect(result?.expiresAt).toBeUndefined(); + expect(result?.idTokenExpiresAt).toBe(nowSeconds - 3600); + expect(isOpenIDTokenValid(result)).toBe(true); + expect(processOpenIDPlaceholders('{{LIBRECHAT_OPENID_ACCESS_TOKEN}}', result)).toBe( + 'opaque-access-token', + ); + expect(() => processOpenIDPlaceholders('{{LIBRECHAT_OPENID_ID_TOKEN}}', result)).toThrow( + OpenIDReauthRequiredError, + ); + }); + + it('should gate only the ID token on an exp of 0, leaving access token validity untouched', () => { + const idTokenPayload = Buffer.from(JSON.stringify({ sub: 'oidc-sub-456', exp: 0 })).toString( + 'base64', + ); + const user: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + federatedTokens: { + access_token: 'access-token-value', + id_token: `header.${idTokenPayload}.signature`, + }, + }; + + const result = extractOpenIDTokenInfo(user); + + expect(result?.idTokenExpiresAt).toBe(0); + expect(result?.expiresAt).toBeUndefined(); + expect(isOpenIDTokenValid(result)).toBe(true); + expect(() => processOpenIDPlaceholders('{{LIBRECHAT_OPENID_ID_TOKEN}}', result)).toThrow( + /re-authentication is required/, + ); + }); + + it('should ignore a non-numeric ID token exp', () => { + const idTokenPayload = Buffer.from( + JSON.stringify({ sub: 'oidc-sub-456', exp: '1700000000' }), + ).toString('base64'); + const user: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + federatedTokens: { + access_token: 'access-token-value', + id_token: `header.${idTokenPayload}.signature`, + }, + }; + + const result = extractOpenIDTokenInfo(user); + + expect(result?.idTokenExpiresAt).toBeUndefined(); + expect(result?.expiresAt).toBeUndefined(); + expect(() => processOpenIDPlaceholders('{{LIBRECHAT_OPENID_ID_TOKEN}}', result)).toThrow( + /re-authentication is required/, + ); + }); + + it('should still enrich identity fields from ID token claims when expires_at is stored', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const idTokenPayload = Buffer.from( + JSON.stringify({ + sub: 'claims-sub', + email: 'claims@example.com', + name: 'Claims Name', + exp: nowSeconds - 3600, + }), + ).toString('base64'); + const user: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + federatedTokens: { + access_token: 'access-token-value', + id_token: `header.${idTokenPayload}.signature`, + expires_at: nowSeconds + 3600, + }, + }; + + const result = extractOpenIDTokenInfo(user); + + expect(result?.userId).toBe('claims-sub'); + expect(result?.userEmail).toBe('claims@example.com'); + expect(result?.userName).toBe('Claims Name'); + }); }); describe('isOpenIDTokenValid', () => { @@ -173,7 +328,7 @@ describe('OpenID Token Utilities', () => { expect(isOpenIDTokenValid(tokenInfo)).toBe(false); }); - it('should return true when token is just about to expire (within 1 second)', () => { + it('should return false when token expires within the expiry buffer', () => { const almostExpiredTimestamp = Math.floor(Date.now() / 1000) + 1; const tokenInfo = { accessToken: 'access-token-value', @@ -181,11 +336,85 @@ describe('OpenID Token Utilities', () => { userId: 'oidc-sub-456', }; + expect(isOpenIDTokenValid(tokenInfo)).toBe(false); + }); + + it('should return true when token expires beyond the expiry buffer', () => { + const beyondBufferTimestamp = Math.floor(Date.now() / 1000) + 120; + const tokenInfo = { + accessToken: 'access-token-value', + expiresAt: beyondBufferTimestamp, + userId: 'oidc-sub-456', + }; + expect(isOpenIDTokenValid(tokenInfo)).toBe(true); }); + + it('should pin the buffer boundary at 30 seconds', () => { + const nowMs = 1_700_000_000_000; + const nowSeconds = Math.floor(nowMs / 1000); + const nowSpy = jest.spyOn(Date, 'now').mockReturnValue(nowMs); + + try { + expect( + isOpenIDTokenValid({ + accessToken: 'access-token-value', + expiresAt: nowSeconds + 29, + userId: 'oidc-sub-456', + }), + ).toBe(false); + expect( + isOpenIDTokenValid({ + accessToken: 'access-token-value', + expiresAt: nowSeconds + 31, + userId: 'oidc-sub-456', + }), + ).toBe(true); + } finally { + nowSpy.mockRestore(); + } + }); + + it('should return false when expiresAt is 0', () => { + const tokenInfo = { + accessToken: 'access-token-value', + expiresAt: 0, + userId: 'oidc-sub-456', + }; + + expect(isOpenIDTokenValid(tokenInfo)).toBe(false); + }); }); describe('processOpenIDPlaceholders', () => { + it('should not substitute an expired ID token for the ID token placeholder', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const tokenInfo = { + accessToken: 'fresh-access-token', + idToken: 'stale-id-token-value', + expiresAt: nowSeconds + 3600, + idTokenExpiresAt: nowSeconds - 3600, + }; + + expect(() => processOpenIDPlaceholders('{{LIBRECHAT_OPENID_ID_TOKEN}}', tokenInfo)).toThrow( + /re-authentication is required/, + ); + }); + + it('should substitute a current ID token for the ID token placeholder', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const tokenInfo = { + accessToken: 'fresh-access-token', + idToken: 'current-id-token-value', + expiresAt: nowSeconds + 3600, + idTokenExpiresAt: nowSeconds + 1800, + }; + + const result = processOpenIDPlaceholders('{{LIBRECHAT_OPENID_ID_TOKEN}}', tokenInfo); + + expect(result).toBe('current-id-token-value'); + }); + it('should replace LIBRECHAT_OPENID_TOKEN with access token', () => { const tokenInfo = { accessToken: 'access-token-value', @@ -213,6 +442,7 @@ describe('OpenID Token Utilities', () => { it('should replace LIBRECHAT_OPENID_ID_TOKEN with id token', () => { const tokenInfo = { + idTokenExpiresAt: Math.floor(Date.now() / 1000) + 3600, idToken: 'id-token-value', userId: 'oidc-sub-456', }; @@ -262,6 +492,7 @@ describe('OpenID Token Utilities', () => { const tokenInfo = { accessToken: 'access-token-value', idToken: 'id-token-value', + idTokenExpiresAt: Math.floor(Date.now() / 1000) + 3600, userId: 'oidc-sub-456', userEmail: 'test@example.com', }; @@ -278,21 +509,43 @@ describe('OpenID Token Utilities', () => { it('should replace empty string when token field is undefined', () => { const tokenInfo = { accessToken: undefined, - idToken: undefined, userId: 'oidc-sub-456', }; - const input = - 'Access: {{LIBRECHAT_OPENID_TOKEN}}, ID: {{LIBRECHAT_OPENID_ID_TOKEN}}, User: {{LIBRECHAT_OPENID_USER_ID}}'; + const input = 'Access: {{LIBRECHAT_OPENID_TOKEN}}, User: {{LIBRECHAT_OPENID_USER_ID}}'; const result = processOpenIDPlaceholders(input, tokenInfo); - expect(result).toBe('Access: , ID: , User: oidc-sub-456'); + expect(result).toBe('Access: , User: oidc-sub-456'); + }); + + it('should throw for the ID token placeholder when no ID token is stored', () => { + const tokenInfo = { + accessToken: 'access-token-value', + userId: 'oidc-sub-456', + }; + + expect(() => processOpenIDPlaceholders('{{LIBRECHAT_OPENID_ID_TOKEN}}', tokenInfo)).toThrow( + /re-authentication is required/, + ); + }); + + it('should throw for the ID token placeholder when the ID token has no decodable exp', () => { + const tokenInfo = { + accessToken: 'access-token-value', + idToken: 'malformed-id-token', + userId: 'oidc-sub-456', + }; + + expect(() => processOpenIDPlaceholders('{{LIBRECHAT_OPENID_ID_TOKEN}}', tokenInfo)).toThrow( + /re-authentication is required/, + ); }); it('should handle all placeholder types in one value', () => { const tokenInfo = { accessToken: 'access-token-value', idToken: 'id-token-value', + idTokenExpiresAt: Math.floor(Date.now() / 1000) + 3600, userId: 'oidc-sub-456', userEmail: 'test@example.com', userName: 'Test User', @@ -369,11 +622,10 @@ describe('OpenID Token Utilities', () => { userName: undefined, }; - const input = - 'Access: {{LIBRECHAT_OPENID_TOKEN}}, ID: {{LIBRECHAT_OPENID_ID_TOKEN}}, User: {{LIBRECHAT_OPENID_USER_ID}}'; + const input = 'Access: {{LIBRECHAT_OPENID_TOKEN}}, User: {{LIBRECHAT_OPENID_USER_ID}}'; const result = processOpenIDPlaceholders(input, tokenInfo); - expect(result).toBe('Access: , ID: , User: oidc-sub-456'); + expect(result).toBe('Access: , User: oidc-sub-456'); }); it('should return original value when tokenInfo is null', () => { @@ -428,6 +680,11 @@ describe('OpenID Token Utilities', () => { }); it('should resolve LIBRECHAT_OPENID_ID_TOKEN and LIBRECHAT_OPENID_ACCESS_TOKEN to different values', () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const idTokenPayload = Buffer.from( + JSON.stringify({ sub: 'oidc-sub-456', exp: nowSeconds + 3600 }), + ).toString('base64'); + const myIdToken = `header.${idTokenPayload}.signature`; const user: Partial = { id: 'user-123', provider: 'openid', @@ -436,22 +693,22 @@ describe('OpenID Token Utilities', () => { name: 'Test User', federatedTokens: { access_token: 'my-access-token', - id_token: 'my-id-token', + id_token: myIdToken, refresh_token: 'my-refresh-token', - expires_at: Math.floor(Date.now() / 1000) + 3600, + expires_at: nowSeconds + 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!.idToken).toBe(myIdToken); 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'); + expect(result).toBe(`ACCESS=my-access-token, ID=${myIdToken}`); // Verify they are not the same value (the reported bug) expect(result).not.toBe('ACCESS=my-access-token, ID=my-access-token'); }); diff --git a/packages/api/src/utils/oidc.ts b/packages/api/src/utils/oidc.ts index fcf2db247b..27b7dc5e4a 100644 --- a/packages/api/src/utils/oidc.ts +++ b/packages/api/src/utils/oidc.ts @@ -5,6 +5,7 @@ export interface OpenIDTokenInfo { accessToken?: string; idToken?: string; expiresAt?: number; + idTokenExpiresAt?: number; userId?: string; userEmail?: string; userName?: string; @@ -40,6 +41,25 @@ export const GRAPH_TOKEN_PLACEHOLDER = '{{LIBRECHAT_GRAPH_ACCESS_TOKEN}}'; */ export const DEFAULT_GRAPH_SCOPES = 'https://graph.microsoft.com/.default'; +/** Shared with AuthController's OpenID session reuse check: a token within the buffer would expire in transit and 401 downstream */ +export const OPENID_EXPIRY_BUFFER_SECONDS = 30; + +/** + * Signals that the stored OpenID credentials cannot satisfy a placeholder, so the user must + * re-authenticate. `ErrorController` maps this to a 401, and `statusCode` additionally lets + * status-reading callers (the agent generation path's `getInitializationFailure`) answer 401 + * instead of a bare 500. Deliberately carries no `body`, so the structural `isCustomError` + * guard cannot capture it ahead of the explicit mapping. + */ +export class OpenIDReauthRequiredError extends Error { + readonly statusCode = 401; + + constructor(message: string) { + super(message); + this.name = 'OpenIDReauthRequiredError'; + } +} + export function extractOpenIDTokenInfo( user: Partial | null | undefined, ): OpenIDTokenInfo | null { @@ -85,10 +105,13 @@ export function extractOpenIDTokenInfo( ); tokenInfo.claims = payload; + /** Cached profile claims, not an authentication assertion: stale claims stay usable for identity fields even when the ID token itself is expired */ if (payload.sub) tokenInfo.userId = payload.sub; if (payload.email) tokenInfo.userEmail = payload.email; if (payload.name) tokenInfo.userName = payload.name; - if (payload.exp) tokenInfo.expiresAt = payload.exp; + if (typeof payload.exp === 'number') { + tokenInfo.idTokenExpiresAt = payload.exp; + } } catch (jwtError) { logger.warn('Could not parse ID token claims:', jwtError); } @@ -101,14 +124,22 @@ export function extractOpenIDTokenInfo( } } +/** Advisory freshness check, not a security boundary: the ID token signature is not verified here. `exp` is REQUIRED in an ID token, so a missing value means the token is malformed or unparseable and fails closed. */ +function isIdTokenCurrent(tokenInfo: OpenIDTokenInfo): boolean { + if (tokenInfo.idTokenExpiresAt == null) { + return false; + } + return Math.floor(Date.now() / 1000) < tokenInfo.idTokenExpiresAt - OPENID_EXPIRY_BUFFER_SECONDS; +} + export function isOpenIDTokenValid(tokenInfo: OpenIDTokenInfo | null): boolean { if (!tokenInfo || !tokenInfo.accessToken) { return false; } - if (tokenInfo.expiresAt) { + if (tokenInfo.expiresAt != null) { const now = Math.floor(Date.now() / 1000); - if (now >= tokenInfo.expiresAt) { + if (now >= tokenInfo.expiresAt - OPENID_EXPIRY_BUFFER_SECONDS) { logger.warn('OpenID token has expired'); return false; } @@ -140,7 +171,13 @@ export function processOpenIDPlaceholders( replacementValue = tokenInfo.accessToken || ''; break; case 'ID_TOKEN': - replacementValue = tokenInfo.idToken || ''; + if (!tokenInfo.idToken || !isIdTokenCurrent(tokenInfo)) { + logger.warn('OpenID ID token is expired or unavailable; re-authentication is required'); + throw new OpenIDReauthRequiredError( + 'OpenID ID token is expired or unavailable; re-authentication is required to resolve {{LIBRECHAT_OPENID_ID_TOKEN}}', + ); + } + replacementValue = tokenInfo.idToken; break; case 'USER_ID': replacementValue = tokenInfo.userId || ''; @@ -152,7 +189,8 @@ export function processOpenIDPlaceholders( replacementValue = tokenInfo.userName || ''; break; case 'EXPIRES_AT': - replacementValue = tokenInfo.expiresAt ? String(tokenInfo.expiresAt) : ''; + /** The stored token-set expires_at only: the ID token exp never stands in for it */ + replacementValue = tokenInfo.expiresAt != null ? String(tokenInfo.expiresAt) : ''; break; }