Merge remote-tracking branch 'oneric/fedfix-public-ld' into develop

CHANGELOG.md

@@ -33,6 +33,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - /api/pleroma/notification\_settings was rejecting body parameters;
   this also broke changing this setting via akkoma-fe
 - Issue leading to Mastodon bot accounts being rejected
+- Scope misdetection of remote posts resulting from not recognising
+  JSON-LD-compacted forms of public scope; affected e.g. federation with bovine
 
 ## Removed
 - ActivityPub Client-To-Server write API endpoints have been disabled;

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
+  defp fix_addressing_list(addrs) do
-    addrs = map[field]
-
     cond do
-      is_list(addrs) ->
+      is_list(addrs) -> Enum.filter(addrs, &is_binary/1)
-        Map.put(map, field, Enum.filter(addrs, &is_binary/1))
+      is_binary(addrs) -> [addrs]
-
+      true -> []
-      is_binary(addrs) ->
-        Map.put(map, field, [addrs])
-
-      true ->
-        Map.put(map, field, [])
     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 = Pleroma.Constants.as_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_key("to")
-    |> fix_addressing_list("cc")
+    |> fix_addressing_list_key("cc")
-    |> fix_addressing_list("bto")
+    |> fix_addressing_list_key("bto")
-    |> fix_addressing_list("bcc")
+    |> fix_addressing_list_key("bcc")
     |> fix_explicit_addressing(follower_collection)
     |> CommonFixes.fix_implicit_addressing(follower_collection)
   end
@@ -383,11 +414,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),
@@ -408,20 +456,21 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
   end
 
   # disallow objects with bogus IDs
-  def handle_incoming(%{"id" => nil}, _options), do: :error
+  defp handle_incoming_normalised(%{"id" => nil}, _options), do: :error
-  def handle_incoming(%{"id" => ""}, _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,
+  defp handle_incoming_normalised(%{"id" => id}, _options)
-    do: :error
+       when is_binary(id) and byte_size(id) < 8,
+       do: :error
 
-  @doc "Rewrite misskey likes into EmojiReacts"
+  # Rewrite misskey likes into EmojiReacts
-  def handle_incoming(
+  defp handle_incoming_normalised(
-        %{
+         %{
-          "type" => "Like",
+           "type" => "Like",
-          "content" => reaction
+           "content" => reaction
-        } = data,
+         } = data,
-        options
+         options
-      ) do
+       ) do
     if Pleroma.Emoji.is_unicode_emoji?(reaction) || Pleroma.Emoji.matches_shortcode?(reaction) do
       data
       |> Map.put("type", "EmojiReact")
@@ -433,11 +482,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(
+  defp handle_incoming_normalised(
-        %{"type" => "Create", "object" => %{"type" => objtype, "id" => obj_id}} = data,
+         %{"type" => "Create", "object" => %{"type" => objtype, "id" => obj_id}} = data,
-        options
+         options
-      )
+       )
-      when objtype in ~w{Question Answer Audio Video Event Article Note Page} do
+       when objtype in ~w{Question Answer Audio Video Event Article Note Page} do
     fetch_options = Keyword.put(options, :depth, (options[:depth] || 0) + 1)
 
     object =
@@ -469,8 +518,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(%{"type" => type} = data, _options)
+  defp handle_incoming_normalised(%{"type" => type} = data, _options)
-      when type in ~w{Like EmojiReact Announce Add Remove} do
+       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}
@@ -480,11 +529,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(
+  defp handle_incoming_normalised(
-        %{"type" => type} = data,
+         %{"type" => type} = data,
-        _options
+         _options
-      )
+       )
-      when type in ~w{Update Block Follow Accept Reject} do
+       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
@@ -492,10 +541,10 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(
+  defp handle_incoming_normalised(
-        %{"type" => "Delete"} = data,
+         %{"type" => "Delete"} = data,
-        _options
+         _options
-      ) do
+       ) do
     with {:ok, activity, _} <-
            Pipeline.common_pipeline(data, local: false) do
       {:ok, activity}
@@ -515,15 +564,15 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(
+  defp handle_incoming_normalised(
-        %{
+         %{
-          "type" => "Undo",
+           "type" => "Undo",
-          "object" => %{"type" => "Follow", "object" => followed},
+           "object" => %{"type" => "Follow", "object" => followed},
-          "actor" => follower,
+           "actor" => follower,
-          "id" => id
+           "id" => id
-        } = _data,
+         } = _data,
-        _options
+         _options
-      ) do
+       ) 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
@@ -534,28 +583,28 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(
+  defp handle_incoming_normalised(
-        %{
+         %{
-          "type" => "Undo",
+           "type" => "Undo",
-          "object" => %{"type" => type}
+           "object" => %{"type" => type}
-        } = data,
+         } = data,
-        _options
+         _options
-      )
+       )
-      when type in ["Like", "EmojiReact", "Announce", "Block"] do
+       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(
+  defp handle_incoming_normalised(
-        %{
+         %{
-          "type" => "Undo",
+           "type" => "Undo",
-          "object" => object
+           "object" => object
-        } = activity,
+         } = activity,
-        options
+         options
-      )
+       )
-      when is_binary(object) do
+       when is_binary(object) do
     with %Activity{data: data} <- Activity.get_by_ap_id(object) do
       activity
       |> Map.put("object", data)
@@ -565,15 +614,15 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(
+  defp handle_incoming_normalised(
-        %{
+         %{
-          "type" => "Move",
+           "type" => "Move",
-          "actor" => origin_actor,
+           "actor" => origin_actor,
-          "object" => origin_actor,
+           "object" => origin_actor,
-          "target" => target_actor
+           "target" => target_actor
-        },
+         },
-        _options
+         _options
-      ) do
+       ) do
     with %User{} = origin_user <- User.get_cached_by_ap_id(origin_actor),
          # Use a dramatically shortened maximum age before refresh here because it is reasonable
          # for a user to
@@ -588,7 +637,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

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",