Skip to content

Commit 16852a8

Browse files
waleedlatif1waleedlatif1
committed
fix(mcp): deliver OAuth callback result over BroadcastChannel so COOP can't strand the connect
An MCP provider whose authorization page sets `Cross-Origin-Opener-Policy: same-origin` (e.g. Gauge) severs `window.opener` when the popup navigates through it, so the callback's `window.opener.postMessage` was silently lost and the row hung on "Connecting…" forever — even when the callback succeeded. - Signal completion over a same-origin `BroadcastChannel` instead, which is origin-scoped and immune to opener severance (the MDN/Chrome-recommended COOP workaround). Scope the message by workspaceId so other open workspaces ignore it. - Log every OAuth callback failure with its reason + serverId. The early-return gates previously returned a silent `ok:false` popup close, so a failed authorization was undiagnosable from the server logs.
1 parent e337308 commit 16852a8

5 files changed

Lines changed: 66 additions & 11 deletions

File tree

‎apps/sim/app/api/mcp/oauth/callback/route.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,31 @@ describe('MCP OAuth callback route', () => {
7272
})
7373
)
7474
})
75+
76+
it('signals success over a same-origin BroadcastChannel scoped to the server and workspace', async () => {
77+
const request = new NextRequest(
78+
'http://localhost:3000/api/mcp/oauth/callback?state=state-1&code=auth-code-1'
79+
)
80+
81+
const body = await (await GET(request)).text()
82+
83+
// The completion is delivered over a BroadcastChannel (not window.opener.postMessage)
84+
// so a COOP `same-origin` provider that severs the opener can't strand the parent.
85+
expect(body).toContain("new BroadcastChannel('mcp-oauth')")
86+
expect(body).toContain('ok: true')
87+
expect(body).toContain('"server-1"')
88+
expect(body).toContain('"workspace-1"')
89+
})
90+
91+
it('reports an early failure over the channel without attempting token exchange', async () => {
92+
// Missing `code` fails at the param gate, before any network work.
93+
const request = new NextRequest(
94+
'http://localhost:3000/api/mcp/oauth/callback?state=state-1'
95+
)
96+
97+
const body = await (await GET(request)).text()
98+
99+
expect(body).toContain('ok: false')
100+
expect(mcpOauthMockFns.mockMcpAuthGuarded).not.toHaveBeenCalled()
101+
})
75102
})

