diff --git a/lib/towerops_web/user_auth.ex b/lib/towerops_web/user_auth.ex index dc91025a..bad284bf 100644 --- a/lib/towerops_web/user_auth.ex +++ b/lib/towerops_web/user_auth.ex @@ -610,39 +610,32 @@ defmodule ToweropsWeb.UserAuth do superuser = conn.assigns.current_scope.user target_user = Accounts.get_user!(target_user_id) - # Prevent impersonating self or other superusers + # Prevent impersonating self if target_user.id == superuser.id do conn |> put_flash(:error, "You cannot impersonate yourself.") |> redirect(to: ~p"/admin/users") |> halt() else - if target_user.is_superuser do - conn - |> put_flash(:error, "You cannot impersonate other superusers.") - |> redirect(to: ~p"/admin/users") - |> halt() - else - # Create audit log - ip = to_string(:inet_parse.ntoa(conn.remote_ip)) + # Create audit log + ip = to_string(:inet_parse.ntoa(conn.remote_ip)) - Towerops.Admin.create_audit_log(%{ - action: "impersonate_start", - superuser_id: superuser.id, - target_user_id: target_user.id, - metadata: %{target_email: target_user.email}, - ip_address: ip - }) + Towerops.Admin.create_audit_log(%{ + action: "impersonate_start", + superuser_id: superuser.id, + target_user_id: target_user.id, + metadata: %{target_email: target_user.email, target_is_superuser: target_user.is_superuser}, + ip_address: ip + }) - # Store superuser ID and target user ID in session and update scope - conn - |> put_session(:superuser_id, superuser.id) - |> put_session(:target_user_id, target_user.id) - |> put_session(:impersonating, true) - |> assign(:current_scope, Scope.for_impersonation(superuser, target_user)) - |> put_flash(:info, "Now impersonating #{target_user.email}") - |> redirect(to: ~p"/orgs") - end + # Store superuser ID and target user ID in session and update scope + conn + |> put_session(:superuser_id, superuser.id) + |> put_session(:target_user_id, target_user.id) + |> put_session(:impersonating, true) + |> assign(:current_scope, Scope.for_impersonation(superuser, target_user)) + |> put_flash(:info, "Now impersonating #{target_user.email}") + |> redirect(to: ~p"/orgs") end end diff --git a/test/towerops_web/user_auth_test.exs b/test/towerops_web/user_auth_test.exs index 936ad9d4..6161520e 100644 --- a/test/towerops_web/user_auth_test.exs +++ b/test/towerops_web/user_auth_test.exs @@ -527,7 +527,7 @@ defmodule ToweropsWeb.UserAuthTest do assert conn.halted end - test "prevents superuser from impersonating other superusers", %{conn: conn} do + test "allows superuser to impersonate other superusers", %{conn: conn} do superuser = user_fixture() |> Ecto.Changeset.change(%{is_superuser: true}) @@ -545,11 +545,13 @@ defmodule ToweropsWeb.UserAuthTest do |> assign(:current_scope, Scope.for_user(superuser)) |> UserAuth.start_impersonation(other_superuser.id) - # Check that the error was set - assert Phoenix.Flash.get(conn.assigns.flash, :error) == - "You cannot impersonate other superusers." - - assert conn.halted + # Verify impersonation was successful + assert get_session(conn, :impersonating) + assert get_session(conn, :superuser_id) == superuser.id + assert get_session(conn, :target_user_id) == other_superuser.id + assert conn.assigns.current_scope.user.id == other_superuser.id + assert conn.assigns.current_scope.superuser.id == superuser.id + assert redirected_to(conn) == ~p"/orgs" end end