diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index 0f377392820..b7d6b0d6dc5 100644 --- a/.github/workflows/test-build.yml +++ b/.github/workflows/test-build.yml @@ -132,13 +132,13 @@ jobs: lib/workspaces/organization-workspaces.postgres.test.ts lib/billing/calculations/usage-reservation.test.ts - - name: Verify access request pagination and impact in PostgreSQL + - name: Verify access request flows, pagination, and impact in PostgreSQL working-directory: apps/sim env: ACCESS_REQUESTS_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_access_requests_test run: | bun -e 'import postgres from "postgres"; const sql = postgres(process.env.DATABASE_URL); await sql.unsafe("CREATE DATABASE sim_access_requests_test"); await sql.end()' - bunx vitest run ee/access-requests/lib/repository.postgres.test.ts ee/access-requests/lib/impact.postgres.test.ts + bunx vitest run ee/access-requests/lib/repository.postgres.test.ts ee/access-requests/lib/impact.postgres.test.ts ee/access-requests/lib/application/flow.postgres.test.ts - name: Verify fork previews ignore execution file history in PostgreSQL working-directory: apps/sim diff --git a/apps/sim/ee/access-requests/lib/application/flow.postgres.test.ts b/apps/sim/ee/access-requests/lib/application/flow.postgres.test.ts new file mode 100644 index 00000000000..ceb0f11e0f5 --- /dev/null +++ b/apps/sim/ee/access-requests/lib/application/flow.postgres.test.ts @@ -0,0 +1,602 @@ +/** @vitest-environment node */ + +import { AuditAction, recordAudit } from '@sim/audit' +import type { SessionPrincipal } from '@sim/auth/principal' +import * as schema from '@sim/db/schema' +import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing' +import { generateId } from '@sim/utils/id' +import { eq } from 'drizzle-orm' +import { drizzle } from 'drizzle-orm/postgres-js' +import postgres from 'postgres' +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const { databaseUrl, select, transaction } = vi.hoisted(() => { + const databaseUrl = process.env.ACCESS_REQUESTS_TEST_DATABASE_URL + if (databaseUrl) { + const url = new URL(databaseUrl) + if ( + !['localhost', '127.0.0.1', '[::1]'].includes(url.hostname) || + url.pathname !== '/sim_access_requests_test' + ) + throw new Error('Use a disposable local sim_access_requests_test database') + } + return { databaseUrl, select: vi.fn(), transaction: vi.fn() } +}) +vi.unmock('drizzle-orm') +vi.unmock('@sim/db/schema') +vi.mock('@sim/db', async () => ({ + ...(await import('@sim/db/schema')), + db: { select, transaction }, + dbReplica: { select }, +})) +vi.mock('@sim/audit', async (original) => ({ + ...(await original()), + recordAudit: vi.fn(), +})) + +import { authorizeWorkspaceOperation } from '@/lib/core/application/workspace-authorization' +import { resolveVerifiedUserAccessControlContext } from '@/lib/permission-groups/resolve.server' +import { tableOperations } from '@/lib/table/application/operations' +import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils' +import { + cancelAccessRequest, + createAccessRequest, + discoverAccessRequests, + listMyAccessRequests, + listOrganizationAccessRequests, + updateAccessRequestSettings, +} from '@/ee/access-requests/lib/application/requests' +import { + previewAccessRequest, + resolveAccessRequest, +} from '@/ee/access-requests/lib/application/review' +import { + PERMISSION_ACCESS_REQUEST_CREATED_EVENT, + PERMISSION_ACCESS_REQUEST_DECIDED_EVENT, +} from '@/ee/access-requests/lib/notification-events' + +const schemaName = `access_flow_${generateId().replaceAll('-', '')}` +const connection = databaseUrl + ? postgres(databaseUrl, { + max: 3, + prepare: false, + connection: { + search_path: schemaName, + application_name: schemaName, + statement_timeout: 5000, + }, + onnotice: () => undefined, + }) + : undefined +const database = connection ? drizzle(connection, { schema }) : undefined +const session = (userId: string): SessionPrincipal => ({ + kind: 'session', + userId, + sessionId: `fixture-${userId}`, +}) +const member = session('member') +const admin = session('admin') +const scope = { kind: 'workspace', workspaceId: 'primary' } as const +const target = { kind: 'feature', configKey: 'hideTablesTab' } as const +const page = { limit: 10, offset: 0 } + +beforeAll(async () => { + if (!connection) return + await connection.unsafe(`CREATE SCHEMA "${schemaName}"`) + await connection.unsafe(` + CREATE TABLE "user" ( + id text PRIMARY KEY, name text NOT NULL, email text NOT NULL, + banned boolean DEFAULT false, ban_expires timestamp, suspended_at timestamp + ); + CREATE TABLE organization (id text PRIMARY KEY); + CREATE TABLE member ( + id text PRIMARY KEY, organization_id text REFERENCES organization(id), + user_id text REFERENCES "user"(id), role text NOT NULL + ); + CREATE TABLE user_stats ( + user_id text PRIMARY KEY, billing_blocked boolean DEFAULT false, billing_blocked_reason text + ); + CREATE TABLE subscription ( + id text PRIMARY KEY, plan text NOT NULL, reference_id text NOT NULL, + stripe_customer_id text, stripe_subscription_id text, status text, + period_start timestamp, period_end timestamp, cancel_at_period_end boolean, + cancel_at timestamp, canceled_at timestamp, ended_at timestamp, seats integer, + trial_start timestamp, trial_end timestamp, billing_interval text, + stripe_schedule_id text, metadata json, last_closed_period_start timestamp + ); + CREATE TABLE workspace ( + id text PRIMARY KEY, name text NOT NULL, owner_id text REFERENCES "user"(id), + organization_id text REFERENCES organization(id), workspace_mode text, + billed_account_user_id text, allow_personal_api_keys boolean DEFAULT true, + archived_at timestamp + ); + CREATE TABLE permissions ( + id text PRIMARY KEY, user_id text REFERENCES "user"(id), entity_type text, + entity_id text, permission_type text, updated_at timestamp DEFAULT now(), + UNIQUE(user_id, entity_type, entity_id) + ); + CREATE TABLE permission_group ( + id text PRIMARY KEY, organization_id text REFERENCES organization(id), name text, + description text, config jsonb NOT NULL DEFAULT '{}', created_by text, + created_at timestamp DEFAULT now(), updated_at timestamp DEFAULT now(), + is_default boolean DEFAULT false, membership_mode text DEFAULT 'inherit' + ); + CREATE TABLE permission_group_workspace ( + id text PRIMARY KEY, permission_group_id text REFERENCES permission_group(id), + workspace_id text REFERENCES workspace(id), organization_id text REFERENCES organization(id), + created_at timestamp DEFAULT now() + ); + CREATE TABLE permission_group_member ( + id text PRIMARY KEY, permission_group_id text REFERENCES permission_group(id), + organization_id text REFERENCES organization(id), user_id text REFERENCES "user"(id), + assigned_by text, assigned_at timestamp DEFAULT now() + ); + CREATE TABLE organization_member_usage_limit ( + id text PRIMARY KEY, organization_id text, user_id text, usage_limit numeric, + set_by text, created_at timestamp DEFAULT now(), updated_at timestamp DEFAULT now(), + UNIQUE(organization_id, user_id) + ); + CREATE TABLE organization_access_request_settings ( + organization_id text PRIMARY KEY REFERENCES organization(id), + allow_requests boolean DEFAULT true, updated_at timestamp DEFAULT now(), updated_by text + ); + CREATE TABLE permission_access_request ( + id text PRIMARY KEY, organization_id text NOT NULL REFERENCES organization(id), + requester_id text NOT NULL REFERENCES "user"(id), workspace_id text, + scope_key text NOT NULL, target_key text NOT NULL, target jsonb NOT NULL, + target_label text NOT NULL, membership_id text NOT NULL, group_id text, group_name text, + reason text NOT NULL DEFAULT '', status text NOT NULL DEFAULT 'pending', + decision_reason text, decided_by text REFERENCES "user"(id), decision jsonb, + created_at timestamp NOT NULL DEFAULT now(), updated_at timestamp NOT NULL DEFAULT now(), + decided_at timestamp, + CHECK (status IN ('pending', 'fulfilled', 'declined', 'cancelled', 'closed')) + ); + CREATE UNIQUE INDEX pending_request_unique ON permission_access_request + (organization_id, requester_id, scope_key, target_key) WHERE status = 'pending'; + CREATE TABLE outbox_event ( + id text PRIMARY KEY, event_type text NOT NULL, payload json NOT NULL, + status text DEFAULT 'pending', attempts integer DEFAULT 0, max_attempts integer DEFAULT 10, + available_at timestamp DEFAULT now(), locked_at timestamp, last_error text, + created_at timestamp DEFAULT now(), processed_at timestamp + ); + `) + select.mockImplementation((fields) => database!.select(fields)) + transaction.mockImplementation((callback) => database!.transaction(callback)) +}) + +beforeEach(async () => { + if (!connection) return + vi.mocked(recordAudit).mockClear() + setEnvFlags({ isHosted: true, isBillingEnabled: true, isAccessControlEnabled: true }) + await connection.unsafe(` + TRUNCATE "user", organization, member, user_stats, subscription, workspace, permissions, + permission_group, permission_group_workspace, permission_group_member, + organization_member_usage_limit, organization_access_request_settings, + permission_access_request, outbox_event; + INSERT INTO "user" (id, name, email) VALUES + ('admin', 'Admin', 'admin@example.test'), ('member', 'Member', 'member@example.test'), + ('peer', 'Peer', 'peer@example.test'), ('external', 'External', 'external@example.test'), + ('outsider', 'Outsider', 'outsider@example.test'); + INSERT INTO organization VALUES ('org'), ('other'); + INSERT INTO member VALUES + ('admin-membership', 'org', 'admin', 'owner'), + ('member-membership', 'org', 'member', 'member'), + ('peer-membership', 'org', 'peer', 'member'), + ('other-owner', 'other', 'outsider', 'owner'), + ('external-membership', 'other', 'external', 'member'); + INSERT INTO user_stats (user_id) VALUES ('admin'), ('outsider'); + INSERT INTO subscription (id, plan, reference_id, status, metadata) VALUES + ('org-subscription', 'enterprise', 'org', 'active', '{}'), + ('other-subscription', 'enterprise', 'other', 'active', '{}'); + INSERT INTO workspace (id, name, owner_id, organization_id, workspace_mode, billed_account_user_id) VALUES + ('primary', 'Primary', 'admin', 'org', 'organization', 'admin'), + ('secondary', 'Secondary', 'admin', 'org', 'organization', 'admin'), + ('foreign', 'Foreign', 'outsider', 'other', 'organization', 'outsider'); + INSERT INTO permissions (id, user_id, entity_type, entity_id, permission_type) VALUES + ('member-primary', 'member', 'workspace', 'primary', 'write'), + ('member-secondary', 'member', 'workspace', 'secondary', 'write'), + ('peer-primary', 'peer', 'workspace', 'primary', 'write'), + ('external-primary', 'external', 'workspace', 'primary', 'write'); + INSERT INTO permission_group (id, organization_id, name, config, created_by, is_default) VALUES + ('default', 'org', 'Default', '{"hideTablesTab":true}', 'admin', true), + ('restricted', 'org', 'Restricted', '{"hideTablesTab":true,"hideFilesTab":true}', 'admin', false); + INSERT INTO permission_group_workspace (id, permission_group_id, workspace_id, organization_id) + VALUES ('group-primary', 'restricted', 'primary', 'org'); + `) +}) + +afterAll(async () => { + resetEnvFlagsMock() + if (!connection) return + try { + await connection.unsafe(`DROP SCHEMA "${schemaName}" CASCADE`) + } finally { + await connection.end() + } +}) + +function create(principal = member) { + return createAccessRequest.execute({ + principal, + input: { scope, target, reason: 'Need tables for the project' }, + }) +} + +function preview(requestId: string) { + return previewAccessRequest.execute({ + principal: admin, + input: { organizationId: 'org', requestId }, + }) +} + +function apply(requestId: string, expectedFingerprint: string) { + return resolveAccessRequest.execute({ + principal: admin, + input: { organizationId: 'org', requestId, decision: { action: 'apply', expectedFingerprint } }, + }) +} + +async function authorizeTables(principal = member, workspaceId = 'primary') { + const workspace = await getWorkspaceWithOwner(workspaceId, { executor: database! }) + if (!workspace) throw new Error('Missing fixture workspace') + return authorizeWorkspaceOperation( + principal, + tableOperations.list, + { + workspaceId: workspace.id, + workspaceOrganizationId: workspace.organizationId, + allowPersonalApiKeys: workspace.allowPersonalApiKeys, + }, + { executor: database! } + ) +} + +async function storedState() { + const [group] = await database! + .select({ config: schema.permissionGroup.config }) + .from(schema.permissionGroup) + .where(eq(schema.permissionGroup.id, 'restricted')) + return { + config: group.config, + requests: await connection!` + SELECT id, status, decision, decision_reason, decided_by, decided_at, updated_at + FROM permission_access_request ORDER BY id + `, + memberLimits: await connection!` + SELECT organization_id, user_id, usage_limit, set_by, updated_at + FROM organization_member_usage_limit ORDER BY organization_id, user_id + `, + events: await connection!`SELECT event_type FROM outbox_event ORDER BY event_type`, + audit: vi.mocked(recordAudit).mock.calls.map(([entry]) => ({ + action: entry.action, + actorId: entry.actorId, + workspaceId: entry.workspaceId, + resourceType: entry.resourceType, + resourceId: entry.resourceId, + metadata: structuredClone(entry.metadata), + })), + } +} + +describe.skipIf(!databaseUrl)('access request member-to-admin flow on PostgreSQL', () => { + it('refuses workspace API keys before any protected read or transaction', async () => { + const readsBefore = select.mock.calls.length + const transactionsBefore = transaction.mock.calls.length + await expect( + createAccessRequest.execute({ + principal: { kind: 'workspace_api_key', workspaceId: 'primary', keyId: 'fixture-key' }, + input: { scope, target }, + }) + ).rejects.toMatchObject({ detailCode: 'WORKSPACE_KEY_OPERATION_NOT_PERMITTED' }) + expect(select.mock.calls).toHaveLength(readsBefore) + expect(transaction.mock.calls).toHaveLength(transactionsBefore) + expect((await storedState()).requests).toEqual([]) + expect(vi.mocked(recordAudit)).not.toHaveBeenCalled() + }) + + it('discovers, deduplicates, previews and applies access through the governing group', async () => { + await expect(authorizeTables()).rejects.toMatchObject({ + detailCode: 'PERMISSION_GROUP_CAPABILITY_BLOCKED', + }) + const discovery = await discoverAccessRequests.execute({ + principal: member, + input: { + ...scope, + targetKind: 'feature', + targetKey: 'feature:hideTablesTab', + state: 'requestable', + limit: 1, + }, + }) + expect(discovery.entries).toMatchObject([ + { target, state: 'requestable', pendingRequestId: null }, + ]) + const created = await create() + expect(created).toMatchObject({ + changed: true, + request: { status: 'pending', requester: { id: 'member' } }, + }) + expect(await create()).toMatchObject({ changed: false, request: { id: created.request.id } }) + const inbox = await listOrganizationAccessRequests.execute({ + principal: admin, + input: { organizationId: 'org', status: 'pending', ...page }, + }) + expect(inbox.requests.map(({ id }) => id)).toEqual([created.request.id]) + const prepared = await preview(created.request.id) + expect(prepared).toMatchObject({ + canApply: true, + resolutionKind: 'permission', + group: { id: 'restricted' }, + impact: { memberCount: 4, workspaceCount: 1, workspaceNames: ['Primary'], truncated: false }, + }) + expect(await apply(created.request.id, prepared.fingerprint)).toMatchObject({ + changed: true, + request: { status: 'fulfilled' }, + }) + await expect(authorizeTables()).resolves.toBeUndefined() + await expect(authorizeTables(session('peer'))).resolves.toBeUndefined() + await expect(authorizeTables(session('external'))).resolves.toBeUndefined() + await expect(authorizeTables(member, 'secondary')).rejects.toMatchObject({ + detailCode: 'PERMISSION_GROUP_CAPABILITY_BLOCKED', + }) + const effective = await resolveVerifiedUserAccessControlContext( + 'member', + 'primary', + 'org', + database! + ) + expect(effective).toMatchObject({ + entitled: true, + permissionGroup: { id: 'restricted' }, + config: { hideTablesTab: false, hideFilesTab: true }, + }) + const history = await listMyAccessRequests.execute({ + principal: member, + input: { scope, ...page }, + }) + expect(history.requests).toMatchObject([{ id: created.request.id, status: 'fulfilled' }]) + const persisted = await storedState() + expect(persisted.events).toEqual([ + { event_type: PERMISSION_ACCESS_REQUEST_CREATED_EVENT }, + { event_type: PERMISSION_ACCESS_REQUEST_DECIDED_EVENT }, + ]) + expect(vi.mocked(recordAudit).mock.calls.map(([entry]) => entry.action)).toEqual([ + AuditAction.PERMISSION_ACCESS_REQUEST_CREATED, + AuditAction.PERMISSION_ACCESS_REQUEST_FULFILLED, + AuditAction.PERMISSION_GROUP_UPDATED, + ]) + expect(await apply(created.request.id, prepared.fingerprint)).toMatchObject({ changed: false }) + expect(await storedState()).toEqual(persisted) + }) + + it('fulfills an organization member credit-cap request without changing another member', async () => { + await connection!` + INSERT INTO organization_member_usage_limit (id, organization_id, user_id, usage_limit, set_by) + VALUES ('member-cap', 'org', 'member', 10, 'admin'), ('peer-cap', 'org', 'peer', 7, 'admin') + ` + const organizationScope = { kind: 'organization', organizationId: 'org' } as const + const { request } = await createAccessRequest.execute({ + principal: member, + input: { + scope: organizationScope, + target: { kind: 'usage_limit', id: 'member' }, + reason: 'Need more credits for the project', + }, + }) + expect(request).toMatchObject({ organizationId: 'org', workspaceId: null, status: 'pending' }) + const prepared = await preview(request.id) + expect(prepared).toMatchObject({ + canApply: true, + resolutionKind: 'usage_limit', + group: null, + currentLimitCredits: 2000, + newLimitCredits: null, + }) + const before = await storedState() + await expect(apply(request.id, prepared.fingerprint)).rejects.toMatchObject({ + code: 'validation', + }) + await expect( + resolveAccessRequest.execute({ + principal: admin, + input: { + organizationId: 'org', + requestId: request.id, + decision: { + action: 'apply', + expectedFingerprint: 'stale-preview', + newLimitCredits: 3001, + }, + }, + }) + ).rejects.toMatchObject({ code: 'conflict' }) + expect(await storedState()).toEqual(before) + expect( + await resolveAccessRequest.execute({ + principal: admin, + input: { + organizationId: 'org', + requestId: request.id, + decision: { + action: 'apply', + expectedFingerprint: prepared.fingerprint, + newLimitCredits: 3001, + }, + }, + }) + ).toMatchObject({ changed: true, request: { id: request.id, status: 'fulfilled' } }) + const after = await storedState() + expect(after.memberLimits).toMatchObject([ + { organization_id: 'org', user_id: 'member', usage_limit: '15.005', set_by: 'admin' }, + { organization_id: 'org', user_id: 'peer', usage_limit: '7', set_by: 'admin' }, + ]) + expect(after.memberLimits[1]).toEqual(before.memberLimits[1]) + expect(after.config).toEqual(before.config) + expect(after.audit.map(({ action }) => action)).toEqual([ + AuditAction.PERMISSION_ACCESS_REQUEST_CREATED, + AuditAction.PERMISSION_ACCESS_REQUEST_FULFILLED, + AuditAction.ORG_MEMBER_USAGE_LIMIT_CHANGED, + ]) + const history = await listMyAccessRequests.execute({ + principal: member, + input: { scope: organizationScope, ...page }, + }) + expect(history.requests).toMatchObject([ + { + id: request.id, + target: request.target, + reason: request.reason, + createdAt: request.createdAt, + status: 'fulfilled', + workspaceId: null, + }, + ]) + expect(await preview(request.id)).toMatchObject({ + canApply: false, + resolutionKind: 'usage_limit', + currentLimitCredits: 2000, + newLimitCredits: 3001, + fingerprint: prepared.fingerprint, + }) + }) + + it.each(['decline', 'cancel'] as const)( + 'keeps the policy denied after %s and makes the decision idempotent', + async (action) => { + const { request } = await create() + const decide = () => + action === 'cancel' + ? cancelAccessRequest.execute({ + principal: member, + input: { scope, requestId: request.id }, + }) + : resolveAccessRequest.execute({ + principal: admin, + input: { + organizationId: 'org', + requestId: request.id, + decision: { action: 'decline', reason: 'Not needed for this role' }, + }, + }) + expect(await decide()).toMatchObject({ + changed: true, + request: { status: action === 'cancel' ? 'cancelled' : 'declined' }, + }) + const persisted = await storedState() + expect(persisted.config).toMatchObject({ hideTablesTab: true, hideFilesTab: true }) + expect(persisted.events).toHaveLength(2) + expect(await decide()).toMatchObject({ changed: false }) + expect(await storedState()).toEqual(persisted) + await expect(authorizeTables()).rejects.toMatchObject({ + detailCode: 'PERMISSION_GROUP_CAPABILITY_BLOCKED', + }) + } + ) + + it.each(['policy', 'audience'] as const)( + 'rejects a stale preview after %s changes without partial writes', + async (change) => { + const { request } = await create() + const prepared = await preview(request.id) + if (change === 'policy') { + await connection!`UPDATE permission_group SET config = config || '{"disableTableExport":true}'::jsonb WHERE id = 'restricted'` + } else { + await connection!`DELETE FROM permissions WHERE id = 'peer-primary'` + } + const persisted = await storedState() + await expect(apply(request.id, prepared.fingerprint)).rejects.toMatchObject({ + code: 'conflict', + }) + expect(await storedState()).toEqual(persisted) + const refreshed = await preview(request.id) + expect(refreshed.fingerprint).not.toBe(prepared.fingerprint) + expect(await apply(request.id, refreshed.fingerprint)).toMatchObject({ + request: { status: 'fulfilled' }, + }) + } + ) + + it('enforces admin, requester and organization scope against real memberships', async () => { + const { request } = await create() + const persisted = await storedState() + await expect( + previewAccessRequest.execute({ + principal: member, + input: { organizationId: 'org', requestId: request.id }, + }) + ).rejects.toMatchObject({ detailCode: 'ORGANIZATION_ADMIN_REQUIRED' }) + await expect( + resolveAccessRequest.execute({ + principal: member, + input: { + organizationId: 'org', + requestId: request.id, + decision: { action: 'decline', reason: 'No' }, + }, + }) + ).rejects.toMatchObject({ detailCode: 'ORGANIZATION_ADMIN_REQUIRED' }) + await expect( + cancelAccessRequest.execute({ + principal: session('peer'), + input: { scope, requestId: request.id }, + }) + ).rejects.toMatchObject({ code: 'not_found' }) + await expect( + cancelAccessRequest.execute({ + principal: member, + input: { scope: { kind: 'workspace', workspaceId: 'secondary' }, requestId: request.id }, + }) + ).rejects.toMatchObject({ code: 'not_found' }) + await expect( + previewAccessRequest.execute({ + principal: session('outsider'), + input: { organizationId: 'other', requestId: request.id }, + }) + ).rejects.toMatchObject({ code: 'not_found' }) + await expect(create(session('outsider'))).rejects.toMatchObject({ code: 'not_found' }) + expect(await storedState()).toEqual(persisted) + }) + + it('allows workspace collaborators to request access without granting organization authority', async () => { + const { request } = await create(session('external')) + expect((await preview(request.id)).canApply).toBe(true) + await expect( + listMyAccessRequests.execute({ + principal: session('external'), + input: { scope: { kind: 'organization', organizationId: 'org' }, ...page }, + }) + ).rejects.toMatchObject({ code: 'not_found' }) + await connection!`DELETE FROM permissions WHERE id = 'external-primary'` + const prepared = await preview(request.id) + expect(prepared.canApply).toBe(false) + await expect(apply(request.id, prepared.fingerprint)).rejects.toMatchObject({ + code: 'conflict', + }) + expect((await storedState()).requests).toMatchObject([{ id: request.id, status: 'pending' }]) + }) + + it('preserves history and cancellation while requests are paused', async () => { + const { request } = await create() + await updateAccessRequestSettings.execute({ + principal: admin, + input: { organizationId: 'org', allowRequests: false }, + }) + const discovery = await discoverAccessRequests.execute({ + principal: member, + input: { ...scope, targetKind: 'feature', limit: 1 }, + }) + expect(discovery).toMatchObject({ enabled: false, entries: [] }) + await expect(create()).rejects.toMatchObject({ detailCode: 'ACCESS_REQUESTS_DISABLED' }) + expect( + (await listMyAccessRequests.execute({ principal: member, input: { scope, ...page } })) + .requests + ).toMatchObject([{ id: request.id, status: 'pending' }]) + expect((await preview(request.id)).canApply).toBe(false) + expect( + await cancelAccessRequest.execute({ + principal: member, + input: { scope, requestId: request.id }, + }) + ).toMatchObject({ changed: true, request: { status: 'cancelled' } }) + }) +}) diff --git a/apps/sim/lib/api/contracts/access-requests.ts b/apps/sim/lib/api/contracts/access-requests.ts index 02951251afb..6ba59631b56 100644 --- a/apps/sim/lib/api/contracts/access-requests.ts +++ b/apps/sim/lib/api/contracts/access-requests.ts @@ -1,5 +1,9 @@ import { z } from 'zod' -import { organizationIdSchema, workspaceIdSchema } from '@/lib/api/contracts/primitives' +import { + organizationIdSchema, + withMissingFieldMessage, + workspaceIdSchema, +} from '@/lib/api/contracts/primitives' import { defineRouteContract } from '@/lib/api/contracts/types' import { PERMISSION_GROUP_FIELDS } from '@/lib/permission-groups/fields' import { @@ -147,14 +151,20 @@ export const resolveAccessRequestBodySchema = z.discriminatedUnion('action', [ z .object({ action: z.literal('apply'), - expectedFingerprint: fingerprintSchema, + expectedFingerprint: withMissingFieldMessage( + fingerprintSchema, + 'expectedFingerprint is required; preview the request before applying it' + ), newLimitCredits: usageLimitSchema.optional(), }) .strict(), z .object({ action: z.literal('decline'), - reason: reasonSchema.min(1, 'Explain why this request was declined'), + reason: withMissingFieldMessage( + reasonSchema.min(1, 'Explain why this request was declined'), + 'reason is required when declining a request' + ), }) .strict(), ]) diff --git a/apps/sim/lib/api/contracts/v2/required-field-messages.test.ts b/apps/sim/lib/api/contracts/v2/required-field-messages.test.ts index e72f4e2a57d..6611281a779 100644 --- a/apps/sim/lib/api/contracts/v2/required-field-messages.test.ts +++ b/apps/sim/lib/api/contracts/v2/required-field-messages.test.ts @@ -2,6 +2,8 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' +import { resolveAccessRequestBodySchema } from '@/lib/api/contracts/access-requests' +import { v2ResolveAccessRequestBodySchema } from '@/lib/api/contracts/v2/access-requests' import { v2KnowledgeSearchBodySchema } from '@/lib/api/contracts/v2/knowledge' import { v2CreateSkillBodySchema } from '@/lib/api/contracts/v2/skills' import { v2CreateWorkflowBodySchema } from '@/lib/api/contracts/v2/workflows' @@ -16,6 +18,42 @@ function messageAt( return result.error?.issues.find((issue) => issue.path[0] === field)?.message } +describe.each([ + ['internal', resolveAccessRequestBodySchema], + ['v2', v2ResolveAccessRequestBodySchema], +] as const)('%s access request decisions name missing required fields', (_surface, schema) => { + it.each([ + [ + 'apply', + 'expectedFingerprint', + 'expectedFingerprint is required; preview the request before applying it', + ], + ['decline', 'reason', 'reason is required when declining a request'], + ] as const)('names the missing field for %s', (action, field, message) => { + expect(messageAt(schema.safeParse({ action }), field)).toBe(message) + expect(messageAt(schema.safeParse({ action, [field]: 123 }), field)).toBe( + 'Invalid input: expected string, received number' + ) + }) + + it('preserves decision validation and trimming', () => { + expect(schema.safeParse({ action: 'apply', expectedFingerprint: '' }).success).toBe(false) + expect( + schema.safeParse({ action: 'apply', expectedFingerprint: 'x'.repeat(129) }).success + ).toBe(false) + expect(schema.safeParse({ action: 'decline', reason: ' ' }).success).toBe(false) + expect(schema.safeParse({ action: 'decline', reason: 'x'.repeat(1001) }).success).toBe(false) + expect(schema.parse({ action: 'apply', expectedFingerprint: 'reviewed' })).toEqual({ + action: 'apply', + expectedFingerprint: 'reviewed', + }) + expect(schema.parse({ action: 'decline', reason: ' Not needed ' })).toEqual({ + action: 'decline', + reason: 'Not needed', + }) + }) +}) + /** * A required field that is *omitted* and one that is *wrong-typed* are different * mistakes. Both used to answer with wording that pointed at the other: the diff --git a/apps/sim/lib/billing/enterprise-provisioning.test.ts b/apps/sim/lib/billing/enterprise-provisioning.test.ts index 58afc58ab15..1368760c0ee 100644 --- a/apps/sim/lib/billing/enterprise-provisioning.test.ts +++ b/apps/sim/lib/billing/enterprise-provisioning.test.ts @@ -740,6 +740,195 @@ describe('Enterprise creation invitations', () => { }) }) + it.each(['admin', 'owner'] as const)( + 'recognizes inherited organization %s access without explicit workspace grants', + async (role) => { + const payload = operationPayload({ + request: { + ...operationPayload().request, + workspaceIds: ['workspace-1', 'workspace-2'], + }, + applicationResult: { + appliedAt: '2026-08-13T00:00:00.000Z', + subscriptionId: 'sub-1', + }, + }) + queueTableRows(schemaMock.outboxEvent, [ + { eventType: 'stripe.provision-enterprise', payload }, + ]) + queueTableRows(schemaMock.outboxEvent, []) + queueTableRows(schemaMock.outboxEvent, [{ status: 'completed' }, { status: 'completed' }]) + queueTableRows( + schemaMock.user, + ['workspace-1', 'workspace-2'].map((workspaceId) => ({ + userId: 'invitee-1', + workspaceId, + role, + permission: null, + })) + ) + const checkpointPayload = vi.fn() + + await inviteEnterprisePeople( + { + provisioningOperationId: 'operation-1', + organizationId: 'org-1', + ownerUserId: 'owner-1', + email: 'new@example.com', + role: 'admin', + permission: 'admin', + sequence: 0, + }, + { + eventId: 'invite-1', + eventType: 'enterprise.invite-people', + attempts: 0, + checkpointPayload, + } + ) + + expect(checkpointPayload).toHaveBeenCalledExactlyOnceWith({ + delivery: { + completedAt: expect.any(String), + resultId: 'invitee-1', + outcome: 'unchanged', + }, + }) + expect(mocks.createWorkspaceInvitation).not.toHaveBeenCalled() + expect(mocks.prepareWorkspaceInvitationContext).not.toHaveBeenCalled() + expect(mocks.sendInvitationEmail).not.toHaveBeenCalled() + } + ) + + it.each([ + { + name: 'a concurrent promotion', + role: 'admin', + permission: null, + requestedRole: 'member', + workspaceIds: ['workspace-1'], + applied: true, + }, + { + name: 'a sufficient explicit grant', + role: 'member', + permission: 'write', + requestedRole: 'member', + workspaceIds: ['workspace-1'], + applied: true, + }, + { + name: 'an insufficient explicit grant', + role: 'member', + permission: 'read', + requestedRole: 'member', + workspaceIds: ['workspace-1'], + applied: false, + }, + { + name: 'a workspace leaving the organization scope', + role: null, + permission: null, + requestedRole: 'member', + workspaceIds: ['workspace-1'], + applied: false, + }, + { + name: 'a workspace admin grant without the requested organization admin role', + role: 'member', + permission: 'admin', + requestedRole: 'admin', + workspaceIds: ['workspace-1'], + applied: false, + }, + { + name: 'inherited access to only one of two requested workspaces', + role: 'admin', + permission: null, + requestedRole: 'member', + workspaceIds: ['workspace-1', 'workspace-2'], + applied: false, + }, + ] as const)( + 'checks the final effective access after $name', + async ({ role, permission, requestedRole, workspaceIds, applied }) => { + const payload = operationPayload({ + request: { ...operationPayload().request, workspaceIds: [...workspaceIds] }, + applicationResult: { + appliedAt: '2026-08-13T00:00:00.000Z', + subscriptionId: 'sub-1', + }, + }) + queueTableRows(schemaMock.outboxEvent, [ + { eventType: 'stripe.provision-enterprise', payload }, + ]) + queueTableRows(schemaMock.outboxEvent, []) + queueTableRows( + schemaMock.outboxEvent, + workspaceIds.map(() => ({ status: 'completed' })) + ) + queueTableRows(schemaMock.user, [ + { userId: 'invitee-1', workspaceId: 'workspace-1', role: 'member', permission: null }, + ]) + queueTableRows(schemaMock.invitation, []) + queueTableRows(schemaMock.user, [{ organizationId: 'org-1' }]) + queueTableRows(schemaMock.user, [ + { id: 'owner-1', name: 'Owner', email: 'owner@example.com' }, + ]) + queueTableRows( + schemaMock.user, + role ? [{ userId: 'invitee-1', workspaceId: 'workspace-1', role, permission }] : [] + ) + queueTableRows(schemaMock.invitation, []) + mocks.createWorkspaceInvitation.mockResolvedValueOnce({ + id: 'invitee-1', + instantAdd: true, + outcome: 'unchanged', + workspaceIds: [], + }) + const checkpointPayload = vi.fn() + const result = inviteEnterprisePeople( + { + provisioningOperationId: 'operation-1', + organizationId: 'org-1', + ownerUserId: 'owner-1', + email: 'new@example.com', + role: requestedRole, + permission: 'write', + sequence: 0, + }, + { + eventId: 'invite-1', + eventType: 'enterprise.invite-people', + attempts: 0, + checkpointPayload, + } + ) + + if (applied) { + await expect(result).resolves.toBeUndefined() + expect(checkpointPayload).toHaveBeenLastCalledWith({ + delivery: { + completedAt: expect.any(String), + resultId: 'invitee-1', + outcome: 'unchanged', + }, + }) + } else { + await expect(result).rejects.toThrow( + 'did not apply the requested organization role and workspace permissions' + ) + expect(checkpointPayload).toHaveBeenCalledExactlyOnceWith({ + attemptedAt: expect.any(String), + }) + } + expect(mocks.createWorkspaceInvitation).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ existingAccessPolicy: 'ensure-at-least' }) + ) + expect(mocks.sendInvitationEmail).not.toHaveBeenCalled() + } + ) + it('waits without consuming attempts until every selected workspace move completes', async () => { const payload = operationPayload({ request: { diff --git a/apps/sim/lib/billing/enterprise-provisioning.ts b/apps/sim/lib/billing/enterprise-provisioning.ts index 6a5f1152037..0629fb63c55 100644 --- a/apps/sim/lib/billing/enterprise-provisioning.ts +++ b/apps/sim/lib/billing/enterprise-provisioning.ts @@ -2746,36 +2746,50 @@ async function resolveEnterpriseInvitationApplicationState( workspaceIds: string[] ): Promise { const normalizedEmail = normalizeEmail(payload.email) - const [existingUser] = await db - .select({ id: user.id, organizationId: member.organizationId, role: member.role }) + const accessRows = await db + .select({ + userId: user.id, + workspaceId: workspace.id, + role: member.role, + permission: permissions.permissionType, + }) .from(user) - .leftJoin(member, eq(member.userId, user.id)) - .where(eq(user.normalizedEmail, normalizedEmail)) - .limit(1) - const roleSatisfied = - existingUser?.organizationId === payload.organizationId && - (payload.role === 'member' || isOrgAdminRole(existingUser.role)) - if (existingUser && roleSatisfied) { - const accessRows = await db - .select({ workspaceId: permissions.entityId, permission: permissions.permissionType }) - .from(permissions) - .where( - and( - eq(permissions.entityType, 'workspace'), - eq(permissions.userId, existingUser.id), - inArray(permissions.entityId, workspaceIds) - ) + .innerJoin( + member, + and(eq(member.userId, user.id), eq(member.organizationId, payload.organizationId)) + ) + .innerJoin( + workspace, + and( + eq(workspace.organizationId, member.organizationId), + inArray(workspace.id, workspaceIds), + isNull(workspace.archivedAt) ) - const accessByWorkspace = new Map( - accessRows.map((row) => [row.workspaceId, row.permission] as const) ) - if ( - workspaceIds.every((workspaceId) => - permissionSatisfies(accessByWorkspace.get(workspaceId), payload.permission) + .leftJoin( + permissions, + and( + eq(permissions.entityType, 'workspace'), + eq(permissions.userId, user.id), + eq(permissions.entityId, workspace.id) ) - ) { - return { kind: 'applied', resultId: existingUser.id } - } + ) + .where(eq(user.normalizedEmail, normalizedEmail)) + const accessByWorkspace = new Map(accessRows.map((row) => [row.workspaceId, row] as const)) + const existingUserId = accessRows[0]?.userId + if ( + existingUserId && + workspaceIds.every((workspaceId) => { + const access = accessByWorkspace.get(workspaceId) + if (!access) return false + const inheritsAdmin = isOrgAdminRole(access.role) + return ( + (payload.role === 'member' || inheritsAdmin) && + (inheritsAdmin || permissionSatisfies(access.permission, payload.permission)) + ) + }) + ) { + return { kind: 'applied', resultId: existingUserId } } const pendingRows = await db diff --git a/apps/sim/lib/invitations/direct-grant.test.ts b/apps/sim/lib/invitations/direct-grant.test.ts index cf225e72b66..cdf19ff5d8b 100644 --- a/apps/sim/lib/invitations/direct-grant.test.ts +++ b/apps/sim/lib/invitations/direct-grant.test.ts @@ -145,13 +145,40 @@ describe('grantWorkspaceAccessDirectly', () => { expect(dbChainMockFns.for.mock.invocationCallOrder[0]).toBeLessThan( mockGetEffectiveWorkspacePermission.mock.invocationCallOrder[0] ) - expect(dbChainMockFns.for).toHaveBeenCalledTimes(3) + expect(dbChainMockFns.for).toHaveBeenCalledTimes(4) expect(dbChainMockFns.for.mock.invocationCallOrder[1]).toBeLessThan( mockGetEffectiveWorkspacePermission.mock.invocationCallOrder[0] ) expect(dbChainMockFns.from).toHaveBeenCalledWith(member) }) + it.each(['admin', 'owner'] as const)( + 'preserves an invitee who became organization %s before the transaction without redundant effects', + async (role) => { + mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-1', role }) + + const result = await grantWorkspaceAccessDirectly({ + ...baseInput, + existingPermissionPolicy: 'ensure-at-least', + }) + + expect(result).toEqual({ outcome: 'unchanged', permission: 'admin' }) + expect(mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.for.mock.invocationCallOrder[1] + ) + expect(dbChainMockFns.for.mock.invocationCallOrder[1]).toBeLessThan( + mockGetUserOrganization.mock.invocationCallOrder[0] + ) + expect(dbChainMockFns.insert).not.toHaveBeenCalled() + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(mockEnqueueOutboxEvent).not.toHaveBeenCalled() + expect(mockSyncWorkspaceEnvCredentials).not.toHaveBeenCalled() + expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() + expect(mockWorkspaceMemberAdded).not.toHaveBeenCalled() + expect(mockCaptureServerEvent).not.toHaveBeenCalled() + } + ) + it('delivers the transactionally enqueued notification through the outbox', async () => { await directGrantOutboxHandlers[DIRECT_GRANT_EMAIL_EVENT_TYPE]( { diff --git a/apps/sim/lib/invitations/direct-grant.ts b/apps/sim/lib/invitations/direct-grant.ts index e23f27da23d..56877067ad1 100644 --- a/apps/sim/lib/invitations/direct-grant.ts +++ b/apps/sim/lib/invitations/direct-grant.ts @@ -10,7 +10,7 @@ import { workspaceEnvironment, } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { permissionSatisfies } from '@sim/platform-authz/workspace' +import { isOrgAdminRole, permissionSatisfies } from '@sim/platform-authz/workspace' import { generateId } from '@sim/utils/id' import { isRecordLike } from '@sim/utils/object' import { normalizeEmail } from '@sim/utils/string' @@ -179,6 +179,13 @@ export async function grantWorkspaceAccessDirectly( and(eq(member.userId, input.actorId), eq(member.organizationId, input.organizationId)) ) .for('update') + await tx + .select({ id: member.id }) + .from(member) + .where( + and(eq(member.userId, input.userId), eq(member.organizationId, input.organizationId)) + ) + .for('update') const workspaceRow = await getWorkspaceWithOwner(input.workspaceId, { executor: tx, @@ -228,7 +235,9 @@ export async function grantWorkspaceAccessDirectly( .limit(1) let outcome: DirectGrantOutcome - if (existing) { + if (isOrgAdminRole(inviteeMembership.role)) { + outcome = { outcome: 'unchanged', permission: 'admin' } + } else if (existing) { const existingPermission = existing.permissionType as PermissionType if ( input.existingPermissionPolicy === 'ensure-at-least' && diff --git a/apps/sim/lib/invitations/workspace-invitations.test.ts b/apps/sim/lib/invitations/workspace-invitations.test.ts index d3eaa085368..541b27057ee 100644 --- a/apps/sim/lib/invitations/workspace-invitations.test.ts +++ b/apps/sim/lib/invitations/workspace-invitations.test.ts @@ -264,6 +264,336 @@ describe('createWorkspaceInvitation', () => { expect(mockCreatePendingInvitation).not.toHaveBeenCalled() }) + it.each(['admin', 'owner'] as const)( + 'rejects inviting an organization %s who already inherits workspace access', + async (role) => { + queueTableRows(userTable, [{ id: 'user-2', email: 'member@example.com' }]) + queueTableRows(member, [{ role: 'owner' }]) + queueTableRows(member, [{ role }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role, + }) + + await expect( + createWorkspaceInvitation({ + context: makeContext(['ws-1', 'ws-2']), + email: 'member@example.com', + permission: 'write', + request, + }) + ).rejects.toThrow('already has access to every selected workspace') + + expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled() + expect(mockCreatePendingInvitation).not.toHaveBeenCalled() + expect(mockSendInvitationEmail).not.toHaveBeenCalled() + expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() + expect(mockAcquireOrganizationUserMutationLocks).toHaveBeenCalledOnce() + expect(dbChainMockFns.update).not.toHaveBeenCalled() + } + ) + + it.each(['member', 'admin'] as const)( + 'preserves current membership after a concurrent demotion in ordinary %s invitations', + async (membership) => { + queueTableRows(userTable, [{ id: 'user-2' }]) + queueTableRows(member, [{ role: 'member' }]) + queueTableRows(member, [{ role: 'member' }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role: 'admin', + }) + + const result = await createWorkspaceInvitation({ + context: makeContext(), + email: 'member@example.com', + permission: 'write', + membership, + request, + }) + + expect(result).toMatchObject({ outcome: 'added', workspaceIds: ['ws-1'] }) + expect(mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + mockGrantWorkspaceAccessDirectly.mock.invocationCallOrder[0] + ) + expect(mockGrantWorkspaceAccessDirectly).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ userId: 'user-2', existingPermissionPolicy: 'preserve' }) + ) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() + expect(mockCreatePendingInvitation).not.toHaveBeenCalled() + } + ) + + it.each(['admin', 'owner'] as const)( + 'leaves inherited access unchanged when ensuring access for an organization %s', + async (role) => { + queueTableRows(userTable, [{ id: 'user-2', email: 'member@example.com' }]) + queueTableRows(member, [{ role: 'owner' }]) + queueTableRows(member, [{ role }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role, + }) + + const result = await createWorkspaceInvitation({ + context: makeContext(['ws-1', 'ws-2']), + email: 'member@example.com', + permission: 'admin', + membership: 'admin', + existingAccessPolicy: 'ensure-at-least', + request, + }) + + expect(result).toMatchObject({ + workspaceIds: [], + instantAdd: true, + outcome: 'unchanged', + membershipIntent: 'internal', + }) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled() + expect(mockCreatePendingInvitation).not.toHaveBeenCalled() + expect(mockSendInvitationEmail).not.toHaveBeenCalled() + expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() + } + ) + + it.each(['member', 'admin'])( + 'reports a current member promotion correctly after initially observing %s', + async (observedRole) => { + queueTableRows(userTable, [{ id: 'user-2' }]) + queueTableRows(member, [{ role: 'owner' }]) + queueTableRows(member, [{ role: 'member' }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role: observedRole, + }) + + const result = await createWorkspaceInvitation({ + context: makeContext(['ws-1', 'ws-2']), + email: 'member@example.com', + permission: 'write', + membership: 'admin', + existingAccessPolicy: 'ensure-at-least', + request, + }) + + expect(result).toMatchObject({ + workspaceIds: [], + instantAdd: true, + outcome: 'updated', + membershipIntent: 'internal', + }) + expect(dbChainMockFns.update).toHaveBeenCalledExactlyOnceWith(member) + expect(dbChainMockFns.set).toHaveBeenCalledWith({ role: 'admin' }) + expect(auditMockFns.mockRecordAudit).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + action: 'org_member.role_changed', + metadata: expect.objectContaining({ previousRole: 'member', newRole: 'admin' }), + }) + ) + expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled() + expect(mockCreatePendingInvitation).not.toHaveBeenCalled() + expect(mockSendInvitationEmail).not.toHaveBeenCalled() + } + ) + + it('reconciles workspace access when an inherited admin was demoted before the locked check', async () => { + queueTableRows(userTable, [{ id: 'user-2' }]) + queueTableRows(member, [{ role: 'member' }]) + queueTableRows(member, [{ role: 'member' }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role: 'admin', + }) + + const result = await createWorkspaceInvitation({ + context: makeContext(), + email: 'member@example.com', + membership: 'member', + permission: 'write', + existingAccessPolicy: 'ensure-at-least', + }) + + expect(result).toMatchObject({ + outcome: 'added', + workspaceIds: ['ws-1'], + instantAdd: true, + }) + expect(mockAcquireInvitationMutationLocks).toHaveBeenCalledExactlyOnceWith(expect.anything(), { + invitationIds: [], + workspaceIds: ['ws-1'], + }) + expect(mockAcquireInvitationMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0] + ) + expect(mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.for.mock.invocationCallOrder[0] + ) + expect(mockGetEffectiveWorkspacePermission).toHaveBeenCalledExactlyOnceWith( + 'user-1', + expect.objectContaining({ id: 'ws-1', organizationId: 'org-1' }), + expect.anything() + ) + expect(mockGrantWorkspaceAccessDirectly).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ userId: 'user-2', permission: 'write' }) + ) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(mockCreatePendingInvitation).not.toHaveBeenCalled() + }) + + it('lets workspace admins preserve current inherited access without organization-admin authority', async () => { + queueTableRows(userTable, [{ id: 'user-2' }]) + queueTableRows(member, [{ role: 'member' }]) + queueTableRows(member, [{ role: 'admin' }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role: 'admin', + }) + + const result = await createWorkspaceInvitation({ + context: makeContext(), + email: 'member@example.com', + membership: 'member', + existingAccessPolicy: 'ensure-at-least', + }) + + expect(result).toMatchObject({ outcome: 'unchanged', workspaceIds: [] }) + expect(mockGetEffectiveWorkspacePermission).toHaveBeenCalled() + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled() + expect(mockSendInvitationEmail).not.toHaveBeenCalled() + }) + + it('excludes workspaces already covered when the direct grant observes a concurrent promotion', async () => { + queueTableRows(userTable, [{ id: 'user-2' }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role: 'member', + }) + mockGrantWorkspaceAccessDirectly.mockResolvedValueOnce({ + outcome: 'unchanged', + permission: 'admin', + }) + + const result = await createWorkspaceInvitation({ + context: makeContext(), + email: 'member@example.com', + existingAccessPolicy: 'ensure-at-least', + }) + + expect(result).toMatchObject({ outcome: 'unchanged', workspaceIds: [], instantAdd: true }) + expect(mockGrantWorkspaceAccessDirectly).toHaveBeenCalledOnce() + expect(mockCreatePendingInvitation).not.toHaveBeenCalled() + expect(mockSendInvitationEmail).not.toHaveBeenCalled() + }) + + it('rejects inherited access reconciliation when the workspace changed organizations', async () => { + queueTableRows(userTable, [{ id: 'user-2' }]) + queueTableRows(member, [{ role: 'owner' }]) + queueTableRows(member, [{ role: 'admin' }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role: 'admin', + }) + mockGetWorkspaceWithOwner.mockResolvedValueOnce(makeTarget('ws-1', 'org-2').workspaceDetails) + + await expect( + createWorkspaceInvitation({ + context: makeContext(), + email: 'member@example.com', + existingAccessPolicy: 'ensure-at-least', + }) + ).rejects.toMatchObject({ status: 409 }) + + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled() + expect(mockSendInvitationEmail).not.toHaveBeenCalled() + expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() + }) + + it('rejects an inherited-access no-op when the inviter lost workspace admin access', async () => { + queueTableRows(userTable, [{ id: 'user-2' }]) + queueTableRows(member, [{ role: 'member' }]) + queueTableRows(member, [{ role: 'admin' }]) + mockGetUserOrganization.mockResolvedValueOnce({ + organizationId: 'org-1', + memberId: 'member-2', + role: 'admin', + }) + mockGetEffectiveWorkspacePermission.mockResolvedValueOnce('read') + + await expect( + createWorkspaceInvitation({ + context: makeContext(), + email: 'member@example.com', + existingAccessPolicy: 'ensure-at-least', + }) + ).rejects.toMatchObject({ status: 409 }) + + expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled() + expect(mockSendInvitationEmail).not.toHaveBeenCalled() + expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled() + }) + + it.each(['admin', 'owner'] as const)( + 'does not inherit workspace access from a different organization %s role', + async (role) => { + queueWhereResponses([[{ id: 'user-3', email: 'ext@example.com' }], []]) + mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-2', role }) + + const result = await createWorkspaceInvitation({ + context: makeContext(), + email: 'ext@example.com', + permission: 'read', + request, + }) + + expect(result.membershipIntent).toBe('external') + expect(result.workspaceIds).toEqual(['ws-1']) + expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled() + expect(mockCreatePendingInvitation).toHaveBeenCalledWith( + expect.objectContaining({ + membershipIntent: 'external', + grants: [{ workspaceId: 'ws-1', permission: 'read' }], + }) + ) + } + ) + + it('does not treat a foreign organization admin role as a stronger explicit workspace grant', async () => { + queueWhereResponses([ + [{ id: 'user-3', email: 'ext@example.com' }], + [{ workspaceId: 'ws-1', permission: 'read' }], + ]) + mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-2', role: 'admin' }) + + await createWorkspaceInvitation({ + context: makeContext(), + email: 'ext@example.com', + permission: 'write', + existingAccessPolicy: 'ensure-at-least', + request, + }) + + expect(mockCreatePendingInvitation).toHaveBeenCalledWith( + expect.objectContaining({ + membershipIntent: 'external', + grants: [{ workspaceId: 'ws-1', permission: 'write' }], + }) + ) + expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled() + }) + it('creates an external pending invitation when the user belongs to a different org', async () => { queueWhereResponses([[{ id: 'user-3', email: 'ext@example.com' }], []]) mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-2', role: 'member' }) diff --git a/apps/sim/lib/invitations/workspace-invitations.ts b/apps/sim/lib/invitations/workspace-invitations.ts index 0cf92749162..d29ccaf7771 100644 --- a/apps/sim/lib/invitations/workspace-invitations.ts +++ b/apps/sim/lib/invitations/workspace-invitations.ts @@ -140,17 +140,15 @@ async function ensureExistingMemberOrganizationRole({ request?: OrchestrationRequestContext validateLockedWorkspace?: GrantWorkspaceAccessDirectlyInput['validateLockedWorkspace'] }): Promise<{ role: string; updated: boolean }> { - if (requestedRole !== 'admin' || isOrgAdminRole(currentRole)) { + if (requestedRole !== 'admin' && !isOrgAdminRole(currentRole)) { return { role: currentRole, updated: false } } - const updated = await db.transaction(async (tx) => { - if (validateLockedWorkspace) { - await acquireInvitationMutationLocks(tx, { - invitationIds: [], - workspaceIds: context.targets.map((target) => target.workspaceId), - }) - } + const result = await db.transaction(async (tx) => { + await acquireInvitationMutationLocks(tx, { + invitationIds: [], + workspaceIds: context.targets.map((target) => target.workspaceId), + }) await acquireOrganizationUserMutationLocks(tx, { userId, organizationIds: [organizationId], @@ -173,36 +171,60 @@ async function ensureExistingMemberOrganizationRole({ ) .for('update') .limit(1) - if (!actorMembership || !isOrgAdminRole(actorMembership.role) || !targetMembership) { + const needsPromotion = requestedRole === 'admin' && !isOrgAdminRole(targetMembership?.role) + if (!targetMembership || (needsPromotion && !isOrgAdminRole(actorMembership?.role))) { throw new WorkspaceInvitationError({ message: 'Organization membership changed. Refresh and try again.', status: 409, email, }) } - if (isOrgAdminRole(targetMembership.role)) return false - if (validateLockedWorkspace) { - for (const workspaceId of context.targets.map((target) => target.workspaceId).sort()) { - const workspaceDetails = await getWorkspaceWithOwner(workspaceId, { - executor: tx, - forUpdate: true, + for (const workspaceId of context.targets.map((target) => target.workspaceId).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, + email, + }) + } + 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, + email, }) - if (!workspaceDetails || workspaceDetails.organizationId !== organizationId) { - throw new WorkspaceInvitationError({ - message: - 'A selected workspace changed organizations. Review the selection and try again.', - status: 409, - email, - }) - } - await validateLockedWorkspace(tx, workspaceDetails) } + await validateLockedWorkspace?.(tx, workspaceDetails) + } + if (needsPromotion) { + await tx.update(member).set({ role: 'admin' }).where(eq(member.id, memberId)) + } + return { + role: needsPromotion ? 'admin' : targetMembership.role, + updated: needsPromotion, + previousRole: targetMembership.role, } - await tx.update(member).set({ role: 'admin' }).where(eq(member.id, memberId)) - return true }) - if (updated) { + if (result.updated) { recordAudit({ actorId: context.auditActor ? context.auditActor.id : context.inviterId, actorName: context.auditActor ? context.auditActor.name : context.inviterName, @@ -216,13 +238,13 @@ async function ensureExistingMemberOrganizationRole({ ...context.auditActor?.metadata, targetUserId: userId, memberId, - previousRole: currentRole, + previousRole: result.previousRole, newRole: 'admin', }, request, }) } - return { role: 'admin', updated } + return { role: result.role, updated: result.updated } } /** @@ -555,10 +577,10 @@ export async function createWorkspaceInvitation({ let existingOrganizationRole = existingMembership?.role let organizationRoleUpdated = false if ( - existingAccessPolicy === 'ensure-at-least' && existingUser && organizationId && - existingMembership?.organizationId === organizationId + existingMembership?.organizationId === organizationId && + (existingAccessPolicy === 'ensure-at-least' || isOrgAdminRole(existingMembership.role)) ) { const ensuredRole = await ensureExistingMemberOrganizationRole({ context, @@ -566,7 +588,8 @@ export async function createWorkspaceInvitation({ memberId: existingMembership.memberId, userId: existingUser.id, currentRole: existingMembership.role, - requestedRole: membership === 'admin' ? 'admin' : 'member', + requestedRole: + existingAccessPolicy === 'ensure-at-least' && membership === 'admin' ? 'admin' : 'member', email: normalizedEmail, request, validateLockedWorkspace, @@ -577,6 +600,10 @@ export async function createWorkspaceInvitation({ let pendingTargets = context.targets if (existingUser) { + const inheritsWorkspaceAdmin = + organizationId !== null && + existingMembership?.organizationId === organizationId && + isOrgAdminRole(existingOrganizationRole) const accessibleRows = await db .select({ workspaceId: permissions.entityId, permission: permissions.permissionType }) .from(permissions) @@ -588,14 +615,15 @@ export async function createWorkspaceInvitation({ ) ) const accessibleWorkspaceIds = new Set( - accessibleRows - .filter( - (row) => - existingAccessPolicy === 'preserve' || - isOrgAdminRole(existingOrganizationRole) || - permissionSatisfies(row.permission, invitationPermission) - ) - .map((row) => row.workspaceId) + inheritsWorkspaceAdmin + ? allWorkspaceIds + : accessibleRows + .filter( + (row) => + existingAccessPolicy === 'preserve' || + permissionSatisfies(row.permission, invitationPermission) + ) + .map((row) => row.workspaceId) ) /** @@ -637,6 +665,7 @@ export async function createWorkspaceInvitation({ */ if (organizationId && existingMembership?.organizationId === organizationId) { let outcome: DirectGrantOutcome['outcome'] = organizationRoleUpdated ? 'updated' : 'unchanged' + const grantedWorkspaceIds: string[] = [] for (const target of pendingTargets) { let directGrant: DirectGrantOutcome try { @@ -668,6 +697,7 @@ export async function createWorkspaceInvitation({ } throw error } + if (directGrant.outcome !== 'unchanged') grantedWorkspaceIds.push(target.workspaceId) if (directGrant.outcome === 'added') outcome = 'added' else if (directGrant.outcome === 'updated' && outcome === 'unchanged') outcome = 'updated' } @@ -675,7 +705,7 @@ export async function createWorkspaceInvitation({ return { id: existingUser.id, email: normalizedEmail, - workspaceIds: pendingTargets.map((target) => target.workspaceId), + workspaceIds: grantedWorkspaceIds, permission: invitationPermission, membershipIntent: 'internal', instantAdd: true,