From a7f16911b26ebb3705a43fcc32295ecaabea370a Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 9 Jun 2026 17:50:02 -0400 Subject: [PATCH] =?UTF-8?q?=E2=8F=B3=20fix:=20Extend=20and=20Decouple=20MC?= =?UTF-8?q?P=20OAuth=20Flow=20Timeouts=20(#13622)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ⏳ fix: Extend and decouple MCP OAuth flow timeouts The OAuth auth button disappeared after 2 minutes (the internal OAuth handling timeout) while the flow state lived for 3 minutes, leaving users who didn't click immediately stuck in an unrecoverable re-auth loop. The handling timeouts also reused the connection/init timeout, so a short initTimeout would shrink the OAuth window further. - Add MCP_OAUTH_HANDLING_TIMEOUT (10m) and MCP_OAUTH_FLOW_TTL (15m) to mcpConfig - Decouple the reactive/proactive OAuth waits from initTimeout/connectionTimeout - Use OAUTH_FLOW_TTL for the FlowStateManager TTL and the UI status window - Ensure the flow TTL outlives the handling timeout, fixing the "Flow state not found" race - Remove dead FLOW_TTL constant and document new env vars Fixes #13615 * ⏳ fix: Coordinate OAuth pending window with handling timeout Address Codex review: the extended OAuth wait was still capped by other timeouts that were not updated. - Align PENDING_STALE_MS (button validity + pending-flow reuse window) with MCP_OAUTH_HANDLING_TIMEOUT so a flow stays reusable for the full wait instead of 2 minutes (Finding 3) - Clamp MCP_OAUTH_FLOW_TTL to never fall below the handling timeout so a callback near the deadline still finds its flow state (Finding 2) - Floor attemptToConnect's timeout to the handling window for OAuth servers so the reactive in-connect OAuth wait is not killed by the 30s connection timeout (Finding 1) - Update flow staleness tests to reference the threshold symbolically * ⏳ fix: Align OAuth window across status, action flows, and client polling Address Codex round 2: extending the server wait exposed three more windows that were still capped or now over-extended. - checkOAuthFlowStatus reports a PENDING flow as active only within the usable PENDING_STALE_MS window, not the longer Keyv retention TTL, so the connect button reappears instead of a stuck 'connecting' state - Give Action (custom tool) OAuth its own FlowStateManager on the prior 3-minute TTL so the longer MCP OAuth TTL can't leave an action tool call waiting up to 15 minutes - Extend the MCP server-card client polling to the 10-minute handling window so a user who completes OAuth after 3 minutes is still picked up * 🧪 test: Make stale-flow CSRF test track PENDING_STALE_MS The CSRF-fallback stale-flow test hardcoded a 3-minute age, which is now within the 10-minute PENDING_STALE_MS window and was wrongly treated as active. Derive the age from PENDING_STALE_MS so it tracks the constant. * ⏳ fix: Add grace buffers and surface OAuth timeout to the client Address Codex round 3 (near-deadline edges): - Clamp MCP_OAUTH_FLOW_TTL to handling timeout + 60s grace (not equality), so flow state outlives the wait instead of expiring at the same instant - Extend attemptToConnect's OAuth floor by a 60s grace so a user who authorizes near the deadline still gets the post-OAuth reconnect - Surface OAUTH_HANDLING_TIMEOUT on the connection-status response and have the client poll for the configured window instead of a hardcoded 10 minutes, so a tuned server deadline isn't capped on the client * ⏳ fix: Refresh client OAuth timeout from the first status refetch If the connection-status cache is empty when polling starts, the client captured the 10-minute fallback and never picked up a tuned oauthTimeout. Re-read it after each refetch so a longer configured deadline is honored even on a cold cache. * 📝 refactor: Type oauthTimeout on MCPConnectionStatusResponse Declare the oauthTimeout field on the shared response type in data-provider instead of an ad-hoc inline cast in the client hook, and replace the pre-existing 'as any' on the status query read with the typed getQueryData. Type-level only; no runtime change. --- .env.example | 6 +++ api/config/index.js | 25 ++++++++++- api/server/routes/__tests__/mcp.spec.js | 5 ++- api/server/routes/actions.js | 4 +- api/server/routes/mcp.js | 2 + api/server/services/ActionService.js | 6 +-- api/server/services/MCP.js | 6 ++- api/server/services/MCP.spec.js | 4 +- client/src/hooks/MCP/useMCPServerManager.ts | 43 ++++++++++++++----- packages/api/src/flow/manager.test.ts | 12 +++--- packages/api/src/flow/manager.ts | 11 ++++- packages/api/src/index.ts | 1 + packages/api/src/mcp/MCPConnectionFactory.ts | 14 +++++- .../mcp/__tests__/MCPOAuthTokenExpiry.test.ts | 12 +++--- packages/api/src/mcp/connection.ts | 2 +- packages/api/src/mcp/mcpConfig.ts | 19 ++++++++ packages/api/src/mcp/oauth/handler.ts | 13 +++--- packages/data-provider/src/types/queries.ts | 2 + 18 files changed, 141 insertions(+), 46 deletions(-) diff --git a/.env.example b/.env.example index 468a20968f..5e8d023ded 100644 --- a/.env.example +++ b/.env.example @@ -987,6 +987,12 @@ OPENWEATHER_API_KEY= # Timeout for OAuth detection requests in milliseconds # MCP_OAUTH_DETECTION_TIMEOUT=5000 +# How long to wait (ms) for a user to complete the OAuth flow before timing out (default: 10 minutes) +# MCP_OAUTH_HANDLING_TIMEOUT=600000 + +# TTL (ms) for OAuth flow state; must outlive MCP_OAUTH_HANDLING_TIMEOUT (default: 15 minutes) +# MCP_OAUTH_FLOW_TTL=900000 + # Cache connection status checks for this many milliseconds to avoid expensive verification # MCP_CONNECTION_CHECK_TTL=60000 diff --git a/api/config/index.js b/api/config/index.js index 3b6d869332..9adfee5637 100644 --- a/api/config/index.js +++ b/api/config/index.js @@ -1,6 +1,7 @@ const { EventSource } = require('eventsource'); const { Time } = require('librechat-data-provider'); const { + mcpConfig, MCPManager, FlowStateManager, MCPServersRegistry, @@ -10,22 +11,41 @@ const logger = require('./winston'); global.EventSource = EventSource; -/** @type {MCPManager} */ +/** @type {FlowStateManager} */ let flowManager = null; +/** @type {FlowStateManager} */ +let actionFlowManager = null; /** + * Flow manager for MCP OAuth flows. Uses the longer MCP OAuth TTL so the auth + * button and flow state outlive the user-completion window. * @param {Keyv} flowsCache * @returns {FlowStateManager} */ function getFlowStateManager(flowsCache) { if (!flowManager) { flowManager = new FlowStateManager(flowsCache, { - ttl: Time.ONE_MINUTE * 3, + ttl: mcpConfig.OAUTH_FLOW_TTL, }); } return flowManager; } +/** + * Flow manager for Action (custom tool) OAuth flows. Kept on the shorter TTL so an + * unclicked action login does not leave the tool call waiting for the MCP OAuth window. + * @param {Keyv} flowsCache + * @returns {FlowStateManager} + */ +function getActionFlowStateManager(flowsCache) { + if (!actionFlowManager) { + actionFlowManager = new FlowStateManager(flowsCache, { + ttl: Time.ONE_MINUTE * 3, + }); + } + return actionFlowManager; +} + module.exports = { logger, createMCPServersRegistry: MCPServersRegistry.createInstance, @@ -33,6 +53,7 @@ module.exports = { createMCPManager: MCPManager.createInstance, getMCPManager: MCPManager.getInstance, getFlowStateManager, + getActionFlowStateManager, createOAuthReconnectionManager: OAuthReconnectionManager.createInstance, getOAuthReconnectionManager: OAuthReconnectionManager.getInstance, }; diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index ee316811a8..9d953e93f8 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -3,7 +3,7 @@ const express = require('express'); const request = require('supertest'); const mongoose = require('mongoose'); const cookieParser = require('cookie-parser'); -const { getBasePath } = require('@librechat/api'); +const { getBasePath, PENDING_STALE_MS } = require('@librechat/api'); const { MongoMemoryServer } = require('mongodb-memory-server'); function generateTestCsrfToken(flowId) { @@ -690,7 +690,7 @@ describe('MCP Routes', () => { const mockFlowManager = { getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING', - createdAt: Date.now() - 3 * 60 * 1000, + createdAt: Date.now() - PENDING_STALE_MS - 60 * 1000, }), }; @@ -1665,6 +1665,7 @@ describe('MCP Routes', () => { expect(response.status).toBe(200); expect(response.body).toEqual({ success: true, + oauthTimeout: expect.any(Number), connectionStatus: { server1: { connectionState: 'connected', diff --git a/api/server/routes/actions.js b/api/server/routes/actions.js index 38d0dc8c94..d9a2f2f7fa 100644 --- a/api/server/routes/actions.js +++ b/api/server/routes/actions.js @@ -14,7 +14,7 @@ const { } = require('@librechat/api'); const { findToken, updateToken, createToken } = require('~/models'); const { requireJwtAuth } = require('~/server/middleware'); -const { getFlowStateManager } = require('~/config'); +const { getActionFlowStateManager } = require('~/config'); const { getLogStores } = require('~/cache'); const router = express.Router(); @@ -56,7 +56,7 @@ router.get('/:action_id/oauth/callback', async (req, res) => { const { action_id } = req.params; const { code, state } = req.query; const flowsCache = getLogStores(CacheKeys.FLOWS); - const flowManager = getFlowStateManager(flowsCache); + const flowManager = getActionFlowStateManager(flowsCache); const basePath = getBasePath(); let identifier = action_id; try { diff --git a/api/server/routes/mcp.js b/api/server/routes/mcp.js index d3de8e29ab..41c1bcd7bd 100644 --- a/api/server/routes/mcp.js +++ b/api/server/routes/mcp.js @@ -14,6 +14,7 @@ const { MCPTokenStorage, setOAuthSession, PENDING_STALE_MS, + mcpConfig: mcpSettings, getUserMCPAuthMap, validateOAuthCsrf, OAUTH_CSRF_COOKIE, @@ -715,6 +716,7 @@ router.get('/connection/status', requireJwtAuth, async (req, res) => { res.json({ success: true, connectionStatus, + oauthTimeout: mcpSettings.OAUTH_HANDLING_TIMEOUT, }); } catch (error) { logger.error('[MCP Connection Status] Failed to get connection status', error); diff --git a/api/server/services/ActionService.js b/api/server/services/ActionService.js index 2e324b5312..db8a536d8e 100644 --- a/api/server/services/ActionService.js +++ b/api/server/services/ActionService.js @@ -29,7 +29,7 @@ const { deleteActions, deleteAssistant, } = require('~/models'); -const { getFlowStateManager } = require('~/config'); +const { getActionFlowStateManager } = require('~/config'); const { getLogStores } = require('~/cache'); const JWT_SECRET = process.env.JWT_SECRET; @@ -243,7 +243,7 @@ async function createActionTool({ }, }; const flowsCache = getLogStores(CacheKeys.FLOWS); - const flowManager = getFlowStateManager(flowsCache); + const flowManager = getActionFlowStateManager(flowsCache); await flowManager.createFlowWithHandler( `${identifier}:oauth_login:${config.metadata.thread_id}:${config.metadata.run_id}`, 'oauth_login', @@ -341,7 +341,7 @@ async function createActionTool({ }, ); const flowsCache = getLogStores(CacheKeys.FLOWS); - const flowManager = getFlowStateManager(flowsCache); + const flowManager = getActionFlowStateManager(flowsCache); const refreshData = await flowManager.createFlowWithHandler( `${identifier}:refresh`, 'oauth_refresh', diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index dbd2c16f39..a837901bc0 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -3,6 +3,7 @@ const { logger, getTenantId } = require('@librechat/data-schemas'); const { Providers, Constants: AgentConstants } = require('@librechat/agents'); const { sendEvent, + PENDING_STALE_MS, MCPOAuthHandler, isMCPDomainAllowed, normalizeServerName, @@ -852,7 +853,10 @@ async function checkOAuthFlowStatus(userId, serverName) { } const flowAge = Date.now() - flowState.createdAt; - const flowTTL = flowState.ttl || 180000; // Default 3 minutes + // Report active only while the flow is still usable (the handling/reuse window), + // not for the full Keyv retention TTL — otherwise the UI shows "connecting" for a + // flow the initiate/callback paths already reject, hiding the connect button. + const flowTTL = flowState.ttl || PENDING_STALE_MS; if (flowState.status === 'FAILED' || flowAge > flowTTL) { const wasCancelled = flowState.error && flowState.error.includes('cancelled'); diff --git a/api/server/services/MCP.spec.js b/api/server/services/MCP.spec.js index cee8bccf40..5ac6f825d6 100644 --- a/api/server/services/MCP.spec.js +++ b/api/server/services/MCP.spec.js @@ -329,8 +329,8 @@ describe('tests for the new helper functions used by the MCP connection status e it('should detect failed flow when TTL not specified and flow exceeds default TTL', async () => { const mockFlowState = { status: 'PENDING', - createdAt: Date.now() - 200000, // 200 seconds ago (> 180s default TTL) - // ttl not specified, should use 180000 default + createdAt: Date.now() - 16 * 60 * 1000, // 16 minutes ago (past the PENDING_STALE_MS window) + // ttl not specified, should fall back to the PENDING_STALE_MS default }; const mockFlowManager = { getFlowState: jest.fn(() => mockFlowState) }; mockGetFlowStateManager.mockReturnValue(mockFlowManager); diff --git a/client/src/hooks/MCP/useMCPServerManager.ts b/client/src/hooks/MCP/useMCPServerManager.ts index 7803107f4d..6126545c35 100644 --- a/client/src/hooks/MCP/useMCPServerManager.ts +++ b/client/src/hooks/MCP/useMCPServerManager.ts @@ -16,7 +16,12 @@ import { useReinitializeMCPServerMutation, useGetAllEffectivePermissionsQuery, } from 'librechat-data-provider/react-query'; -import type { TUpdateUserPlugins, TPlugin, MCPServersResponse } from 'librechat-data-provider'; +import type { + TUpdateUserPlugins, + TPlugin, + MCPServersResponse, + MCPConnectionStatusResponse, +} from 'librechat-data-provider'; import type { MCPServerInitState } from '~/store/mcp'; import type { ConfigFieldDetail } from '~/common'; import { useLocalize, useHasAccess, useMCPSelect, useMCPConnectionStatus } from '~/hooks'; @@ -197,29 +202,39 @@ export function useMCPServerManager({ let pollAttempts = 0; let timeoutId: NodeJS.Timeout | null = null; - /** OAuth typically completes in 5 seconds to 3 minutes - * We enforce a strict 3-minute timeout with gradual backoff + /** OAuth can take several minutes if the user steps away from the consent screen. + * Poll for the full server-side handling window (MCP_OAUTH_HANDLING_TIMEOUT + * default = 10 minutes) with gradual backoff, so the button stays usable as long + * as the server will accept the callback. */ const getPollInterval = (attempt: number): number => { if (attempt < 12) return 5000; // First minute: every 5s (12 polls) if (attempt < 22) return 6000; // Second minute: every 6s (10 polls) - return 7500; // Final minute: every 7.5s (8 polls) + return 7500; // Thereafter: every 7.5s }; - const maxAttempts = 30; // Exactly 3 minutes (180 seconds) total - const OAUTH_TIMEOUT_MS = 180000; // 3 minutes in milliseconds + /** Honor the server's configured MCP_OAUTH_HANDLING_TIMEOUT (surfaced on the + * connection-status response) so a tuned deadline isn't capped at the default. + * The cache may be empty at start, so this is refreshed from the first status + * refetch below rather than captured once. */ + const connectionData = queryClient.getQueryData([ + QueryKeys.mcpConnectionStatus, + ]); + let oauthTimeoutMs = connectionData?.oauthTimeout ?? 600000; // default 10 minutes + // Backstop only; the elapsed-time guard governs. Sized above the worst-case poll count. + let maxAttempts = Math.ceil(oauthTimeoutMs / 5000) + 5; const pollOnce = async () => { try { pollAttempts++; const state = getServerInitState(serverInitStates, serverName); - /** Stop polling after 3 minutes or max attempts */ + /** Stop polling once the handling window or max attempts is exceeded */ const elapsedTime = state?.oauthStartTime ? Date.now() - state.oauthStartTime : pollAttempts * 5000; // Rough estimate if no start time - if (pollAttempts > maxAttempts || elapsedTime > OAUTH_TIMEOUT_MS) { + if (pollAttempts > maxAttempts || elapsedTime > oauthTimeoutMs) { console.warn( `[MCP Manager] OAuth timeout for ${serverName} after ${(elapsedTime / 1000).toFixed(0)}s (attempt ${pollAttempts})`, ); @@ -236,9 +251,15 @@ export function useMCPServerManager({ await queryClient.refetchQueries([QueryKeys.mcpConnectionStatus]); - const freshConnectionData = queryClient.getQueryData([ + const freshConnectionData = queryClient.getQueryData([ QueryKeys.mcpConnectionStatus, - ]) as any; + ]); + // Pick up the configured timeout once the status response lands (cache may have + // been empty when polling started), so a tuned deadline is honored mid-flight. + if (typeof freshConnectionData?.oauthTimeout === 'number') { + oauthTimeoutMs = freshConnectionData.oauthTimeout; + maxAttempts = Math.ceil(oauthTimeoutMs / 5000) + 5; + } const freshConnectionStatus = freshConnectionData?.connectionStatus || {}; const serverStatus = freshConnectionStatus[serverName]; @@ -269,7 +290,7 @@ export function useMCPServerManager({ } // Check for OAuth timeout (should align with maxAttempts) - if (state?.oauthStartTime && Date.now() - state.oauthStartTime > OAUTH_TIMEOUT_MS) { + if (state?.oauthStartTime && Date.now() - state.oauthStartTime > oauthTimeoutMs) { showToast({ message: localize('com_ui_mcp_oauth_timeout', { 0: serverName }), status: 'error', diff --git a/packages/api/src/flow/manager.test.ts b/packages/api/src/flow/manager.test.ts index b34dcbafab..7112a822d4 100644 --- a/packages/api/src/flow/manager.test.ts +++ b/packages/api/src/flow/manager.test.ts @@ -1,5 +1,5 @@ import { Keyv } from 'keyv'; -import { FlowStateManager } from './manager'; +import { FlowStateManager, PENDING_STALE_MS } from './manager'; import { FlowState } from './types'; jest.mock('@librechat/data-schemas', () => ({ @@ -965,21 +965,21 @@ describe('FlowStateManager', () => { expect(result2.isStale).toBe(false); }); - it('uses default threshold of 2 minutes when not specified', async () => { - const timestamp = Date.now() - 3 * 60 * 1000; // 3 minutes ago + it('uses the default PENDING_STALE_MS threshold when not specified', async () => { + const timestamp = Date.now() - (PENDING_STALE_MS + 60 * 1000); // just past the default await store.set(flowKey, { type, status: 'COMPLETED', metadata: {}, - createdAt: Date.now() - 5 * 60 * 1000, + createdAt: Date.now() - (PENDING_STALE_MS + 3 * 60 * 1000), completedAt: timestamp, }); - // Should use default 2 minute threshold + // Should use the default PENDING_STALE_MS threshold const result = await flowManager.isFlowStale(flowId, type); expect(result.isStale).toBe(true); - expect(result.age).toBeGreaterThan(2 * 60 * 1000); + expect(result.age).toBeGreaterThan(PENDING_STALE_MS); }); it('falls back to createdAt when completedAt/failedAt are not present', async () => { diff --git a/packages/api/src/flow/manager.ts b/packages/api/src/flow/manager.ts index 1f36dfe98d..27de97bbe6 100644 --- a/packages/api/src/flow/manager.ts +++ b/packages/api/src/flow/manager.ts @@ -3,8 +3,17 @@ import { logger } from '@librechat/data-schemas'; import type { StoredDataNoRaw } from 'keyv'; import type { FlowState, FlowMetadata, FlowManagerOptions } from './types'; import { registerShutdownTask } from '../app/shutdown'; +import { math } from '~/utils/math'; -export const PENDING_STALE_MS: number = 2 * 60 * 1000; +/** + * Lifetime of a PENDING OAuth flow: how long the auth button stays valid and an + * in-flight flow can be reused before it is replaced. Mirrors + * `mcpConfig.OAUTH_HANDLING_TIMEOUT` (`MCP_OAUTH_HANDLING_TIMEOUT`) so the reuse + * window matches the wait the server grants the user. Default: 10 minutes. + */ +export const PENDING_STALE_MS: number = math( + process.env.MCP_OAUTH_HANDLING_TIMEOUT ?? 10 * 60 * 1000, +); const SECONDS_THRESHOLD = 1e10; diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index d923a4e75d..f0a9bcc48f 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -9,6 +9,7 @@ export * from './auth'; /* API Keys */ export * from './apiKeys'; /* MCP */ +export * from './mcp/mcpConfig'; export * from './mcp/registry/MCPServersRegistry'; export * from './mcp/MCPManager'; export * from './mcp/connection'; diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index c604aa57cf..3905ac1d8a 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -18,6 +18,7 @@ import { PENDING_STALE_MS, normalizeExpiresAt } from '~/flow/manager'; import { withTimeout } from '~/utils/promise'; import { MCPConnection } from './connection'; import { processMCPEnv } from '~/utils'; +import { mcpConfig } from './mcpConfig'; export interface ToolDiscoveryResult { tools: Tool[] | null; @@ -365,7 +366,7 @@ export class MCPConnectionFactory { throw new Error(`${this.logPrefix} OAuth required but server URL is missing from config`); } - const oauthTimeout = this.connectionTimeout ?? 60000 * 2; + const oauthTimeout = mcpConfig.OAUTH_HANDLING_TIMEOUT; logger.info( `${this.logPrefix} No stored tokens, proactively triggering OAuth flow before connecting (timeout: ${oauthTimeout}ms)`, ); @@ -633,7 +634,16 @@ export class MCPConnectionFactory { /** Attempts to establish connection with timeout handling */ protected async attemptToConnect(connection: MCPConnection): Promise { - const connectTimeout = this.connectionTimeout ?? this.serverConfig.initTimeout ?? 30000; + const baseTimeout = this.connectionTimeout ?? this.serverConfig.initTimeout ?? 30000; + // OAuth servers may pause mid-connect to wait for the user to authorize in the browser. + // The transport connect itself is still bounded by initTimeout inside connection.connect(), + // so this floor only extends the window for an active OAuth wait, not ordinary failures. + // The grace covers the reconnect after `oauthHandled` (retry backoff + transport connect), + // which happens *after* the handling wait, so a user who authorizes near the deadline still + // gets a connection instead of a timeout. + const connectTimeout = this.useOAuth + ? Math.max(baseTimeout, mcpConfig.OAUTH_HANDLING_TIMEOUT + 60000) + : baseTimeout; await withTimeout( this.connectTo(connection), connectTimeout, diff --git a/packages/api/src/mcp/__tests__/MCPOAuthTokenExpiry.test.ts b/packages/api/src/mcp/__tests__/MCPOAuthTokenExpiry.test.ts index b5cbc869a8..dad34d31be 100644 --- a/packages/api/src/mcp/__tests__/MCPOAuthTokenExpiry.test.ts +++ b/packages/api/src/mcp/__tests__/MCPOAuthTokenExpiry.test.ts @@ -13,11 +13,11 @@ import { Keyv } from 'keyv'; import { logger } from '@librechat/data-schemas'; -import { FlowStateManager, PENDING_STALE_MS } from '~/flow/manager'; -import { MCPTokenStorage, ReauthenticationRequiredError } from '~/mcp/oauth'; -import { MockKeyv, InMemoryTokenStore, createOAuthMCPServer } from './helpers/oauthTestServer'; import type { OAuthTestServer } from './helpers/oauthTestServer'; import type { MCPOAuthTokens } from '~/mcp/oauth'; +import { MockKeyv, InMemoryTokenStore, createOAuthMCPServer } from './helpers/oauthTestServer'; +import { MCPTokenStorage, ReauthenticationRequiredError } from '~/mcp/oauth'; +import { FlowStateManager, PENDING_STALE_MS } from '~/flow/manager'; jest.mock('@librechat/data-schemas', () => ({ logger: { @@ -613,10 +613,10 @@ describe('MCP OAuth Token Expiry Scenarios', () => { authorizationUrl: 'https://example.com/auth', }); - // Manually age the flow to 3 minutes + // Manually age the flow past the staleness window const state = await flowManager.getFlowState(flowId, 'mcp_oauth'); if (state) { - state.createdAt = Date.now() - 3 * 60 * 1000; + state.createdAt = Date.now() - PENDING_STALE_MS - 60 * 1000; await (flowStore as unknown as { set: (k: string, v: unknown) => Promise }).set( `mcp_oauth:${flowId}`, state, @@ -627,7 +627,7 @@ describe('MCP OAuth Token Expiry Scenarios', () => { expect(agedState?.status).toBe('PENDING'); const age = agedState?.createdAt ? Date.now() - agedState.createdAt : 0; - expect(age).toBeGreaterThan(2 * 60 * 1000); + expect(age).toBeGreaterThan(PENDING_STALE_MS); // A new flow should be created (the stale one would be deleted + recreated) // This verifies our staleness check threshold diff --git a/packages/api/src/mcp/connection.ts b/packages/api/src/mcp/connection.ts index 35e619bbba..217a44d89a 100644 --- a/packages/api/src/mcp/connection.ts +++ b/packages/api/src/mcp/connection.ts @@ -1905,7 +1905,7 @@ export class MCPConnection extends EventEmitter { `${this.getLogPrefix()} Server URL for OAuth: ${serverUrl ? sanitizeUrlForLogging(serverUrl) : 'undefined'}`, ); - const oauthTimeout = this.options.initTimeout ?? 60000 * 2; + const oauthTimeout = mcpConfig.OAUTH_HANDLING_TIMEOUT; /** Promise that will resolve when OAuth is handled */ const oauthHandledPromise = new Promise((resolve, reject) => { let timeoutId: NodeJS.Timeout | null = null; diff --git a/packages/api/src/mcp/mcpConfig.ts b/packages/api/src/mcp/mcpConfig.ts index 824b0fd3fd..68e22a81c2 100644 --- a/packages/api/src/mcp/mcpConfig.ts +++ b/packages/api/src/mcp/mcpConfig.ts @@ -1,5 +1,16 @@ import { math, isEnabled } from '~/utils'; +const oauthHandlingTimeout = math(process.env.MCP_OAUTH_HANDLING_TIMEOUT ?? 10 * 60 * 1000); +/** Grace so flow state outlives the handling wait rather than expiring at the same instant — + * covers callback processing, the monitor poll interval, and multi-replica clock skew. */ +const OAUTH_FLOW_TTL_GRACE_MS = 60 * 1000; +/** Flow state must outlive the handling wait, otherwise a callback arriving near the + * deadline cannot find its flow. Clamp the configured TTL above the handling timeout. */ +const oauthFlowTtl = Math.max( + math(process.env.MCP_OAUTH_FLOW_TTL ?? 15 * 60 * 1000), + oauthHandlingTimeout + OAUTH_FLOW_TTL_GRACE_MS, +); + /** * Centralized configuration for MCP-related environment variables. * Provides typed access to MCP settings with default values. @@ -7,6 +18,10 @@ import { math, isEnabled } from '~/utils'; export const mcpConfig: { OAUTH_ON_AUTH_ERROR: boolean; OAUTH_DETECTION_TIMEOUT: number; + /** How long (ms) to wait for the user to complete an OAuth flow before timing out. Default: 10 minutes */ + OAUTH_HANDLING_TIMEOUT: number; + /** TTL (ms) for OAuth flow state. Must outlive OAUTH_HANDLING_TIMEOUT so the state survives the wait. Default: 15 minutes */ + OAUTH_FLOW_TTL: number; CONNECTION_CHECK_TTL: number; /** Idle timeout (ms) after which user connections are disconnected. Default: 15 minutes */ USER_CONNECTION_IDLE_TIMEOUT: number; @@ -27,6 +42,10 @@ export const mcpConfig: { } = { OAUTH_ON_AUTH_ERROR: isEnabled(process.env.MCP_OAUTH_ON_AUTH_ERROR ?? true), OAUTH_DETECTION_TIMEOUT: math(process.env.MCP_OAUTH_DETECTION_TIMEOUT ?? 5000), + /** How long (ms) to wait for the user to complete an OAuth flow before timing out. Default: 10 minutes */ + OAUTH_HANDLING_TIMEOUT: oauthHandlingTimeout, + /** TTL (ms) for OAuth flow state. Clamped to never fall below OAUTH_HANDLING_TIMEOUT. Default: 15 minutes */ + OAUTH_FLOW_TTL: oauthFlowTtl, CONNECTION_CHECK_TTL: math(process.env.MCP_CONNECTION_CHECK_TTL ?? 60000), /** Idle timeout (ms) after which user connections are disconnected. Default: 15 minutes */ USER_CONNECTION_IDLE_TIMEOUT: math( diff --git a/packages/api/src/mcp/oauth/handler.ts b/packages/api/src/mcp/oauth/handler.ts index be503ae97e..db54e9600b 100644 --- a/packages/api/src/mcp/oauth/handler.ts +++ b/packages/api/src/mcp/oauth/handler.ts @@ -1,7 +1,7 @@ import { randomBytes } from 'crypto'; import { logger } from '@librechat/data-schemas'; -import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport'; import { OAuthMetadataSchema } from '@modelcontextprotocol/sdk/shared/auth.js'; +import { TokenExchangeMethodEnum, type MCPOptions } from 'librechat-data-provider'; import { checkResourceAllowed, resourceUrlFromServerUrl, @@ -13,9 +13,8 @@ import { discoverAuthorizationServerMetadata, discoverOAuthProtectedResourceMetadata, } from '@modelcontextprotocol/sdk/client/auth.js'; -import { TokenExchangeMethodEnum, type MCPOptions } from 'librechat-data-provider'; +import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport'; import type { TokenMethods } from '@librechat/data-schemas'; -import type { FlowStateManager } from '~/flow/manager'; import type { OAuthClientInformation, OAuthProtectedResourceMetadata, @@ -23,6 +22,7 @@ import type { MCPOAuthTokens, OAuthMetadata, } from './types'; +import type { FlowStateManager } from '~/flow/manager'; import { resolveTokenEndpointAuthMethod, getForcedTokenEndpointAuthMethod, @@ -31,17 +31,16 @@ import { } from './methods'; import { isSSRFTarget, resolveHostnameSSRF, isOAuthUrlAllowed } from '~/auth'; import { probeResourceMetadataHint } from './resourceHint'; -import { MCPTokenStorage } from './tokens'; import { createHardenedOAuthFetch } from './hardenedFetch'; -import { getOAuthUrlPort } from './url'; import { sanitizeUrlForLogging } from '~/mcp/utils'; +import { MCPTokenStorage } from './tokens'; +import { getOAuthUrlPort } from './url'; /** Type for the OAuth metadata from the SDK */ type SDKOAuthMetadata = Parameters[1]['metadata']; export class MCPOAuthHandler { private static readonly FLOW_TYPE = 'mcp_oauth'; - private static readonly FLOW_TTL = 10 * 60 * 1000; // 10 minutes /** * Creates a fetch function with custom headers injected @@ -836,7 +835,7 @@ export class MCPOAuthHandler { if (metadata.resourceMetadata) { /** * Defense-in-depth: re-assert the RFC 9728 §3.3 binding against the flow's stored - * server URL. Flow state has a 10-minute TTL, so a flow initiated under older + * server URL. Flow state has a bounded TTL, so a flow initiated under older * (pre-fix) code could still be in-flight at upgrade time carrying unvalidated * resource metadata. Re-validating here closes that window without requiring ops * teams to flush flow state on deploy (GHSA-gvpj-vm2f-2m23). diff --git a/packages/data-provider/src/types/queries.ts b/packages/data-provider/src/types/queries.ts index adbddcd41c..abe773fce2 100644 --- a/packages/data-provider/src/types/queries.ts +++ b/packages/data-provider/src/types/queries.ts @@ -201,6 +201,8 @@ export interface MCPServerStatus { export interface MCPConnectionStatusResponse { success: boolean; connectionStatus: Record; + /** Server-configured OAuth completion window in ms (`MCP_OAUTH_HANDLING_TIMEOUT`) */ + oauthTimeout?: number; } export interface MCPServerConnectionStatusResponse {