Merge branch 'main' into feature/428_export_groups

lib/mv_web/controllers/auth_controller.ex

@@ -45,9 +45,7 @@ defmodule MvWeb.AuthController do
   - Generic authentication failures
   """
   def failure(conn, activity, reason) do
-    Logger.warning(
-      "Authentication failure - Activity: #{inspect(activity)}, Reason: #{inspect(reason)}"
-    )
+    log_failure_safely(activity, reason)
 
     case {activity, reason} do
       {{:rauthy, _action}, reason} ->
@@ -57,10 +55,70 @@ defmodule MvWeb.AuthController do
         handle_authentication_failed(conn, caused_by)
 
       _ ->
-        redirect_with_error(conn, gettext("Incorrect email or password"))
+        conn
+        |> put_flash(:error, gettext("Incorrect email or password"))
+        |> redirect(to: ~p"/sign-in")
     end
   end
 
+  # Log authentication failures safely, avoiding sensitive data for {:rauthy, _} activities
+  defp log_failure_safely({:rauthy, _action} = activity, reason) do
+    # For Assent errors, use safe_assent_meta to avoid logging tokens/URLs with query params
+    case reason do
+      %Assent.ServerUnreachableError{} = err ->
+        meta = safe_assent_meta(err)
+        message = format_safe_log_message("Authentication failure", activity, meta)
+        Logger.warning(message)
+
+      %Assent.InvalidResponseError{} = err ->
+        meta = safe_assent_meta(err)
+        message = format_safe_log_message("Authentication failure", activity, meta)
+        Logger.warning(message)
+
+      _ ->
+        # For other rauthy errors, log only error type, not full details
+        error_type = get_error_type(reason)
+
+        Logger.warning(
+          "Authentication failure - Activity: #{inspect(activity)}, Error type: #{error_type}"
+        )
+    end
+  end
+
+  defp log_failure_safely(activity, reason) do
+    # For non-rauthy activities, safe to log full reason
+    Logger.warning(
+      "Authentication failure - Activity: #{inspect(activity)}, Reason: #{inspect(reason)}"
+    )
+  end
+
+  # Extract safe error type identifier without sensitive data
+  defp get_error_type(%struct{}), do: "#{struct}"
+  defp get_error_type(atom) when is_atom(atom), do: inspect(atom)
+  defp get_error_type(_other), do: "[redacted]"
+
+  # Format safe log message with metadata included in the message string
+  defp format_safe_log_message(base_message, activity, meta) when is_list(meta) do
+    activity_str = "Activity: #{inspect(activity)}"
+    meta_str = format_meta_string(meta)
+    "#{base_message} - #{activity_str}#{meta_str}"
+  end
+
+  defp format_meta_string([]), do: ""
+
+  defp format_meta_string(meta) when is_list(meta) do
+    parts =
+      Enum.map(meta, fn
+        {:request_url, url} -> "Request URL: #{url}"
+        {:status, status} -> "Status: #{status}"
+        {:http_adapter, adapter} -> "HTTP Adapter: #{inspect(adapter)}"
+        _ -> nil
+      end)
+      |> Enum.filter(&(&1 != nil))
+
+    if Enum.empty?(parts), do: "", else: " - " <> Enum.join(parts, ", ")
+  end
+
   # Handle all Rauthy (OIDC) authentication failures
   defp handle_rauthy_failure(conn, %Ash.Error.Invalid{errors: errors}) do
     handle_oidc_email_collision(conn, errors)
@@ -74,14 +132,46 @@ defmodule MvWeb.AuthController do
         handle_oidc_email_collision(conn, errors)
 
       _ ->
-        redirect_with_error(conn, gettext("Unable to authenticate with OIDC. Please try again."))
+        conn
+        |> put_flash(:error, gettext("Unable to authenticate with OIDC. Please try again."))
+        |> redirect(to: ~p"/sign-in")
     end
   end
 
+  # Handle Assent server unreachable errors (network/connectivity issues)
+  defp handle_rauthy_failure(conn, %Assent.ServerUnreachableError{} = _err) do
+    # Logging already done safely in failure/3 via log_failure_safely/2
+    # No need to log again here to avoid duplicate logs
+
+    conn
+    |> put_flash(
+      :error,
+      gettext("The authentication server is currently unavailable. Please try again later.")
+    )
+    |> redirect(to: ~p"/sign-in")
+  end
+
+  # Handle Assent invalid response errors (configuration or malformed responses)
+  defp handle_rauthy_failure(conn, %Assent.InvalidResponseError{} = _err) do
+    # Logging already done safely in failure/3 via log_failure_safely/2
+    # No need to log again here to avoid duplicate logs
+
+    conn
+    |> put_flash(
+      :error,
+      gettext("Authentication configuration error. Please contact the administrator.")
+    )
+    |> redirect(to: ~p"/sign-in")
+  end
+
   # Catch-all clause for any other error types
-  defp handle_rauthy_failure(conn, reason) do
-    Logger.warning("Unhandled Rauthy failure reason: #{inspect(reason)}")
-    redirect_with_error(conn, gettext("Unable to authenticate with OIDC. Please try again."))
+  defp handle_rauthy_failure(conn, _reason) do
+    # Logging already done safely in failure/3 via log_failure_safely/2
+    # No need to log again here to avoid duplicate logs
+
+    conn
+    |> put_flash(:error, gettext("Unable to authenticate with OIDC. Please try again."))
+    |> redirect(to: ~p"/sign-in")
   end
 
   # Handle generic AuthenticationFailed errors
@@ -93,14 +183,20 @@ defmodule MvWeb.AuthController do
         You can confirm your account using the link we sent to you, or by resetting your password.
         """)
 
