User: fix last-admin validation and forbid non-admin role_id change

- Last-admin only when target role is non-admin (admins may switch admin roles).
- Use Ash.Changeset.get_attribute for new role_id. Tests: admin role switch, non-admin update_user role_id forbidden.

lib/accounts/user.ex

@@ -391,38 +391,52 @@ defmodule Mv.Accounts.User do
       end
     end
 
-    # Last-admin: prevent the only admin from changing their role (at least one admin required).
+    # Last-admin: prevent the only admin from leaving the admin role (at least one admin required).
+    # Only block when the user is leaving admin (target role is not admin). Switching between
+    # two admin roles (e.g. "Admin" and "Superadmin" both with permission_set_name "admin") is allowed.
     validate fn changeset, _context ->
                if Ash.Changeset.changing_attribute?(changeset, :role_id) do
-                 current_role_id = changeset.data.role_id
+                 new_role_id = Ash.Changeset.get_attribute(changeset, :role_id)
 
-                 current_role =
-                   Mv.Authorization.Role
-                   |> Ash.get!(current_role_id, authorize?: false)
-
-                 if current_role.permission_set_name != "admin" do
+                 if is_nil(new_role_id) do
                    :ok
                  else
-                   admin_role_ids =
+                   current_role_id = changeset.data.role_id
+
+                   current_role =
                      Mv.Authorization.Role
-                     |> Ash.Query.for_read(:read)
-                     |> Ash.Query.filter(expr(permission_set_name == "admin"))
-                     |> Ash.read!(authorize?: false)
-                     |> Enum.map(& &1.id)
+                     |> Ash.get!(current_role_id, authorize?: false)
 
-                   # Count only non-system users with admin role (system user is for internal ops)
-                   system_email = Mv.Helpers.SystemActor.system_user_email()
+                   new_role =
+                     Mv.Authorization.Role
+                     |> Ash.get!(new_role_id, authorize?: false)
 
-                   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)
+                   # Only block when current user is admin and target role is not admin (leaving admin)
+                   if current_role.permission_set_name == "admin" and
+                        new_role.permission_set_name != "admin" do
+                     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)
 
-                   if count <= 1 do
-                     {:error,
-                      field: :role_id, message: "At least one user must keep the Admin role."}
+                     # 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
                    else
                      :ok
                    end

test/mv/accounts/user_policies_test.exs

@@ -105,6 +105,19 @@ defmodule Mv.Accounts.UserPoliciesTest do
           Ash.destroy!(user, actor: user)
         end
       end
+
+      @tag permission_set: permission_set
+      test "cannot change role via update_user - forbidden (#{permission_set})", %{
+        user: user,
+        other_user: other_user
+      } do
+        other_role = Mv.Fixtures.role_fixture("read_only")
+
+        assert {:error, %Ash.Error.Forbidden{}} =
+                 other_user
+                 |> Ash.Changeset.for_update(:update_user, %{role_id: other_role.id})
+                 |> Ash.update(actor: user, domain: Mv.Accounts)
+      end
     end
   end
 
@@ -221,6 +234,31 @@ defmodule Mv.Accounts.UserPoliciesTest do
              end),
              "Expected last-admin validation message, got: #{inspect(error_messages)}"
     end
+
+    test "admin can switch to another admin role (two roles with permission_set_name admin)", %{
+      actor: _actor
+    } do
+      # Two distinct roles both with permission_set_name "admin" (e.g. "Admin" and "Superadmin")
+      admin_role_a = Mv.Fixtures.role_fixture("admin")
+      admin_role_b = Mv.Fixtures.role_fixture("admin")
+
+      admin_user = Mv.Fixtures.user_with_role_fixture("admin")
+      # Ensure user has role_a so we can switch to role_b
+      {:ok, admin_user} =
+        admin_user
+        |> Ash.Changeset.for_update(:update_user, %{role_id: admin_role_a.id})
+        |> Ash.update(actor: admin_user)
+
+      assert admin_user.role_id == admin_role_a.id
+
+      # Switching to another admin role must be allowed (no last-admin error)
+      {:ok, updated} =
+        admin_user
+        |> Ash.Changeset.for_update(:update_user, %{role_id: admin_role_b.id})
+        |> Ash.update(actor: admin_user)
+
+      assert updated.role_id == admin_role_b.id
+    end
   end
 
   describe "AshAuthentication bypass" do