mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-06-11 10:37:22 +00:00
⌛ fix: Use JWT exp claim for MCP when OAuth token omits expires_in (#13248)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
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 <noreply@anthropic.com>
This commit is contained in:
parent
03b477a84c
commit
01af63cb52
2 changed files with 135 additions and 3 deletions
90
packages/api/src/mcp/oauth/tokens.test.ts
Normal file
90
packages/api/src/mcp/oauth/tokens.test.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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()}`);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue