Add Persistent Activity Logger (#2730)

* Adjust ActivityLogger.log_activity signature

* Add an ActivityLog catalog determining what is of interest to be logged

* Add an ActivityParser behaviour that extracts relevant information from an activity context

* Add an ActivityLogWriter behaviour that is responsible to write activity logs to a destination

* Add a PhoenixConnParser able to parse/extract activity from phoenix http connections

* Add a DatabaseWriter that writes activity logs to the database

* Add a Persistent activity logger

* Adjust configuration

* Define custom activity_type type

* Alias Trento Repo in Database log writer

* Refactor activity logging

* Defer activity log writer abstraction

config/config.exs

@@ -203,9 +203,6 @@ config :trento, Trento.Infrastructure.SoftwareUpdates.MockSuma, relevant_patches
 config :trento, Trento.Infrastructure.SoftwareUpdates.SumaApi,
   executor: Trento.Infrastructure.SoftwareUpdates.Suma.HttpExecutor
 
-config :trento, Trento.ActivityLog.ActivityLogger,
-  adapter: Trento.Infrastructure.ActivityLog.Logger.NoopLogger
-
 config :bodyguard,
   # The second element of the {:error, reason} tuple returned on auth failure
   default_error: :forbidden

lib/trento/activity_logging/activity_catalog.ex

@@ -0,0 +1,82 @@
+defmodule Trento.ActivityLog.ActivityCatalog do
+  @moduledoc """
+  Activity logging catalog
+  """
+
+  @type logged_activity :: {controller :: module(), activity :: atom()}
+
+  @type activity_type ::
+          :login_attempt
+          | :resource_tagging
+          | :resource_untagging
+          | :api_key_generation
+          | :saving_suma_settings
+          | :changing_suma_settings
+          | :clearing_suma_settings
+          | :user_creation
+          | :user_modification
+          | :user_deletion
+          | :profile_update
+          | :cluster_checks_execution_request
+
+  @login_attempt {TrentoWeb.SessionController, :create}
+  @api_key_generation {TrentoWeb.V1.SettingsController, :update_api_key_settings}
+  @saving_suma_settings {TrentoWeb.V1.SUMACredentialsController, :create}
+  @changing_suma_settings {TrentoWeb.V1.SUMACredentialsController, :update}
+  @clearing_suma_settings {TrentoWeb.V1.SUMACredentialsController, :delete}
+  @tagging {TrentoWeb.V1.TagsController, :add_tag}
+  @untagging {TrentoWeb.V1.TagsController, :remove_tag}
+  @user_creation {TrentoWeb.V1.UsersController, :create}
+  @user_modification {TrentoWeb.V1.UsersController, :update}
+  @user_deletion {TrentoWeb.V1.UsersController, :delete}
+  @profile_update {TrentoWeb.V1.ProfileController, :update}
+  # @cluster_checks_selection {TrentoWeb.V1.ClusterController, :select_checks} # <-- this is **also** event sourced
+  @cluster_checks_execution_request {TrentoWeb.V1.ClusterController, :request_checks_execution}
+
+  @activity_catalog %{
+    @login_attempt => {:login_attempt, :any},
+    @tagging => {:resource_tagging, 201},
+    @untagging => {:resource_untagging, 204},
+    @api_key_generation => {:api_key_generation, 200},
+    @saving_suma_settings => {:saving_suma_settings, 201},
+    @changing_suma_settings => {:changing_suma_settings, 200},
+    @clearing_suma_settings => {:clearing_suma_settings, 204},
+    @user_creation => {:user_creation, 201},
+    @user_modification => {:user_modification, 200},
+    @user_deletion => {:user_deletion, 204},
+    @profile_update => {:profile_update, 200},
+    @cluster_checks_execution_request => {:cluster_checks_execution_request, 202}
+  }
+
+  Enum.each(@activity_catalog, fn {activity, {activity_type, _}} ->
+    defmacro unquote(activity_type)(), do: unquote(activity)
+  end)
+
+  def activity_catalog, do: Enum.map(@activity_catalog, fn {activity, _} -> activity end)
+
+  def interested?(activity, %Plug.Conn{status: status}),
+    do:
+      Map.has_key?(@activity_catalog, activity) &&
+        @activity_catalog
+        |> Map.fetch!(activity)
+        |> interesting_occurrence?(status)
+
+  def interested?(_, _), do: false
+
+  @spec get_activity_type(logged_activity()) :: atom() | nil
+  def get_activity_type(activity) do
+    case Map.fetch(@activity_catalog, activity) do
+      {:ok, {activity_type, _}} -> activity_type
+      :error -> nil
+    end
+  end
+
+  @spec interesting_occurrence?(
+          conn_activity_occurrence :: {activity_type(), relevant_status :: integer() | :any},
+          detected_status :: integer()
+        ) ::
+          boolean()
+  defp interesting_occurrence?({_, :any}, _), do: true
+  defp interesting_occurrence?({_, status}, status), do: true
+  defp interesting_occurrence?(_, _), do: false
+end

lib/trento/activity_logging/activity_logger.ex

@@ -3,9 +3,61 @@ defmodule Trento.ActivityLog.ActivityLogger do
   ActivityLogger entry point
   """
 
-  @callback log_activity(context :: any()) :: :ok
+  alias Trento.Repo
 
-  def log_activity(context), do: adapter().log_activity(context)
+  alias Trento.ActivityLog.ActivityCatalog
+  alias Trento.ActivityLog.ActivityLog
 
-  defp adapter, do: Application.fetch_env!(:trento, __MODULE__)[:adapter]
+  require Logger
+
+  def log_activity(activity_context) do
+    with {:ok, activity_parser} <- get_activity_parser(activity_context),
+         detected_activity <- detect_activity(activity_parser, activity_context),
+         true <- ActivityCatalog.interested?(detected_activity, activity_context) do
+      write_log(%{
+        type: get_activity_type(detected_activity),
+        actor: get_actor(activity_parser, detected_activity, activity_context),
+        metadata: get_metadata(activity_parser, detected_activity, activity_context)
+      })
+    end
+
+    :ok
+  end
+
+  defp get_activity_parser(%Plug.Conn{}),
+    do: {:ok, Trento.ActivityLog.Logger.Parser.PhoenixConnParser}
+
+  defp get_activity_parser(_), do: {:error, :unsupported_activity}
+
+  # defp get_activity_parser(%Commanded.Middleware.Pipeline{}),
+  #   do: {:ok, Trento.ActivityLog.Logger.Parser.CommandedParser}
+
+  defp detect_activity(activity_parser, activity_context),
+    do: activity_parser.detect_activity(activity_context)
+
+  defp get_activity_type(detected_activity),
+    do:
+      detected_activity
+      |> ActivityCatalog.get_activity_type()
+      |> Atom.to_string()
+
+  defp get_actor(activity_parser, detected_activity, activity_context),
+    do: activity_parser.get_activity_actor(detected_activity, activity_context)
+
+  defp get_metadata(activity_parser, detected_activity, activity_context),
+    do: activity_parser.get_activity_metadata(detected_activity, activity_context)
+
+  defp write_log(%{type: activity_type} = entry) do
+    case %ActivityLog{}
+         |> ActivityLog.changeset(entry)
+         |> Repo.insert() do
+      {:ok, _} ->
+        Logger.info("Logged activity: #{activity_type}")
+
+      {:error, reason} ->
+        Logger.error(
+          "An error occurred while logging activity: #{activity_type}. Reason: #{inspect(reason)}"
+        )
+    end
+  end
 end

lib/trento/activity_logging/parser/activity_parser.ex

@@ -0,0 +1,20 @@
+defmodule Trento.ActivityLog.Parser.ActivityParser do
+  @moduledoc """
+  Behavior for activity parsers.
+  It extracts the activity relevant information from the context.
+  """
+
+  alias Trento.ActivityLog.ActivityCatalog
+
+  @callback detect_activity(activity_context :: any()) :: ActivityCatalog.logged_activity() | nil
+
+  @callback get_activity_actor(
+              activity :: ActivityCatalog.logged_activity(),
+              activity_context :: any()
+            ) :: any()
+
+  @callback get_activity_metadata(
+              activity :: ActivityCatalog.logged_activity(),
+              activity_context :: any()
+            ) :: map()
+end

lib/trento/activity_logging/parser/phoenix_conn_parser.ex

@@ -0,0 +1,140 @@
+defmodule Trento.ActivityLog.Logger.Parser.PhoenixConnParser do
+  @moduledoc """
+  Phoenix connection activity parser
+  """
+
+  alias Phoenix.Controller
+
+  alias Trento.Users.User
+
+  require Trento.ActivityLog.ActivityCatalog, as: ActivityCatalog
+
+  @behaviour Trento.ActivityLog.Parser.ActivityParser
+
+  @impl true
+  def detect_activity(%Plug.Conn{} = conn) do
+    {Controller.controller_module(conn), Controller.action_name(conn)}
+  rescue
+    _ -> nil
+  end
+
+  @impl true
+  def get_activity_actor(ActivityCatalog.login_attempt(), %Plug.Conn{body_params: request_payload}),
+      do: Map.get(request_payload, "username", "no_username")
+
+  @impl true
+  def get_activity_actor(_, %Plug.Conn{} = conn) do
+    case Pow.Plug.current_user(conn) do
+      %User{username: username} -> username
+      _ -> "system"
+    end
+  end
+
+  @impl true
+  def get_activity_metadata(
+        ActivityCatalog.login_attempt() = action,
+        %Plug.Conn{
+          assigns: %{
+            reason: reason
+          },
+          status: 401
+        } = conn
+      ) do
+    %{
+      username: get_activity_actor(action, conn),
+      reason: reason
+    }
+  end
+
+  @impl true
+  def get_activity_metadata(
+        ActivityCatalog.resource_tagging(),
+        %Plug.Conn{
+          params: %{id: resource_id},
+          assigns: %{
+            resource_type: resource_type
+          },
+          body_params: %{value: added_tag}
+        }
+      ) do
+    %{
+      resource_id: resource_id,
+      resource_type: resource_type,
+      added_tag: added_tag
+    }
+  end
+
+  @impl true
+  def get_activity_metadata(
+        ActivityCatalog.resource_untagging(),
+        %Plug.Conn{
+          params: %{
+            id: resource_id,
+            value: removed_tag
+          },
+          assigns: %{
+            resource_type: resource_type
+          }
+        }
+      ) do
+    %{
+      resource_id: resource_id,
+      resource_type: resource_type,
+      removed_tag: removed_tag
+    }
+  end
+
+  @impl true
+  def get_activity_metadata(
+        ActivityCatalog.api_key_generation(),
+        %Plug.Conn{
+          body_params: request_body
+        }
+      ) do
+    request_body
+  end
+
+  @impl true
+  def get_activity_metadata(
+        action,
+        %Plug.Conn{
+          body_params: request_body
+        }
+      )
+      when action in [
+             ActivityCatalog.saving_suma_settings(),
+             ActivityCatalog.changing_suma_settings()
+           ] do
+    request_body
+    |> redact(:password)
+    |> redact(:ca_cert)
+  end
+
+  @impl true
+  def get_activity_metadata(
+        action,
+        %Plug.Conn{
+          body_params: request_body
+        }
+      )
+      when action in [
+             ActivityCatalog.user_creation(),
+             ActivityCatalog.user_modification(),
+             ActivityCatalog.profile_update()
+           ] do
+    request_body
+    |> redact(:password)
+    |> redact(:current_password)
+    |> redact(:password_confirmation)
+  end
+
+  @impl true
+  def get_activity_metadata(_, _), do: %{}
+
+  defp redact(request_body, key) do
+    case Map.has_key?(request_body, key) && Map.fetch!(request_body, key) != nil do
+      true -> Map.update!(request_body, key, fn _ -> "REDACTED" end)
+      false -> request_body
+    end
+  end
+end

test/support/conn_case.ex

@@ -17,6 +17,8 @@ defmodule TrentoWeb.ConnCase do
 
   use ExUnit.CaseTemplate
 
+  use Trento.TaskCase
+
   alias Ecto.Adapters.SQL.Sandbox
 
   using do
@@ -35,17 +37,13 @@ defmodule TrentoWeb.ConnCase do
 
   setup tags do
     pid = Sandbox.start_owner!(Trento.Repo, shared: not tags[:async])
-    on_exit(fn -> Sandbox.stop_owner(pid) end)
 
-    stub_activity_logger()
+    on_exit(fn ->
+      wait_for_tasks_completion()
+
+      Sandbox.stop_owner(pid)
+    end)
 
     {:ok, conn: Phoenix.ConnTest.build_conn()}
   end
-
-  defp stub_activity_logger,
-    do:
-      Mox.stub_with(
-        Trento.ActivityLog.ActivityLogger.Mock,
-        Trento.Infrastructure.ActivityLog.Logger.NoopLogger
-      )
 end

test/test_helper.exs

@@ -30,14 +30,6 @@ Application.put_env(:trento, Trento.Infrastructure.SoftwareUpdates.Suma,
   auth: Trento.Infrastructure.SoftwareUpdates.Auth.Mock
 )
 
-Mox.defmock(Trento.ActivityLog.ActivityLogger.Mock,
-  for: Trento.ActivityLog.ActivityLogger
-)
-
-Application.put_env(:trento, Trento.ActivityLog.ActivityLogger,
-  adapter: Trento.ActivityLog.ActivityLogger.Mock
-)
-
 Mox.defmock(Trento.Infrastructure.Messaging.Adapter.Mock,
   for: Trento.Infrastructure.Messaging.Adapter.Gen
 )