diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index bcaf506..6e450fb 100644 --- a/lib/accounts/user.ex +++ b/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 @@ -181,6 +187,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 +370,19 @@ defmodule Mv.Accounts.User do :ok end end + + # Prevent deletion of the system actor user (required for internal operations) + validate fn changeset, _context -> + if to_string(changeset.data.email) == Mv.Helpers.SystemActor.system_user_email() do + {:error, + field: :email, + message: + "Cannot delete system actor user. This user is required for internal operations."} + else + :ok + end + end, + on: [:destroy] end def validate_oidc_id_present(changeset, _context) do diff --git a/lib/mv_web/live/user_live/form.ex b/lib/mv_web/live/user_live/form.ex index 6cf3f0f..28af7c4 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -266,10 +266,31 @@ defmodule MvWeb.UserLive.Form do user = case params["id"] do - nil -> nil - id -> Ash.get!(Mv.Accounts.User, id, domain: Mv.Accounts, load: [:member], actor: actor) + nil -> + nil + + id -> + loaded = + Ash.get!(Mv.Accounts.User, id, domain: Mv.Accounts, load: [:member], actor: actor) + + if to_string(loaded.email) == Mv.Helpers.SystemActor.system_user_email() do + {:redirect, loaded} + else + loaded + end end + if match?({:redirect, _}, user) do + {:ok, + socket + |> put_flash(:error, gettext("This user cannot be edited.")) + |> push_navigate(to: ~p"/users")} + else + mount_continue(user, params, socket) + 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") diff --git a/lib/mv_web/live/user_live/index.ex b/lib/mv_web/live/user_live/index.ex index 2062ae6..ef3583e 100644 --- a/lib/mv_web/live/user_live/index.ex +++ b/lib/mv_web/live/user_live/index.ex @@ -25,10 +25,17 @@ defmodule MvWeb.UserLive.Index do import MvWeb.LiveHelpers, only: [current_actor: 1] + require Ash.Query + @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, @@ -138,8 +145,11 @@ defmodule MvWeb.UserLive.Index do 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) + defp format_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 defp format_error(error) do diff --git a/lib/mv_web/live/user_live/show.ex b/lib/mv_web/live/user_live/show.ex index 641e091..fe2dd24 100644 --- a/lib/mv_web/live/user_live/show.ex +++ b/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 to_string(user.email) == Mv.Helpers.SystemActor.system_user_email() 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 diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 5496213..865dff4 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -2125,3 +2125,13 @@ msgstr "E-Mail" #, elixir-autogen, elixir-format msgid "email %{email} has already been taken" msgstr "E-Mail %{email} wurde bereits verwendet" + +#: lib/mv_web/live/user_live/form.ex +#, elixir-autogen, elixir-format +msgid "This user cannot be edited." +msgstr "Dieser Benutzer kann nicht bearbeitet werden." + +#: lib/mv_web/live/user_live/show.ex +#, elixir-autogen, elixir-format +msgid "This user cannot be viewed." +msgstr "Dieser Benutzer kann nicht angezeigt werden." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index fc3a78c..e63621d 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2126,3 +2126,13 @@ msgstr "" #, elixir-autogen, elixir-format msgid "email %{email} has already been taken" msgstr "" + +#: lib/mv_web/live/user_live/form.ex +#, elixir-autogen, elixir-format +msgid "This user cannot be edited." +msgstr "" + +#: lib/mv_web/live/user_live/show.ex +#, elixir-autogen, elixir-format +msgid "This user cannot be viewed." +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 9432a47..15c6f9a 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2126,3 +2126,13 @@ msgstr "" #, elixir-autogen, elixir-format msgid "email %{email} has already been taken" msgstr "" + +#: lib/mv_web/live/user_live/form.ex +#, elixir-autogen, elixir-format +msgid "This user cannot be edited." +msgstr "" + +#: lib/mv_web/live/user_live/show.ex +#, elixir-autogen, elixir-format +msgid "This user cannot be viewed." +msgstr "" diff --git a/priv/repo/migrations/20260127124937_ensure_system_actor_user_exists.exs b/priv/repo/migrations/20260127124937_ensure_system_actor_user_exists.exs new file mode 100644 index 0000000..b0ee775 --- /dev/null +++ b/priv/repo/migrations/20260127124937_ensure_system_actor_user_exists.exs @@ -0,0 +1,59 @@ +defmodule Mv.Repo.Migrations.EnsureSystemActorUserExists do + @moduledoc """ + Ensures the system actor user always exists. + + The system actor is used for systemic operations (email sync, cycle generation, + background jobs). It is created by seeds in development; in production seeds + may not run, so this migration guarantees the user exists. + + Creates a user with email "system@mila.local" (default from Mv.Helpers.SystemActor) + and the Admin role. The user has no password and no OIDC ID, so it cannot log in. + """ + use Ecto.Migration + import Ecto.Query + + @system_user_email "system@mila.local" + + def up do + admin_role_id = ensure_admin_role_exists() + ensure_system_actor_user_exists(admin_role_id) + end + + def down do + # Not reversible - do not delete system user on rollback + :ok + end + + defp ensure_admin_role_exists do + case repo().one(from(r in "roles", where: r.name == "Admin", select: r.id)) do + nil -> + execute(""" + INSERT INTO roles (id, name, description, permission_set_name, is_system_role, inserted_at, updated_at) + VALUES (uuid_generate_v7(), 'Admin', 'Administrator with full access', 'admin', false, (now() AT TIME ZONE 'utc'), (now() AT TIME ZONE 'utc')) + """) + + role_id = repo().one(from(r in "roles", where: r.name == "Admin", select: r.id)) + IO.puts("✅ Created 'Admin' role (was missing)") + role_id + + role_id -> + role_id + end + end + + defp ensure_system_actor_user_exists(_admin_role_id) do + case repo().one(from(u in "users", where: u.email == ^@system_user_email, select: u.id)) do + nil -> + execute(""" + INSERT INTO users (id, email, hashed_password, oidc_id, member_id, role_id) + SELECT gen_random_uuid(), '#{@system_user_email}', NULL, NULL, NULL, r.id + FROM roles r WHERE r.name = 'Admin' LIMIT 1 + """) + + IO.puts("✅ Created system actor user (#{@system_user_email})") + + _ -> + :ok + end + end +end diff --git a/test/accounts/user_email_sync_test.exs b/test/accounts/user_email_sync_test.exs index d324783..eab4e38 100644 --- a/test/accounts/user_email_sync_test.exs +++ b/test/accounts/user_email_sync_test.exs @@ -120,6 +120,43 @@ defmodule Mv.Accounts.UserEmailSyncTest do {:ok, member_after_unlink} = Ash.get(Mv.Membership.Member, member.id, actor: actor) assert member_after_unlink.email == "user@example.com" end + + test "admin_set_password with email change syncs to linked member", %{actor: actor} do + # Create member and user linked to it (with password so admin_set_password applies) + {:ok, member} = Membership.create_member(@valid_member_attrs, actor: actor) + + {:ok, user} = + Mv.Accounts.User + |> Ash.Changeset.for_create(:register_with_password, %{ + email: "user@example.com", + password: "initialpass123" + }) + |> Ash.create(actor: actor) + + {:ok, user} = + user + |> Ash.Changeset.for_update(:update_user, %{member: %{id: member.id}}) + |> Ash.update(actor: actor) + + assert user.member_id == member.id + {:ok, m} = Ash.get(Mv.Membership.Member, member.id, actor: actor) + assert m.email == "user@example.com" + + # Change both email and password via admin_set_password (e.g. user form "Change Password") + {:ok, updated_user} = + user + |> Ash.Changeset.for_update(:admin_set_password, %{ + email: "newemail@example.com", + password: "newpassword123" + }) + |> Ash.update(actor: actor) + + assert to_string(updated_user.email) == "newemail@example.com" + + # Member email must be synced (Option A: SyncUserEmailToMember on admin_set_password) + {:ok, synced_member} = Ash.get(Mv.Membership.Member, member.id, actor: actor) + assert synced_member.email == "newemail@example.com" + end end describe "AshAuthentication compatibility" do diff --git a/test/mv/helpers/system_actor_test.exs b/test/mv/helpers/system_actor_test.exs index af28443..e676130 100644 --- a/test/mv/helpers/system_actor_test.exs +++ b/test/mv/helpers/system_actor_test.exs @@ -10,6 +10,14 @@ defmodule Mv.Helpers.SystemActorTest do require Ash.Query + # Deletes a user row directly via SQL, bypassing Ash validations. + # Use only in tests when setting up "no system user" / "no users" scenarios; + # Ash.destroy! forbids deleting the system actor user. + defp delete_user_bypass_ash(user) do + id = Ecto.UUID.dump!(user.id) + Ecto.Adapters.SQL.query!(Mv.Repo, "DELETE FROM users WHERE id = $1", [id]) + end + # Helper function to ensure admin role exists defp ensure_admin_role do case Authorization.list_roles() do @@ -124,7 +132,7 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.Query.filter(email == ^"system@mila.local") |> Ash.read_one(domain: Mv.Accounts, actor: system_actor) do {:ok, user} when not is_nil(user) -> - Ash.destroy!(user, domain: Mv.Accounts, actor: system_actor) + delete_user_bypass_ash(user) _ -> :ok @@ -163,7 +171,7 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.Query.filter(email == ^"system@mila.local") |> Ash.read_one(domain: Mv.Accounts, actor: system_actor) do {:ok, user} when not is_nil(user) -> - Ash.destroy!(user, domain: Mv.Accounts, actor: system_actor) + delete_user_bypass_ash(user) _ -> :ok @@ -177,7 +185,7 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.Query.filter(email == ^admin_email) |> Ash.read_one(domain: Mv.Accounts, actor: system_actor) do {:ok, user} when not is_nil(user) -> - Ash.destroy!(user, domain: Mv.Accounts, actor: system_actor) + delete_user_bypass_ash(user) _ -> :ok @@ -227,7 +235,7 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.Query.filter(email == ^"system@mila.local") |> Ash.read_one(domain: Mv.Accounts, actor: system_actor) do {:ok, user} when not is_nil(user) -> - Ash.destroy!(user, domain: Mv.Accounts, actor: system_actor) + delete_user_bypass_ash(user) _ -> :ok @@ -241,7 +249,7 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.Query.filter(email == ^admin_email) |> Ash.read_one(domain: Mv.Accounts, actor: system_actor) do {:ok, user} when not is_nil(user) -> - Ash.destroy!(user, domain: Mv.Accounts, actor: system_actor) + delete_user_bypass_ash(user) _ -> :ok @@ -275,7 +283,7 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.Query.filter(email == ^"system@mila.local") |> Ash.read_one(domain: Mv.Accounts, actor: system_actor) do {:ok, user} when not is_nil(user) -> - Ash.destroy!(user, domain: Mv.Accounts, actor: system_actor) + delete_user_bypass_ash(user) _ -> :ok @@ -314,7 +322,7 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.Query.filter(email == ^"system@mila.local") |> Ash.read_one(domain: Mv.Accounts, actor: system_actor) do {:ok, user} when not is_nil(user) -> - Ash.destroy!(user, domain: Mv.Accounts, actor: system_actor) + delete_user_bypass_ash(user) _ -> :ok @@ -328,7 +336,7 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.Query.filter(email == ^admin_email) |> Ash.read_one(domain: Mv.Accounts, actor: system_actor) do {:ok, user} when not is_nil(user) -> - Ash.destroy!(user, domain: Mv.Accounts, actor: system_actor) + delete_user_bypass_ash(user) _ -> :ok diff --git a/test/mv_web/live/user_live/show_test.exs b/test/mv_web/live/user_live/show_test.exs index 3551fdf..9518106 100644 --- a/test/mv_web/live/user_live/show_test.exs +++ b/test/mv_web/live/user_live/show_test.exs @@ -154,4 +154,14 @@ defmodule MvWeb.UserLive.ShowTest do assert html =~ gettext("Show User") || html =~ to_string(user.email) end end + + describe "system actor user" do + test "redirects to user list when viewing system actor user", %{conn: conn} do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + conn = conn_with_oidc_user(conn) + + assert {:error, {:live_redirect, %{to: "/users"}}} = + live(conn, ~p"/users/#{system_actor.id}") + end + end end diff --git a/test/mv_web/user_live/form_test.exs b/test/mv_web/user_live/form_test.exs index ed309fb..4b76c19 100644 --- a/test/mv_web/user_live/form_test.exs +++ b/test/mv_web/user_live/form_test.exs @@ -420,4 +420,14 @@ defmodule MvWeb.UserLive.FormTest do assert is_nil(updated_user.member) end end + + describe "system actor user" do + test "redirects to user list when editing system actor user", %{conn: conn} do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + conn = conn_with_oidc_user(conn, %{email: "admin@example.com"}) + + assert {:error, {:live_redirect, %{to: "/users"}}} = + live(conn, "/users/#{system_actor.id}/edit") + end + end end diff --git a/test/mv_web/user_live/index_test.exs b/test/mv_web/user_live/index_test.exs index 41c198d..a1a02ea 100644 --- a/test/mv_web/user_live/index_test.exs +++ b/test/mv_web/user_live/index_test.exs @@ -405,6 +405,27 @@ defmodule MvWeb.UserLive.IndexTest do end end + describe "system actor user" do + test "does not show system actor user in list", %{conn: conn} do + # Ensure system actor exists (e.g. via get_system_actor in conn_with_oidc_user) + _system_actor = Mv.Helpers.SystemActor.get_system_actor() + system_email = Mv.Helpers.SystemActor.system_user_email() + + conn = conn_with_oidc_user(conn) + {:ok, _view, html} = live(conn, "/users") + + refute html =~ system_email, + "System actor user (#{system_email}) must not appear in the user list" + end + + test "destroying system actor user returns error", %{current_user: current_user} do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + + assert {:error, %Ash.Error.Invalid{}} = + Ash.destroy(system_actor, domain: Mv.Accounts, actor: current_user) + end + end + describe "member linking display" do test "displays linked member name in user list", %{conn: conn} do system_actor = Mv.Helpers.SystemActor.get_system_actor()