Skip to content

Commit c683f10

Browse files
fix(credentials): use organization groups for personal connections
1 parent 9b1d6ed commit c683f10

11 files changed

Lines changed: 329 additions & 112 deletions

‎apps/sim/lib/credentials/api/route-policies.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ describe('personal account connection errors', () => {
103103
)
104104
expect(response).toMatchObject({
105105
status: 409,
106-
body: { error: 'Ask a workspace admin to configure this integration in Connected accounts' },
106+
body: {
107+
error: 'Ask an organization admin to configure this integration in organization settings',
108+
},
107109
})
108110
})
109111

‎apps/sim/lib/credentials/api/route-policies.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export const internalPersonalCredentialConnectionErrorPolicy = extendInternalErr
7575
(error) => {
7676
if (error instanceof CredentialGroupProviderConfigurationError) {
7777
return internalErrorResponse(409, {
78-
error: 'Ask a workspace admin to configure this integration in Connected accounts',
78+
error: 'Ask an organization admin to configure this integration in organization settings',
7979
})
8080
}
8181
const status =

‎apps/sim/lib/credentials/application/credential-crud.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ const mocks = vi.hoisted(() => ({
1717
getActor: vi.fn(),
1818
updateRecord: vi.fn(),
1919
createRecord: vi.fn(),
20+
personalAccounts: vi.fn(),
21+
createPersonalToken: vi.fn(),
2022
}))
2123

2224
const resolveGroupConfigMock = permissionGroupScopeMockFns.mockResolvePermissionGroupConfig
@@ -44,6 +46,18 @@ vi.mock('@/lib/credentials/orchestration', () => ({
4446
createCredentialRecord: mocks.createRecord,
4547
isProviderOutageCode: () => false,
4648
}))
49+
vi.mock('@/lib/credentials/application/workspace-personal-accounts', () => ({
50+
requireWorkspacePersonalAccounts: mocks.personalAccounts,
51+
}))
52+
vi.mock('@/lib/credentials/personal-tokens', () => ({
53+
createPersonalTokenCredential: mocks.createPersonalToken,
54+
updatePersonalTokenCredential: vi.fn(),
55+
}))
56+
vi.mock('@/lib/core/config/block-visibility', () => ({ getBlockVisibility: vi.fn() }))
57+
vi.mock('@/lib/integrations/principal-scope.server', () => ({ allowedIntegrationTypes: vi.fn() }))
58+
vi.mock('@/lib/integrations/credential-visibility.server', () => ({
59+
createIntegrationCredentialVisibility: () => ({ isCredentialVisible: () => true }),
60+
}))
4761
vi.mock('@/lib/permission-groups/config-scope.server', () => permissionGroupScopeMock)
4862
vi.mock('@/lib/credentials/oauth', () => ({ syncWorkspaceOAuthCredentialsForUser: vi.fn() }))
4963
vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: vi.fn() }))
@@ -414,3 +428,53 @@ describe('personal-credential capability', () => {
414428
expect(result.credential).toEqual(created)
415429
})
416430
})
431+
432+
describe('personal-token organization enrollment', () => {
433+
const accounts = { organizationId: 'organization', credentialGroupId: 'organization-group' }
434+
const tokenInput = {
435+
workspaceId: WORKSPACE_ID,
436+
type: 'personal_token' as const,
437+
providerId: 'gitlab',
438+
displayName: 'My GitLab',
439+
apiToken: 'personal-token',
440+
}
441+
442+
beforeEach(() => {
443+
vi.clearAllMocks()
444+
resolveGroupConfigMock.mockResolvedValue(null)
445+
mocks.loadWorkspace.mockResolvedValue({ ...workspace, workspaceOrganizationId: 'organization' })
446+
mocks.resolvePermission.mockResolvedValue('write')
447+
mocks.personalAccounts.mockResolvedValue(accounts)
448+
})
449+
450+
it('passes the authorized organization group into token creation', async () => {
451+
const created = { ...credential, type: 'personal_token', providerId: 'gitlab' }
452+
mocks.createPersonalToken.mockResolvedValue({
453+
success: true,
454+
created: true,
455+
credential: created,
456+
})
457+
mocks.getActor.mockResolvedValue({ credential: created, isAdmin: true })
458+
await createWorkspaceCredential.execute({ principal: sessionPrincipal, input: tokenInput })
459+
expect(mocks.personalAccounts).toHaveBeenCalledWith(
460+
sessionPrincipal,
461+
expect.objectContaining({ workspaceOrganizationId: 'organization' })
462+
)
463+
expect(mocks.createPersonalToken).toHaveBeenCalledWith({
464+
...tokenInput,
465+
userId: 'user-1',
466+
accounts,
467+
})
468+
expect(mocks.createRecord).not.toHaveBeenCalled()
469+
})
470+
471+
it('refuses unapproved organization access before verifying or storing a token', async () => {
472+
mocks.personalAccounts.mockRejectedValueOnce(new Error('Organization accounts unavailable'))
473+
await expect(
474+
createWorkspaceCredential.execute({ principal: sessionPrincipal, input: tokenInput })
475+
).rejects.toThrow('Organization accounts unavailable')
476+
expect(mocks.createPersonalToken).not.toHaveBeenCalled()
477+
expect(mocks.createRecord).not.toHaveBeenCalled()
478+
expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
479+
})
480+
})

