Purge obsolete ap_enabled indicator

It was used to migrate OStatus connections to ActivityPub if possible,
but support for OStatus was long since dropped, all new actors always AP
and if anything wasn't migrated before, their instance is already marked
as unreachable anyway.

The associated logic was also buggy in several ways and deleted users
got set to ap_enabled=false also causing some issues.

This patch is a pretty direct port of the original Pleroma MR;
follow-up commits will further fix and clean up remaining issues.
Changes made (other than trivial merge conflict resolutions):
  - converted CHANGELOG format
  - adapted migration id for Akkoma’s timeline
  - removed ap_enabled from additional tests

Ported-from: https://git.pleroma.social/pleroma/pleroma/-/merge_requests/3880

CHANGELOG.md

@@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased
 
+## Added
+
+## Fixed
+
+## Changed
+- Dropped obsolete `ap_enabled` indicator from user table and associated buggy logic
+
 ## 2025.01.01
 
 Hotfix: Federation could break if a null value found its way into `should_federate?\1`

lib/pleroma/user.ex

@@ -127,7 +127,6 @@ defmodule Pleroma.User do
     field(:domain_blocks, {:array, :string}, default: [])
     field(:is_active, :boolean, default: true)
     field(:no_rich_text, :boolean, default: false)
-    field(:ap_enabled, :boolean, default: false)
     field(:is_moderator, :boolean, default: false)
     field(:is_admin, :boolean, default: false)
     field(:show_role, :boolean, default: true)
@@ -473,7 +472,6 @@ def remote_user_changeset(struct \\ %User{local: false}, params) do
         :shared_inbox,
         :nickname,
         :avatar,
-        :ap_enabled,
         :banner,
         :background,
         :is_locked,
@@ -1006,12 +1004,8 @@ def maybe_direct_follow(%User{} = follower, %User{local: true} = followed) do
   end
 
   def maybe_direct_follow(%User{} = follower, %User{} = followed) do
-    if not ap_enabled?(followed) do
-      follow(follower, followed)
-    else
     {:ok, follower, followed}
   end
-  end
 
   @doc "A mass follow for local users. Respects blocks in both directions but does not create activities."
   @spec follow_all(User.t(), list(User.t())) :: {atom(), User.t()}
@@ -1826,7 +1820,6 @@ def purge_user_changeset(user) do
       confirmation_token: nil,
       domain_blocks: [],
       is_active: false,
-      ap_enabled: false,
       is_moderator: false,
       is_admin: false,
       mastofe_settings: nil,
@@ -2073,10 +2066,6 @@ def get_public_key_for_ap_id(ap_id) do
     end
   end
 
-  def ap_enabled?(%User{local: true}), do: true
-  def ap_enabled?(%User{ap_enabled: ap_enabled}), do: ap_enabled
-  def ap_enabled?(_), do: false
-
   @doc "Gets or fetch a user by uri or nickname."
   @spec get_or_fetch(String.t()) :: {:ok, User.t()} | {:error, String.t()}
   def get_or_fetch("http://" <> _host = uri), do: get_or_fetch_by_ap_id(uri)

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -1626,7 +1626,6 @@ defp object_to_user_data(data, additional) do
     %{
       ap_id: data["id"],
       uri: get_actor_url(data["url"]),
-      ap_enabled: true,
       banner: normalize_image(data["image"]),
       background: normalize_image(data["backgroundUrl"]),
       fields: fields,
@@ -1743,7 +1742,7 @@ def user_data_from_user_object(data, additional \\ []) do
     end
   end
 
-  def fetch_and_prepare_user_from_ap_id(ap_id, additional \\ []) do
+  defp fetch_and_prepare_user_from_ap_id(ap_id, additional) do
     with {:ok, data} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id),
          {:valid, {:ok, _, _}} <- {:valid, UserValidator.validate(data, [])},
          {:ok, data} <- user_data_from_user_object(data, additional) do
@@ -1857,9 +1856,6 @@ def enqueue_pin_fetches(_), do: nil
   def make_user_from_ap_id(ap_id, additional \\ []) do
     user = User.get_cached_by_ap_id(ap_id)
 
-    if user && !User.ap_enabled?(user) do
-      Transmogrifier.upgrade_user_from_ap_id(ap_id)
-    else
     with {:ok, data} <- fetch_and_prepare_user_from_ap_id(ap_id, additional) do
       user =
         if data.ap_id != ap_id do
@@ -1884,7 +1880,6 @@ def make_user_from_ap_id(ap_id, additional \\ []) do
       end
     end
   end
-  end
 
   def make_user_from_nickname(nickname) do
     with {:ok, %{"ap_id" => ap_id, "subject" => "acct:" <> acct}} when not is_nil(ap_id) <-

lib/pleroma/web/activity_pub/object_validators/add_remove_validator.ex

