Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Persistent Activity Logger #2730

Merged
merged 12 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 82 additions & 0 deletions lib/trento/activity_logging/activity_catalog.ex
Original file line number Diff line number Diff line change
@@ -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)
gagandeepb marked this conversation as resolved.
Show resolved Hide resolved

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
58 changes: 55 additions & 3 deletions lib/trento/activity_logging/activity_logger.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 20 additions & 0 deletions lib/trento/activity_logging/parser/activity_parser.ex
Original file line number Diff line number Diff line change
@@ -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
Comment on lines +1 to +20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defprotocol Trento.ActivityLog.ParseToActivity do
  alias Trento.ActivityLog.ActivityCatalog

  def detect_activity(activity_context :: any()) :: ActivityCatalog.logged_activity() | nil

  def get_activity_actor(
              activity :: ActivityCatalog.logged_activity(),
              activity_context :: any()
            ) :: any()

  def get_activity_metadata(
              activity :: ActivityCatalog.logged_activity(),
              activity_context :: any()
            ) :: map()
end

For reference/ consideration. I think this should work.

140 changes: 140 additions & 0 deletions lib/trento/activity_logging/parser/phoenix_conn_parser.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
defmodule Trento.ActivityLog.Logger.Parser.PhoenixConnParser do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example:

defimpl Trento.ActivityLog.ParseToActivity, for: %Plug.Conn{} do
...
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tips on the protocols, might come good as we proceed further

@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

This file was deleted.

16 changes: 7 additions & 9 deletions test/support/conn_case.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ defmodule TrentoWeb.ConnCase do

use ExUnit.CaseTemplate

use Trento.TaskCase

alias Ecto.Adapters.SQL.Sandbox

using do
Expand All @@ -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
8 changes: 0 additions & 8 deletions test/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
Loading
Loading