federation: always prefer the shared inbox
In theory a pedantic reading of the spec indeed suggests DMs must only be delivered to personal inboxes. However, in practice the normative force of real-world implementations disagrees. Mastodon, Iceshrimp.NET and GtS (the latter notably has a config option to never use sharedInboxes) all unconditionally prefer sharedInbox for everything without ill effect. This saves on duplicate deliveries on the sending and processing on the receiving end. (Typically the receiving side ends up rejecting all but the first copy as duplicates) Furthermore current determine_inbox logic also actually needs up forcing personal inboxes for follower-only posts, unless they additionally explicitly address at least one specific actor. This is even much wasteful and directly contradicts the explicit intent of the spec. There’s one part where the use of sharedInbox falls apart, namely spec-compliant bcc and bto addressing. AP spec requires bcc/bto fields to be stripped before delivery and then implicitly reconstructed by the receiver based on the addressed personal inbox. In practice however, this addressing mode is almost unused. Neither of the three implementations brought up above supports it and while *oma does use bcc for list addressing, it does not use it in a spec-compliant way and even copies same-host recipients into cc before delivery. Messages with bcc addressing are handled in another function clause, always force personal inboxes for every recipient and not affected by this commit. In theory it would be beneficial to use sharedInbox there too for all but bcc recipients. But in practice list addressing has been broken for quite some time already and is not actually exposed in any frontend, as discussed in https://akkoma.dev/AkkomaGang/akkoma/issues/812. Therefore any changes here have virtually no effect anyway and all code concerning it may just be outright removed.
This commit is contained in:
Oneric
parent
0d38385d6f
commit
a80444041c
2 changed files with 3 additions and 114 deletions
|
|
@ -172,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.
|
||||
|
|
@ -263,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)
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Reference in a new issue