diff --git a/CHANGELOG.md b/CHANGELOG.md index 42ed630fb..b0448b0d8 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index dfeab0410..3042d86f2 100644 --- a/lib/pleroma/user.ex +++ b/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,11 +1004,7 @@ 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 + {:ok, follower, followed} end @doc "A mass follow for local users. Respects blocks in both directions but does not create activities." @@ -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) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 9b28e64d9..494a27421 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/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,31 +1856,27 @@ 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 - User.get_cached_by_ap_id(data.ap_id) - else - user - end - - if user do - user - |> User.remote_user_changeset(data) - |> User.update_and_set_cache() - |> tap(fn _ -> enqueue_pin_fetches(data) end) + with {:ok, data} <- fetch_and_prepare_user_from_ap_id(ap_id, additional) do + user = + if data.ap_id != ap_id do + User.get_cached_by_ap_id(data.ap_id) else - maybe_handle_clashing_nickname(data) - - data - |> User.remote_user_changeset() - |> Repo.insert() - |> User.set_cache() - |> tap(fn _ -> enqueue_pin_fetches(data) end) + user end + + if user do + user + |> User.remote_user_changeset(data) + |> User.update_and_set_cache() + |> tap(fn _ -> enqueue_pin_fetches(data) end) + else + maybe_handle_clashing_nickname(data) + + data + |> User.remote_user_changeset() + |> Repo.insert() + |> User.set_cache() + |> tap(fn _ -> enqueue_pin_fetches(data) end) end end end diff --git a/lib/pleroma/web/activity_pub/object_validators/add_remove_validator.ex b/lib/pleroma/web/activity_pub/object_validators/add_remove_validator.ex index fc482c9c0..b2fa35831 100644 --- a/lib/pleroma/web/activity_pub/object_validators/add_remove_validator.ex +++ b/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 diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index 07f430805..056de24bc 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/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) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 5c4db39b9..251ba6540 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/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 diff --git a/lib/pleroma/web/federator.ex b/lib/pleroma/web/federator.ex index 3a00424c6..aaf89412f 100644 --- a/lib/pleroma/web/federator.ex +++ b/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 diff --git a/lib/pleroma/workers/transmogrifier_worker.ex b/lib/pleroma/workers/transmogrifier_worker.ex deleted file mode 100644 index b39c1ea62..000000000 --- a/lib/pleroma/workers/transmogrifier_worker.ex +++ /dev/null @@ -1,15 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2021 Pleroma Authors -# 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 diff --git a/priv/repo/migrations/20241213000000_remove_user_ap_enabled.exs b/priv/repo/migrations/20241213000000_remove_user_ap_enabled.exs new file mode 100644 index 000000000..0aea41324 --- /dev/null +++ b/priv/repo/migrations/20241213000000_remove_user_ap_enabled.exs @@ -0,0 +1,13 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2023 Pleroma Authors +# 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 diff --git a/test/pleroma/user_test.exs b/test/pleroma/user_test.exs index ac886aaf9..0d5c9faec 100644 --- a/test/pleroma/user_test.exs +++ b/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", - ap_enabled: true + following_address: "http://remote.org/users/masto_closed/following" ) 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", - ap_enabled: true + following_address: "http://remote.org/users/masto_closed/following" ) 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", - ap_enabled: true + following_address: "http://remote.org/users/masto_closed/following" ) assert other_user.following_count == 0 diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 5b3697244..4cc7f93f5 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/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 ) diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index c8f93f84d..7990b7ef5 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/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 diff --git a/test/pleroma/web/activity_pub/publisher_test.exs b/test/pleroma/web/activity_pub/publisher_test.exs index 5896568b8..b90422d61 100644 --- a/test/pleroma/web/activity_pub/publisher_test.exs +++ b/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", - ap_enabled: true + inbox: "https://domain.com/users/nick1/inbox" }) another_follower = insert(:user, %{ local: false, - inbox: "https://rejected.com/users/nick2/inbox", - ap_enabled: true + inbox: "https://rejected.com/users/nick2/inbox" }) 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", - ap_enabled: true + inbox: "https://domain.com/users/nick1/inbox" }) 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", - ap_enabled: true + inbox: "https://domain.com/users/nick1/inbox" }) 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", - ap_enabled: true + inbox: "https://domain.com/users/nick1/inbox" ) another_fetcher = insert(:user, local: false, - inbox: "https://domain2.com/users/nick1/inbox", - ap_enabled: true + inbox: "https://domain2.com/users/nick1/inbox" ) actor = insert(:user) diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index 1be69317c..94df25100 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/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 = %{ diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs index 379cf85b8..b32334389 100644 --- a/test/pleroma/web/common_api_test.exs +++ b/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) diff --git a/test/pleroma/web/federator_test.exs b/test/pleroma/web/federator_test.exs index d3cc239cf..c9a13632a 100644 --- a/test/pleroma/web/federator_test.exs +++ b/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, - ap_enabled: true + inbox: inbox1 }) insert(:user, %{ local: false, nickname: "nick2@domain2.com", ap_id: "https://domain2.com/users/nick2", - inbox: inbox2, - ap_enabled: true + inbox: inbox2 }) dt = NaiveDateTime.utc_now() diff --git a/test/support/factory.ex b/test/support/factory.ex index 54e5f91b7..7797fc9b8 100644 --- a/test/support/factory.ex +++ b/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{}, - ap_enabled: true + multi_factor_authentication_settings: %Pleroma.MFA.Settings{} } urls =