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(