Skip to content

Commit 4489fc2

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(credential-groups): resolve external actor enrollments
1 parent 59373c8 commit 4489fc2

6 files changed

Lines changed: 201 additions & 11 deletions

File tree

‎apps/sim/blocks/blocks/credential-group.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ export const CredentialGroupBlock: BlockConfig<CredentialGroupBlockOutput> = {
9999
longDescription:
100100
'List usable managed credentials, inspect invited people, send an account-connection invitation, or discover Credential Groups in the current workspace. The block returns credential IDs and account metadata without exposing OAuth tokens.',
101101
bestPractices: `
102-
- Use "List Credentials" with a ForEach loop to run a provider block once for every connected account.
103102
- "List Credentials" returns every active credential. Filter by email to select one enrolled person, by provider to select one account type, or by both for an exact match.
103+
- Provider blocks can use the current actor's enrolled credential by default. Using another enrollment requires an explicit workflow access grant.
104+
- With a workflow access grant, use "List Credentials" with a ForEach loop to run a provider block once for every connected account.
104105
- Continue with nextCursor until hasMore is false when a list operation returns multiple pages.
105106
- "List Credentials" returns active, usable credentials only. Reconnect-needed and revoked credentials are excluded.
106107
- Use "List People" to inspect invitation and connection progress without exposing credential secrets.

‎apps/sim/lib/credential-groups/application/authorization.test.ts‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import type { WorkflowExecutionDelegatedPrincipal } from '@sim/auth/principal'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const mocks = vi.hoisted(() => ({
8+
loadEnrollmentAccess: vi.fn(),
9+
}))
10+
11+
vi.mock('@/lib/credential-groups/credentials', () => ({
12+
loadCredentialGroupEnrollmentAccessForSubject: mocks.loadEnrollmentAccess,
13+
}))
14+
15+
import { requireCredentialGroupEnrollmentAccess } from '@/lib/credential-groups/application/authorization'
16+
17+
function executorPrincipal(): WorkflowExecutionDelegatedPrincipal {
18+
return {
19+
kind: 'delegated',
20+
serviceId: 'executor',
21+
workspaceId: 'workspace-1',
22+
delegationId: 'delegation-1',
23+
audience: 'sim:managed-oauth-credentials',
24+
issuedAt: new Date(Date.now() - 1_000),
25+
expiresAt: new Date(Date.now() + 60_000),
26+
delegationContext: {
27+
kind: 'workflow_execution',
28+
workflowId: 'workflow-1',
29+
principal: {
30+
kind: 'system',
31+
serviceId: 'webhook',
32+
workspaceId: 'workspace-1',
33+
workflowId: 'workflow-1',
34+
webhookId: 'webhook-1',
35+
provider: 'slack',
36+
subject: {
37+
kind: 'external_user',
38+
provider: 'slack',
39+
tenantId: 'T123',
40+
subjectId: 'U123',
41+
},
42+
},
43+
},
44+
}
45+
}
46+
47+
describe('requireCredentialGroupEnrollmentAccess', () => {
48+
beforeEach(() => {
49+
vi.clearAllMocks()
50+
mocks.loadEnrollmentAccess.mockResolvedValue({
51+
enrollmentId: 'enrollment-1',
52+
email: 'person@example.com',
53+
})
54+
})
55+
56+
it('resolves an external workflow actor to their enrollment', async () => {
57+
const principal = executorPrincipal()
58+
59+
await expect(requireCredentialGroupEnrollmentAccess(principal, 'group-1')).resolves.toEqual({
60+
enrollmentId: 'enrollment-1',
61+
email: 'person@example.com',
62+
})
63+
expect(mocks.loadEnrollmentAccess).toHaveBeenCalledWith('group-1', {
64+
kind: 'external_user',
65+
provider: 'slack',
66+
tenantId: 'T123',
67+
subjectId: 'U123',
68+
})
69+
})
70+
71+
it('rejects an actorless workflow principal', async () => {
72+
const principal = executorPrincipal()
73+
principal.delegationContext!.principal = {
74+
kind: 'system',
75+
serviceId: 'schedule',
76+
workspaceId: 'workspace-1',
77+
workflowId: 'workflow-1',
78+
}
79+
80+
await expect(
81+
requireCredentialGroupEnrollmentAccess(principal, 'group-1')
82+
).rejects.toMatchObject({
83+
code: 'forbidden',
84+
message: 'Credential Group enrollment access required',
85+
})
86+
expect(mocks.loadEnrollmentAccess).not.toHaveBeenCalled()
87+
})
88+
89+
it('rejects an executor delegation whose Sim subject does not match the workflow actor', async () => {
90+
const principal = executorPrincipal()
91+
principal.subjectUserId = 'user-2'
92+
principal.delegationContext!.principal = {
93+
kind: 'session',
94+
userId: 'user-1',
95+
sessionId: 'session-1',
96+
}
97+
98+
await expect(
99+
requireCredentialGroupEnrollmentAccess(principal, 'group-1')
100+
).rejects.toMatchObject({ code: 'forbidden' })
101+
expect(mocks.loadEnrollmentAccess).not.toHaveBeenCalled()
102+
})
103+
})

‎apps/sim/lib/credential-groups/application/authorization.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
type Principal,
33
requirePrincipalSubjectUserId,
4+
resolvePrincipalSubject,
45
type WorkflowExecutionDelegatedPrincipal,
56
} from '@sim/auth/principal'
67
import type {
@@ -11,7 +12,7 @@ import { OrchestrationError } from '@/lib/core/orchestration/types'
1112
import type { CredentialGroupCredentialListContext } from '@/lib/credential-groups/credentials'
1213
import {
1314
type CredentialGroupEnrollmentAccess,
14-
loadCredentialGroupEnrollmentAccess,
15+
loadCredentialGroupEnrollmentAccessForSubject,
1516
} from '@/lib/credential-groups/credentials'
1617

1718
export const CREDENTIAL_GROUP_DELEGATION_AUDIENCE = 'sim:credential-groups'
@@ -52,13 +53,18 @@ export async function requireCredentialGroupEnrollmentAccess(
5253
principal: Principal,
5354
credentialGroupId: string
5455
): Promise<CredentialGroupEnrollmentAccess> {
55-
let subjectUserId: string
56-
try {
57-
subjectUserId = requireCredentialGroupWorkflowSubject(principal)
58-
} catch {
56+
const executionPrincipal = requireWorkflowExecutionPrincipal(principal)
57+
const subject = resolvePrincipalSubject(executionPrincipal)
58+
if (!subject) {
59+
throw new OrchestrationError('forbidden', 'Credential Group enrollment access required')
60+
}
61+
if (
62+
subject.kind === 'sim_user' &&
63+
(principal.kind !== 'delegated' || principal.subjectUserId !== subject.userId)
64+
) {
5965
throw new OrchestrationError('forbidden', 'Credential Group enrollment access required')
6066
}
61-
const access = await loadCredentialGroupEnrollmentAccess(credentialGroupId, subjectUserId)
67+
const access = await loadCredentialGroupEnrollmentAccessForSubject(credentialGroupId, subject)
6268
if (!access) {
6369
throw new OrchestrationError('forbidden', 'Credential Group enrollment access required')
6470
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ vi.mock('@/lib/credential-groups/credentials', () => ({
3030
}
3131
},
3232
listCredentialGroupCredentialReferences: mocks.listCredentials,
33-
loadCredentialGroupEnrollmentAccess: mocks.loadEnrollmentAccess,
33+
loadCredentialGroupEnrollmentAccessForSubject: mocks.loadEnrollmentAccess,
3434
loadCredentialGroupCredentialListContext: mocks.loadGroup,
3535
MAX_CREDENTIAL_GROUP_CREDENTIAL_PAGE_SIZE: 100,
3636
}))

‎apps/sim/lib/credential-groups/credentials.test.ts‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,16 @@
33
*/
44
import { dbChainMockFns, hasMockCondition, resetDbChainMock } from '@sim/testing'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
6-
import { listCredentialGroupCredentialReferences } from '@/lib/credential-groups/credentials'
6+
7+
vi.mock('@/lib/credential-groups/providers', () => ({
8+
isCredentialGroupProvider: (provider: string) => provider === 'slack',
9+
getCredentialGroupProviderId: () => 'slack',
10+
}))
11+
12+
import {
13+
listCredentialGroupCredentialReferences,
14+
loadCredentialGroupEnrollmentAccessForSubject,
15+
} from '@/lib/credential-groups/credentials'
716

817
describe('listCredentialGroupCredentialReferences', () => {
918
beforeEach(() => {
@@ -65,4 +74,35 @@ describe('listCredentialGroupCredentialReferences', () => {
6574
)
6675
).toBe(true)
6776
})
77+
78+
it('resolves a Slack subject by stable tenant and user identifiers', async () => {
79+
dbChainMockFns.limit.mockResolvedValueOnce([
80+
{ enrollmentId: 'enrollment-1', email: 'person@example.com' },
81+
])
82+
83+
await expect(
84+
loadCredentialGroupEnrollmentAccessForSubject('group-1', {
85+
kind: 'external_user',
86+
provider: 'slack',
87+
tenantId: 'T123',
88+
subjectId: 'U123',
89+
})
90+
).resolves.toEqual({ enrollmentId: 'enrollment-1', email: 'person@example.com' })
91+
})
92+
93+
it('fails fast when one external subject resolves to multiple enrollments', async () => {
94+
dbChainMockFns.limit.mockResolvedValueOnce([
95+
{ enrollmentId: 'enrollment-1', email: 'first@example.com' },
96+
{ enrollmentId: 'enrollment-2', email: 'second@example.com' },
97+
])
98+
99+
await expect(
100+
loadCredentialGroupEnrollmentAccessForSubject('group-1', {
101+
kind: 'external_user',
102+
provider: 'slack',
103+
tenantId: 'T123',
104+
subjectId: 'U123',
105+
})
106+
).rejects.toThrow('multiple Credential Group enrollments')
107+
})
68108
})

‎apps/sim/lib/credential-groups/credentials.ts‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { PrincipalSubject } from '@sim/auth/principal'
12
import { db } from '@sim/db'
23
import {
34
type CredentialGroupOptionConfig,
@@ -6,7 +7,11 @@ import {
67
credentialGroupEnrollment,
78
user,
89
} from '@sim/db/schema'
9-
import { and, asc, eq, gt, inArray, or } from 'drizzle-orm'
10+
import { and, asc, eq, gt, inArray, or, sql } from 'drizzle-orm'
11+
import {
12+
getCredentialGroupProviderId,
13+
isCredentialGroupProvider,
14+
} from '@/lib/credential-groups/providers'
1015

1116
export const MAX_CREDENTIAL_GROUP_CREDENTIAL_PAGE_SIZE = 100
1217

@@ -60,7 +65,7 @@ export async function loadCredentialGroupEnrollmentAccess(
6065
email: credentialGroupEnrollment.email,
6166
})
6267
.from(credentialGroupEnrollment)
63-
.innerJoin(user, eq(user.normalizedEmail, credentialGroupEnrollment.email))
68+
.innerJoin(user, eq(sql<string>`lower(btrim(${user.email}))`, credentialGroupEnrollment.email))
6469
.where(
6570
and(
6671
eq(user.id, userId),
@@ -73,6 +78,41 @@ export async function loadCredentialGroupEnrollmentAccess(
7378
return row ?? null
7479
}
7580

81+
/** Resolves a verified Sim or provider subject to exactly one active enrollment. */
82+
export async function loadCredentialGroupEnrollmentAccessForSubject(
83+
credentialGroupId: string,
84+
subject: PrincipalSubject
85+
): Promise<CredentialGroupEnrollmentAccess | null> {
86+
if (subject.kind === 'sim_user') {
87+
return loadCredentialGroupEnrollmentAccess(credentialGroupId, subject.userId)
88+
}
89+
if (!isCredentialGroupProvider(subject.provider)) return null
90+
const providerId = getCredentialGroupProviderId(subject.provider)
91+
const rows = await db
92+
.selectDistinct({
93+
enrollmentId: credentialGroupEnrollment.id,
94+
email: credentialGroupEnrollment.email,
95+
})
96+
.from(credentialGroupEnrollment)
97+
.innerJoin(credential, eq(credential.credentialGroupEnrollmentId, credentialGroupEnrollment.id))
98+
.where(
99+
and(
100+
eq(credentialGroupEnrollment.credentialGroupId, credentialGroupId),
101+
inArray(credentialGroupEnrollment.status, ['in_progress', 'completed']),
102+
eq(credential.type, 'managed_oauth'),
103+
eq(credential.managedOauthStatus, 'active'),
104+
eq(credential.providerId, providerId),
105+
eq(credential.providerTenantId, subject.tenantId),
106+
eq(credential.providerSubjectId, subject.subjectId)
107+
)
108+
)
109+
.limit(2)
110+
if (rows.length > 1) {
111+
throw new Error('External subject resolves to multiple Credential Group enrollments')
112+
}
113+
return rows[0] ?? null
114+
}
115+
76116
/** Loads the canonical group ownership needed by the application authorization boundary. */
77117
export async function loadCredentialGroupCredentialListContext(
78118
credentialGroupId: string

0 commit comments

Comments
 (0)