diff --git a/.tool-versions b/.tool-versions index cdd7ae13..1b175d46 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,2 @@ erlang 28.3 -elixir 1.19.4-otp-28 +elixir 1.19.5-otp-28 diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex index 0ebd4b41..853607bc 100644 --- a/lib/towerops/accounts.ex +++ b/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 diff --git a/lib/towerops_web/controllers/user_session_controller.ex b/lib/towerops_web/controllers/user_session_controller.ex index 70c24591..400c2f4f 100644 --- a/lib/towerops_web/controllers/user_session_controller.ex +++ b/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 diff --git a/lib/towerops_web/user_auth.ex b/lib/towerops_web/user_auth.ex index 269bdcbe..7c2b9eab 100644 --- a/lib/towerops_web/user_auth.ex +++ b/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)) diff --git a/test/towerops/monitoring/ping_test.exs b/test/towerops/monitoring/ping_test.exs index b4511b53..400262b7 100644 --- a/test/towerops/monitoring/ping_test.exs +++ b/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 diff --git a/test/towerops_web/controllers/user_session_controller_test.exs b/test/towerops_web/controllers/user_session_controller_test.exs index 4bd03c27..a285c1b4 100644 --- a/test/towerops_web/controllers/user_session_controller_test.exs +++ b/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"