From 5a2f035eccf9cc76d9993ae123bbe064eda8b072 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 29 Jan 2026 15:30:07 +0100 Subject: [PATCH] CustomField policies: actor required, no system-actor fallback, error handling - list_required_custom_fields: require actor (two clauses, no default) - Member validation: use context.actor only, differentiate Forbidden vs transient errors - stream_custom_fields: log + send flash on error instead of returning [] - GlobalSettingsLive: handle_info for custom_fields_load_error, put_flash - Seeds: use Membership.update_member with actor, format --- lib/membership/member.ex | 18 +++++++++++++ lib/membership/membership.ex | 16 +++++++----- .../live/custom_field_live/index_component.ex | 18 ++++++++++--- lib/mv_web/live/global_settings_live.ex | 9 +++++++ priv/repo/seeds.exs | 25 ++++++++++++------- 5 files changed, 67 insertions(+), 19 deletions(-) diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 6de7c77..7b49c86 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -486,6 +486,24 @@ defmodule Mv.Membership.Member do build_custom_field_validation_error(missing_fields) end + {:error, %Ash.Error.Forbidden{}} -> + Logger.warning( + "Required custom fields validation: actor not authorized to read CustomField" + ) + + {:error, + field: :custom_field_values, + message: + "You are not authorized to perform this action. Please sign in again or contact support."} + + {:error, :missing_actor} -> + Logger.warning("Required custom fields validation: no actor in context") + + {:error, + field: :custom_field_values, + message: + "You are not authorized to perform this action. Please sign in again or contact support."} + {:error, error} -> Logger.error( "Failed to load custom fields for validation: #{inspect(error)}. Required field validation cannot be performed." diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 3f84194..74735e4 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -14,7 +14,7 @@ defmodule Mv.Membership do The domain exposes these main actions: - Member CRUD: `create_member/1`, `list_members/0`, `update_member/2`, `destroy_member/1` - Custom field value management: `create_custom_field_value/1`, `list_custom_field_values/0`, etc. - - Custom field management: `create_custom_field/1`, `list_custom_fields/0`, `list_required_custom_fields/0`, etc. + - Custom field management: `create_custom_field/1`, `list_custom_fields/0`, `list_required_custom_fields/1`, etc. - Settings management: `get_settings/0`, `update_settings/2`, `update_member_field_visibility/2`, `update_single_member_field_visibility/3` - Group management: `create_group/1`, `list_groups/0`, `update_group/2`, `destroy_group/1` - Member-group associations: `create_member_group/1`, `list_member_groups/0`, `destroy_member_group/1` @@ -156,7 +156,7 @@ defmodule Mv.Membership do This is an optimized version that filters at the database level instead of loading all custom fields and filtering in memory. Requires an actor for - authorization (CustomField read policy). + authorization (CustomField read policy). Callers must pass `actor:`; no default. ## Options @@ -166,22 +166,26 @@ defmodule Mv.Membership do ## Returns - `{:ok, required_custom_fields}` - List of required custom fields - - `{:error, error}` - Error reading custom fields (e.g. Forbidden when no actor) + - `{:error, :missing_actor}` - When actor is nil (caller must pass actor) + - `{:error, error}` - Error reading custom fields (e.g. Forbidden) ## Examples iex> {:ok, required_fields} = Mv.Membership.list_required_custom_fields(actor: actor) iex> Enum.all?(required_fields, & &1.required) true - """ - def list_required_custom_fields(opts \\ []) do - actor = Keyword.get(opts, :actor) + iex> Mv.Membership.list_required_custom_fields(actor: nil) + {:error, :missing_actor} + """ + def list_required_custom_fields(actor: actor) when not is_nil(actor) do Mv.Membership.CustomField |> Ash.Query.filter(expr(required == true)) |> Ash.read(domain: __MODULE__, actor: actor) end + def list_required_custom_fields(actor: nil), do: {:error, :missing_actor} + @doc """ Updates the member field visibility configuration. diff --git a/lib/mv_web/live/custom_field_live/index_component.ex b/lib/mv_web/live/custom_field_live/index_component.ex index 1e0bb3f..784d1ef 100644 --- a/lib/mv_web/live/custom_field_live/index_component.ex +++ b/lib/mv_web/live/custom_field_live/index_component.ex @@ -12,6 +12,8 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do """ use MvWeb, :live_component + require Logger + @impl true def render(assigns) do assigns = assign(assigns, :field_type_label, &MvWeb.Translations.FieldTypes.label/1) @@ -177,10 +179,18 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do """ end - defp stream_custom_fields(actor) do + defp stream_custom_fields(actor, parent_pid) do case Ash.read(Mv.Membership.CustomField, actor: actor) do - {:ok, custom_fields} -> custom_fields - {:error, _} -> [] + {:ok, custom_fields} -> + custom_fields + + {:error, error} -> + Logger.warning( + "CustomFieldLive.IndexComponent: failed to load custom fields: #{inspect(error)}" + ) + + send(parent_pid, {:custom_fields_load_error, error}) + [] end end @@ -215,7 +225,7 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do |> assign_new(:show_delete_modal, fn -> false end) |> assign_new(:custom_field_to_delete, fn -> nil end) |> assign_new(:slug_confirmation, fn -> "" end) - |> stream(:custom_fields, stream_custom_fields(assigns[:actor]), reset: true)} + |> stream(:custom_fields, stream_custom_fields(assigns[:actor], self()), reset: true)} end @impl true diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index 7b29cae..bd0036b 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -504,6 +504,15 @@ defmodule MvWeb.GlobalSettingsLive do {:noreply, put_flash(socket, :error, gettext("Slug does not match. Deletion cancelled."))} end + def handle_info({:custom_fields_load_error, _error}, socket) do + {:noreply, + put_flash( + socket, + :error, + gettext("Could not load data fields. Please check your permissions.") + )} + end + @impl true def handle_info({:editing_section_changed, section}, socket) do {:noreply, assign(socket, :active_editing_section, section)} diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 71ae3ff..4240336 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -381,12 +381,16 @@ Enum.each(member_attrs_list, fn member_attrs -> final_member = if is_nil(member.membership_fee_type_id) and Map.has_key?(member_attrs_without_status, :membership_fee_type_id) do - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: member_attrs_without_status.membership_fee_type_id - }) - |> Ash.Changeset.put_context(:actor, admin_user_with_role) - |> Ash.update!(actor: admin_user_with_role) + {:ok, updated} = + Membership.update_member( + member, + %{ + membership_fee_type_id: member_attrs_without_status.membership_fee_type_id + }, + actor: admin_user_with_role + ) + + updated else member end @@ -546,9 +550,12 @@ Enum.with_index(linked_members) fee_type_index = rem(3 + index, length(all_fee_types)) fee_type = Enum.at(all_fee_types, fee_type_index) - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: admin_user_with_role) + {:ok, updated} = + Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, + actor: admin_user_with_role + ) + + updated else member end