CERBERUS/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2026-08-03 22:32:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

🔐 fix: Reuse MCP OAuth Authorization URL (#13532)

Browse source 
* fix: reuse MCP OAuth authorization URL

* fix: validate MCP OAuth initiate flow ID
This commit is contained in:
Danny Avila 2026-06-05 17:18:59 -04:00 • committed by GitHub
parent 2ed59ac98a
commit da5876331e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 159 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

104
api/server/routes/__tests__/mcp.spec.js
View file

@ -184,6 +184,9 @@ describe('MCP Routes', () => {
beforeEach(() => {
jest.clearAllMocks();
currentUser = undefined;
require('@librechat/api').MCPOAuthHandler.generateFlowId.mockImplementation(
(userId, serverName) => `${userId}:${serverName}`,
);
mockResolveAllMcpConfigs.mockResolvedValue({});
mockResolveMcpConfigNames.mockResolvedValue([]);
mockMCPUseAllowed = true;
@ -203,11 +206,70 @@ describe('MCP Routes', () => {
const { MCPOAuthHandler } = require('@librechat/api');
const { getLogStores } = require('~/cache');
it('should initiate OAuth flow successfully', async () => {
it('should reuse stored authorization URL without starting a new OAuth flow', async () => {
const mockFlowManager = {
getFlowState: jest.fn().mockResolvedValue({
status: 'PENDING',
createdAt: Date.now(),
metadata: {
serverName: 'test-server',
userId: 'test-user-id',
authorizationUrl: 'https://oauth.example.com/auth?state=stored-state',
},
}),
};
getLogStores.mockReturnValue({});
require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager);
const response = await request(app).get('/api/mcp/test-server/oauth/initiate').query({
userId: 'test-user-id',
flowId: 'test-user-id:test-server',
});
expect(response.status).toBe(302);
expect(response.headers.location).toBe('https://oauth.example.com/auth?state=stored-state');
expect(response.headers['set-cookie']?.join('')).toContain('oauth_csrf=');
expect(MCPOAuthHandler.initiateOAuthFlow).not.toHaveBeenCalled();
expect(MCPOAuthHandler.storeStateMapping).not.toHaveBeenCalled();
expect(mockRegistryInstance.getServerConfig).not.toHaveBeenCalled();
});
it('should reject stored authorization URL when flow is no longer pending', async () => {
const mockFlowManager = {
getFlowState: jest.fn().mockResolvedValue({
status: 'COMPLETED',
createdAt: Date.now(),
metadata: {
serverName: 'test-server',
userId: 'test-user-id',
authorizationUrl: 'https://oauth.example.com/auth?state=stored-state',
},
}),
};
getLogStores.mockReturnValue({});
require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager);
const response = await request(app).get('/api/mcp/test-server/oauth/initiate').query({
userId: 'test-user-id',
flowId: 'test-user-id:test-server',
});
expect(response.status).toBe(400);
expect(response.body).toEqual({ error: 'Invalid flow state' });
expect(MCPOAuthHandler.initiateOAuthFlow).not.toHaveBeenCalled();
expect(MCPOAuthHandler.storeStateMapping).not.toHaveBeenCalled();
});
it('should initiate OAuth flow when stored authorization URL is missing', async () => {
const mockFlowManager = {
getFlowState: jest.fn().mockResolvedValue({
status: 'PENDING',
createdAt: Date.now(),
metadata: {
serverUrl: 'https://test-server.com',
state: 'old-state-value',
oauth: { clientId: 'test-client-id' },
},
}),
@ -242,6 +304,23 @@ describe('MCP Routes', () => {
undefined,
null,
);
expect(MCPOAuthHandler.deleteStateMapping).toHaveBeenCalledWith(
'old-state-value',
mockFlowManager,
);
expect(mockFlowManager.initFlow).toHaveBeenCalledWith(
'test-user-id:test-server',
'mcp_oauth',
expect.objectContaining({
state: 'random-state-value',
authorizationUrl: 'https://oauth.example.com/auth',
}),
);
expect(MCPOAuthHandler.storeStateMapping).toHaveBeenCalledWith(
'random-state-value',
'test-user-id:test-server',
mockFlowManager,
);
});
it('should return 403 when userId does not match authenticated user', async () => {
@ -254,6 +333,27 @@ describe('MCP Routes', () => {
expect(response.body).toEqual({ error: 'User mismatch' });
});
it('should return 403 when flowId does not match authenticated user and server', async () => {
const response = await request(app).get('/api/mcp/test-server/oauth/initiate').query({
userId: 'test-user-id',
flowId: 'other-user-id:test-server',
});
expect(response.status).toBe(403);
expect(response.body).toEqual({ error: 'Flow mismatch' });
expect(getLogStores).not.toHaveBeenCalled();
});
it('should return 403 when flowId query value is not a string', async () => {
const response = await request(app)
.get('/api/mcp/test-server/oauth/initiate')
.query('userId=test-user-id&flowId=test-user-id:test-server&flowId=other-flow');
expect(response.status).toBe(403);
expect(response.body).toEqual({ error: 'Flow mismatch' });
expect(getLogStores).not.toHaveBeenCalled();
});
it('should return 404 when flow state is not found', async () => {
const mockFlowManager = {
getFlowState: jest.fn().mockResolvedValue(null),
@ -264,7 +364,7 @@ describe('MCP Routes', () => {
const response = await request(app).get('/api/mcp/test-server/oauth/initiate').query({
userId: 'test-user-id',
flowId: 'non-existent-flow-id',
flowId: 'test-user-id:test-server',
});
expect(response.status).toBe(404);

59
api/server/routes/mcp.js
View file

@ -83,10 +83,21 @@ router.get('/:serverName/oauth/initiate', requireJwtAuth, setOAuthSession, async
const user = req.user;
// Verify the userId matches the authenticated user
if (userId !== user.id) {
if (typeof userId !== 'string' || userId !== user.id) {
return res.status(403).json({ error: 'User mismatch' });
}
const expectedFlowId = MCPOAuthHandler.generateFlowId(user.id, serverName);
if (typeof flowId !== 'string' || flowId !== expectedFlowId) {
logger.error('[MCP OAuth] Invalid flow ID for initiate request', {
serverName,
userId,
flowId,
expectedFlowId,
});
return res.status(403).json({ error: 'Flow mismatch' });
}
logger.debug('[MCP OAuth] Initiate request', { serverName, userId, flowId });
const flowsCache = getLogStores(CacheKeys.FLOWS);
@ -99,7 +110,45 @@ router.get('/:serverName/oauth/initiate', requireJwtAuth, setOAuthSession, async
return res.status(404).json({ error: 'Flow not found' });
}
const { serverUrl, oauth: oauthConfig } = flowState.metadata || {};
const {
authorizationUrl: storedAuthorizationUrl,
serverName: flowServerName,
userId: flowUserId,
serverUrl,
oauth: oauthConfig,
} = flowState.metadata || {};
if (flowUserId && flowUserId !== user.id) {
logger.error('[MCP OAuth] Flow user mismatch', { flowId, userId, flowUserId });
return res.status(403).json({ error: 'User mismatch' });
}
if (flowServerName && flowServerName !== serverName) {
logger.error('[MCP OAuth] Flow server mismatch', { flowId, serverName, flowServerName });
return res.status(400).json({ error: 'Invalid flow state' });
}
const pendingAge = flowState.createdAt ? Date.now() - flowState.createdAt : Infinity;
const isFreshPendingFlow = flowState.status === 'PENDING' && pendingAge < PENDING_STALE_MS;
if (!isFreshPendingFlow) {
logger.error('[MCP OAuth] Flow is not active for initiation', {
flowId,
status: flowState.status,
pendingAge,
});
return res.status(400).json({ error: 'Invalid flow state' });
}
if (typeof storedAuthorizationUrl === 'string' && storedAuthorizationUrl.length > 0) {
logger.debug('[MCP OAuth] Reusing stored authorization URL', {
serverName,
userId,
flowId,
});
setOAuthCsrfCookie(res, flowId, OAUTH_CSRF_COOKIE_PATH);
return res.redirect(storedAuthorizationUrl);
}
if (!serverUrl || !oauthConfig) {
logger.error('[MCP OAuth] Missing server URL or OAuth config in flow state');
return res.status(400).json({ error: 'Invalid flow state' });
@ -127,6 +176,12 @@ router.get('/:serverName/oauth/initiate', requireJwtAuth, setOAuthSession, async
logger.debug('[MCP OAuth] OAuth flow initiated', { oauthFlowId, authorizationUrl });
const oldState = flowState.metadata?.state;
if (typeof oldState === 'string') {
await MCPOAuthHandler.deleteStateMapping(oldState, flowManager);
}
const metadataWithUrl = { ...flowMetadata, authorizationUrl, tenantId: getTenantId() };
await flowManager.initFlow(oauthFlowId, 'mcp_oauth', metadataWithUrl);
await MCPOAuthHandler.storeStateMapping(flowMetadata.state, oauthFlowId, flowManager);
setOAuthCsrfCookie(res, oauthFlowId, OAUTH_CSRF_COOKIE_PATH);
res.redirect(authorizationUrl);