fix: Avoid reprocessing MCP OAuth reauth config

This commit is contained in:
Danny Avila 2026-05-29 11:03:10 -07:00
parent d8602de446
commit 7f60fdf38e
5 changed files with 105 additions and 7 deletions

View file

@ -245,13 +245,15 @@ export class MCPConnectionFactory {
basic: t.BasicConnectionOptions,
options?: t.OAuthConnectionOptions | t.UserConnectionContext,
) {
this.serverConfig = processMCPEnv({
user: options?.user,
body: options?.requestBody,
dbSourced: basic.dbSourced,
options: basic.serverConfig,
customUserVars: options?.customUserVars,
});
this.serverConfig = basic.skipEnvProcessing
? basic.serverConfig
: processMCPEnv({
user: options?.user,
body: options?.requestBody,
dbSourced: basic.dbSourced,
options: basic.serverConfig,
customUserVars: options?.customUserVars,
});
this.serverName = basic.serverName;
this.useSSRFProtection = basic.useSSRFProtection === true;
this.allowedDomains = basic.allowedDomains;

View file

@ -355,6 +355,7 @@ Please follow these instructions when using tools from the respective MCP server
serverName,
serverConfig: currentOptions,
dbSourced: isDbSourced,
skipEnvProcessing: true,
useSSRFProtection: registry.shouldEnableSSRFProtection(),
allowedDomains: registry.getAllowedDomains(),
allowedAddresses: registry.getAllowedAddresses(),

View file

@ -1648,6 +1648,38 @@ describe('MCPConnectionFactory', () => {
expect(mockMCPOAuthHandler.initiateOAuthFlow).toHaveBeenCalled();
});
it('should not process already resolved configs for request OAuth handlers', () => {
const sseConfig = {
...mockServerConfig,
url: 'https://api.example.com/${SHOULD_NOT_EXPAND}',
type: 'sse' as const,
} as t.SSEOptions;
mockProcessMCPEnv.mockClear();
const cleanup = MCPConnectionFactory.attachRequestOAuthHandler(
{
serverName: 'test-server',
serverConfig: sseConfig,
skipEnvProcessing: true,
},
{
useOAuth: true,
user: mockUser,
flowManager: mockFlowManager,
oauthStart: jest.fn(),
},
mockConnectionInstance,
);
expect(mockProcessMCPEnv).not.toHaveBeenCalled();
expect(mockConnectionInstance.on).toHaveBeenCalledWith(
'oauthReauthenticationRequired',
expect.any(Function),
);
cleanup();
});
it('should not reuse request-scoped OAuth callbacks after connection is cached', async () => {
const sseConfig = {
...mockServerConfig,

View file

@ -9,6 +9,7 @@ import { ConnectionsRepository } from '~/mcp/ConnectionsRepository';
import { MCPConnection } from '~/mcp/connection';
import { MCPManager } from '~/mcp/MCPManager';
import * as graphUtils from '~/utils/graph';
import { processMCPEnv } from '~/utils/env';
// Mock external dependencies
jest.mock('@librechat/data-schemas', () => ({
@ -55,6 +56,7 @@ jest.mock('~/mcp/ConnectionsRepository');
jest.mock('~/mcp/MCPConnectionFactory');
const mockLogger = logger as jest.Mocked<typeof logger>;
const mockProcessMCPEnv = processMCPEnv as jest.MockedFunction<typeof processMCPEnv>;
describe('MCPManager', () => {
const userId = 'test-user-123';
@ -534,6 +536,65 @@ describe('MCPManager', () => {
);
});
it('should attach request OAuth handler without reprocessing resolved config', async () => {
const rawServerConfig = {
type: 'sse',
url: 'https://api.example.com/{{LIBRECHAT_USER_ID}}',
headers: {
Authorization: 'Bearer {{USER_TOKEN}}',
},
requiresOAuth: true,
oauth: {
authorization_url: 'https://auth.example.com/authorize',
},
} as t.ParsedServerConfig;
const processedServerConfig = {
...rawServerConfig,
url: 'https://api.example.com/user-123',
headers: {
Authorization: 'Bearer ${SHOULD_NOT_EXPAND}',
},
};
const cleanupOAuthHandler = jest.fn();
mockProcessMCPEnv.mockReturnValueOnce(processedServerConfig);
(MCPConnectionFactory.attachRequestOAuthHandler as jest.Mock).mockReturnValue(
cleanupOAuthHandler,
);
mockAppConnections({
get: jest.fn().mockResolvedValue(mockConnection),
});
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(rawServerConfig);
const manager = await MCPManager.createInstance(newMCPServersConfig());
const oauthStart = jest.fn();
await manager.callTool({
user: mockUser as IUser,
serverName,
toolName: 'test_tool',
provider: 'openai',
oauthStart,
flowManager: mockFlowManager as unknown as Parameters<
typeof manager.callTool
>[0]['flowManager'],
});
expect(mockProcessMCPEnv).toHaveBeenCalledTimes(1);
expect(MCPConnectionFactory.attachRequestOAuthHandler).toHaveBeenCalledWith(
expect.objectContaining({
serverConfig: processedServerConfig,
skipEnvProcessing: true,
}),
expect.objectContaining({
oauthStart,
user: mockUser,
}),
mockConnection,
);
expect(cleanupOAuthHandler).toHaveBeenCalled();
});
it('should pass options unchanged when no graphTokenResolver is provided', async () => {
const serverConfig: t.SSEOptions = {
type: 'sse',

View file

@ -184,6 +184,8 @@ export interface BasicConnectionOptions {
allowedAddresses?: string[] | null;
/** When true, only resolve customUserVars in processMCPEnv (for DB-stored servers) */
dbSourced?: boolean;
/** When true, serverConfig has already gone through processMCPEnv for this request */
skipEnvProcessing?: boolean;
}
/** User context for placeholder resolution in MCP connections (non-OAuth and OAuth alike) */