Normalise public adressing to fix federation

Due to JSON LD compacting the full address of public scope may also occur in shorter forms and the spec requires us to treat them all equivalently. To save us the pain internally, normalise all inbound data to just one form. See note at: https://www.w3.org/TR/activitypub/#public-addressing This needs to happen very early, even before the other addressing fixes else an earlier validator will reject the object. This in turn required to move the list-tpye normalisation earlier as well, but since I was unsure about putting empty lists into the data when no such filed existed before, I excluded this case and thus the later fixing had to be kept as well. Fixes: #670
2024-01-23 20:10:01 +01:00 · 2024-01-23 20:10:01 +01:00 · 505e498b1a
parent 086d6100e1
commit 505e498b1a
3 changed files with 162 additions and 80 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -24,6 +24,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Issue where a bad inbox URL could break federation
 - Issue where hashtag rel values would be scrubbed
 - Issue where short domains listed in `transparency_obfuscate_domains` were not actually obfuscated
+- Scope misdetection of remote posts resulting from not recognising
+  JSON-LD-compacted forms of public scope; affected e.g. federation with bovine

 ## 2023.08

--- a/lib/pleroma/web/activity_pub/transmogrifier.ex
+++ b/lib/pleroma/web/activity_pub/transmogrifier.ex
@ -58,21 +58,48 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do

  def fix_summary(object), do: Map.put(object, "summary", "")

-  def fix_addressing_list(map, field) do
-    addrs = map[field]
-
+  defp fix_addressing_list(addrs) do
    cond do
-      is_list(addrs) ->
-        Map.put(map, field, Enum.filter(addrs, &is_binary/1))
-
-      is_binary(addrs) ->
-        Map.put(map, field, [addrs])
-
-      true ->
-        Map.put(map, field, [])
+      is_list(addrs) -> Enum.filter(addrs, &is_binary/1)
+      is_binary(addrs) -> [addrs]
+      true -> []
    end
  end

+  # Due to JSON-LD simply "Public" and "as:Public" are equivalent to the full URI
+  # but to simplify later checks we only want to deal with one reperesentation internally
+  defp normalise_addressing_public_list(map, all_fields)
+
+  defp normalise_addressing_public_list(%{} = map, [field | fields]) do
+    full_uri = "https://www.w3.org/ns/activitystreams#Public"
+
+    map =
+      if map[field] != nil do
+        new_fval =
+          map[field]
+          |> fix_addressing_list()
+          |> Enum.map(fn
+            "Public" -> full_uri
+            "as:Public" -> full_uri
+            x -> x
+          end)
+
+        Map.put(map, field, new_fval)
+      else
+        map
+      end
+
+    normalise_addressing_public_list(map, fields)
+  end
+
+  defp normalise_addressing_public_list(map, _) do
+    map
+  end
+
+  defp normalise_addressing_public(map) do
+    normalise_addressing_public_list(map, ["to", "cc", "bto", "bcc"])
+  end
+
  # if directMessage flag is set to true, leave the addressing alone
  def fix_explicit_addressing(%{"directMessage" => true} = object, _follower_collection),
    do: object
@ -96,6 +123,10 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    |> Map.put("cc", final_cc)
  end

+  def fix_addressing_list_key(map, field) do
+    Map.put(map, field, fix_addressing_list(map[field]))
+  end
+
  def fix_addressing(object) do
    {:ok, %User{follower_address: follower_collection}} =
      object
@ -103,10 +134,10 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
      |> User.get_or_fetch_by_ap_id()

    object
-    |> fix_addressing_list("to")
-    |> fix_addressing_list("cc")
-    |> fix_addressing_list("bto")
-    |> fix_addressing_list("bcc")
+    |> fix_addressing_list_key("to")
+    |> fix_addressing_list_key("cc")
+    |> fix_addressing_list_key("bto")
+    |> fix_addressing_list_key("bcc")
    |> fix_explicit_addressing(follower_collection)
    |> CommonFixes.fix_implicit_addressing(follower_collection)
  end
@ -384,11 +415,28 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end)
  end

