Skip to content

Commit 4c57c51

Browse files
waleedlatif1waleedlatif1
committed
fix(mcp): react to the OAuth broadcast only in the tab that opened the popup
A BroadcastChannel reaches every same-origin tab, so the previous workspace-id scoping (which the success path carried but failure paths omitted) still let unrelated tabs clear state, refetch, and show spurious toasts. Gate every reaction on whether this tab actually has an in-flight popup for the result's server (`popupIntervalsRef`) — strictly more precise than workspace scoping and correct for both cross-workspace and same-workspace-second-tab cases. Removes the now-redundant workspaceId from the callback message.
1 parent 16852a8 commit 4c57c51

4 files changed

Lines changed: 16 additions & 22 deletions

File tree

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,26 +73,24 @@ describe('MCP OAuth callback route', () => {
7373
)
7474
})
7575

76-
it('signals success over a same-origin BroadcastChannel scoped to the server and workspace', async () => {
76+
it('signals success over a same-origin BroadcastChannel carrying the server id', async () => {
7777
const request = new NextRequest(
7878
'http://localhost:3000/api/mcp/oauth/callback?state=state-1&code=auth-code-1'
7979
)
8080

8181
const body = await (await GET(request)).text()
8282

8383
// 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.
84+
// so a COOP `same-origin` provider that severs the opener can't strand the parent. The
85+
// server id lets the hook react only in the tab that opened this flow's popup.
8586
expect(body).toContain("new BroadcastChannel('mcp-oauth')")
8687
expect(body).toContain('ok: true')
8788
expect(body).toContain('"server-1"')
88-
expect(body).toContain('"workspace-1"')
8989
})
9090

9191
it('reports an early failure over the channel without attempting token exchange', async () => {
9292
// 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-
)
93+
const request = new NextRequest('http://localhost:3000/api/mcp/oauth/callback?state=state-1')
9694

9795
const body = await (await GET(request)).text()
9896

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

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ function htmlClose(
4343
message: string,
4444
ok: boolean,
4545
reason: McpOauthCallbackReason,
46-
serverId?: string,
47-
workspaceId?: string
46+
serverId?: string
4847
): NextResponse {
4948
if (!ok) {
5049
logger.warn(
@@ -57,9 +56,9 @@ function htmlClose(
5756
// `window.opener.postMessage`: a provider whose authorize page sets COOP
5857
// `same-origin` severs `window.opener`, which would silently drop the result and
5958
// leave the parent stuck on "Connecting…". A BroadcastChannel is origin-scoped and
60-
// unaffected by opener severance.
59+
// unaffected by opener severance; the hook ignores results for popups it did not open.
6160
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>
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) {}
61+
try { var ch = new BroadcastChannel('mcp-oauth'); ch.postMessage({ type: 'mcp-oauth', ok: ${ok ? 'true' : 'false'}, serverId: ${jsonLiteral(serverId)}, reason: ${jsonLiteral(reason)} }); ch.close() } catch (e) {}
6362
setTimeout(function () { window.close() }, 800)
6463
</script></body></html>`
6564
return new NextResponse(body, {
@@ -184,13 +183,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
184183
logger.warn('Post-auth tools refresh failed', toError(e).message)
185184
}
186185

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

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,14 @@ export function useMcpOauthPopup({ workspaceId }: UseMcpOauthPopupProps) {
6161
channel.onmessage = (event) => {
6262
const data = event.data as Partial<McpOauthCallbackMessage> | null
6363
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
64+
// A BroadcastChannel reaches every same-origin tab, so only the tab that actually
65+
// opened this flow's popup should react — otherwise an unrelated tab (any workspace)
66+
// would clear state, refetch, and show a spurious toast. `popupIntervalsRef` holds
67+
// this tab's in-flight popups; a result for a server it never opened is not ours.
68+
const initiatedHere = data.serverId
69+
? popupIntervalsRef.current.has(data.serverId)
70+
: popupIntervalsRef.current.size > 0
71+
if (!initiatedHere) return
6772
if (data.serverId) {
6873
const serverId = data.serverId
6974
const interval = popupIntervalsRef.current.get(serverId)

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,5 @@ export interface McpOauthCallbackMessage {
2222
type: 'mcp-oauth'
2323
ok: boolean
2424
serverId?: string
25-
/** Scopes the broadcast so other open workspaces ignore it. Absent on early failures. */
26-
workspaceId?: string
2725
reason?: McpOauthCallbackReason
2826
}

0 commit comments

Comments
 (0)