Skip to content

Commit 2403a8d

Browse files
fix(credentials): cover OAuth handoff edge cases
1 parent 5139056 commit 2403a8d

15 files changed

Lines changed: 170 additions & 25 deletions

File tree

apps/desktop/src/main/handoff.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,14 +249,17 @@ describe('createHandoffManager', () => {
249249
const manager = createHandoffManager(deps, makeCallbacks())
250250
await manager.beginConnect('google-email', {
251251
workspaceId: 'workspace-1',
252+
draftId: 'draft-1',
252253
chatAttemptId: 'attempt-1',
253254
})
254-
const state = new URL(vi.mocked(deps.openExternal).mock.calls[0][0]).searchParams.get(
255-
'state'
256-
) as string
255+
const landing = new URL(vi.mocked(deps.openExternal).mock.calls[0][0])
256+
const state = landing.searchParams.get('state') as string
257+
258+
expect(landing.searchParams.get('draftId')).toBe('draft-1')
257259

258260
expect(manager.consumeConnect(state)).toEqual({
259261
workspaceId: 'workspace-1',
262+
draftId: 'draft-1',
260263
chatAttemptId: 'attempt-1',
261264
})
262265
expect(manager.consumeConnect(state)).toBeNull()

apps/desktop/src/main/handoff.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export interface HandoffManagerDeps {
6767
export interface ConnectScope {
6868
workspaceId?: string
6969
credentialId?: string
70+
draftId?: string
7071
chatAttemptId?: string
7172
}
7273

@@ -292,6 +293,7 @@ export function createHandoffManager(
292293
...(userId ? { user: userId } : {}),
293294
...(scope.workspaceId ? { workspaceId: scope.workspaceId } : {}),
294295
...(scope.credentialId ? { credentialId: scope.credentialId } : {}),
296+
...(scope.draftId ? { draftId: scope.draftId } : {}),
295297
},
296298
scope
297299
)

apps/desktop/src/main/ipc.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,21 +335,24 @@ describe('registerIpcHandlers', () => {
335335
expect(await handler?.(appEvent, 'slack')).toBe(true)
336336
expect(deps.beginOAuthConnect).toHaveBeenCalledWith('slack', {})
337337

338-
// Chip-initiated connects carry workspace/credential scope; malformed
338+
// Connects carry workspace/credential or exact-draft scope; malformed
339339
// scopes (wrong types, unsafe ids) are rejected before the handoff.
340340
expect(
341341
await handler?.(appEvent, 'slack', {
342342
workspaceId: 'ws1',
343343
credentialId: 'cred_1',
344+
draftId: 'draft_1',
344345
chatAttemptId: 'attempt_1',
345346
})
346347
).toBe(true)
347348
expect(deps.beginOAuthConnect).toHaveBeenCalledWith('slack', {
348349
workspaceId: 'ws1',
349350
credentialId: 'cred_1',
351+
draftId: 'draft_1',
350352
chatAttemptId: 'attempt_1',
351353
})
352354
expect(await handler?.(appEvent, 'slack', { workspaceId: 'ws/../evil' })).toBe(false)
355+
expect(await handler?.(appEvent, 'slack', { draftId: '../wrong' })).toBe(false)
353356
expect(await handler?.(appEvent, 'slack', { chatAttemptId: '../wrong' })).toBe(false)
354357
expect(await handler?.(appEvent, 'slack', 'not-an-object')).toBe(false)
355358
})

apps/desktop/src/main/ipc.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ function parseDesktopScope(raw: unknown): string | null {
9595
export interface OAuthConnectScope {
9696
workspaceId?: string
9797
credentialId?: string
98+
draftId?: string
9899
chatAttemptId?: string
99100
}
100101

@@ -110,9 +111,10 @@ export function parseOAuthConnectScope(raw: unknown): OAuthConnectScope | undefi
110111
if (typeof raw !== 'object') {
111112
return undefined
112113
}
113-
const { workspaceId, credentialId, chatAttemptId } = raw as {
114+
const { workspaceId, credentialId, draftId, chatAttemptId } = raw as {
114115
workspaceId?: unknown
115116
credentialId?: unknown
117+
draftId?: unknown
116118
chatAttemptId?: unknown
117119
}
118120
if (
@@ -127,6 +129,9 @@ export function parseOAuthConnectScope(raw: unknown): OAuthConnectScope | undefi
127129
) {
128130
return undefined
129131
}
132+
if (draftId !== undefined && (typeof draftId !== 'string' || !ID_PATTERN.test(draftId))) {
133+
return undefined
134+
}
130135
if (
131136
chatAttemptId !== undefined &&
132137
(typeof chatAttemptId !== 'string' || !ID_PATTERN.test(chatAttemptId))
@@ -136,6 +141,7 @@ export function parseOAuthConnectScope(raw: unknown): OAuthConnectScope | undefi
136141
return {
137142
...(workspaceId !== undefined ? { workspaceId } : {}),
138143
...(credentialId !== undefined ? { credentialId } : {}),
144+
...(draftId !== undefined ? { draftId } : {}),
139145
...(chatAttemptId !== undefined ? { chatAttemptId } : {}),
140146
}
141147
}

apps/sim/app/desktop/connect/page.tsx

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,16 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec
5050
const port = parseLoopbackPort(typeof params.port === 'string' ? params.port : '')
5151
const workspaceId = isValidOpaqueId(params.workspaceId) ? params.workspaceId : undefined
5252
const credentialId = isValidOpaqueId(params.credentialId) ? params.credentialId : undefined
53+
const draftId = isValidOpaqueId(params.draftId) ? params.draftId : undefined
5354
const expectedUserId = isValidOpaqueId(params.user) ? params.user : undefined
54-
if (!isValidOAuthProviderId(providerId) || !isValidHandoffState(state) || port === null) {
55+
const hasInvalidDraftId = params.draftId !== undefined && draftId === undefined
56+
if (
57+
!isValidOAuthProviderId(providerId) ||
58+
!isValidHandoffState(state) ||
59+
port === null ||
60+
hasInvalidDraftId ||
61+
(workspaceId !== undefined && draftId !== undefined)
62+
) {
5563
return <InvalidRequest />
5664
}
5765

@@ -65,6 +73,7 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec
6573
buildDesktopConnectPath(providerId, state, port, {
6674
workspaceId,
6775
credentialId,
76+
draftId,
6877
user: expectedUserId,
6978
})
7079
)}`
@@ -86,6 +95,7 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec
8695
returnTo={buildDesktopConnectPath(providerId, state, port, {
8796
workspaceId,
8897
credentialId,
98+
draftId,
8999
user: expectedUserId,
90100
})}
91101
/>
@@ -113,6 +123,9 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec
113123
}
114124

115125
return (
116-
<ConnectLauncher providerId={providerId} completePath={buildConnectCompletePath(state, port)} />
126+
<ConnectLauncher
127+
providerId={providerId}
128+
completePath={buildConnectCompletePath(state, port, draftId)}
129+
/>
117130
)
118131
}

apps/sim/app/desktop/connect/validation.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5+
import { OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM } from '@/lib/credentials/draft-constants'
56
import {
67
buildConnectCompletePath,
78
buildConnectLoopbackUrl,
@@ -36,19 +37,23 @@ describe('sanitizeOAuthErrorSlug', () => {
3637

3738
describe('URL builders', () => {
3839
it('buildDesktopConnectPath round-trips provider, state, and port', () => {
39-
const path = buildDesktopConnectPath('google-email', STATE, 49152)
40+
const path = buildDesktopConnectPath('google-email', STATE, 49152, {
41+
draftId: 'draft-1',
42+
})
4043
const url = new URL(path, 'https://sim.ai')
4144
expect(url.pathname).toBe('/desktop/connect')
4245
expect(url.searchParams.get('provider')).toBe('google-email')
4346
expect(url.searchParams.get('state')).toBe(STATE)
4447
expect(url.searchParams.get('port')).toBe('49152')
48+
expect(url.searchParams.get('draftId')).toBe('draft-1')
4549
})
4650

47-
it('buildConnectCompletePath carries state and port', () => {
48-
const url = new URL(buildConnectCompletePath(STATE, 49152), 'https://sim.ai')
51+
it('buildConnectCompletePath carries state, port, and the exact credential draft', () => {
52+
const url = new URL(buildConnectCompletePath(STATE, 49152, 'draft-1'), 'https://sim.ai')
4953
expect(url.pathname).toBe('/desktop/connect/complete')
5054
expect(url.searchParams.get('state')).toBe(STATE)
5155
expect(url.searchParams.get('port')).toBe('49152')
56+
expect(url.searchParams.get(OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM)).toBe('draft-1')
5257
})
5358

5459
it('buildConnectLoopbackUrl targets the 127.0.0.1 connect callback, error optional', () => {

apps/sim/app/desktop/connect/validation.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM } from '@/lib/credentials/draft-constants'
2+
13
/**
24
* OAuth providerIds are kebab-case service slugs (e.g. "google-email"). The
35
* value is only used to start a better-auth oauth2.link flow, which validates
@@ -36,6 +38,7 @@ export function isValidOpaqueId(value: unknown): value is string {
3638
export interface ConnectScope {
3739
workspaceId?: string
3840
credentialId?: string
41+
draftId?: string
3942
/** The account the desktop app is signed in as; the flow is pinned to it. */
4043
user?: string
4144
}
@@ -53,6 +56,7 @@ export function buildDesktopConnectPath(
5356
const params = new URLSearchParams({ provider: providerId, state, port: String(port) })
5457
if (scope.workspaceId) params.set('workspaceId', scope.workspaceId)
5558
if (scope.credentialId) params.set('credentialId', scope.credentialId)
59+
if (scope.draftId) params.set('draftId', scope.draftId)
5660
if (scope.user) params.set('user', scope.user)
5761
return `/desktop/connect?${params.toString()}`
5862
}
@@ -61,8 +65,9 @@ export function buildDesktopConnectPath(
6165
* The same-origin path better-auth redirects the browser to after the OAuth
6266
* callback — the complete page then bounces to the desktop app's loopback.
6367
*/
64-
export function buildConnectCompletePath(state: string, port: number): string {
68+
export function buildConnectCompletePath(state: string, port: number, draftId?: string): string {
6569
const params = new URLSearchParams({ state, port: String(port) })
70+
if (draftId) params.set(OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM, draftId)
6671
return `/desktop/connect/complete?${params.toString()}`
6772
}
6873

apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,11 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) {
285285
.filter(
286286
(credential) => credential.type === 'oauth' && credential.providerId === providerId
287287
)
288-
.map((credential) => ({ id: credential.id, accountId: credential.accountId })),
288+
.map((credential) => ({
289+
id: credential.id,
290+
accountId: credential.accountId,
291+
updatedAt: credential.updatedAt,
292+
})),
289293
workspaceId,
290294
requestedAt: Date.now(),
291295
}

apps/sim/hooks/queries/oauth/oauth-connections.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,10 @@ export function useConnectOAuthService() {
180180
// which refreshes caches and shows the connected toast.
181181
const desktopBridge = getDesktopBridge()
182182
if (desktopBridge?.beginOAuthConnect) {
183-
const opened = await desktopBridge.beginOAuthConnect(providerId)
183+
const opened = await desktopBridge.beginOAuthConnect(
184+
providerId,
185+
draftId ? { draftId } : undefined
186+
)
184187
if (!opened) {
185188
throw new Error('Could not open your browser to connect this account.')
186189
}
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({
7+
requestJson: vi.fn(),
8+
requireWorkspaceCredentialListResponse: vi.fn(),
9+
}))
10+
11+
vi.mock('@sim/emcn', () => ({
12+
toast: { error: vi.fn(), success: vi.fn() },
13+
}))
14+
vi.mock('next/navigation', () => ({
15+
useParams: vi.fn(),
16+
useRouter: vi.fn(),
17+
}))
18+
vi.mock('@/lib/api/client/request', () => ({ requestJson: mocks.requestJson }))
19+
vi.mock('@/hooks/queries/utils/fetch-workspace-credentials', () => ({
20+
requireWorkspaceCredentialListResponse: mocks.requireWorkspaceCredentialListResponse,
21+
}))
22+
23+
import type { OAuthReturnContext } from '@/lib/credentials/client-state'
24+
import { resolveOAuthMessage } from '@/hooks/use-oauth-return'
25+
26+
const context: OAuthReturnContext = {
27+
origin: 'integrations',
28+
displayName: 'New Gmail',
29+
providerId: 'google-email',
30+
preCount: 1,
31+
baselineCredentials: [
32+
{
33+
id: 'credential-existing',
34+
accountId: 'account-1',
35+
updatedAt: '2026-08-14T17:00:00.000Z',
36+
},
37+
],
38+
workspaceId: 'workspace-1',
39+
requestedAt: Date.now(),
40+
}
41+
42+
const existingCredential = {
43+
id: 'credential-existing',
44+
workspaceId: 'workspace-1',
45+
type: 'oauth' as const,
46+
displayName: 'Existing Gmail',
47+
description: null,
48+
providerId: 'google-email',
49+
accountId: 'account-1',
50+
envKey: null,
51+
envOwnerUserId: null,
52+
createdBy: 'user-1',
53+
createdAt: '2026-08-01T00:00:00.000Z',
54+
updatedAt: '2026-08-14T18:00:00.000Z',
55+
}
56+
57+
describe('resolveOAuthMessage', () => {
58+
beforeEach(() => {
59+
vi.clearAllMocks()
60+
mocks.requestJson.mockResolvedValue({})
61+
})
62+
63+
it('recognizes an idempotent already-connected account from its reconnect timestamp', async () => {
64+
mocks.requireWorkspaceCredentialListResponse.mockReturnValue([existingCredential])
65+
66+
await expect(resolveOAuthMessage(context)).resolves.toEqual({
67+
kind: 'success',
68+
text: 'This account is already connected as "Existing Gmail".',
69+
})
70+
})
71+
72+
it('does not report success when the credential list is unchanged', async () => {
73+
mocks.requireWorkspaceCredentialListResponse.mockReturnValue([
74+
{
75+
...existingCredential,
76+
updatedAt: context.baselineCredentials?.[0].updatedAt,
77+
},
78+
])
79+
80+
await expect(resolveOAuthMessage(context)).resolves.toEqual({
81+
kind: 'error',
82+
text: 'We couldn’t verify the "New Gmail" connection. Try again.',
83+
})
84+
})
85+
})

0 commit comments

Comments
 (0)