From 641184fc7aff694e4e7e802b9204a1d313c0877c Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 10 Sep 2020 19:45:42 +0200 Subject: [PATCH] recipients fixes/hardening for CreateGenericValidator --- .../object_validators/recipients.ex | 25 ++++---- .../object_validators/common_fixes.ex | 34 ++++++----- .../create_generic_validator.ex | 60 +++++++++++++------ .../transmogrifier/note_handling_test.exs | 12 ++-- 4 files changed, 82 insertions(+), 49 deletions(-) diff --git a/lib/pleroma/ecto_type/activity_pub/object_validators/recipients.ex b/lib/pleroma/ecto_type/activity_pub/object_validators/recipients.ex index a03471462..06fed8fb3 100644 --- a/lib/pleroma/ecto_type/activity_pub/object_validators/recipients.ex +++ b/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 - |> Enum.reduce_while({:ok, []}, fn element, {:ok, list} -> - case ObjectID.cast(element) do - {:ok, id} -> - {:cont, {:ok, [id | list]}} + data = + data + |> Enum.reduce_while([], fn element, list -> + case ObjectID.cast(element) do + {:ok, id} -> + {:cont, [id | list]} - _ -> - {:cont, {:ok, list}} - end - end) + _ -> + {:cont, list} + end + end) + |> Enum.sort() + |> Enum.uniq() + + {:ok, data} end def cast(data) do 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 7309f6af2..009cd51b0 100644 --- a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex +++ b/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) - |> Map.put("cc", cc) + |> cast_recipients("to") + |> 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 - {:ok, data} = ObjectValidators.Recipients.cast(activity[field] || object[field]) - - Map.put(activity, field, data) - end - - def fix_activity_defaults(activity, meta) do - object = meta[:object_data] || %{} + def fix_activity_addressing(activity, _meta) do + %User{follower_address: follower_collection} = User.get_cached_by_ap_id(activity["actor"]) activity - |> fix_activity_recipients("to", object) - |> fix_activity_recipients("cc", object) - |> fix_activity_recipients("bto", object) - |> fix_activity_recipients("bcc", object) + |> cast_recipients("to") + |> cast_recipients("cc") + |> cast_recipients("bto") + |> cast_recipients("bcc") + |> Transmogrifier.fix_explicit_addressing(follower_collection) + |> Transmogrifier.fix_implicit_addressing(follower_collection) end def fix_actor(data) do 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 99e8dc6c7..51d43e8d0 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 @@ -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 - if object = meta[:object_data] do - Map.put_new(data, "context", object["context"]) - else - data - end + # CommonFixes.fix_activity_addressing adapted for Create specific behavior + defp fix_addressing(data, object) do + %User{follower_address: follower_collection} = User.get_cached_by_ap_id(data["actor"]) + + data + |> 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(meta) - |> validate_context_match(meta) + |> validate_actors_match(object) + |> validate_context_match(object) + |> 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 - attributed_to = meta[:object_data]["attributedTo"] || meta[:object_data]["actor"] + def validate_actors_match(cng, object) do + 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 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 3eeae4004..b79f2c94c 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs +++ b/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"]