From 8d6cc6cb65d8216d9f8d75e274e23fc78b75639c Mon Sep 17 00:00:00 2001 From: floatingghost Date: Fri, 2 Dec 2022 11:12:37 +0000 Subject: [PATCH] Resolve follow activity from accept/reject without ID (#328) Co-authored-by: FloatingGhost Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/328 --- lib/pleroma/activity.ex | 8 ++ lib/pleroma/web/activity_pub/activity_pub.ex | 2 +- .../accept_reject_validator.ex | 32 +++++++- .../transmogrifier/reject_handling_test.exs | 76 +++++++++++++++++++ test/support/factory.ex | 7 +- 5 files changed, 120 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index b01a838d8..c5b514742 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -367,6 +367,14 @@ defmodule Pleroma.Activity do |> Repo.all() end + def follow_activity(%User{ap_id: ap_id}, %User{ap_id: followed_ap_id}) do + Queries.by_type("Follow") + |> where([a], a.actor == ^ap_id) + |> where([a], fragment("?->>'object' = ?", a.data, ^followed_ap_id)) + |> where([a], fragment("?->>'state'", a.data) in ["pending", "accept"]) + |> Repo.one() + end + def restrict_deactivated_users(query) do query |> join( diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index db5dbc815..a4f1c7041 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -105,7 +105,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do end end - @unpersisted_activity_types ~w[Undo Delete Remove] + @unpersisted_activity_types ~w[Undo Delete Remove Accept Reject] @impl true def persist(%{"type" => type} = object, [local: false] = meta) when type in @unpersisted_activity_types do diff --git a/lib/pleroma/web/activity_pub/object_validators/accept_reject_validator.ex b/lib/pleroma/web/activity_pub/object_validators/accept_reject_validator.ex index 7c3c8d0fa..847d0be62 100644 --- a/lib/pleroma/web/activity_pub/object_validators/accept_reject_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/accept_reject_validator.ex @@ -6,6 +6,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AcceptRejectValidator do use Ecto.Schema alias Pleroma.Activity + alias Pleroma.Object + alias Pleroma.User import Ecto.Changeset import Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations @@ -29,7 +31,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AcceptRejectValidator do defp validate_data(cng) do cng - |> validate_required([:id, :type, :actor, :to, :cc, :object]) + |> validate_required([:type, :actor, :to, :cc, :object]) |> validate_inclusion(:type, ["Accept", "Reject"]) |> validate_actor_presence() |> validate_object_presence(allowed_types: ["Follow"]) @@ -38,6 +40,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AcceptRejectValidator do def cast_and_validate(data) do data + |> maybe_fetch_object() |> cast_data |> validate_data end @@ -53,4 +56,31 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AcceptRejectValidator do |> add_error(:actor, "can't accept or reject the given activity") end end + + defp maybe_fetch_object(%{"object" => %{} = object} = activity) do + # If we don't have an ID, we may have to fetch the object + if Map.has_key?(object, "id") do + # Do nothing + activity + else + Map.put(activity, "object", fetch_transient_object(object)) + end + end + + defp maybe_fetch_object(activity), do: activity + + defp fetch_transient_object( + %{"actor" => actor, "object" => target, "type" => "Follow"} = object + ) do + with %User{} = actor <- User.get_cached_by_ap_id(actor), + %User{local: true} = target <- User.get_cached_by_ap_id(target), + %Activity{} = activity <- Activity.follow_activity(actor, target) do + activity.data + else + _e -> + object + end + end + + defp fetch_transient_object(_), do: {:error, "not a supported transient object"} end diff --git a/test/pleroma/web/activity_pub/transmogrifier/reject_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/reject_handling_test.exs index 355e664d4..52a61f310 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/reject_handling_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier/reject_handling_test.exs @@ -8,6 +8,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.RejectHandlingTest do alias Pleroma.Activity alias Pleroma.User alias Pleroma.Web.ActivityPub.Transmogrifier + alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.CommonAPI import Pleroma.Factory @@ -53,6 +54,81 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.RejectHandlingTest do assert User.following?(follower, followed) == false end + describe "when accept/reject references a transient activity" do + test "it handles accept activities that do not contain an ID key" do + follower = insert(:user) + followed = insert(:user, is_locked: true) + + pending_follow = + insert(:follow_activity, follower: follower, followed: followed, state: "pending") + + refute User.following?(follower, followed) + + without_id = Map.delete(pending_follow.data, "id") + + reject_data = + File.read!("test/fixtures/mastodon-reject-activity.json") + |> Jason.decode!() + |> Map.put("actor", followed.ap_id) + |> Map.delete("id") + |> Map.put("object", without_id) + + {:ok, %Activity{data: _}} = Transmogrifier.handle_incoming(reject_data) + + follower = User.get_cached_by_id(follower.id) + + refute User.following?(follower, followed) + assert Utils.fetch_latest_follow(follower, followed).data["state"] == "reject" + end + + test "it handles reject activities that do not contain an ID key" do + follower = insert(:user) + followed = insert(:user) + {:ok, follower, followed} = User.follow(follower, followed) + {:ok, _, _, follow_activity} = CommonAPI.follow(follower, followed) + assert Utils.fetch_latest_follow(follower, followed).data["state"] == "accept" + assert User.following?(follower, followed) + + without_id = Map.delete(follow_activity.data, "id") + + reject_data = + File.read!("test/fixtures/mastodon-reject-activity.json") + |> Jason.decode!() + |> Map.put("actor", followed.ap_id) + |> Map.delete("id") + |> Map.put("object", without_id) + + {:ok, %Activity{data: _}} = Transmogrifier.handle_incoming(reject_data) + + follower = User.get_cached_by_id(follower.id) + + refute User.following?(follower, followed) + assert Utils.fetch_latest_follow(follower, followed).data["state"] == "reject" + end + + test "it does not accept follows that are not in pending or accepted" do + follower = insert(:user) + followed = insert(:user, is_locked: true) + + rejected_follow = + insert(:follow_activity, follower: follower, followed: followed, state: "reject") + + refute User.following?(follower, followed) + + without_id = Map.delete(rejected_follow.data, "id") + + accept_data = + File.read!("test/fixtures/mastodon-accept-activity.json") + |> Jason.decode!() + |> Map.put("actor", followed.ap_id) + |> Map.put("object", without_id) + + {:error, _} = Transmogrifier.handle_incoming(accept_data) + + refute User.following?(follower, followed) + end + end + test "it rejects activities without a valid ID" do user = insert(:user) diff --git a/test/support/factory.ex b/test/support/factory.ex index 904987aaf..3e426c565 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -452,15 +452,16 @@ defmodule Pleroma.Factory do } end - def follow_activity_factory do - follower = insert(:user) - followed = insert(:user) + def follow_activity_factory(attrs \\ %{}) do + follower = attrs[:follower] || insert(:user) + followed = attrs[:followed] || insert(:user) data = %{ "id" => Pleroma.Web.ActivityPub.Utils.generate_activity_id(), "actor" => follower.ap_id, "type" => "Follow", "object" => followed.ap_id, + "state" => attrs[:state] || "pending", "published_at" => DateTime.utc_now() |> DateTime.to_iso8601() }