Skip to content

Commit 517a5f3

Browse files
authored
fix(integrations): complete account connections in place (#8493)
* fix(integrations): complete account connections in place * fix(integrations): preserve active account authorizations
1 parent f363541 commit 517a5f3

27 files changed

Lines changed: 657 additions & 114 deletions

File tree

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

Lines changed: 175 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
1+
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
22
import { createServer } from 'node:http'
33
import { tmpdir } from 'node:os'
44
import { dirname, join } from 'node:path'
@@ -8,6 +8,8 @@ import { getErrorMessage } from '@sim/utils/errors'
88
import { sleep } from '@sim/utils/helpers'
99
import { generateShortId } from '@sim/utils/id'
1010
import { build } from 'esbuild'
11+
import postcss from 'postcss'
12+
import loadPostcssConfig from 'postcss-load-config'
1113

1214
const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url))
1315
const SIM_DIR = fileURLToPath(new URL('../../sim/', import.meta.url))
@@ -42,6 +44,9 @@ test('source authorization returns to its desktop screen and refreshes live', as
4244
}
4345
const tickets = new Map<string, unknown>()
4446
const attempts = new Map<string, string>()
47+
const accountAttempts = new Map<string, { session: string; mcp: boolean }>()
48+
let accountConnected = false
49+
let mcpAccountConnected = false
4550
const startSessions: string[] = []
4651
const callbackSessions: string[] = []
4752
const githubAttempts = new Map<string, { session: string; completed: boolean }>()
@@ -50,6 +55,7 @@ test('source authorization returns to its desktop screen and refreshes live', as
5055
let nativeCredentialVisible = false
5156
let installed = false
5257
let javascript = ''
58+
let stylesheet = ''
5359
let origin = ''
5460
let app: Awaited<ReturnType<typeof electron.launch>> | undefined
5561
let browser: Awaited<ReturnType<typeof chromium.launch>> | undefined
@@ -73,9 +79,76 @@ test('source authorization returns to its desktop screen and refreshes live', as
7379
for await (const chunk of request) text += chunk.toString()
7480
return JSON.parse(text)
7581
}
76-
if (path === '/fixture.js') {
77-
response.setHeader('content-type', 'text/javascript')
78-
response.end(javascript)
82+
if (path === '/fixture.js' || path === '/fixture.css') {
83+
response.setHeader('content-type', path.endsWith('.js') ? 'text/javascript' : 'text/css')
84+
response.end(path.endsWith('.js') ? javascript : stylesheet)
85+
return
86+
}
87+
if (path === '/api/organizations/fixture-organization/connected-accounts') {
88+
json({
89+
credentialGroup: null,
90+
availableProviders: [],
91+
availableMcpConnectors: [],
92+
canManage: false,
93+
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+
: [],
104+
viewerAccounts: accountConnected
105+
? [
106+
{
107+
credentialId: 'fixture-account',
108+
displayName: 'Fixture account',
109+
providerId: 'google-drive',
110+
groupId: 'fixture-group',
111+
optionId: 'fixture-option',
112+
status: 'active',
113+
},
114+
]
115+
: [],
116+
})
117+
return
118+
}
119+
if (
120+
path === '/api/organizations/fixture-organization/connected-accounts/connect' ||
121+
path === '/api/users/me/organization-accounts/fixture-account/reconnect'
122+
) {
123+
const input = request.method === 'POST' && path.endsWith('/connect') ? await body() : null
124+
const completionId = input?.oauthCompletionId ?? url.searchParams.get('oauthCompletionId')
125+
if (!completionId) {
126+
json({ error: 'Missing completion ID' }, 400)
127+
return
128+
}
129+
accountAttempts.set(completionId, { session, mcp: Boolean(input?.mcpServerId) })
130+
json({
131+
invitationLink: `${origin}/credential-groups/enroll/fixture-account-invitation`,
132+
authorizationUrl: `${origin}/account-provider?completionId=${completionId}`,
133+
})
134+
return
135+
}
136+
if (path === '/account-callback') {
137+
const completionId = url.searchParams.get('completionId') ?? ''
138+
const attempt = accountAttempts.get(completionId)
139+
if (attempt?.session !== session) {
140+
json({ error: 'Wrong attempt' }, 403)
141+
return
142+
}
143+
accountAttempts.delete(completionId)
144+
const denied = url.searchParams.has('error')
145+
if (!denied) {
146+
if (attempt.mcp) mcpAccountConnected = true
147+
else accountConnected = true
148+
}
149+
redirect(
150+
`/credential-groups/complete?completionId=${completionId}&organizationId=fixture-organization${denied ? '&oauth=denied' : ''}`
151+
)
79152
return
80153
}
81154
if (path === '/api/auth/get-session') {
@@ -217,6 +290,14 @@ test('source authorization returns to its desktop screen and refreshes live', as
217290
return
218291
}
219292
response.setHeader('content-type', 'text/html')
293+
if (path === '/account-provider') {
294+
response.setHeader('Cross-Origin-Opener-Policy', 'same-origin')
295+
const completionId = url.searchParams.get('completionId') ?? ''
296+
response.end(
297+
`<!doctype html><a href="/account-callback?completionId=${completionId}">Authorize account</a><a href="/account-callback?completionId=${completionId}&error=denied">Deny account</a>`
298+
)
299+
return
300+
}
220301
if (path === '/github-provider') {
221302
response.end(
222303
`<!doctype html><a href="/github-callback?setupId=${url.searchParams.get('setupId')}">Authorize GitHub</a>`
@@ -239,10 +320,18 @@ test('source authorization returns to its desktop screen and refreshes live', as
239320
'set-cookie',
240321
'better-auth.session_token=desktop-fixture; HttpOnly; SameSite=Lax; Path=/'
241322
)
242-
response.end('<!doctype html><div id="root"></div><script src="/fixture.js"></script>')
323+
response.end(
324+
'<!doctype html><html><head><link rel="stylesheet" href="/fixture.css"></head><body><div id="root"></div><script src="/fixture.js"></script></body></html>'
325+
)
243326
})
244327
try {
245328
await check('launch the production source hook and native bridge', async () => {
329+
const config = await loadPostcssConfig({}, SIM_DIR)
330+
const cssPath = join(SIM_DIR, 'app/_styles/globals.css')
331+
const css = await postcss(config.plugins).process(
332+
`${readFileSync(cssPath, 'utf8')}\n@source ${JSON.stringify(FIXTURE)};`,
333+
{ from: cssPath }
334+
)
246335
const bundle = await build({
247336
entryPoints: [FIXTURE],
248337
bundle: true,
@@ -257,6 +346,7 @@ test('source authorization returns to its desktop screen and refreshes live', as
257346
define: { 'process.env.NODE_ENV': '"development"' },
258347
})
259348
javascript = bundle.outputFiles.find((file) => file.path.endsWith('.js'))?.text ?? ''
349+
stylesheet = `${css.css}\n${bundle.outputFiles.find((file) => file.path.endsWith('.css'))?.text ?? ''}`
260350
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
261351
const address = server.address()
262352
if (!address || typeof address === 'string') throw new Error('Missing fixture address')
@@ -400,6 +490,86 @@ test('source authorization returns to its desktop screen and refreshes live', as
400490
await expect(page.getByRole('alert')).toContainText('Sign in to Sim in your browser')
401491
expect(page.url()).toBe(`${origin}/home`)
402492
})
493+
await check('managed accounts return through the desktop completion handoff', async () => {
494+
await page.getByRole('button', { name: 'Connect MCP account', exact: true }).click()
495+
await expect.poll(async () => (await opened()).length).toBe(9)
496+
await external.goto((await opened())[8])
497+
await external.getByRole('link', { name: 'Authorize account' }).click()
498+
await expect(page.getByLabel('Account authorization', { exact: true })).toHaveText('success')
499+
await expect(page.getByLabel('Account count')).toHaveText('1')
500+
expect(page.url()).toBe(`${origin}/home`)
501+
await expect(page.getByLabel('Source draft')).toHaveValue('Preserved while connecting')
502+
})
503+
const web = await context.newPage()
504+
web.on('pageerror', (error) => pageErrors.push(error.message))
505+
await web.goto(`${origin}/o/fixture-organization/integrations?search=fixture`)
506+
await check(
507+
'web authorization preserves the origin and refreshes after an isolated provider window',
508+
async () => {
509+
accountConnected = false
510+
mcpAccountConnected = false
511+
await web.reload()
512+
await web.getByLabel('Source draft').fill('Web draft retained')
513+
await expect(web.getByLabel('Account count')).toHaveText('0')
514+
const popupReady = context.waitForEvent('page')
515+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
516+
const popup = await popupReady
517+
await popup.getByRole('link', { name: 'Authorize account' }).click()
518+
await expect(web.getByLabel('Account count')).toHaveText('1')
519+
await expect(web.getByLabel('Account authorization', { exact: true })).toHaveText('success')
520+
await expect(web.getByLabel('Source draft')).toHaveValue('Web draft retained')
521+
expect(web.url()).toBe(`${origin}/o/fixture-organization/integrations?search=fixture`)
522+
}
523+
)
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+
})
537+
await check('web denial and cancellation leave the initiating page usable', async () => {
538+
const popupReady = context.waitForEvent('page')
539+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
540+
const popup = await popupReady
541+
await popup.getByRole('link', { name: 'Deny account' }).click()
542+
await expect(web.getByLabel('Account error')).toContainText('canceled')
543+
await popup.close()
544+
await expect(web.getByRole('button', { name: 'Cancel', exact: true })).toHaveCount(0)
545+
const nextPopupReady = context.waitForEvent('page')
546+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
547+
const nextPopup = await nextPopupReady
548+
await nextPopup.getByRole('link', { name: 'Authorize account' }).waitFor()
549+
await expect(web.getByRole('button', { name: 'Cancel', exact: true })).toHaveCount(1)
550+
await web.getByRole('button', { name: 'Cancel', exact: true }).click()
551+
expect(pageErrors).toEqual([])
552+
await expect(web.getByLabel('Account error')).toContainText('canceled')
553+
await expect(web.getByRole('button', { name: 'Connect account', exact: true })).toBeEnabled()
554+
await expect(web.getByLabel('Account count')).toHaveText('1')
555+
})
556+
await check('reconnect uses the same completion lifecycle', async () => {
557+
const popupReady = context.waitForEvent('page')
558+
await web.getByRole('button', { name: 'Reconnect account', exact: true }).click()
559+
const popup = await popupReady
560+
await popup.getByRole('link', { name: 'Authorize account' }).click()
561+
await expect(web.getByLabel('Reconnect status')).toHaveText('success')
562+
await expect(web.getByLabel('Source draft')).toHaveValue('Web draft retained')
563+
})
564+
await check('blocked popups complete in the same tab and return to Integrations', async () => {
565+
await web.evaluate(() => {
566+
window.open = () => null
567+
})
568+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
569+
await web.getByRole('link', { name: 'Authorize account' }).click()
570+
await expect(web).toHaveURL(`${origin}/o/fixture-organization/integrations`)
571+
await expect(web.getByLabel('Account count')).toHaveText('1')
572+
})
403573
await page.screenshot({ path: test.info().outputPath('source-connect-desktop.png') })
404574
} finally {
405575
mkdirSync(dirname(reportPath), { recursive: true })

‎apps/sim/app/api/credential-groups/enrollment-redirect.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ export function createCredentialGroupEnrollmentRedirect(
2323

2424
export function createCredentialGroupCompletionRedirect(
2525
oauth?: CredentialGroupOAuthFailure,
26-
completionId?: string
26+
completionId?: string,
27+
organizationId?: string
2728
): NextResponse {
2829
const query = new URLSearchParams()
2930
if (oauth) query.set('oauth', oauth)
3031
if (completionId) query.set('completionId', completionId)
32+
if (organizationId) query.set('organizationId', organizationId)
3133
return new NextResponse(null, {
3234
status: 303,
3335
headers: {

‎apps/sim/app/api/credential-groups/oauth-callback.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,3 +238,33 @@ describe('GitHub installation setup OAuth return target', () => {
238238
expect(url.searchParams.get('setupId')).toBe(completionId)
239239
})
240240
})
241+
242+
describe('Integrations OAuth completion', () => {
243+
it.each([undefined, 'denied'])(
244+
'returns the originating organization on completion: %s',
245+
async (error) => {
246+
mocks.consumeAttempt.mockResolvedValueOnce({
247+
...attempt,
248+
returnTo: 'integrations',
249+
organizationId: 'organization-1',
250+
completionRedirect: true,
251+
completionId,
252+
})
253+
mocks.authenticate.mockResolvedValueOnce({ kind: 'credential_group_enrollment' })
254+
mocks.completeOAuth.mockResolvedValueOnce({ connectedOptionId: 'option-1' })
255+
const response = await handleCredentialGroupOAuthCallback({
256+
request: createMockRequest({
257+
url: 'https://sim.test/api/auth/oauth2/callback/github-repositories',
258+
}),
259+
provider: 'github-repositories',
260+
query: { state: 'cg_state', code: 'code-1', ...(error ? { error } : {}) },
261+
limited: null,
262+
})
263+
const destination = new URL(response.headers.get('location')!, 'https://sim.test')
264+
expect(destination.pathname).toBe('/credential-groups/complete')
265+
expect(destination.searchParams.get('completionId')).toBe(completionId)
266+
expect(destination.searchParams.get('organizationId')).toBe('organization-1')
267+
expect(destination.searchParams.get('oauth')).toBe(error ?? null)
268+
}
269+
)
270+
})

‎apps/sim/app/api/credential-groups/oauth-callback.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,13 @@ export async function handleCredentialGroupOAuthCallback({
8484
})
8585
const installationSetup =
8686
attempt.returnTo === 'github-installation' && attempt.organizationId && attempt.completionId
87+
const returnOrganizationId =
88+
attempt.returnTo === 'integrations' ? attempt.organizationId : undefined
8789
const failureRedirect = (oauth: CredentialGroupOAuthFailure) =>
8890
installationSetup
8991
? setupRedirect(oauth)
9092
: attempt.completionRedirect
91-
? createCredentialGroupCompletionRedirect(oauth, attempt.completionId)
93+
? createCredentialGroupCompletionRedirect(oauth, attempt.completionId, returnOrganizationId)
9294
: createCredentialGroupEnrollmentRedirect(attempt.invitationToken, { ...focus, oauth })
9395
if (limited) {
9496
return failureRedirect('rate_limited')
@@ -117,7 +119,11 @@ export async function handleCredentialGroupOAuthCallback({
117119
request,
118120
})
119121
return attempt.completionRedirect
120-
? createCredentialGroupCompletionRedirect(undefined, attempt.completionId)
122+
? createCredentialGroupCompletionRedirect(
123+
undefined,
124+
attempt.completionId,
125+
returnOrganizationId
126+
)
121127
: createCredentialGroupEnrollmentRedirect(attempt.invitationToken, {
122128
...focus,
123129
connected: attempt.optionId,

‎apps/sim/app/api/mcp/oauth/callback/route.test.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ vi.mock('@/lib/credential-groups/rate-limit', () => ({
3838
enforcePublicCredentialGroupIpRateLimit: mockEnforceCallbackRateLimit,
3939
}))
4040

41-
import { GET } from './route'
41+
import { GET } from '@/app/api/mcp/oauth/callback/route'
4242

4343
const { mockDiscoverServerTools } = mcpServiceMockFns
4444

@@ -88,6 +88,31 @@ describe('MCP OAuth callback route', () => {
8888
mockEnforceCallbackRateLimit.mockResolvedValue(null)
8989
})
9090

91+
it.each([undefined, 'denied'])(
92+
'finishes a direct connection without the invitation form: %s',
93+
async (error) => {
94+
const completionId = '00000000-0000-4000-8000-000000000002'
95+
mockConsumeManagedAttempt.mockResolvedValueOnce({
96+
state: 'mcp_cg_direct',
97+
organizationId: 'organization-1',
98+
invitationToken: 'invitation-token',
99+
mcpServerId: 'server-1',
100+
completionId,
101+
returnTo: 'integrations',
102+
})
103+
const response = await GET(
104+
new NextRequest(
105+
`http://localhost:3000/api/mcp/oauth/callback?state=mcp_cg_direct&${error ? 'error=denied' : 'code=code-1'}`
106+
)
107+
)
108+
const destination = new URL(response.headers.get('location')!, 'http://localhost:3000')
109+
expect(destination.pathname).toBe('/credential-groups/complete')
110+
expect(destination.searchParams.get('completionId')).toBe(completionId)
111+
expect(destination.searchParams.get('organizationId')).toBe('organization-1')
112+
expect(destination.searchParams.get('oauth')).toBe(error ?? null)
113+
}
114+
)
115+
91116
it('performs the token exchange through the SSRF-guarded mcpAuthGuarded wrapper', async () => {
92117
const request = new NextRequest(
93118
'http://localhost:3000/api/mcp/oauth/callback?state=state-1&code=auth-code-1'

0 commit comments

Comments
 (0)