Skip to content

Commit 2ca78d2

Browse files
waleedlatif1waleedlatif1
committed
fix(organizations): verify effective access across invitation flows
1 parent e619b30 commit 2ca78d2

4 files changed

Lines changed: 276 additions & 34 deletions

File tree

‎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

‎apps/sim/lib/invitations/workspace-invitations.test.ts‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -267,11 +267,14 @@ describe('createWorkspaceInvitation', () => {
267267
it.each(['admin', 'owner'] as const)(
268268
'rejects inviting an organization %s who already inherits workspace access',
269269
async (role) => {
270-
queueWhereResponses([
271-
[{ id: 'user-2', email: 'member@example.com' }],
272-
[{ workspaceId: 'ws-1', permission: 'read' }],
273-
])
274-
mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-1', role })
270+
queueTableRows(userTable, [{ id: 'user-2', email: 'member@example.com' }])
271+
queueTableRows(member, [{ role: 'owner' }])
272+
queueTableRows(member, [{ role }])
273+
mockGetUserOrganization.mockResolvedValueOnce({
274+
organizationId: 'org-1',
275+
memberId: 'member-2',
276+
role,
277+
})
275278

276279
await expect(
277280
createWorkspaceInvitation({
@@ -286,6 +289,41 @@ describe('createWorkspaceInvitation', () => {
286289
expect(mockCreatePendingInvitation).not.toHaveBeenCalled()
287290
expect(mockSendInvitationEmail).not.toHaveBeenCalled()
288291
expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
292+
expect(mockAcquireOrganizationUserMutationLocks).toHaveBeenCalledOnce()
293+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
294+
}
295+
)
296+
297+
it.each(['member', 'admin'] as const)(
298+
'preserves current membership after a concurrent demotion in ordinary %s invitations',
299+
async (membership) => {
300+
queueTableRows(userTable, [{ id: 'user-2' }])
301+
queueTableRows(member, [{ role: 'member' }])
302+
queueTableRows(member, [{ role: 'member' }])
303+
mockGetUserOrganization.mockResolvedValueOnce({
304+
organizationId: 'org-1',
305+
memberId: 'member-2',
306+
role: 'admin',
307+
})
308+
309+
const result = await createWorkspaceInvitation({
310+
context: makeContext(),
311+
email: 'member@example.com',
312+
permission: 'write',
313+
membership,
314+
request,
315+
})
316+
317+
expect(result).toMatchObject({ outcome: 'added', workspaceIds: ['ws-1'] })
318+
expect(mockAcquireOrganizationUserMutationLocks.mock.invocationCallOrder[0]).toBeLessThan(
319+
mockGrantWorkspaceAccessDirectly.mock.invocationCallOrder[0]
320+
)
321+
expect(mockGrantWorkspaceAccessDirectly).toHaveBeenCalledExactlyOnceWith(
322+
expect.objectContaining({ userId: 'user-2', existingPermissionPolicy: 'preserve' })
323+
)
324+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
325+
expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
326+
expect(mockCreatePendingInvitation).not.toHaveBeenCalled()
289327
}
290328
)
291329

‎apps/sim/lib/invitations/workspace-invitations.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -577,18 +577,19 @@ export async function createWorkspaceInvitation({
577577
let existingOrganizationRole = existingMembership?.role
578578
let organizationRoleUpdated = false
579579
if (
580-
existingAccessPolicy === 'ensure-at-least' &&
581580
existingUser &&
582581
organizationId &&
583-
existingMembership?.organizationId === organizationId
582+
existingMembership?.organizationId === organizationId &&
583+
(existingAccessPolicy === 'ensure-at-least' || isOrgAdminRole(existingMembership.role))
584584
) {
585585
const ensuredRole = await ensureExistingMemberOrganizationRole({
586586
context,
587587
organizationId,
588588
memberId: existingMembership.memberId,
589589
userId: existingUser.id,
590590
currentRole: existingMembership.role,
591-
requestedRole: membership === 'admin' ? 'admin' : 'member',
591+
requestedRole:
592+
existingAccessPolicy === 'ensure-at-least' && membership === 'admin' ? 'admin' : 'member',
592593
email: normalizedEmail,
593594
request,
594595
validateLockedWorkspace,

0 commit comments

Comments
 (0)