From 070d9d1fc3acb19772c983f776ace763c2111ffe Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 14:56:07 +0200 Subject: [PATCH] refactor(authorization): unify own_data read check across linked resources --- lib/membership/member_group.ex | 2 +- lib/membership_fees/membership_fee_cycle.ex | 2 +- .../member_group_read_linked_for_own_data.ex | 63 ---------------- ...ship_fee_cycle_read_linked_for_own_data.ex | 62 ---------------- .../checks/read_linked_for_own_data.ex | 74 +++++++++++++++++++ 5 files changed, 76 insertions(+), 127 deletions(-) delete mode 100644 lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex delete mode 100644 lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex create mode 100644 lib/mv/authorization/checks/read_linked_for_own_data.ex diff --git a/lib/membership/member_group.ex b/lib/membership/member_group.ex index 22a1f70..d09f0e5 100644 --- a/lib/membership/member_group.ex +++ b/lib/membership/member_group.ex @@ -63,7 +63,7 @@ defmodule Mv.Membership.MemberGroup do policies do bypass action_type(:read) do description "own_data: read only member_groups where member_id == actor.member_id" - authorize_if Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData + authorize_if {Mv.Authorization.Checks.ReadLinkedForOwnData, member_id_field: :member_id} end policy action_type(:read) do diff --git a/lib/membership_fees/membership_fee_cycle.ex b/lib/membership_fees/membership_fee_cycle.ex index f0dd1a7..cd62887 100644 --- a/lib/membership_fees/membership_fee_cycle.ex +++ b/lib/membership_fees/membership_fee_cycle.ex @@ -88,7 +88,7 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do policies do bypass action_type(:read) do description "own_data: read only cycles where member_id == actor.member_id" - authorize_if Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData + authorize_if {Mv.Authorization.Checks.ReadLinkedForOwnData, member_id_field: :member_id} end policy action_type([:read, :create, :update, :destroy]) do diff --git a/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex b/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex deleted file mode 100644 index a553fde..0000000 --- a/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex +++ /dev/null @@ -1,63 +0,0 @@ -defmodule Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData do - @moduledoc """ - Policy check for MemberGroup read: true only when actor has permission set "own_data" - AND record.member_id == actor.member_id. - - Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries), - while admin with member_id does not match and gets :all from HasPermission. - - - With a record (e.g. get by id): returns true only when own_data and member_id match. - - Without a record (list query): strict_check returns false; auto_filter adds filter when own_data. - """ - use Ash.Policy.Check - - alias Mv.Authorization.Checks.ActorPermissionSetIs - - @impl true - def type, do: :filter - - @impl true - def describe(_opts), - do: "own_data can read only member_groups where member_id == actor.member_id" - - @impl true - def strict_check(actor, authorizer, _opts) do - record = get_record_from_authorizer(authorizer) - is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") - - cond do - # List query + own_data: return :unknown so authorizer applies auto_filter (keyword list) - is_nil(record) and is_own_data -> - {:ok, :unknown} - - is_nil(record) -> - {:ok, false} - - not is_own_data -> - {:ok, false} - - record.member_id == actor.member_id -> - {:ok, true} - - true -> - {:ok, false} - end - end - - @impl true - def auto_filter(actor, _authorizer, _opts) do - if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && - Map.get(actor, :member_id) do - [member_id: actor.member_id] - else - [] - end - end - - defp get_record_from_authorizer(authorizer) do - case authorizer.subject do - %{data: data} when not is_nil(data) -> data - _ -> nil - end - end -end diff --git a/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex b/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex deleted file mode 100644 index 092558c..0000000 --- a/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex +++ /dev/null @@ -1,62 +0,0 @@ -defmodule Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData do - @moduledoc """ - Policy check for MembershipFeeCycle read: true only when actor has permission set "own_data" - AND record.member_id == actor.member_id. - - Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries), - while admin with member_id does not match and gets :all from HasPermission. - - - With a record (e.g. get by id): returns true only when own_data and member_id match. - - Without a record (list query): return :unknown so authorizer applies auto_filter. - """ - use Ash.Policy.Check - - alias Mv.Authorization.Checks.ActorPermissionSetIs - - @impl true - def type, do: :filter - - @impl true - def describe(_opts), - do: "own_data can read only membership_fee_cycles where member_id == actor.member_id" - - @impl true - def strict_check(actor, authorizer, _opts) do - record = get_record_from_authorizer(authorizer) - is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") - - cond do - is_nil(record) and is_own_data -> - {:ok, :unknown} - - is_nil(record) -> - {:ok, false} - - not is_own_data -> - {:ok, false} - - record.member_id == actor.member_id -> - {:ok, true} - - true -> - {:ok, false} - end - end - - @impl true - def auto_filter(actor, _authorizer, _opts) do - if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && - Map.get(actor, :member_id) do - [member_id: actor.member_id] - else - [] - end - end - - defp get_record_from_authorizer(authorizer) do - case authorizer.subject do - %{data: data} when not is_nil(data) -> data - _ -> nil - end - end -end diff --git a/lib/mv/authorization/checks/read_linked_for_own_data.ex b/lib/mv/authorization/checks/read_linked_for_own_data.ex new file mode 100644 index 0000000..eafeb8b --- /dev/null +++ b/lib/mv/authorization/checks/read_linked_for_own_data.ex @@ -0,0 +1,74 @@ +defmodule Mv.Authorization.Checks.ReadLinkedForOwnData do + @moduledoc """ + Generic policy check for resources that link to a member via a member-id + attribute: read is allowed only when the actor has the "own_data" permission + set AND `record. == actor.member_id`. + + Used in a read bypass so that own_data gets the linked filter (via auto_filter + for list queries), while admin with a member_id does not match and falls + through to `HasPermission` for `:all`. + + - With a record (e.g. get by id): returns true only when own_data and the + member ids match. + - Without a record (list query) + own_data: returns `:unknown` so the + authorizer applies `auto_filter`. + + ## Options + - `:member_id_field` - the attribute on the resource holding the member id. + Defaults to `:member_id`. + """ + use Ash.Policy.Check + + alias Mv.Authorization.Checks.ActorPermissionSetIs + + @impl true + def type, do: :filter + + @impl true + def describe(opts) do + "own_data can read only records where #{member_id_field(opts)} == actor.member_id" + end + + @impl true + def strict_check(actor, authorizer, opts) do + field = member_id_field(opts) + record = get_record_from_authorizer(authorizer) + is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") + + cond do + is_nil(record) and is_own_data -> + {:ok, :unknown} + + is_nil(record) -> + {:ok, false} + + not is_own_data -> + {:ok, false} + + Map.get(record, field) == actor.member_id -> + {:ok, true} + + true -> + {:ok, false} + end + end + + @impl true + def auto_filter(actor, _authorizer, opts) do + if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && + Map.get(actor, :member_id) do + [{member_id_field(opts), actor.member_id}] + else + [] + end + end + + defp member_id_field(opts), do: Keyword.get(opts, :member_id_field, :member_id) + + defp get_record_from_authorizer(authorizer) do + case authorizer.subject do + %{data: data} when not is_nil(data) -> data + _ -> nil + end + end +end