Deactivate local users on deletion instead of deleting the record

Prevents the possibility of re-registration, which allowed to read
DMs of the deleted account.

Also includes a migration that tries to find any already deleted
accounts and insert skeletons for them.

Closes pleroma/pleroma#1687

lib/pleroma/user.ex

@@ -1445,8 +1445,15 @@ def perform(:delete, %User{} = user) do
     end)
 
     delete_user_activities(user)
-    invalidate_cache(user)
+
-    Repo.delete(user)
+    if user.local do
+      user
+      |> change(%{deactivated: true, email: nil})
+      |> update_and_set_cache()
+    else
+      invalidate_cache(user)
+      Repo.delete(user)
+    end
   end
 
   def perform(:deactivate_async, user, status), do: deactivate(user, status)

lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex

@@ -61,7 +61,10 @@ def emoji_reactions_by(%{assigns: %{user: user}} = conn, %{"id" => activity_id}
           else
             users =
               Enum.map(user_ap_ids, &User.get_cached_by_ap_id/1)
-              |> Enum.filter(& &1)
+              |> Enum.filter(fn
+                %{deactivated: false} -> true
+                _ -> false
+              end)
 
             %{
               name: emoji,

priv/repo/migrations/20200428221338_insert_skeletons_for_deleted_users.exs

@@ -0,0 +1,45 @@
+defmodule Pleroma.Repo.Migrations.InsertSkeletonsForDeletedUsers do
+  use Ecto.Migration
+
+  alias Pleroma.User
+  alias Pleroma.Repo
+
+  import Ecto.Query
+
+  def change do
+    Application.ensure_all_started(:flake_id)
+
+    local_ap_id =
+      User.Query.build(%{local: true})
+      |> select([u], u.ap_id)
+      |> limit(1)
+      |> Repo.one()
+
+    unless local_ap_id == nil do
+      # Hack to get instance base url because getting it from Phoenix
+      # would require starting the whole application
+      instance_uri =
+        local_ap_id
+        |> URI.parse()
+        |> Map.put(:query, nil)
+        |> Map.put(:path, nil)
+        |> URI.to_string()
+
+      {:ok, %{rows: ap_ids}} =
+        Ecto.Adapters.SQL.query(
+          Repo,
+          "select distinct unnest(nonexistent_locals.recipients) from activities, lateral (select array_agg(recipient) as recipients from unnest(activities.recipients) as recipient where recipient similar to '#{
+            instance_uri
+          }/users/[A-Za-z0-9]*' and not(recipient in (select ap_id from users where local = true))) nonexistent_locals;",
+          [],
+          timeout: :infinity
+        )
+
+      ap_ids
+      |> Enum.each(fn [ap_id] ->
+        Ecto.Changeset.change(%User{}, deactivated: true, ap_id: ap_id)
+        |> Repo.insert()
+      end)
+    end
+  end
+end

test/tasks/user_test.exs

@@ -92,7 +92,7 @@ test "user is deleted" do
       assert_received {:mix_shell, :info, [message]}
       assert message =~ " deleted"
 
-      refute User.get_by_nickname(user.nickname)
+      assert %{deactivated: true} = User.get_by_nickname(user.nickname)
     end
 
     test "no user to delete" do

test/user_test.exs

@@ -1135,16 +1135,7 @@ test ".delete_user_activities deletes all create activities", %{user: user} do
       refute Activity.get_by_id(activity.id)
     end
 
-    test "it deletes deactivated user" do
+    test "it deactivates a user, all follow relationships and all activities", %{user: user} do
-      {:ok, user} = insert(:user, deactivated: true) |> User.set_cache()
-
-      {:ok, job} = User.delete(user)
-      {:ok, _user} = ObanHelpers.perform(job)
-
-      refute User.get_by_id(user.id)
-    end
-
-    test "it deletes a user, all follow relationships and all activities", %{user: user} do
       follower = insert(:user)
       {:ok, follower} = User.follow(follower, user)
 
@@ -1164,8 +1155,7 @@ test "it deletes a user, all follow relationships and all activities", %{user: u
       follower = User.get_cached_by_id(follower.id)
 
       refute User.following?(follower, user)
-      refute User.get_by_id(user.id)
+      assert %{deactivated: true} = User.get_by_id(user.id)
-      assert {:ok, nil} == Cachex.get(:user_cache, "ap_id:#{user.ap_id}")
 
       user_activities =
         user.ap_id

test/web/activity_pub/transmogrifier_test.exs

@@ -872,7 +872,8 @@ test "it fails for incoming deletes with spoofed origin" do
 
     @tag capture_log: true
     test "it works for incoming user deletes" do
-      %{ap_id: ap_id} = insert(:user, ap_id: "http://mastodon.example.org/users/admin")
+      %{ap_id: ap_id} =
+        insert(:user, ap_id: "http://mastodon.example.org/users/admin", local: false)
 
       data =
         File.read!("test/fixtures/mastodon-delete-user.json")