Skip to content

Commit 62d0800

Browse files
TheodoreSpeaksTheodoreSpeaks
authored
fix(credential-groups): stop requiring a human subject on workflow ops (#7275)
A Slack-triggered run's subject is the external Slack user, and a schedule or public-API run has no subject at all. list_groups, list_people, and send_invite demanded a Sim user, so every unattended run got "Credential Group user access required" — including reads that need no actor. Authority for an actorless caller comes from the deployment the workspace layer already checks. Invitations no longer name an inviter when there is no person to name, rather than borrowing the run's actor and claiming someone invited when they did not.
1 parent f516361 commit 62d0800

11 files changed

Lines changed: 333 additions & 39 deletions

File tree

‎apps/sim/app/credential-groups/enroll/[token]/page.tsx‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,15 @@ export default async function CredentialGroupEnrollmentPage({
134134
Connect your accounts
135135
</h1>
136136
<p className='mt-4 max-w-[560px] text-pretty text-[var(--text-muted)] text-base leading-relaxed'>
137-
<span className='font-medium text-[var(--text-body)]'>{enrollment.inviterName}</span>{' '}
138-
invited you to connect accounts for{' '}
137+
{enrollment.inviterName ? (
138+
<>
139+
<span className='font-medium text-[var(--text-body)]'>{enrollment.inviterName}</span>{' '}
140+
invited you
141+
</>
142+
) : (
143+
'You have been invited'
144+
)}{' '}
145+
to connect accounts for{' '}
139146
<span className='font-medium text-[var(--text-body)]'>{enrollment.workspaceName}</span>.
140147
</p>
141148
</header>

‎apps/sim/components/emails/credential-groups/credential-group-invitation-email.tsx‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import { getBrandConfig } from '@/ee/whitelabeling'
55

66
interface CredentialGroupInvitationEmailProps {
77
recipientEmail: string
8-
inviterName: string
8+
/** Absent when a workflow issued the invitation: there is no person to name. */
9+
inviterName?: string
910
workspaceName: string
1011
credentialGroupName: string
1112
invitationLink: string
@@ -22,14 +23,26 @@ export function CredentialGroupInvitationEmail({
2223

2324
return (
2425
<EmailLayout
25-
preview={`${inviterName} invited you to connect accounts for ${workspaceName}`}
26+
preview={
27+
inviterName
28+
? `${inviterName} invited you to connect accounts for ${workspaceName}`
29+
: `You have been invited to connect accounts for ${workspaceName}`
30+
}
2631
showUnsubscribe={false}
2732
>
2833
<Text style={baseStyles.paragraph}>Hello,</Text>
2934
<Text style={baseStyles.paragraph}>
30-
<strong>{inviterName}</strong> invited <strong>{recipientEmail}</strong> to connect accounts
31-
for <strong>{credentialGroupName}</strong> in the <strong>{workspaceName}</strong> workspace
32-
on {brand.name}.
35+
{inviterName ? (
36+
<>
37+
<strong>{inviterName}</strong> invited <strong>{recipientEmail}</strong>
38+
</>
39+
) : (
40+
<>
41+
<strong>{recipientEmail}</strong> has been invited
42+
</>
43+
)}{' '}
44+
to connect accounts for <strong>{credentialGroupName}</strong> in the{' '}
45+
<strong>{workspaceName}</strong> workspace on {brand.name}.
3346
</Text>
3447

3548
<Link href={invitationLink} style={{ textDecoration: 'none' }}>

‎apps/sim/components/emails/credential-groups/render.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { CredentialGroupInvitationEmail } from '@/components/emails/credential-g
33

44
export async function renderCredentialGroupInvitationEmail(params: {
55
recipientEmail: string
6-
inviterName: string
6+
inviterName?: string
77
workspaceName: string
88
credentialGroupName: string
99
invitationLink: string

‎apps/sim/components/emails/subjects.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,16 @@ export function getOtpSubject(resourceLabel: string): string {
110110
return `Verification code for ${resourceLabel}`
111111
}
112112

113-
/** Names both the inviter and workspace so an external recipient can identify the request. */
113+
/**
114+
* Names the workspace so an external recipient can identify the request, and the
115+
* inviter when there is one — a workflow-issued invitation has no person to name.
116+
*/
114117
export function getCredentialGroupInvitationSubject(
115-
inviterName: string,
118+
inviterName: string | undefined,
116119
workspaceName: string
117120
): string {
118-
return `${inviterName} invited you to connect accounts for ${workspaceName} on ${getBrandConfig().name}`
121+
const brandName = getBrandConfig().name
122+
return inviterName
123+
? `${inviterName} invited you to connect accounts for ${workspaceName} on ${brandName}`
124+
: `You have been invited to connect accounts for ${workspaceName} on ${brandName}`
119125
}

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

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ vi.mock('@/lib/resource-policies/repository', () => ({
2020
requireResourcePolicy: mocks.requirePolicy,
2121
}))
2222

23-
import { requireCredentialGroupCredentialAccess } from '@/lib/credential-groups/application/authorization'
23+
import {
24+
requireCredentialGroupCredentialAccess,
25+
requireCredentialGroupWorkflowActor,
26+
} from '@/lib/credential-groups/application/authorization'
2427

2528
const context = {
2629
workspaceId: 'workspace-1',
@@ -216,3 +219,60 @@ describe('requireCredentialGroupCredentialAccess', () => {
216219
expect(mocks.loadEnrollmentAccess).not.toHaveBeenCalled()
217220
})
218221
})
222+
223+
describe('requireCredentialGroupWorkflowActor', () => {
224+
it('returns the external subject a Slack-triggered run acts as', () => {
225+
expect(requireCredentialGroupWorkflowActor(executorPrincipal())).toEqual({
226+
kind: 'external_user',
227+
provider: 'slack',
228+
tenantId: 'T123',
229+
subjectId: 'U123',
230+
})
231+
})
232+
233+
it('returns no subject for an actorless deployed run', () => {
234+
const principal = executorPrincipal()
235+
principal.delegationContext!.principal = {
236+
kind: 'system',
237+
serviceId: 'schedule',
238+
workspaceId: 'workspace-1',
239+
workflowId: 'root-workflow',
240+
}
241+
242+
expect(requireCredentialGroupWorkflowActor(principal)).toBeNull()
243+
})
244+
245+
it('returns the Sim subject a session-actor run acts as', () => {
246+
const principal = executorPrincipal()
247+
principal.subjectUserId = 'user-1'
248+
principal.delegationContext!.principal = {
249+
kind: 'session',
250+
userId: 'user-1',
251+
sessionId: 'session-1',
252+
}
253+
254+
expect(requireCredentialGroupWorkflowActor(principal)).toEqual({
255+
kind: 'sim_user',
256+
userId: 'user-1',
257+
})
258+
})
259+
260+
it('rejects a delegation whose asserted subject contradicts its run', () => {
261+
const invented = executorPrincipal()
262+
invented.subjectUserId = 'invented-user'
263+
expect(() => requireCredentialGroupWorkflowActor(invented)).toThrow(
264+
'Credential Group actor access required'
265+
)
266+
267+
const mismatched = executorPrincipal()
268+
mismatched.subjectUserId = 'user-2'
269+
mismatched.delegationContext!.principal = {
270+
kind: 'session',
271+
userId: 'user-1',
272+
sessionId: 'session-1',
273+
}
274+
expect(() => requireCredentialGroupWorkflowActor(mismatched)).toThrow(
275+
'Credential Group actor access required'
276+
)
277+
})
278+
})

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
type Principal,
3+
type PrincipalSubject,
34
resolvePrincipalSubject,
45
type WorkflowExecutionAuthority,
56
type WorkflowExecutionPrincipal,
@@ -67,16 +68,19 @@ function requireConsistentWorkflowSubject(
6768
return subject
6869
}
6970

70-
export function requireCredentialGroupWorkflowSubject(principal: Principal): string {
71-
const subject = resolvePrincipalSubject(requireWorkflowExecutionPrincipal(principal))
72-
if (
73-
subject?.kind !== 'sim_user' ||
74-
principal.kind !== 'delegated' ||
75-
principal.subjectUserId !== subject.userId
76-
) {
77-
throw new OrchestrationError('forbidden', 'Credential Group user access required')
78-
}
79-
return subject.userId
71+
/**
72+
* Asserts the delegation still names the subject its run was minted for, without
73+
* requiring that subject to be a Sim user.
74+
*
75+
* A Slack-triggered run's subject is the external Slack user, and a scheduled,
76+
* public-API, or subject-less webhook run has no subject at all. Neither is
77+
* representable as a Sim user, and neither is what authorizes the call — for an
78+
* actorless caller that is the deployment the workspace layer already checked.
79+
* Whoever the run acts as is attribution only; an invitation issued with no Sim
80+
* user simply records none.
81+
*/
82+
export function requireCredentialGroupWorkflowActor(principal: Principal): PrincipalSubject | null {
83+
return requireConsistentWorkflowSubject(principal, requireWorkflowExecutionPrincipal(principal))
8084
}
8185

8286
export async function requireCredentialGroupCredentialAccess(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
22
import { OrchestrationError } from '@/lib/core/orchestration/types'
33
import {
44
credentialGroupWorkspaceDelegationPolicy,
5-
requireCredentialGroupWorkflowSubject,
5+
requireCredentialGroupWorkflowActor,
66
} from '@/lib/credential-groups/application/authorization'
77
import {
88
requireCredentialGroupsAvailable,
@@ -35,7 +35,7 @@ export const listCredentialGroupsForWorkflow = defineAuthorizedWorkspaceUseCase(
3535
resolveCredentialGroupWorkspaceContext(input.workspaceId),
3636
authorizationOptions: { delegation: credentialGroupWorkspaceDelegationPolicy },
3737
authorizeResource({ principal }) {
38-
requireCredentialGroupWorkflowSubject(principal)
38+
requireCredentialGroupWorkflowActor(principal)
3939
},
4040
execute: async ({ input, context }): Promise<ListCredentialGroupsResult> => {
4141
if (

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
33
import { OrchestrationError } from '@/lib/core/orchestration/types'
44
import {
55
credentialGroupDelegationPolicy,
6-
requireCredentialGroupWorkflowSubject,
6+
requireCredentialGroupWorkflowActor,
77
} from '@/lib/credential-groups/application/authorization'
88
import {
99
requireCredentialGroupsAvailable,
@@ -38,7 +38,7 @@ export const listCredentialGroupPeople = defineAuthorizedWorkspaceUseCase({
3838
resolveCredentialGroupContext(input.credentialGroupId),
3939
authorizationOptions: { delegation: credentialGroupDelegationPolicy },
4040
authorizeResource({ principal }) {
41-
requireCredentialGroupWorkflowSubject(principal)
41+
requireCredentialGroupWorkflowActor(principal)
4242
},
4343
execute: async ({ input, context }) => {
4444
if (context.status !== 'active') {

0 commit comments

Comments
 (0)