-  def handle_incoming(data, options \\ [])
+  def handle_incoming(data, options \\ []) do
+    data = normalise_addressing_public(data)
+
+    data =
+      if data["object"] != nil do
+        object = normalise_addressing_public(data["object"])
+        Map.put(data, "object", object)
+      else
+        data
+      end
+
+    handle_incoming_normalised(data, options)
+  end
+
+  defp handle_incoming_normalised(data, options \\ [])

  # Flag objects are placed ahead of the ID check because Mastodon 2.8 and earlier send them
  # with nil ID.
-  def handle_incoming(%{"type" => "Flag", "object" => objects, "actor" => actor} = data, _options) do
+  defp handle_incoming_normalised(
+         %{"type" => "Flag", "object" => objects, "actor" => actor} = data,
+         _options
+       ) do
    with context <- data["context"] || Utils.generate_context_id(),
         content <- data["content"] || "",
         %User{} = actor <- User.get_cached_by_ap_id(actor),
@ -409,20 +457,21 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
  end

  # disallow objects with bogus IDs
-  def handle_incoming(%{"id" => nil}, _options), do: :error
-  def handle_incoming(%{"id" => ""}, _options), do: :error
+  defp handle_incoming_normalised(%{"id" => nil}, _options), do: :error
+  defp handle_incoming_normalised(%{"id" => ""}, _options), do: :error
  # length of https:// = 8, should validate better, but good enough for now.
-  def handle_incoming(%{"id" => id}, _options) when is_binary(id) and byte_size(id) < 8,
-    do: :error
+  defp handle_incoming_normalised(%{"id" => id}, _options)
+       when is_binary(id) and byte_size(id) < 8,
+       do: :error

-  @doc "Rewrite misskey likes into EmojiReacts"
-  def handle_incoming(
-        %{
-          "type" => "Like",
-          "content" => reaction
-        } = data,
-        options
-      ) do
+  # Rewrite misskey likes into EmojiReacts
+  defp handle_incoming_normalised(
+         %{
+           "type" => "Like",
+           "content" => reaction
+         } = data,
+         options
+       ) do
    if Pleroma.Emoji.is_unicode_emoji?(reaction) || Pleroma.Emoji.matches_shortcode?(reaction) do
      data
      |> Map.put("type", "EmojiReact")
@ -434,11 +483,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end
  end

-  def handle_incoming(
-        %{"type" => "Create", "object" => %{"type" => objtype, "id" => obj_id}} = data,
-        options
-      )
-      when objtype in ~w{Question Answer Audio Video Event Article Note Page} do
+  defp handle_incoming_normalised(
+         %{"type" => "Create", "object" => %{"type" => objtype, "id" => obj_id}} = data,
+         options
+       )
+       when objtype in ~w{Question Answer Audio Video Event Article Note Page} do
    fetch_options = Keyword.put(options, :depth, (options[:depth] || 0) + 1)

    object =
@ -470,8 +519,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end
  end

-  def handle_incoming(%{"type" => type} = data, _options)
-      when type in ~w{Like EmojiReact Announce Add Remove} do
+  defp handle_incoming_normalised(%{"type" => type} = data, _options)
+       when type in ~w{Like EmojiReact Announce Add Remove} do
    with :ok <- ObjectValidator.fetch_actor_and_object(data),
         {:ok, activity, _meta} <- Pipeline.common_pipeline(data, local: false) do
      {:ok, activity}
@ -481,11 +530,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end
  end

-  def handle_incoming(
-        %{"type" => type} = data,
-        _options
-      )
-      when type in ~w{Update Block Follow Accept Reject} do
+  defp handle_incoming_normalised(
+         %{"type" => type} = data,
+         _options
+       )
+       when type in ~w{Update Block Follow Accept Reject} do
    with {:ok, %User{}} <- ObjectValidator.fetch_actor(data),
         {:ok, activity, _} <-
           Pipeline.common_pipeline(data, local: false) do
@ -493,10 +542,10 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end
  end

-  def handle_incoming(
-        %{"type" => "Delete"} = data,
-        _options
-      ) do
+  defp handle_incoming_normalised(
+         %{"type" => "Delete"} = data,
+         _options
+       ) do
    with {:ok, activity, _} <-
           Pipeline.common_pipeline(data, local: false) do
      {:ok, activity}
@ -516,15 +565,15 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end
  end

