You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Two org-management controller guards currently treat global platform admins as plain users and block them from moderating organizations they don't belong to:
organizations.membership.controller.js#remove — 403 when req.membership is undefined (admin not in the org), regardless of req.user.roles.
organizations.controller.js#remove — 422 "cannot delete your last organization" whenever listByUser(req.user).length <= 1, even for admins touching someone else's org.
Both now short-circuit to allow when req.user.roles includes admin. The UX protection on org delete is preserved for regular users (only skipped when the actor is not a member of the target org, or is a global admin).
The issue mentions a concern about cascade-delete integrity (dangling user.currentOrganization refs when an org is deleted by someone who isn't a member). Per the ticket, that is intentionally not bundled here — this PR stays narrowly the admin-bypass fix. A follow-up issue should be filed if the audit of the delete handler shows a real gap.
Test plan
npm run test:unit — 550 passed
Organizations integration + e2e + unit in isolation — 24 passed
npm run lint — clean
New unit tests cover:
admin with membership=undefined can remove any member, including owners
admin with zero memberships can delete any org (UX check skipped)
admin who is a member of the target org with memberships.length <= 1 can still delete (UX check skipped)
non-admin member with one membership still gets 422 (regression guard)
non-admin member with several memberships can delete their current org (regression guard)
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
@PierreBrisorgueil has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 42 minutes and 55 seconds before requesting another review.
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 55 seconds.
⌛ How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.
We recommend that you space out your commits to avoid hitting the rate limit.
🚦 How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.67%. Comparing base (c93a577) to head (266b1b2). ⚠️ Report is 1 commits behind head on master.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Fixes org-management delete/remove controller guards so global platform admins (roles includes admin) aren’t incorrectly blocked when they’re not members of the target organization, while preserving the “don’t delete your last org” UX guard for regular org members.
Changes:
Allow global admins to remove organization members even when req.membership is missing (admin is not a member of that org).
Scope the “cannot delete your last organization” guard to non-admin actors who are members of the target org.
Add unit tests covering admin bypass and non-admin last-org protection behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR successfully implements the bypass for global admins to delete organizations and remove members regardless of their membership status, addressing the core issue. Codacy results indicate the project remains up to standards; however, a functional gap exists: the updateRole function remains restricted, preventing platform admins from performing critical moderation tasks like transferring ownership. Additionally, there is a significant increase in code clones (16), primarily due to the repeated logic used to verify the 'admin' role across controllers. This duplication should be addressed to ensure future maintainability and consistency in how platform-level permissions are enforced.
About this PR
The pattern for checking global admin status (Array.isArray(req.user?.roles) && req.user.roles.includes('admin')) is now duplicated across several controllers (Organizations, Tasks, Users). This logic should be centralized into an authorization middleware or a helper on the request object to improve maintainability.
Test suggestions
A regular user with exactly one membership attempts to delete their organization and receives a 422 error.
A regular user with multiple memberships successfully deletes their current organization.
A global admin with no memberships in the target organization successfully deletes the organization.
A global admin who is a member of only the target organization successfully deletes it.
A global admin with no membership in an organization successfully removes an owner from that organization.
…alAdmin 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Two org-management controller guards currently treat global platform admins as plain users and block them from moderating organizations they don't belong to:
organizations.membership.controller.js#remove— 403 whenreq.membershipis undefined (admin not in the org), regardless ofreq.user.roles.organizations.controller.js#remove— 422 "cannot delete your last organization" wheneverlistByUser(req.user).length <= 1, even for admins touching someone else's org.Both now short-circuit to allow when
req.user.rolesincludesadmin. The UX protection on org delete is preserved for regular users (only skipped when the actor is not a member of the target org, or is a global admin).Closes #3505.
Scope / intentionally excluded
The issue mentions a concern about cascade-delete integrity (dangling
user.currentOrganizationrefs when an org is deleted by someone who isn't a member). Per the ticket, that is intentionally not bundled here — this PR stays narrowly the admin-bypass fix. A follow-up issue should be filed if the audit of the delete handler shows a real gap.Test plan
npm run test:unit— 550 passednpm run lint— cleanmembership=undefinedcan remove any member, including ownersmemberships.length <= 1can still delete (UX check skipped)