Skip to content

Commit f812911

Browse files
committed
fix(mship): return the chat connect to its own tab, on main's credential UI
Reverts #6403 and #6385 to restore the credential UI users had on main — individual inline chips, no combined card — and re-adds only the part that was worth keeping: the connect no longer strands you on a second copy of the chat. The chip opens the flow in a popup whose return leg is rewritten to a self-closing completion page, so this tab is never navigated. A refused popup falls through to the anchor pointed at the same URL, so that tab still closes itself rather than loading the app again. Nothing tracks per-chip attempt state, which is what the reverted work needed to drive the card's connected/pending labels. Main's chip is stateless, so the ledger, the status hook, the popup watcher and its deadlines all go with it — along with the races they kept producing. Keeps oauth/chat-complete out of the strict COOP rule: same-origin would disown the popup from its opener the moment it loads, leaving it not reliably script-closable, which is the one thing the page exists to do.
1 parent a463e3e commit f812911

7 files changed

Lines changed: 302 additions & 12 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
'use client'
2+
3+
import { useEffect } from 'react'
4+
import { OAUTH_POPUP_RETURN_TO_PARAM } from '@/lib/credentials/oauth-popup-return'
5+
6+
const CLOSE_FALLBACK_DELAY_MS = 400
7+
8+
/**
9+
* The fallback redirect must never leave this origin — the target rides in a
10+
* query param the user could have tampered with.
11+
*/
12+
function sanitizeReturnTo(raw: string | null): string | null {
13+
if (!raw) return null
14+
try {
15+
const url = new URL(raw, window.location.origin)
16+
return url.origin === window.location.origin ? url.toString() : null
17+
} catch {
18+
return null
19+
}
20+
}
21+
22+
/**
23+
* Behavior half of the chat OAuth return leg: closes the window it runs in.
24+
* Renders nothing, so the page's frame paints as server markup before this
25+
* hydrates.
26+
*
27+
* The credential has already landed server-side by the time this runs — the
28+
* page exists only so the flow stops somewhere disposable instead of loading a
29+
* second copy of the app over the tab the user started from.
30+
*
31+
* A window the browser refuses to close redirects on to where the flow began.
32+
* That is the popup-blocked path: the anchor's `target='_blank'` opens this leg
33+
* in a new tab, which no script may close.
34+
*/
35+
export function ChatCompleteHandoff() {
36+
useEffect(() => {
37+
const returnTo = sanitizeReturnTo(
38+
new URL(window.location.href).searchParams.get(OAUTH_POPUP_RETURN_TO_PARAM)
39+
)
40+
window.close()
41+
const timer = window.setTimeout(() => {
42+
window.location.replace(returnTo ?? '/workspace')
43+
}, CLOSE_FALLBACK_DELAY_MS)
44+
return () => window.clearTimeout(timer)
45+
}, [])
46+
47+
return null
48+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import type { Metadata } from 'next'
2+
import { DesktopHandoffShell } from '@/app/desktop/components/desktop-handoff-shell'
3+
import { ChatCompleteHandoff } from '@/app/oauth/chat-complete/chat-complete-handoff'
4+
5+
export const metadata: Metadata = {
6+
title: 'Returning to Sim',
7+
robots: { index: false },
8+
}
9+
10+
/**
11+
* Post-OAuth return leg for the chat credential chips. The chip rewrites the
12+
* authorize URL's return param to land here, so the OAuth window finishes on
13+
* this page — which closes itself — instead of loading a second copy of the app
14+
* over the chat the user started from.
15+
*
16+
* Visible for a few hundred milliseconds in a popup, or briefly in a tab when
17+
* the popup was blocked, so it wears the same handoff frame as the other
18+
* minimal-chrome gates rather than styling of its own.
19+
*/
20+
export default function ChatCompletePage() {
21+
return (
22+
<>
23+
<ChatCompleteHandoff />
24+
<DesktopHandoffShell title='Finishing the connection' description='Returning you to Sim.' />
25+
</>
26+
)
27+
}

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/**
22
* @vitest-environment jsdom
3+
* @vitest-environment-options { "url": "https://sim.test/workspace/workspace-1/chat/chat-1" }
34
*/
45
import { act } from 'react'
56
import { createRoot, type Root } from 'react-dom/client'
@@ -86,6 +87,77 @@ describe('CredentialDisplay link tag', () => {
8687
act(() => root.unmount())
8788
})
8889

90+
it('runs the connect in a popup so the chat tab is never navigated', () => {
91+
const popup = { focus: vi.fn() }
92+
const openSpy = vi
93+
.spyOn(window, 'open')
94+
.mockReturnValue(popup as unknown as ReturnType<typeof window.open>)
95+
const { container, root } = renderCredentialLink({
96+
type: 'link',
97+
provider: 'google-email',
98+
value:
99+
'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1',
100+
})
101+
102+
const link = container.querySelector('a')
103+
const defaultPrevented = !link?.dispatchEvent(
104+
new MouseEvent('click', { bubbles: true, cancelable: true })
105+
)
106+
107+
expect(defaultPrevented).toBe(true)
108+
expect(popup.focus).toHaveBeenCalledOnce()
109+
const callbackUrl = new URL(
110+
new URL(openSpy.mock.calls[0][0] as string).searchParams.get('callbackURL') ?? ''
111+
)
112+
expect(callbackUrl.pathname).toBe('/oauth/chat-complete')
113+
openSpy.mockRestore()
114+
act(() => root.unmount())
115+
})
116+
117+
it('falls back to the anchor, still via the completion page, when the popup is blocked', () => {
118+
const openSpy = vi.spyOn(window, 'open').mockReturnValue(null)
119+
const { container, root } = renderCredentialLink({
120+
type: 'link',
121+
provider: 'google-email',
122+
value:
123+
'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1',
124+
})
125+
126+
const link = container.querySelector('a')
127+
const defaultPrevented = !link?.dispatchEvent(
128+
new MouseEvent('click', { bubbles: true, cancelable: true })
129+
)
130+
131+
// The tab still opens, but on a page that closes itself instead of a second
132+
// copy of the chat.
133+
expect(defaultPrevented).toBe(false)
134+
const callbackUrl = new URL(
135+
new URL(link?.getAttribute('href') ?? '').searchParams.get('callbackURL') ?? ''
136+
)
137+
expect(callbackUrl.pathname).toBe('/oauth/chat-complete')
138+
openSpy.mockRestore()
139+
act(() => root.unmount())
140+
})
141+
142+
it('keeps a cross-origin connect URL on the anchor instead of a popup', () => {
143+
const openSpy = vi.spyOn(window, 'open')
144+
const { container, root } = renderCredentialLink({
145+
type: 'link',
146+
provider: 'google-email',
147+
value:
148+
'https://evil.example/api/auth/oauth2/authorize?callbackURL=https%3A%2F%2Fevil.example',
149+
})
150+
151+
const link = container.querySelector('a')
152+
link?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }))
153+
154+
// The anchor carries rel='noopener noreferrer'; window.open would not.
155+
expect(openSpy).not.toHaveBeenCalled()
156+
expect(link?.getAttribute('rel')).toBe('noopener noreferrer')
157+
openSpy.mockRestore()
158+
act(() => root.unmount())
159+
})
160+
89161
it('renders nothing when the user cannot edit, regardless of URL safety', () => {
90162
mockUseUserPermissionsContext.mockReturnValue({ canEdit: false })
91163
const { container, root } = renderCredentialLink({

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ import { canManageWorkspaceBilling } from '@/lib/billing/workspace-permissions'
2323
import { isBrowserAgentAvailable, sendBrowserPanelAction } from '@/lib/browser-agent/transport'
2424
import { isHosted } from '@/lib/core/config/env-flags'
2525
import { isSafeHttpUrl } from '@/lib/core/utils/urls'
26+
import {
27+
buildOAuthPopupAuthorizeUrl,
28+
OAUTH_POPUP_FEATURES,
29+
OAUTH_POPUP_WINDOW_NAME,
30+
} from '@/lib/credentials/oauth-popup-return'
2631
import { getDesktopBridge } from '@/lib/desktop'
2732
import { desktopChatScopeId } from '@/lib/desktop/chat-scope'
2833
import {
@@ -1869,16 +1874,33 @@ function CredentialLinkDisplay({ data }: { data: CredentialTagData }) {
18691874
* completion returns through the app's loopback and refreshes credentials.
18701875
*/
18711876
const handleClick = (event: React.MouseEvent<HTMLAnchorElement>) => {
1877+
if (!data.value) return
18721878
const bridge = getDesktopBridge()
1873-
if (!bridge?.beginOAuthConnect || !data.value) return
1879+
if (bridge?.beginOAuthConnect) {
1880+
event.preventDefault()
1881+
const url = new URL(data.value)
1882+
const providerId = url.searchParams.get('providerId') ?? data.provider
1883+
if (!providerId) return
1884+
void bridge.beginOAuthConnect(providerId, {
1885+
workspaceId: url.searchParams.get('workspaceId') ?? undefined,
1886+
credentialId: url.searchParams.get('credentialId') ?? undefined,
1887+
})
1888+
return
1889+
}
1890+
1891+
// Web: run the flow in a popup routed through the self-closing completion
1892+
// page, so this tab is never navigated and the user does not end up on a
1893+
// second copy of the chat they started from. A refused popup falls through
1894+
// to the anchor, pointed at the same URL so that tab still closes itself.
1895+
const popupUrl = buildOAuthPopupAuthorizeUrl(data.value)
1896+
if (!popupUrl) return
1897+
const popup = window.open(popupUrl, OAUTH_POPUP_WINDOW_NAME, OAUTH_POPUP_FEATURES)
1898+
if (!popup) {
1899+
event.currentTarget.href = popupUrl
1900+
return
1901+
}
18741902
event.preventDefault()
1875-
const url = new URL(data.value)
1876-
const providerId = url.searchParams.get('providerId') ?? data.provider
1877-
if (!providerId) return
1878-
void bridge.beginOAuthConnect(providerId, {
1879-
workspaceId: url.searchParams.get('workspaceId') ?? undefined,
1880-
credentialId: url.searchParams.get('credentialId') ?? undefined,
1881-
})
1903+
popup.focus?.()
18821904
}
18831905

18841906
return (
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* @vitest-environment jsdom
3+
* @vitest-environment-options { "url": "https://sim.test/workspace/workspace-1/chat/chat-1" }
4+
*/
5+
import { describe, expect, it } from 'vitest'
6+
import {
7+
buildOAuthPopupAuthorizeUrl,
8+
OAUTH_POPUP_COMPLETE_PATH,
9+
OAUTH_POPUP_RETURN_TO_PARAM,
10+
} from '@/lib/credentials/oauth-popup-return'
11+
12+
describe('buildOAuthPopupAuthorizeUrl', () => {
13+
it('routes the return through the completion page and keeps the original destination', () => {
14+
const result = buildOAuthPopupAuthorizeUrl(
15+
'https://sim.test/api/auth/oauth2/authorize?providerId=google-email&callbackURL=https%3A%2F%2Fsim.test%2Fworkspace%2Fworkspace-1%2Fchat%2Fchat-1'
16+
)
17+
18+
const callbackUrl = new URL(new URL(result as string).searchParams.get('callbackURL') ?? '')
19+
expect(callbackUrl.pathname).toBe(OAUTH_POPUP_COMPLETE_PATH)
20+
expect(callbackUrl.searchParams.get(OAUTH_POPUP_RETURN_TO_PARAM)).toBe(
21+
'https://sim.test/workspace/workspace-1/chat/chat-1'
22+
)
23+
})
24+
25+
it('rewrites returnUrl for the custom-provider authorize routes', () => {
26+
const result = buildOAuthPopupAuthorizeUrl(
27+
'https://sim.test/api/auth/trello/authorize?returnUrl=https%3A%2F%2Fsim.test%2Fworkspace%2Fws-1%2Fchat%2Fc-1'
28+
)
29+
30+
const returnUrl = new URL(new URL(result as string).searchParams.get('returnUrl') ?? '')
31+
expect(returnUrl.pathname).toBe(OAUTH_POPUP_COMPLETE_PATH)
32+
})
33+
34+
it('anchors the completion page on the server-generated return origin, not this tab', () => {
35+
// The authorize route accepts a return target only when it matches the
36+
// deployment's configured base URL, which a proxied origin need not equal.
37+
const result = buildOAuthPopupAuthorizeUrl(
38+
'https://sim.test/api/auth/oauth2/authorize?providerId=slack&callbackURL=https%3A%2F%2Fapp.sim.test%2Fworkspace%2Fws-1'
39+
)
40+
41+
const callbackUrl = new URL(new URL(result as string).searchParams.get('callbackURL') ?? '')
42+
expect(callbackUrl.origin).toBe('https://app.sim.test')
43+
})
44+
45+
it('refuses a cross-origin authorize URL so the caller keeps its noopener anchor', () => {
46+
expect(
47+
buildOAuthPopupAuthorizeUrl(
48+
'https://evil.example/api/auth/oauth2/authorize?callbackURL=https%3A%2F%2Fevil.example%2Fsink'
49+
)
50+
).toBeNull()
51+
})
52+
53+
it('refuses an authorize URL with no return param to rewrite', () => {
54+
expect(
55+
buildOAuthPopupAuthorizeUrl('https://sim.test/api/auth/oauth2/authorize?providerId=github')
56+
).toBeNull()
57+
})
58+
})
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/**
2+
* Routes a chat OAuth flow's return leg through the self-closing completion
3+
* page, so a connect started from chat comes back to the tab that started it
4+
* instead of leaving a second copy of the app open behind the provider.
5+
*/
6+
7+
export const OAUTH_POPUP_COMPLETE_PATH = '/oauth/chat-complete'
8+
export const OAUTH_POPUP_RETURN_TO_PARAM = 'returnTo'
9+
10+
/** Matches the MCP OAuth popup (`hooks/mcp/use-mcp-oauth-popup.ts`) so the two consent windows open alike. */
11+
export const OAUTH_POPUP_FEATURES = 'width=560,height=720,resizable=yes,scrollbars=yes'
12+
13+
/**
14+
* Shared across chips on purpose: a second connect reuses (and refocuses) the
15+
* one window rather than leaving an abandoned consent screen behind. Nothing
16+
* here tracks a per-chip attempt, so there is no result for a reused window to
17+
* strand.
18+
*/
19+
export const OAUTH_POPUP_WINDOW_NAME = 'sim-oauth-connect'
20+
21+
/**
22+
* Rewrites an authorize URL so its eventual return lands on the completion
23+
* page, carrying the original destination for the case where the window cannot
24+
* close itself.
25+
*
26+
* Returns null when the flow must stay on the plain anchor: a cross-origin
27+
* authorize URL (the value is streamed model output, checked only for a safe
28+
* protocol) would otherwise take the popup path, dropping the anchor's
29+
* `rel="noopener"` and handing a foreign page our window handle. An authorize
30+
* URL with no return param to rewrite has nowhere to put the completion page.
31+
*/
32+
export function buildOAuthPopupAuthorizeUrl(rawUrl: string): string | null {
33+
let authorizeUrl: URL
34+
try {
35+
authorizeUrl = new URL(rawUrl, window.location.origin)
36+
} catch {
37+
return null
38+
}
39+
if (authorizeUrl.origin !== window.location.origin) return null
40+
41+
const returnParam = authorizeUrl.searchParams.has('callbackURL') ? 'callbackURL' : 'returnUrl'
42+
const rawReturnUrl = authorizeUrl.searchParams.get(returnParam)
43+
if (!rawReturnUrl) return null
44+
45+
// Anchored on the return URL the server generated rather than this tab's
46+
// origin: both the authorize route and the custom-provider callbacks accept a
47+
// return target only when it matches the deployment's configured base URL,
48+
// which a proxied or aliased origin need not equal.
49+
const completeUrl = new URL(
50+
OAUTH_POPUP_COMPLETE_PATH,
51+
new URL(rawReturnUrl, window.location.origin).origin
52+
)
53+
completeUrl.searchParams.set(OAUTH_POPUP_RETURN_TO_PARAM, rawReturnUrl)
54+
authorizeUrl.searchParams.set(returnParam, completeUrl.toString())
55+
return authorizeUrl.toString()
56+
}

apps/sim/next.config.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,12 @@ const nextConfig: NextConfig = {
318318
},
319319
{
320320
// Exclude Vercel internal resources and static assets from strict COOP, Google Drive Picker
321-
// and the /demo Cal.com booking embed to prevent 'refused to connect' / slow-load issues
322-
source: '/((?!_next|_vercel|api|favicon.ico|w/.*|workspace/.*|api/tools/drive|demo).*)',
321+
// and the /demo Cal.com booking embed to prevent 'refused to connect' / slow-load issues.
322+
// `oauth/chat-complete` runs *as* a popup: `same-origin` moves a document into its own
323+
// browsing-context group, disowning it from the opener that launched it the moment it
324+
// loads — which leaves it not reliably script-closable, the one thing it exists to do.
325+
source:
326+
'/((?!_next|_vercel|api|favicon.ico|w/.*|workspace/.*|api/tools/drive|demo|oauth/chat-complete).*)',
323327
headers: [
324328
{
325329
key: 'Cross-Origin-Opener-Policy',
@@ -342,8 +346,11 @@ const nextConfig: NextConfig = {
342346
],
343347
},
344348
{
345-
// For main app routes, Google Drive Picker, the /demo Cal.com embed, and Vercel resources - use permissive policies
346-
source: '/(w/.*|workspace/.*|api/tools/drive|demo.*|_next/.*|_vercel/.*)',
349+
// For main app routes, Google Drive Picker, the /demo Cal.com embed, the chat OAuth popup
350+
// return leg, and Vercel resources - use permissive policies. The return leg matches its
351+
// opener's value so the two stay in one browsing-context group.
352+
source:
353+
'/(w/.*|workspace/.*|api/tools/drive|demo.*|oauth/chat-complete|_next/.*|_vercel/.*)',
347354
headers: [
348355
{
349356
key: 'Cross-Origin-Embedder-Policy',

0 commit comments

Comments
 (0)