Skip to content

Commit b6a598e

Browse files
authored
fix(accounts): scope organization OAuth outbound requests (#8533)
* fix(accounts): scope organization OAuth outbound requests * fix(accounts): preserve outbound ownership during reconnect
1 parent 4d51401 commit b6a598e

3 files changed

Lines changed: 329 additions & 48 deletions

File tree

Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
import { createHash } from 'node:crypto'
2+
import { createServer } from 'node:http'
3+
import { db } from '@sim/db'
4+
import { credential, credentialGroupEnrollment, member, organization, user } from '@sim/db/schema'
5+
import { readTestRedisUrl } from '@sim/db/testing/test-infrastructure'
6+
import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
7+
import { generateId } from '@sim/utils/id'
8+
import { toRecord } from '@sim/utils/object'
9+
import { eq, inArray } from 'drizzle-orm'
10+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
11+
import { env } from '@/lib/core/config/env'
12+
import {
13+
resolveCurrentOutboundRoute,
14+
runWithOutboundOrganization,
15+
} from '@/lib/core/network/context.server'
16+
import { startOrganizationAccountConnection } from '@/lib/credential-groups/application/organization-accounts'
17+
import { reconnectPersonalOrganizationAccount } from '@/lib/credential-groups/application/personal-organization-accounts'
18+
import { createManagedMcpConnector } from '@/lib/credential-groups/managed-mcp-service'
19+
import { consumeCredentialGroupMcpOAuthAttempt } from '@/lib/credential-groups/mcp-oauth-state'
20+
import { createViewerCredentialGroupEnrollment } from '@/lib/credential-groups/self-enrollment'
21+
import { ensureWorkspaceAccountsGroup } from '@/lib/credential-groups/service'
22+
import { encryptManagedMcpTokens } from '@/lib/credentials/managed-mcp'
23+
import * as oauth from '@/lib/mcp/oauth/auth'
24+
import { createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
25+
26+
const RESOURCE = 'https://mcp.lucid.app/mcp/readonly'
27+
const ISSUER = 'https://oauth.fixture.test'
28+
const owner = generateId()
29+
const blockedOwner = generateId()
30+
const outsider = generateId()
31+
const directOrg = generateId()
32+
const blockedOrg = generateId()
33+
const servers = new Map<string, string>()
34+
const grants = new Map<string, { credentialId: string; enrollmentId: string }>()
35+
const requests: string[] = []
36+
37+
/** Real OAuth discovery and registration over a socket; only the remote destination is replaced. */
38+
const providerServer = createServer(async (request, response) => {
39+
const path = request.url ?? '/'
40+
requests.push(path)
41+
response.setHeader('content-type', 'application/json')
42+
if (path.includes('oauth-protected-resource')) {
43+
response.end(JSON.stringify({ resource: RESOURCE, authorization_servers: [ISSUER] }))
44+
} else if (path.includes('oauth-authorization-server') || path.includes('openid-configuration')) {
45+
response.end(
46+
JSON.stringify({
47+
issuer: ISSUER,
48+
authorization_endpoint: `${ISSUER}/authorize`,
49+
token_endpoint: `${ISSUER}/token`,
50+
registration_endpoint: `${ISSUER}/register`,
51+
response_types_supported: ['code'],
52+
code_challenge_methods_supported: ['S256'],
53+
token_endpoint_auth_methods_supported: ['none'],
54+
})
55+
)
56+
} else if (path === '/register' && request.method === 'POST') {
57+
const chunks: Buffer[] = []
58+
for await (const chunk of request) chunks.push(Buffer.from(chunk))
59+
const metadata: unknown = JSON.parse(Buffer.concat(chunks).toString())
60+
response.writeHead(201).end(
61+
JSON.stringify({
62+
...toRecord(metadata),
63+
client_id: 'fixture-dynamic-client',
64+
})
65+
)
66+
} else {
67+
response.writeHead(404).end(JSON.stringify({ error: 'unknown fixture endpoint' }))
68+
}
69+
})
70+
71+
beforeAll(async () => {
72+
Object.assign(env, {
73+
REDIS_URL: readTestRedisUrl(),
74+
OUTBOUND_ROUTING_SOURCE: 'env',
75+
OUTBOUND_ROUTING_CONFIG: JSON.stringify({
76+
schemaVersion: 1,
77+
revision: 'account-oauth-fixture',
78+
organizations: { [directOrg]: { kind: 'direct' }, [blockedOrg]: { kind: 'blocked' } },
79+
}),
80+
OUTBOUND_GATEWAYS: JSON.stringify({
81+
direct: {
82+
organizationId: directOrg,
83+
url: 'https://direct.fixture.test/',
84+
credentialId: 'direct',
85+
},
86+
blocked: {
87+
organizationId: blockedOrg,
88+
url: 'https://blocked.fixture.test/',
89+
credentialId: 'blocked',
90+
},
91+
}),
92+
OUTBOUND_GATEWAY_CREDENTIALS: JSON.stringify({
93+
direct: { token: 'd'.repeat(32) },
94+
blocked: { token: 'b'.repeat(32) },
95+
}),
96+
})
97+
await db.insert(user).values(
98+
[owner, blockedOwner, outsider].map((id) => ({
99+
id,
100+
name: 'OAuth fixture',
101+
email: `${id}@fixture.test`,
102+
emailVerified: true,
103+
createdAt: new Date(),
104+
updatedAt: new Date(),
105+
}))
106+
)
107+
for (const organizationId of [directOrg, blockedOrg]) {
108+
const userId = organizationId === directOrg ? owner : blockedOwner
109+
await db
110+
.insert(organization)
111+
.values({ id: organizationId, name: 'OAuth fixture', slug: organizationId })
112+
await db.insert(member).values({ id: generateId(), organizationId, userId, role: 'owner' })
113+
const group = await ensureWorkspaceAccountsGroup(
114+
{ kind: 'organization', organizationId },
115+
userId
116+
)
117+
const { mcpServer } = await db.transaction((tx) =>
118+
createManagedMcpConnector(
119+
{
120+
organizationId,
121+
credentialGroupId: group.id,
122+
userId,
123+
validated: { input: { connectorId: 'lucid' }, url: RESOURCE },
124+
},
125+
tx
126+
)
127+
)
128+
servers.set(organizationId, mcpServer.id)
129+
const { enrollment } = await createViewerCredentialGroupEnrollment({
130+
organizationId,
131+
credentialGroupId: group.id,
132+
userId,
133+
})
134+
const credentialId = `mcp-cg-${generateId()}`
135+
await db.insert(credential).values({
136+
id: credentialId,
137+
organizationId,
138+
type: 'managed_mcp',
139+
displayName: 'OAuth fixture',
140+
grantedAt: new Date(),
141+
credentialGroupEnrollmentId: enrollment.id,
142+
mcpServerId: mcpServer.id,
143+
managedOauthStatus: 'active',
144+
mcpTools: [],
145+
encryptedOauthTokenSet: await encryptManagedMcpTokens({
146+
access_token: 'fixture-access',
147+
token_type: 'Bearer',
148+
}),
149+
})
150+
grants.set(organizationId, { credentialId, enrollmentId: enrollment.id })
151+
}
152+
await new Promise<void>((resolve) => providerServer.listen(0, '127.0.0.1', resolve))
153+
const address = providerServer.address()
154+
if (!address || typeof address === 'string') throw new Error('OAuth fixture did not bind')
155+
const origin = `http://127.0.0.1:${address.port}`
156+
const guardedFetch = createSsrfGuardedMcpFetch({ serverUrl: origin })
157+
const authenticate = oauth.mcpAuthGuarded
158+
vi.spyOn(oauth, 'mcpAuthGuarded').mockImplementation((provider, options) =>
159+
authenticate(provider, {
160+
...options,
161+
fetchFn: (input, init) => {
162+
const remote = new URL(input instanceof Request ? input.url : input)
163+
if (![new URL(RESOURCE).origin, ISSUER].includes(remote.origin)) {
164+
throw new Error('Unexpected OAuth fixture origin')
165+
}
166+
return guardedFetch(new URL(`${remote.pathname}${remote.search}`, origin), init)
167+
},
168+
})
169+
)
170+
})
171+
172+
afterAll(async () => {
173+
if (providerServer.listening)
174+
await new Promise<void>((resolve, reject) => {
175+
providerServer.close((error) => (error ? reject(error) : resolve()))
176+
providerServer.closeAllConnections()
177+
})
178+
await db.delete(organization).where(inArray(organization.id, [directOrg, blockedOrg]))
179+
await db.delete(user).where(inArray(user.id, [owner, blockedOwner, outsider]))
180+
})
181+
182+
function connect(
183+
organizationId: string,
184+
userId = owner,
185+
mcpServerId = servers.get(organizationId)!
186+
) {
187+
return startOrganizationAccountConnection.execute({
188+
principal: createSessionPrincipal({ userId, sessionId: generateId() }),
189+
input: {
190+
organizationId,
191+
mcpServerId,
192+
oauthCompletionId: generateId(),
193+
returnTo: 'integrations',
194+
},
195+
})
196+
}
197+
198+
function reconnect(organizationId: string, userId = owner) {
199+
return reconnectPersonalOrganizationAccount.execute({
200+
principal: createSessionPrincipal({ userId, sessionId: generateId() }),
201+
input: {
202+
credentialId: grants.get(organizationId)!.credentialId,
203+
oauthCompletionId: generateId(),
204+
},
205+
})
206+
}
207+
208+
async function verifyAuthorization(start: typeof connect | typeof reconnect) {
209+
const result = await start(directOrg)
210+
if (!result.authorizationUrl) throw new Error('OAuth authorization URL is missing')
211+
const authorization = new URL(result.authorizationUrl)
212+
expect(`${authorization.origin}${authorization.pathname}`).toBe(`${ISSUER}/authorize`)
213+
expect(authorization.searchParams.get('client_id')).toBe('fixture-dynamic-client')
214+
expect(authorization.searchParams.get('code_challenge_method')).toBe('S256')
215+
const state = authorization.searchParams.get('state')!
216+
const attempt = await consumeCredentialGroupMcpOAuthAttempt(state)
217+
expect(attempt).toMatchObject({
218+
organizationId: directOrg,
219+
userId: owner,
220+
mcpServerId: servers.get(directOrg),
221+
returnTo: 'integrations',
222+
})
223+
expect(authorization.searchParams.get('code_challenge')).toBe(
224+
createHash('sha256').update(attempt!.codeVerifier).digest('base64url')
225+
)
226+
expect(await consumeCredentialGroupMcpOAuthAttempt(state)).toBeNull()
227+
}
228+
229+
describe.each([
230+
{ name: 'Connect', start: connect },
231+
{ name: 'Reconnect', start: reconnect },
232+
])('$name account OAuth outbound ownership', ({ start }) => {
233+
it('starts dynamic OAuth with a bound single-use attempt when no ambient scope exists', async () => {
234+
await verifyAuthorization(start)
235+
await expect(resolveCurrentOutboundRoute()).rejects.toMatchObject({ code: 'MISSING_SCOPE' })
236+
})
237+
it('uses authorized ownership instead of an ambient blocked organization and restores the outer scope', async () => {
238+
await runWithOutboundOrganization(blockedOrg, async () => {
239+
await verifyAuthorization(start)
240+
await expect(resolveCurrentOutboundRoute()).rejects.toMatchObject({ code: 'ROUTE_BLOCKED' })
241+
})
242+
})
243+
it('does not bypass an organization block through ambient platform scope', async () => {
244+
const before = requests.length
245+
await runWithOutboundOrganization(null, async () => {
246+
await expect(start(blockedOrg, blockedOwner)).rejects.toMatchObject({
247+
code: 'ROUTE_BLOCKED',
248+
})
249+
expect(await resolveCurrentOutboundRoute()).toEqual({ kind: 'direct' })
250+
})
251+
expect(requests.length).toBe(before)
252+
})
253+
})
254+
255+
describe('Account authorization before outbound OAuth', () => {
256+
it('denies nonmembers and cross-organization providers before contacting OAuth', async () => {
257+
const before = requests.length
258+
await expect(connect(directOrg, outsider)).rejects.toMatchObject({ code: 'not_found' })
259+
await expect(connect(directOrg, owner, servers.get(blockedOrg))).rejects.toMatchObject({
260+
code: 'not_found',
261+
})
262+
expect(requests.length).toBe(before)
263+
})
264+
it('denies another contributor and a revoked enrollment during reconnect', async () => {
265+
const before = requests.length
266+
await expect(reconnect(directOrg, outsider)).rejects.toMatchObject({ code: 'not_found' })
267+
const enrollmentId = grants.get(directOrg)!.enrollmentId
268+
await db
269+
.update(credentialGroupEnrollment)
270+
.set({ status: 'revoked' })
271+
.where(eq(credentialGroupEnrollment.id, enrollmentId))
272+
await expect(reconnect(directOrg)).rejects.toMatchObject({ code: 'forbidden' })
273+
expect(requests.length).toBe(before)
274+
})
275+
})

‎apps/sim/lib/credential-groups/application/organization-accounts.ts‎

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
defineOrganizationOperation,
1313
type OrganizationOperation,
1414
} from '@/lib/core/application/organization-operation'
15+
import { runWithOutboundOrganization } from '@/lib/core/network/context.server'
1516
import { OrchestrationError } from '@/lib/core/orchestration/types'
1617
import { validateUpdateCredentialGroupInput } from '@/lib/credential-groups/application/validation'
1718
import { loadScopedAccountsCredentialListContext } from '@/lib/credential-groups/credentials'
@@ -119,41 +120,43 @@ export function defineOrganizationAccountsUseCase<
119120
if (group)
120121
await requireOrganizationAccountsSetup(context.organizationId, group.credentialGroupId)
121122
}
122-
const result = await definition.execute({ input, context }).catch((error: unknown) => {
123-
if (error instanceof ManagedMcpConnectorError)
124-
throw new OrchestrationError(
125-
error.code === 'bad_gateway' ? 'internal' : error.code,
126-
error.message
127-
)
128-
if (error instanceof CredentialGroupEnrollmentError)
129-
throw new OrchestrationError(
130-
error.status === 404
131-
? 'not_found'
132-
: error.status === 409
133-
? 'conflict'
134-
: error.status === 400
135-
? 'validation'
136-
: 'internal',
137-
error.message
138-
)
139-
throw error
140-
})
141-
const audit = definition.projectAudit?.(result)
142-
if (audit)
143-
recordAudit({
144-
...audit,
145-
actorId: context.userId,
146-
action: AuditAction.CREDENTIAL_GROUP_UPDATED,
147-
resourceType: AuditResourceType.CREDENTIAL_GROUP,
148-
metadata: {
149-
organizationId: context.organizationId,
150-
operation: definition.operation.id,
151-
actor: resolvePrincipalAuditAttribution(principal).actor,
152-
},
153-
request,
123+
return runWithOutboundOrganization(context.organizationId, async () => {
124+
const result = await definition.execute({ input, context }).catch((error: unknown) => {
125+
if (error instanceof ManagedMcpConnectorError)
126+
throw new OrchestrationError(
127+
error.code === 'bad_gateway' ? 'internal' : error.code,
128+
error.message
129+
)
130+
if (error instanceof CredentialGroupEnrollmentError)
131+
throw new OrchestrationError(
132+
error.status === 404
133+
? 'not_found'
134+
: error.status === 409
135+
? 'conflict'
136+
: error.status === 400
137+
? 'validation'
138+
: 'internal',
139+
error.message
140+
)
141+
throw error
154142
})
155-
await definition.afterSuccess?.({ result, context })
156-
return result
143+
const audit = definition.projectAudit?.(result)
144+
if (audit)
145+
recordAudit({
146+
...audit,
147+
actorId: context.userId,
148+
action: AuditAction.CREDENTIAL_GROUP_UPDATED,
149+
resourceType: AuditResourceType.CREDENTIAL_GROUP,
150+
metadata: {
151+
organizationId: context.organizationId,
152+
operation: definition.operation.id,
153+
actor: resolvePrincipalAuditAttribution(principal).actor,
154+
},
155+
request,
156+
})
157+
await definition.afterSuccess?.({ result, context })
158+
return result
159+
})
157160
},
158161
}
159162
}

0 commit comments

Comments
 (0)