‎apps/sim/lib/credentials/application/credential-crud.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '@/lib/credentials/application/authorized-credential-use-case'
1616
import { resolveCredentialApplicationContext } from '@/lib/credentials/application/credential-context'
1717
import { credentialOperations } from '@/lib/credentials/application/operations'
18+
import { requireWorkspacePersonalAccounts } from '@/lib/credentials/application/workspace-personal-accounts'
1819
import { syncWorkspaceOAuthCredentialsForUser } from '@/lib/credentials/oauth'
1920
import {
2021
createCredentialRecord,
@@ -230,7 +231,11 @@ export const createWorkspaceCredential = defineAuthorizedWorkspaceUseCase({
230231
}
231232
const result =
232233
input.type === 'personal_token'
233-
? await createPersonalTokenCredential({ ...input, userId })
234+
? await createPersonalTokenCredential({
235+
...input,
236+
userId,
237+
accounts: await requireWorkspacePersonalAccounts(principal, context),
238+
})
234239
: await createCredentialRecord({ ...input, userId }, { authorizeWorkspace: false })
235240
if (!result.success) throwCredentialMutationFailure(result)
236241
if (!result.credential) throw new Error('Credential creation succeeded without a credential')

‎apps/sim/lib/credentials/application/personal-connection.test.ts‎

Lines changed: 113 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ const mocks = vi.hoisted(() => ({
1212
personal: vi.fn(),
1313
oauthContext: vi.fn(),
1414
startOAuth: vi.fn(),
15+
organizationMembership: vi.fn(),
16+
available: vi.fn(),
17+
policy: vi.fn(),
1518
}))
1619
vi.mock('@/lib/workspaces/application/workspace-context', () => ({
1720
loadActiveWorkspaceApplicationContext: mocks.workspace,
@@ -27,7 +30,17 @@ vi.mock('@/lib/credentials/application/provider-catalog', () => ({
2730
listCredentialProviderCatalog: mocks.catalog,
2831
}))
2932
vi.mock('@/lib/credential-groups/credentials', () => ({
30-
loadWorkspaceAccountsCredentialListContext: mocks.group,
33+
loadScopedAccountsCredentialListContext: mocks.group,
34+
}))
35+
vi.mock('@/lib/core/application/organization-authorization', () => ({
36+
requireOrganizationMembership: mocks.organizationMembership,
37+
}))
38+
vi.mock('@/lib/credential-groups/scoped-availability', () => ({
39+
isScopedCredentialGroupsAvailable: mocks.available,
40+
}))
41+
vi.mock('@/lib/resource-policies/repository', () => ({
42+
requireResourcePolicy: mocks.policy,
43+
ResourcePolicyNotFoundError: class extends Error {},
3144
}))
3245
vi.mock('@/lib/credential-groups/enrollments', () => ({
3346
getCredentialGroupOAuthContextForEnrollment: mocks.oauthContext,
@@ -40,13 +53,15 @@ vi.mock('@/lib/credential-groups/self-enrollment', () => ({
4053
vi.mock('@/lib/credentials/personal', () => ({ getPersonalOAuthCredentials: mocks.personal }))
4154
vi.mock('@/lib/core/utils/urls', () => ({ getBaseUrl: () => 'https://sim.test' }))
4255

56+
import { buildOrganizationAccountAccessPolicy } from '@/lib/credential-groups/application/workspace-access-policy'
4357
import { startPersonalCredentialConnection } from '@/lib/credentials/application/personal-connection'
4458

4559
const principal: Principal = { kind: 'session', userId: 'viewer', sessionId: 'session' }
4660
const input = { workspaceId: 'workspace', providerId: 'confluence' }
4761
const group = {
4862
credentialGroupId: 'canonical-group',
49-
workspaceId: 'workspace',
63+
workspaceId: null,
64+
organizationId: 'organization',
5065
status: 'active',
5166
options: [{ id: 'option', provider: 'confluence', status: 'active' }],
5267
}
@@ -60,10 +75,15 @@ describe('personal connection launch', () => {
6075
vi.clearAllMocks()
6176
mocks.workspace.mockResolvedValue({
6277
workspaceId: 'workspace',
63-
workspaceOrganizationId: null,
78+
workspaceOrganizationId: 'organization',
6479
allowPersonalApiKeys: true,
6580
})
6681
mocks.permission.mockResolvedValue('read')
82+
mocks.organizationMembership.mockResolvedValue({ userId: 'viewer', role: 'member' })
83+
mocks.available.mockResolvedValue(true)
84+
mocks.policy.mockResolvedValue({
85+
document: buildOrganizationAccountAccessPolicy('canonical-group', ['workspace']),
86+
})
6787
mocks.catalog.mockResolvedValue([
6888
{
6989
type: 'oauth',
@@ -89,7 +109,7 @@ describe('personal connection launch', () => {
89109
})
90110
expect(mocks.oauthContext).toHaveBeenCalledWith(
91111
{
92-
workspaceId: 'workspace',
112+
organizationId: 'organization',
93113
credentialGroupId: 'canonical-group',
94114
enrollmentId: 'enrollment',
95115
email: 'viewer@example.com',
@@ -103,14 +123,24 @@ describe('personal connection launch', () => {
103123
)
104124
expect(mocks.enroll).toHaveBeenCalledWith({
105125
userId: 'viewer',
106-
workspaceId: 'workspace',
126+
organizationId: 'organization',
107127
credentialGroupId: 'canonical-group',
108128
})
109129
expect(mocks.ensure).not.toHaveBeenCalled()
130+
expect(mocks.group).toHaveBeenCalledWith({
131+
kind: 'organization',
132+
organizationId: 'organization',
133+
})
134+
expect(mocks.organizationMembership).toHaveBeenCalledWith(
135+
principal,
136+
'organization',
137+
'member',
138+
'integrations.manage'
139+
)
110140
expect(mocks.catalog).toHaveBeenCalledWith(principal, expect.any(Object), 'managed_oauth')
111141
})
112142

113-
it('connects a configured Slack workspace app through its enrollment', async () => {
143+
it('connects a configured organization Slack app through its enrollment', async () => {
114144
mocks.catalog.mockResolvedValue([
115145
{
116146
type: 'oauth',
@@ -145,23 +175,19 @@ describe('personal connection launch', () => {
145175
expect(mocks.enroll).not.toHaveBeenCalled()
146176
})
147177

148-
it('does not let a reader add a provider to workspace configuration', async () => {
178+
it('does not let a reader add a provider to organization configuration', async () => {
149179
mocks.group.mockResolvedValue({ ...group, options: [] })
150-
await expect(execute()).rejects.toThrow('Ask a workspace admin')
180+
await expect(execute()).rejects.toThrow('Ask an organization admin')
151181
expect(mocks.ensure).not.toHaveBeenCalled()
152182
expect(mocks.enroll).not.toHaveBeenCalled()
153183
})
154184

155-
it('lets an admin configure the standard provider once before connecting their own account', async () => {
185+
it('requires provider setup in organization settings even for a workspace admin', async () => {
156186
mocks.permission.mockResolvedValue('admin')
157-
mocks.group.mockResolvedValueOnce({ ...group, options: [] }).mockResolvedValueOnce(group)
158-
await execute()
159-
expect(mocks.ensure).toHaveBeenCalledWith('workspace', 'viewer', {
160-
provider: 'confluence',
161-
label: 'Confluence',
162-
required: false,
163-
})
164-
expect(mocks.enroll).toHaveBeenCalledTimes(1)
187+
mocks.group.mockResolvedValue({ ...group, options: [] })
188+
await expect(execute()).rejects.toThrow('Ask an organization admin')
189+
expect(mocks.ensure).not.toHaveBeenCalled()
190+
expect(mocks.enroll).not.toHaveBeenCalled()
165191
})
166192

167193
it.each([
@@ -206,12 +232,81 @@ describe('personal connection launch', () => {
206232
authorizationOptions: [{ providerId: 'slack' }],
207233
},
208234
])
209-
await expect(execute({ providerId: 'slack' })).rejects.toThrow('Configure Slack')
235+
await expect(execute({ providerId: 'slack' })).rejects.toThrow(
236+
'enable Slack in organization settings'
237+
)
210238
expect(mocks.ensure).not.toHaveBeenCalled()
211239
})
212240

213241
it('propagates revoked enrollment refusal', async () => {
214242
mocks.enroll.mockRejectedValue(new Error('Access revoked'))
215243
await expect(execute()).rejects.toThrow('Access revoked')
216244
})
245+
246+
it('does not create a group when the organization has not configured accounts', async () => {
247+
mocks.group.mockResolvedValue(null)
248+
await expect(execute()).rejects.toThrow('set up Connected accounts in organization settings')
249+
expect(mocks.ensure).not.toHaveBeenCalled()
250+
expect(mocks.enroll).not.toHaveBeenCalled()
251+
})
252+
253+
it('refuses personal workspaces before looking up organization accounts', async () => {
254+
mocks.workspace.mockResolvedValue({
255+
workspaceId: 'workspace',
256+
workspaceOrganizationId: null,
257+
allowPersonalApiKeys: true,
258+
})
259+
await expect(execute()).rejects.toThrow('does not belong to an organization')
260+
expect(mocks.group).not.toHaveBeenCalled()
261+
expect(mocks.enroll).not.toHaveBeenCalled()
262+
})
263+
264+
it('requires organization membership even when the caller administers the workspace', async () => {
265+
mocks.permission.mockResolvedValue('admin')
266+
mocks.organizationMembership.mockRejectedValueOnce(new Error('Organization not found'))
267+
await expect(execute()).rejects.toThrow('Organization not found')
268+
expect(mocks.group).not.toHaveBeenCalled()
269+
expect(mocks.enroll).not.toHaveBeenCalled()
270+
})
271+
272+
it('honors the organization feature flag before enrollment', async () => {
273+
mocks.available.mockResolvedValue(false)
274+
await expect(execute()).rejects.toThrow('not available')
275+
expect(mocks.available).toHaveBeenCalledWith({
276+
kind: 'organization',
277+
organizationId: 'organization',
278+
})
279+
expect(mocks.policy).not.toHaveBeenCalled()
280+
expect(mocks.enroll).not.toHaveBeenCalled()
281+
})
282+
283+
it('connects the person’s own account without granting their workspace workflow access', async () => {
284+
mocks.policy.mockResolvedValue({
285+
document: buildOrganizationAccountAccessPolicy('canonical-group', []),
286+
})
287+
await expect(execute()).resolves.toMatchObject({ providerId: 'confluence' })
288+
expect(mocks.enroll).toHaveBeenCalledWith({
289+
organizationId: 'organization',
290+
credentialGroupId: 'canonical-group',
291+
userId: 'viewer',
292+
})
293+
})
294+
295+
it('propagates policy read failures without provisioning or enrollment', async () => {
296+
mocks.policy.mockRejectedValueOnce(new Error('Database unavailable'))
297+
await expect(execute()).rejects.toThrow('Database unavailable')
298+
expect(mocks.ensure).not.toHaveBeenCalled()
299+
expect(mocks.enroll).not.toHaveBeenCalled()
300+
})
301+
302+
it('rejects workspace keys before loading protected context', async () => {
303+
await expect(
304+
startPersonalCredentialConnection.execute({
305+
principal: { kind: 'workspace_api_key', keyId: 'key', workspaceId: 'workspace' },
306+
input,
307+
})
308+
).rejects.toThrow()
309+
expect(mocks.workspace).not.toHaveBeenCalled()
310+
expect(mocks.enroll).not.toHaveBeenCalled()
311+
})
217312
})

0 commit comments

Comments
 (0)