From d17ff65248e24968ebb4cef885f203ae039eba10 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 1 Oct 2026 17:23:03 -0700 Subject: [PATCH] fix(accounts): report managed OAuth failures accurately --- .../app/api/mcp/oauth/callback/route.test.ts | 136 +++++++++++++++--- apps/sim/app/api/mcp/oauth/callback/route.ts | 33 ++++- apps/sim/lib/credential-groups/oauth-popup.ts | 21 +++ 3 files changed, 166 insertions(+), 24 deletions(-) diff --git a/apps/sim/app/api/mcp/oauth/callback/route.test.ts b/apps/sim/app/api/mcp/oauth/callback/route.test.ts index d9b38f12508..28fdcff930f 100644 --- a/apps/sim/app/api/mcp/oauth/callback/route.test.ts +++ b/apps/sim/app/api/mcp/oauth/callback/route.test.ts @@ -1,3 +1,5 @@ +import { runInNewContext } from 'node:vm' +import { BroadcastChannel } from 'node:worker_threads' import { authMockFns, dbChainMockFns, @@ -5,10 +7,14 @@ import { mcpOauthMockFns, resetDbChainMock, } from '@sim/testing' +import { flushMicrotasks } from '@sim/testing/helpers/async' +import { emcnMock } from '@sim/testing/mocks/emcn.mock' +import { getMockLogger } from '@sim/testing/mocks/logger.mock' import { mcpServiceMock, mcpServiceMockFns } from '@sim/testing/mocks/mcp-service.mock' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' import { CredentialGroupOAuthStateVersionError } from '@/lib/credential-groups/oauth-attempt-version' +import { connectCredentialGroupInPopup } from '@/lib/credential-groups/oauth-popup' const { mockAuthenticateEnrollment, @@ -24,6 +30,7 @@ const { vi.mock('@/lib/mcp/oauth', () => mcpOauthMock) vi.mock('@/lib/mcp/service', () => mcpServiceMock) +vi.mock('@sim/emcn', () => emcnMock) vi.mock('@/lib/credential-groups/application/enrollment-auth', () => ({ credentialGroupOAuthAttemptPrincipal: mockAuthenticateEnrollment, })) @@ -88,30 +95,41 @@ describe('MCP OAuth callback route', () => { mockEnforceCallbackRateLimit.mockResolvedValue(null) }) - it.each([undefined, 'denied'])( - 'finishes a direct connection without the invitation form: %s', - async (error) => { - const completionId = '00000000-0000-4000-8000-000000000002' - mockConsumeManagedAttempt.mockResolvedValueOnce({ - state: 'mcp_cg_direct', - organizationId: 'organization-1', - invitationToken: 'invitation-token', - mcpServerId: 'server-1', - completionId, - returnTo: 'integrations', - }) - const response = await GET( - new NextRequest( - `http://localhost:3000/api/mcp/oauth/callback?state=mcp_cg_direct&${error ? 'error=denied' : 'code=code-1'}` - ) + it.each([ + [undefined, null], + ['access_denied', 'denied'], + ['invalid_request', 'failed'], + ['invalid_scope', 'failed'], + ['server_error', 'provider_unavailable'], + ['temporarily_unavailable', 'provider_unavailable'], + ['unexpected-secret-value', 'failed'], + ])('finishes a direct connection without the invitation form: %s', async (error, expected) => { + const completionId = '00000000-0000-4000-8000-000000000002' + mockConsumeManagedAttempt.mockResolvedValueOnce({ + state: 'mcp_cg_direct', + organizationId: 'organization-1', + invitationToken: 'invitation-token', + mcpServerId: 'server-1', + completionId, + returnTo: 'integrations', + }) + const response = await GET( + new NextRequest( + `http://localhost:3000/api/mcp/oauth/callback?state=mcp_cg_direct&${error ? `error=${error}&error_description=private-provider-detail` : 'code=code-1'}` ) - const destination = new URL(response.headers.get('location')!, 'http://localhost:3000') - expect(destination.pathname).toBe('/credential-groups/complete') - expect(destination.searchParams.get('completionId')).toBe(completionId) - expect(destination.searchParams.get('organizationId')).toBe('organization-1') - expect(destination.searchParams.get('oauth')).toBe(error ?? null) + ) + const destination = new URL(response.headers.get('location')!, 'http://localhost:3000') + expect(destination.pathname).toBe('/credential-groups/complete') + expect(destination.searchParams.get('completionId')).toBe(completionId) + expect(destination.searchParams.get('organizationId')).toBe('organization-1') + expect(destination.searchParams.get('oauth')).toBe(expected) + if (error) { + const warnings = JSON.stringify(getMockLogger('McpOauthCallbackAPI').warn.mock.calls) + expect(warnings).toContain(error === 'unexpected-secret-value' ? 'unknown' : error) + expect(warnings).not.toContain('unexpected-secret-value') + expect(warnings).not.toContain('private-provider-detail') } - ) + }) it('performs the token exchange through the SSRF-guarded mcpAuthGuarded wrapper', async () => { const request = new NextRequest( @@ -185,4 +203,78 @@ describe('MCP OAuth callback route', () => { expect(mockAuthenticateEnrollment).not.toHaveBeenCalled() expect(mockCompleteManagedMcpOAuth).not.toHaveBeenCalled() }) + + it.each(['missing', 'version'])( + 'delivers a %s managed-state failure to only the initiating popup', + async (kind) => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + vi.stubGlobal('BroadcastChannel', BroadcastChannel) + vi.stubGlobal('window', { + open: () => ({ closed: false, opener: null, location: { replace() {} }, close() {} }), + }) + const controller = new AbortController() + const completionId = '00000000-0000-4000-8000-000000000010' + const secondId = '00000000-0000-4000-8000-000000000011' + const state = 'mcp_cg_first' + let first: string | undefined + let second: string | undefined + const start = (nonce: string) => async () => ({ + invitationLink: 'http://localhost:3000/credential-groups/enroll/fixture', + authorizationUrl: `https://oauth.example.com/authorize?state=${nonce}`, + }) + const outcomes = [ + connectCredentialGroupInPopup(completionId, start(state), controller.signal).then( + () => { + first = 'connected' + }, + (error: Error) => { + first = error.message + } + ), + connectCredentialGroupInPopup(secondId, start('mcp_cg_second'), controller.signal).then( + () => { + second = 'connected' + }, + (error: Error) => { + second = error.message + } + ), + ] + const observer = new BroadcastChannel('mcp-oauth') + const success = new BroadcastChannel(`sim:credential-group-oauth:${secondId}`) + try { + await flushMicrotasks() + if (kind === 'missing') mockConsumeManagedAttempt.mockResolvedValueOnce(null) + else + mockConsumeManagedAttempt.mockRejectedValueOnce( + new CredentialGroupOAuthStateVersionError() + ) + const response = await GET( + new NextRequest(`http://localhost:3000/api/mcp/oauth/callback?state=${state}&code=unused`) + ) + const body = await response.text() + const script = body.match(/