From 4c9848453d72b40684474a741ce8053cdabf0e78 Mon Sep 17 00:00:00 2001 From: Brian Manifold Date: Tue, 15 Apr 2025 07:25:06 -0700 Subject: [PATCH] refactor(portal): Add more logging around sign in errors (#8789) Why: * To allow for more accurate and efficient troubleshooting in production. --- .../lib/web/controllers/auth_controller.ex | 20 ++++++++++++++++--- .../web/controllers/auth_controller_test.exs | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/elixir/apps/web/lib/web/controllers/auth_controller.ex b/elixir/apps/web/lib/web/controllers/auth_controller.ex index bd1d66dc1..a2cbe45cd 100644 --- a/elixir/apps/web/lib/web/controllers/auth_controller.ex +++ b/elixir/apps/web/lib/web/controllers/auth_controller.ex @@ -252,11 +252,11 @@ defmodule Web.AuthController do ) end else - :error -> + {:error, :auth_state_missing} -> params = Web.Auth.take_sign_in_params(params) conn - |> put_flash(:error, "The sign in token is expired.") + |> put_flash(:error, "The sign in token is missing or expired. Please try again.") |> redirect(to: ~p"/#{account_id_or_slug}?#{params}") end end @@ -393,7 +393,18 @@ defmodule Web.AuthController do :ok <- OpenIDConnect.ensure_states_equal(state, persisted_state) do {:ok, redirect_params, persisted_verifier, delete_auth_state(conn, provider_id)} else - _ -> {:error, :invalid_state, delete_auth_state(conn, provider_id)} + {:error, :auth_state_missing} -> + Logger.info("Auth state missing", provider_id: provider_id) + {:error, :invalid_state, delete_auth_state(conn, provider_id)} + + {:error, :invalid_state} -> + Logger.info("Auth state is invalid", provider_id: provider_id) + {:error, :invalid_state, delete_auth_state(conn, provider_id)} + + _ -> + # Not logging the actual error as it may contain sensitive info + Logger.warning("Unknown error while verifying state", provider_id: provider_id) + {:error, :invalid_state, delete_auth_state(conn, provider_id)} end end @@ -414,6 +425,9 @@ defmodule Web.AuthController do with {:ok, encoded_state} <- Map.fetch(conn.cookies, key) do {:ok, Plug.Crypto.non_executable_binary_to_term(encoded_state, [:safe]), conn} + else + :error -> + {:error, :auth_state_missing} end end diff --git a/elixir/apps/web/test/web/controllers/auth_controller_test.exs b/elixir/apps/web/test/web/controllers/auth_controller_test.exs index e7be8ae41..4212d4722 100644 --- a/elixir/apps/web/test/web/controllers/auth_controller_test.exs +++ b/elixir/apps/web/test/web/controllers/auth_controller_test.exs @@ -547,7 +547,7 @@ defmodule Web.AuthControllerTest do }) assert redirected_to(conn) == ~p"/#{account}" - assert flash(conn, :error) == "The sign in token is expired." + assert flash(conn, :error) == "The sign in token is missing or expired. Please try again." end test "uses redirect params from the request query when auth state does not exist", %{