Prevent privilege escalation through role creation/update/assignment - #26914
Draft
dennisoelkers wants to merge 11 commits into
Draft
Prevent privilege escalation through role creation/update/assignment#26914dennisoelkers wants to merge 11 commits into
dennisoelkers wants to merge 11 commits into
Conversation
A user holding only `users:create` was able to create another user with more permissions than they hold themselves, either by supplying the permissions directly or by assigning a role that grants them. `UsersResource#create` now collects the permissions requested for the new user, resolves the permissions granted by the requested roles, and refuses the request with a `BadRequestException` if the current user is missing at least one of them. Reading the requested roles additionally requires `roles:read` on each role.
Cover `UsersResource#changeUser` with the two escalation cases that were only tested for user creation so far: assigning a permission the editing user does not hold, and assigning a role that grants such a permission. In both cases the editing user is granted `users:permissionsedit` / `users:rolesedit` on the target, so the submitted fields would actually be applied and the rejection can only come from the escalation check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gning-permissions-current-user-lacks
dennisoelkers
added a commit
that referenced
this pull request
Aug 6, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reuse PermissionsValidator in RolesResource so that a user cannot grant
permissions they do not hold themselves:
- `POST /roles` validates the permissions of the new role.
- `PUT /roles/{rolename}` validates the permissions of the updated role.
- `PUT /roles/{rolename}/members/{username}` validates the permissions
granted by the role before assigning it.
Previously `roles:create`/`roles:edit`/`roles:assign` were sufficient to
mint or assign a role granting arbitrary permissions, which bypassed the
equivalent checks already added to the user endpoints.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers all three validated endpoints in RolesResource, for both the allowed and the rejected case, plus the wildcard edge cases: - holding `streams:*` allows granting `streams:read` - holding `streams:read:12345` does NOT allow granting `streams:read` - removing a member stays unvalidated, since un-assigning a role reduces privileges The tests use a real PermissionsValidator so they exercise the actual Shiro permission resolution rather than asserting on a mock. Verified by mutation: commenting out the three validatePermissions calls fails 7 of these tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dennisoelkers
force-pushed
the
security/block-privilege-escalation-via-roles
branch
from
August 6, 2026 12:29
439c921 to
bf81255
Compare
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
1 participant
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.
Note: This is based on #26883, which needs to be merged before.
Note: This needs a backport to previous, supported versions.
Description
Motivation and Context
This PR is extending role creation/updating/assignment by a check that verifies that the current user holds (at least) the same permissions that the role that is created/updated/assigned contains. This is supposed to prevent privilege escalation through creating a role with more permissions that the current user has and assigning it to the current or another user. Same for updating an existing role with elevated privileges, which is already assigned to the current/other users.
For this, we reuse the
PermissionsValidatorinRolesResourceso that a user cannot grant permissions they do not hold themselves:POST /rolesvalidates the permissions of the new role.PUT /roles/{rolename}validates the permissions of the updated role.PUT /roles/{rolename}/members/{username}validates the permissions granted by the role before assigning it.Previously
roles:create/roles:edit/roles:assignwere sufficient to mint or assign a role granting arbitrary permissions, which bypassed the equivalent checks already added to the user endpoints.How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: