User form: persist role, member linking, Forbidden handling

- User resource: update_user accepts role_id, manage_relationship :member
- user_live/form: touch role_id, params_with_member_if_unchanged to avoid unlink
- Handle Forbidden in form, extract error message for display
- user_policies_test and form_test coverage

lib/accounts/user.ex

@@ -8,6 +8,9 @@ defmodule Mv.Accounts.User do
     extensions: [AshAuthentication],
     authorizers: [Ash.Policy.Authorizer]
 
+  require Ash.Query
+  import Ash.Expr
+
   postgres do
     table "users"
     repo Mv.Repo
@@ -146,9 +149,10 @@ defmodule Mv.Accounts.User do
 
     update :update_user do
       description "Updates a user and manages the optional member relationship. To change an existing member link, first remove it (set member to nil), then add the new one."
-      # Only accept email directly - member_id is NOT in accept list
-      # This prevents direct foreign key manipulation, forcing use of manage_relationship
-      accept [:email]
+
+      # Accept email and role_id (role_id only used by admins; policy restricts update_user to admins).
+      # member_id is NOT in accept list - use argument :member for relationship management.
+      accept [:email, :role_id]
       # Allow member to be passed as argument for relationship management
       argument :member, :map, allow_nil?: true
 
@@ -387,6 +391,49 @@ defmodule Mv.Accounts.User do
       end
     end
 
+    # Last-admin: prevent the only admin from changing their role (at least one admin required).
+    validate fn changeset, _context ->
+               if Ash.Changeset.changing_attribute?(changeset, :role_id) do
+                 current_role_id = changeset.data.role_id
+
+                 current_role =
+                   Mv.Authorization.Role
+                   |> Ash.get!(current_role_id, authorize?: false)
+
+                 if current_role.permission_set_name != "admin" do
+                   :ok
+                 else
+                   admin_role_ids =
+                     Mv.Authorization.Role
+                     |> Ash.Query.for_read(:read)
+                     |> Ash.Query.filter(expr(permission_set_name == "admin"))
+                     |> Ash.read!(authorize?: false)
+                     |> Enum.map(& &1.id)
+
+                   # Count only non-system users with admin role (system user is for internal ops)
+                   system_email = Mv.Helpers.SystemActor.system_user_email()
+
+                   count =
+                     Mv.Accounts.User
+                     |> Ash.Query.for_read(:read)
+                     |> Ash.Query.filter(expr(role_id in ^admin_role_ids))
+                     |> Ash.Query.filter(expr(email != ^system_email))
+                     |> Ash.count!(authorize?: false)
+
+                   if count <= 1 do
+                     {:error,
+                      field: :role_id, message: "At least one user must keep the Admin role."}
+                   else
+                     :ok
+                   end
+                 end
+               else
+                 :ok
+               end
+             end,
+             on: [:update],
+             where: [action_is(:update_user)]
+
     # 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 ->

lib/mv_web/live/user_live/form.ex

@@ -35,6 +35,8 @@ defmodule MvWeb.UserLive.Form do
 
   require Jason
 
+  alias Mv.Authorization
+
   import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3]
   import MvWeb.Authorization, only: [can?: 3]
 
@@ -49,6 +51,18 @@ defmodule MvWeb.UserLive.Form do
 
       <.form class="max-w-xl" for={@form} id="user-form" phx-change="validate" phx-submit="save">
         <.input field={@form[:email]} label={gettext("Email")} required type="email" />
+
+        <%= if @user && @can_assign_role do %>
+          <div class="mt-4">
+            <.input
+              field={@form[:role_id]}
+              type="select"
+              label={gettext("Role")}
+              options={Enum.map(@roles, &{&1.name, &1.id})}
+              prompt={gettext("Select role...")}
+            />
+          </div>
+        <% end %>
         
     <!-- Password Section -->
         <div class="mt-6">
@@ -300,6 +314,9 @@ defmodule MvWeb.UserLive.Form do
 
     # Only admins can link/unlink users to members (permission docs; prevents privilege escalation).
     can_manage_member_linking = can?(actor, :destroy, Mv.Accounts.User)
+    # Only admins can assign user roles (Role update permission).
+    can_assign_role = can?(actor, :update, Mv.Authorization.Role)
+    roles = if can_assign_role, do: load_roles(actor), else: []
 
     {:ok,
      socket
@@ -307,6 +324,8 @@ defmodule MvWeb.UserLive.Form do
      |> assign(user: user)
      |> assign(:page_title, page_title)
      |> assign(:can_manage_member_linking, can_manage_member_linking)
+     |> assign(:can_assign_role, can_assign_role)
+     |> assign(:roles, roles)
      |> assign(:show_password_fields, false)
      |> assign(:member_search_query, "")
      |> assign(:available_members, [])
@@ -357,7 +376,10 @@ defmodule MvWeb.UserLive.Form do
   def handle_event("save", %{"user" => user_params}, socket) do
     actor = current_actor(socket)
 
-    # First save the user without member changes
+    # Include current member in params when not linking/unlinking so update_user's
+    # manage_relationship(on_missing: :unrelate) does not accidentally unlink.
+    user_params = params_with_member_if_unchanged(socket, user_params)
+
     case submit_form(socket.assigns.form, user_params, actor) do
       {:ok, user} ->
         handle_member_linking(socket, user, actor)
@@ -529,6 +551,20 @@ defmodule MvWeb.UserLive.Form do
   defp get_action_name(:update), do: gettext("updated")
   defp get_action_name(other), do: to_string(other)
 
+  # When user has a linked member and we are not linking/unlinking, include current member in params
+  # so update_user's manage_relationship(on_missing: :unrelate) does not unlink the member.
+  defp params_with_member_if_unchanged(socket, params) do
+    user = socket.assigns.user
+    linking = socket.assigns.selected_member_id
+    unlinking = socket.assigns[:unlink_member]
+
+    if user && user.member_id && !linking && !unlinking do
+      Map.put(params, "member", %{"id" => user.member_id})
+    else
+      params
+    end
+  end
+
   defp handle_member_link_error(socket, error) do
     error_message = extract_error_message(error)
 
@@ -572,7 +608,8 @@ defmodule MvWeb.UserLive.Form do
            assigns: %{
              user: user,
              show_password_fields: show_password_fields,
-             can_manage_member_linking: can_manage_member_linking
+             can_manage_member_linking: can_manage_member_linking,
+             can_assign_role: can_assign_role
            }
          } = socket
        ) do
@@ -580,16 +617,25 @@ defmodule MvWeb.UserLive.Form do
 
     form =
       if user do
-        # For existing users: admin uses update_user (email + member); non-admin uses update (email only).
+        # For existing users: admin uses update_user (email + member + role_id); non-admin uses update (email only).
         # Password change uses admin_set_password for both.
         action =
           cond do
             show_password_fields -> :admin_set_password
-            can_manage_member_linking -> :update_user
+            can_manage_member_linking or can_assign_role -> :update_user
             true -> :update
           end
 
-        AshPhoenix.Form.for_update(user, action, domain: Mv.Accounts, as: "user", actor: actor)
+        form =
+          AshPhoenix.Form.for_update(user, action, domain: Mv.Accounts, as: "user", actor: actor)
+
+        # Ensure role_id is always included on submit when role dropdown is shown (AshPhoenix.Form
+        # only submits keys in touched_forms; marking as touched avoids role change being dropped).
+        if can_assign_role and action == :update_user do
+          AshPhoenix.Form.touch(form, [:role_id])
+        else
+          form
+        end
       else
         # For new users, use password registration if password fields are shown
         action = if show_password_fields, do: :register_with_password, else: :create_user
@@ -668,6 +714,14 @@ defmodule MvWeb.UserLive.Form do
     Mv.Membership.Member.filter_by_email_match(members, user_email_str)
   end
 
+  @spec load_roles(any()) :: [Mv.Authorization.Role.t()]
+  defp load_roles(actor) do
+    case Authorization.list_roles(actor: actor) do
+      {:ok, roles} -> roles
+      {:error, _} -> []
+    end
+  end
+
   # Extract user-friendly error message from Ash.Error
   @spec extract_error_message(any()) :: String.t()
   defp extract_error_message(%Ash.Error.Invalid{errors: errors}) when is_list(errors) do

