[6.x] Preserve unregistered permissions when saving a role#14975
Merged
Conversation
The roles CP payload is built from the registered permission tree, so any permission a role held that wasn't currently registered - one from a disabled addon, or a deprecated core permission - was silently dropped the first time anyone saved that role. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verified flattened()->map->value() yields the same resolved value set as walking tree() - same values across all placeholder types (collections, taxonomies, globals, assets, navs, forms), both populated and empty states. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Saving a role in the Control Panel replaces its permissions with what the UI submitted. That payload only contains currently registered permissions, so anything else the role held gets silently dropped. This bites two cases:
Now
update()merges any permission the role already holds that isn't in the registered tree back into what's saved.superis excluded (otherwise demoting a super role would resurrect it), and registered-but-unchecked permissions are still removed, so "Uncheck All" is unaffected.store()is untouched since a new role has nothing to preserve.Accepted tradeoff
The registered tree resolves per-item permissions from collections, taxonomies, forms, etc. that currently exist. So deleting a collection makes its permissions (
view blog entries) unregistered, and they'll now persist in the role's YAML instead of being pruned on save. Not a security concern — a permission naming a nonexistent collection grants nothing — and it means a deleted-then-recreated collection keeps its permissions. The cost is some potential YAML cruft, which beats trying to tell "unregistered because deprecated" apart from "unregistered because deleted".Why this surfaced
In #14957 we're adjusting some permissions. Removing
config form fields, but keeping it referenced in appropriate policies for backwards compatibility. But when you saved the role, you lost that permission if you weren't paying attention.Note on the existing test
it_updates_a_roleused fake handles (one,two,three) that were never registered, so they now count as unregistered and get preserved — breaking its full-replace assertion. Swapped for real core handles.🤖 Generated with Claude Code