From 8f5f69744c939923d172878e09075a1138622d10 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:12 +0100 Subject: [PATCH 01/12] Add CustomFieldValue create/destroy :linked to own_data permission set Allows members to create and delete custom field values for their linked member. --- lib/mv/authorization/permission_sets.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 22132cb..e133ed7 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -105,9 +105,11 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # CustomFieldValue: Can read/update custom field values of linked member + # CustomFieldValue: Can read/update/create/destroy custom field values of linked member %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :linked, granted: true}, # CustomField: Can read all (needed for forms) %{resource: "CustomField", action: :read, scope: :all, granted: true} -- 2.47.2 From c7c6b318acef273edb2110070bf805244ec21dd0 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:17 +0100 Subject: [PATCH 02/12] Add CustomFieldValueCreateScope check for create actions Ash cannot apply filters to create; this check enforces :linked/:all scope via strict_check only (no filter). --- .../checks/custom_field_value_create_scope.ex | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 lib/mv/authorization/checks/custom_field_value_create_scope.ex diff --git a/lib/mv/authorization/checks/custom_field_value_create_scope.ex b/lib/mv/authorization/checks/custom_field_value_create_scope.ex new file mode 100644 index 0000000..f5be53d --- /dev/null +++ b/lib/mv/authorization/checks/custom_field_value_create_scope.ex @@ -0,0 +1,64 @@ +defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do + @moduledoc """ + Policy check for CustomFieldValue create actions only. + + Use this for create instead of HasPermission because Ash cannot apply + filters to create actions ("Cannot use a filter to authorize a create"). + This check performs the same scope logic as HasPermission for create + (PermissionSets + :linked/:all) but only implements strict_check, so it + never adds a filter. + + Used in CustomFieldValue policies: + policy action_type(:create) do + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope + end + """ + use Ash.Policy.Check + alias Mv.Authorization.PermissionSets + require Logger + + @impl true + def describe(_opts), + do: "CustomFieldValue create allowed by permission set scope (:linked or :all)" + + @impl true + def strict_check(actor, authorizer, _opts) do + actor = ensure_role_loaded(actor) + + with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- actor, + {:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name), + permissions <- PermissionSets.get_permissions(ps_atom), + perm <- find_custom_field_value_create(permissions.resources) do + case perm do + nil -> {:ok, false} + %{scope: :all} -> {:ok, true} + %{scope: :linked} -> {:ok, member_id_matches?(authorizer, actor)} + end + else + _ -> {:ok, false} + end + end + + defp find_custom_field_value_create(resources) do + Enum.find(resources, fn p -> + p.resource == "CustomFieldValue" and p.action == :create and p.granted + end) + end + + defp member_id_matches?(authorizer, actor) do + member_id = get_create_member_id(authorizer) + !is_nil(member_id) and member_id == actor.member_id + end + + defp get_create_member_id(authorizer) do + changeset = authorizer.changeset || authorizer.subject + + if changeset && function_exported?(Ash.Changeset, :get_attribute, 2) do + Ash.Changeset.get_attribute(changeset, :member_id) + else + nil + end + end + + defp ensure_role_loaded(actor), do: Mv.Authorization.Actor.ensure_loaded(actor) +end -- 2.47.2 From bf2d0352c14acc27cae32d31b648a15d9964c89c Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:22 +0100 Subject: [PATCH 03/12] Add authorization policies to CustomFieldValue resource - Authorizer and policies: bypass for read (member_id == actor.member_id), CustomFieldValueCreateScope for create, HasPermission for read/update/destroy. - HasPermission: pass authorizer into strict_check helper; document that create must use a dedicated check (no filter). --- lib/membership/custom_field_value.ex | 36 ++++++++++++++++++- lib/mv/authorization/checks/has_permission.ex | 7 ++-- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lib/membership/custom_field_value.ex b/lib/membership/custom_field_value.ex index 232ba99..d6d91a6 100644 --- a/lib/membership/custom_field_value.ex +++ b/lib/membership/custom_field_value.ex @@ -39,7 +39,11 @@ defmodule Mv.Membership.CustomFieldValue do """ use Ash.Resource, domain: Mv.Membership, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] + + require Ash.Query + import Ash.Expr postgres do table "custom_field_values" @@ -62,6 +66,36 @@ defmodule Mv.Membership.CustomFieldValue do end end + # Authorization Policies + # Order matters: Most specific policies first, then general permission check + # Pattern aligns with User and Member resources (bypass for READ, HasPermission for update/destroy) + # Create uses CustomFieldValueCreateScope because Ash cannot apply filters to create actions. + policies do + # SPECIAL CASE: Users can READ custom field values of their linked member + # Bypass needed for list queries (expr triggers auto_filter in Ash) + bypass action_type(:read) do + description "Users can read custom field values of their linked member" + authorize_if expr(member_id == ^actor(:member_id)) + end + + # CREATE: CustomFieldValueCreateScope (no filter; Ash rejects filters on create) + # - :own_data -> create allowed when member_id == actor.member_id (scope :linked) + # - :read_only -> no create permission + # - :normal_user / :admin -> create allowed (scope :all) + policy action_type(:create) do + description "CustomFieldValue create allowed by permission set scope" + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope + end + + # READ/UPDATE/DESTROY: HasPermission (scope :linked / :all) + policy action_type([:read, :update, :destroy]) do + description "Check permissions from user's role and permission set" + authorize_if Mv.Authorization.Checks.HasPermission + end + + # DEFAULT: Ash implicitly forbids if no policy authorized (fail-closed) + end + attributes do uuid_primary_key :id diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 1a478b8..1cf1e39 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -110,12 +110,12 @@ defmodule Mv.Authorization.Checks.HasPermission do {:ok, false} true -> - strict_check_with_permissions(actor, resource, action, record) + strict_check_with_permissions(actor, resource, action, record, authorizer) end end # Helper function to reduce nesting depth - defp strict_check_with_permissions(actor, resource, action, record) do + defp strict_check_with_permissions(actor, resource, action, record, _authorizer) do # Ensure role is loaded (fallback if on_mount didn't run) actor = ensure_role_loaded(actor) @@ -148,6 +148,7 @@ defmodule Mv.Authorization.Checks.HasPermission do else # No record yet (e.g., read/list queries) - deny at strict_check level # Resources must use expr-based bypass policies for list filtering + # Create: use a dedicated check that does not return a filter (e.g. CustomFieldValueCreateScope) {:ok, false} end @@ -213,7 +214,7 @@ defmodule Mv.Authorization.Checks.HasPermission do {:filter, filter_expr} -> # :linked or :own scope - apply filter - # filter_expr is a keyword list from expr(...), return it directly + # Create actions must not use HasPermission (use a dedicated check, e.g. CustomFieldValueCreateScope) filter_expr false -> -- 2.47.2 From 17831a09482f4193415d5db6ae8e6c09eb38751d Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:29 +0100 Subject: [PATCH 04/12] Pass actor to CustomFieldValue destroy and load in existing tests Required after CustomFieldValue gained authorization policies. --- test/membership/member_search_with_custom_fields_test.exs | 2 +- test/mv/membership/import/member_csv_test.exs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/membership/member_search_with_custom_fields_test.exs b/test/membership/member_search_with_custom_fields_test.exs index bd28ce5..284fcff 100644 --- a/test/membership/member_search_with_custom_fields_test.exs +++ b/test/membership/member_search_with_custom_fields_test.exs @@ -348,7 +348,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do assert List.first(results).id == member1.id # Delete custom field value - assert :ok = Ash.destroy(cfv) + assert :ok = Ash.destroy(cfv, actor: system_actor) # Value should no longer be found deleted_results = diff --git a/test/mv/membership/import/member_csv_test.exs b/test/mv/membership/import/member_csv_test.exs index 778e82b..0304989 100644 --- a/test/mv/membership/import/member_csv_test.exs +++ b/test/mv/membership/import/member_csv_test.exs @@ -247,7 +247,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do member = Enum.find(members, &(&1.email == "withcustom@example.com")) assert member != nil - {:ok, member_with_cf} = Ash.load(member, :custom_field_values) + {:ok, member_with_cf} = Ash.load(member, :custom_field_values, actor: actor) assert length(member_with_cf.custom_field_values) == 1 cfv = List.first(member_with_cf.custom_field_values) assert cfv.custom_field_id == custom_field.id -- 2.47.2 From 4e032ea778ac60f701fa79ee353b4ce5fc994ad8 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:34 +0100 Subject: [PATCH 05/12] Add CustomFieldValue policy tests (own_data, read_only, normal_user, admin) Covers read/update/create/destroy for linked vs unlinked members and CRUD permissions per permission set. --- .../custom_field_value_policies_test.exs | 487 ++++++++++++++++++ 1 file changed, 487 insertions(+) create mode 100644 test/mv/membership/custom_field_value_policies_test.exs diff --git a/test/mv/membership/custom_field_value_policies_test.exs b/test/mv/membership/custom_field_value_policies_test.exs new file mode 100644 index 0000000..c7a80db --- /dev/null +++ b/test/mv/membership/custom_field_value_policies_test.exs @@ -0,0 +1,487 @@ +defmodule Mv.Membership.CustomFieldValuePoliciesTest do + @moduledoc """ + Tests for CustomFieldValue resource authorization policies. + + Tests all 4 permission sets (own_data, read_only, normal_user, admin) + and verifies that policies correctly enforce access control based on + user roles and permission sets. + """ + # async: false because we need database commits to be visible across queries + use Mv.DataCase, async: false + + alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Accounts + alias Mv.Authorization + + require Ash.Query + + setup do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + %{actor: system_actor} + end + + # Helper to create a role with a specific permission set + defp create_role_with_permission_set(permission_set_name, actor) do + role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}" + + case Authorization.create_role( + %{ + name: role_name, + description: "Test role for #{permission_set_name}", + permission_set_name: permission_set_name + }, + actor: actor + ) do + {:ok, role} -> role + {:error, error} -> raise "Failed to create role: #{inspect(error)}" + end + end + + # Helper to create a user with a specific permission set + # Returns user with role preloaded (required for authorization) + defp create_user_with_permission_set(permission_set_name, actor) do + role = create_role_with_permission_set(permission_set_name, actor) + + {:ok, user} = + Accounts.User + |> Ash.Changeset.for_create(:register_with_password, %{ + email: "user#{System.unique_integer([:positive])}@example.com", + password: "testpassword123" + }) + |> Ash.create(actor: actor) + + {:ok, user} = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove) + |> Ash.update(actor: actor) + + {:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts, actor: actor) + user_with_role + end + + defp create_admin_user(actor) do + create_user_with_permission_set("admin", actor) + end + + defp create_linked_member_for_user(user, actor) do + admin = create_admin_user(actor) + + {:ok, member} = + Member + |> Ash.Changeset.for_create(:create_member, %{ + first_name: "Linked", + last_name: "Member", + email: "linked#{System.unique_integer([:positive])}@example.com" + }) + |> Ash.create(actor: admin, return_notifications?: false) + + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.force_change_attribute(:member_id, member.id) + |> Ash.update(actor: admin, domain: Mv.Accounts, return_notifications?: false) + + member + end + + defp create_unlinked_member(actor) do + admin = create_admin_user(actor) + + {:ok, member} = + Member + |> Ash.Changeset.for_create(:create_member, %{ + first_name: "Unlinked", + last_name: "Member", + email: "unlinked#{System.unique_integer([:positive])}@example.com" + }) + |> Ash.create(actor: admin) + + member + end + + defp create_custom_field(actor) do + {:ok, field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "test_field_#{System.unique_integer([:positive])}", + value_type: :string + }) + |> Ash.create(actor: actor) + + field + end + + defp create_custom_field_value(member_id, custom_field_id, value, actor) do + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member_id, + custom_field_id: custom_field_id, + value: %{"_union_type" => "string", "_union_value" => value} + }) + |> Ash.create(actor: actor, domain: Mv.Membership) + + cfv + end + + describe "own_data permission set (Mitglied)" do + setup %{actor: actor} do + user = create_user_with_permission_set("own_data", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + {:ok, user} = Ash.load(user, :member, domain: Mv.Accounts, actor: actor) + + %{ + user: user, + linked_member: linked_member, + unlinked_member: unlinked_member, + custom_field: custom_field, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + actor: actor + } + end + + test "can read custom field values of linked member", %{user: user, cfv_linked: cfv_linked} do + {:ok, cfv} = + Ash.get(CustomFieldValue, cfv_linked.id, actor: user, domain: Mv.Membership) + + assert cfv.id == cfv_linked.id + end + + test "can list custom field values returns only linked member's values", %{ + user: user, + cfv_linked: cfv_linked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + assert length(values) == 1 + assert hd(values).id == cfv_linked.id + end + + test "can update custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + {:ok, updated} = + cfv_linked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can create custom field value for linked member", %{ + user: user, + linked_member: linked_member, + actor: actor + } do + # Create a second custom field via admin (own_data cannot create CustomField) + custom_field2 = create_custom_field(actor) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: linked_member.id, + custom_field_id: custom_field2.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == linked_member.id + assert cfv.custom_field_id == custom_field2.id + end + + test "can destroy custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + result = Ash.destroy(cfv_linked, actor: user, domain: Mv.Membership) + assert :ok = result + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership) + end + + test "cannot read custom field values of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Invalid, fn -> + Ash.get!(CustomFieldValue, cfv_unlinked.id, actor: user, domain: Mv.Membership) + end + end + + test "cannot update custom field value of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Forbidden, fn -> + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "hacked"} + }) + |> Ash.update!(actor: user, domain: Mv.Membership) + end + end + + test "cannot create custom field value for unlinked member", %{ + user: user, + unlinked_member: unlinked_member, + custom_field: custom_field + } do + assert_raise Ash.Error.Forbidden, fn -> + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "forbidden"} + }) + |> Ash.create!(actor: user, domain: Mv.Membership) + end + end + + test "cannot destroy custom field value of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Forbidden, fn -> + Ash.destroy!(cfv_unlinked, actor: user, domain: Mv.Membership) + end + end + end + + describe "read_only permission set (Vorstand/Buchhaltung)" do + setup %{actor: actor} do + user = create_user_with_permission_set("read_only", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can read individual custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, cfv} = + Ash.get(CustomFieldValue, cfv_unlinked.id, actor: user, domain: Mv.Membership) + + assert cfv.id == cfv_unlinked.id + end + + test "cannot create custom field value (returns forbidden)", %{ + user: user, + linked_member: linked_member, + custom_field: custom_field + } do + assert_raise Ash.Error.Forbidden, fn -> + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: linked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "forbidden"} + }) + |> Ash.create!(actor: user, domain: Mv.Membership) + end + end + + test "cannot update custom field value (returns forbidden)", %{ + user: user, + cfv_linked: cfv_linked + } do + assert_raise Ash.Error.Forbidden, fn -> + cfv_linked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "hacked"} + }) + |> Ash.update!(actor: user, domain: Mv.Membership) + end + end + + test "cannot destroy custom field value (returns forbidden)", %{ + user: user, + cfv_linked: cfv_linked + } do + assert_raise Ash.Error.Forbidden, fn -> + Ash.destroy!(cfv_linked, actor: user, domain: Mv.Membership) + end + end + end + + describe "normal_user permission set (Kassenwart)" do + setup %{actor: actor} do + user = create_user_with_permission_set("normal_user", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member, + actor: actor + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can create custom field value", %{ + user: user, + unlinked_member: unlinked_member, + actor: actor + } do + # normal_user cannot create CustomField; use actor (admin) to create it + custom_field = create_custom_field(actor) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == unlinked_member.id + end + + test "can update any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, updated} = + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + end + end + + describe "admin permission set" do + setup %{actor: actor} do + user = create_user_with_permission_set("admin", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can create custom field value", %{user: user, unlinked_member: unlinked_member} do + custom_field = create_custom_field(user) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == unlinked_member.id + end + + test "can update any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, updated} = + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + end + end +end -- 2.47.2 From db95979bf54c1ed32c90ab070516b0fffcac9ca9 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:38 +0100 Subject: [PATCH 06/12] Document CustomFieldValue policies and own_data create/destroy in architecture Update roles-and-permissions-architecture.md with policy layout and permission matrix for CustomFieldValue (linked). --- docs/roles-and-permissions-architecture.md | 44 +++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index 8934688..acea99e 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -501,9 +501,11 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # CustomFieldValue: Can read/update custom field values of linked member + # CustomFieldValue: Can read/update/create/destroy custom field values of linked member %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :linked, granted: true}, # CustomField: Can read all (needed for forms) %{resource: "CustomField", action: :read, scope: :all, granted: true} @@ -678,7 +680,7 @@ Quick reference table showing what each permission set allows: | **User** (all) | - | - | - | R, C, U, D | | **Member** (linked) | R, U | - | - | - | | **Member** (all) | - | R | R, C, U | R, C, U, D | -| **CustomFieldValue** (linked) | R, U | - | - | - | +| **CustomFieldValue** (linked) | R, U, C, D | - | - | - | | **CustomFieldValue** (all) | - | R | R, C, U, D | R, C, U, D | | **CustomField** (all) | R | R | R | R, C, U, D | | **Role** (all) | - | - | - | R, C, U, D | @@ -1053,35 +1055,33 @@ end ### CustomFieldValue Resource Policies -**Location:** `lib/mv/membership/custom_field_value.ex` +**Location:** `lib/membership/custom_field_value.ex` -**Special Case:** Users can access custom field values of their linked member. +**Pattern:** Bypass for READ (list queries), CustomFieldValueCreateScope for create (no filter), HasPermission for read/update/destroy. Create uses a dedicated check because Ash cannot apply filters to create actions. ```elixir defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ... policies do - # SPECIAL CASE: Users can access custom field values of their linked member - # Note: This uses member_id relationship (CustomFieldValue.member_id → Member.id → User.member_id) - policy action_type([:read, :update]) do - description "Users can access custom field values of their linked member" + # Bypass for READ (list queries; expr triggers auto_filter) + bypass action_type(:read) do + description "Users can read custom field values of their linked member" authorize_if expr(member_id == ^actor(:member_id)) end - # GENERAL: Check permissions from role - policy action_type([:read, :create, :update, :destroy]) do - description "Check permissions from user's role" - authorize_if Mv.Authorization.Checks.HasPermission + # CREATE: CustomFieldValueCreateScope (no filter; Ash rejects filters on create) + # own_data -> create when member_id == actor.member_id; normal_user/admin -> create (scope :all) + policy action_type(:create) do + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope end - # DEFAULT: Forbid - policy action_type([:read, :create, :update, :destroy]) do - forbid_if always() + # READ/UPDATE/DESTROY: HasPermission (scope :linked / :all) + policy action_type([:read, :update, :destroy]) do + authorize_if Mv.Authorization.Checks.HasPermission end + # DEFAULT: Ash implicitly forbids if no policy authorized (fail-closed) end - - # ... end ``` @@ -1089,11 +1089,13 @@ end | Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin | |--------|----------|----------|------------|-------------|-------| -| Read linked | ✅ (special) | ✅ (if linked) | ✅ | ✅ (if linked) | ✅ | -| Update linked | ✅ (special) | ❌ | ✅ | ❌ | ✅ | +| Read linked | ✅ (bypass) | ✅ (if linked) | ✅ | ✅ (if linked) | ✅ | +| Update linked | ✅ (scope :linked) | ❌ | ✅ | ❌ | ✅ | +| Create linked | ✅ (CustomFieldValueCreateScope) | ❌ | ✅ | ❌ | ✅ | +| Destroy linked | ✅ (scope :linked) | ❌ | ✅ | ❌ | ✅ | | Read all | ❌ | ✅ | ✅ | ✅ | ✅ | -| Create | ❌ | ❌ | ✅ | ❌ | ✅ | -| Destroy | ❌ | ❌ | ✅ | ❌ | ✅ | +| Create all | ❌ | ❌ | ✅ | ❌ | ✅ | +| Destroy all | ❌ | ❌ | ✅ | ❌ | ✅ | ### CustomField Resource Policies -- 2.47.2 From 9e6c79bf40af2906db8d9ef9ff6061ba60d6de91 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:41:33 +0100 Subject: [PATCH 07/12] chore: remove start-database from test action --- Justfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Justfile b/Justfile index c68c473..f25041c 100644 --- a/Justfile +++ b/Justfile @@ -41,7 +41,7 @@ audit: mix deps.audit mix hex.audit -test *args: install-dependencies start-database +test *args: install-dependencies mix test {{args}} format: -- 2.47.2 From 7153af23eee18b81478779ba0fe124b3a29d74be Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:38 +0100 Subject: [PATCH 08/12] CustomFieldValueCreateScope: use get_argument_or_attribute for member_id - Read member_id via Ash.Changeset.get_argument_or_attribute/2 so it works when set as attribute or argument - Remove unused require Logger - Document member_id source in moduledoc --- .../checks/custom_field_value_create_scope.ex | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/mv/authorization/checks/custom_field_value_create_scope.ex b/lib/mv/authorization/checks/custom_field_value_create_scope.ex index f5be53d..0b24e74 100644 --- a/lib/mv/authorization/checks/custom_field_value_create_scope.ex +++ b/lib/mv/authorization/checks/custom_field_value_create_scope.ex @@ -8,6 +8,14 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do (PermissionSets + :linked/:all) but only implements strict_check, so it never adds a filter. + ## member_id source + + The check reads `member_id` from the create changeset via + `Ash.Changeset.get_argument_or_attribute/2`, so it works when member_id + is set as an attribute or as an action argument. The CustomFieldValue + resource's default create action must accept and require `member_id` + (e.g. via `default_accept [:value, :member_id, :custom_field_id]`). + Used in CustomFieldValue policies: policy action_type(:create) do authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope @@ -15,7 +23,6 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do """ use Ash.Policy.Check alias Mv.Authorization.PermissionSets - require Logger @impl true def describe(_opts), @@ -53,8 +60,8 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do defp get_create_member_id(authorizer) do changeset = authorizer.changeset || authorizer.subject - if changeset && function_exported?(Ash.Changeset, :get_attribute, 2) do - Ash.Changeset.get_attribute(changeset, :member_id) + if changeset && function_exported?(Ash.Changeset, :get_argument_or_attribute, 2) do + Ash.Changeset.get_argument_or_attribute(changeset, :member_id) else nil end -- 2.47.2 From 3f95a2dd84eceffc84a36cbd65b232b1036e318c Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:39 +0100 Subject: [PATCH 09/12] CustomFieldValue: remove unused require Ash.Query --- lib/membership/custom_field_value.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/membership/custom_field_value.ex b/lib/membership/custom_field_value.ex index d6d91a6..623455d 100644 --- a/lib/membership/custom_field_value.ex +++ b/lib/membership/custom_field_value.ex @@ -42,7 +42,6 @@ defmodule Mv.Membership.CustomFieldValue do data_layer: AshPostgres.DataLayer, authorizers: [Ash.Policy.Authorizer] - require Ash.Query import Ash.Expr postgres do -- 2.47.2 From 4d3a249b0c2aec4165fea2d8918f0dc293006029 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:40 +0100 Subject: [PATCH 10/12] HasPermission: remove unused _authorizer from strict_check helper --- lib/mv/authorization/checks/has_permission.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 1cf1e39..774e767 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -110,12 +110,12 @@ defmodule Mv.Authorization.Checks.HasPermission do {:ok, false} true -> - strict_check_with_permissions(actor, resource, action, record, authorizer) + strict_check_with_permissions(actor, resource, action, record) end end # Helper function to reduce nesting depth - defp strict_check_with_permissions(actor, resource, action, record, _authorizer) do + defp strict_check_with_permissions(actor, resource, action, record) do # Ensure role is loaded (fallback if on_mount didn't run) actor = ensure_role_loaded(actor) -- 2.47.2 From 0219073d338a55d0d4c4a09a29a24c2f87d7c870 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:43 +0100 Subject: [PATCH 11/12] CFV policies test: system_actor for setup, verify destroy with actor - create_linked_member_for_user and create_unlinked_member use actor (system_actor) directly instead of creating admin user per call - Remove create_admin_user helper - After destroy, verify with Ash.get(..., actor: actor) to avoid false positive from Forbidden vs NotFound --- .../custom_field_value_policies_test.exs | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/test/mv/membership/custom_field_value_policies_test.exs b/test/mv/membership/custom_field_value_policies_test.exs index c7a80db..d400aec 100644 --- a/test/mv/membership/custom_field_value_policies_test.exs +++ b/test/mv/membership/custom_field_value_policies_test.exs @@ -60,13 +60,7 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do user_with_role end - defp create_admin_user(actor) do - create_user_with_permission_set("admin", actor) - end - defp create_linked_member_for_user(user, actor) do - admin = create_admin_user(actor) - {:ok, member} = Member |> Ash.Changeset.for_create(:create_member, %{ @@ -74,19 +68,17 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do last_name: "Member", email: "linked#{System.unique_integer([:positive])}@example.com" }) - |> Ash.create(actor: admin, return_notifications?: false) + |> Ash.create(actor: actor, return_notifications?: false) user |> Ash.Changeset.for_update(:update, %{}) |> Ash.Changeset.force_change_attribute(:member_id, member.id) - |> Ash.update(actor: admin, domain: Mv.Accounts, return_notifications?: false) + |> Ash.update(actor: actor, domain: Mv.Accounts, return_notifications?: false) member end defp create_unlinked_member(actor) do - admin = create_admin_user(actor) - {:ok, member} = Member |> Ash.Changeset.for_create(:create_member, %{ @@ -94,7 +86,7 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do last_name: "Member", email: "unlinked#{System.unique_integer([:positive])}@example.com" }) - |> Ash.create(actor: admin) + |> Ash.create(actor: actor) member end @@ -201,11 +193,16 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert cfv.custom_field_id == custom_field2.id end - test "can destroy custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + test "can destroy custom field value of linked member", %{ + user: user, + cfv_linked: cfv_linked, + actor: actor + } do result = Ash.destroy(cfv_linked, actor: user, domain: Mv.Membership) assert :ok = result - assert {:error, _} = Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership, actor: actor) end test "cannot read custom field values of unlinked member", %{ @@ -408,10 +405,15 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert %Ash.Union{value: "updated", type: :string} = updated.value end - test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + test "can destroy any custom field value", %{ + user: user, + cfv_unlinked: cfv_unlinked, + actor: actor + } do :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) - assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership, actor: actor) end end @@ -478,10 +480,15 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert %Ash.Union{value: "updated", type: :string} = updated.value end - test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + test "can destroy any custom field value", %{ + user: user, + cfv_unlinked: cfv_unlinked, + actor: actor + } do :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) - assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership, actor: actor) end end end -- 2.47.2 From bfe9fba2e03e87f7a4d985830a2a299939050a37 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:44 +0100 Subject: [PATCH 12/12] Docs: document bypass read rule for CustomFieldValue pattern - Bypass action_type(:read) is production-side rule: reading own CFVs always allowed, overrides Permission-Sets. Applies to get/list/load. --- docs/roles-and-permissions-architecture.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index acea99e..063de32 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -1059,6 +1059,8 @@ end **Pattern:** Bypass for READ (list queries), CustomFieldValueCreateScope for create (no filter), HasPermission for read/update/destroy. Create uses a dedicated check because Ash cannot apply filters to create actions. +The bypass `action_type(:read)` is a production-side rule: reading own CFVs (where `member_id == actor.member_id`) is always allowed and overrides Permission-Sets; no further policies are needed for that. It applies to all read actions (get, list, load). + ```elixir defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ... -- 2.47.2