Merge branch 'clean-unused-users' into 'master'

Clean unconfirmed users

Closes #51

See merge request framasoft/mobilizon!729

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,

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

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

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

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

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

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