Merge pull request 'federation/in: fix inlined featured collections' (#949) from Oneric/akkoma:fix-inline-featured into develop

Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/949

lib/pleroma/collections/fetcher.ex

@@ -27,6 +27,10 @@ def fetch_collection(%{"type" => type} = page)
     partial_as_success(objects_from_collection(page))
   end
 
+  def fetch_collection(_) do
+    {:error, :invalid_type}
+  end
+
   defp partial_as_success({:partial, items}), do: {:ok, items}
   defp partial_as_success(res), do: res
 

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -1608,8 +1608,11 @@ defp object_to_user_data(data, additional) do
     invisible = data["invisible"] || false
     actor_type = data["type"] || "Person"
 
-    featured_address = data["featured"]
-    {:ok, pinned_objects} = fetch_and_prepare_featured_from_ap_id(featured_address)
+    {featured_address, pinned_objects} =
+      case process_featured_collection(data["featured"]) do
+        {:ok, featured_address, pinned_objects} -> {featured_address, pinned_objects}
+        _ -> {nil, %{}}
+      end
 
     # first, check that the owner is correct
     signing_key =
@@ -1808,57 +1811,35 @@ def maybe_handle_clashing_nickname(data) do
     end
   end
 
-  def pin_data_from_featured_collection(%{
-        "type" => "OrderedCollection",
-        "first" => first
-      }) do
-    with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(first) do
-      page
-      |> Map.get("orderedItems")
-      |> Map.new(fn %{"id" => object_ap_id} -> {object_ap_id, NaiveDateTime.utc_now()} end)
-    else
-      e ->
-        Logger.error("Could not decode featured collection at fetch #{first}, #{inspect(e)}")
-        %{}
-    end
-  end
+  def process_featured_collection(nil), do: {:ok, nil, %{}}
+  def process_featured_collection(""), do: {:ok, nil, %{}}
 
-  def pin_data_from_featured_collection(
-        %{
-          "type" => type
-        } = collection
-      )
-      when type in ["OrderedCollection", "Collection"] do
-    with {:ok, objects} <- Collections.Fetcher.fetch_collection(collection) do
-      # Items can either be a map _or_ a string
-      objects
-      |> Map.new(fn
-        ap_id when is_binary(ap_id) -> {ap_id, NaiveDateTime.utc_now()}
-        %{"id" => object_ap_id} -> {object_ap_id, NaiveDateTime.utc_now()}
-      end)
-    else
-      e ->
-        Logger.warning("Failed to fetch featured collection #{collection}, #{inspect(e)}")
-        %{}
-    end
-  end
+  def process_featured_collection(featured_collection) do
+    featured_address =
+      case get_ap_id(featured_collection) do
+        id when is_binary(id) -> id
+        _ -> nil
+      end
 
-  def pin_data_from_featured_collection(obj) do
-    Logger.error("Could not parse featured collection #{inspect(obj)}")
-    %{}
-  end
+    # TODO: allow passing item/page limit as function opt and use here
+    case Collections.Fetcher.fetch_collection(featured_collection) do
+      {:ok, items} ->
+        now = NaiveDateTime.utc_now()
+        dated_obj_ids = Map.new(items, fn obj -> {get_ap_id(obj), now} end)
+        {:ok, featured_address, dated_obj_ids}
 
-  def fetch_and_prepare_featured_from_ap_id(nil) do
-    {:ok, %{}}
-  end
+      error ->
+        Logger.error(
+          "Could not decode featured collection at fetch #{inspect(featured_collection)}: #{inspect(error)}"
+        )
 
-  def fetch_and_prepare_featured_from_ap_id(ap_id) do
-    with {:ok, data} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id) do
-      {:ok, pin_data_from_featured_collection(data)}
-    else
-      e ->
-        Logger.error("Could not decode featured collection at fetch #{ap_id}, #{inspect(e)}")
-        {:ok, %{}}
+        error =
+          case error do
+            {:error, e} -> e
+            e -> e
+          end
+
+        {:error, error}
     end
   end
 

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

