Skip to content

Commit a05b8fa

Browse files
committed
fix(integrations): close connection recovery edge cases
1 parent 55d877b commit a05b8fa

9 files changed

Lines changed: 230 additions & 4 deletions

File tree

‎.github/workflows/desktop-e2e.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ on:
1717
- 'apps/sim/app/desktop/connect/**'
1818
- 'apps/sim/app/credential-groups/**'
1919
- 'apps/sim/hooks/queries/slack-search.ts'
20+
- 'apps/sim/hooks/queries/personal-search-integrations.ts'
21+
- 'apps/sim/hooks/use-search-integration-connection.ts'
2022
- 'apps/sim/hooks/use-github-installation-setup.ts'
2123
- 'apps/sim/app/o/**/integrations/indexed/use-member-enrollment.ts'
2224
- 'apps/sim/lib/api/contracts/desktop-source-connect.ts'

‎apps/desktop/e2e/source-connect.spec.ts‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ test('source authorization returns to its desktop screen and refreshes live', as
5454
const githubInventorySessions: string[] = []
5555
let nativeCredentialVisible = false
5656
let installed = false
57+
let holdSlackStart = false
58+
let canceledSlackRequests = 0
59+
const personalAttempts = new Map<string, { session: string; completed: boolean }>()
60+
let personalInventoryFailed = false
5761
let javascript = ''
5862
let stylesheet = ''
5963
let origin = ''
@@ -174,9 +178,82 @@ test('source authorization returns to its desktop screen and refreshes live', as
174178
const state = generateShortId(32)
175179
attempts.set(state, session)
176180
startSessions.push(session)
181+
if (holdSlackStart) {
182+
response.on('close', () => {
183+
if (!response.writableEnded) canceledSlackRequests++
184+
})
185+
return
186+
}
177187
json({ authorizationUrl: `${origin}/provider?state=${state}` })
178188
return
179189
}
190+
if (path === '/api/knowledge/sim-search/personal-integrations') {
191+
if (request.method === 'POST') {
192+
const { oauthCompletionId } = await body()
193+
personalAttempts.set(oauthCompletionId, { session, completed: false })
194+
json({
195+
success: true,
196+
data: { url: `${origin}/personal-provider?completionId=${oauthCompletionId}` },
197+
})
198+
} else if (personalInventoryFailed) {
199+
json({ error: 'Inventory temporarily unavailable' }, 503)
200+
} else {
201+
const attempt = personalAttempts.get(url.searchParams.get('completionId') ?? '')
202+
const connected = attempt?.completed === true
203+
json({
204+
success: true,
205+
data: {
206+
completedCredentialId: connected ? 'fixture-personal-account' : null,
207+
connections: connected
208+
? [
209+
{
210+
name: 'Slack',
211+
providerId: 'slack',
212+
connectorType: 'slack',
213+
description: '',
214+
accounts: [
215+
{
216+
credentialId: 'fixture-personal-account',
217+
displayName: 'Fixture',
218+
status: 'connected',
219+
action: null,
220+
},
221+
],
222+
connectionStatus: 'connected',
223+
action: null,
224+
},
225+
]
226+
: [],
227+
available: [
228+
{
229+
name: 'Slack',
230+
description: '',
231+
target: {
232+
type: 'link',
233+
provider: 'slack',
234+
connectorType: 'slack',
235+
connectionMode: 'live',
236+
optionId: 'fixture-option',
237+
},
238+
},
239+
],
240+
nextCursor: null,
241+
},
242+
})
243+
}
244+
return
245+
}
246+
if (path === '/personal-callback') {
247+
const completionId = url.searchParams.get('completionId') ?? ''
248+
const attempt = personalAttempts.get(completionId)
249+
if (!attempt || attempt.session !== session) {
250+
json({ error: 'Wrong attempt' }, 403)
251+
return
252+
}
253+
attempt.completed = true
254+
redirect(`/credential-groups/complete?completionId=${completionId}`)
255+
return
256+
}
180257
if (path === '/api/knowledge/slack/oauth/callback') {
181258
const state = url.searchParams.get('state') ?? ''
182259
callbackSessions.push(session)
@@ -298,6 +375,12 @@ test('source authorization returns to its desktop screen and refreshes live', as
298375
)
299376
return
300377
}
378+
if (path === '/personal-provider') {
379+
response.end(
380+
`<!doctype html><a href="/personal-callback?completionId=${url.searchParams.get('completionId')}">Authorize personal Search</a>`
381+
)
382+
return
383+
}
301384
if (path === '/github-provider') {
302385
response.end(
303386
`<!doctype html><a href="/github-callback?setupId=${url.searchParams.get('setupId')}">Authorize GitHub</a>`
@@ -570,6 +653,61 @@ test('source authorization returns to its desktop screen and refreshes live', as
570653
await expect(web).toHaveURL(`${origin}/o/fixture-organization/integrations`)
571654
await expect(web.getByLabel('Account count')).toHaveText('1')
572655
})
656+
await check('canceling Slack setup aborts the pending web HTTP request', async () => {
657+
holdSlackStart = true
658+
const starts = startSessions.length
659+
try {
660+
await web.getByRole('button', { name: 'Connect Slack', exact: true }).click()
661+
await expect.poll(() => startSessions.length).toBe(starts + 1)
662+
await web.getByRole('button', { name: 'Cancel Slack request', exact: true }).click()
663+
await expect.poll(() => canceledSlackRequests).toBe(1)
664+
await expect(web.getByLabel('Connection', { exact: true })).toHaveText('error')
665+
} finally {
666+
holdSlackStart = false
667+
}
668+
})
669+
await check(
670+
'desktop Search preserves pending receipts after inventory failure and allows cancellation/retry',
671+
async () => {
672+
const previousOpens = (await opened()).length
673+
await page.getByRole('button', { name: 'Connect personal Search', exact: true }).click()
674+
await expect.poll(async () => (await opened()).length).toBe(previousOpens + 1)
675+
await external.goto((await opened())[previousOpens])
676+
await external.getByRole('link', { name: 'Authorize personal Search' }).waitFor()
677+
personalInventoryFailed = true
678+
await external.getByRole('link', { name: 'Authorize personal Search' }).click()
679+
await expect(external).toHaveURL(`${origin}/desktop/done?kind=connect`)
680+
await expect(page.getByLabel('Personal Search inventory error')).toHaveText(
681+
'Inventory temporarily unavailable'
682+
)
683+
await expect(
684+
page.getByRole('button', { name: 'Connect personal Search', exact: true })
685+
).toBeEnabled()
686+
const receipt = () =>
687+
page.evaluate(() => {
688+
const entry = Object.entries(localStorage).find(([key]) =>
689+
key.startsWith('sim.search-connection.')
690+
)
691+
return entry ? (JSON.parse(entry[1]).completionId as string) : null
692+
})
693+
const pendingReceipt = await receipt()
694+
expect(pendingReceipt).toBeTruthy()
695+
await page.getByRole('button', { name: 'Connect personal Search', exact: true }).click()
696+
expect(await receipt()).toBe(pendingReceipt)
697+
await expect(page.getByLabel('Personal Search pending')).toHaveText('true')
698+
await page.getByRole('button', { name: 'Cancel personal Search', exact: true }).click()
699+
await expect(page.getByLabel('Personal Search pending')).toHaveText('false')
700+
personalInventoryFailed = false
701+
await page.getByRole('button', { name: 'Retry personal inventory', exact: true }).click()
702+
await page.getByRole('button', { name: 'Connect personal Search', exact: true }).click()
703+
await expect.poll(async () => (await opened()).length).toBe(previousOpens + 2)
704+
expect(await receipt()).not.toBe(pendingReceipt)
705+
await external.goto((await opened())[previousOpens + 1])
706+
await external.getByRole('link', { name: 'Authorize personal Search' }).click()
707+
await expect(page.getByLabel('Personal Search connected')).toHaveText('true')
708+
await expect(page.getByLabel('Personal Search pending')).toHaveText('false')
709+
}
710+
)
573711
await page.screenshot({ path: test.info().outputPath('source-connect-desktop.png') })
574712
} finally {
575713
mkdirSync(dirname(reportPath), { recursive: true })
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { authMockFns } from '@sim/testing/mocks/auth.mock'
2+
import { createMockRequest } from '@sim/testing/mocks/request.mock'
3+
import { expect, it } from 'vitest'
4+
import { GET } from '@/app/api/credential-groups/slack-managed-users/callback/route'
5+
6+
it('preserves sign-in recovery when the managed Slack callback loses its session', async () => {
7+
authMockFns.mockGetSession.mockResolvedValueOnce(null)
8+
const response = await GET(
9+
createMockRequest({
10+
url: 'http://localhost/api/credential-groups/slack-managed-users/callback?state=fixture-state&code=fixture-code',
11+
})
12+
)
13+
expect(response.status).toBe(303)
14+
const location = new URL(response.headers.get('location')!)
15+
expect(location.pathname).toBe('/credential-groups/slack-complete')
16+
expect(location.searchParams.get('state')).toBe('fixture-state')
17+
expect(location.searchParams.get('reason')).toBe('signin_required')
18+
})

‎apps/sim/app/api/credential-groups/slack-managed-users/callback/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4141
ok: false,
4242
message: 'Sign in to Sim to complete this Slack setup.',
4343
state: rawState,
44-
reason: 'unauthenticated',
44+
reason: 'signin_required',
4545
})
4646
}
4747
const parsed = await parseRequest(slackCredentialGroupConfigurationCallbackContract, request, {})
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { authMockFns } from '@sim/testing/mocks/auth.mock'
2+
import { rateLimiterMock, rateLimiterMockFns } from '@sim/testing/mocks/rate-limiter.mock'
3+
import { createMockRequest } from '@sim/testing/mocks/request.mock'
4+
import { expect, it, vi } from 'vitest'
5+
6+
vi.mock('@/lib/core/rate-limiter', () => rateLimiterMock)
7+
8+
import { POST } from '@/app/api/desktop/source-connect/route'
9+
10+
it.each([true, false])(
11+
'rejects an oversized desktop request before JSON decoding (declared length: %s)',
12+
async (declaredLength) => {
13+
authMockFns.mockGetSession.mockResolvedValueOnce({
14+
user: { id: 'fixture-user' },
15+
session: { id: 'fixture-session' },
16+
})
17+
rateLimiterMockFns.mockEnforceUserRateLimit.mockResolvedValueOnce(null)
18+
const rawBody = ' '.repeat(64 * 1024 + 1)
19+
const response = await POST(
20+
createMockRequest({
21+
method: 'POST',
22+
url: 'http://localhost/api/desktop/source-connect',
23+
rawBody,
24+
headers: {
25+
'content-type': 'application/json',
26+
...(declaredLength ? { 'content-length': String(rawBody.length) } : {}),
27+
},
28+
})
29+
)
30+
expect(response.status).toBe(413)
31+
}
32+
)