-  def handle_incoming(
-        %{
-          "type" => "Undo",
-          "object" => %{"type" => "Follow", "object" => followed},
-          "actor" => follower,
-          "id" => id
-        } = _data,
-        _options
-      ) do
+  defp handle_incoming_normalised(
+         %{
+           "type" => "Undo",
+           "object" => %{"type" => "Follow", "object" => followed},
+           "actor" => follower,
+           "id" => id
+         } = _data,
+         _options
+       ) do
    with %User{local: true} = followed <- User.get_cached_by_ap_id(followed),
         {:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(follower),
         {:ok, activity} <- ActivityPub.unfollow(follower, followed, id, false) do
@ -535,28 +584,28 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end
  end

-  def handle_incoming(
-        %{
-          "type" => "Undo",
-          "object" => %{"type" => type}
-        } = data,
-        _options
-      )
-      when type in ["Like", "EmojiReact", "Announce", "Block"] do
+  defp handle_incoming_normalised(
+         %{
+           "type" => "Undo",
+           "object" => %{"type" => type}
+         } = data,
+         _options
+       )
+       when type in ["Like", "EmojiReact", "Announce", "Block"] do
    with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do
      {:ok, activity}
    end
  end

  # For Undos that don't have the complete object attached, try to find it in our database.
-  def handle_incoming(
-        %{
-          "type" => "Undo",
-          "object" => object
-        } = activity,
-        options
-      )
-      when is_binary(object) do
+  defp handle_incoming_normalised(
+         %{
+           "type" => "Undo",
+           "object" => object
+         } = activity,
+         options
+       )
+       when is_binary(object) do
    with %Activity{data: data} <- Activity.get_by_ap_id(object) do
      activity
      |> Map.put("object", data)
@ -566,15 +615,15 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end
  end

-  def handle_incoming(
-        %{
-          "type" => "Move",
-          "actor" => origin_actor,
-          "object" => origin_actor,
-          "target" => target_actor
-        },
-        _options
-      ) do
+  defp handle_incoming_normalised(
+         %{
+           "type" => "Move",
+           "actor" => origin_actor,
+           "object" => origin_actor,
+           "target" => target_actor
+         },
+         _options
+       ) do
    with %User{} = origin_user <- User.get_cached_by_ap_id(origin_actor),
         {:ok, %User{} = target_user} <- User.get_or_fetch_by_ap_id(target_actor),
         true <- origin_actor in target_user.also_known_as do
@ -584,7 +633,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
    end
  end

-  def handle_incoming(_, _), do: :error
+  defp handle_incoming_normalised(_, _), do: :error

  @spec get_obj_helper(String.t(), Keyword.t()) :: {:ok, Object.t()} | nil
  def get_obj_helper(id, options \\ []) do
--- a/test/pleroma/web/federator_test.exs
+++ b/test/pleroma/web/federator_test.exs
@ -137,6 +137,37 @@ defmodule Pleroma.Web.FederatorTest do
      assert {:error, :already_present} = ObanHelpers.perform(job)
    end

+    test "successfully normalises public scope descriptors" do
+      params = %{
+        "@context" => "https://www.w3.org/ns/activitystreams",
+        "actor" => "http://mastodon.example.org/users/admin",
+        "type" => "Create",
+        "id" => "http://mastodon.example.org/users/admin/activities/1",
+        "object" => %{
+          "type" => "Note",
+          "content" => "hi world!",
+          "id" => "http://mastodon.example.org/users/admin/objects/1",
+          "attributedTo" => "http://mastodon.example.org/users/admin",
+          "to" => ["Public"]
+        },
+        "to" => ["as:Public"]
+      }
+
+      assert {:ok, job} = Federator.incoming_ap_doc(params)
+      assert {:ok, activity} = ObanHelpers.perform(job)
+      assert activity.data["to"] == ["https://www.w3.org/ns/activitystreams#Public"]
+
+      object =
+        from(
+          object in Pleroma.Object,
+          where: fragment("(?)->>'id' = ?", object.data, ^activity.data["object"]),
+          limit: 1
+        )
+        |> Repo.one()
+
+      assert object.data["to"] == ["https://www.w3.org/ns/activitystreams#Public"]
+    end
+
    test "rejects incoming AP docs with incorrect origin" do
      params = %{
        "@context" => "https://www.w3.org/ns/activitystreams",