diff --git a/apps/sim/app/api/credentials/route.test.ts b/apps/sim/app/api/credentials/route.test.ts index c673b39b1a1..e9a4a57e8e4 100644 --- a/apps/sim/app/api/credentials/route.test.ts +++ b/apps/sim/app/api/credentials/route.test.ts @@ -3,17 +3,24 @@ * * @vitest-environment node */ -import { auditMock, authMockFns, createMockRequest, posthogServerMock } from '@sim/testing' +import { + auditMock, + authMockFns, + createMockRequest, + dbChainMockFns, + posthogServerMock, + resetDbChainMock, +} from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors' const { mockCheckWorkspaceAccess, - mockGetWorkspaceMembership, + mockGetCredentialCreationWorkspaceContext, mockVerifyAndBuildServiceAccountSecret, } = vi.hoisted(() => ({ mockCheckWorkspaceAccess: vi.fn(), - mockGetWorkspaceMembership: vi.fn(), + mockGetCredentialCreationWorkspaceContext: vi.fn(), mockVerifyAndBuildServiceAccountSecret: vi.fn(), })) @@ -25,7 +32,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({ })) vi.mock('@/lib/credentials/environment', () => ({ - getWorkspaceMembership: mockGetWorkspaceMembership, + getCredentialCreationWorkspaceContext: mockGetCredentialCreationWorkspaceContext, })) vi.mock('@/lib/credentials/oauth', () => ({ @@ -52,6 +59,7 @@ const WORKSPACE_ID = '11111111-2222-4333-8444-555555555555' describe('POST /api/credentials', () => { beforeEach(() => { vi.clearAllMocks() + resetDbChainMock() authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-1', name: 'Test User', email: 'test@example.com' }, }) @@ -60,7 +68,12 @@ describe('POST /api/credentials', () => { canWrite: true, canAdmin: true, }) - mockGetWorkspaceMembership.mockResolvedValue({ ownerId: 'user-1', memberUserIds: ['user-1'] }) + mockGetCredentialCreationWorkspaceContext.mockResolvedValue({ + ownerId: 'user-1', + organizationId: 'org-1', + memberUserIds: ['user-1'], + canWrite: true, + }) }) describe('client-credential service accounts', () => { @@ -156,5 +169,42 @@ describe('POST /api/credentials', () => { expect(data.error).toContain('clientSecret is required') expect(mockVerifyAndBuildServiceAccountSecret).not.toHaveBeenCalled() }) + + it('re-authorizes a personal credential after the shared org/user locks', async () => { + mockGetCredentialCreationWorkspaceContext + .mockResolvedValueOnce({ + ownerId: 'user-1', + organizationId: 'org-1', + memberUserIds: ['user-1'], + canWrite: true, + }) + .mockResolvedValueOnce({ + ownerId: 'org-owner', + organizationId: 'org-1', + memberUserIds: ['org-owner'], + canWrite: false, + }) + + const req = createMockRequest('POST', { + workspaceId: WORKSPACE_ID, + type: 'env_personal', + envKey: 'MY_API_KEY', + }) + + const response = await POST(req) + const data = await response.json() + + expect(response.status).toBe(403) + expect(data).toEqual({ error: 'Write permission required' }) + expect(mockGetCredentialCreationWorkspaceContext).toHaveBeenCalledTimes(2) + expect(dbChainMockFns.execute).toHaveBeenCalled() + expect(mockGetCredentialCreationWorkspaceContext.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.execute.mock.invocationCallOrder[0] + ) + expect(dbChainMockFns.execute.mock.invocationCallOrder.at(-1)).toBeLessThan( + mockGetCredentialCreationWorkspaceContext.mock.invocationCallOrder[1] + ) + expect(dbChainMockFns.insert).not.toHaveBeenCalled() + }) }) }) diff --git a/apps/sim/app/api/credentials/route.ts b/apps/sim/app/api/credentials/route.ts index 2617cb90a2c..99ba00a6151 100644 --- a/apps/sim/app/api/credentials/route.ts +++ b/apps/sim/app/api/credentials/route.ts @@ -13,6 +13,7 @@ import { } from '@/lib/api/contracts/credentials' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' +import { acquireOrganizationUserMutationLocks } from '@/lib/billing/organizations/membership' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { @@ -21,7 +22,7 @@ import { SHARED_CREDENTIAL_TYPES, } from '@/lib/credentials/access' import { AtlassianValidationError } from '@/lib/credentials/atlassian-service-account' -import { getWorkspaceMembership } from '@/lib/credentials/environment' +import { getCredentialCreationWorkspaceContext } from '@/lib/credentials/environment' import { syncWorkspaceOAuthCredentialsForUser } from '@/lib/credentials/oauth' import { ServiceAccountSecretError, @@ -509,10 +510,52 @@ export const POST = withRouteHandler(async (request: NextRequest) => { resolvedProviderId === SLACK_CUSTOM_BOT_PROVIDER_ID && clientCredentialId ? clientCredentialId : generateId() - const { ownerId: workspaceOwnerId, memberUserIds: workspaceMemberUserIds } = - await getWorkspaceMembership(workspaceId) - await db.transaction(async (tx) => { + const creationResult = await db.transaction(async (tx) => { + /** + * Discover the organization lock scope inside this transaction, then + * acquire the same organization → user → membership locks as org + * removal/transfer and re-authorize from the transaction before writing. + * + * If this insert wins, transfer sees the new source-owned personal + * credential and blocks. If transfer wins, its permission/member cleanup + * is visible to the authoritative re-read below and the insert is + * refused. + */ + const plannedContext = await getCredentialCreationWorkspaceContext({ + executor: tx, + workspaceId, + userId: session.user.id, + }) + if (!plannedContext) { + return { success: false as const, status: 403 as const, error: 'Write permission required' } + } + + await acquireOrganizationUserMutationLocks(tx, { + userId: session.user.id, + organizationIds: plannedContext.organizationId ? [plannedContext.organizationId] : [], + }) + + const currentContext = await getCredentialCreationWorkspaceContext({ + executor: tx, + workspaceId, + userId: session.user.id, + forUpdate: true, + }) + if (!currentContext) { + return { success: false as const, status: 403 as const, error: 'Write permission required' } + } + if (currentContext.organizationId !== plannedContext.organizationId) { + return { + success: false as const, + status: 409 as const, + error: 'Workspace organization changed while creating the credential. Please retry.', + } + } + if (!currentContext.canWrite) { + return { success: false as const, status: 403 as const, error: 'Write permission required' } + } + // service_account has no DB-level unique index on (workspaceId, providerId, // displayName), so we re-check inside the tx. OAuth/env_* are guarded by // partial unique indexes and fall through to the 23505 handler below. @@ -542,9 +585,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => { updatedAt: now, }) - if ((type === 'env_workspace' || type === 'service_account') && workspaceOwnerId) { - if (workspaceMemberUserIds.length > 0) { - for (const memberUserId of workspaceMemberUserIds) { + if ((type === 'env_workspace' || type === 'service_account') && currentContext.ownerId) { + if (currentContext.memberUserIds.length > 0) { + for (const memberUserId of currentContext.memberUserIds) { const isAdmin = memberUserId === session.user.id await tx.insert(credentialMember).values({ id: generateId(), @@ -572,7 +615,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { updatedAt: now, }) } + + return { success: true as const } }) + if (!creationResult.success) { + return NextResponse.json({ error: creationResult.error }, { status: creationResult.status }) + } const [created] = await db .select() diff --git a/apps/sim/app/api/invitations/[id]/route.ts b/apps/sim/app/api/invitations/[id]/route.ts index ffe9f950454..a332d09b190 100644 --- a/apps/sim/app/api/invitations/[id]/route.ts +++ b/apps/sim/app/api/invitations/[id]/route.ts @@ -1,10 +1,6 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' -import { db } from '@sim/db' -import { invitation, invitationWorkspaceGrant } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { isOrgAdminRole } from '@sim/platform-authz/workspace' import { normalizeEmail } from '@sim/utils/string' -import { and, eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { cancelInvitationQuerySchema, @@ -17,11 +13,11 @@ import { getSession } from '@/lib/auth' import { isOrganizationOwnerOrAdmin } from '@/lib/billing/core/organization' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { - cancelInvitation, getInvitationById, getInvitationJoinPreview, isInvitationExpired, - revokeInvitationWorkspaceGrant, + revokeInvitationAsAdmin, + updateInvitation, } from '@/lib/invitations/core' import { hasWorkspaceAdminAccess } from '@/lib/workspaces/permissions/utils' @@ -132,87 +128,44 @@ export const PATCH = withRouteHandler( const { role, grants } = parsed.data.body try { - const inv = await getInvitationById(id) - if (!inv) { - return NextResponse.json({ error: 'Invitation not found' }, { status: 404 }) - } - - if (inv.status !== 'pending') { - return NextResponse.json({ error: 'Can only modify pending invitations' }, { status: 400 }) - } - - if (role !== undefined) { - if (inv.membershipIntent === 'external') { - return NextResponse.json( - { error: 'Role updates are not valid on external workspace invitations' }, - { status: 400 } - ) - } - if (!inv.organizationId) { - return NextResponse.json( - { error: 'Role updates are only valid on organization-scoped invitations' }, - { status: 400 } - ) - } - if (!(await isOrganizationOwnerOrAdmin(session.user.id, inv.organizationId))) { - return NextResponse.json( - { error: 'Only an organization owner or admin can change invitation roles' }, - { status: 403 } - ) - } - /** - * A member-role invite without workspace grants would leave the - * invitee workspace-less after accepting (admins derive access to - * every organization workspace; members do not). - */ - if (!isOrgAdminRole(role) && inv.grants.length === 0) { - return NextResponse.json( - { - error: - 'Member invitations must include at least one workspace. Keep the admin role or send a new invitation with workspace access.', - }, - { status: 400 } - ) - } + const result = await updateInvitation({ + actorId: session.user.id, + invitationId: id, + role, + grants, + }) + if (!result.success) { + const errorByKind = { + 'not-found': ['Invitation not found', 404], + 'not-pending': ['Can only modify pending invitations', 400], + 'external-role': ['Role updates are not valid on external workspace invitations', 400], + 'role-not-organization-scoped': [ + 'Role updates are only valid on organization-scoped invitations', + 400, + ], + 'organization-forbidden': [ + 'Only an organization owner or admin can change invitation roles', + 403, + ], + 'member-requires-workspace': [ + 'Member invitations must include at least one workspace. Keep the admin role or send a new invitation with workspace access.', + 400, + ], + 'grant-not-found': [ + `Invitation does not grant access to workspace ${result.workspaceId}`, + 400, + ], + 'workspace-forbidden': [ + 'Workspace admin access required to change grant permissions', + 403, + ], + } as const + const [error, status] = errorByKind[result.kind] + return NextResponse.json({ error }, { status }) } + const inv = result.invitation const grantsToApply = grants ?? [] - for (const update of grantsToApply) { - const belongsToInvite = inv.grants.some((g) => g.workspaceId === update.workspaceId) - if (!belongsToInvite) { - return NextResponse.json( - { error: `Invitation does not grant access to workspace ${update.workspaceId}` }, - { status: 400 } - ) - } - if (!(await hasWorkspaceAdminAccess(session.user.id, update.workspaceId))) { - return NextResponse.json( - { error: 'Workspace admin access required to change grant permissions' }, - { status: 403 } - ) - } - } - - await db.transaction(async (tx) => { - if (role !== undefined && role !== inv.role) { - await tx - .update(invitation) - .set({ role, updatedAt: new Date() }) - .where(eq(invitation.id, id)) - } - for (const update of grantsToApply) { - await tx - .update(invitationWorkspaceGrant) - .set({ permission: update.permission, updatedAt: new Date() }) - .where( - and( - eq(invitationWorkspaceGrant.invitationId, id), - eq(invitationWorkspaceGrant.workspaceId, update.workspaceId) - ) - ) - } - }) - const isOrgScoped = inv.kind === 'organization' const primaryWorkspaceId = inv.grants[0]?.workspaceId ?? null recordAudit({ @@ -270,49 +223,52 @@ export const DELETE = withRouteHandler( } try { - const inv = await getInvitationById(id) - if (!inv) { - return NextResponse.json({ error: 'Invitation not found' }, { status: 404 }) - } - - if (inv.status !== 'pending') { - return NextResponse.json({ error: 'Can only cancel pending invitations' }, { status: 400 }) - } - - const isOrganizationAdmin = inv.organizationId - ? await isOrganizationOwnerOrAdmin(session.user.id, inv.organizationId) - : false - - /** - * Scoped revocation: an admin of this one workspace may withdraw its own - * grant. Authority over the invitation's other workspaces is not implied, - * so only that grant is removed. - */ - if (scopedWorkspaceId) { - if (!inv.grants.some((grant) => grant.workspaceId === scopedWorkspaceId)) { + const result = await revokeInvitationAsAdmin({ + actorId: session.user.id, + invitationId: id, + workspaceId: scopedWorkspaceId, + }) + if (!result.success) { + if (result.kind === 'not-found') { + return NextResponse.json({ error: 'Invitation not found' }, { status: 404 }) + } + if (result.kind === 'not-pending') { + return NextResponse.json( + { error: 'Can only cancel pending invitations' }, + { status: 400 } + ) + } + if (result.kind === 'grant-not-found') { return NextResponse.json( { error: 'Invitation does not grant access to that workspace' }, { status: 400 } ) } - if ( - !isOrganizationAdmin && - !(await hasWorkspaceAdminAccess(session.user.id, scopedWorkspaceId)) - ) { + if (result.kind === 'scoped-forbidden') { return NextResponse.json( { error: 'You need admin permissions on that workspace to revoke its invitation' }, { status: 403 } ) } - - const { revoked, invitationCancelled } = await revokeInvitationWorkspaceGrant({ - invitationId: id, - workspaceId: scopedWorkspaceId, - }) - if (!revoked) { - return NextResponse.json({ error: 'Invitation not cancellable' }, { status: 400 }) + if (result.kind === 'whole-forbidden') { + return NextResponse.json( + { + error: result.spansMultipleWorkspaces + ? 'This invitation spans several workspaces. Revoke it from a workspace you administer, or ask an organization admin.' + : 'Only an organization or workspace admin can cancel this invitation', + }, + { status: 403 } + ) } + return NextResponse.json({ error: 'Invitation not cancellable' }, { status: 400 }) + } + /** + * Scoped revocation: an admin of this one workspace may withdraw its own + * grant. Authority over the invitation's other workspaces is not implied, + * so only that grant is removed. + */ + if (scopedWorkspaceId) { recordAudit({ workspaceId: scopedWorkspaceId, actorId: session.user.id, @@ -321,50 +277,23 @@ export const DELETE = withRouteHandler( action: AuditAction.INVITATION_REVOKED, resourceType: AuditResourceType.WORKSPACE, resourceId: scopedWorkspaceId, - description: `Revoked ${inv.email}'s pending invitation to this workspace`, + description: `Revoked ${result.invitation.email}'s pending invitation to this workspace`, metadata: { invitationId: id, - targetEmail: inv.email, + targetEmail: result.invitation.email, workspaceId: scopedWorkspaceId, - invitationCancelled, + invitationCancelled: result.invitationCancelled, }, request, }) - return NextResponse.json({ success: true, invitationCancelled }) - } - - /** - * Whole-invitation revocation needs authority over everything it grants: - * organization admins have it implicitly, otherwise the actor must - * administer every granted workspace. Admin of just one is not enough — - * that would let them destroy grants to workspaces they cannot see. - */ - let canCancel = isOrganizationAdmin - if (!canCancel && inv.grants.length > 0) { - const adminChecks = await Promise.all( - inv.grants.map((grant) => hasWorkspaceAdminAccess(session.user.id, grant.workspaceId)) - ) - canCancel = adminChecks.every(Boolean) - } - - if (!canCancel) { - return NextResponse.json( - { - error: - inv.grants.length > 1 - ? 'This invitation spans several workspaces. Revoke it from a workspace you administer, or ask an organization admin.' - : 'Only an organization or workspace admin can cancel this invitation', - }, - { status: 403 } - ) - } - - const cancelled = await cancelInvitation(id) - if (!cancelled) { - return NextResponse.json({ error: 'Invitation not cancellable' }, { status: 400 }) + return NextResponse.json({ + success: true, + invitationCancelled: result.invitationCancelled, + }) } + const inv = result.invitation recordAudit({ workspaceId: inv.grants[0]?.workspaceId ?? null, actorId: session.user.id, @@ -387,7 +316,10 @@ export const DELETE = withRouteHandler( request, }) - return NextResponse.json({ success: true, invitationCancelled: true }) + return NextResponse.json({ + success: true, + invitationCancelled: result.invitationCancelled, + }) } catch (error) { logger.error('Failed to cancel invitation', { invitationId: id, error }) return NextResponse.json({ error: 'Failed to cancel invitation' }, { status: 500 }) diff --git a/apps/sim/app/api/v1/admin/dashboard/workspaces/[id]/move/route.ts b/apps/sim/app/api/v1/admin/dashboard/workspaces/[id]/move/route.ts index 8199fb3d78f..2b9f9d284d3 100644 --- a/apps/sim/app/api/v1/admin/dashboard/workspaces/[id]/move/route.ts +++ b/apps/sim/app/api/v1/admin/dashboard/workspaces/[id]/move/route.ts @@ -27,6 +27,7 @@ export const POST = withRouteHandler( const data = await moveWorkspaceToOrganization({ workspaceId: parsed.data.params.id, destinationOrganizationId: parsed.data.body.destinationOrganizationId, + expectedOwnerId: parsed.data.body.expectedOwnerId, adminEmail: request.headers.get('x-admin-email') ?? 'admin-api@sim.ai', }) return NextResponse.json({ data }) diff --git a/apps/sim/app/api/workspaces/invitations/route.test.ts b/apps/sim/app/api/workspaces/invitations/route.test.ts index 2d8abced766..73d45983edd 100644 --- a/apps/sim/app/api/workspaces/invitations/route.test.ts +++ b/apps/sim/app/api/workspaces/invitations/route.test.ts @@ -134,6 +134,8 @@ describe('POST /api/workspaces/invitations/batch', () => { created: true, addedWorkspaceIds: input.grants.map((grant) => grant.workspaceId), grants: input.grants, + mutationUpdatedAt: new Date('2026-07-30T12:00:00.000Z'), + mutationOrganizationId: 'org-1', }) ) mockSendInvitationEmail.mockResolvedValue({ success: true }) @@ -464,6 +466,9 @@ describe('POST /api/workspaces/invitations/batch', () => { failed: [{ email: 'new@example.com', error: 'mailer unavailable' }], }) ) - expect(mockCancelPendingInvitation).toHaveBeenCalledWith('inv-1') + expect(mockCancelPendingInvitation).toHaveBeenCalledWith('inv-1', { + expectedUpdatedAt: new Date('2026-07-30T12:00:00.000Z'), + expectedOrganizationId: 'org-1', + }) }) }) diff --git a/apps/sim/app/api/workspaces/route.ts b/apps/sim/app/api/workspaces/route.ts index f3bb86871a7..5ec3978dd8e 100644 --- a/apps/sim/app/api/workspaces/route.ts +++ b/apps/sim/app/api/workspaces/route.ts @@ -1,6 +1,6 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { db } from '@sim/db' -import { member, permissions, type WorkspaceMode, workflow, workspace } from '@sim/db/schema' +import { permissions, type WorkspaceMode, workflow, workspace } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' import { and, eq, isNull } from 'drizzle-orm' @@ -10,7 +10,6 @@ import { createWorkspaceContract } from '@/lib/api/contracts/workspaces' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { getActiveOrganizationId } from '@/lib/auth/session-response' -import { acquireUserBillingIdentityLock } from '@/lib/billing/organizations/billing-identity-lock' import { PlatformEvents } from '@/lib/core/telemetry' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { captureServerEvent } from '@/lib/posthog/server' @@ -21,23 +20,14 @@ import { listWorkspacesForViewer } from '@/lib/workspaces/list' import { getWorkspaceCreationPolicy, getWorkspaceInvitePolicy, + lockWorkspaceCreationContext, resolveInviteFlags, WORKSPACE_MODE, + WorkspaceCreationContextChangedError, } from '@/lib/workspaces/policy' const logger = createLogger('Workspaces') -/** - * Thrown when the creator became an organization member between the - * creation-policy read and the insert — the workspace must not land personal. - */ -class PersonalWorkspaceCreationRacedError extends Error { - constructor() { - super('User joined an organization while creating a personal workspace') - this.name = 'PersonalWorkspaceCreationRacedError' - } -} - // Get all workspaces for the current user export const GET = withRouteHandler(async (request: Request) => { const session = await getSession() @@ -83,10 +73,13 @@ export const GET = withRouteHandler(async (request: Request) => { * default-workspace insert. Their workspaces (the join sweep's output) * exist now — re-list and return that instead of failing the load. */ - if (error instanceof PersonalWorkspaceCreationRacedError) { - logger.info('Default workspace creation raced an organization join; re-listing', { - userId: session.user.id, - }) + if (error instanceof WorkspaceCreationContextChangedError) { + logger.info( + 'Default workspace creation raced an organization membership change; re-listing', + { + userId: session.user.id, + } + ) const refreshedPayload = await listWorkspacesForViewer({ userId: session.user.id, activeOrganizationId, @@ -190,11 +183,11 @@ export const POST = withRouteHandler(async (req: NextRequest) => { return NextResponse.json({ workspace: newWorkspace }) } catch (error) { - if (error instanceof PersonalWorkspaceCreationRacedError) { + if (error instanceof WorkspaceCreationContextChangedError) { return NextResponse.json( { error: - 'You joined an organization while this workspace was being created. Organization members create organization workspaces — try again.', + 'Your organization membership changed while this workspace was being created. Please try again.', }, { status: 409 } ) @@ -252,35 +245,27 @@ async function createWorkspace({ const workflowId = generateId() const now = new Date() const color = explicitColor || getRandomWorkspaceColor() + let committedBilledAccountUserId = billedAccountUserId try { await db.transaction(async (tx) => { /** - * Personal creation serializes with organization joins on the user's - * billing-identity lock: joins hold it while sweeping the joiner's - * owned workspaces, so re-checking membership under it here means a - * workspace can never be created personal after (or while) its owner - * joins an organization — the creation-policy read above this - * transaction can be stale by the time the insert runs. + * Creation takes the same organization → user → membership fence as + * source access removal and transfer. If creation commits first, their + * post-lock workspace-set re-read sees this row and cleans it up. If the + * membership mutation commits first, this re-read rejects the stale + * creation policy before inserting anything. */ - if (!organizationId) { - await acquireUserBillingIdentityLock(tx, userId) - const [currentMembership] = await tx - .select({ organizationId: member.organizationId }) - .from(member) - .where(eq(member.userId, userId)) - .limit(1) - /** - * Only a CHANGE since the policy read means the decision is stale. An - * unchanged membership is legitimately personal — the policy returns - * a personal decision for members whose organization has no usable - * Team/Enterprise plan (a dormant org), and those users must still be - * able to create workspaces. - */ - if ((currentMembership?.organizationId ?? null) !== observedOrganizationId) { - throw new PersonalWorkspaceCreationRacedError() - } - } + const lockedCreationContext = await lockWorkspaceCreationContext(tx, { + userId, + organizationId, + observedOrganizationId, + }) + const currentBilledAccountUserId = + workspaceMode === WORKSPACE_MODE.ORGANIZATION + ? lockedCreationContext.billedAccountUserId + : billedAccountUserId + committedBilledAccountUserId = currentBilledAccountUserId await tx.insert(workspace).values({ id: workspaceId, @@ -289,7 +274,7 @@ async function createWorkspace({ ownerId: userId, organizationId, workspaceMode, - billedAccountUserId, + billedAccountUserId: currentBilledAccountUserId, allowPersonalApiKeys: true, createdAt: now, updatedAt: now, @@ -309,14 +294,14 @@ async function createWorkspace({ if ( workspaceMode === WORKSPACE_MODE.ORGANIZATION && - billedAccountUserId && - billedAccountUserId !== userId + currentBilledAccountUserId && + currentBilledAccountUserId !== userId ) { permissionRows.push({ id: generateId(), entityType: 'workspace' as const, entityId: workspaceId, - userId: billedAccountUserId, + userId: currentBilledAccountUserId, permissionType: 'admin' as const, createdAt: now, updatedAt: now, @@ -369,7 +354,7 @@ async function createWorkspace({ const invitePolicy = await getWorkspaceInvitePolicy({ organizationId, workspaceMode, - billedAccountUserId, + billedAccountUserId: committedBilledAccountUserId, ownerId: userId, }) @@ -380,13 +365,13 @@ async function createWorkspace({ ownerId: userId, organizationId, workspaceMode, - billedAccountUserId, + billedAccountUserId: committedBilledAccountUserId, allowPersonalApiKeys: true, createdAt: now, updatedAt: now, role: 'owner', permissions: 'admin', - ...resolveInviteFlags(invitePolicy, billedAccountUserId === userId), + ...resolveInviteFlags(invitePolicy, committedBilledAccountUserId === userId), } } diff --git a/apps/sim/background/cleanup-logs.test.ts b/apps/sim/background/cleanup-logs.test.ts index 9ee5306a39b..2b1adb3cdc5 100644 --- a/apps/sim/background/cleanup-logs.test.ts +++ b/apps/sim/background/cleanup-logs.test.ts @@ -49,13 +49,6 @@ vi.mock('@trigger.dev/sdk', () => ({ task: mockTask })) vi.mock('@/lib/cleanup/batch-delete', () => ({ batchDeleteByWorkspaceAndTimestamp: mockBatchDeleteByWorkspaceAndTimestamp, - chunkArray: (items: string[], size: number) => { - const chunks: string[][] = [] - for (let index = 0; index < items.length; index += size) { - chunks.push(items.slice(index, index + size)) - } - return chunks - }, chunkedBatchDelete: mockChunkedBatchDelete, })) diff --git a/apps/sim/background/cleanup-logs.ts b/apps/sim/background/cleanup-logs.ts index 2ee922fb7e0..2d560ef74c0 100644 --- a/apps/sim/background/cleanup-logs.ts +++ b/apps/sim/background/cleanup-logs.ts @@ -9,12 +9,12 @@ import { workspaceFiles, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { task } from '@trigger.dev/sdk' import { and, asc, eq, inArray, isNull, lt, notInArray, or, sql } from 'drizzle-orm' import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' import { batchDeleteByWorkspaceAndTimestamp, - chunkArray, chunkedBatchDelete, type TableCleanupResult, } from '@/lib/cleanup/batch-delete' diff --git a/apps/sim/background/cleanup-soft-deletes.test.ts b/apps/sim/background/cleanup-soft-deletes.test.ts index eab9ff06e63..ef3e8be58aa 100644 --- a/apps/sim/background/cleanup-soft-deletes.test.ts +++ b/apps/sim/background/cleanup-soft-deletes.test.ts @@ -49,13 +49,6 @@ vi.mock('@/lib/cleanup/batch-delete', () => ({ batchDeleteByWorkspaceAndTimestamp: mockBatchDeleteByWorkspaceAndTimestamp, chunkedBatchDelete: mockChunkedBatchDelete, DEFAULT_DELETE_CHUNK_SIZE: 1000, - chunkArray: (items: string[], size: number) => { - const chunks: string[][] = [] - for (let index = 0; index < items.length; index += size) { - chunks.push(items.slice(index, index + size)) - } - return chunks - }, deleteRowsById: mockDeleteRowsById, selectRowsByIdChunks: mockSelectRowsByIdChunks, })) diff --git a/apps/sim/background/cleanup-soft-deletes.ts b/apps/sim/background/cleanup-soft-deletes.ts index f49be176a3e..42bf64f7237 100644 --- a/apps/sim/background/cleanup-soft-deletes.ts +++ b/apps/sim/background/cleanup-soft-deletes.ts @@ -13,6 +13,7 @@ import { workspaceFiles, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { task } from '@trigger.dev/sdk' import { and, asc, eq, inArray, isNotNull, isNull, lt, sql } from 'drizzle-orm' import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' @@ -23,7 +24,6 @@ import { } from '@/lib/billing/storage' import { batchDeleteByWorkspaceAndTimestamp, - chunkArray, chunkedBatchDelete, DEFAULT_DELETE_CHUNK_SIZE, selectRowsByIdChunks, diff --git a/apps/sim/background/cleanup-tasks.ts b/apps/sim/background/cleanup-tasks.ts index 1745e3bba14..1d35d143498 100644 --- a/apps/sim/background/cleanup-tasks.ts +++ b/apps/sim/background/cleanup-tasks.ts @@ -7,12 +7,12 @@ import { mothershipInboxTask, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { task } from '@trigger.dev/sdk' import { and, inArray, lt } from 'drizzle-orm' import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' import { batchDeleteByWorkspaceAndTimestamp, - chunkArray, DEFAULT_DELETE_CHUNK_SIZE, deleteRowsById, selectRowsByIdChunks, diff --git a/apps/sim/lib/admin/dashboard-credit-grant.test.ts b/apps/sim/lib/admin/dashboard-credit-grant.test.ts index d4a189418c8..b8c26bb0348 100644 --- a/apps/sim/lib/admin/dashboard-credit-grant.test.ts +++ b/apps/sim/lib/admin/dashboard-credit-grant.test.ts @@ -57,6 +57,9 @@ vi.mock('@/lib/billing/organizations/seats', () => ({ vi.mock('@/lib/billing/core/usage', () => ({ syncUsageLimitsFromSubscription: mocks.syncUsageLimits, })) +vi.mock('@/lib/workspaces/organization-workspaces', () => ({ + ownedAttachableWorkspacesWhere: vi.fn(() => undefined), +})) vi.mock('@/lib/workspaces/admin-move', () => ({ moveWorkspaceToOrganization: mocks.moveWorkspace, })) @@ -211,6 +214,9 @@ describe('addDashboardOrganizationMember', () => { resetDbChainMock() mocks.billingSubscriptions = [] mocks.idempotencyCalls = [] + mocks.ensureMembership.mockReset() + mocks.transferMembership.mockReset() + mocks.moveWorkspace.mockReset() }) it('rejects an existing member inside the transaction before touching their cap', async () => { @@ -239,7 +245,50 @@ describe('addDashboardOrganizationMember', () => { expect(mocks.recordAudit).not.toHaveBeenCalled() }) - it('uses the canonical transfer service and reports each selected personal workspace move', async () => { + it('moves every selected workspace through the invitation-aware service after adding a member', async () => { + queueTableRows(workspace, [{ id: 'workspace-1' }, { id: 'workspace-2' }]) + queueTableRows(subscription, [{ plan: 'enterprise' }]) + mocks.ensureMembership.mockResolvedValue({ + success: true, + memberId: 'member-new', + alreadyMember: false, + billingActions: { proUsageSnapshotted: false, proCancelledAtPeriodEnd: false }, + }) + mocks.moveWorkspace.mockResolvedValue({}) + + const result = await addDashboardOrganizationMember( + 'org-1', + { + userId: 'user-1', + role: 'member', + personalWorkspaceIds: ['workspace-1', 'workspace-2'], + }, + { id: 'admin-1', name: 'Admin', email: 'admin@sim.ai' } + ) + + expect(mocks.moveWorkspace).toHaveBeenNthCalledWith(1, { + workspaceId: 'workspace-1', + destinationOrganizationId: 'org-1', + adminEmail: 'admin@sim.ai', + expectedOwnerId: 'user-1', + }) + expect(mocks.moveWorkspace).toHaveBeenNthCalledWith(2, { + workspaceId: 'workspace-2', + destinationOrganizationId: 'org-1', + adminEmail: 'admin@sim.ai', + expectedOwnerId: 'user-1', + }) + expect(result).toEqual({ + memberId: 'member-new', + transferredFromOrganizationId: null, + workspaceMoves: [ + { workspaceId: 'workspace-1', success: true }, + { workspaceId: 'workspace-2', success: true }, + ], + }) + }) + + it('uses the canonical transfer service and reports each selected workspace move', async () => { queueTableRows(workspace, [{ id: 'workspace-1' }, { id: 'workspace-2' }]) queueTableRows(member, [{ id: 'member-old', organizationId: 'org-old' }]) mocks.transferMembership.mockResolvedValue({ @@ -289,4 +338,38 @@ describe('addDashboardOrganizationMember', () => { expect(mocks.reconcileSeats).toHaveBeenCalledTimes(2) expect(mocks.recordAudit).toHaveBeenCalledTimes(2) }) + + it('uses the expected owner guard for an administrator-selected subset', async () => { + // The attachability query is scoped to the selected ids, so it returns only + // `workspace-1` even though the user owns more. + queueTableRows(workspace, [{ id: 'workspace-1' }]) + queueTableRows(member, [{ id: 'member-old', organizationId: 'org-old' }]) + mocks.transferMembership.mockResolvedValue({ + success: true, + memberId: 'member-new', + workspaceAccessRevoked: 0, + credentialMembershipsRevoked: 0, + pendingInvitationsCancelled: 0, + usageCaptured: 0, + }) + mocks.moveWorkspace.mockResolvedValue({}) + + const result = await addDashboardOrganizationMember( + 'org-new', + { + userId: 'user-1', + role: 'member', + personalWorkspaceIds: ['workspace-1'], + }, + { id: 'admin-1', name: 'Admin', email: 'admin@sim.ai' } + ) + + expect(mocks.moveWorkspace).toHaveBeenCalledWith({ + workspaceId: 'workspace-1', + destinationOrganizationId: 'org-new', + adminEmail: 'admin@sim.ai', + expectedOwnerId: 'user-1', + }) + expect(result.workspaceMoves).toEqual([{ workspaceId: 'workspace-1', success: true }]) + }) }) diff --git a/apps/sim/lib/admin/dashboard.ts b/apps/sim/lib/admin/dashboard.ts index bbb983d567e..a2af65f11bf 100644 --- a/apps/sim/lib/admin/dashboard.ts +++ b/apps/sim/lib/admin/dashboard.ts @@ -12,19 +12,7 @@ import { } from '@sim/db/schema' import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { - and, - count, - countDistinct, - desc, - eq, - ilike, - inArray, - isNull, - ne, - or, - sql, -} from 'drizzle-orm' +import { and, count, countDistinct, desc, eq, ilike, inArray, isNull, or, sql } from 'drizzle-orm' import { getOrganizationUsageLimitFallbackDollars, getTeamOrganizationEconomics, @@ -64,6 +52,7 @@ import { executeTransactionallyIdempotent } from '@/lib/core/idempotency/transac import { enqueueOutboxEvent } from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' import { moveWorkspaceToOrganization } from '@/lib/workspaces/admin-move' +import { ownedAttachableWorkspacesWhere } from '@/lib/workspaces/organization-workspaces' interface PaginationInput { search: string @@ -317,7 +306,7 @@ async function getDashboardOrganizationSummary(organizationId: string) { member, and(eq(member.userId, permissions.userId), eq(member.organizationId, organizationId)) ) - .where(and(isNull(member.id), isNull(workspace.archivedAt))), + .where(isNull(member.id)), getLatestSubscription(organizationId), getLatestEnterpriseProvisionings([organizationId]), ]) @@ -398,13 +387,7 @@ export async function listDashboardOrganizations({ search, limit, offset }: Pagi eq(member.organizationId, workspace.organizationId) ) ) - .where( - and( - inArray(workspace.organizationId, organizationIds), - isNull(member.id), - isNull(workspace.archivedAt) - ) - ) + .where(and(inArray(workspace.organizationId, organizationIds), isNull(member.id))) .groupBy(workspace.organizationId), db .selectDistinctOn([subscription.referenceId]) @@ -496,7 +479,7 @@ export async function getDashboardOrganization(organizationId: string) { member, and(eq(member.userId, permissions.userId), eq(member.organizationId, organizationId)) ) - .where(and(isNull(member.id), isNull(workspace.archivedAt))) + .where(isNull(member.id)) .groupBy(user.id, user.name, user.email) .orderBy(user.name), db @@ -939,7 +922,7 @@ export async function getDashboardMemberTransferPreflight( db .select({ id: workspace.id, name: workspace.name, archivedAt: workspace.archivedAt }) .from(workspace) - .where(and(eq(workspace.ownerId, userId), ne(workspace.workspaceMode, 'organization'))) + .where(ownedAttachableWorkspacesWhere({ userId, includeArchived: true })) .orderBy(workspace.name, workspace.id), ]) if (!destination) throw new Error('Destination organization not found') @@ -991,9 +974,8 @@ export async function addDashboardOrganizationMember( .from(workspace) .where( and( - inArray(workspace.id, selectedWorkspaceIds), - eq(workspace.ownerId, values.userId), - ne(workspace.workspaceMode, 'organization') + ownedAttachableWorkspacesWhere({ userId: values.userId, includeArchived: true }), + inArray(workspace.id, selectedWorkspaceIds) ) ) if (selectable.length !== selectedWorkspaceIds.length) { @@ -1095,6 +1077,7 @@ export async function addDashboardOrganizationMember( workspaceId, destinationOrganizationId: organizationId, adminEmail: actor.email ?? 'admin-api', + expectedOwnerId: values.userId, }) workspaceMoves.push({ workspaceId, success: true }) } catch (error) { diff --git a/apps/sim/lib/admin/external-collaborators.test.ts b/apps/sim/lib/admin/external-collaborators.test.ts index faab5dc0658..072a2366de4 100644 --- a/apps/sim/lib/admin/external-collaborators.test.ts +++ b/apps/sim/lib/admin/external-collaborators.test.ts @@ -2,7 +2,12 @@ * @vitest-environment node */ import { member, permissions } from '@sim/db/schema' -import { queueTableRows, resetDbChainMock } from '@sim/testing' +import { + dbChainMockFns, + flattenMockConditions, + queueTableRows, + resetDbChainMock, +} from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ @@ -57,6 +62,10 @@ describe('updateDashboardExternalCollaboratorUsageLimit', () => { metadata: { targetUserId: 'external-1', usageLimitDollars: 30 }, }) ) + const collaboratorPredicate = dbChainMockFns.where.mock.calls[1]?.[0] + expect( + flattenMockConditions(collaboratorPredicate).some((condition) => condition.type === 'isNull') + ).toBe(false) }) it('clears an existing cap', async () => { @@ -84,7 +93,7 @@ describe('updateDashboardExternalCollaboratorUsageLimit', () => { expect(mocks.recordAudit).not.toHaveBeenCalled() }) - it('rejects users without a current non-archived workspace permission', async () => { + it('rejects users without any organization workspace permission', async () => { queueTableRows(member, []) queueTableRows(permissions, []) diff --git a/apps/sim/lib/admin/external-collaborators.ts b/apps/sim/lib/admin/external-collaborators.ts index 768b4306709..0144b1d64d7 100644 --- a/apps/sim/lib/admin/external-collaborators.ts +++ b/apps/sim/lib/admin/external-collaborators.ts @@ -1,7 +1,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { db } from '@sim/db' import { member, permissions, workspace } from '@sim/db/schema' -import { and, eq, isNull } from 'drizzle-orm' +import { and, eq } from 'drizzle-orm' import { setOrgMemberUsageLimit } from '@/lib/billing/organizations/member-limits' import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/membership' @@ -36,13 +36,7 @@ export async function updateDashboardExternalCollaboratorUsageLimit( workspace, and(eq(permissions.entityType, 'workspace'), eq(permissions.entityId, workspace.id)) ) - .where( - and( - eq(permissions.userId, userId), - eq(workspace.organizationId, organizationId), - isNull(workspace.archivedAt) - ) - ) + .where(and(eq(permissions.userId, userId), eq(workspace.organizationId, organizationId))) .limit(1) if (!externalPermission) { throw new Error('User is not a current external collaborator for this organization') diff --git a/apps/sim/lib/api/contracts/v1/admin/dashboard-workspaces.ts b/apps/sim/lib/api/contracts/v1/admin/dashboard-workspaces.ts index 4818e11688a..552daafc1e4 100644 --- a/apps/sim/lib/api/contracts/v1/admin/dashboard-workspaces.ts +++ b/apps/sim/lib/api/contracts/v1/admin/dashboard-workspaces.ts @@ -24,6 +24,7 @@ export const adminDashboardWorkspacePreflightQuerySchema = z.object({ export const adminDashboardWorkspaceMoveBodySchema = z.object({ destinationOrganizationId: z.string().min(1).max(200), + expectedOwnerId: z.string().min(1).max(200).optional(), }) const adminDashboardWorkspaceCandidateSchema = z.object({ @@ -78,9 +79,7 @@ const adminDashboardWorkspacePreflightResponseSchema = z.object({ }) const adminDashboardWorkspaceMoveResponseSchema = z.object({ - data: adminDashboardWorkspacePreflightSchema.extend({ - invitationEmailFailures: z.array(z.string()), - }), + data: adminDashboardWorkspacePreflightSchema, }) export const adminDashboardWorkspaceSearchContract = defineRouteContract({ diff --git a/apps/sim/lib/billing/cleanup-dispatcher.test.ts b/apps/sim/lib/billing/cleanup-dispatcher.test.ts index e1d3f8977ac..27160138359 100644 --- a/apps/sim/lib/billing/cleanup-dispatcher.test.ts +++ b/apps/sim/lib/billing/cleanup-dispatcher.test.ts @@ -23,9 +23,6 @@ vi.mock('@/lib/billing/core/billing', () => ({ vi.mock('@/lib/billing/core/subscription', () => ({ getHighestPriorityPersonalSubscription: vi.fn(), })) -vi.mock('@/lib/cleanup/batch-delete', () => ({ - chunkArray: vi.fn((items: unknown[]) => (items.length > 0 ? [items] : [])), -})) vi.mock('@/lib/core/async-jobs', () => ({ getJobQueue: vi.fn(() => ({ enqueue: mockEnqueue })), })) diff --git a/apps/sim/lib/billing/cleanup-dispatcher.ts b/apps/sim/lib/billing/cleanup-dispatcher.ts index 8b8608d63a4..b53f7ae7faf 100644 --- a/apps/sim/lib/billing/cleanup-dispatcher.ts +++ b/apps/sim/lib/billing/cleanup-dispatcher.ts @@ -2,13 +2,13 @@ import { db } from '@sim/db' import type { DataRetentionSettings, WorkspaceMode } from '@sim/db/schema' import { organization, workspace } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { tasks } from '@trigger.dev/sdk' import { and, asc, eq, gt, isNull } from 'drizzle-orm' import { getOrganizationSubscription } from '@/lib/billing/core/billing' import { getHighestPriorityPersonalSubscription } from '@/lib/billing/core/subscription' import { getPlanType, type PlanCategory } from '@/lib/billing/plan-helpers' import { type RetentionHoursKey, resolveEffectiveRetentionHours } from '@/lib/billing/retention' -import { chunkArray } from '@/lib/cleanup/batch-delete' import { getJobQueue } from '@/lib/core/async-jobs' import { shouldExecuteInline } from '@/lib/core/async-jobs/config' import { resolveTriggerRegion } from '@/lib/core/async-jobs/region' diff --git a/apps/sim/lib/billing/core/billing.ts b/apps/sim/lib/billing/core/billing.ts index fbdc924881d..8372293a0f9 100644 --- a/apps/sim/lib/billing/core/billing.ts +++ b/apps/sim/lib/billing/core/billing.ts @@ -36,6 +36,8 @@ interface GetOrganizationSubscriptionOptions { onError?: 'return-null' | 'throw' /** Primary/replica client or a caller-owned enforcement transaction. */ executor?: DbClient | DbOrTx + /** Row-lock the selected entitlement inside a caller-owned transaction. */ + forUpdate?: boolean } /** @@ -47,17 +49,17 @@ interface GetOrganizationSubscriptionOptions { * (from `core/subscription.ts`), which excludes `past_due`. * Returns `null` when there is no entitled sub. * - * `options.executor` exists for replica routing on display/summary read - * paths only. Enforcement and webhook callers must read the primary — - * omit the executor (or pass `db`). + * Enforcement and webhook callers must read the primary. They may pass a + * caller-owned primary transaction when the subscription must be revalidated + * and row-locked with another mutation. */ export async function getOrganizationSubscription( organizationId: string, options: GetOrganizationSubscriptionOptions = {} ) { - const { onError = 'return-null', executor = db } = options + const { onError = 'return-null', executor = db, forUpdate = false } = options try { - const orgSubs = await executor + const query = executor .select() .from(subscription) .where( @@ -68,6 +70,7 @@ export async function getOrganizationSubscription( ) .orderBy(desc(subscription.periodStart), desc(subscription.id)) .limit(1) + const orgSubs = forUpdate ? await query.for('update') : await query return orgSubs.length > 0 ? orgSubs[0] : null } catch (error) { diff --git a/apps/sim/lib/billing/organizations/lock-order.test.ts b/apps/sim/lib/billing/organizations/lock-order.test.ts index 453828e0c0d..fb1d96893ae 100644 --- a/apps/sim/lib/billing/organizations/lock-order.test.ts +++ b/apps/sim/lib/billing/organizations/lock-order.test.ts @@ -34,7 +34,7 @@ vi.mock('@/lib/billing/storage/payer-transfer', () => ({ })) import { - reapplyPaidOrgJoinBillingForExistingMember, + reapplyPaidOrgJoinBillingForExistingMemberTx, restoreUserProSubscription, transferOrganizationOwnership, withInvitationSafeOrganizationAccessMutation, @@ -113,9 +113,8 @@ describe('paid-org join billing lock ordering', () => { it('locks the personal subscription before mutating userStats', async () => { const { tx, ops } = createRecordingTx() - dbChainMockFns.transaction.mockImplementation(async (cb: (t: unknown) => unknown) => cb(tx)) - await reapplyPaidOrgJoinBillingForExistingMember('user-1', 'org-1') + await reapplyPaidOrgJoinBillingForExistingMemberTx(tx as DbOrTx, 'user-1', 'org-1') const firstUserStatsUpdate = ops.findIndex((o) => o.op === 'update' && o.table === userStats) const subscriptionLock = ops.findIndex((o) => o.op === 'lock' && o.table === subscriptionTable) @@ -127,9 +126,8 @@ describe('paid-org join billing lock ordering', () => { it('still locks an already-paused personal Pro so a concurrent restore cannot pass it', async () => { const { tx, ops } = createRecordingTx({ ...GENERIC_ROW, cancelAtPeriodEnd: true }) - dbChainMockFns.transaction.mockImplementation(async (cb: (t: unknown) => unknown) => cb(tx)) - await reapplyPaidOrgJoinBillingForExistingMember('user-1', 'org-1') + await reapplyPaidOrgJoinBillingForExistingMemberTx(tx as DbOrTx, 'user-1', 'org-1') expect(ops.some((op) => op.op === 'lock' && op.table === subscriptionTable)).toBe(true) }) diff --git a/apps/sim/lib/billing/organizations/membership-external-removal.test.ts b/apps/sim/lib/billing/organizations/membership-external-removal.test.ts new file mode 100644 index 00000000000..164b6979087 --- /dev/null +++ b/apps/sim/lib/billing/organizations/membership-external-removal.test.ts @@ -0,0 +1,38 @@ +/** + * @vitest-environment node + */ +import { dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockSetOrgMemberUsageLimit } = vi.hoisted(() => ({ + mockSetOrgMemberUsageLimit: vi.fn(), +})) + +vi.mock('@/lib/billing/organizations/member-limits', () => ({ + setOrgMemberUsageLimit: mockSetOrgMemberUsageLimit, +})) + +import { removeExternalUserFromOrganizationWorkspaces } from '@/lib/billing/organizations/membership' + +describe('external organization access removal', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('clears a stale per-user organization usage cap in the removal transaction', async () => { + await removeExternalUserFromOrganizationWorkspaces({ + organizationId: 'org-1', + userId: 'external-user', + }) + + expect(dbChainMockFns.transaction).toHaveBeenCalledTimes(1) + expect(mockSetOrgMemberUsageLimit).toHaveBeenCalledWith( + 'org-1', + 'external-user', + null, + undefined, + expect.anything() + ) + }) +}) diff --git a/apps/sim/lib/billing/organizations/membership.ts b/apps/sim/lib/billing/organizations/membership.ts index ab144dce704..1430c5d6732 100644 --- a/apps/sim/lib/billing/organizations/membership.ts +++ b/apps/sim/lib/billing/organizations/membership.ts @@ -100,6 +100,31 @@ export async function acquireOrgMembershipLock( ) } +/** + * Acquires the canonical organization → user-billing-identity → membership + * lock sequence for a mutation whose validity depends on a user's standing in + * one or more organizations. + * + * Keeping this order in one helper lets organization access removal and + * credential creation share the same serialization fence. If credential + * creation wins, a later transfer sees the new source-owned credential and + * blocks. If transfer wins, credential creation re-reads access after the + * transfer and refuses the insert. + */ +export async function acquireOrganizationUserMutationLocks( + tx: DbOrTx, + params: { userId: string; organizationIds: string[] } +): Promise { + const organizationIds = [...new Set(params.organizationIds)].sort() + for (const organizationId of organizationIds) { + await acquireOrganizationMutationLock(tx, organizationId) + } + await acquireUserBillingIdentityLock(tx, params.userId) + for (const organizationId of organizationIds) { + await acquireOrgMembershipLock(tx, params.userId, organizationId) + } +} + export type BillingBlockReason = 'payment_failed' | 'dispute' /** @@ -857,34 +882,6 @@ async function applyPaidOrgJoinBillingTx( return actions } -/** - * Re-applies paid-org join billing for a user who is already a member of - * the organization. Used on re-upgrade after a dormant transition: members - * kept their org membership but had their personal Pro subscriptions - * restored (`cancelAtPeriodEnd=false`) during the cancel/downgrade. When - * the org becomes paid again, those Pros must be re-paused so the user - * isn't double-billed. - * - * No-op when the org has no active Team/Enterprise subscription. - */ -export async function reapplyPaidOrgJoinBillingForExistingMember( - userId: string, - organizationId: string -): Promise { - return db.transaction(async (tx) => { - await acquireOrganizationMutationLock(tx, organizationId) - const [existingMembership] = await tx - .select({ id: member.id }) - .from(member) - .where(and(eq(member.userId, userId), eq(member.organizationId, organizationId))) - .limit(1) - if (!existingMembership) { - return { proUsageSnapshotted: false, proCancelledAtPeriodEnd: false } - } - return reapplyPaidOrgJoinBillingForExistingMemberTx(tx, userId, organizationId) - }) -} - /** * Transaction-enlisted variant used by subscription webhooks. Keeping the * subscription upsert, effective-limit update, provisioning completion, and @@ -989,16 +986,11 @@ export async function withInvitationSafeOrganizationAccessMutation( invitationIds: candidate.invitationIds, workspaceIds: candidate.workspaceIds, }) - const organizationIds = [ - ...new Set([params.organizationId, ...(params.additionalOrganizationIds ?? [])]), - ].sort() - for (const organizationId of organizationIds) { - await acquireOrganizationMutationLock(tx, organizationId) - } - await acquireUserBillingIdentityLock(tx, params.userId) - for (const organizationId of organizationIds) { - await acquireOrgMembershipLock(tx, params.userId, organizationId) - } + const organizationIds = [params.organizationId, ...(params.additionalOrganizationIds ?? [])] + await acquireOrganizationUserMutationLocks(tx, { + userId: params.userId, + organizationIds, + }) const current = await getInvitationRemovalLockSnapshot(tx, params) const candidateInvitations = new Set(candidate.invitationIds) @@ -1636,6 +1628,8 @@ export async function removeExternalUserFromOrganizationWorkspaces(params: { .limit(1) if (currentMember) throw new Error('User is an organization member') + await setOrgMemberUsageLimit(organizationId, userId, null, undefined, tx) + const cancelledInvitations = invitationIds.length ? await tx .update(invitation) @@ -2055,23 +2049,6 @@ export async function isSoleOwnerOfPaidOrganization(userId: string): Promise<{ } } -export async function isUserMemberOfOrganization( - userId: string, - organizationId: string -): Promise<{ isMember: boolean; role?: string; memberId?: string }> { - const [memberRecord] = await db - .select({ id: member.id, role: member.role }) - .from(member) - .where(and(eq(member.userId, userId), eq(member.organizationId, organizationId))) - .limit(1) - - if (memberRecord) { - return { isMember: true, role: memberRecord.role, memberId: memberRecord.id } - } - - return { isMember: false } -} - /** * Get user's current organization membership (if any). */ diff --git a/apps/sim/lib/billing/organizations/provision-seat.test.ts b/apps/sim/lib/billing/organizations/provision-seat.test.ts index a5881054627..55f2c1a9e90 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.test.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.test.ts @@ -181,14 +181,14 @@ describe('ensureTeamOrganizationForAcceptance', () => { new Error('Enterprise issuance is unfinished') ) - const result = await ensureTeamOrganizationForAcceptance({ - billingOwnerUserId: 'owner-1', - workspaceOrganizationId: 'org-1', - executor: testExecutor(), - workspaceIdsToAttach: [], - }) - - expect(result).toEqual({ success: false, failureCode: 'server-error' }) + await expect( + ensureTeamOrganizationForAcceptance({ + billingOwnerUserId: 'owner-1', + workspaceOrganizationId: 'org-1', + executor: testExecutor(), + workspaceIdsToAttach: [], + }) + ).rejects.toThrow('Enterprise issuance is unfinished') expect(mockAcquireOrganizationMutationLock).toHaveBeenCalledWith(expect.anything(), 'org-1') expect(mockGetOrganizationSubscription).not.toHaveBeenCalled() expect(updateCalls.value).toHaveLength(0) @@ -285,14 +285,14 @@ describe('ensureTeamOrganizationForAcceptance', () => { new Error('Enterprise issuance is unfinished') ) - const result = await ensureTeamOrganizationForAcceptance({ - billingOwnerUserId: 'owner-1', - workspaceOrganizationId: null, - executor: testExecutor(), - workspaceIdsToAttach: ['workspace-1'], - }) - - expect(result).toEqual({ success: false, failureCode: 'server-error' }) + await expect( + ensureTeamOrganizationForAcceptance({ + billingOwnerUserId: 'owner-1', + workspaceOrganizationId: null, + executor: testExecutor(), + workspaceIdsToAttach: ['workspace-1'], + }) + ).rejects.toThrow('Enterprise issuance is unfinished') expect(updateCalls.value).toHaveLength(0) expect(enqueueMock).not.toHaveBeenCalled() }) diff --git a/apps/sim/lib/billing/organizations/provision-seat.ts b/apps/sim/lib/billing/organizations/provision-seat.ts index 83eb655ec48..5d045a5fa5b 100644 --- a/apps/sim/lib/billing/organizations/provision-seat.ts +++ b/apps/sim/lib/billing/organizations/provision-seat.ts @@ -98,7 +98,15 @@ export async function ensureTeamOrganizationForAcceptance( workspaceOrganizationId, error, }) - return { success: false, failureCode: 'server-error' } + /** + * This helper runs inside the invitation acceptance transaction and may + * already have created an organization, re-homed a subscription, attached + * workspaces, or queued billing reconciliation. Returning a failure value + * would let that transaction commit those partial writes. Propagate every + * unexpected failure so the transaction rolls back; the acceptance + * boundary converts it to the public `server-error` result afterwards. + */ + throw error } } diff --git a/apps/sim/lib/billing/validation/seat-management.test.ts b/apps/sim/lib/billing/validation/seat-management.test.ts index a38880a6609..00e4c2756ad 100644 --- a/apps/sim/lib/billing/validation/seat-management.test.ts +++ b/apps/sim/lib/billing/validation/seat-management.test.ts @@ -38,6 +38,7 @@ vi.mock('@/lib/messaging/email/validation', () => ({ })) import { + countPendingSeatInvitations, getOrganizationSeatInfo, syncSeatsFromStripeQuantity, validateSeatAvailability, @@ -118,6 +119,32 @@ describe('validateSeatAvailability', () => { }) }) +describe('countPendingSeatInvitations', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('excludes invitees who already belong to any organization by normalized email', async () => { + queueSelectResponses([[{ count: 1 }]]) + + await expect(countPendingSeatInvitations('org-1')).resolves.toBe(1) + + const predicate = dbChainMockFns.where.mock.calls[0]?.[0] as { + conditions?: Array<{ toSQL?: () => { sql: string; params: unknown[] } }> + } + const existingMemberGuard = predicate.conditions?.find( + (condition) => typeof condition?.toSQL === 'function' + ) + const rendered = existingMemberGuard?.toSQL?.() + expect(rendered?.sql.toLowerCase()).toContain('not exists') + expect(rendered?.sql.toLowerCase()).toContain('btrim') + // The member exclusion is deliberately cross-org, so its own SQL fragment + // must not carry the destination organization as a parameter. + expect(rendered?.params?.filter((param) => param === 'org-1')).toHaveLength(0) + }) +}) + describe('syncSeatsFromStripeQuantity', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/lib/billing/validation/seat-management.ts b/apps/sim/lib/billing/validation/seat-management.ts index 9bc0c17713d..d0e9777b6c8 100644 --- a/apps/sim/lib/billing/validation/seat-management.ts +++ b/apps/sim/lib/billing/validation/seat-management.ts @@ -1,7 +1,7 @@ import { db } from '@sim/db' -import { invitation, member, organization, subscription } from '@sim/db/schema' +import { invitation, member, organization, subscription, user } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { and, count, eq, gt, ne } from 'drizzle-orm' +import { and, count, eq, gt, ne, sql } from 'drizzle-orm' import { getOrganizationSubscription } from '@/lib/billing/core/billing' import { resolveEnterpriseMetadataIntent } from '@/lib/billing/enterprise-outbox' import { isEnterprise, isFree } from '@/lib/billing/plan-helpers' @@ -38,9 +38,12 @@ interface ValidateSeatOptions { /** * Counts the pending invitations that stand to become seats. * - * The single definition of that predicate: still `pending`, not yet expired, and - * internal — external collaborators never take a seat. Every seat number in the - * product derives from this one count so no two surfaces can drift. + * The single definition of that predicate: still `pending`, not yet expired, + * internal, and addressed to somebody who is not already a member of any + * organization. Existing organization members cannot consume a destination + * seat: workspace invitations remain external and organization invitations are + * rejected. Every seat number in the product derives from this one count so no + * two surfaces can drift. */ export async function countPendingSeatInvitations( organizationId: string, @@ -52,6 +55,15 @@ export async function countPendingSeatInvitations( eq(invitation.status, 'pending'), ne(invitation.membershipIntent, 'external'), gt(invitation.expiresAt, new Date()), + // Membership is intentionally not scoped to the invitation's destination. + // A user who belongs to any organization cannot consume this seat under the + // acceptance semantics, so reserving one would overstate Enterprise usage. + sql`NOT EXISTS ( + SELECT 1 + FROM ${member} + INNER JOIN ${user} ON ${user.id} = ${member.userId} + WHERE LOWER(BTRIM(${user.email})) = LOWER(BTRIM(${invitation.email})) + )`, ] if (excludePendingInvitationId) { filters.push(ne(invitation.id, excludePendingInvitationId)) @@ -67,7 +79,7 @@ export async function countPendingSeatInvitations( * Resolves the organization's seat capacity, honouring an Enterprise seat * change that is still in flight to Stripe. */ -async function resolveSeatCapacity( +export async function resolveSeatCapacity( organizationSubscription: { id: string; plan: string; metadata?: unknown } & Record< string, unknown diff --git a/apps/sim/lib/billing/webhooks/enterprise.test.ts b/apps/sim/lib/billing/webhooks/enterprise.test.ts new file mode 100644 index 00000000000..982683f0776 --- /dev/null +++ b/apps/sim/lib/billing/webhooks/enterprise.test.ts @@ -0,0 +1,233 @@ +/** + * @vitest-environment node + */ +import { + createMockStripeEvent, + dbChainMockFns, + queueTableRows, + resetDbChainMock, + schemaMock, +} from '@sim/testing' +import type Stripe from 'stripe' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + subscriptionsRetrieve: vi.fn(), + patchOutboxEventPayload: vi.fn(), + reapplyPaidOrgJoinBillingForExistingMemberTx: vi.fn(), +})) + +vi.mock('@sim/audit', () => ({ + AuditAction: { ENTERPRISE_SUBSCRIPTION_PROVISIONED: 'subscription.enterprise_provisioned' }, + AuditResourceType: { SUBSCRIPTION: 'subscription' }, + recordAudit: vi.fn(), +})) + +vi.mock('@sim/utils/id', () => ({ generateId: vi.fn(() => 'generated-id') })) + +vi.mock('@/components/emails', () => ({ + getEmailSubject: vi.fn(() => 'Enterprise subscription'), + renderEnterpriseSubscriptionEmail: vi.fn(), +})) + +vi.mock('@/lib/billing/organizations/membership', () => ({ + acquireOrganizationMutationLock: vi.fn(), + reapplyPaidOrgJoinBillingForExistingMemberTx: mocks.reapplyPaidOrgJoinBillingForExistingMemberTx, +})) + +vi.mock('@/lib/billing/stripe-client', () => ({ + requireStripeClient: () => ({ + subscriptions: { retrieve: mocks.subscriptionsRetrieve }, + }), +})) + +vi.mock('@/lib/billing/webhooks/enterprise-reconciliation-lease', () => ({ + assertEnterpriseReconciliationLeaseHeld: vi.fn(), + withEnterpriseReconciliationLease: vi.fn( + async ( + _subscriptionId: string, + operation: (lease: { key: string; token: string }) => Promise + ) => operation({ key: 'test-lease', token: 'test-token' }) + ), +})) + +vi.mock('@/lib/billing/webhooks/idempotency', () => ({ + stripeWebhookIdempotency: { + executeWithIdempotency: vi.fn( + async (_provider: string, _identifier: string, operation: () => Promise) => + operation() + ), + }, +})) + +vi.mock('@/lib/core/outbox/service', () => ({ + patchOutboxEventPayload: mocks.patchOutboxEventPayload, +})) + +vi.mock('@/lib/messaging/email/mailer', () => ({ + sendEmail: vi.fn(), +})) + +vi.mock('@/lib/messaging/email/utils', () => ({ + getFromEmailAddress: vi.fn(() => 'billing@sim.test'), +})) + +vi.mock('@/lib/posthog/server', () => ({ + captureServerEvent: vi.fn(), +})) + +import { handleManualEnterpriseSubscription } from '@/lib/billing/webhooks/enterprise' + +const ENTERPRISE_PROVISION_EVENT_TYPE = 'stripe.provision-enterprise' + +function operationPayload(options: { applied?: boolean; pausePaymentCollection?: boolean } = {}) { + return { + version: 1 as const, + request: { + requestKey: 'enterprise-v3:owner-1:org-1:12500:24000:12:1250', + ownerUserId: 'owner-1', + organizationId: 'org-1', + requestedByEmail: 'admin@sim.ai', + requestedByUserId: 'admin-1', + invoiceAmountCents: 12500, + usageLimitCredits: 24000, + seats: 12, + concurrencyLimit: 1250, + pausePaymentCollection: options.pausePaymentCollection ?? false, + }, + retryRevision: 0, + stripeProgress: { subscriptionId: 'sub_1' }, + ...(options.applied + ? { + applicationResult: { + appliedAt: '2026-07-30T12:00:00.000Z', + subscriptionId: 'sub_1', + }, + } + : {}), + } +} + +function stripeSubscription(options: { + operationId?: string + paused?: boolean +}): Stripe.Subscription { + return { + id: 'sub_1', + customer: 'cus_1', + status: 'active', + collection_method: 'send_invoice', + days_until_due: 30, + pause_collection: options.paused ? { behavior: 'keep_as_draft', resumes_at: null } : null, + cancel_at_period_end: false, + cancel_at: null, + canceled_at: null, + ended_at: null, + trial_start: null, + trial_end: null, + metadata: { + plan: 'enterprise', + referenceId: 'org-1', + organizationId: 'org-1', + invoiceAmountCents: '12500', + monthlyPrice: '125.00', + usageLimitCredits: '24000', + seats: '12', + concurrencyLimit: '1250', + ...(options.operationId ? { enterpriseOperationId: options.operationId } : {}), + }, + items: { + data: [ + { + quantity: 1, + current_period_start: 1_785_283_200, + current_period_end: 1_787_961_600, + price: { + currency: 'usd', + unit_amount: 12500, + recurring: { interval: 'month', interval_count: 1 }, + }, + }, + ], + }, + } as unknown as Stripe.Subscription +} + +function eventFor(subscription: Stripe.Subscription): Stripe.Event { + return createMockStripeEvent('customer.subscription.created', subscription) +} + +function queueSuccessfulExistingSubscriptionReconciliation(options: { + operation?: ReturnType +}) { + queueTableRows(schemaMock.organization, [{ creditBalance: '0' }]) + if (options.operation) { + queueTableRows(schemaMock.outboxEvent, [ + { eventType: ENTERPRISE_PROVISION_EVENT_TYPE, payload: options.operation }, + ]) + queueTableRows(schemaMock.user, [{ stripeCustomerId: 'cus_1' }]) + } + queueTableRows(schemaMock.member, [{ value: 1 }]) + queueTableRows(schemaMock.member, []) + queueTableRows(schemaMock.subscription, []) + queueTableRows(schemaMock.subscription, [{ id: 'local-sub-1', referenceId: 'org-1' }]) + queueTableRows(schemaMock.user, [{ id: 'owner-1', name: 'Owner', email: 'owner@example.com' }]) +} + +describe('Enterprise webhook issuance correlation', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mocks.patchOutboxEventPayload.mockResolvedValue(true) + mocks.reapplyPaidOrgJoinBillingForExistingMemberTx.mockResolvedValue(undefined) + }) + + afterAll(() => { + resetDbChainMock() + }) + + it('retries when the create webhook races ahead of paused-collection provisioning', async () => { + const subscription = stripeSubscription({ operationId: 'operation-1', paused: false }) + mocks.subscriptionsRetrieve.mockResolvedValue(subscription) + queueTableRows(schemaMock.organization, [{ creditBalance: '0' }]) + queueTableRows(schemaMock.outboxEvent, [ + { + eventType: ENTERPRISE_PROVISION_EVENT_TYPE, + payload: operationPayload({ pausePaymentCollection: true }), + }, + ]) + queueTableRows(schemaMock.user, [{ stripeCustomerId: 'cus_1' }]) + + await expect(handleManualEnterpriseSubscription(eventFor(subscription))).rejects.toThrow( + 'Enterprise issuance operation operation-1 does not yet match the Stripe subscription' + ) + + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(mocks.patchOutboxEventPayload).not.toHaveBeenCalled() + expect(mocks.reapplyPaidOrgJoinBillingForExistingMemberTx).not.toHaveBeenCalled() + }) + + it('allows later Stripe metadata edits after the issuance was already applied', async () => { + const subscription = stripeSubscription({ operationId: 'operation-1', paused: false }) + mocks.subscriptionsRetrieve.mockResolvedValue(subscription) + queueSuccessfulExistingSubscriptionReconciliation({ + operation: operationPayload({ applied: true, pausePaymentCollection: true }), + }) + + await expect( + handleManualEnterpriseSubscription(eventFor(subscription)) + ).resolves.toBeUndefined() + + expect(mocks.patchOutboxEventPayload).not.toHaveBeenCalled() + }) + + it('continues to reconcile manual Enterprise subscriptions without an operation id', async () => { + const subscription = stripeSubscription({}) + mocks.subscriptionsRetrieve.mockResolvedValue(subscription) + queueSuccessfulExistingSubscriptionReconciliation({}) + + await expect( + handleManualEnterpriseSubscription(eventFor(subscription)) + ).resolves.toBeUndefined() + }) +}) diff --git a/apps/sim/lib/billing/webhooks/enterprise.ts b/apps/sim/lib/billing/webhooks/enterprise.ts index 1c3689b944e..59c81542064 100644 --- a/apps/sim/lib/billing/webhooks/enterprise.ts +++ b/apps/sim/lib/billing/webhooks/enterprise.ts @@ -202,6 +202,32 @@ async function reconcileManualEnterpriseSubscription( if (validCorrelation && operationPayload) { correlatedOperation = operationPayload operationNewlyApplied = !operationPayload.applicationResult + } else if ( + operationRow?.eventType === ENTERPRISE_PROVISION_EVENT_TYPE && + (!operationPayload || !operationPayload.applicationResult) + ) { + /** + * An admin issuance is only complete once Stripe reflects the exact + * commercial terms recorded in its durable outbox intent. In + * particular, paused collection is applied immediately after the + * subscription create call, so the create webhook can race ahead of + * that second Stripe write. Treating the interim object as an + * unrelated manual subscription would grant the wrong entitlement and + * strand the issuance in `awaiting_webhook`. + * + * Throwing makes Stripe retry. The next delivery performs another + * authoritative Stripe read and can apply once the worker has finished + * the external operation. Already-applied operations are intentionally + * excluded so later manual metadata edits still reconcile normally. + */ + logger.warn('[subscription] Enterprise operation is not ready for reconciliation', { + operationId, + subscriptionId: stripeSubscription.id, + referenceId, + }) + throw new Error( + `Enterprise issuance operation ${operationId} does not yet match the Stripe subscription` + ) } else { logger.warn('[subscription] Ignoring invalid Enterprise operation correlation', { operationId, diff --git a/apps/sim/lib/cleanup/batch-delete.ts b/apps/sim/lib/cleanup/batch-delete.ts index d71286d9d6e..84b12a039ec 100644 --- a/apps/sim/lib/cleanup/batch-delete.ts +++ b/apps/sim/lib/cleanup/batch-delete.ts @@ -1,5 +1,6 @@ import { db } from '@sim/db' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { and, inArray, isNotNull, lt, type SQL, sql } from 'drizzle-orm' import type { PgColumn, PgTable } from 'drizzle-orm/pg-core' @@ -25,12 +26,6 @@ export const DEFAULT_WORKSPACE_CHUNK_SIZE = 50 /** Bounds FK cascade trigger queue (per-statement in-memory) and bind-parameter count. */ export const DEFAULT_DELETE_CHUNK_SIZE = 1000 -export function chunkArray(arr: T[], size: number): T[][] { - const out: T[][] = [] - for (let i = 0; i < arr.length; i += size) out.push(arr.slice(i, i + size)) - return out -} - export interface SelectByIdChunksOptions { /** Cap on rows returned across all chunks. Defaults to a full per-table cleanup budget. */ overallLimit?: number diff --git a/apps/sim/lib/cleanup/chat-cleanup.ts b/apps/sim/lib/cleanup/chat-cleanup.ts index ba84f1f0279..01ca0777e2d 100644 --- a/apps/sim/lib/cleanup/chat-cleanup.ts +++ b/apps/sim/lib/cleanup/chat-cleanup.ts @@ -1,8 +1,8 @@ import { dbFor } from '@sim/db' import { copilotChats, copilotMessages, workspaceFiles } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { and, inArray, isNull } from 'drizzle-orm' -import { chunkArray } from '@/lib/cleanup/batch-delete' import { SIM_AGENT_API_URL } from '@/lib/copilot/constants' import { env } from '@/lib/core/config/env' import type { StorageContext } from '@/lib/uploads' diff --git a/apps/sim/lib/core/outbox/service.test.ts b/apps/sim/lib/core/outbox/service.test.ts index 7502ed5b8e8..cb2a21d01b2 100644 --- a/apps/sim/lib/core/outbox/service.test.ts +++ b/apps/sim/lib/core/outbox/service.test.ts @@ -24,7 +24,11 @@ vi.mock('@sim/utils/id', () => ({ generateId: vi.fn(() => 'test-event-id'), })) -import { enqueueOutboxEvent, processOutboxEvents } from './service' +import { + enqueueOrReschedulePendingOutboxEvent, + enqueueOutboxEvent, + processOutboxEvents, +} from './service' function makePendingRow(overrides: Partial = {}): OutboxRow { return { @@ -90,6 +94,90 @@ describe('enqueueOutboxEvent', () => { }) }) +describe('enqueueOrReschedulePendingOutboxEvent', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('inserts normally when the subject has no pending event', async () => { + const availableAt = new Date('2026-07-30T12:01:00.000Z') + + const id = await enqueueOrReschedulePendingOutboxEvent( + dbChainMock.db, + 'invitation.send-migrated-link', + { invitationId: 'invite-1' }, + { + availableAt, + coalesceOn: { payloadKey: 'invitationId', payloadValue: 'invite-1' }, + } + ) + + expect(id).toBe('test-event-id') + expect(dbChainMockFns.values).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'test-event-id', + eventType: 'invitation.send-migrated-link', + payload: { invitationId: 'invite-1' }, + availableAt, + }) + ) + }) + + it('extends one pending event instead of inserting a duplicate for the same subject', async () => { + const existingAvailableAt = new Date('2026-07-30T12:00:00.000Z') + const nextAvailableAt = new Date('2026-07-30T12:01:00.000Z') + queueTableRows(outboxEvent, [ + makePendingRow({ + id: 'evt-existing', + payload: { invitationId: 'invite-1' }, + availableAt: existingAvailableAt, + }), + ]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'evt-existing' }]) + + const id = await enqueueOrReschedulePendingOutboxEvent( + dbChainMock.db, + 'invitation.send-migrated-link', + { invitationId: 'invite-1' }, + { + availableAt: nextAvailableAt, + coalesceOn: { payloadKey: 'invitationId', payloadValue: 'invite-1' }, + } + ) + + expect(id).toBe('evt-existing') + expect(dbChainMockFns.insert).not.toHaveBeenCalled() + expect(dbChainMockFns.set).toHaveBeenCalledWith({ availableAt: nextAvailableAt }) + expect(dbChainMockFns.for).toHaveBeenCalledWith('update') + }) + + it('keeps a later existing delivery deadline when another mutation settles sooner', async () => { + const existingAvailableAt = new Date('2026-07-30T12:02:00.000Z') + const requestedAvailableAt = new Date('2026-07-30T12:01:00.000Z') + queueTableRows(outboxEvent, [ + makePendingRow({ + id: 'evt-existing', + payload: { invitationId: 'invite-1' }, + availableAt: existingAvailableAt, + }), + ]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'evt-existing' }]) + + await enqueueOrReschedulePendingOutboxEvent( + dbChainMock.db, + 'invitation.send-migrated-link', + { invitationId: 'invite-1' }, + { + availableAt: requestedAvailableAt, + coalesceOn: { payloadKey: 'invitationId', payloadValue: 'invite-1' }, + } + ) + + expect(dbChainMockFns.set).toHaveBeenCalledWith({ availableAt: existingAvailableAt }) + }) +}) + describe('processOutboxEvents — empty / no handler', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index d519a4b4c78..d18cff7945b 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -4,7 +4,7 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { truncate } from '@sim/utils/string' -import { and, asc, eq, inArray, lte, sql } from 'drizzle-orm' +import { and, asc, desc, eq, inArray, lte, sql } from 'drizzle-orm' const logger = createLogger('OutboxService') @@ -143,6 +143,81 @@ export async function enqueueOutboxEvent( return id } +export interface CoalescedOutboxEnqueueOptions extends EnqueueOptions { + /** + * One scalar payload field that identifies the subject of this event. + * + * Callers must already serialize writers for this subject with their domain + * lock. The helper row-locks an existing pending event against the worker, + * then extends its delivery deadline instead of creating another row. + */ + coalesceOn: { + payloadKey: string + payloadValue: string + } +} + +/** + * Enqueues an outbox event or extends the settle window of the pending event + * for the same subject. + * + * This is for coalescing a burst of transactional mutations into one eventual + * side effect (for example, several workspace moves that all update the same + * surviving invitation). It intentionally coalesces only `pending` rows: + * processing/completed work already crossed the external-side-effect boundary + * and must not be rewritten. + * + * Concurrency between domain writers is supplied by the caller's existing + * subject lock. `FOR UPDATE` covers the outbox-worker race so a due row cannot + * be claimed while its `availableAt` is being extended. + */ +export async function enqueueOrReschedulePendingOutboxEvent( + executor: Pick, + eventType: string, + payload: T, + options: CoalescedOutboxEnqueueOptions +): Promise { + const [existing] = await executor + .select({ id: outboxEvent.id, availableAt: outboxEvent.availableAt }) + .from(outboxEvent) + .where( + and( + eq(outboxEvent.eventType, eventType), + eq(outboxEvent.status, 'pending'), + sql`${outboxEvent.payload} ->> ${options.coalesceOn.payloadKey} = ${options.coalesceOn.payloadValue}` + ) + ) + .orderBy(desc(outboxEvent.createdAt), desc(outboxEvent.id)) + .for('update') + .limit(1) + + if (!existing) { + return enqueueOutboxEvent(executor, eventType, payload, options) + } + + const requestedAvailableAt = options.availableAt ?? new Date() + const availableAt = + existing.availableAt > requestedAvailableAt ? existing.availableAt : requestedAvailableAt + const [rescheduled] = await executor + .update(outboxEvent) + .set({ availableAt }) + .where(and(eq(outboxEvent.id, existing.id), eq(outboxEvent.status, 'pending'))) + .returning({ id: outboxEvent.id }) + + if (rescheduled) { + logger.info('Rescheduled pending outbox event', { + id: rescheduled.id, + eventType, + availableAt, + }) + return rescheduled.id + } + + // A worker won the status transition before this row lock was acquired. + // Preserve the requested side effect with a fresh event rather than losing it. + return enqueueOutboxEvent(executor, eventType, payload, options) +} + /** * Atomically shallow-merge fields into an outbox payload. This is intended for * the transactional consumer of an external callback (for example a Stripe diff --git a/apps/sim/lib/credentials/environment.test.ts b/apps/sim/lib/credentials/environment.test.ts index 93a5eae03ac..64c89d058d8 100644 --- a/apps/sim/lib/credentials/environment.test.ts +++ b/apps/sim/lib/credentials/environment.test.ts @@ -1,9 +1,23 @@ /** * @vitest-environment node */ -import { dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { credential, permissions, workspace } from '@sim/db/schema' +import { dbChainMock, dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { getWorkspaceEnvKeyAdminAccess } from '@/lib/credentials/environment' +import type { DbOrTx } from '@/lib/db/types' + +const { mockAcquireUserBillingIdentityLock } = vi.hoisted(() => ({ + mockAcquireUserBillingIdentityLock: vi.fn(), +})) + +vi.mock('@/lib/billing/organizations/billing-identity-lock', () => ({ + acquireUserBillingIdentityLock: mockAcquireUserBillingIdentityLock, +})) + +import { + getWorkspaceEnvKeyAdminAccess, + syncPersonalEnvCredentialsForUser, +} from '@/lib/credentials/environment' describe('getWorkspaceEnvKeyAdminAccess', () => { beforeEach(() => { @@ -59,3 +73,86 @@ describe('getWorkspaceEnvKeyAdminAccess', () => { expect(dbChainMockFns.where).toHaveBeenCalledTimes(1) }) }) + +describe('syncPersonalEnvCredentialsForUser', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockAcquireUserBillingIdentityLock.mockResolvedValue(undefined) + }) + + it('uses one transaction and acquires the transfer fence before discovering workspaces', async () => { + const base = dbChainMock.db + const tx = { + select: vi.fn(base.select), + insert: vi.fn(base.insert), + delete: vi.fn(base.delete), + } as unknown as DbOrTx + dbChainMockFns.transaction.mockImplementationOnce(async (callback) => callback(tx)) + queueTableRows(permissions, [{ workspaceId: 'ws-1' }]) + queueTableRows(workspace, []) + queueTableRows(credential, [{ id: 'credential-1' }]) + + await syncPersonalEnvCredentialsForUser({ + userId: 'user-1', + envKeys: ['API_KEY'], + }) + + expect(mockAcquireUserBillingIdentityLock).toHaveBeenCalledWith(tx, 'user-1') + expect(mockAcquireUserBillingIdentityLock.mock.invocationCallOrder[0]).toBeLessThan( + (tx.select as ReturnType).mock.invocationCallOrder[0] + ) + expect(tx.select).toHaveBeenCalledTimes(3) + expect(tx.insert).toHaveBeenCalledTimes(2) + expect(tx.delete).toHaveBeenCalledTimes(1) + }) + + it('does not recreate source credentials when transfer won the user lock', async () => { + const base = dbChainMock.db + const tx = { + select: vi.fn(base.select), + insert: vi.fn(base.insert), + delete: vi.fn(base.delete), + } as unknown as DbOrTx + dbChainMockFns.transaction.mockImplementationOnce(async (callback) => callback(tx)) + queueTableRows(permissions, []) + queueTableRows(workspace, []) + + await syncPersonalEnvCredentialsForUser({ + userId: 'user-1', + envKeys: ['API_KEY'], + }) + + expect(mockAcquireUserBillingIdentityLock.mock.invocationCallOrder[0]).toBeLessThan( + (tx.select as ReturnType).mock.invocationCallOrder[0] + ) + expect(tx.insert).not.toHaveBeenCalled() + expect(tx.delete).not.toHaveBeenCalled() + }) + + it('syncs every workspace with one credential insert, lookup, membership insert, and cleanup', async () => { + const base = dbChainMock.db + const tx = { + select: vi.fn(base.select), + insert: vi.fn(base.insert), + delete: vi.fn(base.delete), + } as unknown as DbOrTx + dbChainMockFns.transaction.mockImplementationOnce(async (callback) => callback(tx)) + queueTableRows(permissions, [{ workspaceId: 'ws-2' }, { workspaceId: 'ws-1' }]) + queueTableRows(workspace, []) + queueTableRows(credential, [{ id: 'credential-1' }, { id: 'credential-2' }]) + + await syncPersonalEnvCredentialsForUser({ + userId: 'user-1', + envKeys: ['API_KEY'], + }) + + expect(tx.select).toHaveBeenCalledTimes(3) + expect(tx.insert).toHaveBeenCalledTimes(2) + expect(tx.delete).toHaveBeenCalledTimes(1) + expect(dbChainMockFns.values).toHaveBeenNthCalledWith(1, [ + expect.objectContaining({ workspaceId: 'ws-1', envKey: 'API_KEY' }), + expect.objectContaining({ workspaceId: 'ws-2', envKey: 'API_KEY' }), + ]) + }) +}) diff --git a/apps/sim/lib/credentials/environment.ts b/apps/sim/lib/credentials/environment.ts index 9f2332b7cb1..da4f743cbab 100644 --- a/apps/sim/lib/credentials/environment.ts +++ b/apps/sim/lib/credentials/environment.ts @@ -1,8 +1,17 @@ import { db } from '@sim/db' import { credential, credentialMember, permissions, workspace } from '@sim/db/schema' +import { permissionSatisfies } from '@sim/platform-authz/workspace' +import { chunkArray } from '@sim/utils/helpers' import { generateId } from '@sim/utils/id' import { and, eq, inArray, isNotNull, isNull, notInArray, or, sql } from 'drizzle-orm' -import { hasWorkspaceAdminAccess } from '@/lib/workspaces/permissions/utils' +import { acquireUserBillingIdentityLock } from '@/lib/billing/organizations/billing-identity-lock' +import type { DbOrTx } from '@/lib/db/types' +import { + getEffectiveWorkspacePermission, + hasWorkspaceAdminAccess, +} from '@/lib/workspaces/permissions/utils' + +const PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE = 500 export interface WorkspaceMembership { ownerId: string | null @@ -15,7 +24,7 @@ export interface WorkspaceMembership { * Credential-admin status is derived from workspace role at access time, so * members are seeded only for use access (the owner plus permission holders). */ -export async function getWorkspaceMembership(workspaceId: string): Promise { +async function getWorkspaceMembership(workspaceId: string): Promise { const [workspaceRows, permissionRows] = await Promise.all([ db .select({ ownerId: workspace.ownerId }) @@ -37,6 +46,60 @@ export async function getWorkspaceMembership(workspaceId: string): Promise { + const workspaceQuery = params.executor + .select({ + ownerId: workspace.ownerId, + organizationId: workspace.organizationId, + }) + .from(workspace) + .where(and(eq(workspace.id, params.workspaceId), isNull(workspace.archivedAt))) + const [workspaceRow] = params.forUpdate + ? await workspaceQuery.for('update').limit(1) + : await workspaceQuery.limit(1) + if (!workspaceRow) return null + + const permissionRows = await params.executor + .select({ userId: permissions.userId }) + .from(permissions) + .where( + and(eq(permissions.entityType, 'workspace'), eq(permissions.entityId, params.workspaceId)) + ) + + const effectivePermission = await getEffectiveWorkspacePermission( + params.userId, + { id: params.workspaceId, organizationId: workspaceRow.organizationId }, + params.executor + ) + + const memberUserIds = new Set(permissionRows.map((row) => row.userId)) + memberUserIds.add(workspaceRow.ownerId) + + return { + ownerId: workspaceRow.ownerId, + organizationId: workspaceRow.organizationId, + memberUserIds: [...memberUserIds], + canWrite: permissionSatisfies(effectivePermission, 'write'), + } +} + export interface WorkspaceEnvKeyAdminAccess { /** Keys for which the caller is an active credential admin. */ adminKeys: Set @@ -95,21 +158,22 @@ interface AccessibleEnvCredential { updatedAt: Date } -export async function getUserWorkspaceIds(userId: string): Promise { - const [permissionRows, ownedWorkspaceRows] = await Promise.all([ - db - .select({ workspaceId: workspace.id }) - .from(permissions) - .innerJoin( - workspace, - and(eq(permissions.entityType, 'workspace'), eq(permissions.entityId, workspace.id)) - ) - .where(and(eq(permissions.userId, userId), isNull(workspace.archivedAt))), - db - .select({ workspaceId: workspace.id }) - .from(workspace) - .where(and(eq(workspace.ownerId, userId), isNull(workspace.archivedAt))), - ]) +export async function getUserWorkspaceIds( + userId: string, + executor: DbOrTx = db +): Promise { + const permissionRows = await executor + .select({ workspaceId: workspace.id }) + .from(permissions) + .innerJoin( + workspace, + and(eq(permissions.entityType, 'workspace'), eq(permissions.entityId, workspace.id)) + ) + .where(and(eq(permissions.userId, userId), isNull(workspace.archivedAt))) + const ownedWorkspaceRows = await executor + .select({ workspaceId: workspace.id }) + .from(workspace) + .where(and(eq(workspace.ownerId, userId), isNull(workspace.archivedAt))) const workspaceIds = new Set(permissionRows.map((row) => row.workspaceId)) for (const row of ownedWorkspaceRows) { @@ -337,94 +401,100 @@ export async function syncPersonalEnvCredentialsForUser(params: { envKeys: string[] }): Promise { const { userId, envKeys } = params - const workspaceIds = await getUserWorkspaceIds(userId) - if (!workspaceIds.length) return - const normalizedKeys = Array.from(new Set(envKeys.filter(Boolean))) const now = new Date() - await Promise.all( - workspaceIds.map(async (workspaceId) => { - if (normalizedKeys.length > 0) { - await db - .insert(credential) - .values( - normalizedKeys.map((envKey) => ({ - id: generateId(), - workspaceId, - type: 'env_personal' as const, - displayName: envKey, - envKey, - envOwnerUserId: userId, - createdBy: userId, - createdAt: now, - updatedAt: now, - })) - ) - .onConflictDoNothing() + await db.transaction(async (tx) => { + /** + * Cross-organization transfer takes this same user-identity fence before + * checking source-owned credentials. If this sync wins, transfer observes + * the new env_personal rows and blocks; if transfer wins, this post-lock + * workspace re-read cannot recreate credentials in the departed org. + */ + await acquireUserBillingIdentityLock(tx, userId) + const workspaceIds = (await getUserWorkspaceIds(userId, tx)).sort() + + if (workspaceIds.length === 0) return + + if (normalizedKeys.length > 0) { + const credentialValues = workspaceIds.flatMap((workspaceId) => + normalizedKeys.map((envKey) => ({ + id: generateId(), + workspaceId, + type: 'env_personal' as const, + displayName: envKey, + envKey, + envOwnerUserId: userId, + createdBy: userId, + createdAt: now, + updatedAt: now, + })) + ) + for (const values of chunkArray(credentialValues, PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) { + await tx.insert(credential).values(values).onConflictDoNothing() } - const currentCredentials = - normalizedKeys.length > 0 - ? await db - .select({ id: credential.id }) - .from(credential) - .where( - and( - eq(credential.workspaceId, workspaceId), - eq(credential.type, 'env_personal'), - eq(credential.envOwnerUserId, userId), - inArray(credential.envKey, normalizedKeys) - ) - ) - : [] + const currentCredentials = await tx + .select({ id: credential.id }) + .from(credential) + .where( + and( + inArray(credential.workspaceId, workspaceIds), + eq(credential.type, 'env_personal'), + eq(credential.envOwnerUserId, userId), + inArray(credential.envKey, normalizedKeys) + ) + ) if (currentCredentials.length > 0) { - await db - .insert(credentialMember) - .values( - currentCredentials.map(({ id: credentialId }) => ({ - id: generateId(), - credentialId, - userId, - role: 'admin' as const, - status: 'active' as const, - joinedAt: now, - invitedBy: userId, - createdAt: now, - updatedAt: now, - })) - ) - .onConflictDoUpdate({ - target: [credentialMember.credentialId, credentialMember.userId], - set: { role: 'admin', status: 'active', updatedAt: now }, - }) + const membershipValues = currentCredentials.map(({ id: credentialId }) => ({ + id: generateId(), + credentialId, + userId, + role: 'admin' as const, + status: 'active' as const, + joinedAt: now, + invitedBy: userId, + createdAt: now, + updatedAt: now, + })) + for (const values of chunkArray( + membershipValues, + PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE + )) { + await tx + .insert(credentialMember) + .values(values) + .onConflictDoUpdate({ + target: [credentialMember.credentialId, credentialMember.userId], + set: { role: 'admin', status: 'active', updatedAt: now }, + }) + } } - if (normalizedKeys.length > 0) { - await db - .delete(credential) - .where( - and( - eq(credential.workspaceId, workspaceId), - eq(credential.type, 'env_personal'), - eq(credential.envOwnerUserId, userId), - notInArray(credential.envKey, normalizedKeys) - ) + await tx + .delete(credential) + .where( + and( + inArray(credential.workspaceId, workspaceIds), + eq(credential.type, 'env_personal'), + eq(credential.envOwnerUserId, userId), + notInArray(credential.envKey, normalizedKeys) ) - } else { - await db - .delete(credential) - .where( - and( - eq(credential.workspaceId, workspaceId), - eq(credential.type, 'env_personal'), - eq(credential.envOwnerUserId, userId) - ) - ) - } - }) - ) + ) + return + } + + await tx + .delete(credential) + .where( + and( + inArray(credential.workspaceId, workspaceIds), + eq(credential.type, 'env_personal'), + eq(credential.envOwnerUserId, userId) + ) + ) + }) } export async function getAccessibleEnvCredentials( diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.test.ts b/apps/sim/lib/execution/payloads/large-value-metadata.test.ts index 2675a1f5bd2..4b1e4e7a45e 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.test.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.test.ts @@ -2,16 +2,18 @@ * @vitest-environment node */ +import { db } from '@sim/db' import { executionLargeValueDependencies, executionLargeValueReferences } from '@sim/db/schema' import { dbChainMockFns, resetDbChainMock } from '@sim/testing' import { eq, notInArray } from 'drizzle-orm' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' import { addLargeValueReference, + collectLargeValueReferenceKeys, MAX_LARGE_VALUE_REFERENCES_PER_SCOPE, pruneLargeValueMetadata, registerLargeValueOwner, - replaceLargeValueReferences, + replaceLargeValueReferenceKeysWithClient, } from '@/lib/execution/payloads/large-value-metadata' function largeValueKey(id: string, executionId = 'source-execution'): string { @@ -205,42 +207,44 @@ describe('large value metadata', () => { const otherWorkspaceKey = 'execution/workspace-2/workflow-1/source-execution/large-value-lv_abcdefghijkl.json' - await replaceLargeValueReferences( + const value = { + a: { + __simLargeValueRef: true, + version: 1, + id: 'lv_abcdefghijkl', + kind: 'json', + size: 123, + key: matchingKey, + }, + duplicate: { + __simLargeValueRef: true, + version: 1, + id: 'lv_abcdefghijkl', + kind: 'json', + size: 123, + key: matchingKey, + }, + ignored: { + __simLargeValueRef: true, + version: 1, + id: 'lv_abcdefghijkl', + kind: 'json', + size: 123, + key: otherWorkspaceKey, + }, + } + + await replaceLargeValueReferenceKeysWithClient( + db, { workspaceId: 'workspace-1', workflowId: 'workflow-1', executionId: 'execution-2', source: 'execution_log', }, - { - a: { - __simLargeValueRef: true, - version: 1, - id: 'lv_abcdefghijkl', - kind: 'json', - size: 123, - key: matchingKey, - }, - duplicate: { - __simLargeValueRef: true, - version: 1, - id: 'lv_abcdefghijkl', - kind: 'json', - size: 123, - key: matchingKey, - }, - ignored: { - __simLargeValueRef: true, - version: 1, - id: 'lv_abcdefghijkl', - kind: 'json', - size: 123, - key: otherWorkspaceKey, - }, - } + collectLargeValueReferenceKeys(value, 'workspace-1') ) - expect(dbChainMockFns.transaction).toHaveBeenCalledOnce() expect(dbChainMockFns.delete).toHaveBeenCalledOnce() expect(eq).toHaveBeenCalledWith(executionLargeValueReferences.source, 'execution_log') expect(dbChainMockFns.values).toHaveBeenCalledWith([ diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.ts b/apps/sim/lib/execution/payloads/large-value-metadata.ts index cc0fc07f6f6..dcbc03f4ff6 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.ts @@ -7,8 +7,8 @@ import { workflowExecutionLogs, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { and, eq, inArray, notInArray, sql } from 'drizzle-orm' -import { chunkArray } from '@/lib/cleanup/batch-delete' import { collectLargeValueKeys } from '@/lib/execution/payloads/large-execution-value' const logger = createLogger('LargeValueMetadata') @@ -227,22 +227,6 @@ export async function registerLargeValueOwner( return true } -export async function replaceLargeValueReferencesWithClient( - client: LargeValueMetadataClient, - scope: LargeValueReferenceScope, - value: unknown -): Promise { - if (!scope.workspaceId || !scope.executionId) { - return - } - - await replaceLargeValueReferenceKeysWithClient( - client, - scope, - collectLargeValueReferenceKeys(value, scope.workspaceId) - ) -} - export async function replaceLargeValueReferenceKeysWithClient( client: LargeValueMetadataClient, scope: LargeValueReferenceScope, @@ -359,18 +343,6 @@ export async function addLargeValueReference( .onConflictDoNothing() } -export async function replaceLargeValueReferences( - scope: LargeValueReferenceScope, - value: unknown -): Promise { - const referenceKeys = scope.workspaceId - ? collectLargeValueReferenceKeys(value, scope.workspaceId) - : [] - await dbFor('exec').transaction(async (tx) => { - await replaceLargeValueReferenceKeysWithClient(tx, scope, referenceKeys) - }) -} - export async function markLargeValuesDeleted( keys: string[], dbClient: LargeValueMetadataClient = db diff --git a/apps/sim/lib/invitations/core.test.ts b/apps/sim/lib/invitations/core.test.ts index f824fa652ae..b90d2669a85 100644 --- a/apps/sim/lib/invitations/core.test.ts +++ b/apps/sim/lib/invitations/core.test.ts @@ -88,7 +88,12 @@ vi.mock('@/lib/workspaces/policy', () => ({ vi.mock('@sim/audit', () => auditMock) -import { acceptInvitation } from '@/lib/invitations/core' +import { + acceptInvitation, + rejectInvitation, + revokeInvitationAsAdmin, + updateInvitation, +} from '@/lib/invitations/core' function queueWhereResponses(responses: unknown[][]) { const queue = [...responses] @@ -99,11 +104,16 @@ function queueWhereResponses(responses: unknown[][]) { orderBy: ReturnType returning: ReturnType groupBy: ReturnType + for: ReturnType } thenable.limit = vi.fn(() => Promise.resolve(result)) thenable.orderBy = vi.fn(() => Promise.resolve(result)) thenable.returning = vi.fn(() => Promise.resolve(result)) thenable.groupBy = vi.fn(() => Promise.resolve(result)) + thenable.for = vi.fn((lockMode: string) => { + dbChainMockFns.for(lockMode) + return thenable + }) return thenable as ReturnType }) } @@ -702,6 +712,8 @@ describe('acceptInvitation', () => { [], // Candidate personal workspaces covered by the acceptance lock set. [], + // No billing-owner workspace escaped the conversion's locked sweep. + [], // Post-join owned-set re-check under the billing-identity lock. [], // Grant-txn membership re-check under the lock: member still present. @@ -756,6 +768,137 @@ describe('acceptInvitation', () => { ) }) + it('rolls back when a billing-owner workspace is created after the acceptance lock plan', async () => { + mockGetWorkspaceWithOwner.mockResolvedValue({ + id: 'workspace-1', + name: 'Workspace', + ownerId: 'owner-1', + organizationId: null, + workspaceMode: 'personal', + billedAccountUserId: 'owner-1', + }) + mockEnsureTeamOrganizationForAcceptance.mockResolvedValueOnce({ + success: true, + organizationId: 'org-new', + fixedSeats: false, + }) + + queueWhereResponses([ + [ + { + id: 'inv-1', + kind: 'workspace', + email: 'invitee@example.com', + organizationId: null, + membershipIntent: 'internal', + inviterId: 'owner-1', + role: 'member', + status: 'pending', + token: 'tok-1', + expiresAt: new Date(Date.now() + 60_000), + createdAt: new Date(), + updatedAt: new Date(), + }, + ], + [ + { + id: 'grant-1', + workspaceId: 'workspace-1', + permission: 'write', + workspaceName: 'Workspace', + }, + ], + [{ name: 'Owner', email: 'owner@example.com' }], + // Invitee-owned personal workspaces for the acceptance lock plan. + [], + // Billing-owner workspaces included in the pre-lock conversion plan. + [{ id: 'workspace-1' }], + // A new personal workspace appeared before provisioning acquired the + // billing owner's identity lock, so it escaped the original plan. + [{ id: 'workspace-2' }], + ]) + + const result = await acceptInvitation({ + userId: 'invitee-user', + userEmail: 'invitee@example.com', + invitationId: 'inv-1', + token: 'tok-1', + }) + + expect(result).toEqual({ + success: false, + kind: 'server-error', + message: "The workspace owner's workspaces changed while accepting — please try again.", + }) + expect(mockEnsureTeamOrganizationForAcceptance).toHaveBeenCalledWith( + expect.objectContaining({ + billingOwnerUserId: 'owner-1', + workspaceOrganizationId: null, + workspaceIdsToAttach: ['workspace-1'], + executor: dbChainMock.db, + }) + ) + expect(mockEnsureUserInOrganization).not.toHaveBeenCalled() + expect(auditMock.recordAudit).not.toHaveBeenCalled() + }) + + it('maps an unexpected provisioning failure only after the acceptance transaction rejects', async () => { + mockGetWorkspaceWithOwner.mockResolvedValue({ + id: 'workspace-1', + name: 'Workspace', + ownerId: 'owner-1', + organizationId: null, + workspaceMode: 'personal', + billedAccountUserId: 'owner-1', + }) + mockEnsureTeamOrganizationForAcceptance.mockRejectedValueOnce( + new Error('subscription re-home failed') + ) + + queueWhereResponses([ + [ + { + id: 'inv-1', + kind: 'workspace', + email: 'invitee@example.com', + organizationId: null, + membershipIntent: 'internal', + inviterId: 'owner-1', + role: 'member', + status: 'pending', + token: 'tok-1', + expiresAt: new Date(Date.now() + 60_000), + createdAt: new Date(), + updatedAt: new Date(), + }, + ], + [ + { + id: 'grant-1', + workspaceId: 'workspace-1', + permission: 'write', + workspaceName: 'Workspace', + }, + ], + [{ name: 'Owner', email: 'owner@example.com' }], + // Invitee-owned personal workspaces for the acceptance lock plan. + [], + // Billing-owner workspaces covered by the conversion lock plan. + [{ id: 'workspace-1' }], + ]) + + const result = await acceptInvitation({ + userId: 'invitee-user', + userEmail: 'invitee@example.com', + invitationId: 'inv-1', + token: 'tok-1', + }) + + expect(result).toEqual({ success: false, kind: 'server-error' }) + expect(mockEnsureUserInOrganization).not.toHaveBeenCalled() + expect(auditMock.recordAudit).not.toHaveBeenCalled() + }) + it('re-reads the workspace after locking when another acceptance attaches it first', async () => { mockGetWorkspaceWithOwner .mockResolvedValueOnce({ @@ -1726,7 +1869,7 @@ describe('acceptInvitation', () => { invitationId: 'inv-1', token: 'tok-1', }) - ).rejects.toThrow('commit failed') + ).resolves.toEqual({ success: false, kind: 'server-error' }) expect(auditMock.recordAudit).not.toHaveBeenCalled() expect(mockReconcileOrganizationSeats).not.toHaveBeenCalled() @@ -1857,3 +2000,184 @@ describe('acceptInvitation', () => { expect(mockSetActiveOrganizationForCurrentSession).not.toHaveBeenCalled() }) }) + +function invitationHydrationRows(params?: { + status?: 'pending' | 'accepted' + organizationId?: string +}) { + const organizationId = params?.organizationId ?? 'org-1' + return [ + [ + { + id: 'inv-1', + kind: 'organization', + email: 'invitee@example.com', + organizationId, + membershipIntent: 'internal', + inviterId: 'owner-1', + role: 'member', + status: params?.status ?? 'pending', + token: 'tok-1', + expiresAt: new Date(Date.now() + 60_000), + createdAt: new Date(), + updatedAt: new Date('2026-07-30T12:00:00.000Z'), + }, + ], + [ + { + id: 'grant-1', + workspaceId: 'workspace-1', + permission: 'write', + workspaceName: 'Workspace', + }, + ], + [{ name: 'Acme' }], + [{ name: 'Owner', email: 'owner@example.com' }], + ] +} + +describe('locked invitation mutations', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockGetWorkspaceWithOwner.mockResolvedValue({ + id: 'workspace-1', + name: 'Workspace', + ownerId: 'owner-1', + organizationId: null, + workspaceMode: 'personal', + billedAccountUserId: 'owner-1', + }) + }) + + it('reject cannot overwrite an invitation accepted before its protected re-read', async () => { + queueWhereResponses([ + ...invitationHydrationRows({ status: 'pending' }), + ...invitationHydrationRows({ status: 'accepted' }), + ]) + + const result = await rejectInvitation({ + userId: 'invitee-user', + userEmail: 'invitee@example.com', + invitationId: 'inv-1', + token: 'tok-1', + }) + + expect(result).toEqual({ success: false, kind: 'already-processed' }) + expect(dbChainMockFns.set).not.toHaveBeenCalledWith( + expect.objectContaining({ status: 'rejected' }) + ) + }) + + it('PATCH locks its workspace before row claim and authorizes the protected state', async () => { + queueWhereResponses([ + ...invitationHydrationRows({ organizationId: 'org-old' }), + ...invitationHydrationRows({ organizationId: 'org-new' }), + [{ id: 'permission-1', permissionType: 'admin' }], + [{ id: 'inv-1' }], + [], + ]) + + const result = await updateInvitation({ + actorId: 'admin-1', + invitationId: 'inv-1', + grants: [{ workspaceId: 'workspace-1', permission: 'admin' }], + }) + + expect(result.success).toBe(true) + const executedSql = dbChainMockFns.execute.mock.calls.map(([query]) => + ((query as { strings?: readonly string[] }).strings ?? []).join('') + ) + const invitationLockIndex = executedSql.findIndex((sqlText) => + sqlText.includes('pg_advisory_xact_lock') + ) + const workspaceLockIndex = executedSql.findIndex( + (sqlText, index) => index > invitationLockIndex && sqlText.includes('pg_advisory_xact_lock') + ) + const rowLockIndex = executedSql.findIndex((sqlText) => sqlText.includes('for update')) + expect(invitationLockIndex).toBeGreaterThanOrEqual(0) + expect(workspaceLockIndex).toBeGreaterThan(invitationLockIndex) + expect(rowLockIndex).toBeGreaterThan(workspaceLockIndex) + expect(dbChainMockFns.for).toHaveBeenCalledWith('update') + expect(dbChainMockFns.for.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.set.mock.invocationCallOrder[0] + ) + }) + + it('PATCH never mutates a non-pending protected re-read', async () => { + queueWhereResponses([ + ...invitationHydrationRows({ status: 'pending' }), + ...invitationHydrationRows({ status: 'accepted' }), + ]) + + await expect( + updateInvitation({ + actorId: 'admin-1', + invitationId: 'inv-1', + role: 'admin', + }) + ).resolves.toEqual({ success: false, kind: 'not-pending' }) + expect(dbChainMockFns.for).not.toHaveBeenCalled() + expect(dbChainMockFns.set).not.toHaveBeenCalled() + }) + + it('DELETE authorizes against the organization from its protected re-read', async () => { + queueWhereResponses([ + ...invitationHydrationRows({ organizationId: 'org-old' }), + ...invitationHydrationRows({ organizationId: 'org-new' }), + [{ id: 'member-1', role: 'admin' }], + [{ id: 'inv-1' }], + ]) + + const result = await revokeInvitationAsAdmin({ + actorId: 'admin-1', + invitationId: 'inv-1', + }) + + expect(result.success).toBe(true) + expect(dbChainMockFns.for).toHaveBeenCalledWith('update') + expect(dbChainMockFns.for.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.set.mock.invocationCallOrder[0] + ) + }) + + it('PATCH role update observes an organization-admin demotion before mutating', async () => { + queueWhereResponses([ + ...invitationHydrationRows(), + ...invitationHydrationRows(), + [{ id: 'member-1', role: 'member' }], + ]) + + await expect( + updateInvitation({ + actorId: 'admin-1', + invitationId: 'inv-1', + role: 'admin', + }) + ).resolves.toEqual({ success: false, kind: 'organization-forbidden' }) + expect(dbChainMockFns.for).toHaveBeenCalledWith('update') + expect(dbChainMockFns.set).not.toHaveBeenCalled() + }) + + it('PATCH grant update observes an explicit-admin downgrade before mutating', async () => { + queueWhereResponses([ + ...invitationHydrationRows(), + ...invitationHydrationRows(), + [{ id: 'permission-1', permissionType: 'write' }], + ]) + + await expect( + updateInvitation({ + actorId: 'admin-1', + invitationId: 'inv-1', + grants: [{ workspaceId: 'workspace-1', permission: 'admin' }], + }) + ).resolves.toEqual({ + success: false, + kind: 'workspace-forbidden', + workspaceId: 'workspace-1', + }) + expect(dbChainMockFns.for).toHaveBeenCalledWith('update') + expect(dbChainMockFns.set).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/invitations/core.ts b/apps/sim/lib/invitations/core.ts index f74b39a119e..13be762010e 100644 --- a/apps/sim/lib/invitations/core.ts +++ b/apps/sim/lib/invitations/core.ts @@ -89,6 +89,106 @@ export async function getInvitationById( return hydrateInvitation(row, executor) } +/** + * Claims an invitation for a state-changing operation using the same advisory + * lock namespace and ordering as acceptance, sending, and workspace moves. + * + * The invitation advisory lock is taken first because its grants define the + * optional workspace lock set. Workspace advisory locks are then taken before + * the invitation row lock, preserving the platform's advisory-before-row lock + * order. Finally the invitation is hydrated again so authorization and mutation + * use the protected state. + */ +export async function lockInvitationForMutation( + tx: DbOrTx, + invitationId: string, + options?: { + lockCurrentGrantWorkspaces?: boolean + additionalWorkspaceIds?: string[] + } +): Promise { + await acquireInvitationMutationLocks(tx, { + invitationIds: [invitationId], + workspaceIds: [], + }) + const beforeWorkspaceLocks = await getInvitationById(invitationId, tx) + if (!beforeWorkspaceLocks) return null + + const currentGrantWorkspaceIds = new Set( + beforeWorkspaceLocks.grants.map((grant) => grant.workspaceId) + ) + const workspaceIds = [ + ...new Set([ + ...(options?.lockCurrentGrantWorkspaces ? currentGrantWorkspaceIds : []), + ...(options?.additionalWorkspaceIds ?? []).filter((workspaceId) => + currentGrantWorkspaceIds.has(workspaceId) + ), + ]), + ] + if (workspaceIds.length > 0) { + await acquireInvitationMutationLocks(tx, { + invitationIds: [], + workspaceIds, + }) + } + + await tx.execute(sql`select id from invitation where id = ${invitationId} for update`) + + return getInvitationById(invitationId, tx) +} + +/** + * Locks the exact membership row that can authorize an organization mutation, + * then evaluates the role from that protected version. A concurrent demotion + * or removal either commits first and is observed here, or waits until this + * invitation transaction commits. + */ +async function lockOrganizationAdminAuthority( + tx: DbOrTx, + actorId: string, + organizationId: string +): Promise { + const [authority] = await tx + .select({ id: member.id, role: member.role }) + .from(member) + .where(and(eq(member.userId, actorId), eq(member.organizationId, organizationId))) + .for('update') + .limit(1) + return isOrgAdminRole(authority?.role) +} + +/** + * Locks only the actor rows that can grant admin standing on this workspace: + * the explicit workspace permission first, then (when needed) the workspace + * organization's membership row. Callers visit workspace ids in sorted order, + * giving multi-workspace mutations a deterministic authority-row lock order. + */ +async function lockWorkspaceAdminAuthority( + tx: DbOrTx, + actorId: string, + workspaceId: string +): Promise { + const ws = await getWorkspaceWithOwner(workspaceId, { executor: tx }) + if (!ws) return false + + const [explicitAuthority] = await tx + .select({ id: permissions.id, permissionType: permissions.permissionType }) + .from(permissions) + .where( + and( + eq(permissions.userId, actorId), + eq(permissions.entityType, 'workspace'), + eq(permissions.entityId, workspaceId) + ) + ) + .for('update') + .limit(1) + if (explicitAuthority?.permissionType === 'admin') return true + if (!ws.organizationId) return false + + return lockOrganizationAdminAuthority(tx, actorId, ws.organizationId) +} + async function hydrateInvitation( row: typeof invitation.$inferSelect, executor: DbOrTx = db @@ -482,6 +582,21 @@ class JoinerWorkspacesChangedDuringAcceptError extends Error { } } +/** + * Thrown after a personal subscription conversion when the billing owner + * created another attachable workspace after the pre-lock sweep plan was + * captured. The conversion now holds that owner's billing-identity lock, so + * this re-check is stable; rolling back lets the retry include the new + * workspace in the advisory-lock plan instead of leaving it personally billed + * after the subscription moved to the organization. + */ +class BillingOwnerWorkspacesChangedDuringAcceptError extends Error { + constructor() { + super('Billing owner workspaces changed during invite acceptance') + this.name = 'BillingOwnerWorkspacesChangedDuringAcceptError' + } +} + /** * Thrown when every grant on a member-role organization invite turned stale * (the workspaces left the stamped organization), which would strand the new @@ -682,6 +797,20 @@ export async function acceptInvitation( message: 'Your workspaces changed while accepting — please try again.', } } + if (error instanceof BillingOwnerWorkspacesChangedDuringAcceptError) { + logger.warn( + 'Invite acceptance rolled back: billing owner workspaces changed concurrently', + { + invitationId: input.invitationId, + userId: input.userId, + } + ) + return { + success: false, + kind: 'server-error', + message: "The workspace owner's workspaces changed while accepting — please try again.", + } + } if (error instanceof AllGrantsStaleDuringAcceptError) { logger.warn('Invite acceptance rolled back: every grant turned stale', { invitationId: input.invitationId, @@ -696,7 +825,17 @@ export async function acceptInvitation( }) return { success: false, kind: 'disclosure-outdated' } } - throw error + /** + * This catch is outside `db.transaction`, so reaching it guarantees + * Postgres has rolled back every provisioning, membership, workspace, + * invitation, permission, and outbox write from the failed attempt. + */ + logger.error('Invitation acceptance transaction failed and was rolled back', { + invitationId: input.invitationId, + userId: input.userId, + error, + }) + return { success: false, kind: 'server-error' } }) if (result.success) { await runInvitationAcceptancePostCommitEffects(input, effects) @@ -863,6 +1002,36 @@ async function acceptLockedInvitation( if (!orgResult.success) { return { success: false, kind: orgResult.failureCode } } + + /** + * A personal Pro→Team conversion acquires the billing owner's + * billing-identity lock and attaches every workspace from the pre-lock + * plan inside this transaction. Re-read only after that conversion: + * anything still attachable was created between the plan read and the + * identity lock, so it never received a workspace advisory lock. Abort + * the whole conversion/acceptance and let the retry plan include it. + * + * Do not take the identity lock here before provisioning. Organization + * membership paths acquire organization → identity, and reversing that + * order would introduce a deadlock. + */ + if (!workspaceOrganizationId) { + const [unplannedBillingOwnerWorkspace] = await tx + .select({ id: workspace.id }) + .from(workspace) + .where( + ownedAttachableWorkspacesWhere({ + userId: billingOwnerUserId, + ownerMatch: 'billing-account', + includeArchived: true, + }) + ) + .limit(1) + if (unplannedBillingOwnerWorkspace) { + throw new BillingOwnerWorkspacesChangedDuringAcceptError() + } + } + targetOrganizationId = orgResult.organizationId fixedSeats = orgResult.fixedSeats if (orgResult.postCommitEffects) { @@ -1324,41 +1493,248 @@ export type RejectInvitationResult = | { success: true; invitation: InvitationWithGrants } | { success: false; kind: AcceptInvitationFailure['kind'] } +export type UpdateInvitationFailureKind = + | 'not-found' + | 'not-pending' + | 'external-role' + | 'role-not-organization-scoped' + | 'organization-forbidden' + | 'member-requires-workspace' + | 'grant-not-found' + | 'workspace-forbidden' + +export type UpdateInvitationResult = + | { success: true; invitation: InvitationWithGrants } + | { + success: false + kind: UpdateInvitationFailureKind + workspaceId?: string + } + +/** + * Updates a pending invitation only after claiming the invitation and all of + * its workspaces in the shared mutation lock namespace. Authorization is + * intentionally evaluated inside that transaction against the locked, + * re-hydrated invitation so a workspace move or acceptance cannot turn an + * authorized pre-lock snapshot into an unauthorized write. + */ +export async function updateInvitation(input: { + actorId: string + invitationId: string + role?: 'admin' | 'member' + grants?: Array<{ workspaceId: string; permission: PermissionType }> +}): Promise { + return db.transaction(async (tx): Promise => { + const inv = await lockInvitationForMutation(tx, input.invitationId, { + additionalWorkspaceIds: input.grants?.map((grant) => grant.workspaceId) ?? [], + }) + if (!inv) return { success: false, kind: 'not-found' } + if (inv.status !== 'pending') return { success: false, kind: 'not-pending' } + + if (input.role !== undefined) { + if (inv.membershipIntent === 'external') { + return { success: false, kind: 'external-role' } + } + if (!inv.organizationId) { + return { success: false, kind: 'role-not-organization-scoped' } + } + if (!(await lockOrganizationAdminAuthority(tx, input.actorId, inv.organizationId))) { + return { success: false, kind: 'organization-forbidden' } + } + if (!isOrgAdminRole(input.role) && inv.grants.length === 0) { + return { success: false, kind: 'member-requires-workspace' } + } + } + + const grantsToApply = input.grants ?? [] + const grantWorkspaceIds = [...new Set(grantsToApply.map((grant) => grant.workspaceId))].sort() + for (const workspaceId of grantWorkspaceIds) { + if (!inv.grants.some((grant) => grant.workspaceId === workspaceId)) { + return { + success: false, + kind: 'grant-not-found', + workspaceId, + } + } + if (!(await lockWorkspaceAdminAuthority(tx, input.actorId, workspaceId))) { + return { + success: false, + kind: 'workspace-forbidden', + workspaceId, + } + } + } + + const now = new Date() + const [claimed] = await tx + .update(invitation) + .set({ + ...(input.role !== undefined && input.role !== inv.role ? { role: input.role } : {}), + updatedAt: now, + }) + .where(and(eq(invitation.id, input.invitationId), eq(invitation.status, 'pending'))) + .returning({ id: invitation.id }) + if (!claimed) return { success: false, kind: 'not-pending' } + + for (const update of grantsToApply) { + await tx + .update(invitationWorkspaceGrant) + .set({ permission: update.permission, updatedAt: now }) + .where( + and( + eq(invitationWorkspaceGrant.invitationId, input.invitationId), + eq(invitationWorkspaceGrant.workspaceId, update.workspaceId) + ) + ) + } + + return { + success: true, + invitation: { + ...inv, + role: input.role ?? inv.role, + updatedAt: now, + grants: inv.grants.map((grant) => { + const update = grantsToApply.find( + (candidate) => candidate.workspaceId === grant.workspaceId + ) + return update ? { ...grant, permission: update.permission } : grant + }), + }, + } + }) +} + export async function rejectInvitation( input: AcceptInvitationInput ): Promise { - const inv = await getInvitationById(input.invitationId) + return db.transaction(async (tx): Promise => { + const inv = await lockInvitationForMutation(tx, input.invitationId) + + if (!inv) return { success: false, kind: 'not-found' } + if (input.token && inv.token !== input.token) return { success: false, kind: 'invalid-token' } + if (inv.status !== 'pending') return { success: false, kind: 'already-processed' } + if (isInvitationExpired(inv)) { + const expired = await tx + .update(invitation) + .set({ status: 'expired', updatedAt: new Date() }) + .where(and(eq(invitation.id, inv.id), eq(invitation.status, 'pending'))) + .returning({ id: invitation.id }) + return { + success: false, + kind: expired.length > 0 ? 'expired' : 'already-processed', + } + } + if (normalizeEmail(input.userEmail) !== normalizeEmail(inv.email)) { + return { success: false, kind: 'email-mismatch' } + } - if (!inv) return { success: false, kind: 'not-found' } - if (input.token && inv.token !== input.token) return { success: false, kind: 'invalid-token' } - if (inv.status !== 'pending') return { success: false, kind: 'already-processed' } - if (isInvitationExpired(inv)) { - await db + const now = new Date() + const rejected = await tx .update(invitation) - .set({ status: 'expired', updatedAt: new Date() }) + .set({ status: 'rejected', updatedAt: now }) .where(and(eq(invitation.id, inv.id), eq(invitation.status, 'pending'))) - return { success: false, kind: 'expired' } - } - if (normalizeEmail(input.userEmail) !== normalizeEmail(inv.email)) { - return { success: false, kind: 'email-mismatch' } - } - - await db - .update(invitation) - .set({ status: 'rejected', updatedAt: new Date() }) - .where(eq(invitation.id, inv.id)) + .returning({ id: invitation.id }) + if (rejected.length === 0) { + return { success: false, kind: 'already-processed' } + } - return { success: true, invitation: { ...inv, status: 'rejected' } } + return { success: true, invitation: { ...inv, status: 'rejected', updatedAt: now } } + }) } -export async function cancelInvitation(invitationId: string): Promise { - const result = await db - .update(invitation) - .set({ status: 'cancelled', updatedAt: new Date() }) - .where(and(eq(invitation.id, invitationId), eq(invitation.status, 'pending'))) - .returning({ id: invitation.id }) +export type AuthorizedInvitationRevocationResult = + | { + success: true + invitation: InvitationWithGrants + invitationCancelled: boolean + } + | { + success: false + kind: + | 'not-found' + | 'not-pending' + | 'grant-not-found' + | 'scoped-forbidden' + | 'whole-forbidden' + | 'not-cancellable' + spansMultipleWorkspaces?: boolean + } + +/** + * API-facing revocation path. Unlike generic internal cleanup helpers, this + * claims the invitation and relevant workspace scopes first, then evaluates + * the actor's organization/workspace authority against the protected live + * state in the same transaction as the conditional pending-only mutation. + */ +export async function revokeInvitationAsAdmin(input: { + actorId: string + invitationId: string + workspaceId?: string +}): Promise { + return db.transaction(async (tx): Promise => { + const inv = await lockInvitationForMutation(tx, input.invitationId, { + lockCurrentGrantWorkspaces: input.workspaceId === undefined, + additionalWorkspaceIds: input.workspaceId ? [input.workspaceId] : [], + }) + if (!inv) return { success: false, kind: 'not-found' } + if (inv.status !== 'pending') return { success: false, kind: 'not-pending' } + + const isOrganizationAdmin = inv.organizationId + ? await lockOrganizationAdminAuthority(tx, input.actorId, inv.organizationId) + : false + + if (input.workspaceId) { + if (!inv.grants.some((grant) => grant.workspaceId === input.workspaceId)) { + return { success: false, kind: 'grant-not-found' } + } + if ( + !isOrganizationAdmin && + !(await lockWorkspaceAdminAuthority(tx, input.actorId, input.workspaceId)) + ) { + return { success: false, kind: 'scoped-forbidden' } + } + + const revoked = await revokeInvitationWorkspaceGrantTx(tx, { + invitationId: input.invitationId, + workspaceId: input.workspaceId, + }) + if (!revoked.revoked) return { success: false, kind: 'not-cancellable' } + return { + success: true, + invitation: inv, + invitationCancelled: revoked.invitationCancelled, + } + } - return result.length > 0 + let canCancel = isOrganizationAdmin + if (!canCancel && inv.grants.length > 0) { + canCancel = true + const workspaceIds = [...new Set(inv.grants.map((grant) => grant.workspaceId))].sort() + for (const workspaceId of workspaceIds) { + if (!(await lockWorkspaceAdminAuthority(tx, input.actorId, workspaceId))) { + canCancel = false + break + } + } + } + if (!canCancel) { + return { + success: false, + kind: 'whole-forbidden', + spansMultipleWorkspaces: inv.grants.length > 1, + } + } + + const cancelled = await tx + .update(invitation) + .set({ status: 'cancelled', updatedAt: new Date() }) + .where(and(eq(invitation.id, input.invitationId), eq(invitation.status, 'pending'))) + .returning({ id: invitation.id }) + if (cancelled.length === 0) return { success: false, kind: 'not-cancellable' } + + return { success: true, invitation: inv, invitationCancelled: true } + }) } /** @@ -1370,53 +1746,60 @@ export async function cancelInvitation(invitationId: string): Promise { * admin of one workspace has no authority over the others. Removing the final * grant would otherwise strand a pending invitation that grants nothing, so * that case cancels it instead. + * + * Transaction-local: callers must already hold the canonical + * invitation/workspace advisory lock set. Keeping the grant deletion and + * final-grant cancellation in one implementation prevents direct grants, + * scoped revocation, and future callers from drifting on multi-workspace + * invitation semantics. */ -export async function revokeInvitationWorkspaceGrant({ - invitationId, - workspaceId, -}: { - invitationId: string - workspaceId: string -}): Promise<{ revoked: boolean; invitationCancelled: boolean }> { - return db.transaction(async (tx) => { - const [pending] = await tx - .select({ id: invitation.id }) - .from(invitation) - .where(and(eq(invitation.id, invitationId), eq(invitation.status, 'pending'))) - .for('update') - .limit(1) - if (!pending) return { revoked: false, invitationCancelled: false } +export async function revokeInvitationWorkspaceGrantTx( + tx: DbOrTx, + { + invitationId, + workspaceId, + }: { + invitationId: string + workspaceId: string + } +): Promise<{ revoked: boolean; invitationCancelled: boolean }> { + const [pending] = await tx + .select({ id: invitation.id }) + .from(invitation) + .where(and(eq(invitation.id, invitationId), eq(invitation.status, 'pending'))) + .for('update') + .limit(1) + if (!pending) return { revoked: false, invitationCancelled: false } - const removed = await tx - .delete(invitationWorkspaceGrant) - .where( - and( - eq(invitationWorkspaceGrant.invitationId, invitationId), - eq(invitationWorkspaceGrant.workspaceId, workspaceId) - ) + const removed = await tx + .delete(invitationWorkspaceGrant) + .where( + and( + eq(invitationWorkspaceGrant.invitationId, invitationId), + eq(invitationWorkspaceGrant.workspaceId, workspaceId) ) - .returning({ id: invitationWorkspaceGrant.id }) - if (removed.length === 0) return { revoked: false, invitationCancelled: false } - - const [remaining] = await tx - .select({ value: count() }) - .from(invitationWorkspaceGrant) - .where(eq(invitationWorkspaceGrant.invitationId, invitationId)) + ) + .returning({ id: invitationWorkspaceGrant.id }) + if (removed.length === 0) return { revoked: false, invitationCancelled: false } - if ((remaining?.value ?? 0) > 0) { - await tx - .update(invitation) - .set({ updatedAt: new Date() }) - .where(eq(invitation.id, invitationId)) - return { revoked: true, invitationCancelled: false } - } + const [remaining] = await tx + .select({ value: count() }) + .from(invitationWorkspaceGrant) + .where(eq(invitationWorkspaceGrant.invitationId, invitationId)) + if ((remaining?.value ?? 0) > 0) { await tx .update(invitation) - .set({ status: 'cancelled', updatedAt: new Date() }) + .set({ updatedAt: new Date() }) .where(eq(invitation.id, invitationId)) - return { revoked: true, invitationCancelled: true } - }) + return { revoked: true, invitationCancelled: false } + } + + await tx + .update(invitation) + .set({ status: 'cancelled', updatedAt: new Date() }) + .where(eq(invitation.id, invitationId)) + return { revoked: true, invitationCancelled: true } } /** diff --git a/apps/sim/lib/invitations/direct-grant.test.ts b/apps/sim/lib/invitations/direct-grant.test.ts index db987ac4603..58b3dda1058 100644 --- a/apps/sim/lib/invitations/direct-grant.test.ts +++ b/apps/sim/lib/invitations/direct-grant.test.ts @@ -1,20 +1,35 @@ /** * @vitest-environment node */ -import { auditMock, auditMockFns, dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { invitation, member } from '@sim/db/schema' +import { + auditMock, + auditMockFns, + dbChainMockFns, + queueTableRows, + resetDbChainMock, +} from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' const { + mockAcquireInvitationMutationLocks, + mockAcquireOrganizationUserMutationLocks, mockGetUserOrganization, + mockGetEffectiveWorkspacePermission, + mockGetWorkspaceWithOwner, + mockRevokeInvitationWorkspaceGrantTx, mockSyncWorkspaceEnvCredentials, - mockCancelPendingInvitation, mockSendWorkspaceAddedEmail, mockCaptureServerEvent, mockWorkspaceMemberAdded, } = vi.hoisted(() => ({ + mockAcquireInvitationMutationLocks: vi.fn(), + mockAcquireOrganizationUserMutationLocks: vi.fn(), mockGetUserOrganization: vi.fn(), + mockGetEffectiveWorkspacePermission: vi.fn(), + mockGetWorkspaceWithOwner: vi.fn(), + mockRevokeInvitationWorkspaceGrantTx: vi.fn(), mockSyncWorkspaceEnvCredentials: vi.fn(), - mockCancelPendingInvitation: vi.fn(), mockSendWorkspaceAddedEmail: vi.fn(), mockCaptureServerEvent: vi.fn(), mockWorkspaceMemberAdded: vi.fn(), @@ -23,9 +38,18 @@ const { vi.mock('@sim/audit', () => auditMock) vi.mock('@/lib/billing/organizations/membership', () => ({ + acquireOrganizationUserMutationLocks: mockAcquireOrganizationUserMutationLocks, getUserOrganization: mockGetUserOrganization, })) +vi.mock('@/lib/invitations/locks', () => ({ + acquireInvitationMutationLocks: mockAcquireInvitationMutationLocks, +})) + +vi.mock('@/lib/invitations/core', () => ({ + revokeInvitationWorkspaceGrantTx: mockRevokeInvitationWorkspaceGrantTx, +})) + vi.mock('@/lib/core/telemetry', () => ({ PlatformEvents: { workspaceMemberAdded: mockWorkspaceMemberAdded }, })) @@ -35,37 +59,22 @@ vi.mock('@/lib/credentials/environment', () => ({ })) vi.mock('@/lib/invitations/send', () => ({ - cancelPendingInvitation: mockCancelPendingInvitation, sendWorkspaceAddedEmail: mockSendWorkspaceAddedEmail, })) +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + getEffectiveWorkspacePermission: mockGetEffectiveWorkspacePermission, + getWorkspaceWithOwner: mockGetWorkspaceWithOwner, +})) + vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mockCaptureServerEvent, })) -import { grantWorkspaceAccessDirectly, isSameOrgMember } from '@/lib/invitations/direct-grant' - -/** - * Drives `db.select().from().where()` results in call order. Both an awaited - * `where()` and a chained `.limit()` resolve to the same per-call value. - */ -function queueWhereResponses(responses: unknown[][]) { - const queue = [...responses] - dbChainMockFns.where.mockImplementation(() => { - const result = queue.shift() ?? [] - const thenable = Promise.resolve(result) as Promise & { - limit: ReturnType - orderBy: ReturnType - returning: ReturnType - groupBy: ReturnType - } - thenable.limit = vi.fn(() => Promise.resolve(result)) - thenable.orderBy = vi.fn(() => Promise.resolve(result)) - thenable.returning = vi.fn(() => Promise.resolve(result)) - thenable.groupBy = vi.fn(() => Promise.resolve(result)) - return thenable as ReturnType - }) -} +import { + DirectGrantContextChangedError, + grantWorkspaceAccessDirectly, +} from '@/lib/invitations/direct-grant' const baseInput = { userId: 'user-2', @@ -83,6 +92,25 @@ describe('grantWorkspaceAccessDirectly', () => { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() + mockAcquireInvitationMutationLocks.mockResolvedValue(undefined) + mockAcquireOrganizationUserMutationLocks.mockResolvedValue(undefined) + mockGetUserOrganization.mockResolvedValue({ + organizationId: 'org-1', + role: 'member', + }) + mockGetWorkspaceWithOwner.mockResolvedValue({ + id: 'ws-1', + name: 'Workspace 1', + ownerId: 'user-1', + organizationId: 'org-1', + workspaceMode: 'organization', + billedAccountUserId: 'user-1', + }) + mockGetEffectiveWorkspacePermission.mockResolvedValue('admin') + mockRevokeInvitationWorkspaceGrantTx.mockResolvedValue({ + revoked: true, + invitationCancelled: false, + }) mockSendWorkspaceAddedEmail.mockResolvedValue({ success: true }) // Insert path reports the new row via `.returning()`. dbChainMockFns.returning.mockResolvedValue([{ id: 'perm-new' }]) @@ -103,6 +131,14 @@ describe('grantWorkspaceAccessDirectly', () => { expect(mockSendWorkspaceAddedEmail).toHaveBeenCalledWith( expect.objectContaining({ email: 'member@example.com', workspaceId: 'ws-1' }) ) + expect(dbChainMockFns.for.mock.invocationCallOrder[0]).toBeLessThan( + mockGetEffectiveWorkspacePermission.mock.invocationCallOrder[0] + ) + expect(dbChainMockFns.for).toHaveBeenCalledTimes(3) + expect(dbChainMockFns.for.mock.invocationCallOrder[1]).toBeLessThan( + mockGetEffectiveWorkspacePermission.mock.invocationCallOrder[0] + ) + expect(dbChainMockFns.from).toHaveBeenCalledWith(member) }) it('reports unchanged (no audit/email) when a concurrent insert wins the race', async () => { @@ -167,41 +203,45 @@ describe('grantWorkspaceAccessDirectly', () => { }) it('supersedes lingering pending workspace invitations for the same email', async () => { - queueWhereResponses([ - [], // existing permission lookup (transaction) - [{ invitationId: 'old-inv' }], // supersede lookup - [], // env lookup - ]) + queueTableRows(invitation, [{ invitationId: 'old-inv' }]) + queueTableRows(invitation, [{ invitationId: 'old-inv' }]) await grantWorkspaceAccessDirectly({ ...baseInput }) - expect(mockCancelPendingInvitation).toHaveBeenCalledWith('old-inv') - }) -}) - -describe('isSameOrgMember', () => { - beforeEach(() => { - vi.clearAllMocks() - resetDbChainMock() - }) - - it('returns false when the workspace has no organization', async () => { - expect(await isSameOrgMember('user-2', null)).toBe(false) - expect(mockGetUserOrganization).not.toHaveBeenCalled() + expect(mockRevokeInvitationWorkspaceGrantTx).toHaveBeenCalledWith(expect.anything(), { + invitationId: 'old-inv', + workspaceId: 'ws-1', + }) }) - it('returns false when the user belongs to no organization', async () => { - mockGetUserOrganization.mockResolvedValueOnce(null) - expect(await isSameOrgMember('user-2', 'org-1')).toBe(false) - }) - - it('returns true when the user belongs to the workspace organization', async () => { - mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-1', role: 'member' }) - expect(await isSameOrgMember('user-2', 'org-1')).toBe(true) - }) + it('locks before re-reading scope and aborts when the workspace moved', async () => { + mockGetWorkspaceWithOwner.mockResolvedValueOnce({ + id: 'ws-1', + name: 'Workspace 1', + ownerId: 'user-1', + organizationId: 'org-2', + workspaceMode: 'organization', + billedAccountUserId: 'user-3', + }) + + await expect(grantWorkspaceAccessDirectly({ ...baseInput })).rejects.toBeInstanceOf( + DirectGrantContextChangedError + ) - it('returns false when the user belongs to a different organization', async () => { - mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-2', role: 'member' }) - expect(await isSameOrgMember('user-2', 'org-1')).toBe(false) + expect(mockAcquireInvitationMutationLocks).toHaveBeenCalledWith(expect.anything(), { + invitationIds: [], + workspaceIds: ['ws-1'], + }) + expect(mockAcquireOrganizationUserMutationLocks).toHaveBeenCalledWith(expect.anything(), { + userId: 'user-2', + organizationIds: ['org-1'], + }) + expect(mockAcquireInvitationMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0] + ) + expect(mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + mockGetWorkspaceWithOwner.mock.invocationCallOrder[0] + ) + expect(dbChainMockFns.insert).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/lib/invitations/direct-grant.ts b/apps/sim/lib/invitations/direct-grant.ts index 93118f7034a..29601204537 100644 --- a/apps/sim/lib/invitations/direct-grant.ts +++ b/apps/sim/lib/invitations/direct-grant.ts @@ -3,20 +3,31 @@ import { db } from '@sim/db' import { invitation, invitationWorkspaceGrant, + member, permissions, workspaceEnvironment, } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' import { normalizeEmail } from '@sim/utils/string' -import { and, eq } from 'drizzle-orm' +import { and, eq, sql } from 'drizzle-orm' import type { NextRequest } from 'next/server' -import { getUserOrganization } from '@/lib/billing/organizations/membership' +import { + acquireOrganizationUserMutationLocks, + getUserOrganization, +} from '@/lib/billing/organizations/membership' import { PlatformEvents } from '@/lib/core/telemetry' import { syncWorkspaceEnvCredentials } from '@/lib/credentials/environment' -import { cancelPendingInvitation, sendWorkspaceAddedEmail } from '@/lib/invitations/send' +import type { DbOrTx } from '@/lib/db/types' +import { revokeInvitationWorkspaceGrantTx } from '@/lib/invitations/core' +import { acquireInvitationMutationLocks } from '@/lib/invitations/locks' +import { sendWorkspaceAddedEmail } from '@/lib/invitations/send' import { captureServerEvent } from '@/lib/posthog/server' -import type { PermissionType } from '@/lib/workspaces/permissions/utils' +import { + getEffectiveWorkspacePermission, + getWorkspaceWithOwner, + type PermissionType, +} from '@/lib/workspaces/permissions/utils' const logger = createLogger('InvitationDirectGrant') @@ -24,6 +35,20 @@ export type DirectGrantOutcome = | { outcome: 'added'; permission: PermissionType } | { outcome: 'unchanged'; permission: PermissionType } +export class DirectGrantContextChangedError extends Error { + constructor() { + super('Workspace or organization membership changed before access could be granted') + this.name = 'DirectGrantContextChangedError' + } +} + +class DirectGrantInvitationSetChangedError extends Error { + constructor(readonly invitationIds: string[]) { + super('Pending invitation set changed while granting workspace access') + this.name = 'DirectGrantInvitationSetChangedError' + } +} + export interface GrantWorkspaceAccessDirectlyInput { /** Registered user receiving access. */ userId: string @@ -42,46 +67,23 @@ export interface GrantWorkspaceAccessDirectlyInput { notify?: boolean } -/** - * Returns whether the given user is already a member of the workspace's - * organization. Only same-org members are eligible for direct (no-acceptance) - * workspace access. - */ -export async function isSameOrgMember( - userId: string, - workspaceOrganizationId: string | null -): Promise { - if (!workspaceOrganizationId) return false - const membership = await getUserOrganization(userId) - return !!membership && membership.organizationId === workspaceOrganizationId -} - -/** - * Cancels any pending single-workspace invitations that grant exactly this - * workspace to this email. Multi-workspace organization invitations are left - * untouched — their remaining grants stay valid and the accept flow upserts - * permissions idempotently. - */ -async function supersedePendingWorkspaceInvites( +async function getPendingWorkspaceInvitationIds( + executor: DbOrTx, workspaceId: string, normalizedEmail: string -): Promise { - const rows = await db +): Promise { + const rows = await executor .select({ invitationId: invitation.id }) .from(invitation) .innerJoin(invitationWorkspaceGrant, eq(invitationWorkspaceGrant.invitationId, invitation.id)) .where( and( - eq(invitation.kind, 'workspace'), - eq(invitation.email, normalizedEmail), + sql`lower(${invitation.email}) = ${normalizedEmail}`, eq(invitation.status, 'pending'), eq(invitationWorkspaceGrant.workspaceId, workspaceId) ) ) - - for (const row of rows) { - await cancelPendingInvitation(row.invitationId) - } + return [...new Set(rows.map((row) => row.invitationId))].sort() } /** @@ -95,57 +97,149 @@ export async function grantWorkspaceAccessDirectly( input: GrantWorkspaceAccessDirectlyInput ): Promise { const normalizedEmail = normalizeEmail(input.email) + let candidateInvitationIds = await getPendingWorkspaceInvitationIds( + db, + input.workspaceId, + normalizedEmail + ) + let result: DirectGrantOutcome | undefined + + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + result = await db.transaction(async (tx): Promise => { + /** + * Match acceptance/admin-move/removal ordering: invitation + workspace + * advisory locks first, then organization → user → membership locks. + * Every authorization read below is transaction-local and happens only + * after that shared fence. + */ + await acquireInvitationMutationLocks(tx, { + invitationIds: candidateInvitationIds, + workspaceIds: [input.workspaceId], + }) + await acquireOrganizationUserMutationLocks(tx, { + userId: input.userId, + organizationIds: [input.organizationId], + }) - const result = await db.transaction(async (tx): Promise => { - const [existing] = await tx - .select({ id: permissions.id, permissionType: permissions.permissionType }) - .from(permissions) - .where( - and( - eq(permissions.entityId, input.workspaceId), - eq(permissions.entityType, 'workspace'), - eq(permissions.userId, input.userId) + const currentInvitationIds = await getPendingWorkspaceInvitationIds( + tx, + input.workspaceId, + normalizedEmail ) - ) - .limit(1) + const candidateSet = new Set(candidateInvitationIds) + if (currentInvitationIds.some((id) => !candidateSet.has(id))) { + throw new DirectGrantInvitationSetChangedError(currentInvitationIds) + } - if (existing) { - return { outcome: 'unchanged', permission: existing.permissionType as PermissionType } - } + // Effective admin access may be inherited. Lock the exact actor member + // row so a concurrent direct role demotion cannot commit after this + // authorization check and before the permission insert. + await tx + .select({ id: member.id, role: member.role }) + .from(member) + .where( + and(eq(member.userId, input.actorId), eq(member.organizationId, input.organizationId)) + ) + .for('update') - const inserted = await tx - .insert(permissions) - .values({ - id: generateId(), - entityType: 'workspace', - entityId: input.workspaceId, - userId: input.userId, - permissionType: input.permission, - createdAt: new Date(), - updatedAt: new Date(), - }) - .onConflictDoNothing() - .returning({ id: permissions.id }) + const workspaceRow = await getWorkspaceWithOwner(input.workspaceId, { + executor: tx, + forUpdate: true, + }) + if (!workspaceRow || workspaceRow.organizationId !== input.organizationId) { + throw new DirectGrantContextChangedError() + } - if (inserted.length === 0) { - return { outcome: 'unchanged', permission: input.permission } - } + const inviteeMembership = await getUserOrganization(input.userId, tx) + await tx + .select({ id: permissions.id }) + .from(permissions) + .where( + and( + eq(permissions.entityType, 'workspace'), + eq(permissions.entityId, input.workspaceId), + eq(permissions.userId, input.actorId) + ) + ) + .for('update') + const actorPermission = await getEffectiveWorkspacePermission( + input.actorId, + workspaceRow, + tx + ) + if ( + inviteeMembership?.organizationId !== input.organizationId || + actorPermission !== 'admin' + ) { + throw new DirectGrantContextChangedError() + } - return { outcome: 'added', permission: input.permission } - }) + const [existing] = await tx + .select({ id: permissions.id, permissionType: permissions.permissionType }) + .from(permissions) + .where( + and( + eq(permissions.entityId, input.workspaceId), + eq(permissions.entityType, 'workspace'), + eq(permissions.userId, input.userId) + ) + ) + .for('update') + .limit(1) + + let outcome: DirectGrantOutcome + if (existing) { + outcome = { + outcome: 'unchanged', + permission: existing.permissionType as PermissionType, + } + } else { + const inserted = await tx + .insert(permissions) + .values({ + id: generateId(), + entityType: 'workspace', + entityId: input.workspaceId, + userId: input.userId, + permissionType: input.permission, + createdAt: new Date(), + updatedAt: new Date(), + }) + .onConflictDoNothing() + .returning({ id: permissions.id }) + + outcome = + inserted.length === 0 + ? { outcome: 'unchanged', permission: input.permission } + : { outcome: 'added', permission: input.permission } + } - if (result.outcome === 'unchanged') { - return result + if (currentInvitationIds.length > 0) { + for (const invitationId of currentInvitationIds) { + await revokeInvitationWorkspaceGrantTx(tx, { + invitationId, + workspaceId: input.workspaceId, + }) + } + } + + return outcome + }) + break + } catch (error) { + if (error instanceof DirectGrantInvitationSetChangedError) { + candidateInvitationIds = error.invitationIds + continue + } + throw error + } } - try { - await supersedePendingWorkspaceInvites(input.workspaceId, normalizedEmail) - } catch (error) { - logger.error('Failed to supersede pending workspace invitations after direct grant', { - workspaceId: input.workspaceId, - error, - }) + if (!result) { + throw new Error('Pending invitations kept changing while granting workspace access') } + if (result.outcome === 'unchanged') return result try { const [wsEnvRow] = await db diff --git a/apps/sim/lib/invitations/grant-revocation.test.ts b/apps/sim/lib/invitations/grant-revocation.test.ts new file mode 100644 index 00000000000..2142863a02c --- /dev/null +++ b/apps/sim/lib/invitations/grant-revocation.test.ts @@ -0,0 +1,56 @@ +/** + * @vitest-environment node + */ +import { db } from '@sim/db' +import { invitation, invitationWorkspaceGrant } from '@sim/db/schema' +import { auditMock, dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@sim/audit', () => auditMock) + +vi.mock('@/lib/invitations/locks', () => ({ + acquireInvitationMutationLocks: vi.fn(), +})) + +import { revokeInvitationWorkspaceGrantTx } from '@/lib/invitations/core' + +describe('revokeInvitationWorkspaceGrantTx', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('preserves sibling workspace grants', async () => { + queueTableRows(invitation, [{ id: 'inv-1' }]) + queueTableRows(invitationWorkspaceGrant, [{ value: 1 }]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'grant-1' }]) + + await expect( + revokeInvitationWorkspaceGrantTx(db, { + invitationId: 'inv-1', + workspaceId: 'ws-1', + }) + ).resolves.toEqual({ revoked: true, invitationCancelled: false }) + + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.not.objectContaining({ status: 'cancelled' }) + ) + }) + + it('cancels the invitation only after its final workspace grant is removed', async () => { + queueTableRows(invitation, [{ id: 'inv-1' }]) + queueTableRows(invitationWorkspaceGrant, [{ value: 0 }]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'grant-1' }]) + + await expect( + revokeInvitationWorkspaceGrantTx(db, { + invitationId: 'inv-1', + workspaceId: 'ws-1', + }) + ).resolves.toEqual({ revoked: true, invitationCancelled: true }) + + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ status: 'cancelled' }) + ) + }) +}) diff --git a/apps/sim/lib/invitations/send.test.ts b/apps/sim/lib/invitations/send.test.ts index cd2187609dd..44cc5c66cc9 100644 --- a/apps/sim/lib/invitations/send.test.ts +++ b/apps/sim/lib/invitations/send.test.ts @@ -1,10 +1,75 @@ /** * @vitest-environment node */ -import { describe, expect, it } from 'vitest' -import { createPendingInvitation, GrantlessInvitationError } from '@/lib/invitations/send' +import { workspace } from '@sim/db/schema' +import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + cancelPendingInvitation, + createPendingInvitation, + GrantlessInvitationError, + revertPendingInvitationGrants, +} from '@/lib/invitations/send' + +function queueWhereResponses(responses: unknown[][]) { + const queue = [...responses] + dbChainMockFns.where.mockImplementation(() => { + const result = queue.shift() ?? [] + const thenable = Promise.resolve(result) as Promise & { + limit: ReturnType + orderBy: ReturnType + returning: ReturnType + } + thenable.limit = vi.fn(() => Promise.resolve(result)) + thenable.orderBy = vi.fn(() => Promise.resolve(result)) + thenable.returning = vi.fn(() => Promise.resolve(result)) + return thenable as ReturnType + }) +} + +function hydrationRows(params?: { + status?: 'pending' | 'accepted' + organizationId?: string | null + updatedAt?: Date +}) { + const organizationId = params?.organizationId ?? null + const rows: unknown[][] = [ + [ + { + id: 'inv-1', + kind: 'workspace', + email: 'invitee@example.com', + organizationId, + membershipIntent: 'internal', + inviterId: 'owner-1', + role: 'member', + status: params?.status ?? 'pending', + token: 'tok-1', + expiresAt: new Date(Date.now() + 60_000), + createdAt: new Date(), + updatedAt: params?.updatedAt ?? new Date('2026-07-30T12:00:00.000Z'), + }, + ], + [ + { + id: 'grant-1', + workspaceId: 'workspace-1', + permission: 'write', + workspaceName: 'Workspace', + }, + ], + ] + if (organizationId) rows.push([{ name: 'Acme' }]) + rows.push([{ name: 'Owner', email: 'owner@example.com' }]) + return rows +} describe('createPendingInvitation', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + it.each(['member', 'admin'] as const)( 'rejects a %s-role organization invitation with no workspace grants', async (role) => { @@ -34,4 +99,144 @@ describe('createPendingInvitation', () => { }) ).rejects.toThrow(GrantlessInvitationError) }) + + it('runs locked-context validation after the shared invitation/workspace locks', async () => { + const validateLockedContext = vi.fn().mockResolvedValue(undefined) + queueTableRows(workspace, [{ organizationId: 'org-1' }]) + queueTableRows(workspace, [{ organizationId: 'org-1' }]) + + await createPendingInvitation({ + kind: 'workspace', + email: 'invitee@example.com', + inviterId: 'inviter-1', + organizationId: 'org-1', + membershipIntent: 'internal', + role: 'member', + grants: [{ workspaceId: 'workspace-1', permission: 'write' }], + validateLockedContext, + }) + + expect(validateLockedContext).toHaveBeenCalledWith( + expect.objectContaining({ + organizationId: 'org-1', + workspaceIds: ['workspace-1'], + }) + ) + expect(dbChainMockFns.execute.mock.invocationCallOrder[0]).toBeLessThan( + validateLockedContext.mock.invocationCallOrder[0] + ) + }) +}) + +describe('failed-send compensation', () => { + const expectedUpdatedAt = new Date('2026-07-30T12:00:00.000Z') + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('cancels an unchanged newly-created pending invitation', async () => { + queueWhereResponses([ + ...hydrationRows({ updatedAt: expectedUpdatedAt }), + ...hydrationRows({ updatedAt: expectedUpdatedAt }), + [{ id: 'inv-1' }], + ]) + + await expect( + cancelPendingInvitation('inv-1', { + expectedUpdatedAt, + expectedOrganizationId: null, + }) + ).resolves.toBe(true) + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ status: 'cancelled' }) + ) + }) + + it('does not cancel an invitation accepted while its email was in flight', async () => { + queueWhereResponses([ + ...hydrationRows({ status: 'pending', updatedAt: expectedUpdatedAt }), + ...hydrationRows({ status: 'accepted', updatedAt: expectedUpdatedAt }), + ]) + + await expect( + cancelPendingInvitation('inv-1', { + expectedUpdatedAt, + expectedOrganizationId: null, + }) + ).resolves.toBe(false) + expect(dbChainMockFns.set).not.toHaveBeenCalled() + }) + + it('does not cancel a pending invitation migrated while its email was in flight', async () => { + const migratedAt = new Date('2026-07-30T12:00:01.000Z') + queueWhereResponses([ + ...hydrationRows({ updatedAt: expectedUpdatedAt }), + ...hydrationRows({ organizationId: 'org-2', updatedAt: migratedAt }), + ]) + + await expect( + cancelPendingInvitation('inv-1', { + expectedUpdatedAt, + expectedOrganizationId: null, + }) + ).resolves.toBe(false) + expect(dbChainMockFns.set).not.toHaveBeenCalled() + }) + + it('does not delete merged grants after a later migration revision', async () => { + const migratedAt = new Date('2026-07-30T12:00:01.000Z') + queueWhereResponses([ + ...hydrationRows({ updatedAt: expectedUpdatedAt }), + ...hydrationRows({ organizationId: 'org-2', updatedAt: migratedAt }), + ]) + + await expect( + revertPendingInvitationGrants({ + invitationId: 'inv-1', + workspaceIds: ['workspace-1'], + expectedUpdatedAt, + expectedOrganizationId: null, + }) + ).resolves.toBe(false) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('does not delete grants after a later same-scope merge touched the invitation', async () => { + const laterMergeAt = new Date('2026-07-30T12:00:01.000Z') + queueWhereResponses([ + ...hydrationRows({ organizationId: 'org-1', updatedAt: expectedUpdatedAt }), + ...hydrationRows({ organizationId: 'org-1', updatedAt: laterMergeAt }), + ]) + + await expect( + revertPendingInvitationGrants({ + invitationId: 'inv-1', + workspaceIds: ['workspace-1'], + expectedUpdatedAt, + expectedOrganizationId: 'org-1', + }) + ).resolves.toBe(false) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('deletes only the added grants for an unchanged pending merge', async () => { + queueWhereResponses([ + ...hydrationRows({ updatedAt: expectedUpdatedAt }), + ...hydrationRows({ updatedAt: expectedUpdatedAt }), + [{ id: 'inv-1' }], + [], + ]) + + await expect( + revertPendingInvitationGrants({ + invitationId: 'inv-1', + workspaceIds: ['workspace-1'], + expectedUpdatedAt, + expectedOrganizationId: null, + }) + ).resolves.toBe(true) + expect(dbChainMockFns.delete).toHaveBeenCalledTimes(1) + }) }) diff --git a/apps/sim/lib/invitations/send.ts b/apps/sim/lib/invitations/send.ts index 661cc968be5..a49cb8c21c2 100644 --- a/apps/sim/lib/invitations/send.ts +++ b/apps/sim/lib/invitations/send.ts @@ -22,7 +22,7 @@ import { } from '@/components/emails' import { getBaseUrl } from '@/lib/core/utils/urls' import type { DbOrTx } from '@/lib/db/types' -import { computeInvitationExpiry } from '@/lib/invitations/core' +import { computeInvitationExpiry, lockInvitationForMutation } from '@/lib/invitations/core' import { acquireInvitationMutationLocks } from '@/lib/invitations/locks' import { sendEmail } from '@/lib/messaging/email/mailer' import { getFromEmailAddress } from '@/lib/messaging/email/utils' @@ -44,6 +44,17 @@ export interface CreatePendingInvitationInput { role: 'admin' | 'member' grants: WorkspaceGrantInput[] expiresAt?: Date + /** + * Runs after the canonical invitation/workspace advisory locks are held and + * the live organization scope has been resolved, but before any invitation + * write. Callers use this DB-only hook to acquire organization/user locks + * and re-authorize stale preflight decisions. + */ + validateLockedContext?: (context: { + tx: DbOrTx + organizationId: string | null + workspaceIds: string[] + }) => Promise } export interface CreatePendingInvitationResult { @@ -61,6 +72,14 @@ export interface CreatePendingInvitationResult { addedWorkspaceIds: string[] /** Every workspace the invitation now grants, oldest grant first. */ grants: WorkspaceGrantInput[] + /** + * Optimistic revision for failed-send compensation. A workspace move, + * acceptance, PATCH, or other later mutation changes this timestamp, causing + * compensation to skip rather than undo newer state. + */ + mutationUpdatedAt: Date + /** Scope paired with the revision so a migrated pending invite is never undone. */ + mutationOrganizationId: string | null } /** @@ -69,7 +88,7 @@ export interface CreatePendingInvitationResult { * person per organization, which is what makes coalescing mandatory rather * than optional. */ -const PENDING_INVITATION_UNIQUE_INDEX = 'invitation_pending_email_org_unique' +export const PENDING_INVITATION_UNIQUE_INDEX = 'invitation_pending_email_org_unique' /** * Raised when the granted workspaces changed organization between the @@ -124,6 +143,8 @@ async function findPendingOrganizationInvitation( expiresAt: invitation.expiresAt, role: invitation.role, membershipIntent: invitation.membershipIntent, + updatedAt: invitation.updatedAt, + organizationId: invitation.organizationId, }) .from(invitation) .where( @@ -252,6 +273,7 @@ async function createOrExtendPendingInvitation( }) const organizationId = await resolveInvitationOrganizationId(tx, input, workspaceIds) + await input.validateLockedContext?.({ tx, organizationId, workspaceIds }) const existing = organizationId ? await findPendingOrganizationInvitation(tx, organizationId, email) : null @@ -297,6 +319,8 @@ async function createOrExtendPendingInvitation( created: true, addedWorkspaceIds: workspaceIds, grants: input.grants, + mutationUpdatedAt: now, + mutationOrganizationId: organizationId, } }) } @@ -318,6 +342,8 @@ async function extendPendingInvitation( expiresAt: Date role: string membershipIntent: InvitationMembershipIntent + updatedAt: Date + organizationId: string | null } input: CreatePendingInvitationInput expiresAt: Date @@ -381,6 +407,8 @@ async function extendPendingInvitation( created: false, addedWorkspaceIds: addedGrants.map((grant) => grant.workspaceId), grants: [...existingGrants, ...addedGrants], + mutationUpdatedAt: addedGrants.length > 0 ? now : existing.updatedAt, + mutationOrganizationId: existing.organizationId, } } @@ -392,17 +420,49 @@ async function extendPendingInvitation( export async function revertPendingInvitationGrants(params: { invitationId: string workspaceIds: string[] -}): Promise { - if (params.workspaceIds.length === 0) return + expectedUpdatedAt: Date + expectedOrganizationId: string | null +}): Promise { + if (params.workspaceIds.length === 0) return false - await db - .delete(invitationWorkspaceGrant) - .where( - and( - eq(invitationWorkspaceGrant.invitationId, params.invitationId), - inArray(invitationWorkspaceGrant.workspaceId, params.workspaceIds) + return db.transaction(async (tx) => { + const locked = await lockInvitationForMutation(tx, params.invitationId) + if ( + !locked || + locked.status !== 'pending' || + locked.organizationId !== params.expectedOrganizationId || + locked.updatedAt.getTime() !== params.expectedUpdatedAt.getTime() + ) { + return false + } + + const now = new Date() + const claimed = await tx + .update(invitation) + .set({ updatedAt: now }) + .where( + and( + eq(invitation.id, params.invitationId), + eq(invitation.status, 'pending'), + eq(invitation.updatedAt, params.expectedUpdatedAt), + params.expectedOrganizationId + ? eq(invitation.organizationId, params.expectedOrganizationId) + : sql`${invitation.organizationId} IS NULL` + ) ) - ) + .returning({ id: invitation.id }) + if (claimed.length === 0) return false + + await tx + .delete(invitationWorkspaceGrant) + .where( + and( + eq(invitationWorkspaceGrant.invitationId, params.invitationId), + inArray(invitationWorkspaceGrant.workspaceId, params.workspaceIds) + ) + ) + return true + }) } async function countPendingInvitationsForOrganization(organizationId: string): Promise { @@ -444,11 +504,44 @@ export async function findPendingGrantWorkspaceIds(params: { return new Set(rows.map((row) => row.workspaceId)) } -export async function cancelPendingInvitation(invitationId: string): Promise { - await db - .update(invitation) - .set({ status: 'cancelled', updatedAt: new Date() }) - .where(and(eq(invitation.id, invitationId), eq(invitation.status, 'pending'))) +export async function cancelPendingInvitation( + invitationId: string, + guard?: { + expectedUpdatedAt: Date + expectedOrganizationId: string | null + } +): Promise { + return db.transaction(async (tx) => { + const locked = await lockInvitationForMutation(tx, invitationId) + if (!locked || locked.status !== 'pending') return false + if ( + guard && + (locked.organizationId !== guard.expectedOrganizationId || + locked.updatedAt.getTime() !== guard.expectedUpdatedAt.getTime()) + ) { + return false + } + + const cancelled = await tx + .update(invitation) + .set({ status: 'cancelled', updatedAt: new Date() }) + .where( + and( + eq(invitation.id, invitationId), + eq(invitation.status, 'pending'), + ...(guard + ? [ + eq(invitation.updatedAt, guard.expectedUpdatedAt), + guard.expectedOrganizationId + ? eq(invitation.organizationId, guard.expectedOrganizationId) + : sql`${invitation.organizationId} IS NULL`, + ] + : []) + ) + ) + .returning({ id: invitation.id }) + return cancelled.length > 0 + }) } export interface SendInvitationEmailInput { diff --git a/apps/sim/lib/invitations/workspace-invitations.test.ts b/apps/sim/lib/invitations/workspace-invitations.test.ts index 0db54229533..145b161b8f3 100644 --- a/apps/sim/lib/invitations/workspace-invitations.test.ts +++ b/apps/sim/lib/invitations/workspace-invitations.test.ts @@ -1,18 +1,29 @@ /** * @vitest-environment node */ +import { member, user as userTable } from '@sim/db/schema' import { auditMock, createMockRequest, + dbChainMock, dbChainMockFns, + queueTableRows, resetDbChainMock, resetEnvFlagsMock, setEnvFlags, } from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' +import type { DbOrTx } from '@/lib/db/types' +import type { CreatePendingInvitationInput } from '@/lib/invitations/send' const { + MockConflictingPendingInvitationError, + MockDirectGrantContextChangedError, + mockAcquireOrganizationMutationLock, + mockAcquireOrganizationUserMutationLocks, mockGetUserOrganization, + mockGetEffectiveWorkspacePermission, + mockGetWorkspaceWithOwner, mockValidateSeatAvailability, mockGrantWorkspaceAccessDirectly, mockCreatePendingInvitation, @@ -25,7 +36,13 @@ const { mockWorkspaceMemberInvited, mockCaptureServerEvent, } = vi.hoisted(() => ({ + MockConflictingPendingInvitationError: class extends Error {}, + MockDirectGrantContextChangedError: class extends Error {}, + mockAcquireOrganizationMutationLock: vi.fn(), + mockAcquireOrganizationUserMutationLocks: vi.fn(), mockGetUserOrganization: vi.fn(), + mockGetEffectiveWorkspacePermission: vi.fn(), + mockGetWorkspaceWithOwner: vi.fn(), mockValidateSeatAvailability: vi.fn(), mockGrantWorkspaceAccessDirectly: vi.fn(), mockCreatePendingInvitation: vi.fn(), @@ -42,6 +59,8 @@ const { vi.mock('@sim/audit', () => auditMock) vi.mock('@/lib/billing/organizations/membership', () => ({ + acquireOrganizationMutationLock: mockAcquireOrganizationMutationLock, + acquireOrganizationUserMutationLocks: mockAcquireOrganizationUserMutationLocks, getUserOrganization: mockGetUserOrganization, })) @@ -54,10 +73,12 @@ vi.mock('@/lib/core/telemetry', () => ({ })) vi.mock('@/lib/invitations/direct-grant', () => ({ + DirectGrantContextChangedError: MockDirectGrantContextChangedError, grantWorkspaceAccessDirectly: mockGrantWorkspaceAccessDirectly, })) vi.mock('@/lib/invitations/send', () => ({ + ConflictingPendingInvitationError: MockConflictingPendingInvitationError, createPendingInvitation: mockCreatePendingInvitation, sendInvitationEmail: mockSendInvitationEmail, cancelPendingInvitation: mockCancelPendingInvitation, @@ -70,7 +91,9 @@ vi.mock('@/lib/posthog/server', () => ({ })) vi.mock('@/lib/workspaces/permissions/utils', () => ({ - getWorkspaceWithOwner: vi.fn(), + getEffectiveWorkspacePermission: mockGetEffectiveWorkspacePermission, + getWorkspaceWithOwner: mockGetWorkspaceWithOwner, + hasWorkspaceAdminAccess: vi.fn(), })) vi.mock('@/lib/billing/core/organization', () => ({ @@ -147,6 +170,16 @@ describe('createWorkspaceInvitation', () => { /** Production default; the billing-disabled case opts out explicitly. */ setEnvFlags({ isBillingEnabled: true }) mockIsOrganizationOwnerOrAdmin.mockResolvedValue(false) + mockAcquireOrganizationMutationLock.mockResolvedValue(undefined) + mockAcquireOrganizationUserMutationLocks.mockResolvedValue(undefined) + mockGetWorkspaceWithOwner.mockResolvedValue({ + id: 'ws-1', + name: 'Workspace ws-1', + ownerId: 'user-1', + organizationId: 'org-1', + billedAccountUserId: 'user-1', + }) + mockGetEffectiveWorkspacePermission.mockResolvedValue('admin') mockGrantWorkspaceAccessDirectly.mockResolvedValue({ outcome: 'added', permission: 'write' }) mockCreatePendingInvitation.mockResolvedValue({ invitationId: 'inv-1', @@ -155,6 +188,8 @@ describe('createWorkspaceInvitation', () => { created: true, addedWorkspaceIds: ['ws-1'], grants: [{ workspaceId: 'ws-1', permission: 'write' }], + mutationUpdatedAt: new Date('2026-07-30T12:00:00.000Z'), + mutationOrganizationId: 'org-1', }) mockSendInvitationEmail.mockResolvedValue({ success: true }) mockFindPendingGrantWorkspaceIds.mockResolvedValue(new Set()) @@ -243,6 +278,45 @@ describe('createWorkspaceInvitation', () => { ) }) + it('rechecks invitee membership after locks and rejects a concurrent org join', async () => { + queueTableRows(userTable, [{ id: 'user-4', email: 'noorg@example.com' }]) + mockGetUserOrganization + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ organizationId: 'org-2', role: 'member' }) + mockCreatePendingInvitation.mockImplementationOnce( + async (input: CreatePendingInvitationInput) => { + await input.validateLockedContext?.({ + tx: dbChainMock.db as unknown as DbOrTx, + organizationId: 'org-1', + workspaceIds: ['ws-1'], + }) + throw new Error('unreachable') + } + ) + + await expect( + createWorkspaceInvitation({ + context: makeContext(), + email: 'noorg@example.com', + permission: 'write', + request, + }) + ).rejects.toMatchObject({ status: 409 }) + + expect(mockAcquireOrganizationUserMutationLocks).toHaveBeenCalledWith(expect.anything(), { + userId: 'user-4', + organizationIds: ['org-1'], + }) + expect(mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + mockGetWorkspaceWithOwner.mock.invocationCallOrder[0] + ) + expect(dbChainMockFns.for).toHaveBeenCalledTimes(2) + expect(dbChainMockFns.for.mock.invocationCallOrder[1]).toBeLessThan( + mockGetEffectiveWorkspacePermission.mock.invocationCallOrder[0] + ) + expect(dbChainMockFns.from).toHaveBeenCalledWith(member) + }) + it('creates a pending invitation for a brand-new email', async () => { queueWhereResponses([[]]) @@ -406,6 +480,8 @@ describe('createWorkspaceInvitation', () => { { workspaceId: 'ws-1', permission: 'write' }, { workspaceId: 'ws-2', permission: 'write' }, ], + mutationUpdatedAt: new Date('2026-07-30T12:00:00.000Z'), + mutationOrganizationId: 'org-1', }) const result = await createWorkspaceInvitation({ @@ -470,6 +546,8 @@ describe('createWorkspaceInvitation', () => { { workspaceId: 'ws-1', permission: 'write' }, { workspaceId: 'ws-2', permission: 'write' }, ], + mutationUpdatedAt: new Date('2026-07-30T12:00:00.000Z'), + mutationOrganizationId: 'org-1', }) mockSendInvitationEmail.mockResolvedValueOnce({ success: false, error: 'smtp down' }) @@ -486,6 +564,8 @@ describe('createWorkspaceInvitation', () => { expect(mockRevertPendingInvitationGrants).toHaveBeenCalledWith({ invitationId: 'inv-existing', workspaceIds: ['ws-2'], + expectedUpdatedAt: new Date('2026-07-30T12:00:00.000Z'), + expectedOrganizationId: 'org-1', }) }) }) diff --git a/apps/sim/lib/invitations/workspace-invitations.ts b/apps/sim/lib/invitations/workspace-invitations.ts index 59339eef5b9..ccf2cfe18cf 100644 --- a/apps/sim/lib/invitations/workspace-invitations.ts +++ b/apps/sim/lib/invitations/workspace-invitations.ts @@ -1,15 +1,22 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { db } from '@sim/db' -import { type InvitationMembershipIntent, permissions, user } from '@sim/db/schema' +import { type InvitationMembershipIntent, member, permissions, user } from '@sim/db/schema' +import { isOrgAdminRole } from '@sim/platform-authz/workspace' import { normalizeEmail } from '@sim/utils/string' import { and, eq, inArray, sql } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { isOrganizationOwnerOrAdmin } from '@/lib/billing/core/organization' -import { getUserOrganization } from '@/lib/billing/organizations/membership' +import { + acquireOrganizationMutationLock, + acquireOrganizationUserMutationLocks, + getUserOrganization, +} from '@/lib/billing/organizations/membership' import { validateSeatAvailability } from '@/lib/billing/validation/seat-management' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { PlatformEvents } from '@/lib/core/telemetry' +import type { DbOrTx } from '@/lib/db/types' import { + DirectGrantContextChangedError, type DirectGrantOutcome, grantWorkspaceAccessDirectly, } from '@/lib/invitations/direct-grant' @@ -23,6 +30,7 @@ import { } from '@/lib/invitations/send' import { captureServerEvent } from '@/lib/posthog/server' import { + getEffectiveWorkspacePermission, getWorkspaceWithOwner, hasWorkspaceAdminAccess, type PermissionType, @@ -197,6 +205,136 @@ async function inviteeCanBeExternal(userId: string | undefined): Promise { + /** + * Sending already holds the invitation/workspace advisory locks. Take the + * same organization → user → membership fence used by direct grants and + * organization transfers before re-reading any decision that could have + * gone stale. + */ + if (existingUserId) { + await acquireOrganizationUserMutationLocks(tx, { + userId: existingUserId, + organizationIds: organizationId ? [organizationId] : [], + }) + } else if (organizationId) { + await acquireOrganizationMutationLock(tx, organizationId) + } + + if (organizationId !== context.organizationId) { + throw new WorkspaceInvitationError({ + message: 'A selected workspace changed organizations. Review the selection and try again.', + status: 409, + }) + } + + if (organizationId) { + await tx + .select({ id: member.id, role: member.role }) + .from(member) + .where(and(eq(member.userId, context.inviterId), eq(member.organizationId, organizationId))) + .for('update') + } + + for (const workspaceId of [...new Set(workspaceIds)].sort()) { + const workspaceDetails = await getWorkspaceWithOwner(workspaceId, { + executor: tx, + forUpdate: true, + }) + if (!workspaceDetails || workspaceDetails.organizationId !== organizationId) { + throw new WorkspaceInvitationError({ + message: 'A selected workspace changed organizations. Review the selection and try again.', + status: 409, + }) + } + + /** + * Lock both possible authorization rows before resolving effective access. + * The explicit permission protects direct workspace admin standing; the + * exact member row protects inherited organization-admin standing from a + * concurrent role update that does not share invitation advisory locks. + */ + await tx + .select({ id: permissions.id }) + .from(permissions) + .where( + and( + eq(permissions.entityType, 'workspace'), + eq(permissions.entityId, workspaceId), + eq(permissions.userId, context.inviterId) + ) + ) + .for('update') + if ( + (await getEffectiveWorkspacePermission(context.inviterId, workspaceDetails, tx)) !== 'admin' + ) { + throw new WorkspaceInvitationError({ + message: 'Your workspace permissions changed. Review the selection and try again.', + status: 409, + }) + } + } + + if (requiresOrganizationAdmin) { + const inviterMembership = await getUserOrganization(context.inviterId, tx) + if ( + !organizationId || + inviterMembership?.organizationId !== organizationId || + !isOrgAdminRole(inviterMembership.role) + ) { + throw new WorkspaceInvitationError({ + message: 'Your organization role changed. Review the invitation and try again.', + status: 409, + }) + } + } + + if (existingUserId) { + const currentInviteeMembership = await getUserOrganization(existingUserId, tx) + if ((currentInviteeMembership?.organizationId ?? null) !== observedInviteeOrganizationId) { + throw new WorkspaceInvitationError({ + message: 'The invitee changed organizations. Review the invitation and try again.', + status: 409, + }) + } + + const [existingPermission] = await tx + .select({ id: permissions.id }) + .from(permissions) + .where( + and( + eq(permissions.entityType, 'workspace'), + eq(permissions.userId, existingUserId), + inArray(permissions.entityId, workspaceIds) + ) + ) + .for('update') + .limit(1) + if (existingPermission) { + throw new WorkspaceInvitationError({ + message: 'The invitee already gained access. Refresh and try again.', + status: 409, + }) + } + } +} + /** * Invites one person to every workspace in the context they do not already * have (or already have a pending invitation for), as a single invitation with @@ -240,8 +378,7 @@ export async function createWorkspaceInvitation({ .where(sql`lower(${user.email}) = ${normalizedEmail}`) .then((rows) => rows[0]) - const existingMembership = - existingUser && organizationId ? await getUserOrganization(existingUser.id) : null + const existingMembership = existingUser ? await getUserOrganization(existingUser.id) : null let pendingTargets = context.targets if (existingUser) { @@ -282,18 +419,31 @@ export async function createWorkspaceInvitation({ if (organizationId && existingMembership?.organizationId === organizationId) { let outcome: DirectGrantOutcome['outcome'] = 'unchanged' for (const target of pendingTargets) { - const directGrant = await grantWorkspaceAccessDirectly({ - userId: existingUser.id, - email: normalizedEmail, - workspaceId: target.workspaceId, - workspaceName: target.workspaceDetails.name, - permission: invitationPermission, - organizationId, - actorId: context.inviterId, - actorName: context.inviterName, - actorEmail: context.inviterEmail, - request, - }) + let directGrant: DirectGrantOutcome + try { + directGrant = await grantWorkspaceAccessDirectly({ + userId: existingUser.id, + email: normalizedEmail, + workspaceId: target.workspaceId, + workspaceName: target.workspaceDetails.name, + permission: invitationPermission, + organizationId, + actorId: context.inviterId, + actorName: context.inviterName, + actorEmail: context.inviterEmail, + request, + }) + } catch (error) { + if (error instanceof DirectGrantContextChangedError) { + throw new WorkspaceInvitationError({ + message: + 'Workspace access or organization membership changed. Refresh and try again.', + status: 409, + email: normalizedEmail, + }) + } + throw error + } if (directGrant.outcome === 'added') outcome = 'added' } @@ -406,6 +556,16 @@ export async function createWorkspaceInvitation({ workspaceId: target.workspaceId, permission: invitationPermission, })), + validateLockedContext: ({ tx, organizationId: lockedOrganizationId, workspaceIds }) => + validateLockedWorkspaceInvitationContext({ + tx, + context, + workspaceIds, + organizationId: lockedOrganizationId, + existingUserId: existingUser?.id, + observedInviteeOrganizationId: existingMembership?.organizationId ?? null, + requiresOrganizationAdmin: membershipIntent === 'internal' && membership === 'admin', + }), }) } catch (error) { /** @@ -471,11 +631,16 @@ export async function createWorkspaceInvitation({ if (!emailResult.success) { if (invitationRecord.created) { - await cancelPendingInvitation(invitationRecord.invitationId) + await cancelPendingInvitation(invitationRecord.invitationId, { + expectedUpdatedAt: invitationRecord.mutationUpdatedAt, + expectedOrganizationId: invitationRecord.mutationOrganizationId, + }) } else { await revertPendingInvitationGrants({ invitationId: invitationRecord.invitationId, workspaceIds: invitationRecord.addedWorkspaceIds, + expectedUpdatedAt: invitationRecord.mutationUpdatedAt, + expectedOrganizationId: invitationRecord.mutationOrganizationId, }) } throw new WorkspaceInvitationError({ diff --git a/apps/sim/lib/workspaces/admin-move.test.ts b/apps/sim/lib/workspaces/admin-move.test.ts index a9572771c9e..cd6b6ce9197 100644 --- a/apps/sim/lib/workspaces/admin-move.test.ts +++ b/apps/sim/lib/workspaces/admin-move.test.ts @@ -8,7 +8,10 @@ import type { WorkspaceMoveError } from '@/lib/workspaces/admin-move' import { buildPendingInvitationMergeScopeCondition, classifyWorkspaceMoveState, + invitationMigrationOutboxHandlers, + MIGRATED_INVITATION_EMAIL_EVENT_TYPE, moveWorkspaceToOrganization, + projectDestinationPendingSeatCount, } from '@/lib/workspaces/admin-move' import { WORKSPACE_MODE } from '@/lib/workspaces/policy' @@ -16,14 +19,22 @@ vi.unmock('drizzle-orm') const { recordAudit, - enqueueOutboxEvent, + enqueueOrReschedulePendingOutboxEvent, invalidateWorkspaceTableLimitsCache, changeWorkspaceStoragePayerInTx, + acquireInvitationMutationLocks, + getInvitationById, + isInvitationExpired, + sendInvitationEmail, } = vi.hoisted(() => ({ recordAudit: vi.fn(), - enqueueOutboxEvent: vi.fn(), + enqueueOrReschedulePendingOutboxEvent: vi.fn(), invalidateWorkspaceTableLimitsCache: vi.fn(), changeWorkspaceStoragePayerInTx: vi.fn(), + acquireInvitationMutationLocks: vi.fn(), + getInvitationById: vi.fn(), + isInvitationExpired: vi.fn(() => false), + sendInvitationEmail: vi.fn(), })) vi.mock('@sim/audit', () => ({ @@ -35,10 +46,16 @@ vi.mock('@/lib/billing/organizations/membership', () => ({ acquireOrganizationMutationLock: vi.fn(), })) vi.mock('@/lib/billing/storage/payer-transfer', () => ({ changeWorkspaceStoragePayerInTx })) -vi.mock('@/lib/core/outbox/service', () => ({ enqueueOutboxEvent })) -vi.mock('@/lib/invitations/core', () => ({ getInvitationById: vi.fn() })) -vi.mock('@/lib/invitations/locks', () => ({ acquireInvitationMutationLocks: vi.fn() })) -vi.mock('@/lib/invitations/send', () => ({ sendInvitationEmail: vi.fn() })) +vi.mock('@/lib/core/outbox/service', () => ({ enqueueOrReschedulePendingOutboxEvent })) +vi.mock('@/lib/invitations/core', () => ({ + getInvitationById, + isInvitationExpired, +})) +vi.mock('@/lib/invitations/locks', () => ({ acquireInvitationMutationLocks })) +vi.mock('@/lib/invitations/send', () => ({ + PENDING_INVITATION_UNIQUE_INDEX: 'invitation_pending_email_org_unique', + sendInvitationEmail, +})) vi.mock('@/lib/table/billing', () => ({ invalidateWorkspaceTableLimitsCache })) const movedWorkspace = { @@ -71,13 +88,12 @@ const destination = { } /** - * The move flow reads the workspace three times in order — the pre-lock - * `FOR UPDATE` select (rows ignored), the classification row, and the final - * summary reload — so the workspace queue gets one set per read. All - * invitation/grant/permission selects resolve the queue-less empty default. + * The move flow reads the workspace twice in order — the locked classification + * row and the final summary reload — so the workspace queue gets one set per + * read. All invitation/grant/permission selects resolve the queue-less empty + * default. */ function queueMoveSelects(workspaceRow: Record) { - queueTableRows(workspace, [workspaceRow]) queueTableRows(workspace, [workspaceRow]) queueTableRows(workspace, [workspaceRow]) queueTableRows(organization, [destination]) @@ -127,6 +143,23 @@ describe('classifyWorkspaceMoveState', () => { ) }) + it('rejects a drifted non-organization mode when an organization is still assigned', () => { + expect(() => + classifyWorkspaceMoveState( + { + workspaceMode: WORKSPACE_MODE.PERSONAL, + organizationId: 'org-source', + archivedAt: null, + }, + 'org-destination' + ) + ).toThrowError( + expect.objectContaining>({ + code: 'already-organization-workspace', + }) + ) + }) + it('keeps archived personal workspaces movable so they cannot dodge organization purview', () => { expect( classifyWorkspaceMoveState( @@ -140,20 +173,178 @@ describe('classifyWorkspaceMoveState', () => { describe('pending invitation destination identity', () => { it('matches by email and organization without splitting internal/external intent', () => { const dialect = new PgDialect() + const now = new Date('2026-07-30T12:00:00.000Z') const query = dialect.sqlToQuery( buildPendingInvitationMergeScopeCondition({ email: 'Invitee@Example.com', organizationId: 'org-1', excludeInvitationId: 'invite-source', + now, })! ) expect(query.sql).not.toContain('membership_intent') + expect(query.sql).toContain(' > ') expect(query.params).toContain('invitee@example.com') expect(query.params).toContain('org-1') + expect(query.params).toContain(now) expect(query.params).not.toContain('internal') expect(query.params).not.toContain('external') }) + + it('never selects an unrelated personal invitation as a merge target', () => { + expect( + buildPendingInvitationMergeScopeCondition({ + email: 'invitee@example.com', + organizationId: null, + excludeInvitationId: 'invite-source', + }) + ).toBeUndefined() + }) +}) + +describe('workspace-move pending seat projection', () => { + it('includes existing destination pending seats plus distinct incoming internal invitees', () => { + expect( + projectDestinationPendingSeatCount({ + currentDestinationPendingSeats: 1, + destinationOrganizationId: 'org-1', + movedWorkspaceInvitations: [ + { + email: 'new@example.com', + organizationId: null, + membershipIntent: 'internal', + }, + { + email: 'NEW@example.com', + organizationId: 'org-source', + membershipIntent: 'internal', + }, + { + email: 'external@example.com', + organizationId: null, + membershipIntent: 'external', + }, + ], + existingDestinationInternalEmails: [], + existingMemberEmails: [], + }) + ).toBe(2) + }) + + it('does not double-count internal invitees already pending in the destination', () => { + expect( + projectDestinationPendingSeatCount({ + currentDestinationPendingSeats: 2, + destinationOrganizationId: 'org-1', + movedWorkspaceInvitations: [ + { + email: 'already@example.com', + organizationId: null, + membershipIntent: 'internal', + }, + { + email: 'stamped@example.com', + organizationId: 'org-1', + membershipIntent: 'internal', + }, + ], + existingDestinationInternalEmails: ['ALREADY@example.com', 'stamped@example.com'], + existingMemberEmails: [], + }) + ).toBe(2) + }) + + it('counts an incoming internal invite when the destination invite is only external', () => { + expect( + projectDestinationPendingSeatCount({ + currentDestinationPendingSeats: 0, + destinationOrganizationId: 'org-1', + movedWorkspaceInvitations: [ + { + email: 'upgrade@example.com', + organizationId: null, + membershipIntent: 'internal', + }, + ], + // External destination invitations are deliberately absent from this + // set because migration promotes their intent to internal. + existingDestinationInternalEmails: [], + existingMemberEmails: [], + }) + ).toBe(1) + }) + + it('does not count an incoming internal invitee who belongs to another organization', () => { + expect( + projectDestinationPendingSeatCount({ + currentDestinationPendingSeats: 1, + destinationOrganizationId: 'org-1', + movedWorkspaceInvitations: [ + { + email: 'member@example.com', + organizationId: null, + membershipIntent: 'internal', + }, + ], + existingDestinationInternalEmails: [], + existingMemberEmails: ['MEMBER@example.com'], + }) + ).toBe(1) + }) +}) + +describe('migrated invitation email outbox', () => { + it('re-reads the surviving invitation and sends its final grants', async () => { + getInvitationById.mockResolvedValue({ + id: 'invite-surviving', + status: 'pending', + token: 'final-token', + kind: 'workspace', + email: 'invitee@example.com', + inviterName: 'Workspace Admin', + inviterEmail: 'admin@example.com', + organizationId: 'org-1', + role: 'member', + expiresAt: new Date(Date.now() + 60_000), + grants: [ + { workspaceId: 'workspace-1', permission: 'write' }, + { workspaceId: 'workspace-2', permission: 'read' }, + ], + }) + isInvitationExpired.mockReturnValue(false) + sendInvitationEmail.mockResolvedValue({ success: true }) + + await invitationMigrationOutboxHandlers[MIGRATED_INVITATION_EMAIL_EVENT_TYPE]( + { invitationId: 'invite-surviving' }, + {} as never + ) + + expect(sendInvitationEmail).toHaveBeenCalledWith( + expect.objectContaining({ + invitationId: 'invite-surviving', + token: 'final-token', + grants: [ + { workspaceId: 'workspace-1', permission: 'write' }, + { workspaceId: 'workspace-2', permission: 'read' }, + ], + }) + ) + }) + + it('skips a split token that was cancelled before the settle window elapsed', async () => { + getInvitationById.mockResolvedValue({ + id: 'invite-transient', + status: 'cancelled', + }) + + await invitationMigrationOutboxHandlers[MIGRATED_INVITATION_EMAIL_EVENT_TYPE]( + { invitationId: 'invite-transient' }, + {} as never + ) + + expect(sendInvitationEmail).not.toHaveBeenCalled() + }) }) describe('moveWorkspaceToOrganization retries', () => { @@ -171,7 +362,7 @@ describe('moveWorkspaceToOrganization retries', () => { organizationId: destination.id, workspaceMode: WORKSPACE_MODE.ORGANIZATION, }) - expect(enqueueOutboxEvent).not.toHaveBeenCalled() + expect(enqueueOrReschedulePendingOutboxEvent).not.toHaveBeenCalled() expect(recordAudit).not.toHaveBeenCalled() expect(invalidateWorkspaceTableLimitsCache).not.toHaveBeenCalled() expect(dbChainMockFns.insert).not.toHaveBeenCalled() @@ -179,7 +370,7 @@ describe('moveWorkspaceToOrganization retries', () => { expect(changeWorkspaceStoragePayerInTx).not.toHaveBeenCalled() }) - it('pre-locks a nonzero workspace before changing its storage payer', async () => { + it('takes shared advisory locks before the workspace row lock and payer mutation', async () => { queueMoveSelects(personalWorkspace) await moveWorkspaceToOrganization({ @@ -188,12 +379,30 @@ describe('moveWorkspaceToOrganization retries', () => { adminEmail: 'admin@sim.ai', }) - // The first `.for('update')` in the move path is the workspace pre-lock - // select (the earlier invitation-scan selects carry no row lock), so its - // invocation order against the payer mutation proves lock-before-payer. + const advisoryLock = acquireInvitationMutationLocks.mock.invocationCallOrder[0] const firstForUpdate = dbChainMockFns.for.mock.invocationCallOrder[0] const payerMutation = changeWorkspaceStoragePayerInTx.mock.invocationCallOrder[0] + expect(advisoryLock).toBeGreaterThan(0) + expect(firstForUpdate).toBeGreaterThan(advisoryLock) expect(firstForUpdate).toBeGreaterThan(0) expect(payerMutation).toBeGreaterThan(firstForUpdate) }) + + it('rejects a stale batch selection when workspace ownership changed', async () => { + queueMoveSelects({ ...personalWorkspace, ownerId: 'new-owner' }) + + await expect( + moveWorkspaceToOrganization({ + workspaceId: personalWorkspace.id, + destinationOrganizationId: destination.id, + adminEmail: 'admin@sim.ai', + expectedOwnerId: personalWorkspace.ownerId, + }) + ).rejects.toMatchObject>({ + code: 'workspace-owner-changed', + }) + + expect(changeWorkspaceStoragePayerInTx).not.toHaveBeenCalled() + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/workspaces/admin-move.ts b/apps/sim/lib/workspaces/admin-move.ts index ef6fcf160c3..bd771894502 100644 --- a/apps/sim/lib/workspaces/admin-move.ts +++ b/apps/sim/lib/workspaces/admin-move.ts @@ -12,21 +12,43 @@ import { } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { PERMISSION_RANK, type PermissionType } from '@sim/platform-authz/workspace' +import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { normalizeEmail } from '@sim/utils/string' -import { and, asc, count, eq, ilike, inArray, isNull, ne, or, sql } from 'drizzle-orm' +import { + and, + asc, + count, + eq, + gt, + ilike, + inArray, + isNotNull, + isNull, + lte, + ne, + or, + sql, +} from 'drizzle-orm' import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/membership' -import { isEnterprise } from '@/lib/billing/plan-helpers' import { changeWorkspaceStoragePayerInTx } from '@/lib/billing/storage/payer-transfer' import { ENTITLED_SUBSCRIPTION_STATUSES, hasPaidSubscriptionStatus, } from '@/lib/billing/subscriptions/utils' -import { enqueueOutboxEvent, type OutboxHandler } from '@/lib/core/outbox/service' +import { + countPendingSeatInvitations, + planHasFixedSeatCap, + resolveSeatCapacity, +} from '@/lib/billing/validation/seat-management' +import { + enqueueOrReschedulePendingOutboxEvent, + type OutboxHandler, +} from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' -import { getInvitationById } from '@/lib/invitations/core' +import { getInvitationById, isInvitationExpired } from '@/lib/invitations/core' import { acquireInvitationMutationLocks } from '@/lib/invitations/locks' -import { sendInvitationEmail } from '@/lib/invitations/send' +import { PENDING_INVITATION_UNIQUE_INDEX, sendInvitationEmail } from '@/lib/invitations/send' import { invalidateWorkspaceTableLimitsCache } from '@/lib/table/billing' import { mergeInvitationMembershipIntent, @@ -36,6 +58,10 @@ import { import { WORKSPACE_MODE } from '@/lib/workspaces/policy' const logger = createLogger('AdminWorkspaceMove') +// A dashboard member add may move several grants from one invitation in +// consecutive short transactions. Let that split/merge sequence settle before +// the outbox resolves the live invitation and sends its final token. +const MIGRATED_INVITATION_EMAIL_SETTLE_MS = 60_000 export class WorkspaceMoveError extends Error { constructor( @@ -43,6 +69,7 @@ export class WorkspaceMoveError extends Error { readonly code: | 'workspace-not-found' | 'organization-not-found' + | 'workspace-owner-changed' | 'already-organization-workspace' ) { super(message) @@ -95,6 +122,15 @@ interface InvitationMigrationEvent { relatedInvitationId?: string } +interface PendingWorkspaceInvitationSummary { + id: string + email: string + organizationId: string | null + membershipIntent: 'internal' | 'external' + permission: 'admin' | 'write' | 'read' + workspaceGrantCount: number +} + interface WorkspaceMoveDestination { id: string name: string @@ -121,6 +157,13 @@ class InvitationSetChangedError extends Error { } } +function isConcurrentPendingInvitationInsert(error: unknown): boolean { + return ( + getPostgresErrorCode(error) === '23505' && + getPostgresConstraintName(error) === PENDING_INVITATION_UNIQUE_INDEX + ) +} + /** Returns movable personal/grandfathered workspaces by case-insensitive name or exact UUID. */ export async function searchWorkspaceMoveCandidates( search: string, @@ -146,6 +189,7 @@ export async function searchWorkspaceMoveCandidates( .where( and( ne(workspace.workspaceMode, WORKSPACE_MODE.ORGANIZATION), + isNull(workspace.organizationId), or(eq(workspace.id, query), ilike(workspace.name, `%${query}%`)) ) ) @@ -199,6 +243,7 @@ export async function getWorkspaceMovePreflight( .where(eq(member.organizationId, destinationOrganizationId)), db .select({ + id: subscription.id, plan: subscription.plan, status: subscription.status, metadata: subscription.metadata, @@ -213,14 +258,24 @@ export async function getWorkspaceMovePreflight( .limit(1), ]) - const pendingInternalCount = invitationRows.filter( - (row) => row.membershipIntent === 'internal' - ).length - const seatCapacity = getEnterpriseSeatCapacity(subscriptionRows[0]) + const organizationSubscription = subscriptionRows[0] + const seatCapacity = + organizationSubscription && + hasPaidSubscriptionStatus(organizationSubscription.status) && + planHasFixedSeatCap(organizationSubscription.plan) + ? await resolveSeatCapacity(organizationSubscription) + : null const currentMembers = memberCountRows[0]?.value ?? 0 + const projectedPendingInternalSeats = + seatCapacity === null + ? 0 + : await getProjectedDestinationPendingSeatCount({ + destinationOrganizationId, + movedWorkspaceInvitations: invitationRows, + }) const warning = - seatCapacity !== null && currentMembers + pendingInternalCount > seatCapacity - ? `${pendingInternalCount} pending internal invitation${pendingInternalCount === 1 ? '' : 's'} could exceed the ${seatCapacity}-seat Enterprise capacity when accepted.` + seatCapacity !== null && currentMembers + projectedPendingInternalSeats > seatCapacity + ? `${currentMembers} current member${currentMembers === 1 ? '' : 's'} plus ${projectedPendingInternalSeats} pending internal invitation${projectedPendingInternalSeats === 1 ? '' : 's'} would exceed the ${seatCapacity}-seat Enterprise capacity if all are accepted.` : null return { @@ -233,21 +288,23 @@ export async function getWorkspaceMovePreflight( permission: row.permission, organizationMember: row.memberId !== null, })), - invitations: invitationRows, + invitations: invitationRows.map(({ organizationId: _organizationId, ...row }) => row), warning, } } /** - * Moves one workspace and migrates every pending grant without changing - * ownership, historical usage, credentials, storage attribution, or existing - * collaborator permissions. + * Moves one workspace and migrates every pending grant. Workspace ownership, + * historical usage, credentials, and collaborator permissions are preserved; + * the current billing/storage payer changes to the destination organization. */ export async function moveWorkspaceToOrganization(params: { workspaceId: string destinationOrganizationId: string adminEmail: string -}): Promise { + /** Reject a stale batch selection instead of moving a newly owned workspace. */ + expectedOwnerId?: string +}): Promise { let candidateInvitationIds = await findInvitationMigrationLockIds( params.workspaceId, params.destinationOrganizationId @@ -257,16 +314,10 @@ export async function moveWorkspaceToOrganization(params: { for (let attempt = 0; attempt < 5; attempt += 1) { try { result = await db.transaction(async (tx) => { - await tx - .select({ id: workspace.id }) - .from(workspace) - .where(eq(workspace.id, params.workspaceId)) - .orderBy(asc(workspace.id)) - .for('update') - - // Acceptance takes invitation/workspace locks before the organization - // lock. Use the identical order here so a concurrent accept and move - // cannot wait on one another in opposite directions. + // Acceptance takes invitation/workspace advisory locks before it + // row-locks the workspace. Keep the exact same order here: taking the + // row lock first can deadlock when acceptance owns an invitation lock, + // waits for the workspace row, and this move waits for that invitation. await acquireInvitationMutationLocks(tx, { invitationIds: candidateInvitationIds, workspaceIds: [params.workspaceId], @@ -299,6 +350,12 @@ export async function moveWorkspaceToOrganization(params: { if (!workspaceRow) { throw new WorkspaceMoveError('Workspace not found', 'workspace-not-found') } + if (params.expectedOwnerId && workspaceRow.ownerId !== params.expectedOwnerId) { + throw new WorkspaceMoveError( + 'Workspace owner changed after it was selected', + 'workspace-owner-changed' + ) + } const moveState = classifyWorkspaceMoveState(workspaceRow, params.destinationOrganizationId) const destination = await getDestinationOrganization(params.destinationOrganizationId, tx) @@ -320,8 +377,9 @@ export async function moveWorkspaceToOrganization(params: { } satisfies MoveTransactionResult } - const lockedInvitationIds = await lockCurrentPendingInvitations(tx, params.workspaceId) const now = new Date() + await expireLockedPendingInvitations(tx, candidateInvitationIds, now) + const lockedInvitationIds = await lockCurrentPendingInvitations(tx, params.workspaceId, now) const migration = await migratePendingInvitations(tx, { workspaceId: params.workspaceId, destinationOrganizationId: params.destinationOrganizationId, @@ -329,7 +387,15 @@ export async function moveWorkspaceToOrganization(params: { now, }) for (const invitationId of migration.invitationsToEmail) { - await enqueueOutboxEvent(tx, MIGRATED_INVITATION_EMAIL_EVENT_TYPE, { invitationId }) + await enqueueOrReschedulePendingOutboxEvent( + tx, + MIGRATED_INVITATION_EMAIL_EVENT_TYPE, + { invitationId }, + { + availableAt: new Date(now.getTime() + MIGRATED_INVITATION_EMAIL_SETTLE_MS), + coalesceOn: { payloadKey: 'invitationId', payloadValue: invitationId }, + } + ) } await changeWorkspaceStoragePayerInTx(tx, { @@ -378,8 +444,18 @@ export async function moveWorkspaceToOrganization(params: { }) break } catch (error) { - if (!(error instanceof InvitationSetChangedError)) throw error - candidateInvitationIds = error.invitationIds + if (error instanceof InvitationSetChangedError) { + candidateInvitationIds = error.invitationIds + continue + } + if (isConcurrentPendingInvitationInsert(error)) { + candidateInvitationIds = await findInvitationMigrationLockIds( + params.workspaceId, + params.destinationOrganizationId + ) + continue + } + throw error } } @@ -387,13 +463,12 @@ export async function moveWorkspaceToOrganization(params: { throw new Error('Pending invitations kept changing; retry the workspace move') } - const invitationEmailFailures: string[] = [] if (!result.performedMove) { logger.info('Workspace was already in destination organization', { workspaceId: params.workspaceId, destinationOrganizationId: params.destinationOrganizationId, }) - return { ...result.summary, invitationEmailFailures } + return result.summary } invalidateWorkspaceTableLimitsCache(params.workspaceId) @@ -437,10 +512,9 @@ export async function moveWorkspaceToOrganization(params: { workspaceId: params.workspaceId, destinationOrganizationId: params.destinationOrganizationId, invitationEvents: result.invitationEvents.length, - invitationEmailFailures: invitationEmailFailures.length, }) - return { ...result.summary, invitationEmailFailures } + return result.summary } async function searchWorkspaceById(workspaceId: string): Promise { @@ -469,8 +543,15 @@ async function searchWorkspaceById(workspaceId: string): Promise).seats - const parsed = typeof seats === 'number' ? seats : Number(seats) - return Number.isInteger(parsed) && parsed > 0 ? parsed : null +/** + * Projects the destination's live pending-seat count after this workspace's + * invitation grants are migrated. + * + * The canonical count already includes every live internal invitation stamped + * with the destination. The only additions are distinct internal invitees from + * another scope that are neither current members of any organization nor + * already represented by a destination-internal invitation. Multiple personal + * invitations for one email collapse into one destination invitation during + * migration, so the delta is email-distinct. + */ +export function projectDestinationPendingSeatCount(params: { + currentDestinationPendingSeats: number + destinationOrganizationId: string + movedWorkspaceInvitations: Array<{ + email: string + organizationId: string | null + membershipIntent: 'internal' | 'external' + }> + existingDestinationInternalEmails: string[] + existingMemberEmails: string[] +}): number { + const existingDestinationSeatEmails = new Set( + [...params.existingDestinationInternalEmails, ...params.existingMemberEmails].map( + normalizeEmail + ) + ) + const incomingInternalEmails = new Set( + params.movedWorkspaceInvitations + .filter( + (row) => + row.membershipIntent === 'internal' && + row.organizationId !== params.destinationOrganizationId + ) + .map((row) => normalizeEmail(row.email)) + ) + const incomingSeatDelta = [...incomingInternalEmails].filter( + (email) => !existingDestinationSeatEmails.has(email) + ).length + return params.currentDestinationPendingSeats + incomingSeatDelta +} + +async function getProjectedDestinationPendingSeatCount(params: { + destinationOrganizationId: string + movedWorkspaceInvitations: PendingWorkspaceInvitationSummary[] +}): Promise { + const currentDestinationPendingSeats = await countPendingSeatInvitations( + params.destinationOrganizationId + ) + const incomingInternalEmails = [ + ...new Set( + params.movedWorkspaceInvitations + .filter( + (row) => + row.membershipIntent === 'internal' && + row.organizationId !== params.destinationOrganizationId + ) + .map((row) => normalizeEmail(row.email)) + ), + ] + if (incomingInternalEmails.length === 0) return currentDestinationPendingSeats + + const [existingDestinationRows, existingMembers] = await Promise.all([ + db + .select({ email: invitation.email }) + .from(invitation) + .where( + and( + eq(invitation.organizationId, params.destinationOrganizationId), + eq(invitation.status, 'pending'), + eq(invitation.membershipIntent, 'internal'), + gt(invitation.expiresAt, new Date()), + or( + ...incomingInternalEmails.map( + (email) => sql`lower(${invitation.email}) = ${normalizeEmail(email)}` + ) + ) + ) + ), + db + .select({ email: user.email }) + .from(member) + .innerJoin(user, eq(user.id, member.userId)) + .where( + or( + ...incomingInternalEmails.map( + (email) => sql`lower(btrim(${user.email})) = ${normalizeEmail(email)}` + ) + ) + ), + ]) + + return projectDestinationPendingSeatCount({ + currentDestinationPendingSeats, + destinationOrganizationId: params.destinationOrganizationId, + movedWorkspaceInvitations: params.movedWorkspaceInvitations, + existingDestinationInternalEmails: existingDestinationRows.map((row) => row.email), + existingMemberEmails: existingMembers.map((row) => row.email), + }) } /** @@ -570,12 +743,17 @@ async function findInvitationMigrationLockIds( _destinationOrganizationId: string, executor: DbOrTx = db ): Promise { + const now = new Date() const sourceRows = await executor .select({ id: invitation.id, email: invitation.email }) .from(invitation) .innerJoin(invitationWorkspaceGrant, eq(invitationWorkspaceGrant.invitationId, invitation.id)) .where( - and(eq(invitation.status, 'pending'), eq(invitationWorkspaceGrant.workspaceId, workspaceId)) + and( + eq(invitation.status, 'pending'), + gt(invitation.expiresAt, now), + eq(invitationWorkspaceGrant.workspaceId, workspaceId) + ) ) if (sourceRows.length === 0) return [] @@ -586,7 +764,10 @@ async function findInvitationMigrationLockIds( .where( and( eq(invitation.status, 'pending'), - or(...emails.map((email) => sql`lower(${invitation.email}) = ${email}`)) + or(...emails.map((email) => sql`lower(${invitation.email}) = ${email}`)), + // Null-org invitations never coalesce, so unrelated personal invites + // for the same email are not mutation targets and need no lock. + isNotNull(invitation.organizationId) ) ) return [ @@ -594,13 +775,39 @@ async function findInvitationMigrationLockIds( ].sort() } -async function lockCurrentPendingInvitations(tx: DbOrTx, workspaceId: string): Promise { +async function expireLockedPendingInvitations( + tx: DbOrTx, + invitationIds: string[], + now: Date +): Promise { + if (invitationIds.length === 0) return + await tx + .update(invitation) + .set({ status: 'expired', updatedAt: now }) + .where( + and( + inArray(invitation.id, invitationIds), + eq(invitation.status, 'pending'), + lte(invitation.expiresAt, now) + ) + ) +} + +async function lockCurrentPendingInvitations( + tx: DbOrTx, + workspaceId: string, + now: Date +): Promise { const rows = await tx .select({ id: invitation.id }) .from(invitation) .innerJoin(invitationWorkspaceGrant, eq(invitationWorkspaceGrant.invitationId, invitation.id)) .where( - and(eq(invitation.status, 'pending'), eq(invitationWorkspaceGrant.workspaceId, workspaceId)) + and( + eq(invitation.status, 'pending'), + gt(invitation.expiresAt, now), + eq(invitationWorkspaceGrant.workspaceId, workspaceId) + ) ) .orderBy(invitation.id) .for('update') @@ -623,7 +830,13 @@ async function migratePendingInvitations( const [source] = await tx .select() .from(invitation) - .where(and(eq(invitation.id, invitationId), eq(invitation.status, 'pending'))) + .where( + and( + eq(invitation.id, invitationId), + eq(invitation.status, 'pending'), + gt(invitation.expiresAt, params.now) + ) + ) .limit(1) if (!source) continue @@ -643,6 +856,7 @@ async function migratePendingInvitations( email: source.email, organizationId: params.destinationOrganizationId, excludeInvitationId: source.id, + now: params.now, }) const partition = partitionInvitationGrantsForWorkspaceMove({ grants, @@ -682,6 +896,7 @@ async function migratePendingInvitations( email: source.email, organizationId, excludeInvitationId: source.id, + now: params.now, }) const siblingId = sibling?.id ?? @@ -738,11 +953,23 @@ async function findPendingInvitationForScope( email: string organizationId: string | null excludeInvitationId: string + now: Date } ) { + /** + * Personal invitations are scoped to the inviter/billing owner, not merely to + * the email address. There is deliberately no uniqueness constraint for + * `organization_id IS NULL`, and send.ts never coalesces those invitations. + * Redistribution must preserve the same rule: create a sibling derived from + * this source rather than absorbing an unrelated inviter's personal invite. + */ + if (!params.organizationId) return null + // Membership intent is deliberately not part of destination identity. A // same-email invite for the same organization is one pending claim; merging // promotes internal intent and the strongest role via mergeInvitationIntent. + const condition = buildPendingInvitationMergeScopeCondition(params) + if (!condition) return null const [row] = await tx .select({ id: invitation.id, @@ -750,8 +977,9 @@ async function findPendingInvitationForScope( role: invitation.role, }) .from(invitation) - .where(buildPendingInvitationMergeScopeCondition(params)) + .where(condition) .orderBy(invitation.createdAt) + .for('update') .limit(1) return row ?? null } @@ -760,15 +988,15 @@ export function buildPendingInvitationMergeScopeCondition(params: { email: string organizationId: string | null excludeInvitationId: string + now?: Date }) { - const scope = params.organizationId - ? eq(invitation.organizationId, params.organizationId) - : isNull(invitation.organizationId) + if (!params.organizationId) return undefined return and( sql`lower(${invitation.email}) = ${normalizeEmail(params.email)}`, eq(invitation.status, 'pending'), + gt(invitation.expiresAt, params.now ?? new Date()), ne(invitation.id, params.excludeInvitationId), - scope + eq(invitation.organizationId, params.organizationId) ) } @@ -823,6 +1051,14 @@ async function mergeGrant( grant: { workspaceId: string; permission: 'admin' | 'write' | 'read' }, now: Date ): Promise { + // A surviving merge target may have an invitation email in flight. Touch the + // invitation even when this grant is already present so any stale failed-send + // compensation sees a later migration revision and leaves the target intact. + await tx + .update(invitation) + .set({ updatedAt: now }) + .where(and(eq(invitation.id, invitationId), eq(invitation.status, 'pending'))) + const [existing] = await tx .select({ id: invitationWorkspaceGrant.id, permission: invitationWorkspaceGrant.permission }) .from(invitationWorkspaceGrant) @@ -859,7 +1095,7 @@ async function mergeGrant( const sendMigratedInvitationLink: OutboxHandler<{ invitationId: string }> = async (payload) => { const migrated = await getInvitationById(payload.invitationId) - if (!migrated || migrated.status !== 'pending') return + if (!migrated || migrated.status !== 'pending' || isInvitationExpired(migrated)) return const result = await sendInvitationEmail({ invitationId: migrated.id, token: migrated.token, @@ -938,7 +1174,9 @@ async function getMovedWorkspaceSummary( permission: row.permission, organizationMember: row.memberId !== null, })), - invitations: await getPendingInvitationSummaries(workspaceId, executor), + invitations: (await getPendingInvitationSummaries(workspaceId, executor)).map( + ({ organizationId: _organizationId, ...row }) => row + ), warning: null, } } diff --git a/apps/sim/lib/workspaces/organization-workspaces.test.ts b/apps/sim/lib/workspaces/organization-workspaces.test.ts index 43b5fe564ef..5054af2f3a2 100644 --- a/apps/sim/lib/workspaces/organization-workspaces.test.ts +++ b/apps/sim/lib/workspaces/organization-workspaces.test.ts @@ -11,6 +11,7 @@ const { mockAcquireOrganizationMutationLock, mockAcquireInvitationMutationLocks, mockChangeWorkspaceStoragePayersInTx, + mockInvalidateWorkspaceTableLimitsCache, } = vi.hoisted(() => ({ mockEnsureUserInOrganizationTx: vi.fn(), mockSyncUsageLimitsFromSubscription: vi.fn(), @@ -18,6 +19,7 @@ const { mockAcquireOrganizationMutationLock: vi.fn(), mockAcquireInvitationMutationLocks: vi.fn(), mockChangeWorkspaceStoragePayersInTx: vi.fn(), + mockInvalidateWorkspaceTableLimitsCache: vi.fn(), })) vi.mock('@/lib/billing/organizations/membership', () => ({ @@ -34,6 +36,10 @@ vi.mock('@/lib/invitations/locks', () => ({ acquireInvitationMutationLocks: mockAcquireInvitationMutationLocks, })) +vi.mock('@/lib/table/billing', () => ({ + invalidateWorkspaceTableLimitsCache: mockInvalidateWorkspaceTableLimitsCache, +})) + vi.mock('@/lib/billing/core/usage', () => ({ syncUsageLimitsFromSubscription: mockSyncUsageLimitsFromSubscription, })) @@ -122,10 +128,17 @@ describe('organization workspace helpers', () => { 'owner-1', 'org-1' ) + expect(mockAcquireInvitationMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + mockAcquireOrganizationMutationLock.mock.invocationCallOrder[0] + ) + expect(mockAcquireOrganizationMutationLock.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.for.mock.invocationCallOrder[0] + ) expect(dbChainMockFns.set).toHaveBeenCalledWith( expect.objectContaining({ organizationAssignedAt: expect.any(Date) }) ) expect(mockChangeWorkspaceStoragePayersInTx).toHaveBeenCalledTimes(1) + expect(mockInvalidateWorkspaceTableLimitsCache).toHaveBeenCalledTimes(2) expect(dbChainMockFns.for.mock.invocationCallOrder[0]).toBeLessThan( mockEnsureUserInOrganizationTx.mock.invocationCallOrder[0] ) diff --git a/apps/sim/lib/workspaces/organization-workspaces.ts b/apps/sim/lib/workspaces/organization-workspaces.ts index c6ab1fac739..49c261f0d5a 100644 --- a/apps/sim/lib/workspaces/organization-workspaces.ts +++ b/apps/sim/lib/workspaces/organization-workspaces.ts @@ -13,6 +13,7 @@ import { import { changeWorkspaceStoragePayersInTx } from '@/lib/billing/storage/payer-transfer' import type { DbOrTx } from '@/lib/db/types' import { acquireInvitationMutationLocks } from '@/lib/invitations/locks' +import { invalidateWorkspaceTableLimitsCache } from '@/lib/table/billing' import { getOrganizationOwnerId, WORKSPACE_MODE } from '@/lib/workspaces/policy' const logger = createLogger('OrganizationWorkspaces') @@ -93,10 +94,7 @@ export function ownedAttachableWorkspacesWhere({ ) } -/** - * Locks workspace rows before any membership billing path can lock user or - * organization payer rows. - */ +/** Locks workspace rows before any payer or membership mutation. */ async function lockWorkspaceRowsForPayerChanges(tx: DbOrTx, workspaceIds: string[]): Promise { if (workspaceIds.length === 0) return await tx @@ -134,15 +132,18 @@ export async function attachOwnedWorkspacesToOrganization({ } const attached = await db.transaction(async (tx) => { - await lockWorkspaceRowsForPayerChanges(tx, ownedWorkspaceIds) - // Match admin move and invitation acceptance: workspace/invitation scope - // first, then organization. Membership and assignment now commit or roll - // back together, so a concurrent move cannot leave stray org members. + // Shared advisory scope first, then the organization lock, then workspace + // rows — the order admin move uses. What makes it mandatory is acceptance: + // it holds `workspace-invitations:` while waiting for the workspace + // row, so row-locking first here (as this did) deadlocks against it. + // Taking the organization lock before the rows also matches ownership + // transfer, so those two cannot invert on the org/row pair either. await acquireInvitationMutationLocks(tx, { invitationIds: [], workspaceIds: ownedWorkspaceIds, }) await acquireOrganizationMutationLock(tx, organizationId) + await lockWorkspaceRowsForPayerChanges(tx, ownedWorkspaceIds) return attachOwnedWorkspacesToOrganizationTx(tx, { ownerUserId, organizationId, @@ -153,6 +154,9 @@ export async function attachOwnedWorkspacesToOrganization({ }) }) + for (const workspaceId of attached.attachedWorkspaceIds) { + invalidateWorkspaceTableLimitsCache(workspaceId) + } for (const userId of attached.usageLimitUserIds) { try { await syncUsageLimitsFromSubscription(userId) @@ -417,7 +421,7 @@ export async function detachOrganizationWorkspacesTx( tx: DbOrTx, organizationId: string ): Promise { - const organizationOwnerId = await getOrganizationOwnerId(organizationId) + const organizationOwnerId = await getOrganizationOwnerId(organizationId, tx) if (!organizationOwnerId) { logger.warn( 'Detaching workspaces from an organization without an owner; using workspace owner as billed account', diff --git a/apps/sim/lib/workspaces/permissions/utils.ts b/apps/sim/lib/workspaces/permissions/utils.ts index 6afc4f66554..8832e56641e 100644 --- a/apps/sim/lib/workspaces/permissions/utils.ts +++ b/apps/sim/lib/workspaces/permissions/utils.ts @@ -123,9 +123,10 @@ export async function getWorkspaceWithOwner( */ export async function getEffectiveWorkspacePermission( userId: string, - ws: Pick + ws: Pick, + executor: DbOrTx = db ): Promise { - return resolveEffectiveWorkspacePermission(userId, ws.id, ws.organizationId) + return resolveEffectiveWorkspacePermission(userId, ws.id, ws.organizationId, executor) } /** diff --git a/apps/sim/lib/workspaces/policy.test.ts b/apps/sim/lib/workspaces/policy.test.ts index 82918d27bd2..1c30e6c1d1d 100644 --- a/apps/sim/lib/workspaces/policy.test.ts +++ b/apps/sim/lib/workspaces/policy.test.ts @@ -2,20 +2,30 @@ * @vitest-environment node */ import { member, workspace } from '@sim/db/schema' -import { queueTableRows, resetDbChainMock, resetEnvFlagsMock, setEnvFlags } from '@sim/testing' +import { + dbChainMock, + queueTableRows, + resetDbChainMock, + resetEnvFlagsMock, + setEnvFlags, +} from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' +import type { DbOrTx } from '@/lib/db/types' const { + mockAcquireOrganizationUserMutationLocks, mockGetUserOrganization, mockGetOrganizationSubscription, mockGetHighestPrioritySubscription, } = vi.hoisted(() => ({ + mockAcquireOrganizationUserMutationLocks: vi.fn(), mockGetUserOrganization: vi.fn(), mockGetOrganizationSubscription: vi.fn(), mockGetHighestPrioritySubscription: vi.fn(), })) vi.mock('@/lib/billing/organizations/membership', () => ({ + acquireOrganizationUserMutationLocks: mockAcquireOrganizationUserMutationLocks, getUserOrganization: mockGetUserOrganization, })) @@ -28,9 +38,12 @@ vi.mock('@/lib/billing/core/plan', () => ({ })) import { + getOrganizationOwnerId, getWorkspaceCreationPolicy, getWorkspaceInvitePolicy, + lockWorkspaceCreationContext, WORKSPACE_MODE, + WorkspaceCreationContextChangedError, } from '@/lib/workspaces/policy' import { UPGRADE_TO_INVITE_REASON } from '@/lib/workspaces/policy-constants' @@ -38,6 +51,105 @@ afterAll(resetDbChainMock) afterAll(resetEnvFlagsMock) +describe('getOrganizationOwnerId', () => { + it('uses the supplied transaction executor for the owner lookup', async () => { + const limit = vi.fn().mockResolvedValue([{ userId: 'owner-from-transaction' }]) + const where = vi.fn().mockReturnValue({ limit }) + const from = vi.fn().mockReturnValue({ where }) + const select = vi.fn().mockReturnValue({ from }) + const executor = { select } as unknown as DbOrTx + + await expect(getOrganizationOwnerId('org-1', executor)).resolves.toBe('owner-from-transaction') + expect(select).toHaveBeenCalledWith({ userId: member.userId }) + expect(from).toHaveBeenCalledWith(member) + expect(where).toHaveBeenCalledOnce() + expect(limit).toHaveBeenCalledWith(1) + }) +}) + +describe('lockWorkspaceCreationContext', () => { + it('locks the destination organization and user before rejecting a stale org-mode policy', async () => { + vi.clearAllMocks() + mockAcquireOrganizationUserMutationLocks.mockResolvedValue(undefined) + mockGetUserOrganization.mockResolvedValue(null) + const tx = {} as DbOrTx + + await expect( + lockWorkspaceCreationContext(tx, { + userId: 'user-1', + organizationId: 'org-1', + observedOrganizationId: 'org-1', + }) + ).rejects.toBeInstanceOf(WorkspaceCreationContextChangedError) + + expect(mockAcquireOrganizationUserMutationLocks).toHaveBeenCalledWith(tx, { + userId: 'user-1', + organizationIds: ['org-1'], + }) + expect(mockGetUserOrganization).toHaveBeenCalledWith('user-1', tx) + expect(mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + mockGetUserOrganization.mock.invocationCallOrder[0] + ) + }) + + it('uses the live owner after the org lock and row-locks the current entitlement', async () => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isBillingEnabled: true }) + mockAcquireOrganizationUserMutationLocks.mockResolvedValue(undefined) + mockGetUserOrganization.mockResolvedValue({ + organizationId: 'org-1', + role: 'admin', + }) + mockGetOrganizationSubscription.mockResolvedValue({ + id: 'sub-1', + referenceId: 'org-1', + plan: 'team_6000', + status: 'active', + }) + queueTableRows(member, [{ userId: 'new-owner' }]) + const tx = dbChainMock.db as unknown as DbOrTx + + await expect( + lockWorkspaceCreationContext(tx, { + userId: 'creator-1', + organizationId: 'org-1', + observedOrganizationId: 'org-1', + }) + ).resolves.toEqual({ billedAccountUserId: 'new-owner' }) + + expect(mockGetOrganizationSubscription).toHaveBeenCalledWith('org-1', { + executor: tx, + onError: 'throw', + forUpdate: true, + }) + expect(mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + mockGetOrganizationSubscription.mock.invocationCallOrder[0] + ) + }) + + it('rejects when the paid org entitlement disappeared before insertion', async () => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isBillingEnabled: true }) + mockAcquireOrganizationUserMutationLocks.mockResolvedValue(undefined) + mockGetUserOrganization.mockResolvedValue({ + organizationId: 'org-1', + role: 'owner', + }) + mockGetOrganizationSubscription.mockResolvedValue(null) + const tx = dbChainMock.db as unknown as DbOrTx + + await expect( + lockWorkspaceCreationContext(tx, { + userId: 'creator-1', + organizationId: 'org-1', + observedOrganizationId: 'org-1', + }) + ).rejects.toBeInstanceOf(WorkspaceCreationContextChangedError) + }) +}) + describe('getWorkspaceCreationPolicy', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/lib/workspaces/policy.ts b/apps/sim/lib/workspaces/policy.ts index 6fc9218487f..8f727647c01 100644 --- a/apps/sim/lib/workspaces/policy.ts +++ b/apps/sim/lib/workspaces/policy.ts @@ -5,7 +5,10 @@ import { isOrgAdminRole } from '@sim/platform-authz/workspace' import { and, count, eq, isNull } from 'drizzle-orm' import { getOrganizationSubscription } from '@/lib/billing/core/billing' import { getHighestPrioritySubscription } from '@/lib/billing/core/plan' -import { getUserOrganization } from '@/lib/billing/organizations/membership' +import { + acquireOrganizationUserMutationLocks, + getUserOrganization, +} from '@/lib/billing/organizations/membership' import type { PlanCategory } from '@/lib/billing/plan-helpers' import { getPlanType, isEnterprise, isMaxTier, isPro, isTeam } from '@/lib/billing/plan-helpers' import { hasUsableSubscriptionStatus } from '@/lib/billing/subscriptions/utils' @@ -92,6 +95,68 @@ export interface WorkspaceCreationPolicy { blockedReasonCode?: 'organization-subscription-inactive' } +export class WorkspaceCreationContextChangedError extends Error { + constructor() { + super('Workspace creation context changed before the workspace was inserted') + this.name = 'WorkspaceCreationContextChangedError' + } +} + +/** + * Serializes the final creation-policy check with membership/ownership + * mutations and row-locks the paid entitlement used by organization mode. + * Returns the live billing owner. The caller must invoke this in the same + * transaction as the workspace insert. + */ +export async function lockWorkspaceCreationContext( + tx: DbOrTx, + { + userId, + organizationId, + observedOrganizationId, + }: { + userId: string + organizationId: string | null + observedOrganizationId: string | null + } +): Promise<{ billedAccountUserId: string }> { + await acquireOrganizationUserMutationLocks(tx, { + userId, + organizationIds: organizationId ? [organizationId] : [], + }) + const currentMembership = await getUserOrganization(userId, tx) + if ( + (currentMembership?.organizationId ?? null) !== observedOrganizationId || + (organizationId !== null && currentMembership?.organizationId !== organizationId) + ) { + throw new WorkspaceCreationContextChangedError() + } + + if (!organizationId) return { billedAccountUserId: userId } + + if (isBillingEnabled) { + if (!currentMembership || !isOrgAdminRole(currentMembership.role)) { + throw new WorkspaceCreationContextChangedError() + } + const currentSubscription = await getOrganizationSubscription(organizationId, { + executor: tx, + onError: 'throw', + forUpdate: true, + }) + if ( + !currentSubscription || + !hasUsableSubscriptionStatus(currentSubscription.status) || + (!isTeam(currentSubscription.plan) && !isEnterprise(currentSubscription.plan)) + ) { + throw new WorkspaceCreationContextChangedError() + } + } + + const currentOwnerId = await getOrganizationOwnerId(organizationId, tx) + if (!currentOwnerId) throw new WorkspaceCreationContextChangedError() + return { billedAccountUserId: currentOwnerId } +} + interface GetWorkspaceCreationPolicyParams { userId: string activeOrganizationId?: string | null @@ -466,8 +531,11 @@ async function countNonOrganizationOwnedWorkspaces(userId: string): Promise { - const [ownerMembership] = await db +export async function getOrganizationOwnerId( + organizationId: string, + executor: DbOrTx = db +): Promise { + const [ownerMembership] = await executor .select({ userId: member.userId }) .from(member) .where(and(eq(member.organizationId, organizationId), eq(member.role, 'owner'))) diff --git a/packages/platform-authz/src/workspace.ts b/packages/platform-authz/src/workspace.ts index 775356d754e..0e377bd1e35 100644 --- a/packages/platform-authz/src/workspace.ts +++ b/packages/platform-authz/src/workspace.ts @@ -24,9 +24,10 @@ export * from './predicates' export async function resolveEffectiveWorkspacePermission( userId: string, workspaceId: string, - workspaceOrganizationId: string | null + workspaceOrganizationId: string | null, + executor: Pick = db ): Promise { - const [permissionRow] = await db + const [permissionRow] = await executor .select({ permissionType: permissions.permissionType }) .from(permissions) .where( @@ -41,7 +42,7 @@ export async function resolveEffectiveWorkspacePermission( const explicit = (permissionRow?.permissionType as PermissionType | undefined) ?? null if (workspaceOrganizationId && explicit !== 'admin') { - const [memberRow] = await db + const [memberRow] = await executor .select({ role: member.role }) .from(member) .where(and(eq(member.userId, userId), eq(member.organizationId, workspaceOrganizationId))) diff --git a/packages/utils/src/helpers.test.ts b/packages/utils/src/helpers.test.ts index 82fbeb325b4..b967117bacf 100644 --- a/packages/utils/src/helpers.test.ts +++ b/packages/utils/src/helpers.test.ts @@ -2,7 +2,7 @@ * @vitest-environment node */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { noop, sleep } from './helpers.js' +import { chunkArray, noop, sleep } from './helpers.js' describe('sleep', () => { beforeEach(() => { @@ -39,3 +39,13 @@ describe('noop', () => { expect(noop()).toBeUndefined() }) }) + +describe('chunkArray', () => { + it('preserves order while bounding chunk size', () => { + expect(chunkArray([1, 2, 3, 4, 5], 2)).toEqual([[1, 2], [3, 4], [5]]) + }) + + it('rejects a non-positive chunk size', () => { + expect(() => chunkArray([1], 0)).toThrow('positive integer') + }) +}) diff --git a/packages/utils/src/helpers.ts b/packages/utils/src/helpers.ts index 4a801a47d55..cbc8337b26f 100644 --- a/packages/utils/src/helpers.ts +++ b/packages/utils/src/helpers.ts @@ -8,3 +8,15 @@ export function sleep(ms: number): Promise { /** No-operation function for use as default callback. */ export const noop = () => {} + +/** Splits an array into deterministic, non-empty chunks of at most `size`. */ +export function chunkArray(values: T[], size: number): T[][] { + if (!Number.isInteger(size) || size <= 0) { + throw new Error('Chunk size must be a positive integer') + } + const chunks: T[][] = [] + for (let index = 0; index < values.length; index += size) { + chunks.push(values.slice(index, index + size)) + } + return chunks +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index de3e23b6cda..bdcfc026e81 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -10,7 +10,7 @@ export { formatTimeWithSeconds, getTimezoneAbbreviation, } from './formatting.js' -export { noop, sleep } from './helpers.js' +export { chunkArray, noop, sleep } from './helpers.js' export { generateId, generateShortId, isValidUuid } from './id.js' export type { EmbedInfo } from './media-embed.js' export { getEmbedInfo } from './media-embed.js'