From 01af63cb52f4dccdcccf0ea6ea4f7632bc387723 Mon Sep 17 00:00:00 2001 From: Dev Chohan <87763613+devanchohan@users.noreply.github.com> Date: Sat, 23 May 2026 08:20:59 +0700 Subject: [PATCH] =?UTF-8?q?=E2=8C=9B=20fix:=20Use=20JWT=20exp=20claim=20fo?= =?UTF-8?q?r=20MCP=20when=20OAuth=20token=20omits=20expires=5Fin=20(#13248?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MCP OAuth access tokens are stored with a 365-day default expiry when the provider's token response omits `expires_in` (only RECOMMENDED per RFC 6749 §5.1). Providers that issue short-lived JWT access tokens but omit `expires_in` (e.g. Salesforce) therefore get tokens treated as valid for a year and never refreshed, so every call 401s once the real token lapses until the user manually reconnects. When the access token is a JWT (RFC 9068), read its `exp` claim and use it as the authoritative expiry, falling back to the 365-day default only for opaque tokens. Explicit `expires_at`/`expires_in` still take precedence. Adds unit tests for storeTokens expiry resolution. Co-authored-by: Claude Opus 4.7 --- packages/api/src/mcp/oauth/tokens.test.ts | 90 +++++++++++++++++++++++ packages/api/src/mcp/oauth/tokens.ts | 48 +++++++++++- 2 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 packages/api/src/mcp/oauth/tokens.test.ts diff --git a/packages/api/src/mcp/oauth/tokens.test.ts b/packages/api/src/mcp/oauth/tokens.test.ts new file mode 100644 index 0000000000..f22c7f69c8 --- /dev/null +++ b/packages/api/src/mcp/oauth/tokens.test.ts @@ -0,0 +1,90 @@ +import jwt from 'jsonwebtoken'; +import type { OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth.js'; +import type { TokenMethods } from '@librechat/data-schemas'; +import { MCPTokenStorage } from './tokens'; + +jest.mock('@librechat/data-schemas', () => ({ + logger: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + }, + encryptV2: jest.fn(async (value: string) => `encrypted:${value}`), + decryptV2: jest.fn(async (value: string) => value.replace(/^encrypted:/, '')), +})); + +// Avoid pulling in librechat-data-provider via ~/mcp/utils; storeTokens does not use it. +jest.mock('~/mcp/utils', () => ({ + isInvalidClientMessage: jest.fn(() => false), +})); + +const DEFAULT_TTL_SECONDS = 365 * 24 * 60 * 60; + +/** Signs a JWT carrying the given `exp` (epoch seconds). Signature is irrelevant — storeTokens only decodes. */ +function makeJwt(expEpochSeconds: number): string { + return jwt.sign({ exp: expEpochSeconds, sub: 'test-user' }, 'test-secret'); +} + +/** Runs storeTokens with only createToken wired up and returns the stored access-token record. */ +async function storeAndCapture(tokens: OAuthTokens) { + const createToken = jest.fn().mockResolvedValue({}); + await MCPTokenStorage.storeTokens({ + userId: 'user-1', + serverName: 'salesforce', + tokens, + createToken: createToken as unknown as TokenMethods['createToken'], + }); + const accessTokenCall = createToken.mock.calls.find((call) => call[0]?.type === 'mcp_oauth'); + expect(accessTokenCall).toBeDefined(); + return accessTokenCall![0] as { expiresIn: number }; +} + +describe('MCPTokenStorage.storeTokens expiry handling', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('uses the JWT `exp` claim when the provider omits expires_in/expires_at', async () => { + const expSeconds = Math.floor(Date.now() / 1000) + 1800; // 30 minutes + const stored = await storeAndCapture({ + access_token: makeJwt(expSeconds), + token_type: 'Bearer', + }); + + // ~1800s, not the 365-day default. + expect(stored.expiresIn).toBeGreaterThanOrEqual(1798); + expect(stored.expiresIn).toBeLessThanOrEqual(1800); + expect(stored.expiresIn).toBeLessThan(DEFAULT_TTL_SECONDS); + }); + + it('falls back to the default TTL for opaque (non-JWT) tokens with no expiry', async () => { + const stored = await storeAndCapture({ + access_token: '00Dxx0000001gPF!AQ4AQP_opaque_salesforce_session_token', + token_type: 'Bearer', + }); + + expect(stored.expiresIn).toBe(DEFAULT_TTL_SECONDS); + }); + + it('still prefers an explicit expires_in over the JWT exp', async () => { + const expSeconds = Math.floor(Date.now() / 1000) + 1800; + const stored = await storeAndCapture({ + access_token: makeJwt(expSeconds), + token_type: 'Bearer', + expires_in: 900, + }); + + expect(stored.expiresIn).toBe(900); + }); + + it('ignores a JWT exp that is already in the past and uses the default TTL', async () => { + const expSeconds = Math.floor(Date.now() / 1000) - 100; // already expired + const stored = await storeAndCapture({ + access_token: makeJwt(expSeconds), + token_type: 'Bearer', + }); + + expect(stored.expiresIn).toBe(DEFAULT_TTL_SECONDS); + }); +}); diff --git a/packages/api/src/mcp/oauth/tokens.ts b/packages/api/src/mcp/oauth/tokens.ts index 5476cc302b..ae15268c8f 100644 --- a/packages/api/src/mcp/oauth/tokens.ts +++ b/packages/api/src/mcp/oauth/tokens.ts @@ -1,3 +1,4 @@ +import jwt from 'jsonwebtoken'; import { logger, encryptV2, decryptV2 } from '@librechat/data-schemas'; import type { OAuthTokens, OAuthClientInformation } from '@modelcontextprotocol/sdk/shared/auth.js'; import type { TokenMethods, IToken } from '@librechat/data-schemas'; @@ -54,6 +55,33 @@ interface GetTokensParams { deleteTokens?: TokenMethods['deleteTokens']; } +/** + * Reads the `exp` claim (RFC 7519 §4.1.4 / RFC 9068) from a JWT-format access + * token, returned as epoch milliseconds. Returns null for opaque (non-JWT) + * tokens or when no usable `exp` is present. The signature is intentionally + * not verified — the protected resource server validates the token; here we + * only read its self-declared expiry to avoid a lossy default. + */ +function getJwtAccessTokenExpiry(accessToken?: string): number | null { + if (!accessToken) { + return null; + } + try { + const decoded = jwt.decode(accessToken); + if ( + decoded != null && + typeof decoded !== 'string' && + typeof decoded.exp === 'number' && + Number.isFinite(decoded.exp) + ) { + return decoded.exp * 1000; + } + } catch { + /* Not a JWT or malformed — fall through to other expiry sources. */ + } + return null; +} + export class MCPTokenStorage { static getLogPrefix(userId: string, serverName: string): string { return isSystemUserId(userId) @@ -105,9 +133,23 @@ export class MCPTokenStorage { expiresInSeconds = tokens.expires_in; accessTokenExpiry = new Date(Date.now() + tokens.expires_in * 1000); } else { - logger.debug(`${logPrefix} No expiry provided, using default`); - expiresInSeconds = defaultTTL; - accessTokenExpiry = new Date(Date.now() + defaultTTL * 1000); + /** + * RFC 6749 §5.1 makes `expires_in` only RECOMMENDED, so some providers + * (e.g. Salesforce) omit it. When the access token is a JWT (RFC 9068), + * its `exp` claim is the authoritative lifetime — prefer it over the + * 365-day default so the token is refreshed on time rather than being + * treated as valid for a year and never refreshed. + */ + const jwtExpiryMs = getJwtAccessTokenExpiry(tokens.access_token); + if (jwtExpiryMs != null && jwtExpiryMs > Date.now()) { + logger.debug(`${logPrefix} Using JWT exp claim: ${new Date(jwtExpiryMs).toISOString()}`); + accessTokenExpiry = new Date(jwtExpiryMs); + expiresInSeconds = Math.floor((jwtExpiryMs - Date.now()) / 1000); + } else { + logger.debug(`${logPrefix} No expiry provided, using default`); + expiresInSeconds = defaultTTL; + accessTokenExpiry = new Date(Date.now() + defaultTTL * 1000); + } } logger.debug(`${logPrefix} Calculated expiry date: ${accessTokenExpiry.toISOString()}`);