Skip to content

Commit 96f4e64

Browse files
authored
fix(organizations): respect inherited access when inviting members (#8131)
* fix(organizations): respect inherited access when inviting members * fix(organizations): recheck inherited access under invitation locks * fix(organizations): verify effective access across invitation flows
1 parent 2c96b88 commit 96f4e64

10 files changed

Lines changed: 1324 additions & 75 deletions

File tree

‎.github/workflows/test-build.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,13 @@ jobs:
132132
lib/workspaces/organization-workspaces.postgres.test.ts
133133
lib/billing/calculations/usage-reservation.test.ts
134134
135-
- name: Verify access request pagination and impact in PostgreSQL
135+
- name: Verify access request flows, pagination, and impact in PostgreSQL
136136
working-directory: apps/sim
137137
env:
138138
ACCESS_REQUESTS_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_access_requests_test
139139
run: |
140140
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()'
141-
bunx vitest run ee/access-requests/lib/repository.postgres.test.ts ee/access-requests/lib/impact.postgres.test.ts
141+
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
142142
143143
- name: Verify fork previews ignore execution file history in PostgreSQL
144144
working-directory: apps/sim

‎apps/sim/ee/access-requests/lib/application/flow.postgres.test.ts‎

Lines changed: 602 additions & 0 deletions
Large diffs are not rendered by default.

‎apps/sim/lib/api/contracts/access-requests.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { z } from 'zod'
2-
import { organizationIdSchema, workspaceIdSchema } from '@/lib/api/contracts/primitives'
2+
import {
3+
organizationIdSchema,
4+
withMissingFieldMessage,
5+
workspaceIdSchema,
6+
} from '@/lib/api/contracts/primitives'
37
import { defineRouteContract } from '@/lib/api/contracts/types'
48
import { PERMISSION_GROUP_FIELDS } from '@/lib/permission-groups/fields'
59
import {
@@ -147,14 +151,20 @@ export const resolveAccessRequestBodySchema = z.discriminatedUnion('action', [
147151
z
148152
.object({
149153
action: z.literal('apply'),
150-
expectedFingerprint: fingerprintSchema,
154+
expectedFingerprint: withMissingFieldMessage(
155+
fingerprintSchema,
156+
'expectedFingerprint is required; preview the request before applying it'
157+
),
151158
newLimitCredits: usageLimitSchema.optional(),
152159
})
153160
.strict(),
154161
z
155162
.object({
156163
action: z.literal('decline'),
157-
reason: reasonSchema.min(1, 'Explain why this request was declined'),
164+
reason: withMissingFieldMessage(
165+
reasonSchema.min(1, 'Explain why this request was declined'),
166+
'reason is required when declining a request'
167+
),
158168
})
159169
.strict(),
160170
])

‎apps/sim/lib/api/contracts/v2/required-field-messages.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5+
import { resolveAccessRequestBodySchema } from '@/lib/api/contracts/access-requests'
6+
import { v2ResolveAccessRequestBodySchema } from '@/lib/api/contracts/v2/access-requests'
57
import { v2KnowledgeSearchBodySchema } from '@/lib/api/contracts/v2/knowledge'
68
import { v2CreateSkillBodySchema } from '@/lib/api/contracts/v2/skills'
79
import { v2CreateWorkflowBodySchema } from '@/lib/api/contracts/v2/workflows'
@@ -16,6 +18,42 @@ function messageAt(
1618
return result.error?.issues.find((issue) => issue.path[0] === field)?.message
1719
}
1820

21+
describe.each([
22+
['internal', resolveAccessRequestBodySchema],
23+
['v2', v2ResolveAccessRequestBodySchema],
24+
] as const)('%s access request decisions name missing required fields', (_surface, schema) => {
25+
it.each([
26+
[
27+
'apply',
28+
'expectedFingerprint',
29+
'expectedFingerprint is required; preview the request before applying it',
30+
],
31+
['decline', 'reason', 'reason is required when declining a request'],
32+
] as const)('names the missing field for %s', (action, field, message) => {
33+
expect(messageAt(schema.safeParse({ action }), field)).toBe(message)
34+
expect(messageAt(schema.safeParse({ action, [field]: 123 }), field)).toBe(
35+
'Invalid input: expected string, received number'
36+
)
37+
})
38+
39+
it('preserves decision validation and trimming', () => {
40+
expect(schema.safeParse({ action: 'apply', expectedFingerprint: '' }).success).toBe(false)
41+
expect(
42+
schema.safeParse({ action: 'apply', expectedFingerprint: 'x'.repeat(129) }).success
43+
).toBe(false)
44+
expect(schema.safeParse({ action: 'decline', reason: ' ' }).success).toBe(false)
45+
expect(schema.safeParse({ action: 'decline', reason: 'x'.repeat(1001) }).success).toBe(false)
46+
expect(schema.parse({ action: 'apply', expectedFingerprint: 'reviewed' })).toEqual({
47+
action: 'apply',
48+
expectedFingerprint: 'reviewed',
49+
})
50+
expect(schema.parse({ action: 'decline', reason: ' Not needed ' })).toEqual({
51+
action: 'decline',
52+
reason: 'Not needed',
53+
})
54+
})
55+
})
56+
1957
/**
2058
* A required field that is *omitted* and one that is *wrong-typed* are different
2159
* mistakes. Both used to answer with wording that pointed at the other: the

‎apps/sim/lib/billing/enterprise-provisioning.test.ts‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,195 @@ describe('Enterprise creation invitations', () => {
740740
})
741741
})
742742

743+
it.each(['admin', 'owner'] as const)(
744+
'recognizes inherited organization %s access without explicit workspace grants',
745+
async (role) => {
746+
const payload = operationPayload({
747+
request: {
748+
...operationPayload().request,
749+
workspaceIds: ['workspace-1', 'workspace-2'],
750+
},
751+
applicationResult: {
752+
appliedAt: '2026-08-13T00:00:00.000Z',
753+
subscriptionId: 'sub-1',
754+
},
755+
})
756+
queueTableRows(schemaMock.outboxEvent, [
757+
{ eventType: 'stripe.provision-enterprise', payload },
758+
])
759+
queueTableRows(schemaMock.outboxEvent, [])
760+
queueTableRows(schemaMock.outboxEvent, [{ status: 'completed' }, { status: 'completed' }])
761+
queueTableRows(
762+
schemaMock.user,
763+
['workspace-1', 'workspace-2'].map((workspaceId) => ({
764+
userId: 'invitee-1',
765+
workspaceId,
766+
role,
767+
permission: null,
768+
}))
769+
)
770+
const checkpointPayload = vi.fn()
771+
772+
await inviteEnterprisePeople(
773+
{
774+
provisioningOperationId: 'operation-1',
775+
organizationId: 'org-1',
776+
ownerUserId: 'owner-1',
777+
email: 'new@example.com',
778+
role: 'admin',
779+
permission: 'admin',
780+
sequence: 0,
781+
},
782+
{
783+
eventId: 'invite-1',
784+
eventType: 'enterprise.invite-people',
785+
attempts: 0,
786+
checkpointPayload,
787+
}
788+
)
789+
790+
expect(checkpointPayload).toHaveBeenCalledExactlyOnceWith({
791+
delivery: {
792+
completedAt: expect.any(String),
793+
resultId: 'invitee-1',
794+
outcome: 'unchanged',
795+
},
796+
})
797+
expect(mocks.createWorkspaceInvitation).not.toHaveBeenCalled()
798+
expect(mocks.prepareWorkspaceInvitationContext).not.toHaveBeenCalled()
799+
expect(mocks.sendInvitationEmail).not.toHaveBeenCalled()
800+
}
801+
)
802+
803+
it.each([
804+
{
805+
name: 'a concurrent promotion',
806+
role: 'admin',
807+
permission: null,
808+
requestedRole: 'member',
809+
workspaceIds: ['workspace-1'],
810+
applied: true,
811+
},
812+
{
813+
name: 'a sufficient explicit grant',
814+
role: 'member',
815+
permission: 'write',
816+
requestedRole: 'member',
817+
workspaceIds: ['workspace-1'],
818+
applied: true,
819+
},
820+
{
821+
name: 'an insufficient explicit grant',
822+
role: 'member',
823+
permission: 'read',
824+
requestedRole: 'member',
825+
workspaceIds: ['workspace-1'],
826+
applied: false,
827+
},
828+
{
829+
name: 'a workspace leaving the organization scope',
830+
role: null,
831+
permission: null,
832+
requestedRole: 'member',
833+
workspaceIds: ['workspace-1'],
834+
applied: false,
835+
},
836+
{
837+
name: 'a workspace admin grant without the requested organization admin role',
838+
role: 'member',
839+
permission: 'admin',
840+
requestedRole: 'admin',
841+
workspaceIds: ['workspace-1'],
842+
applied: false,
843+
},
844+
{
845+
name: 'inherited access to only one of two requested workspaces',
846+
role: 'admin',
847+
permission: null,
848+
requestedRole: 'member',
849+
workspaceIds: ['workspace-1', 'workspace-2'],
850+
applied: false,
851+
},
852+
] as const)(
853+
'checks the final effective access after $name',
854+
async ({ role, permission, requestedRole, workspaceIds, applied }) => {
855+
const payload = operationPayload({
856+
request: { ...operationPayload().request, workspaceIds: [...workspaceIds] },
857+
applicationResult: {
858+
appliedAt: '2026-08-13T00:00:00.000Z',
859+
subscriptionId: 'sub-1',
860+
},
861+
})
862+
queueTableRows(schemaMock.outboxEvent, [
863+
{ eventType: 'stripe.provision-enterprise', payload },
864+
])
865+
queueTableRows(schemaMock.outboxEvent, [])
866+
queueTableRows(
867+
schemaMock.outboxEvent,
868+
workspaceIds.map(() => ({ status: 'completed' }))
869+
)
870+
queueTableRows(schemaMock.user, [
871+
{ userId: 'invitee-1', workspaceId: 'workspace-1', role: 'member', permission: null },
872+
])
873+
queueTableRows(schemaMock.invitation, [])
874+
queueTableRows(schemaMock.user, [{ organizationId: 'org-1' }])
875+
queueTableRows(schemaMock.user, [
876+
{ id: 'owner-1', name: 'Owner', email: 'owner@example.com' },
877+
])
878+
queueTableRows(
879+
schemaMock.user,
880+
role ? [{ userId: 'invitee-1', workspaceId: 'workspace-1', role, permission }] : []
881+
)
882+
queueTableRows(schemaMock.invitation, [])
883+
mocks.createWorkspaceInvitation.mockResolvedValueOnce({
884+
id: 'invitee-1',
885+
instantAdd: true,
886+
outcome: 'unchanged',
887+
workspaceIds: [],
888+
})
889+
const checkpointPayload = vi.fn()
890+
const result = inviteEnterprisePeople(
891+
{
892+
provisioningOperationId: 'operation-1',
893+
organizationId: 'org-1',
894+
ownerUserId: 'owner-1',
895+
email: 'new@example.com',
896+
role: requestedRole,
897+
permission: 'write',
898+
sequence: 0,
899+
},
900+
{
901+
eventId: 'invite-1',
902+
eventType: 'enterprise.invite-people',
903+
attempts: 0,
904+
checkpointPayload,
905+
}
906+
)
907+
908+
if (applied) {
909+
await expect(result).resolves.toBeUndefined()
910+
expect(checkpointPayload).toHaveBeenLastCalledWith({
911+
delivery: {
912+
completedAt: expect.any(String),
913+
resultId: 'invitee-1',
914+
outcome: 'unchanged',
915+
},
916+
})
917+
} else {
918+
await expect(result).rejects.toThrow(
919+
'did not apply the requested organization role and workspace permissions'
920+
)
921+
expect(checkpointPayload).toHaveBeenCalledExactlyOnceWith({
922+
attemptedAt: expect.any(String),
923+
})
924+
}
925+
expect(mocks.createWorkspaceInvitation).toHaveBeenCalledExactlyOnceWith(
926+
expect.objectContaining({ existingAccessPolicy: 'ensure-at-least' })
927+
)
928+
expect(mocks.sendInvitationEmail).not.toHaveBeenCalled()
929+
}
930+
)
931+
743932
it('waits without consuming attempts until every selected workspace move completes', async () => {
744933
const payload = operationPayload({
745934
request: {

‎apps/sim/lib/billing/enterprise-provisioning.ts‎

Lines changed: 40 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2746,36 +2746,50 @@ async function resolveEnterpriseInvitationApplicationState(
27462746
workspaceIds: string[]
27472747
): Promise<EnterpriseInvitationApplicationState> {
27482748
const normalizedEmail = normalizeEmail(payload.email)
2749-
const [existingUser] = await db
2750-
.select({ id: user.id, organizationId: member.organizationId, role: member.role })
2749+
const accessRows = await db
2750+
.select({
2751+
userId: user.id,
2752+
workspaceId: workspace.id,
2753+
role: member.role,
2754+
permission: permissions.permissionType,
2755+
})
27512756
.from(user)
2752-
.leftJoin(member, eq(member.userId, user.id))
2753-
.where(eq(user.normalizedEmail, normalizedEmail))
2754-
.limit(1)
2755-
const roleSatisfied =
2756-
existingUser?.organizationId === payload.organizationId &&
2757-
(payload.role === 'member' || isOrgAdminRole(existingUser.role))
2758-
if (existingUser && roleSatisfied) {
2759-
const accessRows = await db
2760-
.select({ workspaceId: permissions.entityId, permission: permissions.permissionType })
2761-
.from(permissions)
2762-
.where(
2763-
and(
2764-
eq(permissions.entityType, 'workspace'),
2765-
eq(permissions.userId, existingUser.id),
2766-
inArray(permissions.entityId, workspaceIds)
2767-
)
2757+
.innerJoin(
2758+
member,
2759+
and(eq(member.userId, user.id), eq(member.organizationId, payload.organizationId))
2760+
)
2761+
.innerJoin(
2762+
workspace,
2763+
and(
2764+
eq(workspace.organizationId, member.organizationId),
2765+
inArray(workspace.id, workspaceIds),
2766+
isNull(workspace.archivedAt)
27682767
)
2769-
const accessByWorkspace = new Map(
2770-
accessRows.map((row) => [row.workspaceId, row.permission] as const)
27712768
)
2772-
if (
2773-
workspaceIds.every((workspaceId) =>
2774-
permissionSatisfies(accessByWorkspace.get(workspaceId), payload.permission)
2769+
.leftJoin(
2770+
permissions,
2771+
and(
2772+
eq(permissions.entityType, 'workspace'),
2773+
eq(permissions.userId, user.id),
2774+
eq(permissions.entityId, workspace.id)
27752775
)
2776-
) {
2777-
return { kind: 'applied', resultId: existingUser.id }
2778-
}
2776+
)
2777+
.where(eq(user.normalizedEmail, normalizedEmail))
2778+
const accessByWorkspace = new Map(accessRows.map((row) => [row.workspaceId, row] as const))
2779+
const existingUserId = accessRows[0]?.userId
2780+
if (
2781+
existingUserId &&
2782+
workspaceIds.every((workspaceId) => {
2783+
const access = accessByWorkspace.get(workspaceId)
2784+
if (!access) return false
2785+
const inheritsAdmin = isOrgAdminRole(access.role)
2786+
return (
2787+
(payload.role === 'member' || inheritsAdmin) &&
2788+
(inheritsAdmin || permissionSatisfies(access.permission, payload.permission))
2789+
)
2790+
})
2791+
) {
2792+
return { kind: 'applied', resultId: existingUserId }
27792793
}
27802794

27812795
const pendingRows = await db

0 commit comments

Comments
 (0)