test/mv/accounts/user_policies_test.exs

@@ -343,6 +343,64 @@ defmodule Mv.Accounts.UserPoliciesTest do
       # Verify user is deleted
       assert {:error, _} = Ash.get(Accounts.User, other_user.id, domain: Mv.Accounts)
     end
+
+    test "admin can assign role to another user via update_user", %{
+      actor: actor,
+      other_user: other_user
+    } do
+      admin = create_user_with_permission_set("admin", actor)
+      normal_user_role = create_role_with_permission_set("normal_user", actor)
+
+      {:ok, updated} =
+        other_user
+        |> Ash.Changeset.for_update(:update_user, %{role_id: normal_user_role.id})
+        |> Ash.update(actor: admin)
+
+      assert updated.role_id == normal_user_role.id
+    end
+  end
+
+  describe "admin role assignment and last-admin validation" do
+    test "two admins: one can change own role to normal_user (other remains admin)", %{
+      actor: actor
+    } do
+      _admin_role = create_role_with_permission_set("admin", actor)
+      normal_user_role = create_role_with_permission_set("normal_user", actor)
+
+      admin_a = create_user_with_permission_set("admin", actor)
+      _admin_b = create_user_with_permission_set("admin", actor)
+
+      {:ok, updated} =
+        admin_a
+        |> Ash.Changeset.for_update(:update_user, %{role_id: normal_user_role.id})
+        |> Ash.update(actor: admin_a)
+
+      assert updated.role_id == normal_user_role.id
+    end
+
+    test "single admin: changing own role to normal_user returns validation error", %{
+      actor: actor
+    } do
+      normal_user_role = create_role_with_permission_set("normal_user", actor)
+      single_admin = create_user_with_permission_set("admin", actor)
+
+      assert {:error, %Ash.Error.Invalid{errors: errors}} =
+               single_admin
+               |> Ash.Changeset.for_update(:update_user, %{role_id: normal_user_role.id})
+               |> Ash.update(actor: single_admin)
+
+      error_messages =
+        Enum.flat_map(errors, fn
+          %Ash.Error.Changes.InvalidAttribute{message: msg} when is_binary(msg) -> [msg]
+          %{message: msg} when is_binary(msg) -> [msg]
+          _ -> []
+        end)
+
+      assert Enum.any?(error_messages, fn msg ->
+               msg =~ "least one user must keep the Admin role" or msg =~ "Admin role"
+             end),
+             "Expected last-admin validation message, got: #{inspect(error_messages)}"
+    end
   end
 
   describe "AshAuthentication bypass" do

test/mv_web/user_live/form_test.exs

@@ -213,6 +213,35 @@ defmodule MvWeb.UserLive.FormTest do
       assert not is_nil(updated_user.hashed_password)
       assert updated_user.hashed_password != ""
     end
+
+    test "admin can change user role and change persists", %{conn: conn} do
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
+      role_a = Mv.Fixtures.role_fixture("normal_user")
+      role_b = Mv.Fixtures.role_fixture("read_only")
+
+      user = create_test_user(%{email: "rolechange@example.com"})
+      {:ok, user} = Mv.Accounts.update_user(user, %{role_id: role_a.id}, actor: system_actor)
+      assert user.role_id == role_a.id
+
+      {:ok, view, _html} = setup_live_view(conn, "/users/#{user.id}/edit")
+
+      view
+      |> form("#user-form",
+        user: %{
+          email: "rolechange@example.com",
+          role_id: role_b.id
+        }
+      )
+      |> render_submit()
+
+      assert_redirected(view, "/users")
+
+      updated_user = Ash.reload!(user, domain: Mv.Accounts, actor: system_actor)
+
+      assert updated_user.role_id == role_b.id,
+             "Expected role_id to persist as #{role_b.id}, got #{inspect(updated_user.role_id)}"
+    end
   end
 
   describe "edit user form - validation" do