Repository navigation
fix(organization): stop admins granting or removing roles above their own - #122
Merged
Merged
Conversation
… own The membership and invitation writes required org admin and then took the role from the request body unchecked. An admin could therefore: - invite an alias as owner (POST /orgs/:orgId/invitations), - add an existing account as owner (POST /orgs/:orgId/members), - remove the owner (DELETE /orgs/:orgId/members/:memberId). UpdateMember already required owner; these did not. Granting a role and removing a member are now bounded by the caller's own rank, using the existing orgRoleRank: only an owner can make or remove an owner, while admins can still grant admin and below and remove admins and members. Built-in role names are matched case-insensitively and trimmed, so "Owner" cannot rank as an unknown role while meaning owner to a consumer that compares loosely. Free-form roles (e.g. "viewer") still pass through.
golangci-lint's govet shadow check flagged the rank-check error in handleCreateInvitation shadowing the handler's err; the same pattern was in handleAddMember and handleRemoveMember.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Problem
Three organization-plugin writes require only org admin, then take the role from the request body without checking it. Any admin can therefore:
POST /orgs/:orgId/invitationsPOST /orgs/:orgId/membersDELETE /orgs/:orgId/members/:memberIdPATCH /orgs/:orgId/members/:memberId(role change) already requires owner, so the same outcome was reachable through the other three.Found while auditing Kineta's role-escalation paths. Kineta now blocks these routes on its side, but every other consumer of the plugin is exposed.
Fix
Bound both writes by the caller's own rank, using the existing
orgRoleRank(owner 3, admin 2, member 1):AddMember,CreateInvitation): the requested role can't outrank the caller's. Only an owner can make an owner; admins can still grant admin and below.RemoveMember): you can't remove a member who outranks you. Admins can still remove admins and members, and owners can remove owners.The built-in role names are normalised (case-insensitive, trimmed). Without that,
"Owner"ranks as an unknown role (0) and slips past the check, while any consumer comparing roles loosely still treats it as owner. Free-form roles that apps use (e.g.viewer) pass through unchanged and outrank no one.Tests
plugins/organization/role_rank_test.go, through the real HTTP routes. Each escalation test failed onmainbefore the fix:owner,Owner,OWNER): was 201, now 403They also pin what must keep working: an admin invites admin, member,
vieweror the default; an admin adds an admin; an admin removes a member or an admin; an owner invites an owner; an owner removes a co-owner.go test ./...green;gofmtandgo vetclean. Lint is left to CI, since it runs golangci-lint v2.🤖 Generated with Claude Code