Skip to content

Commit 7e18c94

Browse files
committed
fix(integrations): preserve active account authorizations
1 parent 08692a4 commit 7e18c94

4 files changed

Lines changed: 51 additions & 37 deletions

File tree

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

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ test('source authorization returns to its desktop screen and refreshes live', as
4444
}
4545
const tickets = new Map<string, unknown>()
4646
const attempts = new Map<string, string>()
47-
const accountAttempts = new Map<string, string>()
47+
const accountAttempts = new Map<string, { session: string; mcp: boolean }>()
4848
let accountConnected = false
49+
let mcpAccountConnected = false
4950
const startSessions: string[] = []
5051
const callbackSessions: string[] = []
5152
const githubAttempts = new Map<string, { session: string; completed: boolean }>()
@@ -90,6 +91,16 @@ test('source authorization returns to its desktop screen and refreshes live', as
9091
availableMcpConnectors: [],
9192
canManage: false,
9293
indexingAvailable: true,
94+
viewerMcpAccounts: mcpAccountConnected
95+
? [
96+
{
97+
credentialId: 'fixture-mcp-account',
98+
displayName: 'Fixture MCP account',
99+
mcpServerId: 'fixture-mcp',
100+
status: 'active',
101+
},
102+
]
103+
: [],
93104
viewerAccounts: accountConnected
94105
? [
95106
{
@@ -109,15 +120,13 @@ test('source authorization returns to its desktop screen and refreshes live', as
109120
path === '/api/organizations/fixture-organization/connected-accounts/connect' ||
110121
path === '/api/users/me/organization-accounts/fixture-account/reconnect'
111122
) {
112-
const completionId =
113-
request.method === 'POST' && path.endsWith('/connect')
114-
? (await body()).oauthCompletionId
115-
: url.searchParams.get('oauthCompletionId')
123+
const input = request.method === 'POST' && path.endsWith('/connect') ? await body() : null
124+
const completionId = input?.oauthCompletionId ?? url.searchParams.get('oauthCompletionId')
116125
if (!completionId) {
117126
json({ error: 'Missing completion ID' }, 400)
118127
return
119128
}
120-
accountAttempts.set(completionId, session)
129+
accountAttempts.set(completionId, { session, mcp: Boolean(input?.mcpServerId) })
121130
json({
122131
invitationLink: `${origin}/credential-groups/enroll/fixture-account-invitation`,
123132
authorizationUrl: `${origin}/account-provider?completionId=${completionId}`,
@@ -126,13 +135,17 @@ test('source authorization returns to its desktop screen and refreshes live', as
126135
}
127136
if (path === '/account-callback') {
128137
const completionId = url.searchParams.get('completionId') ?? ''
129-
if (accountAttempts.get(completionId) !== session) {
138+
const attempt = accountAttempts.get(completionId)
139+
if (attempt?.session !== session) {
130140
json({ error: 'Wrong attempt' }, 403)
131141
return
132142
}
133143
accountAttempts.delete(completionId)
134144
const denied = url.searchParams.has('error')
135-
if (!denied) accountConnected = true
145+
if (!denied) {
146+
if (attempt.mcp) mcpAccountConnected = true
147+
else accountConnected = true
148+
}
136149
redirect(
137150
`/credential-groups/complete?completionId=${completionId}&organizationId=fixture-organization${denied ? '&oauth=denied' : ''}`
138151
)
@@ -494,6 +507,7 @@ test('source authorization returns to its desktop screen and refreshes live', as
494507
'web authorization preserves the origin and refreshes after an isolated provider window',
495508
async () => {
496509
accountConnected = false
510+
mcpAccountConnected = false
497511
await web.reload()
498512
await web.getByLabel('Source draft').fill('Web draft retained')
499513
await expect(web.getByLabel('Account count')).toHaveText('0')
@@ -507,6 +521,19 @@ test('source authorization returns to its desktop screen and refreshes live', as
507521
expect(web.url()).toBe(`${origin}/o/fixture-organization/integrations?search=fixture`)
508522
}
509523
)
524+
await check('overlapping connect and reconnect preserve the active authorization', async () => {
525+
const popupReady = context.waitForEvent('page')
526+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
527+
const popup = await popupReady
528+
await popup.getByRole('link', { name: 'Authorize account' }).waitFor()
529+
const pendingAttempts = accountAttempts.size
530+
await web.getByRole('button', { name: 'Reconnect account', exact: true }).click()
531+
await expect(web.getByLabel('Reconnect error')).toContainText('Finish or cancel')
532+
expect(accountAttempts.size).toBe(pendingAttempts)
533+
await expect(web.getByLabel('Account authorization', { exact: true })).toHaveText('pending')
534+
await popup.getByRole('link', { name: 'Authorize account' }).click()
535+
await expect(web.getByLabel('Account authorization', { exact: true })).toHaveText('success')
536+
})
510537
await check('web denial and cancellation leave the initiating page usable', async () => {
511538
const popupReady = context.waitForEvent('page')
512539
await web.getByRole('button', { name: 'Connect account', exact: true }).click()

‎apps/sim/app/o/[organizationId]/integrations/integrations.test.tsx‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import { NuqsTestingAdapter } from 'nuqs/adapters/testing'
2222
import { createRoot, type Root } from 'react-dom/client'
2323
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2424
import type { SearchSourceSummary } from '@/lib/api/contracts/knowledge/connectors'
25-
import type { OrganizationAccountConnectionResponse } from '@/lib/api/contracts/organization-accounts'
2625
import type { SearchConnector } from '@/lib/sim-search/connectors'
2726

2827
const mocks = vi.hoisted(() => ({
@@ -287,22 +286,6 @@ function menuItem(label: string) {
287286
)!
288287
}
289288

290-
function expectConnectionRedirect(
291-
onSuccess: (response: OrganizationAccountConnectionResponse) => void,
292-
authorizationUrl?: string
293-
) {
294-
const invitationLink = 'https://sim.test/credential-groups/enroll/fixture-token'
295-
const assign = vi.fn()
296-
const browserWindow = window
297-
vi.stubGlobal('window', { location: { assign } })
298-
try {
299-
onSuccess({ invitationLink, ...(authorizationUrl ? { authorizationUrl } : {}) })
300-
expect(assign).toHaveBeenCalledExactlyOnceWith(authorizationUrl ?? invitationLink)
301-
} finally {
302-
vi.stubGlobal('window', browserWindow)
303-
}
304-
}
305-
306289
describe('GitHub member account inventory', () => {
307290
const githubAccount = {
308291
credentialId: 'github-account',
@@ -346,10 +329,7 @@ describe('GitHub member account inventory', () => {
346329
})
347330
})
348331

349-
it.each([
350-
undefined,
351-
'https://sim.test/api/credential-groups/enroll/fixture-token/oauth/github-option?returnTo=search',
352-
])('connects once through the account operation with compatible redirect %s', async (url) => {
332+
it('connects once through the account operation', async () => {
353333
await render()
354334
expect(buttons('Connect')).toHaveLength(1)
355335
expect(container.textContent).toContain('Connect once')
@@ -358,7 +338,6 @@ describe('GitHub member account inventory', () => {
358338
{ organizationId: scope.organizationId, optionId: 'github-option' },
359339
expect.any(Object)
360340
)
361-
expectConnectionRedirect(mocks.connectOrganizationAccount.mock.calls[0][1].onSuccess, url)
362341
expect(mockUseSearchSources).not.toHaveBeenCalled()
363342
expect(mocks.connect).not.toHaveBeenCalled()
364343
expect(mocks.connectSearchSource).not.toHaveBeenCalled()
@@ -430,10 +409,7 @@ describe('GitHub member account inventory', () => {
430409
}
431410
)
432411

433-
it.each([
434-
undefined,
435-
'https://sim.test/api/credential-groups/enroll/fixture-token/oauth/github-option?returnTo=accounts',
436-
])('allows personal reauthorization while Search is disabled with redirect %s', async (url) => {
412+
it('allows personal reauthorization while Search is disabled', async () => {
437413
mockUseSearchSourceOverview.mockReturnValue({ data: { providers: [] }, isPending: false })
438414
mocks.integrations.mockReturnValue({
439415
data: [{ connectorType: 'github', approved: false }],
@@ -457,7 +433,6 @@ describe('GitHub member account inventory', () => {
457433
'github-account',
458434
expect.any(Object)
459435
)
460-
expectConnectionRedirect(mocks.reconnectOrganizationAccount.mock.calls[0][1].onSuccess, url)
461436
expect(mocks.connect).not.toHaveBeenCalled()
462437
})
463438

‎apps/sim/hooks/queries/organization-accounts.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ function useAccountConnectionMutation<Variables>(
6363
const pending = useRef<AbortController | null>(null)
6464
useEffect(() => () => pending.current?.abort(), [])
6565
return useMutation({
66+
mutationKey: organizationAccountsKeys.connection(),
6667
mutationFn: async (variables: Variables) => {
68+
if (client.isMutating({ mutationKey: organizationAccountsKeys.connection() }) > 1) {
69+
throw new Error('Finish or cancel your current account connection before starting another.')
70+
}
6771
pending.current?.abort()
6872
const controller = new AbortController()
6973
pending.current = controller
@@ -131,6 +135,7 @@ export function useDisconnectPersonalOrganizationAccount(organizationId: string)
131135

132136
export const organizationAccountsKeys = {
133137
all: ['organization-accounts'] as const,
138+
connection: () => [...organizationAccountsKeys.all, 'connection'] as const,
134139
workspaces: () => [...organizationAccountsKeys.all, 'workspace'] as const,
135140
workspace: (workspaceId?: string) =>
136141
[...organizationAccountsKeys.workspaces(), workspaceId ?? ''] as const,

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { StrictMode, useEffect, useRef, useState } from 'react'
22
import { ToastProvider } from '@sim/emcn'
33
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
44
import { createRoot } from 'react-dom/client'
5+
import { isCredentialGroupOAuthFailure } from '@/lib/credential-groups/oauth-completion'
56
import { startDesktopSourceBrowser } from '@/lib/desktop/source-browser'
67
import { CredentialGroupCompletionHandoff } from '@/app/credential-groups/complete/completion-handoff'
78
import { SlackCompletion } from '@/app/credential-groups/slack-complete/slack-completion'
@@ -63,7 +64,11 @@ function SourceConnectFixture() {
6364
<output aria-label='Account authorization'>{accountConnection.status}</output>
6465
<output aria-label='Account error'>{accountConnection.error?.message}</output>
6566
<output aria-label='Reconnect status'>{reconnect.status}</output>
66-
<output aria-label='Account count'>{accounts.data?.viewerAccounts?.length ?? 0}</output>
67+
<output aria-label='Reconnect error'>{reconnect.error?.message}</output>
68+
<output aria-label='Account count'>
69+
{(accounts.data?.viewerAccounts?.length ?? 0) +
70+
(accounts.data?.viewerMcpAccounts?.length ?? 0)}
71+
</output>
6772
<input aria-label='Source draft' defaultValue='Unsubmitted source name' />
6873
<button
6974
disabled={connection.isPending}
@@ -118,6 +123,8 @@ function BrowserLauncher() {
118123
}
119124

120125
const params = new URLSearchParams(location.search)
126+
const oauth = params.get('oauth')
127+
const failure = oauth === null ? undefined : isCredentialGroupOAuthFailure(oauth) ? oauth : 'failed'
121128
const content =
122129
location.pathname === '/credential-groups/enroll/fixture-invitation' ? (
123130
params.has('connected') ? (
@@ -130,7 +137,7 @@ const content =
130137
) : location.pathname === '/credential-groups/complete' ? (
131138
<CredentialGroupCompletionHandoff
132139
completionId={params.get('completionId')!}
133-
failure={params.get('oauth') === 'denied' ? 'denied' : undefined}
140+
failure={failure}
134141
returnHref={params.has('organizationId') ? '/o/fixture-organization/integrations' : undefined}
135142
/>
136143
) : location.pathname === '/desktop/connect' ? (

0 commit comments

Comments
 (0)