Skip to content

Commit e625525

Browse files
committed
fix(mship): return the chat connect flow to the tab that started it
Connecting an integration from a chat credential chip opened OAuth in a new tab and returned there, so the user landed on a second copy of the app while the conversation they started from sat stale behind it. The flow now runs in a popup and returns through a new self-closing page at /oauth/chat-complete, which publishes its verdict to the shared attempt record and closes. The chat tab picks that up over its storage listener and updates in place, so it never navigates. A blocked popup takes the same route in a new tab and still lands on the completion page, so both paths share one verdict source. That verdict is now the server's: reaching the completion page means Better Auth routed the flow to its success callback. The previous check diffed the workspace credential list, which reported failure whenever a user re-authorized an account they had already linked -- that path updates the account row and creates no new credential. The lock is stricter than the label. A failure to create the credential from its draft is swallowed server-side, so a flow can report success with nothing in the workspace; the row stays retryable unless the credential actually appears. Also: the popup is named per attempt so sibling rows cannot renavigate each other's window; a cross-origin connect URL keeps the anchor's noopener instead of taking the popup path; the focus verifier reads the attempt after its refetch so a verdict published mid-flight is not overwritten; and the verifier treats a popup parked on a terminal page (/oauth-error, the workspace error exit) as finished rather than waiting on it forever.
1 parent cb8338c commit e625525

7 files changed

Lines changed: 691 additions & 12 deletions

