diff --git a/lib/trento/activity_log.ex b/lib/trento/activity_log.ex index 65bd098fb7..f8d643b577 100644 --- a/lib/trento/activity_log.ex +++ b/lib/trento/activity_log.ex @@ -7,12 +7,11 @@ defmodule Trento.ActivityLog do require Logger - alias Trento.ActivityLog.RetentionTime - require Trento.ActivityLog.RetentionPeriodUnit, as: RetentionPeriodUnit alias Trento.ActivityLog.ActivityLog alias Trento.ActivityLog.MetadataQueryParser - alias Trento.ActivityLog.Settings + alias Trento.ActivityLog.RetentionTime alias Trento.Repo + alias Trento.Settings @user_management_log_types [ "login_attempt", @@ -22,45 +21,6 @@ defmodule Trento.ActivityLog do "profile_update" ] - @spec get_settings() :: - {:ok, Settings.t()} | {:error, :not_found} - def get_settings do - case Repo.one(Settings.base_query()) do - %Settings{} = settings -> {:ok, settings} - nil -> {:error, :not_found} - end - end - - @spec change_retention_period(integer(), RetentionPeriodUnit.t()) :: - {:ok, Settings.t()} - | {:error, :activity_log_settings_not_configured} - def change_retention_period(value, unit) do - case get_settings() do - {:ok, settings} -> - settings - |> Settings.changeset(%{ - retention_time: %{ - value: value, - unit: unit - } - }) - |> Repo.update() - |> log_error("Error while updating activity log retention period") - - {:error, :not_found} -> - {:error, :activity_log_settings_not_configured} - end - end - - @spec clear_expired_logs() :: :ok | {:error, any()} - def clear_expired_logs do - with {:ok, %{retention_time: retention_time}} <- Trento.ActivityLog.get_settings(), - expiration_date <- calculate_expiration_date(retention_time) do - delete_logs_before(expiration_date) - :ok - end - end - @spec list_activity_log(map()) :: {:ok, list(ActivityLog.t()), Flop.Meta.t()} | {:error, :activity_log_fetch_error} def list_activity_log(params, include_all_log_types? \\ false) do @@ -79,6 +39,15 @@ defmodule Trento.ActivityLog do end end + @spec clear_expired_logs() :: :ok | {:error, any()} + def clear_expired_logs do + with {:ok, %{retention_time: retention_time}} <- Settings.get_activity_log_settings(), + expiration_date <- calculate_expiration_date(retention_time) do + delete_logs_before(expiration_date) + :ok + end + end + defp maybe_exclude_user_logs(ActivityLog = q, true = _include_all_log_types?), do: q defp maybe_exclude_user_logs(ActivityLog = q, false = _include_all_log_types?) do @@ -154,13 +123,6 @@ defmodule Trento.ActivityLog do end) end - defp log_error({:error, _} = error, message) do - Logger.error("#{message}: #{inspect(error)}") - error - end - - defp log_error(result, _), do: result - defp calculate_expiration_date(%RetentionTime{value: value, unit: :day}), do: DateTime.add(DateTime.utc_now(), -value, :day) diff --git a/lib/trento/release.ex b/lib/trento/release.ex index 6e9c1db0d0..8774988351 100644 --- a/lib/trento/release.ex +++ b/lib/trento/release.ex @@ -8,9 +8,11 @@ defmodule Trento.Release do alias Mix.Tasks.Phx.Gen.Cert - alias Trento.ActivityLog.Settings, as: ActivityLogSettings - alias Trento.Settings.ApiKeySettings - alias Trento.Settings.SSOCertificatesSettings + alias Trento.Settings.{ + ActivityLogSettings, + ApiKeySettings, + SSOCertificatesSettings + } @app :trento diff --git a/lib/trento/settings.ex b/lib/trento/settings.ex index cb7bbda3d9..677d52c58d 100644 --- a/lib/trento/settings.ex +++ b/lib/trento/settings.ex @@ -8,6 +8,7 @@ defmodule Trento.Settings do alias Trento.SoftwareUpdates.Discovery, as: SoftwareUpdatesDiscovery alias Trento.Settings.{ + ActivityLogSettings, ApiKeySettings, InstallationSettings, SSOCertificatesSettings, @@ -16,6 +17,8 @@ defmodule Trento.Settings do alias Trento.Support.DateService + require Trento.ActivityLog.RetentionPeriodUnit, as: RetentionPeriodUnit + require Logger @type suse_manager_settings_save_submission :: %{ @@ -119,6 +122,38 @@ defmodule Trento.Settings do :ok end + # Activity log settings + + @spec get_activity_log_settings() :: + {:ok, ActivityLogSettings.t()} | {:error, :activity_log_settings_not_configured} + def get_activity_log_settings do + case Repo.one(ActivityLogSettings.base_query()) do + %ActivityLogSettings{} = settings -> {:ok, settings} + nil -> {:error, :activity_log_settings_not_configured} + end + end + + @spec change_activity_log_retention_period(integer(), RetentionPeriodUnit.t()) :: + {:ok, ActivityLogSettings.t()} + | {:error, :activity_log_settings_not_configured} + def change_activity_log_retention_period(value, unit) do + case get_activity_log_settings() do + {:ok, settings} -> + settings + |> ActivityLogSettings.changeset(%{ + retention_time: %{ + value: value, + unit: unit + } + }) + |> Repo.update() + |> log_error("Error while updating activity log retention period") + + error -> + error + end + end + # Certificates settings @spec get_sso_certificates() :: [SSOCertificatesSettings.t()] diff --git a/lib/trento/activity_logging/settings.ex b/lib/trento/settings/activity_log_settings.ex similarity index 81% rename from lib/trento/activity_logging/settings.ex rename to lib/trento/settings/activity_log_settings.ex index 4049cb75c3..4592a7b10b 100644 --- a/lib/trento/activity_logging/settings.ex +++ b/lib/trento/settings/activity_log_settings.ex @@ -1,6 +1,6 @@ -defmodule Trento.ActivityLog.Settings do +defmodule Trento.Settings.ActivityLogSettings do @moduledoc """ - ActivityLog Settings is the STI projection of activity log related settings + ActivityLogSettings is the STI projection of activity log related settings """ use Ecto.Schema @@ -36,6 +36,6 @@ defmodule Trento.ActivityLog.Settings do end def with_default_retention_time do - changeset(%Settings{}, %{retention_time: RetentionTime.default()}) + changeset(%ActivityLogSettings{}, %{retention_time: RetentionTime.default()}) end end diff --git a/lib/trento_web/controllers/v1/settings_controller.ex b/lib/trento_web/controllers/v1/settings_controller.ex index 57bfecfbce..b1f7b65997 100644 --- a/lib/trento_web/controllers/v1/settings_controller.ex +++ b/lib/trento_web/controllers/v1/settings_controller.ex @@ -2,7 +2,6 @@ defmodule TrentoWeb.V1.SettingsController do use TrentoWeb, :controller use OpenApiSpex.ControllerSpecs - alias Trento.ActivityLog alias Trento.Settings alias Trento.SoftwareUpdates alias TrentoWeb.OpenApi.V1.Schema @@ -124,7 +123,7 @@ defmodule TrentoWeb.V1.SettingsController do OpenApiSpex.body_params(conn) with {:ok, updated_settings} <- - ActivityLog.change_retention_period(retention_period, retention_period_unit) do + Settings.change_activity_log_retention_period(retention_period, retention_period_unit) do render(conn, :activity_log_settings, %{ activity_log_settings: updated_settings }) @@ -142,10 +141,9 @@ defmodule TrentoWeb.V1.SettingsController do ] def get_activity_log_settings(conn, _) do - with {:ok, settings} <- ActivityLog.get_settings() do - render(conn, :activity_log_settings, %{ - activity_log_settings: settings - }) + case Settings.get_activity_log_settings() do + {:ok, settings} -> render(conn, :activity_log_settings, %{activity_log_settings: settings}) + {:error, :activity_log_settings_not_configured} -> {:error, :not_found} end end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 3fcaee9157..97d1dda956 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -37,6 +37,6 @@ }) |> Trento.Repo.insert!(on_conflict: :nothing) -Trento.Repo.insert!(Trento.ActivityLog.Settings.with_default_retention_time(), +Trento.Repo.insert!(Trento.Settings.ActivityLogSettings.with_default_retention_time(), on_conflict: :nothing ) diff --git a/test/support/factory.ex b/test/support/factory.ex index dc62c1864f..d2bfe7f3c3 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -130,6 +130,7 @@ defmodule Trento.Factory do } alias Trento.Settings.{ + ActivityLogSettings, ApiKeySettings, InstallationSettings, SSOCertificatesSettings, @@ -138,7 +139,6 @@ defmodule Trento.Factory do alias Trento.ActivityLog.ActivityLog, as: ActivityLogEntry alias Trento.ActivityLog.RetentionTime - alias Trento.ActivityLog.Settings, as: ActivityLogSettings alias Trento.Abilities.{ Ability, diff --git a/test/trento/activity_log_test.exs b/test/trento/activity_log_test.exs index a8f0de5de3..2526615955 100644 --- a/test/trento/activity_log_test.exs +++ b/test/trento/activity_log_test.exs @@ -7,178 +7,11 @@ defmodule Trento.ActivityLogTest do import Trento.Factory - require Trento.ActivityLog.RetentionPeriodUnit, as: RetentionPeriodUnit - alias Trento.ActivityLog alias Trento.ActivityLog.ActivityLog, as: ActivityLogEntry - alias Trento.ActivityLog.RetentionTime - alias Trento.ActivityLog.Settings setup :verify_on_exit! - describe "retrieving activity log settings" do - test "should return an error when settings are not available" do - assert {:error, :not_found} == ActivityLog.get_settings() - end - - test "should return settings" do - %{ - retention_time: %{ - value: value, - unit: unit - } - } = insert(:activity_log_settings) - - assert {:ok, - %Settings{ - retention_time: %RetentionTime{ - value: ^value, - unit: ^unit - } - }} = ActivityLog.get_settings() - end - end - - describe "changing activity log settings" do - test "should not be able to change retention time if no activity log settings were previously saved" do - assert {:error, :activity_log_settings_not_configured} == - ActivityLog.change_retention_period(42, RetentionPeriodUnit.day()) - end - - @validation_scenarios [ - %{ - invalid_retention_periods: [-1, 0], - expected_errors: [ - value: - {"must be greater than %{number}", - [validation: :number, kind: :greater_than, number: 0]} - ] - }, - %{ - invalid_retention_periods: [nil, "", " "], - expected_errors: [value: {"can't be blank", [validation: :required]}] - } - ] - - test "should not accept invalid retention periods" do - insert(:activity_log_settings) - - for %{ - invalid_retention_periods: invalid_retention_periods, - expected_errors: expected_errors - } <- @validation_scenarios do - Enum.each(invalid_retention_periods, fn invalid_retention_period -> - unit = Faker.Util.pick(RetentionPeriodUnit.values()) - - assert {:error, - %{ - valid?: false, - changes: %{retention_time: %{errors: ^expected_errors}} - }} = - ActivityLog.change_retention_period( - invalid_retention_period, - unit - ) - end) - end - end - - test "should not accept unsupported retention period units" do - insert(:activity_log_settings) - - for unit <- [:foo, :bar, :baz] do - assert {:error, - %{ - valid?: false, - changes: %{ - retention_time: %{ - errors: [ - unit: {"is invalid", _} - ] - } - } - }} = ActivityLog.change_retention_period(42, unit) - end - end - - scenarios = [ - %{ - name: "days", - value: 1, - unit: RetentionPeriodUnit.day() - }, - %{ - name: "weeks", - value: 3, - unit: RetentionPeriodUnit.week() - }, - %{ - name: "months", - value: 5, - unit: RetentionPeriodUnit.month() - }, - %{ - name: "years", - value: 7, - unit: RetentionPeriodUnit.year() - } - ] - - for %{name: name} = scenario <- scenarios do - @scenario scenario - - test "should successfully change retention periods #{name}" do - insert(:activity_log_settings, - retention_time: %{ - value: 92, - unit: RetentionPeriodUnit.year() - } - ) - - %{ - value: value, - unit: unit - } = @scenario - - assert {:ok, - %Settings{ - retention_time: %RetentionTime{ - value: ^value, - unit: ^unit - } - }} = - ActivityLog.change_retention_period( - value, - unit - ) - end - end - - test "should successfully handle unchanging retention periods" do - initial_retention_period = 42 - initial_retention_period_unit = RetentionPeriodUnit.day() - - insert(:activity_log_settings, - retention_time: %{ - value: initial_retention_period, - unit: initial_retention_period_unit - } - ) - - assert {:ok, - %Settings{ - retention_time: %RetentionTime{ - value: ^initial_retention_period, - unit: ^initial_retention_period_unit - } - }} = - ActivityLog.change_retention_period( - initial_retention_period, - initial_retention_period_unit - ) - end - end - describe "retrieving logged activity" do test "should return an emtpty list" do assert {:ok, [], %Flop.Meta{}} = ActivityLog.list_activity_log(%{}) diff --git a/test/trento/release_test.exs b/test/trento/release_test.exs index 5fbdc79ef9..64bda20111 100644 --- a/test/trento/release_test.exs +++ b/test/trento/release_test.exs @@ -10,8 +10,11 @@ defmodule Trento.ReleaseTest do alias Trento.Release alias Trento.ActivityLog.RetentionTime - alias Trento.ActivityLog.Settings, as: ActivityLogSettings - alias Trento.Settings.SSOCertificatesSettings + + alias Trento.Settings.{ + ActivityLogSettings, + SSOCertificatesSettings + } describe "Activity Log settings initiation" do test "should init default activity log retention time" do diff --git a/test/trento/settings_test.exs b/test/trento/settings_test.exs index 04970d3922..b822b13d00 100644 --- a/test/trento/settings_test.exs +++ b/test/trento/settings_test.exs @@ -9,9 +9,14 @@ defmodule Trento.SettingsTest do import Trento.Factory + require Trento.ActivityLog.RetentionPeriodUnit, as: RetentionPeriodUnit + + alias Trento.ActivityLog.RetentionTime + alias Trento.Settings alias Trento.Settings.{ + ActivityLogSettings, ApiKeySettings, InstallationSettings, SuseManagerSettings @@ -615,6 +620,168 @@ defmodule Trento.SettingsTest do end end + describe "activity log settings" do + test "should return an error when settings are not available" do + assert {:error, :activity_log_settings_not_configured} == + Settings.get_activity_log_settings() + end + + test "should return activity log settings" do + %{ + retention_time: %{ + value: value, + unit: unit + } + } = insert(:activity_log_settings) + + assert {:ok, + %ActivityLogSettings{ + retention_time: %RetentionTime{ + value: ^value, + unit: ^unit + } + }} = Settings.get_activity_log_settings() + end + + test "should not be able to change retention time if no activity log settings were previously saved" do + assert {:error, :activity_log_settings_not_configured} == + Settings.change_activity_log_retention_period(42, RetentionPeriodUnit.day()) + end + + test "should not accept invalid retention periods" do + insert(:activity_log_settings) + + validation_scenarios = [ + %{ + invalid_retention_periods: [-1, 0], + expected_errors: [ + value: + {"must be greater than %{number}", + [validation: :number, kind: :greater_than, number: 0]} + ] + }, + %{ + invalid_retention_periods: [nil, "", " "], + expected_errors: [value: {"can't be blank", [validation: :required]}] + } + ] + + for %{ + invalid_retention_periods: invalid_retention_periods, + expected_errors: expected_errors + } <- validation_scenarios do + Enum.each(invalid_retention_periods, fn invalid_retention_period -> + unit = Faker.Util.pick(RetentionPeriodUnit.values()) + + assert {:error, + %{ + valid?: false, + changes: %{retention_time: %{errors: ^expected_errors}} + }} = + Settings.change_activity_log_retention_period( + invalid_retention_period, + unit + ) + end) + end + end + + test "should not accept unsupported retention period units" do + insert(:activity_log_settings) + + for unit <- [:foo, :bar, :baz] do + assert {:error, + %{ + valid?: false, + changes: %{ + retention_time: %{ + errors: [ + unit: {"is invalid", _} + ] + } + } + }} = Settings.change_activity_log_retention_period(42, unit) + end + end + + scenarios = [ + %{ + name: "days", + value: 1, + unit: RetentionPeriodUnit.day() + }, + %{ + name: "weeks", + value: 3, + unit: RetentionPeriodUnit.week() + }, + %{ + name: "months", + value: 5, + unit: RetentionPeriodUnit.month() + }, + %{ + name: "years", + value: 7, + unit: RetentionPeriodUnit.year() + } + ] + + for %{name: name} = scenario <- scenarios do + @scenario scenario + + test "should successfully change retention periods #{name}" do + insert(:activity_log_settings, + retention_time: %{ + value: 92, + unit: RetentionPeriodUnit.year() + } + ) + + %{ + value: value, + unit: unit + } = @scenario + + assert {:ok, + %ActivityLogSettings{ + retention_time: %RetentionTime{ + value: ^value, + unit: ^unit + } + }} = + Settings.change_activity_log_retention_period( + value, + unit + ) + end + end + + test "should successfully handle unchanging retention periods" do + initial_retention_period = 42 + initial_retention_period_unit = RetentionPeriodUnit.day() + + insert(:activity_log_settings, + retention_time: %{ + value: initial_retention_period, + unit: initial_retention_period_unit + } + ) + + assert {:ok, + %ActivityLogSettings{ + retention_time: %RetentionTime{ + value: ^initial_retention_period, + unit: ^initial_retention_period_unit + } + }} = + Settings.change_activity_log_retention_period( + initial_retention_period, + initial_retention_period_unit + ) + end + end + describe "sso_certificates_settings" do test "should return SSO certificates" do certificates = insert(:sso_certificates_settings)