‎apps/sim/app/api/mcp/oauth/callback/route.ts‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,23 @@ function htmlClose(
4343
message: string,
4444
ok: boolean,
4545
reason: McpOauthCallbackReason,
46-
serverId?: string
46+
serverId?: string,
47+
workspaceId?: string
4748
): NextResponse {
49+
if (!ok) {
50+
logger.warn(
51+
`MCP OAuth callback did not complete: ${reason}${serverId ? ` (server ${serverId})` : ''}`
52+
)
53+
}
4854
const safeMessage = escapeHtml(message)
4955
const title = ok ? 'Connected' : 'Connection failed'
56+
// Signal the opener over a same-origin BroadcastChannel rather than
57+
// `window.opener.postMessage`: a provider whose authorize page sets COOP
58+
// `same-origin` severs `window.opener`, which would silently drop the result and
59+
// leave the parent stuck on "Connecting…". A BroadcastChannel is origin-scoped and
60+
// unaffected by opener severance.
5061
const body = `<!doctype html><html><head><meta charset="utf-8"><title>${title}</title></head><body style="font-family: system-ui; padding: 24px"><p>${safeMessage}</p><script>
51-
try { window.opener && window.opener.postMessage({ type: 'mcp-oauth', ok: ${ok ? 'true' : 'false'}, serverId: ${jsonLiteral(serverId)}, reason: ${jsonLiteral(reason)} }, window.location.origin) } catch (e) {}
62+
try { var ch = new BroadcastChannel('mcp-oauth'); ch.postMessage({ type: 'mcp-oauth', ok: ${ok ? 'true' : 'false'}, serverId: ${jsonLiteral(serverId)}, workspaceId: ${jsonLiteral(workspaceId)}, reason: ${jsonLiteral(reason)} }); ch.close() } catch (e) {}
5263
setTimeout(function () { window.close() }, 800)
5364
</script></body></html>`
5465
return new NextResponse(body, {
@@ -173,7 +184,13 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
173184
logger.warn('Post-auth tools refresh failed', toError(e).message)
174185
}
175186

176-
return htmlClose('Connected. You can close this window.', true, 'authorized', server.id)
187+
return htmlClose(
188+
'Connected. You can close this window.',
189+
true,
190+
'authorized',
191+
server.id,
192+
server.workspaceId
193+
)
177194
} catch (error) {
178195
logger.error('MCP OAuth callback failed', error)
179196
return htmlClose('Authorization failed. Please try again.', false, 'unknown', serverId)

‎apps/sim/hooks/mcp/use-mcp-oauth-popup.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,17 @@ export function useMcpOauthPopup({ workspaceId }: UseMcpOauthPopupProps) {
5353
}, [])
5454

5555
useEffect(() => {
56-
function onMessage(event: MessageEvent) {
57-
if (event.origin !== window.location.origin) return
56+
// The callback signals over a same-origin BroadcastChannel (see the OAuth callback
57+
// route): a provider whose authorize page sets COOP `same-origin` severs
58+
// `window.opener`, so a popup `postMessage` can be lost and leave the row stuck on
59+
// "Connecting…". A BroadcastChannel is origin-scoped, so it needs no origin check.
60+
const channel = new BroadcastChannel('mcp-oauth')
61+
channel.onmessage = (event) => {
5862
const data = event.data as Partial<McpOauthCallbackMessage> | null
5963
if (data?.type !== 'mcp-oauth') return
64+
// Ignore results for a different workspace open in another tab. Early failures
65+
// carry no workspaceId and clear this tab's in-flight popups regardless.
66+
if (data.workspaceId && data.workspaceId !== workspaceId) return
6067
if (data.serverId) {
6168
const serverId = data.serverId
6269
const interval = popupIntervalsRef.current.get(serverId)
@@ -95,8 +102,7 @@ export function useMcpOauthPopup({ workspaceId }: UseMcpOauthPopupProps) {
95102
toast.error(reasonToMessage(data.reason))
96103
}
97104
}
98-
window.addEventListener('message', onMessage)
99-
return () => window.removeEventListener('message', onMessage)
105+
return () => channel.close()
100106
}, [queryClient, workspaceId])
101107

102108
const startOauthForServer = useCallback(

‎apps/sim/hooks/queries/mcp.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ export function useCreateMcpServer() {
304304
})
305305
}
306306

307-
/** On `redirect`, the caller must wait for `popup.closed` or the `mcp-oauth` postMessage. */
307+
/** On `redirect`, the caller must wait for `popup.closed` or the `mcp-oauth` BroadcastChannel signal. */
308308
export type StartMcpOauthMutationResult =
309309
| { status: 'redirect'; popup: Window }
310310
| { status: 'already_authorized' }

‎apps/sim/lib/mcp/oauth/callback-reasons.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
/**
2-
* Reasons surfaced from the OAuth callback popup back to the parent window via
3-
* `window.opener.postMessage`. Consumed by the popup hook to render user-facing
4-
* status messages and by the callback route to discriminate failure modes.
2+
* Reasons surfaced from the OAuth callback popup back to the parent window over a
3+
* same-origin `BroadcastChannel`. A provider whose authorization page sets
4+
* `Cross-Origin-Opener-Policy: same-origin` severs `window.opener`, so a targeted
5+
* `postMessage` from the popup can be lost; a BroadcastChannel is origin-scoped and
6+
* unaffected. Consumed by the popup hook to render user-facing status messages and
7+
* by the callback route to discriminate failure modes.
58
*/
69
export type McpOauthCallbackReason =
710
| 'authorized'
@@ -19,5 +22,7 @@ export interface McpOauthCallbackMessage {
1922
type: 'mcp-oauth'
2023
ok: boolean
2124
serverId?: string
25+
/** Scopes the broadcast so other open workspaces ignore it. Absent on early failures. */
26+
workspaceId?: string
2227
reason?: McpOauthCallbackReason
2328
}

0 commit comments

Comments
 (0)