recipients fixes/hardening for CreateGenericValidator

lib/pleroma/ecto_type/activity_pub/object_validators/recipients.ex

@@ -15,22 +15,27 @@ def cast(object) when is_binary(object) do
 
   def cast(object) when is_map(object) do
     case ObjectID.cast(object) do
-      {:ok, data} -> {:ok, data}
+      {:ok, data} -> {:ok, [data]}
       _ -> :error
     end
   end
 
   def cast(data) when is_list(data) do
+    data =
       data
-    |> Enum.reduce_while({:ok, []}, fn element, {:ok, list} ->
+      |> Enum.reduce_while([], fn element, list ->
         case ObjectID.cast(element) do
           {:ok, id} ->
-          {:cont, {:ok, [id | list]}}
+            {:cont, [id | list]}
 
           _ ->
-          {:cont, {:ok, list}}
+            {:cont, list}
         end
       end)
+      |> Enum.sort()
+      |> Enum.uniq()
+
+    {:ok, data}
   end
 
   def cast(data) do

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

@@ -9,37 +9,39 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes do
   alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.ActivityPub.Utils
 
+  def cast_recipients(message, field, field_fallback \\ []) do
+    {:ok, data} = ObjectValidators.Recipients.cast(message[field] || field_fallback)
+
+    Map.put(message, field, data)
+  end
+
   def fix_object_defaults(data) do
     %{data: %{"id" => context}, id: context_id} =
       Utils.create_context(data["context"] || data["conversation"])
 
     %User{follower_address: follower_collection} = User.get_cached_by_ap_id(data["attributedTo"])
-    {:ok, to} = ObjectValidators.Recipients.cast(data["to"] || [])
-    {:ok, cc} = ObjectValidators.Recipients.cast(data["cc"] || [])
 
     data
     |> Map.put("context", context)
     |> Map.put("context_id", context_id)
-    |> Map.put("to", to)
+    |> cast_recipients("to")
-    |> Map.put("cc", cc)
+    |> cast_recipients("cc")
+    |> cast_recipients("bto")
+    |> cast_recipients("bcc")
     |> Transmogrifier.fix_explicit_addressing(follower_collection)
     |> Transmogrifier.fix_implicit_addressing(follower_collection)
   end
 
-  defp fix_activity_recipients(activity, field, object) do
+  def fix_activity_addressing(activity, _meta) do
-    {:ok, data} = ObjectValidators.Recipients.cast(activity[field] || object[field])
+    %User{follower_address: follower_collection} = User.get_cached_by_ap_id(activity["actor"])
-
-    Map.put(activity, field, data)
-  end
-
-  def fix_activity_defaults(activity, meta) do
-    object = meta[:object_data] || %{}
 
     activity
-    |> fix_activity_recipients("to", object)
+    |> cast_recipients("to")
-    |> fix_activity_recipients("cc", object)
+    |> cast_recipients("cc")
-    |> fix_activity_recipients("bto", object)
+    |> cast_recipients("bto")
-    |> fix_activity_recipients("bcc", object)
+    |> cast_recipients("bcc")
+    |> Transmogrifier.fix_explicit_addressing(follower_collection)
+    |> Transmogrifier.fix_implicit_addressing(follower_collection)
   end
 
   def fix_actor(data) do

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

@@ -10,8 +10,10 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CreateGenericValidator do
 
   alias Pleroma.EctoType.ActivityPub.ObjectValidators
   alias Pleroma.Object
+  alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes
   alias Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations
+  alias Pleroma.Web.ActivityPub.Transmogrifier
 
   import Ecto.Changeset
 
@@ -23,6 +25,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CreateGenericValidator do
     field(:type, :string)
     field(:to, ObjectValidators.Recipients, default: [])
     field(:cc, ObjectValidators.Recipients, default: [])
+    field(:bto, ObjectValidators.Recipients, default: [])
+    field(:bcc, ObjectValidators.Recipients, default: [])
     field(:object, ObjectValidators.ObjectID)
     field(:expires_at, ObjectValidators.DateTime)
 
@@ -54,29 +58,38 @@ def changeset(struct, data) do
     |> cast(data, __schema__(:fields))
   end
 
-  defp fix_context(data, meta) do
+  # CommonFixes.fix_activity_addressing adapted for Create specific behavior
-    if object = meta[:object_data] do
+  defp fix_addressing(data, object) do
-      Map.put_new(data, "context", object["context"])
+    %User{follower_address: follower_collection} = User.get_cached_by_ap_id(data["actor"])
-    else
+
     data
-    end
+    |> CommonFixes.cast_recipients("to", object["to"])
+    |> CommonFixes.cast_recipients("cc", object["cc"])
+    |> CommonFixes.cast_recipients("bto", object["bto"])
+    |> CommonFixes.cast_recipients("bcc", object["bcc"])
+    |> Transmogrifier.fix_explicit_addressing(follower_collection)
+    |> Transmogrifier.fix_implicit_addressing(follower_collection)
   end
 
-  defp fix(data, meta) do
+  def fix(data, meta) do
+    object = meta[:object_data]
+
     data
-    |> fix_context(meta)
     |> CommonFixes.fix_actor()
-    |> CommonFixes.fix_activity_defaults(meta)
+    |> Map.put_new("context", object["context"])
+    |> fix_addressing(object)
   end
 
   defp validate_data(cng, meta) do
+    object = meta[:object_data]
+
     cng
-    |> validate_required([:actor, :type, :object])
+    |> validate_required([:actor, :type, :object, :to, :cc])
     |> validate_inclusion(:type, ["Create"])
     |> CommonValidations.validate_actor_presence()
-    |> CommonValidations.validate_any_presence([:to, :cc])
+    |> validate_actors_match(object)
-    |> validate_actors_match(meta)
+    |> validate_context_match(object)
-    |> validate_context_match(meta)
+    |> validate_addressing_match(object)
     |> validate_object_nonexistence()
     |> validate_object_containment()
   end
@@ -108,8 +121,8 @@ def validate_object_nonexistence(cng) do
     end)
   end
 
