From 772c209914d5cbfd4f763edc266d0f1541f656f8 Mon Sep 17 00:00:00 2001 From: floatingghost Date: Sat, 27 Aug 2022 18:05:48 +0000 Subject: [PATCH] GTS: cherry-picks and collection usage (#186) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit https://git.pleroma.social/pleroma/pleroma/-/merge_requests/3725?commit_id=61254111e59f02118cad15de49d1e0704c07030e what is this, a yoink of a yoink? good times Co-authored-by: Hélène Co-authored-by: FloatingGhost Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/186 --- lib/pleroma/collections/fetcher.ex | 38 +++++++----- .../article_note_page_validator.ex | 19 ++---- .../object_validators/common_fixes.ex | 29 ++++++++- .../create_generic_validator.ex | 3 +- .../web/activity_pub/transmogrifier.ex | 26 +------- lib/pleroma/web/plugs/http_signature_plug.ex | 8 ++- .../collections/collections_fetcher_test.exs | 59 +++++++++++++++++-- .../activity_pub_controller_test.exs | 11 ++++ .../article_note_page_validator_test.exs | 13 ++++ .../transmogrifier/note_handling_test.exs | 1 - .../web/plugs/http_signature_plug_test.exs | 6 +- test/support/http_request_mock.ex | 9 +++ 12 files changed, 155 insertions(+), 67 deletions(-) diff --git a/lib/pleroma/collections/fetcher.ex b/lib/pleroma/collections/fetcher.ex index 0c81f0b56..ab69f4b84 100644 --- a/lib/pleroma/collections/fetcher.ex +++ b/lib/pleroma/collections/fetcher.ex @@ -11,10 +11,7 @@ defmodule Akkoma.Collections.Fetcher do alias Pleroma.Config require Logger - def fetch_collection_by_ap_id(ap_id) when is_binary(ap_id) do - fetch_collection(ap_id) - end - + @spec fetch_collection(String.t() | map()) :: {:ok, [Pleroma.Object.t()]} | {:error, any()} def fetch_collection(ap_id) when is_binary(ap_id) do with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id) do {:ok, objects_from_collection(page)} @@ -26,7 +23,7 @@ defmodule Akkoma.Collections.Fetcher do end def fetch_collection(%{"type" => type} = page) - when type in ["Collection", "OrderedCollection"] do + when type in ["Collection", "OrderedCollection", "CollectionPage", "OrderedCollectionPage"] do {:ok, objects_from_collection(page)} end @@ -38,12 +35,13 @@ defmodule Akkoma.Collections.Fetcher do when is_list(items) and type in ["Collection", "CollectionPage"], do: items - defp objects_from_collection(%{"type" => "OrderedCollection", "orderedItems" => items}) - when is_list(items), - do: items + defp objects_from_collection(%{"type" => type, "orderedItems" => items} = page) + when is_list(items) and type in ["OrderedCollection", "OrderedCollectionPage"], + do: maybe_next_page(page, items) - defp objects_from_collection(%{"type" => "Collection", "items" => items}) when is_list(items), - do: items + defp objects_from_collection(%{"type" => type, "items" => items} = page) + when is_list(items) and type in ["Collection", "CollectionPage"], + do: maybe_next_page(page, items) defp objects_from_collection(%{"type" => type, "first" => first}) when is_binary(first) and type in ["Collection", "OrderedCollection"] do @@ -55,17 +53,27 @@ defmodule Akkoma.Collections.Fetcher do fetch_page_items(id) end + defp objects_from_collection(_page), do: [] + defp fetch_page_items(id, items \\ []) do if Enum.count(items) >= Config.get([:activitypub, :max_collection_objects]) do items else - {:ok, page} = Fetcher.fetch_and_contain_remote_object_from_id(id) - objects = items_in_page(page) + with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(id) do + objects = items_in_page(page) - if Enum.count(objects) > 0 do - maybe_next_page(page, items ++ objects) + if Enum.count(objects) > 0 do + maybe_next_page(page, items ++ objects) + else + items + end else - items + {:error, "Object has been deleted"} -> + items + + {:error, error} -> + Logger.error("Could not fetch page #{id} - #{inspect(error)}") + {:error, error} end end end diff --git a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex index 28053ea3a..55323bc2e 100644 --- a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex @@ -6,7 +6,6 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do use Ecto.Schema alias Pleroma.User alias Pleroma.EctoType.ActivityPub.ObjectValidators - alias Pleroma.Object.Fetcher alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes alias Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations @@ -58,19 +57,10 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do defp fix_tag(%{"tag" => tag} = data) when is_map(tag), do: Map.put(data, "tag", [tag]) defp fix_tag(data), do: Map.drop(data, ["tag"]) - defp fix_replies(%{"replies" => %{"first" => %{"items" => replies}}} = data) - when is_list(replies), - do: Map.put(data, "replies", replies) - - defp fix_replies(%{"replies" => %{"items" => replies}} = data) when is_list(replies), - do: Map.put(data, "replies", replies) - - defp fix_replies(%{"replies" => replies} = data) when is_bitstring(replies), - do: Map.drop(data, ["replies"]) + defp fix_replies(%{"replies" => replies} = data) when is_list(replies), do: data defp fix_replies(%{"replies" => %{"first" => first}} = data) do - with {:ok, %{"orderedItems" => replies}} <- - Fetcher.fetch_and_contain_remote_object_from_id(first) do + with {:ok, replies} <- Akkoma.Collections.Fetcher.fetch_collection(first) do Map.put(data, "replies", replies) else {:error, _} -> @@ -79,7 +69,10 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do end end - defp fix_replies(data), do: data + defp fix_replies(%{"replies" => %{"items" => replies}} = data) when is_list(replies), + do: Map.put(data, "replies", replies) + + defp fix_replies(data), do: Map.delete(data, "replies") defp remote_mention_resolver( %{"id" => ap_id, "tag" => tags}, diff --git a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex b/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex index 779c8b622..6fa2bbb99 100644 --- a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex +++ b/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex @@ -7,8 +7,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do alias Pleroma.Object alias Pleroma.Object.Containment alias Pleroma.User - alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.ActivityPub.Utils + require Pleroma.Constants def cast_and_filter_recipients(message, field, follower_collection, field_fallback \\ []) do {:ok, data} = ObjectValidators.Recipients.cast(message[field] || field_fallback) @@ -32,7 +32,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do |> cast_and_filter_recipients("cc", follower_collection) |> cast_and_filter_recipients("bto", follower_collection) |> cast_and_filter_recipients("bcc", follower_collection) - |> Transmogrifier.fix_implicit_addressing(follower_collection) + |> fix_implicit_addressing(follower_collection) end def fix_activity_addressing(activity) do @@ -43,7 +43,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do |> cast_and_filter_recipients("cc", follower_collection) |> cast_and_filter_recipients("bto", follower_collection) |> cast_and_filter_recipients("bcc", follower_collection) - |> Transmogrifier.fix_implicit_addressing(follower_collection) + |> fix_implicit_addressing(follower_collection) end def fix_actor(data) do @@ -73,4 +73,27 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do Map.put(data, "to", to) end + + # if as:Public is addressed, then make sure the followers collection is also addressed + # so that the activities will be delivered to local users. + def fix_implicit_addressing(%{"to" => to, "cc" => cc} = object, followers_collection) do + recipients = to ++ cc + + if followers_collection not in recipients do + cond do + Pleroma.Constants.as_public() in cc -> + to = to ++ [followers_collection] + Map.put(object, "to", to) + + Pleroma.Constants.as_public() in to -> + cc = cc ++ [followers_collection] + Map.put(object, "cc", cc) + + true -> + object + end + else + object + end + end end diff --git a/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex b/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex index 803b5d5a1..d868c3915 100644 --- a/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex @@ -13,7 +13,6 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CreateGenericValidator do alias Pleroma.User alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes alias Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations - alias Pleroma.Web.ActivityPub.Transmogrifier import Ecto.Changeset @@ -67,7 +66,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CreateGenericValidator do |> CommonFixes.cast_and_filter_recipients("cc", follower_collection, object["cc"]) |> CommonFixes.cast_and_filter_recipients("bto", follower_collection, object["bto"]) |> CommonFixes.cast_and_filter_recipients("bcc", follower_collection, object["bcc"]) - |> Transmogrifier.fix_implicit_addressing(follower_collection) + |> CommonFixes.fix_implicit_addressing(follower_collection) end def fix(data, meta) do diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index d2077967c..8ec4b0fec 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -19,6 +19,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Visibility + alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes alias Pleroma.Web.Federator alias Pleroma.Workers.TransmogrifierWorker @@ -95,29 +96,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do |> Map.put("cc", final_cc) end - # if as:Public is addressed, then make sure the followers collection is also addressed - # so that the activities will be delivered to local users. - def fix_implicit_addressing(%{"to" => to, "cc" => cc} = object, followers_collection) do - recipients = to ++ cc - - if followers_collection not in recipients do - cond do - Pleroma.Constants.as_public() in cc -> - to = to ++ [followers_collection] - Map.put(object, "to", to) - - Pleroma.Constants.as_public() in to -> - cc = cc ++ [followers_collection] - Map.put(object, "cc", cc) - - true -> - object - end - else - object - end - end - def fix_addressing(object) do {:ok, %User{follower_address: follower_collection}} = object @@ -130,7 +108,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do |> fix_addressing_list("bto") |> fix_addressing_list("bcc") |> fix_explicit_addressing(follower_collection) - |> fix_implicit_addressing(follower_collection) + |> CommonFixes.fix_implicit_addressing(follower_collection) end def fix_actor(%{"attributedTo" => actor} = object) do diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex index cfee392c8..c906a4eec 100644 --- a/lib/pleroma/web/plugs/http_signature_plug.ex +++ b/lib/pleroma/web/plugs/http_signature_plug.ex @@ -27,11 +27,11 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do end end - def route_aliases(%{path_info: ["objects", id]}) do + def route_aliases(%{path_info: ["objects", id], query_string: query_string}) do ap_id = Router.Helpers.o_status_url(Pleroma.Web.Endpoint, :object, id) with %Activity{} = activity <- Activity.get_by_object_ap_id_with_object(ap_id) do - ["/notice/#{activity.id}"] + ["/notice/#{activity.id}", "/notice/#{activity.id}?#{query_string}"] else _ -> [] end @@ -64,7 +64,9 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do if has_signature_header?(conn) do # set (request-target) header to the appropriate value # we also replace the digest header with the one we computed - possible_paths = route_aliases(conn) ++ [conn.request_path] + possible_paths = + route_aliases(conn) ++ [conn.request_path, conn.request_path <> "?#{conn.query_string}"] + assign_valid_signature_on_route_aliases(conn, possible_paths) else Logger.debug("No signature header!") diff --git a/test/pleroma/collections/collections_fetcher_test.exs b/test/pleroma/collections/collections_fetcher_test.exs index b9f84f5c4..7a582a3d7 100644 --- a/test/pleroma/collections/collections_fetcher_test.exs +++ b/test/pleroma/collections/collections_fetcher_test.exs @@ -30,7 +30,7 @@ defmodule Akkoma.Collections.FetcherTest do } end) - {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id) + {:ok, objects} = Fetcher.fetch_collection(ap_id) assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects end @@ -53,7 +53,7 @@ defmodule Akkoma.Collections.FetcherTest do } end) - {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id) + {:ok, objects} = Fetcher.fetch_collection(ap_id) assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects end @@ -106,7 +106,7 @@ defmodule Akkoma.Collections.FetcherTest do } end) - {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id) + {:ok, objects} = Fetcher.fetch_collection(ap_id) assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects end @@ -161,7 +161,58 @@ defmodule Akkoma.Collections.FetcherTest do } end) - {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id) + {:ok, objects} = Fetcher.fetch_collection(ap_id) + assert [%{"type" => "Create"}] = objects + end + + test "it should stop fetching when we hit a 404" do + clear_config([:activitypub, :max_collection_objects], 1) + + unordered_collection = + "test/fixtures/collections/unordered_page_reference.json" + |> File.read!() + + first_page = + "test/fixtures/collections/unordered_page_first.json" + |> File.read!() + + ap_id = "https://example.com/collection/unordered_page_reference" + first_page_id = "https://example.com/collection/unordered_page_reference?page=1" + second_page_id = "https://example.com/collection/unordered_page_reference?page=2" + + Tesla.Mock.mock(fn + %{ + method: :get, + url: ^ap_id + } -> + %Tesla.Env{ + status: 200, + body: unordered_collection, + headers: [{"content-type", "application/activity+json"}] + } + + %{ + method: :get, + url: ^first_page_id + } -> + %Tesla.Env{ + status: 200, + body: first_page, + headers: [{"content-type", "application/activity+json"}] + } + + %{ + method: :get, + url: ^second_page_id + } -> + %Tesla.Env{ + status: 404, + body: nil, + headers: [{"content-type", "application/activity+json"}] + } + end) + + {:ok, objects} = Fetcher.fetch_collection(ap_id) assert [%{"type" => "Create"}] = objects end end 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 da5c87bd8..e209bb46b 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -782,6 +782,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do |> String.replace("{{status_id}}", status_id) status_url = "https://example.com/users/lain/statuses/#{status_id}" + replies_url = status_url <> "/replies?only_other_accounts=true&page=true" user = File.read!("test/fixtures/users_mock/user.json") @@ -820,6 +821,16 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do |> String.replace("{{nickname}}", "lain"), headers: [{"content-type", "application/activity+json"}] } + + %{ + method: :get, + url: ^replies_url + } -> + %Tesla.Env{ + status: 404, + body: "", + headers: [{"content-type", "application/activity+json"}] + } end) data = %{ diff --git a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs index 1d73d6765..7c8e5a4e1 100644 --- a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs +++ b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs @@ -146,4 +146,17 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidatorTest "@akkoma_user" end end + + test "a Note without replies/first/items validates" do + insert(:user, ap_id: "https://mastodon.social/users/emelie") + + note = + "test/fixtures/tesla_mock/status.emelie.json" + |> File.read!() + |> Jason.decode!() + |> pop_in(["replies", "first", "items"]) + |> elem(1) + + %{valid?: true} = ArticleNotePageValidator.cast_and_validate(note) + end end diff --git a/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs index 24df5ea61..002042802 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs @@ -380,7 +380,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.NoteHandlingTest do clear_config([:instance, :federation_incoming_replies_max_depth], 10) {:ok, activity} = Transmogrifier.handle_incoming(data) - object = Object.normalize(activity.data["object"]) assert object.data["replies"] == items diff --git a/test/pleroma/web/plugs/http_signature_plug_test.exs b/test/pleroma/web/plugs/http_signature_plug_test.exs index 02e8b3092..8ce956510 100644 --- a/test/pleroma/web/plugs/http_signature_plug_test.exs +++ b/test/pleroma/web/plugs/http_signature_plug_test.exs @@ -86,10 +86,12 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do test "aliases redirected /object endpoints", _ do obj = insert(:note) act = insert(:note_activity, note: obj) - params = %{"actor" => "http://mastodon.example.org/users/admin"} + params = %{"actor" => "someparam"} path = URI.parse(obj.data["id"]).path conn = build_conn(:get, path, params) - assert ["/notice/#{act.id}"] == HTTPSignaturePlug.route_aliases(conn) + + assert ["/notice/#{act.id}", "/notice/#{act.id}?actor=someparam"] == + HTTPSignaturePlug.route_aliases(conn) end end end diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index 476e0ce04..ab44c489b 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -407,6 +407,15 @@ defmodule HttpRequestMock do }} end + def get( + "http://mastodon.example.org/users/admin/statuses/99512778738411822/replies?min_id=99512778738411824&page=true", + _, + _, + _ + ) do + {:ok, %Tesla.Env{status: 404, body: ""}} + end + def get("http://mastodon.example.org/users/relay", _, _, [ {"accept", "application/activity+json"} ]) do -- 2.45.2