‎apps/sim/app/api/desktop/source-connect/route.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const POST = defineInternalJsonRoute({
1313
operation: createDesktopSourceRequest.operation,
1414
rateLimit: internalRateLimits.user({ bucketName: 'desktop-source-connect' }),
1515
errorPolicy: internalOrchestrationErrorPolicy,
16+
parseOptions: { maxBodyBytes: 64 * 1024 },
1617
mapInput: ({ body }) => ({ requestId: body.requestId, payload: JSON.stringify(body.request) }),
1718
useCase: createDesktopSourceRequest,
1819
staticResponseHeaders: { 'Cache-Control': 'no-store' },

‎apps/sim/hooks/queries/slack-search.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function useStartSlackSearchOAuth() {
4848
await connectDesktopSource({ kind: 'slack-search', body }, signal)
4949
return null
5050
}
51-
return requestJson(startSlackSearchOAuthContract, { body })
51+
return requestJson(startSlackSearchOAuthContract, { body, signal })
5252
},
5353
onSettled: (_data, _error, input) =>
5454
Promise.all([

‎apps/sim/hooks/use-search-integration-connection.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ export function useSearchIntegrationConnection({
160160
return
161161
}
162162
const desktop = isDesktopApp()
163+
if (desktop && pending) return
163164
const tab = desktop ? null : window.open('about:blank', '_blank', 'width=600,height=700')
164165
if (!desktop && !tab) {
165166
setLocalError('Allow pop-ups for this site to connect your account.')

‎apps/sim/scripts/fixtures/desktop-source-connect.tsx‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '@/hooks/queries/organization-accounts'
1616
import { useSlackSearchInstallations, useStartSlackSearchOAuth } from '@/hooks/queries/slack-search'
1717
import { useGitHubInstallationSetup } from '@/hooks/use-github-installation-setup'
18+
import { useSearchIntegrationConnection } from '@/hooks/use-search-integration-connection'
1819

1920
const NO_CONNECTIONS = new Set<string>()
2021
const MEMBERSHIP_KEYS: readonly (readonly string[])[] = []
@@ -32,7 +33,20 @@ function SourceConnectFixture() {
3233
organizationId: 'fixture-organization',
3334
onConnected: setGithubCredential,
3435
})
36+
const slackAbort = useRef<AbortController | null>(null)
3537
const connection = useStartSlackSearchOAuth()
38+
const personal = useSearchIntegrationConnection({
39+
organizationId: 'fixture-organization',
40+
userId: 'fixture-user',
41+
controlId: 'fixture-search-card',
42+
target: {
43+
type: 'link',
44+
provider: 'slack',
45+
connectorType: 'slack',
46+
connectionMode: 'live',
47+
optionId: 'fixture-option',
48+
},
49+
})
3650
const inventory = useSlackSearchInstallations('fixture-organization')
3751
return (
3852
<main className='flex flex-col items-start gap-2 p-6'>
@@ -72,17 +86,37 @@ function SourceConnectFixture() {
7286
<input aria-label='Source draft' defaultValue='Unsubmitted source name' />
7387
<button
7488
disabled={connection.isPending}
75-
onClick={() =>
89+
onClick={() => {
90+
const controller = new AbortController()
91+
slackAbort.current = controller
7692
connection.mutate({
93+
signal: controller.signal,
7794
organizationId: 'fixture-organization',
7895
name: 'Search',
7996
description: 'Search fixture',
8097
mode: 'shared',
8198
})
82-
}
99+
}}
83100
>
84101
Connect Slack
85102
</button>
103+
<button onClick={() => slackAbort.current?.abort()}>Cancel Slack request</button>
104+
<button
105+
disabled={
106+
personal.isLoading ||
107+
personal.isStarting ||
108+
personal.connected ||
109+
(!personal.available && !personal.pending)
110+
}
111+
onClick={() => void personal.connect()}
112+
>
113+
Connect personal Search
114+
</button>
115+
<button onClick={personal.cancel}>Cancel personal Search</button>
116+
<button onClick={() => void personal.retry()}>Retry personal inventory</button>
117+
<output aria-label='Personal Search pending'>{String(personal.pending)}</output>
118+
<output aria-label='Personal Search inventory error'>{personal.inventoryError}</output>
119+
<output aria-label='Personal Search connected'>{String(personal.connected)}</output>
86120
<button disabled={github.pending} onClick={() => void github.connect()}>
87121
Connect GitHub
88122
</button>

0 commit comments

Comments
 (0)