Skip to content

Commit c729839

Browse files
committed
fix(search): authorize organization source credentials
1 parent 73875ae commit c729839

8 files changed

Lines changed: 305 additions & 39 deletions

File tree

apps/sim/lib/credentials/application/organization-credentials.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ export async function authorizeOrganizationCredentialUse(input: {
263263
const row = await getOrganizationCredential(input.organizationId, input.credentialId)
264264
if (
265265
!row ||
266+
row.revokedAt ||
266267
(row.type !== 'oauth' && row.type !== 'service_account') ||
267268
!row.providerId ||
268269
(row.type === 'oauth' && row.createdBy !== context.userId)

apps/sim/lib/knowledge/application/connector-access.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,9 @@ export const updateKnowledgeConnectorAccess = defineAuthorizedKnowledgeUseCase({
234234
}
235235
const previousConfig = connector.sourceConfig as Record<string, unknown>
236236
const sourceConfig = await prepareGitHubInstallationSource({
237+
principal,
238+
requestId,
239+
workspaceId: context.workspaceId,
237240
connectorType: connector.connectorType,
238241
credentialId:
239242
input.credentialId === undefined && input.accessMode === connector.accessMode
@@ -271,6 +274,7 @@ export const updateKnowledgeConnectorAccess = defineAuthorizedKnowledgeUseCase({
271274
}
272275
if (credentialId) {
273276
await requireUsableCredential({
277+
principal,
274278
credentialId,
275279
connectorMeta,
276280
sourceConfig,
@@ -280,6 +284,7 @@ export const updateKnowledgeConnectorAccess = defineAuthorizedKnowledgeUseCase({
280284
accessMode: 'members',
281285
})
282286
const rejection = await validateConnectorSourceConfig({
287+
principal,
283288
connector: { ...connector, accessMode: 'members', credentialId },
284289
sourceConfig,
285290
...owner,
@@ -299,6 +304,7 @@ export const updateKnowledgeConnectorAccess = defineAuthorizedKnowledgeUseCase({
299304
target = {
300305
accessMode: input.accessMode,
301306
credentialId: await requireUsableCredential({
307+
principal,
302308
credentialId: input.credentialId,
303309
connectorMeta,
304310
sourceConfig,
@@ -309,6 +315,7 @@ export const updateKnowledgeConnectorAccess = defineAuthorizedKnowledgeUseCase({
309315
}),
310316
}
311317
const rejection = await validateConnectorSourceConfig({
318+
principal,
312319
connector: {
313320
...connector,
314321
accessMode: target.accessMode,
@@ -372,6 +379,7 @@ export const updateKnowledgeConnectorAccess = defineAuthorizedKnowledgeUseCase({
372379
* source validation verifies it against the target mode before any mutation.
373380
*/
374381
async function requireUsableCredential(input: {
382+
principal: Principal
375383
credentialId: string | null | undefined
376384
connectorMeta: Pick<ConnectorMeta, 'name' | 'auth'>
377385
sourceConfig: Record<string, unknown>
@@ -403,6 +411,7 @@ async function requireUsableCredential(input: {
403411
)
404412
}
405413
const token = await resolveConnectorCredentialAccessToken({
414+
principal: input.principal,
406415
credentialId: input.credentialId,
407416
...resourceScopeFields(resourceScopeFromOwner(input)),
408417
actingUserId: input.actingUserId,
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
/** @vitest-environment node */
2+
import type { Principal } from '@sim/auth/principal'
3+
import { credential, member } from '@sim/db/schema'
4+
import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
5+
import { and, eq, isNull } from 'drizzle-orm'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const mocks = vi.hoisted(() => ({
9+
config: vi.fn(),
10+
catalog: vi.fn(),
11+
requireService: vi.fn(),
12+
requireOAuth: vi.fn(),
13+
repository: vi.fn(),
14+
}))
15+
vi.mock('@/lib/permission-groups/resolve.server', () => ({
16+
getUserPermissionConfigForOrganization: mocks.config,
17+
}))
18+
vi.mock('@/lib/credentials/application/provider-catalog', () => ({
19+
listCredentialProviderCatalog: mocks.catalog,
20+
requireAvailableServiceAccountCredentialProvider: mocks.requireService,
21+
requireAvailableOAuthCredentialProvider: mocks.requireOAuth,
22+
}))
23+
vi.mock('@/lib/credentials/application/credential-crud', () => ({
24+
throwCredentialMutationFailure: vi.fn(),
25+
}))
26+
vi.mock('@/lib/credentials/orchestration/credential-create', () => ({
27+
createCredentialRecord: vi.fn(),
28+
}))
29+
vi.mock('@/lib/credentials/orchestration', () => ({ updateCredentialRecord: vi.fn() }))
30+
vi.mock('@/lib/credentials/connect-draft', () => ({
31+
createConnectDraft: vi.fn(),
32+
getActiveConnectDraft: vi.fn(),
33+
}))
34+
vi.mock('@/lib/oauth/credential-service', () => ({ resolveCredentialTokenBundle: vi.fn() }))
35+
vi.mock('@/lib/core/security/encryption', () => ({
36+
decryptSecret: async () => ({ decrypted: '{}' }),
37+
}))
38+
vi.mock('@/lib/oauth/github-installation', () => ({
39+
parseGitHubInstallationBinding: () => ({ installationId: '42', accountId: '7' }),
40+
resolveGitHubInstallationRepository: mocks.repository,
41+
}))
42+
43+
import { OrchestrationError } from '@/lib/core/orchestration/types'
44+
import { requireConnectorCredential } from '@/lib/knowledge/application/connector-credential'
45+
import { prepareGitHubInstallationSource } from '@/lib/knowledge/application/github-installation-source'
46+
import { DEFAULT_PERMISSION_GROUP_CONFIG } from '@/lib/permission-groups/fields'
47+
48+
const principal: Principal = { kind: 'session', userId: 'admin-1', sessionId: 'session-1' }
49+
const installed = {
50+
id: 'installation-credential',
51+
organizationId: 'org-1',
52+
workspaceId: null,
53+
type: 'service_account',
54+
providerId: 'github-app-installation',
55+
createdBy: 'installer-1',
56+
revokedAt: null,
57+
encryptedServiceAccountKey: 'encrypted',
58+
providerSubjectId: '42',
59+
providerTenantId: '7',
60+
}
61+
const input = {
62+
principal,
63+
credentialId: installed.id,
64+
scope: { kind: 'organization' as const, organizationId: 'org-1' },
65+
actingUserId: 'admin-1',
66+
requestId: 'request-1',
67+
}
68+
69+
beforeEach(() => {
70+
vi.clearAllMocks()
71+
resetDbChainMock()
72+
mocks.config.mockResolvedValue(null)
73+
mocks.catalog.mockResolvedValue([])
74+
mocks.repository.mockResolvedValue({ id: '123', fullName: 'example/private' })
75+
})
76+
77+
describe('organization source credential authorization', () => {
78+
it.each(['owner', 'admin'])('pins an installation repository for a current %s', async (role) => {
79+
queueTableRows(member, [{ role }])
80+
queueTableRows(credential, [installed])
81+
82+
await expect(
83+
prepareGitHubInstallationSource({
84+
principal,
85+
requestId: input.requestId,
86+
connectorType: 'github',
87+
credentialId: installed.id,
88+
organizationId: 'org-1',
89+
isSearchIndex: true,
90+
accessMode: 'members',
91+
actingUserId: 'admin-1',
92+
sourceConfig: { repository: 'example/private' },
93+
})
94+
).resolves.toEqual({ repository: 'example/private', githubRepositoryId: '123' })
95+
expect(dbChainMockFns.where).toHaveBeenCalledWith(
96+
and(eq(member.organizationId, 'org-1'), eq(member.userId, 'admin-1'))
97+
)
98+
expect(dbChainMockFns.where).toHaveBeenCalledWith(
99+
and(
100+
eq(credential.id, installed.id),
101+
and(eq(credential.organizationId, 'org-1'), isNull(credential.workspaceId))
102+
)
103+
)
104+
expect(mocks.requireService).toHaveBeenCalledWith([], 'github-app-installation')
105+
})
106+
107+
it.each([{ rows: [] }, { rows: [{ role: 'member' }] }])(
108+
'refuses missing or insufficient membership: %j',
109+
async ({ rows }) => {
110+
queueTableRows(member, rows)
111+
queueTableRows(credential, [installed])
112+
await expect(requireConnectorCredential(input)).rejects.toBeInstanceOf(OrchestrationError)
113+
expect(dbChainMockFns.from).not.toHaveBeenCalledWith(credential)
114+
expect(mocks.catalog).not.toHaveBeenCalled()
115+
}
116+
)
117+
118+
it('does not substitute the credential creator or attributed user for the principal', async () => {
119+
queueTableRows(member, [])
120+
await expect(
121+
requireConnectorCredential({ ...input, actingUserId: installed.createdBy })
122+
).rejects.toMatchObject({ code: 'not_found' })
123+
expect(dbChainMockFns.where).toHaveBeenCalledWith(
124+
and(eq(member.organizationId, 'org-1'), eq(member.userId, principal.userId))
125+
)
126+
})
127+
128+
it('refuses a credential outside the asserted organization', async () => {
129+
queueTableRows(member, [{ role: 'admin' }])
130+
queueTableRows(credential, [])
131+
await expect(requireConnectorCredential(input)).rejects.toMatchObject({ code: 'not_found' })
132+
expect(mocks.catalog).not.toHaveBeenCalled()
133+
})
134+
135+
it('does not make an organization credential usable from a workspace', async () => {
136+
queueTableRows(credential, [installed])
137+
await expect(
138+
requireConnectorCredential({ ...input, scope: { kind: 'workspace', workspaceId: 'ws-1' } })
139+
).rejects.toMatchObject({ code: 'validation' })
140+
expect(mocks.catalog).not.toHaveBeenCalled()
141+
})
142+
143+
it('refuses revoked credentials before provider access', async () => {
144+
queueTableRows(member, [{ role: 'admin' }])
145+
queueTableRows(credential, [{ ...installed, revokedAt: new Date() }])
146+
await expect(requireConnectorCredential(input)).rejects.toMatchObject({ code: 'not_found' })
147+
expect(mocks.catalog).not.toHaveBeenCalled()
148+
})
149+
150+
it('does not let an admin use another person’s OAuth account', async () => {
151+
queueTableRows(member, [{ role: 'admin' }])
152+
queueTableRows(credential, [{ ...installed, type: 'oauth', providerId: 'github-repositories' }])
153+
await expect(requireConnectorCredential(input)).rejects.toMatchObject({ code: 'not_found' })
154+
})
155+
156+
it('allows an admin to use their own organization OAuth account', async () => {
157+
const ownAccount = {
158+
...installed,
159+
type: 'oauth',
160+
providerId: 'github-repositories',
161+
createdBy: 'admin-1',
162+
}
163+
queueTableRows(member, [{ role: 'admin' }])
164+
queueTableRows(credential, [ownAccount])
165+
await expect(requireConnectorCredential(input)).resolves.toEqual(ownAccount)
166+
expect(mocks.requireOAuth).toHaveBeenCalledWith([], 'github-repositories')
167+
})
168+
169+
it('enforces the existing integration-management capability', async () => {
170+
queueTableRows(member, [{ role: 'admin' }])
171+
mocks.config.mockResolvedValue({
172+
...DEFAULT_PERMISSION_GROUP_CONFIG,
173+
hideIntegrationsTab: true,
174+
})
175+
await expect(requireConnectorCredential(input)).rejects.toMatchObject({ code: 'forbidden' })
176+
expect(dbChainMockFns.from).not.toHaveBeenCalledWith(credential)
177+
})
178+
179+
it('refuses workspace keys before protected loading', async () => {
180+
await expect(
181+
requireConnectorCredential({
182+
...input,
183+
principal: { kind: 'workspace_api_key', workspaceId: 'ws-1', keyId: 'key-1' },
184+
})
185+
).rejects.toMatchObject({ code: 'forbidden' })
186+
expect(dbChainMockFns.from).not.toHaveBeenCalled()
187+
})
188+
189+
it('propagates provider policy denials', async () => {
190+
queueTableRows(member, [{ role: 'admin' }])
191+
queueTableRows(credential, [installed])
192+
const denial = new OrchestrationError('forbidden', 'Provider is unavailable')
193+
mocks.requireService.mockImplementationOnce(() => {
194+
throw denial
195+
})
196+
await expect(requireConnectorCredential(input)).rejects.toBe(denial)
197+
})
198+
})
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import type { Principal } from '@sim/auth/principal'
2+
import { OrchestrationError } from '@/lib/core/orchestration/types'
3+
import {
4+
type ResourceScope,
5+
resourceScopeFromOwner,
6+
sameResourceScope,
7+
} from '@/lib/core/resource-scope'
8+
import { canUseCredential, getCredentialActorContext } from '@/lib/credentials/access'
9+
import { authorizeOrganizationCredentialUse } from '@/lib/credentials/application/organization-credentials'
10+
import type { CredentialRow } from '@/lib/credentials/queries'
11+
12+
/** Resolves source credentials using the authorization policy of their canonical owner scope. */
13+
export async function requireConnectorCredential(input: {
14+
principal: Principal
15+
credentialId: string
16+
scope: ResourceScope
17+
actingUserId: string
18+
requestId: string
19+
}): Promise<CredentialRow> {
20+
if (input.scope.kind === 'organization') {
21+
const { credential } = await authorizeOrganizationCredentialUse({
22+
principal: input.principal,
23+
organizationId: input.scope.organizationId,
24+
credentialId: input.credentialId,
25+
requestId: input.requestId,
26+
})
27+
return credential
28+
}
29+
30+
const access = await getCredentialActorContext(input.credentialId, input.actingUserId)
31+
if (
32+
!access.credential ||
33+
!sameResourceScope(resourceScopeFromOwner(access.credential), input.scope) ||
34+
!canUseCredential(access)
35+
) {
36+
throw new OrchestrationError(
37+
'validation',
38+
'Credential is not available to you in this workspace. Ask a credential administrator to grant access or select another credential.'
39+
)
40+
}
41+
return access.credential
42+
}

apps/sim/lib/knowledge/application/connectors.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ describe('knowledge connector application use cases', () => {
268268
async (accessMode) => {
269269
await expect(
270270
resolveConnectorCredentialAccessToken({
271+
principal: { kind: 'session', userId: 'admin', sessionId: 'session' },
271272
credentialId: 'credential-1',
272273
workspaceId: 'workspace-a',
273274
actingUserId: 'admin',
@@ -285,6 +286,7 @@ describe('knowledge connector application use cases', () => {
285286
mocks.resolveTokenIdentity.mockResolvedValueOnce({ kind: 'service_account' })
286287
await expect(
287288
resolveConnectorCredentialAccessToken({
289+
principal: { kind: 'session', userId: 'admin', sessionId: 'session' },
288290
credentialId: 'credential-1',
289291
workspaceId: 'workspace-a',
290292
actingUserId: 'admin',
@@ -316,6 +318,7 @@ describe('knowledge connector application use cases', () => {
316318
} as Parameters<typeof validateConnectorSourceConfig>[0]['connector']
317319
await expect(
318320
validateConnectorSourceConfig({
321+
principal: { kind: 'session', userId: 'admin', sessionId: 'session' },
319322
connector,
320323
sourceConfig: { adminEmail: 'admin@corp.com' },
321324
workspaceId: 'workspace-a',

0 commit comments

Comments
 (0)