Skip to content

Commit 08692a4

Browse files
committed
fix(integrations): complete account connections in place
1 parent b84828d commit 08692a4

26 files changed

Lines changed: 616 additions & 87 deletions

File tree

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

Lines changed: 148 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,8 @@ 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, string>()
48+
let accountConnected = false
4549
const startSessions: string[] = []
4650
const callbackSessions: string[] = []
4751
const githubAttempts = new Map<string, { session: string; completed: boolean }>()
@@ -50,6 +54,7 @@ test('source authorization returns to its desktop screen and refreshes live', as
5054
let nativeCredentialVisible = false
5155
let installed = false
5256
let javascript = ''
57+
let stylesheet = ''
5358
let origin = ''
5459
let app: Awaited<ReturnType<typeof electron.launch>> | undefined
5560
let browser: Awaited<ReturnType<typeof chromium.launch>> | undefined
@@ -73,9 +78,64 @@ test('source authorization returns to its desktop screen and refreshes live', as
7378
for await (const chunk of request) text += chunk.toString()
7479
return JSON.parse(text)
7580
}
76-
if (path === '/fixture.js') {
77-
response.setHeader('content-type', 'text/javascript')
78-
response.end(javascript)
81+
if (path === '/fixture.js' || path === '/fixture.css') {
82+
response.setHeader('content-type', path.endsWith('.js') ? 'text/javascript' : 'text/css')
83+
response.end(path.endsWith('.js') ? javascript : stylesheet)
84+
return
85+
}
86+
if (path === '/api/organizations/fixture-organization/connected-accounts') {
87+
json({
88+
credentialGroup: null,
89+
availableProviders: [],
90+
availableMcpConnectors: [],
91+
canManage: false,
92+
indexingAvailable: true,
93+
viewerAccounts: accountConnected
94+
? [
95+
{
96+
credentialId: 'fixture-account',
97+
displayName: 'Fixture account',
98+
providerId: 'google-drive',
99+
groupId: 'fixture-group',
100+
optionId: 'fixture-option',
101+
status: 'active',
102+
},
103+
]
104+
: [],
105+
})
106+
return
107+
}
108+
if (
109+
path === '/api/organizations/fixture-organization/connected-accounts/connect' ||
110+
path === '/api/users/me/organization-accounts/fixture-account/reconnect'
111+
) {
112+
const completionId =
113+
request.method === 'POST' && path.endsWith('/connect')
114+
? (await body()).oauthCompletionId
115+
: url.searchParams.get('oauthCompletionId')
116+
if (!completionId) {
117+
json({ error: 'Missing completion ID' }, 400)
118+
return
119+
}
120+
accountAttempts.set(completionId, session)
121+
json({
122+
invitationLink: `${origin}/credential-groups/enroll/fixture-account-invitation`,
123+
authorizationUrl: `${origin}/account-provider?completionId=${completionId}`,
124+
})
125+
return
126+
}
127+
if (path === '/account-callback') {
128+
const completionId = url.searchParams.get('completionId') ?? ''
129+
if (accountAttempts.get(completionId) !== session) {
130+
json({ error: 'Wrong attempt' }, 403)
131+
return
132+
}
133+
accountAttempts.delete(completionId)
134+
const denied = url.searchParams.has('error')
135+
if (!denied) accountConnected = true
136+
redirect(
137+
`/credential-groups/complete?completionId=${completionId}&organizationId=fixture-organization${denied ? '&oauth=denied' : ''}`
138+
)
79139
return
80140
}
81141
if (path === '/api/auth/get-session') {
@@ -217,6 +277,14 @@ test('source authorization returns to its desktop screen and refreshes live', as
217277
return
218278
}
219279
response.setHeader('content-type', 'text/html')
280+
if (path === '/account-provider') {
281+
response.setHeader('Cross-Origin-Opener-Policy', 'same-origin')
282+
const completionId = url.searchParams.get('completionId') ?? ''
283+
response.end(
284+
`<!doctype html><a href="/account-callback?completionId=${completionId}">Authorize account</a><a href="/account-callback?completionId=${completionId}&error=denied">Deny account</a>`
285+
)
286+
return
287+
}
220288
if (path === '/github-provider') {
221289
response.end(
222290
`<!doctype html><a href="/github-callback?setupId=${url.searchParams.get('setupId')}">Authorize GitHub</a>`
@@ -239,10 +307,18 @@ test('source authorization returns to its desktop screen and refreshes live', as
239307
'set-cookie',
240308
'better-auth.session_token=desktop-fixture; HttpOnly; SameSite=Lax; Path=/'
241309
)
242-
response.end('<!doctype html><div id="root"></div><script src="/fixture.js"></script>')
310+
response.end(
311+
'<!doctype html><html><head><link rel="stylesheet" href="/fixture.css"></head><body><div id="root"></div><script src="/fixture.js"></script></body></html>'
312+
)
243313
})
244314
try {
245315
await check('launch the production source hook and native bridge', async () => {
316+
const config = await loadPostcssConfig({}, SIM_DIR)
317+
const cssPath = join(SIM_DIR, 'app/_styles/globals.css')
318+
const css = await postcss(config.plugins).process(
319+
`${readFileSync(cssPath, 'utf8')}\n@source ${JSON.stringify(FIXTURE)};`,
320+
{ from: cssPath }
321+
)
246322
const bundle = await build({
247323
entryPoints: [FIXTURE],
248324
bundle: true,
@@ -257,6 +333,7 @@ test('source authorization returns to its desktop screen and refreshes live', as
257333
define: { 'process.env.NODE_ENV': '"development"' },
258334
})
259335
javascript = bundle.outputFiles.find((file) => file.path.endsWith('.js'))?.text ?? ''
336+
stylesheet = `${css.css}\n${bundle.outputFiles.find((file) => file.path.endsWith('.css'))?.text ?? ''}`
260337
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
261338
const address = server.address()
262339
if (!address || typeof address === 'string') throw new Error('Missing fixture address')
@@ -400,6 +477,72 @@ test('source authorization returns to its desktop screen and refreshes live', as
400477
await expect(page.getByRole('alert')).toContainText('Sign in to Sim in your browser')
401478
expect(page.url()).toBe(`${origin}/home`)
402479
})
480+
await check('managed accounts return through the desktop completion handoff', async () => {
481+
await page.getByRole('button', { name: 'Connect MCP account', exact: true }).click()
482+
await expect.poll(async () => (await opened()).length).toBe(9)
483+
await external.goto((await opened())[8])
484+
await external.getByRole('link', { name: 'Authorize account' }).click()
485+
await expect(page.getByLabel('Account authorization', { exact: true })).toHaveText('success')
486+
await expect(page.getByLabel('Account count')).toHaveText('1')
487+
expect(page.url()).toBe(`${origin}/home`)
488+
await expect(page.getByLabel('Source draft')).toHaveValue('Preserved while connecting')
489+
})
490+
const web = await context.newPage()
491+
web.on('pageerror', (error) => pageErrors.push(error.message))
492+
await web.goto(`${origin}/o/fixture-organization/integrations?search=fixture`)
493+
await check(
494+
'web authorization preserves the origin and refreshes after an isolated provider window',
495+
async () => {
496+
accountConnected = false
497+
await web.reload()
498+
await web.getByLabel('Source draft').fill('Web draft retained')
499+
await expect(web.getByLabel('Account count')).toHaveText('0')
500+
const popupReady = context.waitForEvent('page')
501+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
502+
const popup = await popupReady
503+
await popup.getByRole('link', { name: 'Authorize account' }).click()
504+
await expect(web.getByLabel('Account count')).toHaveText('1')
505+
await expect(web.getByLabel('Account authorization', { exact: true })).toHaveText('success')
506+
await expect(web.getByLabel('Source draft')).toHaveValue('Web draft retained')
507+
expect(web.url()).toBe(`${origin}/o/fixture-organization/integrations?search=fixture`)
508+
}
509+
)
510+
await check('web denial and cancellation leave the initiating page usable', async () => {
511+
const popupReady = context.waitForEvent('page')
512+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
513+
const popup = await popupReady
514+
await popup.getByRole('link', { name: 'Deny account' }).click()
515+
await expect(web.getByLabel('Account error')).toContainText('canceled')
516+
await popup.close()
517+
await expect(web.getByRole('button', { name: 'Cancel', exact: true })).toHaveCount(0)
518+
const nextPopupReady = context.waitForEvent('page')
519+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
520+
const nextPopup = await nextPopupReady
521+
await nextPopup.getByRole('link', { name: 'Authorize account' }).waitFor()
522+
await expect(web.getByRole('button', { name: 'Cancel', exact: true })).toHaveCount(1)
523+
await web.getByRole('button', { name: 'Cancel', exact: true }).click()
524+
expect(pageErrors).toEqual([])
525+
await expect(web.getByLabel('Account error')).toContainText('canceled')
526+
await expect(web.getByRole('button', { name: 'Connect account', exact: true })).toBeEnabled()
527+
await expect(web.getByLabel('Account count')).toHaveText('1')
528+
})
529+
await check('reconnect uses the same completion lifecycle', async () => {
530+
const popupReady = context.waitForEvent('page')
531+
await web.getByRole('button', { name: 'Reconnect account', exact: true }).click()
532+
const popup = await popupReady
533+
await popup.getByRole('link', { name: 'Authorize account' }).click()
534+
await expect(web.getByLabel('Reconnect status')).toHaveText('success')
535+
await expect(web.getByLabel('Source draft')).toHaveValue('Web draft retained')
536+
})
537+
await check('blocked popups complete in the same tab and return to Integrations', async () => {
538+
await web.evaluate(() => {
539+
window.open = () => null
540+
})
541+
await web.getByRole('button', { name: 'Connect account', exact: true }).click()
542+
await web.getByRole('link', { name: 'Authorize account' }).click()
543+
await expect(web).toHaveURL(`${origin}/o/fixture-organization/integrations`)
544+
await expect(web.getByLabel('Account count')).toHaveText('1')
545+
})
403546
await page.screenshot({ path: test.info().outputPath('source-connect-desktop.png') })
404547
} finally {
405548
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'

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

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
isCredentialGroupMcpOAuthState,
1818
} from '@/lib/credential-groups/mcp-oauth-state'
1919
import { CredentialGroupOAuthStateVersionError } from '@/lib/credential-groups/oauth-attempt-version'
20+
import type { CredentialGroupOAuthFailure } from '@/lib/credential-groups/oauth-completion'
2021
import { enforcePublicCredentialGroupIpRateLimit } from '@/lib/credential-groups/rate-limit'
2122
import {
2223
assertSafeOauthServerUrl,
@@ -30,7 +31,10 @@ import {
3031
SimMcpOauthProvider,
3132
} from '@/lib/mcp/oauth'
3233
import { mcpService } from '@/lib/mcp/service'
33-
import { createCredentialGroupEnrollmentRedirect } from '@/app/api/credential-groups/enrollment-redirect'
34+
import {
35+
createCredentialGroupCompletionRedirect,
36+
createCredentialGroupEnrollmentRedirect,
37+
} from '@/app/api/credential-groups/enrollment-redirect'
3438

3539
const logger = createLogger('McpOauthCallbackAPI')
3640
const timedStep = makeTimedStep(logger)
@@ -98,13 +102,17 @@ async function completeManagedMcpCallback(params: {
98102
if (!attempt) {
99103
return htmlClose('Invalid or expired authorization state.', false, 'invalid_state')
100104
}
101-
if (params.error) {
102-
return createCredentialGroupEnrollmentRedirect(attempt.invitationToken, { oauth: 'denied' })
103-
}
105+
const failureRedirect = (oauth: CredentialGroupOAuthFailure) =>
106+
attempt.completionId
107+
? createCredentialGroupCompletionRedirect(
108+
oauth,
109+
attempt.completionId,
110+
attempt.returnTo === 'integrations' ? attempt.organizationId : undefined
111+
)
112+
: createCredentialGroupEnrollmentRedirect(attempt.invitationToken, { oauth })
113+
if (params.error) return failureRedirect('denied')
104114
if (!params.code) {
105-
return createCredentialGroupEnrollmentRedirect(attempt.invitationToken, {
106-
oauth: 'failed',
107-
})
115+
return failureRedirect('failed')
108116
}
109117
try {
110118
const principal = await credentialGroupOAuthAttemptPrincipal(attempt)
@@ -113,13 +121,19 @@ async function completeManagedMcpCallback(params: {
113121
input: { attempt, code: params.code },
114122
request: params.request,
115123
})
124+
if (attempt.completionId)
125+
return createCredentialGroupCompletionRedirect(
126+
undefined,
127+
attempt.completionId,
128+
attempt.returnTo === 'integrations' ? attempt.organizationId : undefined
129+
)
116130
return createCredentialGroupEnrollmentRedirect(attempt.invitationToken, {
117131
mcp: 'connected',
118132
mcpServerId: result.mcpServerId,
119133
})
120134
} catch (error) {
121135
logger.error('Managed MCP OAuth callback failed', error)
122-
return createCredentialGroupEnrollmentRedirect(attempt.invitationToken, { oauth: 'failed' })
136+
return failureRedirect('failed')
123137
}
124138
}
125139

0 commit comments

Comments
 (0)