Merge remote-tracking branch 'origin/main' into feature/372-groups-management

lib/accounts/user.ex

@@ -86,7 +86,13 @@ defmodule Mv.Accounts.User do
     #       - :create_user (for manual user creation with optional member link)
     #       - :register_with_password (for password-based registration)
     #       - :register_with_rauthy (for OIDC-based registration)
-    defaults [:read, :destroy]
+    defaults [:read]
+
+    destroy :destroy do
+      primary? true
+      # Required because custom validation (system actor protection) cannot run atomically
+      require_atomic? false
+    end
 
     # Primary generic update action:
     # - Selected by AshAdmin's generated "Edit" UI and generic AshPhoenix
@@ -169,6 +175,13 @@ defmodule Mv.Accounts.User do
       end
     end
 
+    # Internal update used only by SystemActor/bootstrap and tests to assign role to system user.
+    # Not protected by system-user validation so bootstrap can run.
+    update :update_internal do
+      accept []
+      require_atomic? false
+    end
+
     # Admin action for direct password changes in admin panel
     # Uses the official Ash Authentication HashPasswordChange with correct context
     update :admin_set_password do
@@ -181,6 +194,11 @@ defmodule Mv.Accounts.User do
 
       # Use the official Ash Authentication password change
       change AshAuthentication.Strategy.Password.HashPasswordChange
+
+      # Sync email changes to linked member when email is changed (e.g. form changes both)
+      change Mv.EmailSync.Changes.SyncUserEmailToMember do
+        where [changing(:email)]
+      end
     end
 
     # Action to link an OIDC account to an existing password-only user
@@ -359,6 +377,21 @@ defmodule Mv.Accounts.User do
         :ok
       end
     end
+
+    # Prevent modification of the system actor user (required for internal operations).
+    # Block update/destroy on UI-exposed actions only; :update_internal is used by bootstrap/tests.
+    validate fn changeset, _context ->
+               if Mv.Helpers.SystemActor.system_user?(changeset.data) do
+                 {:error,
+                  field: :email,
+                  message:
+                    "Cannot modify system actor user. This user is required for internal operations."}
+               else
+                 :ok
+               end
+             end,
+             on: [:update, :destroy],
+             where: [action_is([:update, :update_user, :admin_set_password, :destroy])]
   end
 
   def validate_oidc_id_present(changeset, _context) do

lib/mv/helpers/system_actor.ex

@@ -172,6 +172,31 @@ defmodule Mv.Helpers.SystemActor do
     end
   end
 
