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

diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index b451202b255209cee0672c9ffd72e26c982ef5b7..99358ddafbc2444dc29c00cbed795c568648fa4e 100644 (file)
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -1445,8 +1445,15 @@ defmodule Pleroma.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)

diff --git a/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex b/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex
index 2c1874051ed833aac8336be4a58351ae30b5f41b..1bdb3aa4dcc531c00a24164f388af579a34a5411 100644 (file)
--- a/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex
+++ b/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex
@@ -61,7 +61,10 @@ defmodule Pleroma.Web.PleromaAPI.PleromaAPIController do
           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,

diff --git a/priv/repo/migrations/20200428221338_insert_skeletons_for_deleted_users.exs b/priv/repo/migrations/20200428221338_insert_skeletons_for_deleted_users.exs
new file mode 100644 (file)
index 0000000..11d9a70
--- /dev/null
+++ b/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

diff --git a/test/tasks/user_test.exs b/test/tasks/user_test.exs
index 8df835b56d6dba14f2076bdcd949dcde417a5868..0f6ffb2b1d49a73011751289bc708851f22512e3 100644 (file)
--- a/test/tasks/user_test.exs
+++ b/test/tasks/user_test.exs
@@ -92,7 +92,7 @@ defmodule Mix.Tasks.Pleroma.UserTest 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

diff --git a/test/user_test.exs b/test/user_test.exs
index 347c5be72ccde50ebedd3290d01a3a5b947ff9f3..bff337d3e48bacf56991f2c4e4bbe7ed07143fc8 100644 (file)
--- a/test/user_test.exs
+++ b/test/user_test.exs
@@ -1135,16 +1135,7 @@ defmodule Pleroma.UserTest do
       refute Activity.get_by_id(activity.id)
     end
 
-    test "it deletes deactivated 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
+    test "it deactivates a user, all follow relationships and all activities", %{user: user} do
       follower = insert(:user)
       {:ok, follower} = User.follow(follower, user)
 
@@ -1164,8 +1155,7 @@ defmodule Pleroma.UserTest do
       follower = User.get_cached_by_id(follower.id)
 
       refute User.following?(follower, user)
-      refute User.get_by_id(user.id)
-      assert {:ok, nil} == Cachex.get(:user_cache, "ap_id:#{user.ap_id}")
+      assert %{deactivated: true} = User.get_by_id(user.id)
 
       user_activities =
         user.ap_id

diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs
index 10d86ee454a2777aad67f16900da0738f56f30f1..36e1e7bd113c3e927dce3987df48f8e9cac6bf75 100644 (file)
--- a/test/web/activity_pub/transmogrifier_test.exs
+++ b/test/web/activity_pub/transmogrifier_test.exs
@@ -872,7 +872,8 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest 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")