Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 116 additions & 8 deletions api/server/routes/__tests__/mcp.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,55 @@ describe('MCP Routes', () => {
});
});

describe('POST /:serverName/oauth/bind', () => {
it.each([
{ name: 'absent', cookies: [], refresh: true },
{ name: 'invalid', cookies: ['oauth_session=invalid'], refresh: true },
{
name: 'UTF-8 byte-length mismatch',
cookies: [`oauth_session=${encodeURIComponent('é'.repeat(32))}`],
refresh: true,
},
{
name: 'non-string JSON',
cookies: ['oauth_session=j:{"userId":"test-user-id"}'],
refresh: true,
},
{
name: 'previous account',
cookies: [`oauth_session=${generateTestCsrfToken('previous-user-id')}`],
refresh: true,
},
{
name: 'current account',
cookies: [`oauth_session=${generateTestCsrfToken('test-user-id')}`],
refresh: false,
},
])(
'handles a $name session binding before issuing the flow cookie',
async ({ cookies, refresh }) => {
const response = await request(app)
.post('/api/mcp/test-server/oauth/bind')
.set('Cookie', cookies)
.expect(200);
const issuedCookies = response.headers['set-cookie'];
expect(issuedCookies).toEqual(
expect.arrayContaining([
expect.stringContaining(
`oauth_csrf=${generateTestCsrfToken('test-user-id:test-server')}`,
),
]),
);
const sessionCookie = issuedCookies.find((cookie) => cookie.startsWith('oauth_session='));
if (refresh) {
expect(sessionCookie).toContain(`oauth_session=${generateTestCsrfToken('test-user-id')}`);
} else {
expect(sessionCookie).toBeUndefined();
}
},
);
});

