diff --git a/lib/trento/activity_logging/parser/metadata_enricher.ex b/lib/trento/activity_logging/parser/metadata_enricher.ex index d2252db1f8..a725c2f1d8 100644 --- a/lib/trento/activity_logging/parser/metadata_enricher.ex +++ b/lib/trento/activity_logging/parser/metadata_enricher.ex @@ -5,7 +5,7 @@ defmodule Trento.ActivityLog.Logger.Parser.MetadataEnricher do alias Trento.ActivityLog.ActivityCatalog - alias Trento.{Clusters, Databases, Hosts, SapSystems} + alias Trento.{Clusters, Databases, Hosts, SapSystems, Users} @spec enrich(activity :: ActivityCatalog.activity_type(), metadata :: map()) :: {:ok, maybe_enriched_metadata :: map()} @@ -27,13 +27,15 @@ defmodule Trento.ActivityLog.Logger.Parser.MetadataEnricher do |> maybe_enrich(:cluster, Clusters, :name) |> maybe_enrich(:database, Databases, :sid) |> maybe_enrich(:sap_system, SapSystems, :sid) + |> maybe_enrich(:user, Users, :username) |> elem(1) defp maybe_enrich({activity, metadata}, entity_reference, context_module, enriching_field) do enriched_metadata = with {:ok, entity_id} <- detect_enrichment(entity_reference, {activity, metadata}), {:ok, entity} <- context_module.by_id(entity_id), - {:ok, enriching_value} <- Map.fetch(entity, enriching_field) do + polished_entity <- polish_entity(entity), + {:ok, enriching_value} <- Map.fetch(polished_entity, enriching_field) do Map.put(metadata, enriching_field, enriching_value) else _ -> metadata @@ -42,18 +44,6 @@ defmodule Trento.ActivityLog.Logger.Parser.MetadataEnricher do {activity, enriched_metadata} end - defp detect_enrichment( - _target_entity, - {activity, - %{ - resource_id: resource_id, - resource_type: resource_type - }} - ) - when activity in [:resource_tagging, :resource_untagging] and - resource_type in [:host, :cluster, :database, :sap_system], - do: {:ok, resource_id} - defp detect_enrichment(:host, {:host_checks_execution_request, %{host_id: host_id}}), do: {:ok, host_id} @@ -79,7 +69,31 @@ defmodule Trento.ActivityLog.Logger.Parser.MetadataEnricher do defp detect_enrichment(:cluster, {_, %{cluster_id: id}}), do: {:ok, id} defp detect_enrichment(:database, {_, %{database_id: id}}), do: {:ok, id} defp detect_enrichment(:sap_system, {_, %{sap_system_id: id}}), do: {:ok, id} + defp detect_enrichment(:user, {_, %{user_id: id}}), do: {:ok, id} + + defp detect_enrichment( + target_entity, + {activity, + %{ + resource_id: resource_id, + resource_type: target_entity + }} + ) + when activity in [:resource_tagging, :resource_untagging], + do: {:ok, resource_id} defp detect_enrichment(_target_entity, {_activity, _metadata}), do: {:error, :no_enrichment_needed} + + defp polish_entity(%{username: username, deleted_at: deleted_at} = entity) do + # If the user is deleted, we append the deletion date to the username + # It's a implementation detail that we don't want to expose to the user + # See Trento.Users context for more information + clean_username = + String.trim_trailing(username, "__" <> DateTime.to_string(deleted_at)) + + Map.put(entity, :username, clean_username) + end + + defp polish_entity(entity), do: entity end diff --git a/lib/trento/activity_logging/parser/phoenix_conn_parser.ex b/lib/trento/activity_logging/parser/phoenix_conn_parser.ex index c0eb283436..370a0ba5e1 100644 --- a/lib/trento/activity_logging/parser/phoenix_conn_parser.ex +++ b/lib/trento/activity_logging/parser/phoenix_conn_parser.ex @@ -124,6 +124,11 @@ defmodule Trento.ActivityLog.Logger.Parser.PhoenixConnParser do ), do: %{host_id: Map.get(params, :id)} + def get_activity_metadata(:user_deletion, %Plug.Conn{ + params: params + }), + do: %{user_id: Map.get(params, :id)} + def get_activity_metadata(_, _), do: %{} defp redact(request_body, key) do diff --git a/lib/trento/users.ex b/lib/trento/users.ex index bb12353c4b..7a3f5c2e79 100644 --- a/lib/trento/users.ex +++ b/lib/trento/users.ex @@ -15,6 +15,14 @@ defmodule Trento.Users do alias Trento.Users.User + @spec by_id(id :: non_neg_integer()) :: {:ok, User.t()} | {:error, :not_found} + def by_id(id) do + case Repo.get(User, id) do + %User{} = user -> {:ok, user} + nil -> {:error, :not_found} + end + end + @impl true @doc """ get_by function overrides the one defined in Pow.Ecto.Context, diff --git a/lib/trento/users/user.ex b/lib/trento/users/user.ex index fd72605144..8bca3f40e9 100644 --- a/lib/trento/users/user.ex +++ b/lib/trento/users/user.ex @@ -27,6 +27,8 @@ defmodule Trento.Users.User do @sequences ["01234567890", "abcdefghijklmnopqrstuvwxyz", "ABCDEFGHIJKLMNOPQRSTUVWXYZ"] @max_sequential_chars 3 + @type t :: %__MODULE__{} + schema "users" do pow_user_fields() diff --git a/test/trento/activity_logging/activity_logger_test.exs b/test/trento/activity_logging/activity_logger_test.exs index c542760c7c..cfc181273b 100644 --- a/test/trento/activity_logging/activity_logger_test.exs +++ b/test/trento/activity_logging/activity_logger_test.exs @@ -95,6 +95,31 @@ defmodule Trento.ActivityLog.ActivityLoggerTest do end end + describe "user management activity detection" do + test "should trace user deletion", %{ + conn: conn, + user: %{id: admin_id, username: admin_username} + } do + %{id: user_id, username: username} = insert(:user) + + conn + |> with_token(admin_id) + |> delete("/api/v1/users/#{user_id}") + + wait_for_tasks_completion() + + [entry] = Trento.Repo.all(ActivityLog) + + assert [ + %ActivityLog{ + type: "user_deletion", + actor: ^admin_username, + metadata: %{"user_id" => ^user_id, "username" => ^username} + } + ] = [entry] + end + end + describe "tagging/untagging activity detection" do defp tag_resource(conn, resource_id, resource_type, tag) do conn diff --git a/test/trento/activity_logging/metadata_enricher_test.exs b/test/trento/activity_logging/metadata_enricher_test.exs index d3c97b8f40..ad98684304 100644 --- a/test/trento/activity_logging/metadata_enricher_test.exs +++ b/test/trento/activity_logging/metadata_enricher_test.exs @@ -203,6 +203,19 @@ defmodule Trento.ActivityLog.MetadataEnricherTest do }} = MetadataEnricher.enrich(:cluster_checks_execution_request, initial_metadata) end + + test "should enrich user deletion request" do + %{id: user_id, username: username} = insert(:user, deleted_at: Faker.DateTime.backward(1)) + + initial_metadata = %{user_id: user_id} + + assert {:ok, + %{ + user_id: ^user_id, + username: ^username + }} = + MetadataEnricher.enrich(:user_deletion, initial_metadata) + end end describe "domain event activity log metadata enrichment" do diff --git a/test/trento/users_test.exs b/test/trento/users_test.exs index 7f1f69ab78..3e52807243 100644 --- a/test/trento/users_test.exs +++ b/test/trento/users_test.exs @@ -640,6 +640,26 @@ defmodule Trento.UsersTest do assert {:error, :totp_invalid} = Users.validate_totp(user, totp_code) end + + test "by_id/1 get the user by its id" do + %{id: user_id} = user = insert(:user) + + {:ok, found} = Users.by_id(user_id) + + assert found.id == user.id + + # password is autogenerated and cannot be compared + assert Map.drop(found, [:password]) == Map.drop(user, [:password]) + end + + test "by_id/1 returns not found for unknown users" do + insert(:user) + unknown_user_id = Faker.Random.Elixir.random_between(1, 1000) + + result = Users.by_id(unknown_user_id) + + assert {:error, :not_found} == result + end end defp admin_username, do: Application.fetch_env!(:trento, :admin_user)