@@ -73,6 +73,7 @@ defp maybe_refetch_user(%User{featured_address: address} = user) when is_binary(
   end
 
   defp maybe_refetch_user(%User{ap_id: ap_id}) do
-    Pleroma.Web.ActivityPub.Transmogrifier.upgrade_user_from_ap_id(ap_id)
+    # Maybe it could use User.get_or_fetch_by_ap_id to avoid refreshing too often
+    User.fetch_by_ap_id(ap_id)
   end
 end

lib/pleroma/web/activity_pub/publisher.ex

@@ -219,7 +219,6 @@ def publish(%User{} = actor, %{data: %{"bcc" => bcc}} = activity)
 
     inboxes =
       recipients
-      |> Enum.filter(&User.ap_enabled?/1)
       |> Enum.map(fn actor -> actor.inbox end)
       |> Enum.filter(fn inbox -> should_federate?(inbox) end)
       |> Instances.filter_reachable()
@@ -261,7 +260,6 @@ def publish(%User{} = actor, %Activity{} = activity) do
     json = Jason.encode!(data)
 
     recipients(actor, activity)
-    |> Enum.filter(fn user -> User.ap_enabled?(user) end)
     |> Enum.map(fn %User{} = user ->
       determine_inbox(activity, user)
     end)

lib/pleroma/web/activity_pub/transmogrifier.ex

@@ -21,7 +21,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
   alias Pleroma.Web.ActivityPub.Visibility
   alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes
   alias Pleroma.Web.Federator
-  alias Pleroma.Workers.TransmogrifierWorker
 
   import Ecto.Query
 
@@ -1007,47 +1006,6 @@ defp strip_internal_tags(%{"tag" => tags} = object) do
 
   defp strip_internal_tags(object), do: object
 
-  def perform(:user_upgrade, user) do
-    # we pass a fake user so that the followers collection is stripped away
-    old_follower_address = User.ap_followers(%User{nickname: user.nickname})
-
-    from(
-      a in Activity,
-      where: ^old_follower_address in a.recipients,
-      update: [
-        set: [
-          recipients:
-            fragment(
-              "array_replace(?,?,?)",
-              a.recipients,
-              ^old_follower_address,
-              ^user.follower_address
-            )
-        ]
-      ]
-    )
-    |> Repo.update_all([])
-  end
-
-  def upgrade_user_from_ap_id(ap_id) do
-    with %User{local: false} = user <- User.get_cached_by_ap_id(ap_id),
-         {:ok, data} <- ActivityPub.fetch_and_prepare_user_from_ap_id(ap_id),
-         {:ok, user} <- update_user(user, data) do
-      ActivityPub.enqueue_pin_fetches(user)
-      TransmogrifierWorker.enqueue("user_upgrade", %{"user_id" => user.id})
-      {:ok, user}
-    else
-      %User{} = user -> {:ok, user}
-      e -> e
-    end
-  end
-
-  defp update_user(user, data) do
-    user
-    |> User.remote_user_changeset(data)
-    |> User.update_and_set_cache()
-  end
-
   def maybe_fix_user_url(%{"url" => url} = data) when is_map(url) do
     Map.put(data, "url", url["href"])
   end

lib/pleroma/web/federator.ex

@@ -6,7 +6,6 @@ defmodule Pleroma.Web.Federator do
   alias Pleroma.Activity
   alias Pleroma.Object.Containment
   alias Pleroma.User
-  alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.ActivityPub.Utils
   alias Pleroma.Web.Federator.Publisher
@@ -92,7 +91,7 @@ def perform(:incoming_ap_doc, params) do
 
     # NOTE: we use the actor ID to do the containment, this is fine because an
     # actor shouldn't be acting on objects outside their own AP server.
-    with {_, {:ok, _user}} <- {:actor, ap_enabled_actor(actor)},
+    with {_, {:ok, _user}} <- {:actor, User.get_or_fetch_by_ap_id(actor)},
          nil <- Activity.normalize(params["id"]),
          {_, :ok} <-
            {:correct_origin?, Containment.contain_origin_from_id(actor, params)},
@@ -122,14 +121,4 @@ def perform(:incoming_ap_doc, params) do
         {:error, e}
     end
   end
-
-  def ap_enabled_actor(id) do
-    user = User.get_cached_by_ap_id(id)
-
-    if User.ap_enabled?(user) do
-      {:ok, user}
-    else
-      ActivityPub.make_user_from_ap_id(id)
-    end
-  end
 end

lib/pleroma/workers/transmogrifier_worker.ex

@@ -1,15 +0,0 @@
-# Pleroma: A lightweight social networking server
-# Copyright © 2017-2021 Pleroma Authors <https://pleroma.social/>
-# SPDX-License-Identifier: AGPL-3.0-only
-
-defmodule Pleroma.Workers.TransmogrifierWorker do
-  alias Pleroma.User
-
-  use Pleroma.Workers.WorkerHelper, queue: "transmogrifier"
-
-  @impl Oban.Worker
-  def perform(%Job{args: %{"op" => "user_upgrade", "user_id" => user_id}}) do
-    user = User.get_cached_by_id(user_id)
-    Pleroma.Web.ActivityPub.Transmogrifier.perform(:user_upgrade, user)
-  end
-end

priv/repo/migrations/20241213000000_remove_user_ap_enabled.exs

@@ -0,0 +1,13 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2023 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Repo.Migrations.RemoveUserApEnabled do
+  use Ecto.Migration
+
+  def change do
+    alter table(:users) do
+      remove(:ap_enabled, :boolean, default: false, null: false)
+    end
+  end
+end

test/pleroma/user_test.exs

@@ -1694,7 +1694,6 @@ test "delete/1 purges a user when they wouldn't be fully deleted" do
         confirmation_token: "qqqq",
         domain_blocks: ["lain.com"],
         is_active: false,
-        ap_enabled: true,
         is_moderator: true,
         is_admin: true,
         mastofe_settings: %{"a" => "b"},
@@ -1734,7 +1733,6 @@ test "delete/1 purges a user when they wouldn't be fully deleted" do
              confirmation_token: nil,
              domain_blocks: [],
              is_active: false,
-             ap_enabled: false,
              is_moderator: false,
              is_admin: false,
              mastofe_settings: nil,
@@ -2217,8 +2215,7 @@ test "updates the counters normally on following/getting a follow when disabled"
         insert(:user,
           local: false,
           follower_address: "http://remote.org/users/masto_closed/followers",
-          following_address: "http://remote.org/users/masto_closed/following",
+          following_address: "http://remote.org/users/masto_closed/following"
-          ap_enabled: true
         )
 
       assert other_user.following_count == 0
@@ -2239,8 +2236,7 @@ test "synchronizes the counters with the remote instance for the followed when e
         insert(:user,
           local: false,
           follower_address: "http://remote.org/users/masto_closed/followers",
-          following_address: "http://remote.org/users/masto_closed/following",
+          following_address: "http://remote.org/users/masto_closed/following"
-          ap_enabled: true
         )
 
       assert other_user.following_count == 0
@@ -2261,8 +2257,7 @@ test "synchronizes the counters with the remote instance for the follower when e
         insert(:user,
           local: false,
           follower_address: "http://remote.org/users/masto_closed/followers",
-          following_address: "http://remote.org/users/masto_closed/following",
+          following_address: "http://remote.org/users/masto_closed/following"
-          ap_enabled: true
         )
 
       assert other_user.following_count == 0

test/pleroma/web/activity_pub/activity_pub_controller_test.exs

@@ -579,7 +579,6 @@ test "it inserts an incoming activity into the database" <>
       user =
         insert(:user,
           ap_id: "https://mastodon.example.org/users/raymoo",
-          ap_enabled: true,
           local: false,
           last_refreshed_at: nil
         )

test/pleroma/web/activity_pub/activity_pub_test.exs

@@ -178,7 +178,6 @@ test "it returns a user" do
       {:ok, user} = ActivityPub.make_user_from_ap_id(user_id)
       assert user.ap_id == user_id
       assert user.nickname == "admin@mastodon.example.org"
-      assert user.ap_enabled
       assert user.follower_address == "http://mastodon.example.org/users/admin/followers"
     end
 

test/pleroma/web/activity_pub/publisher_test.exs

@@ -306,15 +306,13 @@ test "publish to url with with different ports" do
       follower =
         insert(:user, %{
           local: false,
-          inbox: "https://domain.com/users/nick1/inbox",
+          inbox: "https://domain.com/users/nick1/inbox"
-          ap_enabled: true
         })
 
       another_follower =
         insert(:user, %{
           local: false,
-          inbox: "https://rejected.com/users/nick2/inbox",
+          inbox: "https://rejected.com/users/nick2/inbox"
-          ap_enabled: true
         })
 
       actor =
@@ -386,8 +384,7 @@ test "publish to url with with different ports" do
       follower =
         insert(:user, %{
           local: false,
-          inbox: "https://domain.com/users/nick1/inbox",
+          inbox: "https://domain.com/users/nick1/inbox"
-          ap_enabled: true
         })
 
       actor =
@@ -425,8 +422,7 @@ test "publish to url with with different ports" do
       follower =
         insert(:user, %{
           local: false,
-          inbox: "https://domain.com/users/nick1/inbox",
+          inbox: "https://domain.com/users/nick1/inbox"
-          ap_enabled: true
         })
 
       actor = insert(:user, follower_address: follower.ap_id)
@@ -461,15 +457,13 @@ test "publish to url with with different ports" do
       fetcher =
         insert(:user,
           local: false,
-          inbox: "https://domain.com/users/nick1/inbox",
+          inbox: "https://domain.com/users/nick1/inbox"
-          ap_enabled: true
         )
 
       another_fetcher =
         insert(:user,
           local: false,
-          inbox: "https://domain2.com/users/nick1/inbox",
+          inbox: "https://domain2.com/users/nick1/inbox"
-          ap_enabled: true
         )
 
       actor = insert(:user)

test/pleroma/web/activity_pub/transmogrifier_test.exs

@@ -8,7 +8,6 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
   @moduletag :mocked
   alias Pleroma.Activity
   alias Pleroma.Object
-  alias Pleroma.Tests.ObanHelpers
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.ActivityPub.Utils
@@ -348,69 +347,6 @@ test "Updates of Notes are handled" do
     end
   end
 
-  describe "user upgrade" do
-    test "it upgrades a user to activitypub" do
-      user =
-        insert(:user, %{
-          nickname: "rye@niu.moe",
-          local: false,
-          ap_id: "https://niu.moe/users/rye",
-          follower_address: User.ap_followers(%User{nickname: "rye@niu.moe"})
-        })
-
-      user_two = insert(:user)
-      Pleroma.FollowingRelationship.follow(user_two, user, :follow_accept)
-
-      {:ok, activity} = CommonAPI.post(user, %{status: "test"})
-      {:ok, unrelated_activity} = CommonAPI.post(user_two, %{status: "test"})
-      assert "http://localhost:4001/users/rye@niu.moe/followers" in activity.recipients
-
-      user = User.get_cached_by_id(user.id)
-      assert user.note_count == 1
-
-      {:ok, user} = Transmogrifier.upgrade_user_from_ap_id("https://niu.moe/users/rye")
-      ObanHelpers.perform_all()
-
-      assert user.ap_enabled
-      assert user.note_count == 1
-      assert user.follower_address == "https://niu.moe/users/rye/followers"
-      assert user.following_address == "https://niu.moe/users/rye/following"
-
-      user = User.get_cached_by_id(user.id)
-      assert user.note_count == 1
-
-      activity = Activity.get_by_id(activity.id)
-      assert user.follower_address in activity.recipients
-
-      assert %{
-               "url" => [
-                 %{
-                   "href" =>
-                     "https://cdn.niu.moe/accounts/avatars/000/033/323/original/fd7f8ae0b3ffedc9.jpeg"
-                 }
-               ]
-             } = user.avatar
-
-      assert %{
-               "url" => [
-                 %{
-                   "href" =>
-                     "https://cdn.niu.moe/accounts/headers/000/033/323/original/850b3448fa5fd477.png"
-                 }
-               ]
-             } = user.banner
-
-      refute "..." in activity.recipients
-
-      unrelated_activity = Activity.get_by_id(unrelated_activity.id)
-      refute user.follower_address in unrelated_activity.recipients
-
-      user_two = User.get_cached_by_id(user_two.id)
-      assert User.following?(user_two, user)
-      refute "..." in User.following(user_two)
-    end
-  end
-
   describe "actor rewriting" do
     test "it fixes the actor URL property to be a proper URI" do
       data = %{

test/pleroma/web/common_api_test.exs

@@ -1129,7 +1129,7 @@ test "removes a pending follow for a local user" do
 
     test "cancels a pending follow for a remote user" do
       follower = insert(:user)
-      followed = insert(:user, is_locked: true, local: false, ap_enabled: true)
+      followed = insert(:user, is_locked: true, local: false)
 
       assert {:ok, follower, followed, %{id: _activity_id, data: %{"state" => "pending"}}} =
                CommonAPI.follow(follower, followed)

test/pleroma/web/federator_test.exs

@@ -79,16 +79,14 @@ test "it federates only to reachable instances via AP" do
         local: false,
         nickname: "nick1@domain.com",
         ap_id: "https://domain.com/users/nick1",
-        inbox: inbox1,
+        inbox: inbox1
-        ap_enabled: true
       })
 
       insert(:user, %{
         local: false,
         nickname: "nick2@domain2.com",
         ap_id: "https://domain2.com/users/nick2",
-        inbox: inbox2,
+        inbox: inbox2
-        ap_enabled: true
       })
 
       dt = NaiveDateTime.utc_now()

test/support/factory.ex

@@ -62,8 +62,7 @@ def user_factory(attrs \\ %{}) do
       last_digest_emailed_at: NaiveDateTime.utc_now(),
       last_refreshed_at: NaiveDateTime.utc_now(),
       notification_settings: %Pleroma.User.NotificationSetting{},
-      multi_factor_authentication_settings: %Pleroma.MFA.Settings{},
+      multi_factor_authentication_settings: %Pleroma.MFA.Settings{}
-      ap_enabled: true
     }
 
     urls =