@@ -14,6 +14,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.UserValidator do
   @behaviour Pleroma.Web.ActivityPub.ObjectValidator.Validating
 
   alias Pleroma.Object.Containment
+  alias Pleroma.Web.ActivityPub.Utils
 
   require Pleroma.Constants
 
@@ -62,6 +63,7 @@ defp validate_inbox(%{"id" => id, "inbox" => inbox}) do
   defp validate_inbox(_), do: {:error, "No inbox"}
 
   defp check_field_value(%{"id" => id} = _data, value) do
+    value = Utils.get_ap_id(value)
     Containment.same_origin(id, value)
   end
 

test/pleroma/web/activity_pub/activity_pub_test.exs

@@ -172,7 +172,7 @@ test "it excludes by the appropriate visibility" do
     end
   end
 
-  describe "building a user from his ap id" do
+  describe "building a user from AP id" do
     test "it returns a user" do
       user_id = "http://mastodon.example.org/users/admin"
       {:ok, user} = ActivityPub.make_user_from_ap_id(user_id)
@@ -274,19 +274,11 @@ test "works for actors with malformed attachment fields" do
       assert [] = user.fields
     end
 
-    test "fetches user featured collection" do
+    defp test_featured(inlined) do
       ap_id = "https://example.com/users/lain"
 
       featured_url = "https://example.com/users/lain/collections/featured"
 
-      user_data =
-        "test/fixtures/users_mock/user.json"
-        |> File.read!()
-        |> String.replace("{{nickname}}", "lain")
-        |> Jason.decode!()
-        |> Map.put("featured", featured_url)
-        |> Jason.encode!()
-
       object_id = Ecto.UUID.generate()
 
       featured_data =
@@ -296,6 +288,16 @@ test "fetches user featured collection" do
         |> String.replace("{{nickname}}", "lain")
         |> String.replace("{{object_id}}", object_id)
 
+      featured_ref = if inlined, do: Jason.decode!(featured_data), else: featured_url
+
+      user_data =
+        "test/fixtures/users_mock/user.json"
+        |> File.read!()
+        |> String.replace("{{nickname}}", "lain")
+        |> Jason.decode!()
+        |> Map.put("featured", featured_ref)
+        |> Jason.encode!()
+
       object_url = "https://example.com/objects/#{object_id}"
 
       object_data =
@@ -349,6 +351,14 @@ test "fetches user featured collection" do
 
       assert %{data: %{"id" => ^object_url}} = Object.get_by_ap_id(object_url)
     end
+
+    test "fetches user featured collection by bare id" do
+      test_featured(false)
+    end
+
+    test "fetches user featured collection when embedded" do
+      test_featured(true)
+    end
   end
 
   test "fetches user featured collection using the first property" do
@@ -385,7 +395,7 @@ test "fetches user featured collection using the first property" do
         }
     end)
 
-    {:ok, data} = ActivityPub.fetch_and_prepare_featured_from_ap_id(featured_url)
+    {:ok, ^featured_url, data} = ActivityPub.process_featured_collection(featured_url)
     assert Map.has_key?(data, "http://inserted")
   end
 
@@ -419,7 +429,7 @@ test "fetches user featured when it has string IDs" do
         }
     end)
 
-    {:ok, %{}} = ActivityPub.fetch_and_prepare_featured_from_ap_id(featured_url)
+    {:ok, ^featured_url, %{}} = ActivityPub.process_featured_collection(featured_url)
   end
 
   test "it fetches the appropriate tag-restricted posts" do
@@ -2661,9 +2671,9 @@ test "allow fetching of accounts with an empty string name field" do
     assert user.name == " "
   end
 
-  test "pin_data_from_featured_collection will ignore unsupported values" do
-    assert %{} ==
-             ActivityPub.pin_data_from_featured_collection(%{
+  test "process_featured_collection will ignore unsupported values" do
+    assert {:error, :invalid_type} ==
+             ActivityPub.process_featured_collection(%{
                "type" => "CollectionThatIsNotRealAndCannotHurtMe",
                "first" => "https://social.example/users/alice/collections/featured?page=true"
              })