From 279b08264241355d734f4830975ce3b415e0d231 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Thu, 26 Nov 2020 14:48:10 +0100 Subject: [PATCH] Clean unconfirmed users Signed-off-by: Thomas Citharel --- config/config.exs | 7 +- lib/mix/tasks/mobilizon/users/clean.ex | 91 ++++++++++++ lib/mobilizon/users/users.ex | 3 + lib/service/clean_unconfirmed_users.ex | 59 ++++++++ .../workers/clean_unconfirmed_users_worker.ex | 31 +++++ test/service/clean_unconfirmed_users_test.exs | 56 ++++++++ .../users/clean_unconfirmed_users_test.exs | 131 ++++++++++++++++++ 7 files changed, 376 insertions(+), 2 deletions(-) create mode 100644 lib/mix/tasks/mobilizon/users/clean.ex create mode 100644 lib/service/clean_unconfirmed_users.ex create mode 100644 lib/service/workers/clean_unconfirmed_users_worker.ex create mode 100644 test/service/clean_unconfirmed_users_test.exs create mode 100644 test/tasks/users/clean_unconfirmed_users_test.exs diff --git a/config/config.exs b/config/config.exs index f9c50f8bc..44cac4fe5 100644 --- a/config/config.exs +++ b/config/config.exs @@ -30,6 +30,8 @@ config :mobilizon, :instance, banner_upload_limit: 4_000_000, remove_orphan_uploads: true, orphan_upload_grace_period_hours: 48, + remove_unconfirmed_users: true, + unconfirmed_user_grace_period_hours: 48, email_from: "noreply@localhost", email_reply_to: "noreply@localhost" @@ -251,9 +253,10 @@ config :mobilizon, Oban, queues: [default: 10, search: 5, mailers: 10, background: 5], crontab: [ {"@hourly", Mobilizon.Service.Workers.BuildSiteMap, queue: :background}, - {"17 * * * *", Mobilizon.Service.Workers.RefreshGroups, queue: :background} + {"17 * * * *", Mobilizon.Service.Workers.RefreshGroups, queue: :background}, # To be activated in Mobilizon 1.2 - # {"@hourly", Mobilizon.Service.Workers.CleanOrphanMediaWorker, queue: :background} + # {"@hourly", Mobilizon.Service.Workers.CleanOrphanMediaWorker, queue: :background}, + {"@hourly", Mobilizon.Service.Workers.CleanUnconfirmedUsersWorker, queue: :background} ] config :mobilizon, :rich_media, diff --git a/lib/mix/tasks/mobilizon/users/clean.ex b/lib/mix/tasks/mobilizon/users/clean.ex new file mode 100644 index 000000000..883d6de90 --- /dev/null +++ b/lib/mix/tasks/mobilizon/users/clean.ex @@ -0,0 +1,91 @@ +defmodule Mix.Tasks.Mobilizon.Users.Clean do + @moduledoc """ + Clean unconfirmed users + """ + + use Mix.Task + import Mix.Tasks.Mobilizon.Common + alias Mobilizon.Service.CleanUnconfirmedUsers + + @shortdoc "Clean unconfirmed users from Mobilizon" + @grace_period Mobilizon.Config.get([:instance, :unconfirmed_user_grace_period_hours], 48) + + @impl Mix.Task + def run(options) do + {options, [], []} = + OptionParser.parse( + options, + strict: [ + dry_run: :boolean, + days: :integer, + verbose: :boolean + ], + aliases: [ + d: :days, + v: :verbose + ] + ) + + dry_run = Keyword.get(options, :dry_run, false) + grace_period = Keyword.get(options, :days) + grace_period = if is_nil(grace_period), do: @grace_period, else: grace_period * 24 + verbose = Keyword.get(options, :verbose, false) + + start_mobilizon() + + case CleanUnconfirmedUsers.clean(dry_run: dry_run, grace_period: grace_period) do + {:ok, deleted_users} -> + if length(deleted_users) > 0 do + if dry_run or verbose do + details(deleted_users, dry_run, verbose) + end + + result(dry_run, length(deleted_users)) + else + empty_result(dry_run) + end + + :ok + + _err -> + shell_error("Error while cleaning unconfirmed users") + end + end + + @spec details(list(Media.t()), boolean(), boolean()) :: :ok + defp details(deleted_users, dry_run, verbose) do + cond do + dry_run -> + shell_info("List of users that would have been deleted") + + verbose -> + shell_info("List of users that have been deleted") + end + + Enum.each(deleted_users, fn deleted_user -> + shell_info( + "ID: #{deleted_user.id}, Email: #{deleted_user.email}, Profile: @#{ + hd(deleted_user.actors).preferred_username + }" + ) + end) + end + + @spec result(boolean(), boolean()) :: :ok + defp result(dry_run, nb_deleted_users) do + if dry_run do + shell_info("#{nb_deleted_users} users would have been deleted") + else + shell_info("#{nb_deleted_users} users have been deleted") + end + end + + @spec empty_result(boolean()) :: :ok + defp empty_result(dry_run) do + if dry_run do + shell_info("No users would have been deleted") + else + shell_info("No users were deleted") + end + end +end diff --git a/lib/mobilizon/users/users.ex b/lib/mobilizon/users/users.ex index 458cb227e..74f15b543 100644 --- a/lib/mobilizon/users/users.ex +++ b/lib/mobilizon/users/users.ex @@ -127,6 +127,9 @@ defmodule Mobilizon.Users do @doc """ Deletes an user. + + Options: + * `reserve_email` whether to keep a record of the email so that the user can't register again """ @spec delete_user(User.t()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} def delete_user(%User{id: user_id} = user, options \\ @delete_user_default_options) do diff --git a/lib/service/clean_unconfirmed_users.ex b/lib/service/clean_unconfirmed_users.ex new file mode 100644 index 000000000..145fe11cd --- /dev/null +++ b/lib/service/clean_unconfirmed_users.ex @@ -0,0 +1,59 @@ +defmodule Mobilizon.Service.CleanUnconfirmedUsers do + @moduledoc """ + Service to clean unconfirmed users + """ + + alias Mobilizon.{Actors, Users} + alias Mobilizon.Federation.ActivityPub.Relay + alias Mobilizon.Storage.Repo + alias Mobilizon.Users.User + import Ecto.Query + + @grace_period Mobilizon.Config.get([:instance, :unconfirmed_user_grace_period_hours], 48) + + @doc """ + Clean unattached media + + Remove media that is not attached to an entity, such as media uploads that were never used in entities. + """ + @spec clean(Keyword.t()) :: {:ok, list(Media.t())} | {:error, String.t()} + def clean(opts \\ []) do + users_to_delete = find_unconfirmed_users_to_clean(opts) + + if Keyword.get(opts, :dry_run, false) do + {:ok, users_to_delete} + else + users_to_delete = Enum.map(users_to_delete, &delete_user/1) + + {:ok, users_to_delete} + end + end + + @spec delete_user(User.t()) :: {:ok, User.t()} + defp delete_user(%User{} = user) do + with actors <- Users.get_actors_for_user(user), + :ok <- + Enum.each(actors, fn actor -> + actor_performing = Relay.get_actor() + + Actors.perform(:delete_actor, actor, + author_id: actor_performing.id, + reserve_username: false + ) + end), + {:ok, %User{} = user} <- Users.delete_user(user, reserve_email: false), + %User{} = user <- %User{user | actors: actors} do + user + end + end + + @spec find_unconfirmed_users_to_clean(Keyword.t()) :: list(User.t()) + defp find_unconfirmed_users_to_clean(opts) do + grace_period = Keyword.get(opts, :grace_period, @grace_period) + expiration_date = DateTime.add(DateTime.utc_now(), grace_period * -3600) + + User + |> where([u], is_nil(u.confirmed_at) and u.confirmation_sent_at < ^expiration_date) + |> Repo.all() + end +end diff --git a/lib/service/workers/clean_unconfirmed_users_worker.ex b/lib/service/workers/clean_unconfirmed_users_worker.ex new file mode 100644 index 000000000..472f3a03a --- /dev/null +++ b/lib/service/workers/clean_unconfirmed_users_worker.ex @@ -0,0 +1,31 @@ +defmodule Mobilizon.Service.Workers.CleanUnconfirmedUsersWorker do + @moduledoc """ + Worker to clean unattached media + """ + + use Oban.Worker, queue: "background" + alias Mobilizon.Service.CleanUnconfirmedUsers + + @grace_period Mobilizon.Config.get([:instance, :unconfirmed_user_grace_period_hours], 48) + + @impl Oban.Worker + def perform(%Job{}) do + if Mobilizon.Config.get!([:instance, :remove_unconfirmed_users]) and should_perform?() do + CleanUnconfirmedUsers.clean() + end + end + + @spec should_perform? :: boolean() + defp should_perform? do + case Cachex.get(:key_value, "last_media_cleanup") do + {:ok, %DateTime{} = last_media_cleanup} -> + DateTime.compare( + last_media_cleanup, + DateTime.add(DateTime.utc_now(), @grace_period * -3600) + ) == :lt + + _ -> + true + end + end +end diff --git a/test/service/clean_unconfirmed_users_test.exs b/test/service/clean_unconfirmed_users_test.exs new file mode 100644 index 000000000..51ced50a3 --- /dev/null +++ b/test/service/clean_unconfirmed_users_test.exs @@ -0,0 +1,56 @@ +defmodule Mix.Tasks.Mobilizon.User.CleanUnconfirmedUsersTest do + use Mobilizon.DataCase + + import Mobilizon.Factory + + alias Mobilizon.Service.CleanUnconfirmedUsers + alias Mobilizon.Users + alias Mobilizon.Users.User + + describe "clean unconfirmed users" do + test "with default values" do + {:ok, old, _} = DateTime.from_iso8601("2020-11-20T17:35:23+01:00") + %User{id: user_id} = insert(:user, confirmation_sent_at: old, confirmed_at: nil) + %User{id: user_2_id} = insert(:user) + + refute is_nil(Users.get_user(user_id)) + refute is_nil(Users.get_user(user_2_id)) + + assert {:ok, [found_user]} = CleanUnconfirmedUsers.clean() + assert found_user.id == user_id + + assert is_nil(Users.get_user(user_id)) + refute is_nil(Users.get_user(user_2_id)) + end + + test "as dry-run" do + {:ok, old, _} = DateTime.from_iso8601("2020-11-20T17:35:23+01:00") + %User{id: user_id} = insert(:user, confirmation_sent_at: old, confirmed_at: nil) + %User{id: user_2_id} = insert(:user) + + refute is_nil(Users.get_user(user_id)) + refute is_nil(Users.get_user(user_2_id)) + + assert {:ok, [found_user]} = CleanUnconfirmedUsers.clean(dry_run: true) + assert found_user.id == user_id + + refute is_nil(Users.get_user(user_id)) + refute is_nil(Users.get_user(user_2_id)) + end + + test "with custom grace period" do + date = DateTime.utc_now() |> DateTime.add(24 * -3600) + %User{id: user_id} = insert(:user, confirmation_sent_at: date, confirmed_at: nil) + %User{id: user_2_id} = insert(:user) + + refute is_nil(Users.get_user(user_id)) + refute is_nil(Users.get_user(user_2_id)) + + assert {:ok, [found_user]} = CleanUnconfirmedUsers.clean(grace_period: 12) + assert found_user.id == user_id + + assert is_nil(Users.get_user(user_id)) + refute is_nil(Users.get_user(user_2_id)) + end + end +end diff --git a/test/tasks/users/clean_unconfirmed_users_test.exs b/test/tasks/users/clean_unconfirmed_users_test.exs new file mode 100644 index 000000000..10d56c573 --- /dev/null +++ b/test/tasks/users/clean_unconfirmed_users_test.exs @@ -0,0 +1,131 @@ +defmodule Mix.Tasks.Mobilizon.Media.CleanUnconfirmedUsersTest do + use Mobilizon.DataCase + import Mock + import Mobilizon.Factory + + alias Mix.Tasks.Mobilizon.Users.Clean + alias Mobilizon.Service.CleanUnconfirmedUsers + + Mix.shell(Mix.Shell.Process) + + describe "with default options" do + test "nothing returned" do + with_mock CleanUnconfirmedUsers, + clean: fn [dry_run: false, grace_period: 48] -> {:ok, []} end do + Clean.run([]) + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "No users were deleted" + end + end + + test "users returned" do + actor1 = insert(:actor) + user1 = insert(:user, actors: [actor1]) + actor2 = insert(:actor) + user2 = insert(:user, actors: [actor2]) + + with_mock CleanUnconfirmedUsers, + clean: fn [dry_run: false, grace_period: 48] -> {:ok, [user1, user2]} end do + Clean.run([]) + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "2 users have been deleted" + end + end + end + + describe "with dry-run option" do + test "with nothing returned" do + with_mock CleanUnconfirmedUsers, + clean: fn [dry_run: true, grace_period: 48] -> {:ok, []} end do + Clean.run(["--dry-run"]) + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "No users would have been deleted" + end + end + + test "with users returned" do + actor1 = insert(:actor) + user1 = insert(:user, actors: [actor1]) + actor2 = insert(:actor) + user2 = insert(:user, actors: [actor2]) + + with_mock CleanUnconfirmedUsers, + clean: fn [dry_run: true, grace_period: 48] -> {:ok, [user1, user2]} end do + Clean.run(["--dry-run"]) + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "List of users that would have been deleted" + assert_received {:mix_shell, :info, [output_received]} + + assert output_received == + "ID: #{user1.id}, Email: #{user1.email}, Profile: @#{actor1.preferred_username}" + + assert_received {:mix_shell, :info, [output_received]} + + assert output_received == + "ID: #{user2.id}, Email: #{user2.email}, Profile: @#{actor2.preferred_username}" + + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "2 users would have been deleted" + end + end + end + + describe "with verbose option" do + test "with nothing returned" do + with_mock CleanUnconfirmedUsers, + clean: fn [dry_run: false, grace_period: 48] -> {:ok, []} end do + Clean.run(["--verbose"]) + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "No users were deleted" + end + end + + test "with users returned" do + actor1 = insert(:actor) + user1 = insert(:user, actors: [actor1]) + actor2 = insert(:actor) + user2 = insert(:user, actors: [actor2]) + + with_mock CleanUnconfirmedUsers, + clean: fn [dry_run: false, grace_period: 48] -> {:ok, [user1, user2]} end do + Clean.run(["--verbose"]) + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "List of users that have been deleted" + assert_received {:mix_shell, :info, [output_received]} + + assert output_received == + "ID: #{user1.id}, Email: #{user1.email}, Profile: @#{actor1.preferred_username}" + + assert_received {:mix_shell, :info, [output_received]} + + assert output_received == + "ID: #{user2.id}, Email: #{user2.email}, Profile: @#{actor2.preferred_username}" + + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "2 users have been deleted" + end + end + end + + describe "with days option" do + test "with nothing returned" do + with_mock CleanUnconfirmedUsers, + clean: fn [dry_run: false, grace_period: 120] -> {:ok, []} end do + Clean.run(["--days", "5"]) + assert_received {:mix_shell, :info, [output_received]} + assert output_received == "No users were deleted" + end + end + end + + describe "returns an error" do + test "for some reason" do + with_mock CleanUnconfirmedUsers, + clean: fn [dry_run: false, grace_period: 48] -> {:error, "Some error"} end do + Clean.run([]) + assert_received {:mix_shell, :error, [output_received]} + assert output_received == "Error while cleaning unconfirmed users" + end + end + end +end