Skip to content

fix: scope the formit permission to its own policy template#314

Open
Ibochkarev wants to merge 1 commit into
Sterc:3.xfrom
Ibochkarev:fix/permissions-resolver-template-scope
Open

fix: scope the formit permission to its own policy template#314
Ibochkarev wants to merge 1 commit into
Sterc:3.xfrom
Ibochkarev:fix/permissions-resolver-template-scope

Conversation

@Ibochkarev

Copy link
Copy Markdown

permissions.resolver.php creates the formit and formit_encryptions modAccessPermission bits only on templates listed in each permission's templates key (AdministratorTemplate), which is correct.

The second loop, which writes the actual true/false value into every modAccessPolicy's data blob, never checked that same templates restriction. Any permission without an explicit policies allowlist (formit) fell into the unconditional $data[$permission['name']] = true branch for every policy in the system, including ones built from unrelated templates like ElementTemplate. That's the "10 of 9 active policies" the issue describes.

Reproduced both the bug and the fix with a standalone script stubbing xPDO's getCollection()/getObject()/newObject() against the resolver's actual logic: with a policy on AdministratorTemplate and one on an unrelated ElementTemplate, the original code added formit => true to both; this fix only adds it to the AdministratorTemplate one, leaving the unrelated policy's data untouched.

Fix: build a template id → name lookup while walking the templates in the first loop, then skip a policy in the second loop whenever its template isn't in the permission's templates list.

See #246 for the broader question of whether FormIt should own its own policy template instead of piggybacking on AdministratorTemplate. This PR doesn't attempt that redesign, it just stops the current approach from leaking into policies it has no business touching.

Fixes #272

permissions.resolver.php created the "formit" and "formit_encryptions"
modAccessPermission bits only on templates listed in each
permission's `templates` key (AdministratorTemplate), which is
correct. But the second loop, which writes the actual true/false
value into every modAccessPolicy's `data` blob, never checked that
`templates` restriction. Any permission without an explicit
`policies` allowlist (formit) fell into the unconditional
`$data[$permission['name']] = true` branch for every single policy
in the system, including ones built from unrelated templates like
ElementTemplate.

Build a template id => name lookup while walking the templates, then
skip a policy in the second loop whenever its template isn't in the
permission's `templates` list, so the value is only written for
policies that actually have that permission bit defined on their
template.

Reproduced both the bug and the fix with a standalone script stubbing
xPDO's getCollection()/getObject()/newObject() against the resolver's
actual logic: before the fix, a policy on an unrelated template
picked up `formit => true`; after the fix, that key is never added.

See Sterc#246 for the broader question of whether FormIt should own its
own policy template instead of piggybacking on AdministratorTemplate.
This just stops the current approach from leaking into policies it
has no business touching.

Fixes Sterc#272
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Permissions resolver assigns too much

1 participant