fix(organizations): recheck inherited access under invitation locks

Author: waleedlatif1

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](
       {

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' &&

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

@@ -292,8 +292,14 @@ describe('createWorkspaceInvitation', () => {
   it.each(['admin', 'owner'] as const)(
     'leaves inherited access unchanged when ensuring access for an organization %s',
     async (role) => {
-      queueWhereResponses([[{ id: 'user-2', email: 'member@example.com' }], []])
-      mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-1', 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']),
@@ -318,41 +324,189 @@ describe('createWorkspaceInvitation', () => {
     }
   )
 
-  it('reports a promotion as updated without granting redundant workspace permissions', async () => {
+  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: 'owner' }])
+    queueTableRows(member, [{ role: 'member' }])
     queueTableRows(member, [{ role: 'member' }])
     mockGetUserOrganization.mockResolvedValueOnce({
       organizationId: 'org-1',
       memberId: 'member-2',
-      role: 'member',
+      role: 'admin',
     })
 
     const result = await createWorkspaceInvitation({
-      context: makeContext(['ws-1', 'ws-2']),
+      context: makeContext(),
       email: 'member@example.com',
+      membership: 'member',
       permission: 'write',
-      membership: 'admin',
       existingAccessPolicy: 'ensure-at-least',
-      request,
     })
 
     expect(result).toMatchObject({
-      workspaceIds: [],
+      outcome: 'added',
+      workspaceIds: ['ws-1'],
       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' })
+    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) => {