+  @doc """
+  Returns whether the given user is the system actor user (case-insensitive email match).
+
+  Use this instead of ad-hoc `to_string(user.email) == system_user_email()` so
+  comparisons are consistent and case-insensitive everywhere.
+
+  ## Returns
+
+  - `boolean()` - true if user's email matches system user email (case-insensitive)
+
+  ## Examples
+
+      iex> Mv.Helpers.SystemActor.system_user?(user_with_system_email)
+      true
+      iex> Mv.Helpers.SystemActor.system_user?(other_user)
+      false
+
+  """
+  @spec system_user?(Mv.Accounts.User.t() | map() | nil) :: boolean()
+  def system_user?(%{email: email}) when not is_nil(email) do
+    normalized_email(to_string(email)) == normalized_system_user_email()
+  end
+
+  def system_user?(_), do: false
+
   @doc """
   Returns the email address of the system user.
 
@@ -191,6 +216,11 @@ defmodule Mv.Helpers.SystemActor do
   @spec system_user_email() :: String.t()
   def system_user_email, do: system_user_email_config()
 
+  # Case-insensitive normalized form for comparisons
+  defp normalized_system_user_email, do: normalized_email(system_user_email_config())
+
+  defp normalized_email(email) when is_binary(email), do: String.downcase(email)
+
   # Returns the system user email from environment variable or default
   # This allows configuration via SYSTEM_ACTOR_EMAIL env var
   @spec system_user_email_config() :: String.t()
@@ -368,7 +398,7 @@ defmodule Mv.Helpers.SystemActor do
       upsert_identity: :unique_email,
       authorize?: false
     )
-    |> Ash.Changeset.for_update(:update, %{})
+    |> Ash.Changeset.for_update(:update_internal, %{})
     |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
     |> Ash.update!(authorize?: false)
     |> Ash.load!(:role, domain: Mv.Accounts, authorize?: false)

lib/mv_web/error_helpers.ex

@@ -0,0 +1,25 @@
+defmodule MvWeb.ErrorHelpers do
+  @moduledoc """
+  Shared helpers for formatting errors in the web layer.
+
+  Use `format_ash_error/1` for Ash errors so behaviour stays consistent
+  (e.g. handling Invalid errors whose entries may lack a `:message` field).
+  """
+
+  @doc """
+  Formats an Ash error for display to the user.
+
+  Handles `Ash.Error.Invalid` by joining error messages; entries without
+  a `:message` field are inspected to avoid FunctionClauseError.
+  Other errors are inspected.
+  """
+  @spec format_ash_error(Ash.Error.t() | term()) :: String.t()
+  def format_ash_error(%Ash.Error.Invalid{errors: errors}) when is_list(errors) do
+    Enum.map_join(errors, ", ", fn
+      %{message: message} when is_binary(message) -> message
+      other -> inspect(other)
+    end)
+  end
+
+  def format_ash_error(error), do: inspect(error)
+end

lib/mv_web/live/user_live/form.ex

@@ -264,12 +264,31 @@ defmodule MvWeb.UserLive.Form do
   def mount(params, _session, socket) do
     actor = current_actor(socket)
 
-    user =
-      case params["id"] do
-        nil -> nil
-        id -> Ash.get!(Mv.Accounts.User, id, domain: Mv.Accounts, load: [:member], actor: actor)
-      end
+    case load_user_or_redirect(params["id"], actor, socket) do
+      {:redirect, socket} ->
+        {:ok, socket}
 
+      {:ok, user} ->
+        mount_continue(user, params, socket)
+    end
+  end
+
+  defp load_user_or_redirect(nil, _actor, _socket), do: {:ok, nil}
+
+  defp load_user_or_redirect(id, actor, socket) do
+    user = Ash.get!(Mv.Accounts.User, id, domain: Mv.Accounts, load: [:member], actor: actor)
+
+    if Mv.Helpers.SystemActor.system_user?(user) do
+      {:redirect,
+       socket
+       |> put_flash(:error, gettext("This user cannot be edited."))
+       |> push_navigate(to: ~p"/users")}
+    else
+      {:ok, user}
+    end
+  end
+
+  defp mount_continue(user, params, socket) do
     action = if is_nil(user), do: gettext("New"), else: gettext("Edit")
     page_title = action <> " " <> gettext("User")
 

lib/mv_web/live/user_live/index.ex

@@ -25,10 +25,18 @@ defmodule MvWeb.UserLive.Index do
 
   import MvWeb.LiveHelpers, only: [current_actor: 1]
 
+  require Ash.Query
+  import MvWeb.ErrorHelpers, only: [format_ash_error: 1]
+
   @impl true
   def mount(_params, _session, socket) do
     actor = current_actor(socket)
-    users = Ash.read!(Mv.Accounts.User, domain: Mv.Accounts, load: [:member], actor: actor)
+
+    users =
+      Mv.Accounts.User
+      |> Ash.Query.filter(email != ^Mv.Helpers.SystemActor.system_user_email())
+      |> Ash.read!(domain: Mv.Accounts, load: [:member], actor: actor)
+
     sorted = Enum.sort_by(users, & &1.email)
 
     {:ok,
@@ -64,7 +72,7 @@ defmodule MvWeb.UserLive.Index do
              )}
 
           {:error, error} ->
-            {:noreply, put_flash(socket, :error, format_error(error))}
+            {:noreply, put_flash(socket, :error, format_ash_error(error))}
         end
 
       {:error, %Ash.Error.Query.NotFound{}} ->
@@ -75,7 +83,7 @@ defmodule MvWeb.UserLive.Index do
          put_flash(socket, :error, gettext("You do not have permission to access this user"))}
 
       {:error, error} ->
-        {:noreply, put_flash(socket, :error, format_error(error))}
+        {:noreply, put_flash(socket, :error, format_ash_error(error))}
     end
   end
 
@@ -137,12 +145,4 @@ defmodule MvWeb.UserLive.Index do
   defp toggle_order(:desc), do: :asc
   defp sort_fun(:asc), do: &<=/2
   defp sort_fun(:desc), do: &>=/2
-
-  defp format_error(%Ash.Error.Invalid{errors: errors}) do
-    Enum.map_join(errors, ", ", fn %{message: message} -> message end)
-  end
-
-  defp format_error(error) do
-    inspect(error)
-  end
 end

lib/mv_web/live/user_live/show.ex

@@ -75,9 +75,16 @@ defmodule MvWeb.UserLive.Show do
     actor = current_actor(socket)
     user = Ash.get!(Mv.Accounts.User, id, domain: Mv.Accounts, load: [:member], actor: actor)
 
-    {:ok,
-     socket
-     |> assign(:page_title, gettext("Show User"))
-     |> assign(:user, user)}
+    if Mv.Helpers.SystemActor.system_user?(user) do
+      {:ok,
+       socket
+       |> put_flash(:error, gettext("This user cannot be viewed."))
+       |> push_navigate(to: ~p"/users")}
+    else
+      {:ok,
+       socket
+       |> assign(:page_title, gettext("Show User"))
+       |> assign(:user, user)}
+    end
   end
 end