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 0115abed53c059600be9490fd4c14ad1af2646f9..0e5121694bd4ab298ca87baf635039ce82f2c26d 100644 (file)
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -1416,8 +1416,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 dae7f0f2f7aff92eab595b1f80e1bf332937f1b2..41677d04db9cd2c3fd4cf2c49ee8645455a2a987 100644 (file)
--- a/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex
+++ b/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex
@@ -53,7 +53,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 b45f372630fa6594911b3df16efab20fcd9af1d5..22030a4235dad8231b08239a4c6333260e23e719 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 f3d044a803e147992b314c52f4ab1fd07651ccbe..555bbb92f0fe844f9d319c204fd32af4b2a90da7 100644 (file)
--- a/test/user_test.exs
+++ b/test/user_test.exs
@@ -1127,16 +1127,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)
 
@@ -1156,8 +1147,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 efbca82f620b177bd9691ef19d612afd8369bac7..2baf9ce03a420dace487ef08bbcc793c551bf034 100644 (file)
--- a/test/web/activity_pub/transmogrifier_test.exs
+++ b/test/web/activity_pub/transmogrifier_test.exs
@@ -870,7 +870,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")