-      redirect_with_error(conn, message)
+      conn
+      |> put_flash(:error, message)
+      |> redirect(to: ~p"/sign-in")
     else
-      redirect_with_error(conn, gettext("Authentication failed. Please try again."))
+      conn
+      |> put_flash(:error, gettext("Authentication failed. Please try again."))
+      |> redirect(to: ~p"/sign-in")
     end
   end
 
   defp handle_authentication_failed(conn, _other) do
-    redirect_with_error(conn, gettext("Authentication failed. Please try again."))
+    conn
+    |> put_flash(:error, gettext("Authentication failed. Please try again."))
+    |> redirect(to: ~p"/sign-in")
   end
 
   # Handle OIDC email collision - user needs to verify password to link accounts
@@ -112,7 +208,10 @@ defmodule MvWeb.AuthController do
       nil ->
         # Check if it's a "different OIDC account" error or email uniqueness error
         error_message = extract_meaningful_error_message(errors)
-        redirect_with_error(conn, error_message)
+
+        conn
+        |> put_flash(:error, error_message)
+        |> redirect(to: ~p"/sign-in")
     end
   end
 
@@ -177,13 +276,47 @@ defmodule MvWeb.AuthController do
     |> redirect(to: ~p"/auth/link-oidc-account")
   end
 
-  # Generic error redirect helper
-  defp redirect_with_error(conn, message) do
-    conn
-    |> put_flash(:error, message)
-    |> redirect(to: ~p"/sign-in")
+  # Extract safe metadata from Assent errors for logging
+  # Never logs sensitive data: no tokens, secrets, or full request URLs
+  # Returns keyword list for Logger.warning/2
+  defp safe_assent_meta(%{request_url: url} = err) when is_binary(url) do
+    [
+      request_url: redact_url(url),
+      http_adapter: Map.get(err, :http_adapter)
+    ]
+    |> Enum.filter(fn {_key, value} -> not is_nil(value) end)
   end
 
+  # Handle InvalidResponseError which has :response field (HTTPResponse struct)
+  defp safe_assent_meta(%{response: %{status: status} = response} = err) do
+    [
+      status: status,
+      http_adapter: Map.get(response, :http_adapter) || Map.get(err, :http_adapter)
+    ]
+    |> Enum.filter(fn {_key, value} -> not is_nil(value) end)
+  end
+
+  defp safe_assent_meta(err) do
+    # Only extract safe, simple fields
+    [
+      http_adapter: Map.get(err, :http_adapter)
+    ]
+    |> Enum.filter(fn {_key, value} -> not is_nil(value) end)
+  end
+
+  # Redact URL to only show scheme and host, hiding path, query, and fragments
+  defp redact_url(url) when is_binary(url) do
+    case URI.parse(url) do
+      %URI{scheme: scheme, host: host} when not is_nil(scheme) and not is_nil(host) ->
+        "#{scheme}://#{host}"
+
+      _ ->
+        "[redacted]"
+    end
+  end
+
+  defp redact_url(_), do: "[redacted]"
+
   def sign_out(conn, _params) do
     return_to = get_session(conn, :return_to) || ~p"/"