File tree

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
7+
import {
8+
createOAuthChatAttempt,
9+
type OAuthChatAttempt,
10+
readOAuthChatAttempt,
11+
} from '@/lib/credentials/oauth-chat-attempt'
12+
import { ChatCompleteHandoff } from '@/app/oauth/chat-complete/chat-complete-handoff'
13+
14+
function renderAt(search: string): { root: Root } {
15+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
16+
window.history.replaceState({}, '', `/oauth/chat-complete${search}`)
17+
const root: Root = createRoot(document.createElement('div'))
18+
act(() => root.render(<ChatCompleteHandoff />))
19+
return { root }
20+
}
21+
22+
describe('ChatCompleteHandoff', () => {
23+
let attempt: OAuthChatAttempt
24+
25+
beforeEach(() => {
26+
vi.clearAllMocks()
27+
window.localStorage.clear()
28+
vi.spyOn(window, 'close').mockImplementation(() => {})
29+
attempt = createOAuthChatAttempt({
30+
workspaceId: 'workspace-1',
31+
providerId: 'google-email',
32+
baseProviderId: 'google',
33+
displayName: 'Gmail',
34+
controlId: 'message-1:0:0',
35+
baselineCredentialIds: ['existing-gmail'],
36+
})
37+
})
38+
39+
it('publishes success on arrival, without requiring a new credential to appear', () => {
40+
// Re-authorizing an already-linked account updates the account row rather
41+
// than creating one, so no new credential lands — reaching this page is
42+
// still the server telling us the flow succeeded.
43+
const { root } = renderAt(`?oauthAttempt=${attempt.id}`)
44+
45+
expect(readOAuthChatAttempt(attempt.id)?.status).toBe('connected')
46+
expect(window.close).toHaveBeenCalledOnce()
47+
act(() => root.unmount())
48+
})
49+
50+
it('publishes failure when the provider returned an error', () => {
51+
const { root } = renderAt(`?oauthAttempt=${attempt.id}&error=access_denied`)
52+
53+
expect(readOAuthChatAttempt(attempt.id)?.status).toBe('failed')
54+
act(() => root.unmount())
55+
})
56+
57+
it('leaves an unrelated attempt untouched when no attempt is named', () => {
58+
const { root } = renderAt('')
59+
60+
expect(readOAuthChatAttempt(attempt.id)?.status).toBe('pending')
61+
expect(window.close).toHaveBeenCalledOnce()
62+
act(() => root.unmount())
63+
})
64+
65+
it('still publishes the verdict when the attempt store rejects the write', () => {
66+
const setItem = vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => {
67+
throw new DOMException('quota', 'QuotaExceededError')
68+
})
69+
70+
// The verdict is lost, but the window must still be released — an
71+
// unguarded throw would strand the popup open on this page.
72+
expect(() => renderAt(`?oauthAttempt=${attempt.id}`)).not.toThrow()
73+
expect(window.close).toHaveBeenCalledOnce()
74+
setItem.mockRestore()
75+
})
76+
77+
describe('close-refused fallback', () => {
78+
const realLocation = window.location
79+
80+
afterEach(() => {
81+
vi.useRealTimers()
82+
Object.defineProperty(window, 'location', { configurable: true, value: realLocation })
83+
})
84+
85+
/**
86+
* jsdom performs no navigation and forbids redefining `location.replace`,
87+
* so the whole location is swapped for a stub carrying only what the
88+
* handoff reads: the current href, the origin, and the redirect sink.
89+
*/
90+
function renderWithStubbedLocation(search: string): { calls: string[]; root: Root } {
91+
const calls: string[] = []
92+
Object.defineProperty(window, 'location', {
93+
configurable: true,
94+
value: {
95+
href: `https://sim.test/oauth/chat-complete${search}`,
96+
origin: 'https://sim.test',
97+
replace: (url: string) => calls.push(url),
98+
},
99+
})
100+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
101+
const root: Root = createRoot(document.createElement('div'))
102+
act(() => root.render(<ChatCompleteHandoff />))
103+
return { calls, root }
104+
}
105+
106+
it('redirects to a same-origin returnTo once the close is refused', () => {
107+
vi.useFakeTimers()
108+
const returnTo = 'https://sim.test/workspace/workspace-1/chat/chat-1'
109+
110+
const { calls, root } = renderWithStubbedLocation(
111+
`?oauthAttempt=${attempt.id}&returnTo=${encodeURIComponent(returnTo)}`
112+
)
113+
act(() => {
114+
vi.advanceTimersByTime(400)
115+
})
116+
117+
expect(calls).toEqual([returnTo])
118+
act(() => root.unmount())
119+
})
120+
121+
it('refuses a cross-origin returnTo and falls back to the workspace', () => {
122+
vi.useFakeTimers()
123+
124+
const { calls, root } = renderWithStubbedLocation(
125+
`?oauthAttempt=${attempt.id}&returnTo=${encodeURIComponent('https://evil.example/steal')}`
126+
)
127+
act(() => {
128+
vi.advanceTimersByTime(400)
129+
})
130+
131+
expect(calls).toEqual(['/workspace'])
132+
act(() => root.unmount())
133+
})
134+
})
135+
})
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
'use client'
2+
3+
import { useEffect, useRef } from 'react'
4+
import {
5+
OAUTH_CHAT_ATTEMPT_PARAM,
6+
OAUTH_CHAT_RETURN_TO_PARAM,
7+
setOAuthChatAttemptStatus,
8+
} from '@/lib/credentials/oauth-chat-attempt'
9+
10+
const CLOSE_FALLBACK_DELAY_MS = 400
11+
12+
/**
13+
* The fallback redirect must never leave this origin — the target rides in a
14+
* query param the user could have tampered with.
15+
*/
16+
function sanitizeReturnTo(raw: string | null): string | null {
17+
if (!raw) return null
18+
try {
19+
const url = new URL(raw, window.location.origin)
20+
return url.origin === window.location.origin ? url.toString() : null
21+
} catch {
22+
return null
23+
}
24+
}
25+
26+
/**
27+
* Behavior half of the chat OAuth return leg: publishes the verdict to the
28+
* attempt record — which the chat tab's chip picks up over its storage
29+
* listener — then closes the window. Renders nothing, so the page's frame is
30+
* plain server-rendered markup that paints before this hydrates.
31+
*
32+
* Reaching this page IS the verdict. Better Auth routes a flow here only as
33+
* its success `callbackURL`, sending failures to `onAPIError.errorURL`
34+
* (`/oauth-error`) or back here with an `error` code, so the server has
35+
* already decided by the time this runs. That is a strictly better signal than
36+
* the credential diffing the generic-page return does: re-authorizing an
37+
* already-linked account updates the account row instead of creating one, so
38+
* no new credential appears and a diff would call a perfectly good connect a
39+
* failure.
40+
*
41+
* A window the browser refuses to close redirects on to the chat surface
42+
* instead. That is the popup-blocked path: the anchor's `target='_blank'`
43+
* opens this leg in a new tab, which no script may close. That URL
44+
* deliberately carries no attempt id — the verdict is already published, and
45+
* the destination's return router would otherwise re-decide it by the very
46+
* diff this page exists to avoid.
47+
*/
48+
export function ChatCompleteHandoff() {
49+
const ranRef = useRef(false)
50+
51+
useEffect(() => {
52+
if (ranRef.current) return
53+
ranRef.current = true
54+
55+
const params = new URL(window.location.href).searchParams
56+
const attemptId = params.get(OAUTH_CHAT_ATTEMPT_PARAM)
57+
const returnTo = sanitizeReturnTo(params.get(OAUTH_CHAT_RETURN_TO_PARAM))
58+
59+
if (attemptId) {
60+
setOAuthChatAttemptStatus(attemptId, params.has('error') ? 'failed' : 'connected')
61+
}
62+
63+
window.close()
64+
const timer = window.setTimeout(() => {
65+
window.location.replace(returnTo ?? '/workspace')
66+
}, CLOSE_FALLBACK_DELAY_MS)
67+
return () => window.clearTimeout(timer)
68+
}, [])
69+
70+
return null
71+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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 signals the chat tab and closes — instead of loading a
14+
* second copy of the app.
15+
*
16+
* Shown for a few hundred milliseconds in a popup, or briefly in the original
17+
* tab when the popup was blocked, so it wears the same handoff frame as the
18+
* other minimal-chrome gates (the 404, the desktop connect screens) rather
19+
* than styling of its own.
20+
*/
21+
export default function ChatCompletePage() {
22+
return (
23+
<>
24+
<ChatCompleteHandoff />
25+
<DesktopHandoffShell title='Finishing the connection' description='Returning you to Sim.' />
26+
</>
27+
)
28+
}

0 commit comments

Comments
 (0)