fix(organizations): allow global admin to bypass org-level guards (#3509)

Two controller guards were blocking platform admins from moderating
organizations they don't belong to:

- remove-member refused because req.membership was undefined, so the
  actor-role check fell through to a 403 regardless of the global role.
- delete-org always called listByUser(req.user) and bailed with 422
  "cannot delete your last organization" for any admin with zero or one
  own memberships.

Short-circuit both to "allow" when req.user.roles includes "admin".
The delete-org UX protection is preserved for regular members (only
skipped when the actor is not in the target org, or is a global admin),
so a normal user deleting their own last org still sees the 422.

Tests: extend organizations.membership.controller.unit to cover admin
bypass with membership=undefined and admin removing an owner. Add
organizations.controller.unit for the delete-org paths (regular last
org rejected, multi-org member allowed, admin with zero/one memberships
allowed).

Closes #3505

Author: PierreBrisorgueil

modules/organizations/controllers/organizations.controller.js

@@ -104,10 +104,16 @@ const update = async (req, res) => {
  */
 const remove = async (req, res) => {
   try {
-    // Prevent deleting the user's last organization
-    const userMemberships = await MembershipService.listByUser(req.user._id || req.user.id);
-    if (userMemberships.length <= 1) {
-      return responses.error(res, 422, 'Unprocessable Entity', 'You cannot delete your last organization')();
+    // UX protection: prevent a regular user from deleting their own last organization.
+    // Global platform admins bypass this entirely (moderation); a member of multiple orgs
+    // is also safe to delete the current one since they keep at least one membership.
+    const isGlobalAdmin = Array.isArray(req.user?.roles) && req.user.roles.includes('admin');
+    const isMemberOfTarget = !!req.membership;
+    if (!isGlobalAdmin && isMemberOfTarget) {
+      const userMemberships = await MembershipService.listByUser(req.user._id || req.user.id);
+      if (userMemberships.length <= 1) {
+        return responses.error(res, 422, 'Unprocessable Entity', 'You cannot delete your last organization')();
+      }
     }
     const result = await OrganizationsService.remove(req.organization);
     responses.success(res, 'organization deleted')({ id: req.organization.id, ...result });

modules/organizations/controllers/organizations.membership.controller.js

@@ -54,10 +54,13 @@ const updateRole = async (req, res) => {
  */
 const remove = async (req, res) => {
   try {
-    // Only owners can remove anyone; admins can only remove members
+    // Only owners can remove anyone; admins can only remove members.
+    // Global platform admins bypass org-level RBAC for moderation needs.
+    const isGlobalAdmin = Array.isArray(req.user?.roles) && req.user.roles.includes('admin');
     const actorRole = req.membership?.role;
     const targetRole = req.membershipDoc.role;
-    const canRemove = actorRole === MEMBERSHIP_ROLES.OWNER
+    const canRemove = isGlobalAdmin
+      || actorRole === MEMBERSHIP_ROLES.OWNER
       || (actorRole === MEMBERSHIP_ROLES.ADMIN && targetRole === MEMBERSHIP_ROLES.MEMBER);
     if (!canRemove) {
       return responses.error(res, 403, 'Forbidden', 'Insufficient permissions to remove this member')();

modules/organizations/tests/organizations.controller.unit.tests.js

@@ -0,0 +1,120 @@
+/**
+ * Module dependencies.
+ */
+import { jest, describe, test, expect, beforeEach } from '@jest/globals';
+
+const mockCrudRemove = jest.fn();
+const mockListByUser = jest.fn();
+
+jest.unstable_mockModule('../services/organizations.crud.service.js', () => ({
+  default: {
+    remove: mockCrudRemove,
+  },
+}));
+
+jest.unstable_mockModule('../services/organizations.membership.service.js', () => ({
+  default: {
+    listByUser: mockListByUser,
+  },
+}));
+
+jest.unstable_mockModule('../../../lib/services/analytics.js', () => ({
+  default: {
+    groupIdentify: jest.fn(),
+  },
+}));
+
+const { default: organizationsController } = await import('../controllers/organizations.controller.js');
+
+/**
+ * Unit tests for the organizations controller remove handler.
+ */
+describe('Organizations controller unit tests:', () => {
+  /**
+   * @desc Build a minimal Express-like req object
+   * @param {Object} overrides
+   * @returns {Object} mock request
+   */
+  function mockReq(overrides = {}) {
+    return {
+      user: { _id: 'u1', id: 'u1', roles: ['user'] },
+      organization: { _id: 'org1', id: 'org1' },
+      membership: { role: 'owner' },
+      ...overrides,
+    };
+  }
+
+  /**
+   * @desc Build a minimal Express-like res object with spies
+   * @returns {Object} mock response
+   */
+  function mockRes() {
+    const res = {};
+    res.status = jest.fn().mockReturnValue(res);
+    res.json = jest.fn().mockReturnValue(res);
+    return res;
+  }
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe('remove', () => {
+    test('should reject when a regular member tries to delete their own last organization', async () => {
+      mockListByUser.mockResolvedValue([{ _id: 'mem1' }]);
+      const req = mockReq();
+      const res = mockRes();
+
+      await organizationsController.remove(req, res);
+
+      expect(mockListByUser).toHaveBeenCalledWith('u1');
+      expect(res.status).toHaveBeenCalledWith(422);
+      expect(mockCrudRemove).not.toHaveBeenCalled();
+    });
+
+    test('should allow a regular member to delete when they belong to several organizations', async () => {
+      mockListByUser.mockResolvedValue([{ _id: 'mem1' }, { _id: 'mem2' }]);
+      mockCrudRemove.mockResolvedValue({ success: true });
+
+      const req = mockReq();
+      const res = mockRes();
+
+      await organizationsController.remove(req, res);
+
+      expect(mockListByUser).toHaveBeenCalledTimes(1);
+      expect(mockCrudRemove).toHaveBeenCalledTimes(1);
+      expect(res.status).not.toHaveBeenCalledWith(422);
+    });
+
+    test('should allow a global admin with zero memberships to delete any organization', async () => {
+      mockCrudRemove.mockResolvedValue({ success: true });
+      const req = mockReq({
+        user: { _id: 'adm', id: 'adm', roles: ['user', 'admin'] },
+        membership: undefined,
+      });
+      const res = mockRes();
+
+      await organizationsController.remove(req, res);
+
+      expect(mockListByUser).not.toHaveBeenCalled();
+      expect(mockCrudRemove).toHaveBeenCalledTimes(1);
+      expect(res.status).not.toHaveBeenCalledWith(422);
+    });
+
+    test('should allow a global admin who is a member of the target org with only one membership', async () => {
+      mockCrudRemove.mockResolvedValue({ success: true });
+      const req = mockReq({
+        user: { _id: 'adm', id: 'adm', roles: ['admin'] },
+        membership: { role: 'owner' },
+      });
+      const res = mockRes();
+
+      await organizationsController.remove(req, res);
+
+      // Admin skips the last-org UX check entirely.
+      expect(mockListByUser).not.toHaveBeenCalled();
+      expect(mockCrudRemove).toHaveBeenCalledTimes(1);
+      expect(res.status).not.toHaveBeenCalledWith(422);
+    });
+  });
+});

modules/organizations/tests/organizations.membership.controller.unit.tests.js

@@ -33,6 +33,7 @@ describe('Membership controller unit tests:', () => {
     return {
       query: {},
       body: {},
+      user: { _id: 'u1', roles: ['user'] },
       organization: { _id: 'org1' },
       membership: { role: MEMBERSHIP_ROLES.OWNER },
       membershipDoc: { id: 'mem1', role: MEMBERSHIP_ROLES.MEMBER, organizationId: 'org1' },
@@ -154,5 +155,37 @@ describe('Membership controller unit tests:', () => {
       expect(mockRemove).toHaveBeenCalledTimes(1);
       expect(res.status).not.toHaveBeenCalledWith(403);
     });
+
+    test('should allow global admin with no org membership to remove any member', async () => {
+      mockRemove.mockResolvedValue({ success: true });
+
+      const req = mockReq({
+        user: { _id: 'adm', roles: ['user', 'admin'] },
+        membership: undefined,
+        membershipDoc: { id: 'mem2', role: MEMBERSHIP_ROLES.OWNER },
+      });
+      const res = mockRes();
+
+      await membershipController.remove(req, res);
+
+      expect(mockRemove).toHaveBeenCalledTimes(1);
+      expect(res.status).not.toHaveBeenCalledWith(403);
+    });
+
+    test('should allow global admin to remove an owner even without being in the org', async () => {
+      mockRemove.mockResolvedValue({ success: true });
+
+      const req = mockReq({
+        user: { _id: 'adm', roles: ['admin'] },
+        membership: undefined,
+        membershipDoc: { id: 'mem2', role: MEMBERSHIP_ROLES.OWNER },
+      });
+      const res = mockRes();
+
+      await membershipController.remove(req, res);
+
+      expect(mockRemove).toHaveBeenCalledTimes(1);
+      expect(res.status).not.toHaveBeenCalledWith(403);
+    });
   });
 });