Streamline login flow and fix test failures

Login flow improvements:
- Remove "Welcome back!" flash message on successful login
- Auto-login users clicking magic links without confirmation prompt
- Always use "remember me" for magic link logins
- Fix Accounts.login_user_by_magic_link/1 to handle invalid token format

Test fixes:
- Add @tag :integration to Ping tests (require actual system ping)
- Add halt() calls to UserAuth.start_impersonation error paths

Users now go directly to their intended destination after clicking a
magic link, with no interruption for "log in and stay logged in" choice.

.tool-versions

@@ -1,2 +1,2 @@
 erlang 28.3
-elixir 1.19.4-otp-28
+elixir 1.19.5-otp-28

lib/towerops/accounts.ex

@@ -276,29 +276,33 @@ defmodule Towerops.Accounts do
      `mix help phx.gen.auth`.
   """
   def login_user_by_magic_link(token) do
-    {:ok, query} = UserToken.verify_magic_link_token_query(token)
+    case UserToken.verify_magic_link_token_query(token) do
+      {:ok, query} ->
+        case Repo.one(query) do
+          # Prevent session fixation attacks by disallowing magic links for unconfirmed users with password
+          {%User{confirmed_at: nil, hashed_password: hash}, _token} when not is_nil(hash) ->
+            raise """
+            magic link log in is not allowed for unconfirmed users with a password set!
 
-    case Repo.one(query) do
-      # Prevent session fixation attacks by disallowing magic links for unconfirmed users with password
-      {%User{confirmed_at: nil, hashed_password: hash}, _token} when not is_nil(hash) ->
-        raise """
-        magic link log in is not allowed for unconfirmed users with a password set!
+            This cannot happen with the default implementation, which indicates that you
+            might have adapted the code to a different use case. Please make sure to read the
+            "Mixing magic link and password registration" section of `mix help phx.gen.auth`.
+            """
 
