fix(organizations): extend global-admin bypass to updateRole + isGlobalAdmin helper (#3512)

* fix(organizations): extend global-admin bypass to updateRole + isGlobalAdmin helper

Completes the platform-admin bypass started in #3509 which missed updateRole.
A global admin with no membership on the target org used to get a 403 on
PUT /api/organizations/:orgId/memberships/:memberId, blocking moderation
use-cases like transferring ownership.

Also extracts the repeated `Array.isArray(req.user?.roles) && r.includes('admin')`
expression into `lib/helpers/isGlobalAdmin.js` — single source of truth for the
three controller call-sites (remove-member, remove-org, updateRole). Policies
still inline the check; a broader refactor is out of scope here.

Closes #3510

* refactor(organizations): rename admin→isPlatformAdmin + clarify 403 message + test.each

Address review feedback on #3512:
- Rename local `admin` → `isPlatformAdmin` in all 3 controller call-sites to
  avoid confusion with the org-level `MEMBERSHIP_ROLES.ADMIN` used nearby.
- Update the updateRole 403 message to reflect the actual rule ("Only owners
  or global admins can change member roles").
- Refactor the two updateRole rejection regression tests with `test.each`.

* refactor(helpers): use function declaration for isGlobalAdmin to silence Codacy

Author: PierreBrisorgueil

MIGRATIONS.md

@@ -4,6 +4,36 @@ Breaking changes and upgrade notes for downstream projects.
 
 ---
 
+## Organizations: global admin bypass extended to updateRole + `isGlobalAdmin` helper (2026-04-24)
+
+Completes the platform-admin bypass started in #3509, and centralizes the repeated `Array.isArray(req.user?.roles) && req.user.roles.includes('admin')` check into a shared helper.
+
+### What changed
+
+- New helper `lib/helpers/isGlobalAdmin.js` — single source of truth for the global admin check used by moderation guards.
+- `modules/organizations/controllers/organizations.membership.controller.js` — `updateRole` now admits global admins who are not members of the target org (required to transfer ownership during moderation). `remove` now uses the shared helper.
+- `modules/organizations/controllers/organizations.controller.js` — `remove` now uses the shared helper (no behavior change).
+
+### Why
+
+`updateRole` had exactly the same buggy pattern that `remove` used to have before #3509: `if (!req.membership || req.membership.role !== OWNER)` rejected global admins with `req.membership === undefined` when they were not a member of the target org. The inline comment even said "Belt-and-suspenders: only owners (CASL blocks admins via no 'update Membership')" — the intent never anticipated platform admins. Same class of bug, same fix shape.
+
+While at it, the duplicated `isGlobalAdmin` expression across three call-sites was extracted into a helper. Policies (`organizations.policy.js`, `users.policy.js`, etc.) still inline the check for now — migrating them is out of scope here (wider refactor, different test surface).
+
+### Non-breaking
+
+- No contract changes for regular users / owners / non-global admins.
+- New capability: a user with `roles: ['admin']` can `PUT /api/organizations/:orgId/memberships/:memberId` without needing a membership on the target org.
+- Belt-and-suspenders guard is preserved: the handler still blocks non-owner, non-admin org roles regardless of CASL.
+
+### Action for downstream
+
+1. `/update-stack` pulls the change.
+2. No env var changes.
+3. No Mongo migration.
+
+---
+
 ## Auth signout endpoint (2026-04-23)
 
 New `POST /api/auth/signout` endpoint that clears the httpOnly `TOKEN` cookie on the client.

lib/helpers/isGlobalAdmin.js

@@ -0,0 +1,14 @@
+/**
+ * @desc True when the authenticated user has the global 'admin' role.
+ *   Use sparingly — prefer CASL abilities for per-resource checks. This
+ *   helper is only for belt-and-suspenders guards inside controllers that
+ *   need to bypass org-scoped membership checks (moderation endpoints).
+ * @param {Object} user - req.user object (may be undefined)
+ * @returns {boolean}
+ */
+function isGlobalAdmin(user) {
+  if (!user || !Array.isArray(user.roles)) return false;
+  return user.roles.includes('admin');
+}
+
+export default isGlobalAdmin;

lib/helpers/tests/isGlobalAdmin.unit.tests.js

@@ -0,0 +1,43 @@
+/**
+ * Unit tests for isGlobalAdmin helper.
+ */
+import { describe, it, expect } from '@jest/globals';
+import isGlobalAdmin from '../isGlobalAdmin.js';
+
+describe('isGlobalAdmin', () => {
+  it('should return false for undefined user', () => {
+    expect(isGlobalAdmin(undefined)).toBe(false);
+  });
+
+  it('should return false for null user', () => {
+    expect(isGlobalAdmin(null)).toBe(false);
+  });
+
+  it('should return false when user has no roles property', () => {
+    expect(isGlobalAdmin({ _id: 'u1' })).toBe(false);
+  });
+
+  it('should return false when user roles is not an array', () => {
+    expect(isGlobalAdmin({ roles: 'admin' })).toBe(false);
+  });
+
+  it('should return false when user roles is empty', () => {
+    expect(isGlobalAdmin({ roles: [] })).toBe(false);
+  });
+
+  it('should return false when roles contains only user', () => {
+    expect(isGlobalAdmin({ roles: ['user'] })).toBe(false);
+  });
+
+  it('should return false when roles contains lookalikes but not admin', () => {
+    expect(isGlobalAdmin({ roles: ['user', 'superadmin', 'Admin'] })).toBe(false);
+  });
+
+  it('should return true when roles includes admin alongside user', () => {
+    expect(isGlobalAdmin({ roles: ['user', 'admin'] })).toBe(true);
+  });
+
+  it('should return true when roles is exactly [admin]', () => {
+    expect(isGlobalAdmin({ roles: ['admin'] })).toBe(true);
+  });
+});

modules/organizations/controllers/organizations.controller.js

@@ -4,6 +4,7 @@
 import jwt from 'jsonwebtoken';
 import errors from '../../../lib/helpers/errors.js';
 import responses from '../../../lib/helpers/responses.js';
+import isGlobalAdmin from '../../../lib/helpers/isGlobalAdmin.js';
 import config from '../../../config/index.js';
 import mailer from '../../../lib/helpers/mailer/index.js';
 import policy from '../../../lib/middlewares/policy.js';
@@ -107,9 +108,10 @@ const remove = async (req, res) => {
     // 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');
+    // Note: `isPlatformAdmin` is the global/platform role, distinct from org-level admin.
+    const isPlatformAdmin = isGlobalAdmin(req.user);
     const isMemberOfTarget = !!req.membership;
-    if (!isGlobalAdmin && isMemberOfTarget) {
+    if (!isPlatformAdmin && 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')();

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

@@ -3,6 +3,7 @@
  */
 import errors from '../../../lib/helpers/errors.js';
 import responses from '../../../lib/helpers/responses.js';
+import isGlobalAdmin from '../../../lib/helpers/isGlobalAdmin.js';
 import MembershipService from '../services/organizations.membership.service.js';
 import { MEMBERSHIP_ROLES } from '../lib/constants.js';
 
@@ -34,9 +35,14 @@ const list = async (req, res) => {
  */
 const updateRole = async (req, res) => {
   try {
-    // Belt-and-suspenders: only owners can change roles (CASL blocks admins via no 'update Membership')
-    if (!req.membership || req.membership.role !== MEMBERSHIP_ROLES.OWNER) {
-      return responses.error(res, 403, 'Forbidden', 'Only owners can change member roles')();
+    // Belt-and-suspenders: only org owners can change roles (CASL blocks non-owner org
+    // roles via no 'update Membership'). Global platform admins bypass the membership
+    // requirement for moderation — notably to transfer ownership on a third-party org.
+    // Note: `isPlatformAdmin` is the global/platform role (`user.roles.includes('admin')`),
+    // distinct from the org-level `MEMBERSHIP_ROLES.ADMIN`.
+    const isPlatformAdmin = isGlobalAdmin(req.user);
+    if (!isPlatformAdmin && (!req.membership || req.membership.role !== MEMBERSHIP_ROLES.OWNER)) {
+      return responses.error(res, 403, 'Forbidden', 'Only owners or global admins can change member roles')();
     }
     const membership = await MembershipService.updateRole(req.membershipDoc, req.body.role);
     responses.success(res, 'membership updated')(membership);
@@ -56,10 +62,12 @@ const remove = async (req, res) => {
   try {
     // 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');
+    // Note: `isPlatformAdmin` is the global/platform role, distinct from the
+    // org-level `MEMBERSHIP_ROLES.ADMIN` referenced below.
+    const isPlatformAdmin = isGlobalAdmin(req.user);
     const actorRole = req.membership?.role;
     const targetRole = req.membershipDoc.role;
-    const canRemove = isGlobalAdmin
+    const canRemove = isPlatformAdmin
       || actorRole === MEMBERSHIP_ROLES.OWNER
       || (actorRole === MEMBERSHIP_ROLES.ADMIN && targetRole === MEMBERSHIP_ROLES.MEMBER);
     if (!canRemove) {

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

@@ -89,6 +89,41 @@ describe('Membership controller unit tests:', () => {
       expect(mockUpdateRole).toHaveBeenCalledTimes(1);
       expect(res.status).not.toHaveBeenCalledWith(403);
     });
+
+    test('should allow global admin with no org membership to change a role', async () => {
+      const updatedMembership = { id: 'mem1', role: MEMBERSHIP_ROLES.OWNER };
+      mockUpdateRole.mockResolvedValue(updatedMembership);
+
+      const req = mockReq({
+        user: { _id: 'adm', roles: ['user', 'admin'] },
+        membership: undefined,
+        body: { role: MEMBERSHIP_ROLES.OWNER },
+      });
+      const res = mockRes();
+
+      await membershipController.updateRole(req, res);
+
+      expect(mockUpdateRole).toHaveBeenCalledTimes(1);
+      expect(mockUpdateRole).toHaveBeenCalledWith(req.membershipDoc, MEMBERSHIP_ROLES.OWNER);
+      expect(res.status).not.toHaveBeenCalledWith(403);
+    });
+
+    test.each([
+      ['non-admin non-owner (member)', { role: MEMBERSHIP_ROLES.MEMBER }, MEMBERSHIP_ROLES.ADMIN],
+      ['org-level admin (not global)', { role: MEMBERSHIP_ROLES.ADMIN }, MEMBERSHIP_ROLES.OWNER],
+    ])('should still reject a %s trying to change a role', async (_label, membership, targetRole) => {
+      const req = mockReq({
+        user: { _id: 'u1', roles: ['user'] },
+        membership,
+        body: { role: targetRole },
+      });
+      const res = mockRes();
+
+      await membershipController.updateRole(req, res);
+
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(mockUpdateRole).not.toHaveBeenCalled();
+    });
   });
 
   describe('remove', () => {