[FEATURE]: Permission system hardening: Role policies and member user-link restriction

moritz commented

2026-02-04 12:05:09 +01:00

Owner

Description

Implement two security improvements identified in the permission system analysis:

Role resource: add defense-in-depth
The Role resource currently has no authorizers and no policies block. Page access to /admin/roles is protected by CheckPagePermission, but resource-level authorization is missing. Add Ash policies so that only users with the admin permission set can read, create, update, or destroy roles. This ensures authorization is enforced at both page and resource layers.
Member create/update: restrict user link to admins
Today, any user with Member create permission (including normal_user / Kassenwart) can pass the :user argument when creating or updating a member and thus link any member to any user. The intended rule is: only admins may set or change the user–member link. Enforce this with a policy-based approach: keep a single create (and update) action, and add a forbid policy that applies when the :user argument is present and the actor is not an admin. No new actions are required; only the policy layer is extended.

Acceptance criteria

Role resource

Add authorizers: [Ash.Policy.Authorizer] to the Role resource.
Add a policies do ... end block that authorizes read, create, update, and destroy via Mv.Authorization.Checks.HasPermission (admin is the only permission set with Role permissions in PermissionSets).
Ensure existing admin flows (role CRUD from the UI) still work with an actor present.
Seeds or bootstrap that create/read roles use authorize?: false where they already do; no change to Role’s public API (code interface).
Add or extend tests: non-admin actor cannot read/create/update/destroy Role; admin can.

Member user link (create & update)

Add a custom policy check (e.g. Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin) that returns “forbid” when the action has the :user argument (or equivalent relationship input) set and the actor is not an admin (use Mv.Authorization.Actor.admin?/1 or Mv.Authorization.Checks.ActorIsAdmin).
In the Member resource, for the create action that accepts :user: add forbid_if ForbidMemberUserLinkUnlessAdmin (or equivalent) before authorize_if HasPermission, so that normal_user and read_only cannot pass :user when creating a member.
In the Member resource, for the update action that accepts :user: apply the same forbid check when the :user argument (or relationship) is being changed, so only admins can set or change the user link on update.
Document the rule in code and, if needed, in docs/roles-and-permissions-architecture.md (e.g. “Only admins may set or change the user–member link on create or update”).
Add or extend tests: normal_user can create/update member without providing :user; normal_user cannot create/update member with :user set; admin can create/update member with or without :user.

General

Run existing permission and member tests; fix any that assumed normal_user could pass :user.
No new routes or UI changes required; only backend policy and optional docs/test updates.

## Description Implement two security improvements identified in the permission system analysis: 1. **Role resource: add defense-in-depth** The Role resource currently has no `authorizers` and no `policies` block. Page access to `/admin/roles` is protected by `CheckPagePermission`, but resource-level authorization is missing. Add Ash policies so that only users with the admin permission set can read, create, update, or destroy roles. This ensures authorization is enforced at both page and resource layers. 2. **Member create/update: restrict user link to admins** Today, any user with `Member` create permission (including `normal_user` / Kassenwart) can pass the `:user` argument when creating or updating a member and thus link any member to any user. The intended rule is: only admins may set or change the user–member link. Enforce this with a **policy-based approach**: keep a single create (and update) action, and add a **forbid** policy that applies when the `:user` argument is present and the actor is not an admin. No new actions are required; only the policy layer is extended. ## Acceptance criteria ### Role resource - [ ] Add `authorizers: [Ash.Policy.Authorizer]` to the Role resource. - [ ] Add a `policies do ... end` block that authorizes read, create, update, and destroy via `Mv.Authorization.Checks.HasPermission` (admin is the only permission set with Role permissions in PermissionSets). - [ ] Ensure existing admin flows (role CRUD from the UI) still work with an actor present. - [ ] Seeds or bootstrap that create/read roles use `authorize?: false` where they already do; no change to Role’s public API (code interface). - [ ] Add or extend tests: non-admin actor cannot read/create/update/destroy Role; admin can. ### Member user link (create & update) - [ ] Add a custom policy check (e.g. `Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin`) that returns “forbid” when the action has the `:user` argument (or equivalent relationship input) set and the actor is not an admin (use `Mv.Authorization.Actor.admin?/1` or `Mv.Authorization.Checks.ActorIsAdmin`). - [ ] In the Member resource, for the create action that accepts `:user`: add `forbid_if ForbidMemberUserLinkUnlessAdmin` (or equivalent) before `authorize_if HasPermission`, so that normal_user and read_only cannot pass `:user` when creating a member. - [ ] In the Member resource, for the update action that accepts `:user`: apply the same forbid check when the `:user` argument (or relationship) is being changed, so only admins can set or change the user link on update. - [ ] Document the rule in code and, if needed, in `docs/roles-and-permissions-architecture.md` (e.g. “Only admins may set or change the user–member link on create or update”). - [ ] Add or extend tests: normal_user can create/update member without providing `:user`; normal_user cannot create/update member with `:user` set; admin can create/update member with or without `:user`. ### General - [ ] Run existing permission and member tests; fix any that assumed normal_user could pass `:user`. - [ ] No new routes or UI changes required; only backend policy and optional docs/test updates.