Skip to content

fix: a permission preset needs the permission it grants - #940

Merged
blaipr merged 1 commit into
mainfrom
fix/a-permission-preset-needs-the-permission-it-grants
Sep 24, 2026
Merged

blaipr merged 1 commit into
mainfrom
fix/a-permission-preset-needs-the-permission-it-grants

Conversation

@blaipr

@blaipr blaipr commented Sep 24, 2026

Copy link
Copy Markdown
Member

Every item-preset action is gated on isMgmItemsPreset() alone ("Default Values Management" on a profile). Choosing who an account is shared with is gated on AccountAcl::getShowPermission(): an application administrator, an account administrator, or isAccPermission().

A permission preset is applied by AccountPreset::addPresetPermissions() to every account its target creates, and that runs outside the $userCanChangePermissions gate. That gate is what drops hand-picked sharing on the same request. So a user holding only "Default Values Management" could write a preset targeting a colleague, group or profile that shares every account they create from then on with themselves, including for editing. They could never have granted that sharing by hand.

Change

  • ItemsPresetForm::makePermissionPreset() now refuses unless the signed-in user passes getShowPermission(). The error is "You don't have permission to assign account permissions".
  • The other three preset types don't grant access, so they stay on isMgmItemsPreset():
    • password policy
    • privacy
    • session timeout

Tests

  • New unit test: a user with only mgmItemsPreset is refused. Mutation-verified: it fails without the fix.
  • The existing permission-preset tests, unit and integration, now sign in as a user who may share accounts. The integration harness's random profile held accPermission only about half the time.

@blaipr
blaipr merged commit c932ee7 into main Sep 24, 2026
8 checks passed
@blaipr
blaipr deleted the fix/a-permission-preset-needs-the-permission-it-grants branch September 24, 2026 13:51
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.

1 participant