describe('GET /:serverName/oauth/callback', () => {
const { MCPOAuthHandler, MCPTokenStorage } = require('@librechat/api');
const { getLogStores } = require('~/cache');
Expand Down Expand Up @@ -921,7 +970,7 @@ describe('MCP Routes', () => {
expect(mockFlowManager.completeFlow).not.toHaveBeenCalled();
});

it('should let the current tab complete after a stale callback burned the CSRF cookie', async () => {
it('refreshes a previous account binding and recovers after a stale callback consumes CSRF', async () => {
const flowId = 'test-user-id:test-server';
const mockFlowManager = {
getFlowState: jest.fn().mockResolvedValue({
Expand Down Expand Up @@ -955,20 +1004,29 @@ describe('MCP Routes', () => {
});
require('~/server/services/Config/mcp').updateMCPServerTools.mockResolvedValue();

const browser = request.agent(app);
const binding = await browser
.post('/api/mcp/test-server/oauth/bind')
.set('Cookie', [`oauth_session=${generateTestCsrfToken('previous-user-id')}`])
.expect(200);
expect(binding.headers['set-cookie']).toEqual(
expect.arrayContaining([
expect.stringContaining(`oauth_session=${generateTestCsrfToken('test-user-id')}`),
]),
);

MCPOAuthHandler.resolveStateToFlowId.mockResolvedValueOnce(flowId);
const csrfToken = generateTestCsrfToken(flowId);
const staleResponse = await request(app)
const staleResponse = await browser
.get('/api/mcp/test-server/oauth/callback')
.set('Cookie', [`oauth_csrf=${csrfToken}`])
.query({ code: 'stale-auth-code', state: 'superseded-attempt-state' });

const basePath = getBasePath();
expect(staleResponse.status).toBe(302);
expect(staleResponse.headers.location).toBe(`${basePath}/oauth/error?error=invalid_state`);
expect(MCPOAuthHandler.completeOAuthFlow).not.toHaveBeenCalled();

/** The stale callback consumed the CSRF cookie; the current tab recovers via the fresh PENDING flow */
const legitResponse = await request(app)
/** The stale callback consumed the CSRF cookie; the initiating browser retains its session binding. */
const legitResponse = await browser
.get('/api/mcp/test-server/oauth/callback')
.query({ code: 'current-auth-code', state: flowId });

Expand All @@ -977,8 +1035,54 @@ describe('MCP Routes', () => {
expect(MCPOAuthHandler.completeOAuthFlow).toHaveBeenCalledTimes(1);
});

describe('CSRF fallback via active PENDING flow', () => {
it('should proceed when a fresh PENDING flow exists and no cookies are present', async () => {
describe('browser binding for active PENDING flows', () => {
it.each([
{ cookies: [] },
{ cookies: ['oauth_csrf=invalid'] },
{ cookies: [`oauth_csrf=${encodeURIComponent('é'.repeat(32))}`] },
{ cookies: [`oauth_session=${encodeURIComponent('é'.repeat(32))}`] },
{ cookies: ['oauth_csrf=j:{"flowId":"test-user-id:test-server"}'] },
{ cookies: ['oauth_session=j:{"userId":"test-user-id"}'] },
{ cookies: [`oauth_session=${generateTestCsrfToken('victim-user-id')}`] },
{
cookies: [
'oauth_csrf=invalid',
`oauth_session=${generateTestCsrfToken('victim-user-id')}`,
],
},
])(
'rejects an unbound pending flow with %j before exchange or mutation',
async ({ cookies }) => {
const flowId = 'test-user-id:test-server';
const mockFlowManager = {
getFlowState: jest.fn().mockResolvedValue({
status: 'PENDING',
createdAt: Date.now(),
metadata: { state: flowId, userId: 'test-user-id', codeVerifier: 'test-verifier' },
}),
completeFlow: jest.fn(),
failFlow: jest.fn(),
deleteFlow: jest.fn(),
};
require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager);
const response = await request(app)
.get('/api/mcp/test-server/oauth/callback')
.set('Cookie', cookies)
.query({ code: 'victim-auth-code', state: flowId });

expect(response.headers.location).toBe(
`${getBasePath()}/oauth/error?error=csrf_validation_failed`,
);
expect(MCPOAuthHandler.completeOAuthFlow).not.toHaveBeenCalled();
expect(MCPTokenStorage.storeTokens).not.toHaveBeenCalled();
expect(mockFlowManager.getFlowState).not.toHaveBeenCalled();
expect(mockFlowManager.completeFlow).not.toHaveBeenCalled();
expect(mockFlowManager.failFlow).not.toHaveBeenCalled();
expect(mockFlowManager.deleteFlow).not.toHaveBeenCalled();
},
);

it('should proceed when a fresh PENDING flow has a valid session binding', async () => {
const flowId = 'test-user-id:test-server';
const pendingCreatedAt = Date.now();
const mockFlowManager = {
Expand Down Expand Up @@ -1027,6 +1131,7 @@ describe('MCP Routes', () => {

const response = await request(app)
.get('/api/mcp/test-server/oauth/callback')
.set('Cookie', [`oauth_session=${generateTestCsrfToken('test-user-id')}`])
.query({ code: 'test-code', state: flowId });

const basePath = getBasePath();
Expand Down Expand Up @@ -1099,6 +1204,7 @@ describe('MCP Routes', () => {

const response = await request(app)
.get('/api/mcp/test-server/oauth/callback')
.set('Cookie', [`oauth_csrf=${generateTestCsrfToken(flowId)}`])
.query({ code: 'test-code', state: flowId });

expect(response.status).toBe(302);
Expand Down Expand Up @@ -1166,6 +1272,7 @@ describe('MCP Routes', () => {

const response = await request(app)
.get('/api/mcp/test-server/oauth/callback')
.set('Cookie', [`oauth_csrf=${generateTestCsrfToken(flowId)}`])
.query({ code: 'test-code', state: flowId });

expect(response.status).toBe(302);
Expand Down Expand Up @@ -1229,6 +1336,7 @@ describe('MCP Routes', () => {

const response = await request(app)
.get('/api/mcp/test-server/oauth/callback')
.set('Cookie', [`oauth_csrf=${generateTestCsrfToken(flowId)}`])
.query({ code: 'test-code', state: flowId });

expect(response.status).toBe(302);
Expand Down
29 changes: 6 additions & 23 deletions api/server/routes/mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,30 +352,13 @@ router.get('/:serverName/oauth/callback', async (req, res) => {

const hasCsrf = validateOAuthCsrf(req, res, flowId, OAUTH_CSRF_COOKIE_PATH);
const hasSession = !hasCsrf && validateOAuthSession(req, parsedFlowId.userId);
let hasActiveFlow = false;
if (!hasCsrf && !hasSession) {
const pendingFlow = await flowManager.getFlowState(flowId, 'mcp_oauth');
const pendingAge = pendingFlow?.createdAt ? Date.now() - pendingFlow.createdAt : Infinity;
hasActiveFlow = pendingFlow?.status === 'PENDING' && pendingAge < PENDING_STALE_MS;
if (hasActiveFlow) {
logger.debug(
'[MCP OAuth] CSRF/session cookies absent, validating via active PENDING flow',
{
flowId,
},
);
}
}

if (!hasCsrf && !hasSession && !hasActiveFlow) {
logger.error(
'[MCP OAuth] CSRF validation failed: no valid CSRF cookie, session cookie, or active flow',
{
flowId,
hasCsrfCookie: !!req.cookies?.[OAUTH_CSRF_COOKIE],
hasSessionCookie: !!req.cookies?.[OAUTH_SESSION_COOKIE],
},
);
if (!hasCsrf && !hasSession) {
logger.error('[MCP OAuth] CSRF validation failed: no valid CSRF or session cookie', {
flowId,
hasCsrfCookie: !!req.cookies?.[OAUTH_CSRF_COOKIE],
hasSessionCookie: !!req.cookies?.[OAUTH_SESSION_COOKIE],
});
return res.redirect(`${basePath}/oauth/error?error=csrf_validation_failed`);
}

Expand Down
31 changes: 30 additions & 1 deletion e2e/specs/mock/mcp-oauth-resume.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ type GenerationStatus = {
};

test.describe('MCP OAuth stream resume', () => {
test('restores one actionable OAuth prompt after reloading the stream', async ({ page }) => {
test('restores one browser-bound OAuth prompt after rejecting an unbound callback', async ({
page,
browser,
}) => {
test.setTimeout(120000);
await page.goto(NEW_CHAT_PATH, { timeout: 10000 });

Expand Down Expand Up @@ -78,6 +81,7 @@ test.describe('MCP OAuth stream resume', () => {
const signIn = page.getByRole('button', { name: SIGN_IN_BUTTON });
await expect(signIn).toBeVisible({ timeout: 30000 });
await expect(signIn).toHaveCount(1);
await expect(signIn).toBeEnabled();

const status = await fetchJson<GenerationStatus>(
page,
Expand All @@ -93,6 +97,31 @@ test.describe('MCP OAuth stream resume', () => {
}),
]);

const authorizationUrl = new URL(status.resumeState!.pendingOAuthPrompts![0].authURL);
const callbackUrl = new URL(authorizationUrl.searchParams.get('redirect_uri')!);
callbackUrl.searchParams.set('state', authorizationUrl.searchParams.get('state')!);
callbackUrl.searchParams.set('code', 'unbound-provider-code');
const unboundBrowser = await browser.newContext();
try {
const callback = await unboundBrowser.request.get(callbackUrl.toString(), {
maxRedirects: 0,
});
expect(callback.status()).toBe(302);
expect(callback.headers().location).toContain('/oauth/error?error=csrf_validation_failed');
} finally {
await unboundBrowser.close();
}

const afterUnboundCallback = await fetchJson<GenerationStatus>(
page,
`/api/agents/chat/status/${encodeURIComponent(conversationId)}`,
token,
);
expect(afterUnboundCallback.active).toBe(true);
expect(afterUnboundCallback.resumeState?.pendingOAuthPrompts).toEqual(
status.resumeState?.pendingOAuthPrompts,
);

const resumeRequest = page.waitForRequest(
(request) => {
const url = new URL(request.url());
Expand Down
Loading
Loading