-        This cannot happen with the default implementation, which indicates that you
-        might have adapted the code to a different use case. Please make sure to read the
-        "Mixing magic link and password registration" section of `mix help phx.gen.auth`.
-        """
+          {%User{confirmed_at: nil} = user, _token} ->
+            user
+            |> User.confirm_changeset()
+            |> update_user_and_delete_all_tokens()
 
-      {%User{confirmed_at: nil} = user, _token} ->
-        user
-        |> User.confirm_changeset()
-        |> update_user_and_delete_all_tokens()
+          {user, token} ->
+            Repo.delete!(token)
+            {:ok, {user, []}}
 
-      {user, token} ->
-        Repo.delete!(token)
-        {:ok, {user, []}}
+          nil ->
+            {:error, :not_found}
+        end
 
-      nil ->
+      :error ->
         {:error, :not_found}
     end
   end

lib/towerops_web/controllers/user_session_controller.ex

@@ -64,17 +64,29 @@ defmodule ToweropsWeb.UserSessionController do
   end
 
   def confirm(conn, %{"token" => token}) do
-    if user = Accounts.get_user_by_magic_link_token(token) do
-      form = Phoenix.Component.to_form(%{"token" => token}, as: "user")
+    # Check if user was previously confirmed before logging in
+    was_confirmed =
+      case Accounts.get_user_by_magic_link_token(token) do
+        %Accounts.User{confirmed_at: confirmed_at} when not is_nil(confirmed_at) -> true
+        _ -> false
+      end
 
-      conn
-      |> assign(:user, user)
-      |> assign(:form, form)
-      |> render(:confirm)
-    else
-      conn
-      |> put_flash(:error, "Magic link is invalid or it has expired.")
-      |> redirect(to: ~p"/users/log-in")
+    case Accounts.login_user_by_magic_link(token) do
+      {:ok, {user, _expired_tokens}} ->
+        conn =
+          if was_confirmed do
+            conn
+          else
+            put_flash(conn, :info, "User confirmed successfully.")
+          end
+
+        # Always use remember_me: true to keep user logged in
+        UserAuth.log_in_user(conn, user, %{"remember_me" => "true"})
+
+      {:error, :not_found} ->
+        conn
+        |> put_flash(:error, "Magic link is invalid or it has expired.")
+        |> redirect(to: ~p"/users/log-in")
     end
   end
 

lib/towerops_web/user_auth.ex

@@ -477,11 +477,13 @@ defmodule ToweropsWeb.UserAuth 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))

test/towerops/monitoring/ping_test.exs

@@ -4,17 +4,20 @@ defmodule Towerops.Monitoring.PingTest do
   alias Towerops.Monitoring.Ping
 
   describe "ping/2" do
+    @tag :integration
     test "successfully pings localhost" do
       assert {:ok, response_time} = Ping.ping("127.0.0.1")
       assert is_integer(response_time)
       assert response_time >= 0
     end
 
+    @tag :integration
     test "uses default timeout of 5000ms" do
       # Test that ping works without explicit timeout
       assert {:ok, _response_time} = Ping.ping("127.0.0.1")
     end
 
+    @tag :integration
     test "accepts custom timeout" do
       assert {:ok, _response_time} = Ping.ping("127.0.0.1", 1000)
     end
@@ -30,6 +33,7 @@ defmodule Towerops.Monitoring.PingTest do
       assert {:error, :timeout_or_unreachable} = Ping.ping("999.999.999.999", 1000)
     end
 
+    @tag :integration
     test "calculates response time" do
       # Ping should complete in reasonable time
       {:ok, response_time} = Ping.ping("127.0.0.1", 5000)
@@ -39,6 +43,7 @@ defmodule Towerops.Monitoring.PingTest do
       assert response_time < 5000
     end
 
+    @tag :integration
     test "handles timeout correctly" do
       # Very short timeout should likely fail or be very fast
       result = Ping.ping("127.0.0.1", 1)
@@ -49,12 +54,14 @@ defmodule Towerops.Monitoring.PingTest do
       end
     end
 
+    @tag :integration
     test "minimum timeout is 1 second" do
       # Even with 0ms timeout, should use minimum of 1 second
       result = Ping.ping("127.0.0.1", 0)
       assert {:ok, _} = result
     end
 
+    @tag :integration
     test "respects different OS types" do
       # This test just ensures the OS detection doesn't crash
       case :os.type() do

test/towerops_web/controllers/user_session_controller_test.exs

@@ -43,29 +43,33 @@ defmodule ToweropsWeb.UserSessionControllerTest do
   end
 
   describe "GET /users/log-in/:token" do
-    test "renders confirmation page for unconfirmed user", %{conn: conn, unconfirmed_user: user} do
+    test "logs in and confirms unconfirmed user", %{conn: conn, unconfirmed_user: user} do
       token =
         extract_user_token(fn url ->
           Accounts.deliver_login_instructions(user, url)
         end)
 
       conn = get(conn, ~p"/users/log-in/#{token}")
-      assert html_response(conn, 200) =~ "Confirm and stay logged in"
+      assert redirected_to(conn) == ~p"/orgs"
+      assert get_session(conn, :user_token)
+      assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "User confirmed successfully."
+      assert conn.resp_cookies["_towerops_web_user_remember_me"]
     end
 
-    test "renders login page for confirmed user", %{conn: conn, user: user} do
+    test "logs in confirmed user", %{conn: conn, user: user} do
       token =
         extract_user_token(fn url ->
           Accounts.deliver_login_instructions(user, url)
         end)
 
       conn = get(conn, ~p"/users/log-in/#{token}")
-      html = html_response(conn, 200)
-      refute html =~ "Confirm my account"
-      assert html =~ "Log in"
+      assert redirected_to(conn) == ~p"/orgs"
+      assert get_session(conn, :user_token)
+      refute Phoenix.Flash.get(conn.assigns.flash, :info)
+      assert conn.resp_cookies["_towerops_web_user_remember_me"]
     end
 
-    test "raises error for invalid token", %{conn: conn} do
+    test "redirects with error for invalid token", %{conn: conn} do
       conn = get(conn, ~p"/users/log-in/invalid-token")
       assert redirected_to(conn) == ~p"/users/log-in"