-  def validate_actors_match(cng, meta) do
+  def validate_actors_match(cng, object) do
-    attributed_to = meta[:object_data]["attributedTo"] || meta[:object_data]["actor"]
+    attributed_to = object["attributedTo"] || object["actor"]
 
     cng
     |> validate_change(:actor, fn :actor, actor ->
@@ -121,7 +134,7 @@ def validate_actors_match(cng, meta) do
     end)
   end
 
-  def validate_context_match(cng, %{object_data: %{"context" => object_context}}) do
+  def validate_context_match(cng, %{"context" => object_context}) do
     cng
     |> validate_change(:context, fn :context, context ->
       if context == object_context do
@@ -132,5 +145,18 @@ def validate_context_match(cng, %{object_data: %{"context" => object_context}})
     end)
   end
 
-  def validate_context_match(cng, _), do: cng
+  def validate_addressing_match(cng, object) do
+    [:to, :cc, :bcc, :bto]
+    |> Enum.reduce(cng, fn field, cng ->
+      object_data = object[to_string(field)]
+
+      validate_change(cng, field, fn field, data ->
+        if data == object_data do
+          []
+        else
+          [{field, "field doesn't match with object (#{inspect(object_data)})"}]
+        end
+      end)
+    end)
+  end
 end

test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs

@@ -171,8 +171,8 @@ test "it works for incoming notices" do
       assert data["to"] == ["https://www.w3.org/ns/activitystreams#Public"]
 
       assert data["cc"] == [
-               "http://mastodon.example.org/users/admin/followers",
+               "http://localtesting.pleroma.lol/users/lain",
-               "http://localtesting.pleroma.lol/users/lain"
+               "http://mastodon.example.org/users/admin/followers"
              ]
 
       assert data["actor"] == "http://mastodon.example.org/users/admin"
@@ -185,8 +185,8 @@ test "it works for incoming notices" do
       assert object_data["to"] == ["https://www.w3.org/ns/activitystreams#Public"]
 
       assert object_data["cc"] == [
-               "http://mastodon.example.org/users/admin/followers",
+               "http://localtesting.pleroma.lol/users/lain",
-               "http://localtesting.pleroma.lol/users/lain"
+               "http://mastodon.example.org/users/admin/followers"
              ]
 
       assert object_data["actor"] == "http://mastodon.example.org/users/admin"
@@ -350,8 +350,8 @@ test "it correctly processes messages with non-array to field" do
       assert {:ok, activity} = Transmogrifier.handle_incoming(data)
 
       assert [
-               "http://mastodon.example.org/users/admin/followers",
+               "http://localtesting.pleroma.lol/users/lain",
-               "http://localtesting.pleroma.lol/users/lain"
+               "http://mastodon.example.org/users/admin/followers"
              ] == activity.data["cc"]
 
       assert ["https://www.w3.org/ns/activitystreams#Public"] == activity.data["to"]