fix(acl): resolve renamed Resource policy for parallel groups - #16996
Open
Ibochkarev wants to merge 1 commit into
Open
fix(acl): resolve renamed Resource policy for parallel groups#16996Ibochkarev wants to merge 1 commit into
Ibochkarev wants to merge 1 commit into
Conversation
Lookup core Access Policies by shipped name with a fail-closed template fallback so renaming Resource no longer breaks addParallelResourceGroup (modxcms#13831).
Ibochkarev
added
area-acl
bug
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.
What changed and why
Renaming the core Access Policy
Resourcebroke “create parallel resource group” on user group create: the wizard looked up the policy by the literal nameResource, returned null, and never wrotemodAccessResourceGrouprows (#13831).modAccessPolicy::getPolicy()resolves the shipped name first. If missing, and the policy maps 1:1 to a core template (Resource/Element/Context/Hidden Namespace) with exactly one policy on that template, it returns that policy. Multiple policies on the template fail closed (null).User/resource group create wizards now call
getPolicy()withPOLICY_*constants.Out of scope (per issue comments): making core policies read-only. Duplicate-and-customize remains the recommended workflow; this fix only unbreaks rename.
How to test
Resourceto another name.php -l core/src/Revolution/modAccessPolicy.php→ exit 0composer run-script phpunit -- --filter modAccessPolicyTest→ OK (3 tests, 9 assertions)Related issue(s)/PR(s)
Resolves #13831
Compatibility notes
Affects ACL wizards that previously hardcoded core policy names. Sites that never rename core policies behave as before. If the Resource policy was deleted and replaced with multiple custom policies on
ResourceTemplate, parallel-group ACL creation still fails closed (same as a missing policy).Breaking change assessment
No public method signatures removed. Adds
modAccessPolicy::getPolicy(). Safe for patch consumers.Test coverage
_build/test/Tests/Model/Security/modAccessPolicyTest.php: resolve by name, resolve after rename, fail closed when two policies shareResourceTemplate.Contributors
Thanks @intersel for the report and pinpointing
addParallelResourceGroup, @wshawn for the duplicate-vs-rename guidance, and @alroniks for confirming 3.x.AI tool use
Cursor agent assisted implementation, tests, review loops, and PR prose. Human review still required before merge.