Skip to content

Commit 47e785c

Browse files
committed
fix(oauth): confirm workspace connect links before starting the connection
1 parent 776c33a commit 47e785c

9 files changed

Lines changed: 269 additions & 260 deletions

File tree

‎apps/sim/app/(auth)/auth-redirect.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ export function resolveAuthRedirect({ redirect, callbackUrl, inviteFlow }: AuthR
8181

8282
/**
8383
* OAuth reauthentication must reach the form even when an older session cookie exists.
84-
* Only an OAuth client's request qualifies: a workspace account launch on the same path
85-
* would otherwise turn a cross-site link into a same-origin request after sign-in.
84+
* Only an OAuth client's request qualifies: a workspace connect link on the same path
85+
* needs no fresh sign-in, so a signed-in user continues to the app instead.
8686
*/
8787
export function isOAuthAuthorizationCallback(callbackUrl: string, origin: string): boolean {
8888
if (!callbackUrl) return false

‎apps/sim/app/api/auth/instagram/authorize/route.test.ts‎

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,15 @@ import { urlsMockFns } from '@sim/testing/mocks/urls.mock'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77

88
const mocks = vi.hoisted(() => ({
9-
createCredentialConnection: vi.fn(),
109
requireConfiguredOAuthClient: vi.fn(),
1110
}))
1211

1312
vi.mock('@/lib/core/config/env-capabilities.server', () => ({
1413
requireConfiguredOAuthClient: mocks.requireConfiguredOAuthClient,
1514
}))
1615

17-
vi.mock('@/lib/credentials/application/create-credential-connection', () => ({
18-
createCredentialConnection: { execute: mocks.createCredentialConnection },
19-
}))
20-
2116
vi.mock('@/lib/oauth/utils', () => oauthUtilsMock)
2217

23-
import { OrchestrationError } from '@/lib/core/orchestration/types'
2418
import { GET } from '@/app/api/auth/instagram/authorize/route'
2519

2620
urlsMockFns.mockGetBaseUrl.mockReturnValue('https://sim.test')
@@ -35,7 +29,6 @@ describe('Instagram authorize route', () => {
3529
mocks.requireConfiguredOAuthClient.mockReturnValue({
3630
values: { INSTAGRAM_CLIENT_ID: 'instagram-client' },
3731
})
38-
mocks.createCredentialConnection.mockResolvedValue({ draftId: 'draft-created' })
3932
})
4033

4134
it('preserves an exact credential draft when workspaceId is also supplied', async () => {
@@ -52,42 +45,21 @@ describe('Instagram authorize route', () => {
5245
expect(response.headers.get('set-cookie')).toContain(
5346
'instagram_credential_draft_id=draft-exact'
5447
)
55-
expect(mocks.createCredentialConnection).not.toHaveBeenCalled()
5648
})
5749

58-
it('refuses a cross-site workspace launch before creating a connection draft', async () => {
50+
it('sends a workspace connect link to the confirmation page without starting it', async () => {
5951
const request = createMockRequest(
6052
'GET',
6153
undefined,
62-
{ 'sec-fetch-site': 'cross-site' },
63-
'https://sim.test/api/auth/instagram/authorize?workspaceId=workspace-1'
54+
{ 'sec-fetch-site': 'same-origin' },
55+
'https://sim.test/api/auth/instagram/authorize?workspaceId=workspace-1&returnUrl=https%3A%2F%2Fsim.test%2Fhome'
6456
)
6557

6658
const response = await GET(request, createRouteContext({}))
6759

68-
expect(response.status).toBe(403)
69-
expect(mocks.createCredentialConnection).not.toHaveBeenCalled()
70-
})
71-
72-
it('returns a conflict when a different connection intent is already active', async () => {
73-
mocks.createCredentialConnection.mockRejectedValue(
74-
new OrchestrationError(
75-
'conflict',
76-
'A different OAuth connection flow is already active for this provider'
77-
)
60+
expect(response.headers.get('location')).toBe(
61+
'https://sim.test/oauth/connect?providerId=instagram&workspaceId=workspace-1&callbackURL=https%3A%2F%2Fsim.test%2Fhome'
7862
)
79-
const request = createMockRequest(
80-
'GET',
81-
undefined,
82-
{},
83-
'https://sim.test/api/auth/instagram/authorize?workspaceId=workspace-1'
84-
)
85-
86-
const response = await GET(request, createRouteContext({}))
87-
88-
expect(response.status).toBe(409)
89-
await expect(response.json()).resolves.toEqual({
90-
error: 'A different OAuth connection flow is already active for this provider',
91-
})
63+
expect(response.headers.get('set-cookie')).toBeNull()
9264
})
9365
})

‎apps/sim/app/api/auth/instagram/authorize/route.ts‎

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,9 @@ import { authorizeInstagramContract } from '@/lib/api/contracts/oauth-connection
55
import { parseRequest } from '@/lib/api/server'
66
import { getSession } from '@/lib/auth'
77
import { requireConfiguredOAuthClient } from '@/lib/core/config/env-capabilities.server'
8-
import { asOrchestrationError } from '@/lib/core/orchestration/types'
9-
import { isCrossSiteSessionRequest } from '@/lib/core/security/same-origin'
108
import { getBaseUrl } from '@/lib/core/utils/urls'
119
import { isSameOrigin } from '@/lib/core/utils/validation'
1210
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
13-
import { createCredentialConnection } from '@/lib/credentials/application/create-credential-connection'
1411
import { CREDENTIAL_DRAFT_TTL_SECONDS } from '@/lib/credentials/draft-constants'
1512
import { getCanonicalScopesForProvider } from '@/lib/oauth/utils'
1613

@@ -29,8 +26,6 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
2926
if (!session?.user?.id) {
3027
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
3128
}
32-
const sessionId = session.session?.id
33-
if (!sessionId) throw new Error('Authenticated session is missing its session ID')
3429

3530
const {
3631
values: { INSTAGRAM_CLIENT_ID: clientId },
@@ -39,39 +34,18 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
3934
const parsed = await parseRequest(authorizeInstagramContract, request, {})
4035
if (!parsed.success) return parsed.response
4136
const { returnUrl, workspaceId, draftId } = parsed.data.query
42-
let credentialDraftId = draftId
37+
const baseUrl = getBaseUrl()
4338

39+
// Same gesture gate as the shared authorize route: a workspace link opens the
40+
// confirmation page, which creates the draft on the user's click.
4441
if (workspaceId && !draftId) {
45-
// The query alone picks the workspace, so a cross-site page must not be
46-
// able to start this launch with the viewer's session cookie.
47-
if (isCrossSiteSessionRequest(request)) {
48-
logger.warn('Rejected cross-site Instagram workspace launch', { userId: session.user.id })
49-
return NextResponse.json({ error: 'Cross-site launch refused' }, { status: 403 })
50-
}
51-
try {
52-
const connection = await createCredentialConnection.execute({
53-
principal: { kind: 'session', userId: session.user.id, sessionId },
54-
input: { workspaceId, providerId: 'instagram' },
55-
request,
56-
})
57-
credentialDraftId = connection.draftId
58-
} catch (error) {
59-
const classified = asOrchestrationError(error)
60-
if (classified?.code === 'conflict') {
61-
logger.warn('Rejected conflicting Instagram OAuth connection intent', {
62-
userId: session.user.id,
63-
workspaceId,
64-
})
65-
return NextResponse.json({ error: classified.message }, { status: 409 })
66-
}
67-
if (classified?.code === 'forbidden' || classified?.code === 'not_found') {
68-
return NextResponse.json({ error: 'Workspace write access denied' }, { status: 403 })
69-
}
70-
throw error
71-
}
42+
const confirmUrl = new URL('/oauth/connect', baseUrl)
43+
confirmUrl.searchParams.set('providerId', 'instagram')
44+
confirmUrl.searchParams.set('workspaceId', workspaceId)
45+
if (returnUrl) confirmUrl.searchParams.set('callbackURL', returnUrl)
46+
return NextResponse.redirect(confirmUrl)
7247
}
7348

74-
const baseUrl = getBaseUrl()
7549
const state = generateShortId(32)
7650
const redirectUri = `${baseUrl}/api/auth/oauth2/callback/instagram`
7751
const scope = getCanonicalScopesForProvider('instagram').join(',')
@@ -91,8 +65,8 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
9165
maxAge: CREDENTIAL_DRAFT_TTL_SECONDS,
9266
path: INSTAGRAM_STATE_COOKIE_PATH,
9367
})
94-
if (credentialDraftId) {
95-
response.cookies.set(INSTAGRAM_CREDENTIAL_DRAFT_COOKIE, credentialDraftId, {
68+
if (draftId) {
69+
response.cookies.set(INSTAGRAM_CREDENTIAL_DRAFT_COOKIE, draftId, {
9670
httpOnly: true,
9771
secure: process.env.NODE_ENV === 'production',
9872
sameSite: 'lax',

‎apps/sim/app/api/auth/oauth2/authorize/route.test.ts‎

Lines changed: 32 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,12 @@ import { oauthUtilsMock, oauthUtilsMockFns } from '@sim/testing/mocks/oauth-util
1010
import { urlsMockFns } from '@sim/testing/mocks/urls.mock'
1111
import type { NextRequest } from 'next/server'
1212
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
13-
import { ForbiddenOperationError } from '@/lib/core/application/forbidden'
14-
import { OrchestrationError } from '@/lib/core/orchestration/types'
15-
import { CredentialConnectionProviderMismatchError } from '@/lib/credentials/application/connection-target'
1613

1714
const mocks = vi.hoisted(() => ({
1815
betterAuthGET: vi.fn(),
1916
getSession: vi.fn(),
2017
linkAccount: vi.fn(),
2118
requireClient: vi.fn(),
22-
createConnection: vi.fn(),
2319
launchConnection: vi.fn(),
2420
decryptQuickBooksClientConfig: vi.fn(),
2521
createQuickBooksState: vi.fn(),
@@ -46,12 +42,6 @@ vi.mock('@/lib/core/config/env-capabilities.server', () => ({
4642
execute: vi.fn(),
4743
}),
4844
}))
49-
vi.mock('@/lib/credentials/application/create-credential-connection', () => ({
50-
createCredentialConnection: {
51-
operation: { id: 'credentials.connections.create' },
52-
execute: mocks.createConnection,
53-
},
54-
}))
5545
vi.mock('@/lib/credentials/application/launch-scoped-credential-connection', () => ({
5646
launchScopedCredentialConnection: mocks.launchConnection,
5747
}))
@@ -97,13 +87,6 @@ describe('OAuth2 authorize route', () => {
9787
user: { id: 'user-1' },
9888
session: { id: 'session-1' },
9989
})
100-
mocks.createConnection.mockResolvedValue({
101-
providerId: 'google-email',
102-
workspaceId: WORKSPACE_ID,
103-
draftId: 'draft-1',
104-
expiresAt: new Date('2026-08-14T12:00:00.000Z'),
105-
authorizationUrl: `${BASE_URL}/api/auth/oauth2/authorize?draftId=draft-1`,
106-
})
10790
mocks.launchConnection.mockResolvedValue({
10891
draft: {
10992
id: 'draft-1',
@@ -442,90 +425,51 @@ describe('OAuth2 authorize route', () => {
442425
expect(mocks.betterAuthGET).not.toHaveBeenCalled()
443426
})
444427

445-
it('refuses a cross-site workspace launch before creating a connection draft', async () => {
428+
it('sends a workspace connect link to the confirmation page without starting it', async () => {
429+
const callbackURL = `${BASE_URL}/workspace/${WORKSPACE_ID}`
446430
const response = await GET(
447431
request(
448-
{ providerId: 'google-email', workspaceId: WORKSPACE_ID, credentialId: 'credential-1' },
449-
{ 'sec-fetch-site': 'cross-site' }
432+
{
433+
providerId: 'google-email',
434+
workspaceId: WORKSPACE_ID,
435+
credentialId: 'credential-1',
436+
callbackURL,
437+
},
438+
{ 'sec-fetch-site': 'same-origin' }
450439
)
451440
)
452441

453-
expect(response.headers.get('location')).toBe(`${BASE_URL}/home?error=oauth_link_invalid`)
454-
expect(mocks.createConnection).not.toHaveBeenCalled()
442+
const location = new URL(response.headers.get('location') ?? '')
443+
expect(`${location.origin}${location.pathname}`).toBe(`${BASE_URL}/oauth/connect`)
444+
expect(Object.fromEntries(location.searchParams)).toEqual({
445+
providerId: 'google-email',
446+
workspaceId: WORKSPACE_ID,
447+
credentialId: 'credential-1',
448+
callbackURL,
449+
})
450+
expect(mocks.launchConnection).not.toHaveBeenCalled()
455451
expect(mocks.linkAccount).not.toHaveBeenCalled()
456452
})
457453

458-
it.each([{ 'sec-fetch-site': 'none' }, { 'sec-fetch-site': 'same-site' }])(
459-
'starts a workspace launch that is not cross-site: %o',
460-
async (headers) => {
461-
const response = await GET(
462-
request({ providerId: 'google-email', workspaceId: WORKSPACE_ID }, headers)
463-
)
464-
465-
expect(mocks.createConnection).toHaveBeenCalledOnce()
466-
expect(response.headers.get('location')).toBe('https://provider.example/authorize')
467-
}
468-
)
469-
470454
it('starts a cross-site launch of the signed-in user’s own connection draft', async () => {
471455
const response = await GET(request({ draftId: 'draft-1' }, { 'sec-fetch-site': 'cross-site' }))
472456

473457
expect(mocks.launchConnection).toHaveBeenCalledOnce()
474458
expect(response.headers.get('location')).toBe('https://provider.example/authorize')
475459
})
476460

477-
it('requires a configured OAuth client before creating a legacy draft', async () => {
478-
mocks.requireClient.mockImplementationOnce(() => {
479-
throw new Error('OAuth client is not configured')
480-
})
481-
482-
const response = await GET(request({ providerId: 'google-email', workspaceId: WORKSPACE_ID }))
483-
484-
expect(response.headers.get('location')).toBe(`${BASE_URL}/home?error=oauth_link_failed`)
485-
expect(mocks.requireClient).toHaveBeenCalledWith('google-email')
486-
expect(mocks.createConnection).not.toHaveBeenCalled()
487-
})
488-
489-
it('maps a provider mismatch without exposing the credential', async () => {
490-
mocks.createConnection.mockRejectedValue(
491-
new CredentialConnectionProviderMismatchError('google-email', 'slack')
492-
)
493-
494-
const response = await GET(
495-
request({
496-
providerId: 'slack',
497-
workspaceId: WORKSPACE_ID,
498-
credentialId: 'credential-1',
499-
})
500-
)
501-
502-
expect(response.headers.get('location')).toBe(
503-
`${BASE_URL}/home?error=credential_provider_mismatch`
504-
)
505-
})
506-
507-
it('maps credential and workspace authorization failures separately', async () => {
508-
mocks.createConnection.mockRejectedValueOnce(
509-
new ForbiddenOperationError(
510-
'CREDENTIAL_ADMIN_ACCESS_REQUIRED',
511-
'Credential admin permission required'
512-
)
513-
)
514-
const credentialResponse = await GET(
515-
request({
516-
providerId: 'google-email',
517-
workspaceId: WORKSPACE_ID,
518-
credentialId: 'credential-1',
519-
})
520-
)
521-
mocks.createConnection.mockRejectedValueOnce(
522-
new OrchestrationError('forbidden', 'Write permission required')
523-
)
524-
const workspaceResponse = await GET(
525-
request({ providerId: 'google-email', workspaceId: WORKSPACE_ID })
526-
)
527-
528-
expect(credentialResponse.headers.get('location')).toContain('credential_access_denied')
529-
expect(workspaceResponse.headers.get('location')).toContain('workspace_access_denied')
530-
})
461+
it.each([
462+
{
463+
callbackURL: undefined,
464+
errorCallbackURL: `${BASE_URL}/oauth/credential-connected?result=failed`,
465+
},
466+
{ callbackURL: `${BASE_URL}/oauth/chat-complete`, errorCallbackURL: undefined },
467+
])(
468+
'sends a failed draft launch to the status page only when no return page was named: %o',
469+
async ({ callbackURL, errorCallbackURL }) => {
470+
await GET(request({ draftId: 'draft-1', ...(callbackURL && { callbackURL }) }))
471+
472+
expect(mocks.linkAccount.mock.calls[0][0].body.errorCallbackURL).toBe(errorCallbackURL)
473+
}
474+
)
531475
})

0 commit comments

Comments
 (0)