diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index 056de24bc..8d9d07874 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -47,7 +47,9 @@ def is_representable?(%Activity{} = activity) do * `actor`: the actor which is signing the message * `id`: the ActivityStreams URI of the message """ - def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = params) do + def publish_one( + %{"inbox" => inbox, "json" => json, "actor" => %User{} = actor, "id" => id} = params + ) do Logger.debug("Federating #{id} to #{inbox}") uri = %{path: path} = URI.parse(inbox) digest = "SHA-256=" <> (:crypto.hash(:sha256, json) |> Base.encode64()) @@ -74,24 +76,24 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa {"digest", digest} ] ) do - if not Map.has_key?(params, :unreachable_since) || params[:unreachable_since] do + if not Map.has_key?(params, "unreachable_since") || params["unreachable_since"] do Instances.set_reachable(inbox) end result else {_post_result, response} -> - unless params[:unreachable_since], do: Instances.set_unreachable(inbox) + unless params["unreachable_since"], do: Instances.set_unreachable(inbox) {:error, response} end end - def publish_one(%{actor_id: actor_id} = params) do + def publish_one(%{"actor_id" => actor_id} = params) do actor = User.get_cached_by_id(actor_id) params - |> Map.delete(:actor_id) - |> Map.put(:actor, actor) + |> Map.delete("actor_id") + |> Map.put("actor", actor) |> publish_one() end @@ -170,42 +172,8 @@ defp get_cc_ap_ids(ap_id, recipients) do |> Enum.map(& &1.ap_id) end - defp maybe_use_sharedinbox(%User{shared_inbox: nil, inbox: inbox}), do: inbox - defp maybe_use_sharedinbox(%User{shared_inbox: shared_inbox}), do: shared_inbox - - @doc """ - Determine a user inbox to use based on heuristics. These heuristics - are based on an approximation of the ``sharedInbox`` rules in the - [ActivityPub specification][ap-sharedinbox]. - - Please do not edit this function (or its children) without reading - the spec, as editing the code is likely to introduce some breakage - without some familiarity. - - [ap-sharedinbox]: https://www.w3.org/TR/activitypub/#shared-inbox-delivery - """ - def determine_inbox( - %Activity{data: activity_data}, - %User{inbox: inbox} = user - ) do - to = activity_data["to"] || [] - cc = activity_data["cc"] || [] - type = activity_data["type"] - - cond do - type == "Delete" -> - maybe_use_sharedinbox(user) - - Pleroma.Constants.as_public() in to || Pleroma.Constants.as_public() in cc -> - maybe_use_sharedinbox(user) - - length(to) + length(cc) > 1 -> - maybe_use_sharedinbox(user) - - true -> - inbox - end - end + defp try_sharedinbox(%User{shared_inbox: nil, inbox: inbox}), do: inbox + defp try_sharedinbox(%User{shared_inbox: shared_inbox}), do: shared_inbox @doc """ Publishes an activity with BCC to all relevant peers. @@ -237,11 +205,11 @@ def publish(%User{} = actor, %{data: %{"bcc" => bcc}} = activity) |> Jason.encode!() Pleroma.Web.Federator.Publisher.enqueue_one(__MODULE__, %{ - inbox: inbox, - json: json, - actor_id: actor.id, - id: activity.data["id"], - unreachable_since: unreachable_since + "inbox" => inbox, + "json" => json, + "actor_id" => actor.id, + "id" => activity.data["id"], + "unreachable_since" => unreachable_since }) end) end) @@ -261,7 +229,7 @@ def publish(%User{} = actor, %Activity{} = activity) do recipients(actor, activity) |> Enum.map(fn %User{} = user -> - determine_inbox(activity, user) + try_sharedinbox(user) end) |> Enum.uniq() |> Enum.filter(fn inbox -> should_federate?(inbox) end) @@ -270,11 +238,11 @@ def publish(%User{} = actor, %Activity{} = activity) do Pleroma.Web.Federator.Publisher.enqueue_one( __MODULE__, %{ - inbox: inbox, - json: json, - actor_id: actor.id, - id: activity.data["id"], - unreachable_since: unreachable_since + "inbox" => inbox, + "json" => json, + "actor_id" => actor.id, + "id" => activity.data["id"], + "unreachable_since" => unreachable_since } ) end) diff --git a/lib/pleroma/workers/publisher_worker.ex b/lib/pleroma/workers/publisher_worker.ex index be94134b9..495956f3b 100644 --- a/lib/pleroma/workers/publisher_worker.ex +++ b/lib/pleroma/workers/publisher_worker.ex @@ -30,7 +30,6 @@ def perform(%Job{ end def perform(%Job{args: %{"op" => "publish_one", "module" => module_name, "params" => params}}) do - params = Map.new(params, fn {k, v} -> {String.to_atom(k), v} end) - Federator.perform(:publish_one, String.to_atom(module_name), params) + Federator.perform(:publish_one, String.to_existing_atom(module_name), params) end end diff --git a/test/pleroma/web/activity_pub/publisher_test.exs b/test/pleroma/web/activity_pub/publisher_test.exs index b90422d61..40e635467 100644 --- a/test/pleroma/web/activity_pub/publisher_test.exs +++ b/test/pleroma/web/activity_pub/publisher_test.exs @@ -11,7 +11,6 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do import Tesla.Mock import Mock - alias Pleroma.Activity alias Pleroma.Instances alias Pleroma.Object alias Pleroma.Web.ActivityPub.Publisher @@ -51,82 +50,6 @@ test "it returns links" do end end - describe "determine_inbox/2" do - test "it returns sharedInbox for messages involving as:Public in to" do - user = insert(:user, %{shared_inbox: "http://example.com/inbox"}) - - activity = %Activity{ - data: %{"to" => [@as_public], "cc" => [user.follower_address]} - } - - assert Publisher.determine_inbox(activity, user) == "http://example.com/inbox" - end - - test "it returns sharedInbox for messages involving as:Public in cc" do - user = insert(:user, %{shared_inbox: "http://example.com/inbox"}) - - activity = %Activity{ - data: %{"cc" => [@as_public], "to" => [user.follower_address]} - } - - assert Publisher.determine_inbox(activity, user) == "http://example.com/inbox" - end - - test "it returns sharedInbox for messages involving multiple recipients in to" do - user = insert(:user, %{shared_inbox: "http://example.com/inbox"}) - user_two = insert(:user) - user_three = insert(:user) - - activity = %Activity{ - data: %{"cc" => [], "to" => [user.ap_id, user_two.ap_id, user_three.ap_id]} - } - - assert Publisher.determine_inbox(activity, user) == "http://example.com/inbox" - end - - test "it returns sharedInbox for messages involving multiple recipients in cc" do - user = insert(:user, %{shared_inbox: "http://example.com/inbox"}) - user_two = insert(:user) - user_three = insert(:user) - - activity = %Activity{ - data: %{"to" => [], "cc" => [user.ap_id, user_two.ap_id, user_three.ap_id]} - } - - assert Publisher.determine_inbox(activity, user) == "http://example.com/inbox" - end - - test "it returns sharedInbox for messages involving multiple recipients in total" do - user = - insert(:user, %{ - shared_inbox: "http://example.com/inbox", - inbox: "http://example.com/personal-inbox" - }) - - user_two = insert(:user) - - activity = %Activity{ - data: %{"to" => [user_two.ap_id], "cc" => [user.ap_id]} - } - - assert Publisher.determine_inbox(activity, user) == "http://example.com/inbox" - end - - test "it returns inbox for messages involving single recipients in total" do - user = - insert(:user, %{ - shared_inbox: "http://example.com/inbox", - inbox: "http://example.com/personal-inbox" - }) - - activity = %Activity{ - data: %{"to" => [user.ap_id], "cc" => []} - } - - assert Publisher.determine_inbox(activity, user) == "http://example.com/personal-inbox" - end - end - describe "publish_one/1" do test "publish to url with with different ports" do inbox80 = "http://42.site/users/nick1/inbox" @@ -146,20 +69,20 @@ test "publish to url with with different ports" do assert {:ok, %{body: "port 42"}} = Publisher.publish_one(%{ - inbox: inbox42, - json: "{}", - actor: actor, - id: 1, - unreachable_since: true + "inbox" => inbox42, + "json" => "{}", + "actor" => actor, + "id" => 1, + "unreachable_since" => true }) assert {:ok, %{body: "port 80"}} = Publisher.publish_one(%{ - inbox: inbox80, - json: "{}", - actor: actor, - id: 1, - unreachable_since: true + "inbox" => inbox80, + "json" => "{}", + "actor" => actor, + "id" => 1, + "unreachable_since" => true }) end @@ -173,7 +96,14 @@ test "publish to url with with different ports" do inbox = "http://200.site/users/nick1/inbox" - assert {:ok, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) + assert {:ok, _} = + Publisher.publish_one(%{ + "inbox" => inbox, + "json" => "{}", + "actor" => actor, + "id" => 1 + }) + assert called(Instances.set_reachable(inbox)) end @@ -189,11 +119,11 @@ test "publish to url with with different ports" do assert {:ok, _} = Publisher.publish_one(%{ - inbox: inbox, - json: "{}", - actor: actor, - id: 1, - unreachable_since: NaiveDateTime.utc_now() + "inbox" => inbox, + "json" => "{}", + "actor" => actor, + "id" => 1, + "unreachable_since" => NaiveDateTime.utc_now() }) assert called(Instances.set_reachable(inbox)) @@ -211,11 +141,11 @@ test "publish to url with with different ports" do assert {:ok, _} = Publisher.publish_one(%{ - inbox: inbox, - json: "{}", - actor: actor, - id: 1, - unreachable_since: nil + "inbox" => inbox, + "json" => "{}", + "actor" => actor, + "id" => 1, + "unreachable_since" => nil }) refute called(Instances.set_reachable(inbox)) @@ -231,7 +161,13 @@ test "publish to url with with different ports" do inbox = "http://404.site/users/nick1/inbox" - assert {:error, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) + assert {:error, _} = + Publisher.publish_one(%{ + "inbox" => inbox, + "json" => "{}", + "actor" => actor, + "id" => 1 + }) assert called(Instances.set_unreachable(inbox)) end @@ -248,7 +184,12 @@ test "publish to url with with different ports" do assert capture_log(fn -> assert {:error, _} = - Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) + Publisher.publish_one(%{ + "inbox" => inbox, + "json" => "{}", + "actor" => actor, + "id" => 1 + }) end) =~ "connrefused" assert called(Instances.set_unreachable(inbox)) @@ -264,7 +205,13 @@ test "publish to url with with different ports" do inbox = "http://200.site/users/nick1/inbox" - assert {:ok, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) + assert {:ok, _} = + Publisher.publish_one(%{ + "inbox" => inbox, + "json" => "{}", + "actor" => actor, + "id" => 1 + }) refute called(Instances.set_unreachable(inbox)) end @@ -282,11 +229,11 @@ test "publish to url with with different ports" do assert capture_log(fn -> assert {:error, _} = Publisher.publish_one(%{ - inbox: inbox, - json: "{}", - actor: actor, - id: 1, - unreachable_since: NaiveDateTime.utc_now() + "inbox" => inbox, + "json" => "{}", + "actor" => actor, + "id" => 1, + "unreachable_since" => NaiveDateTime.utc_now() }) end) =~ "connrefused" @@ -344,33 +291,33 @@ test "publish to url with with different ports" do assert not called( Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{ - inbox: "https://domain.com/users/nick1/inbox", - actor_id: actor.id, - id: note_activity.data["id"] + "inbox" => "https://domain.com/users/nick1/inbox", + "actor_id" => actor.id, + "id" => note_activity.data["id"] }) ) assert not called( Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{ - inbox: "https://domain.com/users/nick1/inbox", - actor_id: actor.id, - id: public_note_activity.data["id"] + "inbox" => "https://domain.com/users/nick1/inbox", + "actor_id" => actor.id, + "id" => public_note_activity.data["id"] }) ) assert not called( Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{ - inbox: "https://rejected.com/users/nick2/inbox", - actor_id: actor.id, - id: note_activity.data["id"] + "inbox" => "https://rejected.com/users/nick2/inbox", + "actor_id" => actor.id, + "id" => note_activity.data["id"] }) ) assert not called( Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{ - inbox: "https://rejected.com/users/nick2/inbox", - actor_id: actor.id, - id: public_note_activity.data["id"] + "inbox" => "https://rejected.com/users/nick2/inbox", + "actor_id" => actor.id, + "id" => public_note_activity.data["id"] }) ) end @@ -406,9 +353,9 @@ test "publish to url with with different ports" do assert called( Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{ - inbox: "https://domain.com/users/nick1/inbox", - actor_id: actor.id, - id: note_activity.data["id"] + "inbox" => "https://domain.com/users/nick1/inbox", + "actor_id" => actor.id, + "id" => note_activity.data["id"] }) ) end @@ -441,9 +388,9 @@ test "publish to url with with different ports" do assert called( Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{ - inbox: "https://domain.com/users/nick1/inbox", - actor_id: actor.id, - id: note_activity.data["id"] + "inbox" => "https://domain.com/users/nick1/inbox", + "actor_id" => actor.id, + "id" => note_activity.data["id"] }) ) end @@ -493,17 +440,17 @@ test "publish to url with with different ports" do assert called( Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{ - inbox: "https://domain.com/users/nick1/inbox", - actor_id: actor.id, - id: delete.data["id"] + "inbox" => "https://domain.com/users/nick1/inbox", + "actor_id" => actor.id, + "id" => delete.data["id"] }) ) assert called( Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{ - inbox: "https://domain2.com/users/nick1/inbox", - actor_id: actor.id, - id: delete.data["id"] + "inbox" => "https://domain2.com/users/nick1/inbox", + "actor_id" => actor.id, + "id" => delete.data["id"] }) ) end