From fb3d452385dc52b9bb070c896861f4ae5f2273b3 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Sat, 29 Aug 2026 00:41:02 -0700 Subject: [PATCH] fix(credential-groups): stop requiring a human subject on workflow ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../credential-groups/enroll/[token]/page.tsx | 11 +- .../credential-group-invitation-email.tsx | 23 +- .../emails/credential-groups/render.ts | 2 +- apps/sim/components/emails/subjects.ts | 12 +- .../application/authorization.test.ts | 62 +++++- .../application/authorization.ts | 24 ++- .../application/list-groups.ts | 4 +- .../application/list-people.ts | 4 +- .../application/send-invite.test.ts | 199 ++++++++++++++++++ .../application/send-invite.ts | 14 +- apps/sim/lib/credential-groups/enrollments.ts | 17 +- 11 files changed, 333 insertions(+), 39 deletions(-) create mode 100644 apps/sim/lib/credential-groups/application/send-invite.test.ts diff --git a/apps/sim/app/credential-groups/enroll/[token]/page.tsx b/apps/sim/app/credential-groups/enroll/[token]/page.tsx index fcafbdb8af5..3336b84ad1b 100644 --- a/apps/sim/app/credential-groups/enroll/[token]/page.tsx +++ b/apps/sim/app/credential-groups/enroll/[token]/page.tsx @@ -134,8 +134,15 @@ export default async function CredentialGroupEnrollmentPage({ Connect your accounts

- {enrollment.inviterName}{' '} - invited you to connect accounts for{' '} + {enrollment.inviterName ? ( + <> + {enrollment.inviterName}{' '} + invited you + + ) : ( + 'You have been invited' + )}{' '} + to connect accounts for{' '} {enrollment.workspaceName}.

diff --git a/apps/sim/components/emails/credential-groups/credential-group-invitation-email.tsx b/apps/sim/components/emails/credential-groups/credential-group-invitation-email.tsx index 1e2383a9579..0b0e4f04650 100644 --- a/apps/sim/components/emails/credential-groups/credential-group-invitation-email.tsx +++ b/apps/sim/components/emails/credential-groups/credential-group-invitation-email.tsx @@ -5,7 +5,8 @@ import { getBrandConfig } from '@/ee/whitelabeling' interface CredentialGroupInvitationEmailProps { recipientEmail: string - inviterName: string + /** Absent when a workflow issued the invitation: there is no person to name. */ + inviterName?: string workspaceName: string credentialGroupName: string invitationLink: string @@ -22,14 +23,26 @@ export function CredentialGroupInvitationEmail({ return ( Hello, - {inviterName} invited {recipientEmail} to connect accounts - for {credentialGroupName} in the {workspaceName} workspace - on {brand.name}. + {inviterName ? ( + <> + {inviterName} invited {recipientEmail} + + ) : ( + <> + {recipientEmail} has been invited + + )}{' '} + to connect accounts for {credentialGroupName} in the{' '} + {workspaceName} workspace on {brand.name}. diff --git a/apps/sim/components/emails/credential-groups/render.ts b/apps/sim/components/emails/credential-groups/render.ts index 63779541237..f5bc8188fb0 100644 --- a/apps/sim/components/emails/credential-groups/render.ts +++ b/apps/sim/components/emails/credential-groups/render.ts @@ -3,7 +3,7 @@ import { CredentialGroupInvitationEmail } from '@/components/emails/credential-g export async function renderCredentialGroupInvitationEmail(params: { recipientEmail: string - inviterName: string + inviterName?: string workspaceName: string credentialGroupName: string invitationLink: string diff --git a/apps/sim/components/emails/subjects.ts b/apps/sim/components/emails/subjects.ts index e2d5f71fa95..c803df0eaab 100644 --- a/apps/sim/components/emails/subjects.ts +++ b/apps/sim/components/emails/subjects.ts @@ -110,10 +110,16 @@ export function getOtpSubject(resourceLabel: string): string { return `Verification code for ${resourceLabel}` } -/** Names both the inviter and workspace so an external recipient can identify the request. */ +/** + * Names the workspace so an external recipient can identify the request, and the + * inviter when there is one — a workflow-issued invitation has no person to name. + */ export function getCredentialGroupInvitationSubject( - inviterName: string, + inviterName: string | undefined, workspaceName: string ): string { - return `${inviterName} invited you to connect accounts for ${workspaceName} on ${getBrandConfig().name}` + const brandName = getBrandConfig().name + return inviterName + ? `${inviterName} invited you to connect accounts for ${workspaceName} on ${brandName}` + : `You have been invited to connect accounts for ${workspaceName} on ${brandName}` } diff --git a/apps/sim/lib/credential-groups/application/authorization.test.ts b/apps/sim/lib/credential-groups/application/authorization.test.ts index 052dfba31f9..fec8b1a7bbb 100644 --- a/apps/sim/lib/credential-groups/application/authorization.test.ts +++ b/apps/sim/lib/credential-groups/application/authorization.test.ts @@ -20,7 +20,10 @@ vi.mock('@/lib/resource-policies/repository', () => ({ requireResourcePolicy: mocks.requirePolicy, })) -import { requireCredentialGroupCredentialAccess } from '@/lib/credential-groups/application/authorization' +import { + requireCredentialGroupCredentialAccess, + requireCredentialGroupWorkflowActor, +} from '@/lib/credential-groups/application/authorization' const context = { workspaceId: 'workspace-1', @@ -216,3 +219,60 @@ describe('requireCredentialGroupCredentialAccess', () => { expect(mocks.loadEnrollmentAccess).not.toHaveBeenCalled() }) }) + +describe('requireCredentialGroupWorkflowActor', () => { + it('returns the external subject a Slack-triggered run acts as', () => { + expect(requireCredentialGroupWorkflowActor(executorPrincipal())).toEqual({ + kind: 'external_user', + provider: 'slack', + tenantId: 'T123', + subjectId: 'U123', + }) + }) + + it('returns no subject for an actorless deployed run', () => { + const principal = executorPrincipal() + principal.delegationContext!.principal = { + kind: 'system', + serviceId: 'schedule', + workspaceId: 'workspace-1', + workflowId: 'root-workflow', + } + + expect(requireCredentialGroupWorkflowActor(principal)).toBeNull() + }) + + it('returns the Sim subject a session-actor run acts as', () => { + const principal = executorPrincipal() + principal.subjectUserId = 'user-1' + principal.delegationContext!.principal = { + kind: 'session', + userId: 'user-1', + sessionId: 'session-1', + } + + expect(requireCredentialGroupWorkflowActor(principal)).toEqual({ + kind: 'sim_user', + userId: 'user-1', + }) + }) + + it('rejects a delegation whose asserted subject contradicts its run', () => { + const invented = executorPrincipal() + invented.subjectUserId = 'invented-user' + expect(() => requireCredentialGroupWorkflowActor(invented)).toThrow( + 'Credential Group actor access required' + ) + + const mismatched = executorPrincipal() + mismatched.subjectUserId = 'user-2' + mismatched.delegationContext!.principal = { + kind: 'session', + userId: 'user-1', + sessionId: 'session-1', + } + expect(() => requireCredentialGroupWorkflowActor(mismatched)).toThrow( + 'Credential Group actor access required' + ) + }) +}) diff --git a/apps/sim/lib/credential-groups/application/authorization.ts b/apps/sim/lib/credential-groups/application/authorization.ts index 2cde1459083..13cc444e606 100644 --- a/apps/sim/lib/credential-groups/application/authorization.ts +++ b/apps/sim/lib/credential-groups/application/authorization.ts @@ -1,5 +1,6 @@ import { type Principal, + type PrincipalSubject, resolvePrincipalSubject, type WorkflowExecutionAuthority, type WorkflowExecutionPrincipal, @@ -67,16 +68,19 @@ function requireConsistentWorkflowSubject( return subject } -export function requireCredentialGroupWorkflowSubject(principal: Principal): string { - const subject = resolvePrincipalSubject(requireWorkflowExecutionPrincipal(principal)) - if ( - subject?.kind !== 'sim_user' || - principal.kind !== 'delegated' || - principal.subjectUserId !== subject.userId - ) { - throw new OrchestrationError('forbidden', 'Credential Group user access required') - } - return subject.userId +/** + * Asserts the delegation still names the subject its run was minted for, without + * requiring that subject to be a Sim user. + * + * A Slack-triggered run's subject is the external Slack user, and a scheduled, + * public-API, or subject-less webhook run has no subject at all. Neither is + * representable as a Sim user, and neither is what authorizes the call — for an + * actorless caller that is the deployment the workspace layer already checked. + * Whoever the run acts as is attribution only; an invitation issued with no Sim + * user simply records none. + */ +export function requireCredentialGroupWorkflowActor(principal: Principal): PrincipalSubject | null { + return requireConsistentWorkflowSubject(principal, requireWorkflowExecutionPrincipal(principal)) } export async function requireCredentialGroupCredentialAccess( diff --git a/apps/sim/lib/credential-groups/application/list-groups.ts b/apps/sim/lib/credential-groups/application/list-groups.ts index d33938db34e..75599f6734e 100644 --- a/apps/sim/lib/credential-groups/application/list-groups.ts +++ b/apps/sim/lib/credential-groups/application/list-groups.ts @@ -2,7 +2,7 @@ import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { credentialGroupWorkspaceDelegationPolicy, - requireCredentialGroupWorkflowSubject, + requireCredentialGroupWorkflowActor, } from '@/lib/credential-groups/application/authorization' import { requireCredentialGroupsAvailable, @@ -35,7 +35,7 @@ export const listCredentialGroupsForWorkflow = defineAuthorizedWorkspaceUseCase( resolveCredentialGroupWorkspaceContext(input.workspaceId), authorizationOptions: { delegation: credentialGroupWorkspaceDelegationPolicy }, authorizeResource({ principal }) { - requireCredentialGroupWorkflowSubject(principal) + requireCredentialGroupWorkflowActor(principal) }, execute: async ({ input, context }): Promise => { if ( diff --git a/apps/sim/lib/credential-groups/application/list-people.ts b/apps/sim/lib/credential-groups/application/list-people.ts index 10338d8defe..6d5d2ae9043 100644 --- a/apps/sim/lib/credential-groups/application/list-people.ts +++ b/apps/sim/lib/credential-groups/application/list-people.ts @@ -3,7 +3,7 @@ import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { credentialGroupDelegationPolicy, - requireCredentialGroupWorkflowSubject, + requireCredentialGroupWorkflowActor, } from '@/lib/credential-groups/application/authorization' import { requireCredentialGroupsAvailable, @@ -38,7 +38,7 @@ export const listCredentialGroupPeople = defineAuthorizedWorkspaceUseCase({ resolveCredentialGroupContext(input.credentialGroupId), authorizationOptions: { delegation: credentialGroupDelegationPolicy }, authorizeResource({ principal }) { - requireCredentialGroupWorkflowSubject(principal) + requireCredentialGroupWorkflowActor(principal) }, execute: async ({ input, context }) => { if (context.status !== 'active') { diff --git a/apps/sim/lib/credential-groups/application/send-invite.test.ts b/apps/sim/lib/credential-groups/application/send-invite.test.ts new file mode 100644 index 00000000000..d72495678fa --- /dev/null +++ b/apps/sim/lib/credential-groups/application/send-invite.test.ts @@ -0,0 +1,199 @@ +/** + * @vitest-environment node + */ +import type { WorkflowExecutionDelegatedPrincipal } from '@sim/auth/principal' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + inviteEnrollment: vi.fn(), + loadInviter: vi.fn(), + requireAvailable: vi.fn(), + resolveGroup: vi.fn(), + resolvePermission: vi.fn(), +})) + +vi.mock('@/lib/credential-groups/application/context', () => ({ + requireCredentialGroupsAvailable: mocks.requireAvailable, + resolveCredentialGroupContext: mocks.resolveGroup, +})) + +vi.mock('@/lib/credential-groups/enrollments', () => ({ + inviteCredentialGroupEnrollment: mocks.inviteEnrollment, + loadCredentialGroupInviterIdentity: mocks.loadInviter, + CredentialGroupEnrollmentError: class CredentialGroupEnrollmentError extends Error { + constructor( + message: string, + readonly status: 400 | 404 | 409 | 502 + ) { + super(message) + } + }, +})) + +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: (permission: string | null, required: string) => + permission === 'admin' || permission === required, + resolveEffectiveWorkspacePermission: mocks.resolvePermission, +})) + +import { sendCredentialGroupInvite } from '@/lib/credential-groups/application/send-invite' + +const context = { + workspaceId: 'workspace-1', + workspaceOrganizationId: null, + allowPersonalApiKeys: true, + billedAccountUserId: 'billing-owner-1', + credentialGroupId: 'group-1', + name: 'Support', + status: 'active' as const, + options: [], +} + +function executorPrincipal(): WorkflowExecutionDelegatedPrincipal { + return { + kind: 'delegated', + serviceId: 'executor', + subjectUserId: 'admin-1', + workspaceId: 'workspace-1', + delegationId: 'delegation-1', + audience: 'sim:credential-groups', + issuedAt: new Date(Date.now() - 1_000), + expiresAt: new Date(Date.now() + 60_000), + resourceScope: { credentialGroupId: 'group-1' }, + delegationContext: { + kind: 'workflow_execution', + workflowId: 'workflow-1', + principal: { kind: 'session', userId: 'admin-1', sessionId: 'session-1' }, + currentWorkflow: { workflowId: 'workflow-1', mode: 'draft' }, + }, + } +} + +/** A deployed run whose only actor is the external identity that triggered it. */ +function unattendedPrincipal( + principal: NonNullable['principal'] +): WorkflowExecutionDelegatedPrincipal { + const { subjectUserId: _subject, ...base } = executorPrincipal() + return { + ...base, + delegationContext: { + kind: 'workflow_execution', + workflowId: 'workflow-1', + principal, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'version-1', + }, + }, + } +} + +function slackPrincipal(): WorkflowExecutionDelegatedPrincipal { + return unattendedPrincipal({ + kind: 'system', + serviceId: 'webhook', + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + webhookId: 'webhook-1', + provider: 'slack', + subject: { kind: 'external_user', provider: 'slack', tenantId: 'T123', subjectId: 'U123' }, + }) +} + +function invite(principal: WorkflowExecutionDelegatedPrincipal) { + return sendCredentialGroupInvite.execute({ + principal, + input: { credentialGroupId: 'group-1', email: 'person@example.com' }, + }) +} + +describe('sendCredentialGroupInvite', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.resolveGroup.mockResolvedValue(context) + mocks.resolvePermission.mockResolvedValue('admin') + mocks.requireAvailable.mockResolvedValue(undefined) + mocks.loadInviter.mockResolvedValue({ name: 'Ada Lovelace', email: 'ada@example.com' }) + mocks.inviteEnrollment.mockResolvedValue({ + id: 'enrollment-1', + email: 'person@example.com', + status: 'invited', + }) + }) + + it('invites without naming an inviter on a Slack-triggered run', async () => { + const result = await invite(slackPrincipal()) + + expect(result.enrollment.id).toBe('enrollment-1') + expect(mocks.loadInviter).not.toHaveBeenCalled() + expect(mocks.inviteEnrollment).toHaveBeenCalledWith( + 'workspace-1', + 'group-1', + undefined, + undefined, + 'person@example.com' + ) + }) + + it('invites without naming an inviter on an actorless run', async () => { + await invite( + unattendedPrincipal({ + kind: 'system', + serviceId: 'schedule', + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + }) + ) + + expect(mocks.inviteEnrollment).toHaveBeenCalledWith( + 'workspace-1', + 'group-1', + undefined, + undefined, + 'person@example.com' + ) + }) + + it('names the human a session-actor run acts as', async () => { + await invite(executorPrincipal()) + + expect(mocks.loadInviter).toHaveBeenCalledWith('admin-1') + expect(mocks.inviteEnrollment).toHaveBeenCalledWith( + 'workspace-1', + 'group-1', + 'admin-1', + 'Ada Lovelace', + 'person@example.com' + ) + }) + + it('falls back to the inviter email when they have no name', async () => { + mocks.loadInviter.mockResolvedValue({ name: ' ', email: 'ada@example.com' }) + + await invite(executorPrincipal()) + + expect(mocks.inviteEnrollment).toHaveBeenCalledWith( + 'workspace-1', + 'group-1', + 'admin-1', + 'ada@example.com', + 'person@example.com' + ) + }) + + it('requires the current subject to remain a workspace admin', async () => { + mocks.resolvePermission.mockResolvedValue('write') + + await expect(invite(executorPrincipal())).rejects.toMatchObject({ code: 'forbidden' }) + expect(mocks.inviteEnrollment).not.toHaveBeenCalled() + }) + + it('rejects a delegation asserting a subject its run never had', async () => { + const spoofed = slackPrincipal() + spoofed.subjectUserId = 'invented-user' + + await expect(invite(spoofed)).rejects.toMatchObject({ code: 'forbidden' }) + expect(mocks.inviteEnrollment).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/credential-groups/application/send-invite.ts b/apps/sim/lib/credential-groups/application/send-invite.ts index 31ed72e9330..3e970b81675 100644 --- a/apps/sim/lib/credential-groups/application/send-invite.ts +++ b/apps/sim/lib/credential-groups/application/send-invite.ts @@ -1,10 +1,11 @@ import { AuditAction, AuditResourceType } from '@sim/audit' +import { resolvePrincipalSubjectUserId } from '@sim/auth/principal' import { isValidEmailSyntax, normalizeEmail } from '@sim/utils/string' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { credentialGroupDelegationPolicy, - requireCredentialGroupWorkflowSubject, + requireCredentialGroupWorkflowActor, } from '@/lib/credential-groups/application/authorization' import { requireCredentialGroupsAvailable, @@ -28,7 +29,7 @@ export const sendCredentialGroupInvite = defineAuthorizedWorkspaceUseCase({ resolveCredentialGroupContext(input.credentialGroupId), authorizationOptions: { delegation: credentialGroupDelegationPolicy }, authorizeResource({ principal }) { - requireCredentialGroupWorkflowSubject(principal) + requireCredentialGroupWorkflowActor(principal) }, execute: async ({ principal, input, context }) => { if (context.status !== 'active') { @@ -40,12 +41,11 @@ export const sendCredentialGroupInvite = defineAuthorizedWorkspaceUseCase({ } await requireCredentialGroupsAvailable(context.workspaceId) - const userId = requireCredentialGroupWorkflowSubject(principal) - const inviter = await loadCredentialGroupInviterIdentity(userId) + // Attribution, not authority. An actorless or Slack-triggered run names no + // inviter rather than borrowing its actor, so the email claims no one invited. + const userId = resolvePrincipalSubjectUserId(principal) + const inviter = userId ? await loadCredentialGroupInviterIdentity(userId) : null const inviterName = inviter?.name?.trim() || inviter?.email - if (!inviterName) { - throw new OrchestrationError('conflict', 'Inviting user has no display identity') - } try { const enrollment = await inviteCredentialGroupEnrollment( diff --git a/apps/sim/lib/credential-groups/enrollments.ts b/apps/sim/lib/credential-groups/enrollments.ts index 91e661c7238..3f99519ab2f 100644 --- a/apps/sim/lib/credential-groups/enrollments.ts +++ b/apps/sim/lib/credential-groups/enrollments.ts @@ -72,7 +72,8 @@ interface IssuedInvitation { } export interface PublicCredentialGroupEnrollment { - inviterName: string + /** Null when the invitation was issued by a workflow or a since-deleted user. */ + inviterName: string | null workspaceName: string credentialGroupName: string options: Array< @@ -426,8 +427,10 @@ async function issueInvitation( async function sendInvitation( context: InvitationContext, - userId: string, - inviterName: string, + /** See {@link issueInvitation}: the issuer is attribution, never the authority. */ + userId: string | undefined, + /** Absent when a workflow issued the invitation — the copy drops the inviter. */ + inviterName: string | undefined, email: string, options: SendInvitationOptions ): Promise { @@ -658,8 +661,10 @@ export async function loadCredentialGroupInviterIdentity( export async function inviteCredentialGroupEnrollment( workspaceId: string, groupId: string, - userId: string, - inviterName: string, + /** See {@link issueInvitation}: the issuer is attribution, never the authority. */ + userId: string | undefined, + /** See {@link sendInvitation}: absent for a workflow-issued invitation. */ + inviterName: string | undefined, email: string ): Promise { const context = await getInvitationContext(workspaceId, groupId) @@ -789,7 +794,7 @@ async function buildPublicCredentialGroupEnrollment( ) return { - inviterName: row.inviterName ?? 'A workspace admin', + inviterName: row.inviterName, workspaceName: row.workspaceName, credentialGroupName: row.groupName, options: await Promise.all(