[6.x] Hide permissions covered by a broader permission#14970
Merged
Conversation
Permissions can declare a broader permission that supersedes them. The roles UI hides superseded permissions while the superseder is checked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers hiding a superseded permission (and its children) once the superseding permission is checked, and leaving it visible otherwise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When creating an entry, the ability was resolved from the raw
"publish {collection} entries" permission, which sidesteps the policy.
Users able to configure collections were denied the publish state
toggle, despite the policy granting them everything.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The configure permissions already grant every ability their policies guard, so the per-item permissions they cover no longer need to be shown in the roles UI alongside them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hiding is purely visual, so name it for what it does rather than implying the hidden permission has been replaced. A permission may now be hidden by any one of several permissions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The roles UI hides permissions that a configure permission covers, so the coverage needs pinning down. Entries and terms had no such tests, and globals only covered creating and deleting variables. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Unlike the other configure permissions, "configure addons" governs a different resource to the addon settings permissions it would hide. Whether it should grant access to every addon's settings is a separate question to this one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The tree is self-recursive, so there is only one call site to thread it through. A prop keeps the component standalone and makes the data flow visible, rather than reaching for an ancestor's provide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The children of a permission with replacements are rebuilt when the tree is generated, which dropped their hiddenBy. Core only declares it on top level permissions, but addons may declare it anywhere. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Permissions with replacements, and their children, are rebuilt when the tree is generated, which dropped their descriptions. The publish and reorder entry permissions have never shown theirs in the roles UI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The roles form needs to know which permissions are visible too, so the predicate shouldn't live in the tree component. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The buttons walked the whole tree, so a group could read as "Check All" with every visible box already ticked, and checking would grant permissions the user never saw. Checking now only affects visible permissions, using the checked state from before anything is mutated. Unchecking clears the group outright, since unchecking a broader permission reveals the ones it was hiding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Permissions may now declare one or more permissions that hide them in the roles UI:
When any of the hiding permissions are checked, the permission and its children disappear from the roles UI. Unchecking brings them back, with their checked state intact.
This is purely visual. It doesn't grant anything, and it doesn't establish a hierarchy between permissions. It just avoids showing redundant checkboxes, cleaning up the UI.
The concept is cherry-picked from #14957, where it was introduced as
supersededByas part of Forms 2. It's extracted here so it can land on its own, renamed, and widened to accept multiple permissions.Core permissions
The
configurepermissions already grant every ability their policies guard, eg. configure collections -> view {collection} entries.Fixes coming along for the ride
Publish state on new entries
EntriesController::create()resolvedcanManagePublishStatefrom the rawpublish {collection} entriespermission, which sidestepsEntryPolicyand falls through toGate::after. Users able to configure collections were denied the publish state toggle, despite the policy granting them everything. It now goes through the policy, matching the edit page.Permission descriptions
Permissions using
replacements(), and their children, are rebuilt when the tree is generated. That rebuild never copied the description across, sopublish {collection} entriesandreorder {collection} entrieshave never shown theirs in the roles UI, despite having one inlang/en/permissions.php. Fixed alongside the same bug affectinghiddenBy.Tests
Nothing asserted that the
configurepermissions actually grant the abilities behind the permissions we're now hiding. Addeduser_with_configure_permission_can_do_it_alltoEntryPolicyTestandTermPolicyTest, and view/edit coverage toGlobalSetVariablesPolicyTest. Navs and assets were already covered.