Merge remote-tracking branch 'origin/develop' into fork

This commit is contained in:
marcin mikołajczak 2024-08-07 09:50:15 +02:00
commit e9cea96940
10 changed files with 199 additions and 93 deletions

View file

@ -0,0 +1 @@
Fixed malformed follow requests that cause them to appear stuck pending due to the recipient being unable to process them.

View file

@ -0,0 +1 @@
Support `id` param in `GET /api/v1/statuses`

View file

@ -11,6 +11,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.Repo alias Pleroma.Repo
alias Pleroma.User alias Pleroma.User
alias Pleroma.Web.ActivityPub.Publisher.Prepared
alias Pleroma.Web.ActivityPub.Relay alias Pleroma.Web.ActivityPub.Relay
alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.ActivityPub.Transmogrifier
alias Pleroma.Workers.PublisherWorker alias Pleroma.Workers.PublisherWorker
@ -76,14 +77,13 @@ def representable?(%Activity{} = activity) do
end end
@doc """ @doc """
Publish a single message to a peer. Takes a struct with the following Prepare an activity for publishing from an Oban job
parameters set:
* `inbox`: the inbox to publish to * `inbox`: the inbox to publish to
* `activity_id`: the internal activity id * `activity_id`: the internal activity id
* `cc`: the cc recipients relevant to this inbox (optional) * `cc`: the cc recipients relevant to this inbox (optional)
""" """
def publish_one(%{inbox: inbox, activity_id: activity_id} = params) do @spec prepare_one(map()) :: Prepared.t()
def prepare_one(%{inbox: inbox, activity_id: activity_id} = params) do
activity = Activity.get_by_id_with_user_actor(activity_id) activity = Activity.get_by_id_with_user_actor(activity_id)
actor = activity.user_actor actor = activity.user_actor
@ -93,7 +93,7 @@ def publish_one(%{inbox: inbox, activity_id: activity_id} = params) do
{:ok, data} = Transmogrifier.prepare_outgoing(activity.data) {:ok, data} = Transmogrifier.prepare_outgoing(activity.data)
cc = Map.get(params, :cc) cc = Map.get(params, :cc, [])
json = json =
data data
@ -113,27 +113,49 @@ def publish_one(%{inbox: inbox, activity_id: activity_id} = params) do
date: date date: date
}) })
%Prepared{
activity_id: activity_id,
json: json,
date: date,
signature: signature,
digest: digest,
inbox: inbox,
unreachable_since: params[:unreachable_since]
}
end
@doc """
Publish a single message to a peer. Takes a struct with the following
parameters set:
* `activity_id`: the activity id
* `json`: the json payload
* `date`: the signed date from Pleroma.Signature.signed_date()
* `signature`: the signature from Pleroma.Signature.sign/2
* `digest`: base64 encoded the hash of the json payload prefixed with "SHA-256="
* `inbox`: the inbox URI of this delivery
* `unreachable_since`: timestamp the instance was marked unreachable
"""
def publish_one(%Prepared{} = p) do
with {:ok, %{status: code}} = result when code in 200..299 <- with {:ok, %{status: code}} = result when code in 200..299 <-
HTTP.post( HTTP.post(
inbox, p.inbox,
json, p.json,
[ [
{"Content-Type", "application/activity+json"}, {"Content-Type", "application/activity+json"},
{"Date", date}, {"Date", p.date},
{"signature", signature}, {"signature", p.signature},
{"digest", digest} {"digest", p.digest}
] ]
) do ) do
if not Map.has_key?(params, :unreachable_since) || params[:unreachable_since] do maybe_set_reachable(p.unreachable_since, p.inbox)
Instances.set_reachable(inbox)
end
result result
else else
{_post_result, %{status: code} = response} = e -> {_post_result, %{status: code} = response} = e ->
unless params[:unreachable_since], do: Instances.set_unreachable(inbox) maybe_set_unreachable(p.unreachable_since, p.inbox)
Logger.metadata(activity: activity_id, inbox: inbox, status: code) Logger.metadata(activity: p.activity_id, inbox: p.inbox, status: code)
Logger.error("Publisher failed to inbox #{inbox} with status #{code}") Logger.error("Publisher failed to inbox #{p.inbox} with status #{code}")
case response do case response do
%{status: 400} -> {:cancel, :bad_request} %{status: 400} -> {:cancel, :bad_request}
@ -152,15 +174,21 @@ def publish_one(%{inbox: inbox, activity_id: activity_id} = params) do
connection_pool_snooze() connection_pool_snooze()
e -> e ->
unless params[:unreachable_since], do: Instances.set_unreachable(inbox) maybe_set_unreachable(p.unreachable_since, p.inbox)
Logger.metadata(activity: activity_id, inbox: inbox) Logger.metadata(activity: p.activity_id, inbox: p.inbox)
Logger.error("Publisher failed to inbox #{inbox} #{inspect(e)}") Logger.error("Publisher failed to inbox #{p.inbox} #{inspect(e)}")
{:error, e} {:error, e}
end end
end end
defp connection_pool_snooze, do: {:snooze, 3} defp connection_pool_snooze, do: {:snooze, 3}
defp maybe_set_reachable(%NaiveDateTime{}, inbox), do: Instances.set_reachable(inbox)
defp maybe_set_reachable(_, _), do: :ok
defp maybe_set_unreachable(nil, inbox), do: Instances.set_unreachable(inbox)
defp maybe_set_unreachable(%NaiveDateTime{}, _), do: :ok
defp signature_host(%URI{port: port, scheme: scheme, host: host}) do defp signature_host(%URI{port: port, scheme: scheme, host: host}) do
if port == URI.default_port(scheme) do if port == URI.default_port(scheme) do
host host

View file

@ -0,0 +1,8 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.ActivityPub.Publisher.Prepared do
@type t :: %__MODULE__{}
defstruct [:activity_id, :json, :date, :signature, :digest, :inbox, :unreachable_since]
end

View file

@ -32,11 +32,17 @@ def index_operation do
security: [%{"oAuth" => ["read:statuses"]}], security: [%{"oAuth" => ["read:statuses"]}],
parameters: [ parameters: [
Operation.parameter( Operation.parameter(
:ids, :id,
:query, :query,
%Schema{type: :array, items: FlakeID}, %Schema{type: :array, items: FlakeID},
"Array of status IDs" "Array of status IDs"
), ),
Operation.parameter(
:ids,
:query,
%Schema{type: :array, items: FlakeID},
"Deprecated, use `id` instead"
),
Operation.parameter( Operation.parameter(
:with_muted, :with_muted,
:query, :query,

View file

@ -71,7 +71,10 @@ defp publish_priority(_), do: 0
# Job Worker Callbacks # Job Worker Callbacks
@spec perform(atom(), any()) :: {:ok, any()} | {:error, any()} @spec perform(atom(), any()) :: {:ok, any()} | {:error, any()}
def perform(:publish_one, params), do: Publisher.publish_one(params) def perform(:publish_one, params) do
Publisher.prepare_one(params)
|> Publisher.publish_one()
end
def perform(:publish, activity) do def perform(:publish, activity) do
Logger.debug(fn -> "Running publish for #{activity.data["id"]}" end) Logger.debug(fn -> "Running publish for #{activity.data["id"]}" end)

View file

@ -114,10 +114,11 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do
`ids` query param is required `ids` query param is required
""" """
def index( def index(
%{assigns: %{user: user}, private: %{open_api_spex: %{params: %{ids: ids} = params}}} = %{assigns: %{user: user}, private: %{open_api_spex: %{params: params}}} =
conn, conn,
_ _
) do ) do
ids = Map.get(params, :id, Map.get(params, :ids))
limit = 100 limit = 100
activities = activities =

View file

@ -3,6 +3,7 @@
# SPDX-License-Identifier: AGPL-3.0-only # SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.ActivityPub.PublisherTest do defmodule Pleroma.Web.ActivityPub.PublisherTest do
use Oban.Testing, repo: Pleroma.Repo
use Pleroma.Web.ConnCase use Pleroma.Web.ConnCase
import ExUnit.CaptureLog import ExUnit.CaptureLog
@ -13,6 +14,7 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do
alias Pleroma.Activity alias Pleroma.Activity
alias Pleroma.Instances alias Pleroma.Instances
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.Tests.ObanHelpers
alias Pleroma.Web.ActivityPub.Publisher alias Pleroma.Web.ActivityPub.Publisher
alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI
@ -150,32 +152,20 @@ test "publish to url with with different ports" do
_actor = insert(:user) _actor = insert(:user)
assert {:ok, %{body: "port 42"}} = assert {:ok, %{body: "port 42"}} =
Publisher.publish_one(%{ Publisher.prepare_one(%{
inbox: inbox42, inbox: inbox42,
activity_id: activity.id, activity_id: activity.id,
unreachable_since: true unreachable_since: true
}) })
|> Publisher.publish_one()
assert {:ok, %{body: "port 80"}} = assert {:ok, %{body: "port 80"}} =
Publisher.publish_one(%{ Publisher.prepare_one(%{
inbox: inbox80, inbox: inbox80,
activity_id: activity.id, activity_id: activity.id,
unreachable_since: true unreachable_since: true
}) })
end |> Publisher.publish_one()
test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is not specified",
Instances,
[:passthrough],
[] do
_actor = insert(:user)
inbox = "http://200.site/users/nick1/inbox"
activity = insert(:note_activity)
assert {:ok, _} =
Publisher.publish_one(%{inbox: inbox, activity_id: activity.id})
assert called(Instances.set_reachable(inbox))
end end
test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is set", test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is set",
@ -187,11 +177,12 @@ test "publish to url with with different ports" do
activity = insert(:note_activity) activity = insert(:note_activity)
assert {:ok, _} = assert {:ok, _} =
Publisher.publish_one(%{ Publisher.prepare_one(%{
inbox: inbox, inbox: inbox,
activity_id: activity.id, activity_id: activity.id,
unreachable_since: NaiveDateTime.utc_now() unreachable_since: NaiveDateTime.utc_now()
}) })
|> Publisher.publish_one()
assert called(Instances.set_reachable(inbox)) assert called(Instances.set_reachable(inbox))
end end
@ -205,11 +196,12 @@ test "publish to url with with different ports" do
activity = insert(:note_activity) activity = insert(:note_activity)
assert {:ok, _} = assert {:ok, _} =
Publisher.publish_one(%{ Publisher.prepare_one(%{
inbox: inbox, inbox: inbox,
activity_id: activity.id, activity_id: activity.id,
unreachable_since: nil unreachable_since: nil
}) })
|> Publisher.publish_one()
refute called(Instances.set_reachable(inbox)) refute called(Instances.set_reachable(inbox))
end end
@ -223,7 +215,8 @@ test "publish to url with with different ports" do
activity = insert(:note_activity) activity = insert(:note_activity)
assert {:cancel, _} = assert {:cancel, _} =
Publisher.publish_one(%{inbox: inbox, activity_id: activity.id}) Publisher.prepare_one(%{inbox: inbox, activity_id: activity.id})
|> Publisher.publish_one()
assert called(Instances.set_unreachable(inbox)) assert called(Instances.set_unreachable(inbox))
end end
@ -238,10 +231,11 @@ test "publish to url with with different ports" do
assert capture_log(fn -> assert capture_log(fn ->
assert {:error, _} = assert {:error, _} =
Publisher.publish_one(%{ Publisher.prepare_one(%{
inbox: inbox, inbox: inbox,
activity_id: activity.id activity_id: activity.id
}) })
|> Publisher.publish_one()
end) =~ "connrefused" end) =~ "connrefused"
assert called(Instances.set_unreachable(inbox)) assert called(Instances.set_unreachable(inbox))
@ -256,7 +250,8 @@ test "publish to url with with different ports" do
activity = insert(:note_activity) activity = insert(:note_activity)
assert {:ok, _} = assert {:ok, _} =
Publisher.publish_one(%{inbox: inbox, activity_id: activity.id}) Publisher.prepare_one(%{inbox: inbox, activity_id: activity.id})
|> Publisher.publish_one()
refute called(Instances.set_unreachable(inbox)) refute called(Instances.set_unreachable(inbox))
end end
@ -271,11 +266,12 @@ test "publish to url with with different ports" do
assert capture_log(fn -> assert capture_log(fn ->
assert {:error, _} = assert {:error, _} =
Publisher.publish_one(%{ Publisher.prepare_one(%{
inbox: inbox, inbox: inbox,
activity_id: activity.id, activity_id: activity.id,
unreachable_since: NaiveDateTime.utc_now() unreachable_since: NaiveDateTime.utc_now()
}) })
|> Publisher.publish_one()
end) =~ "connrefused" end) =~ "connrefused"
refute called(Instances.set_unreachable(inbox)) refute called(Instances.set_unreachable(inbox))
@ -310,12 +306,15 @@ test "publish to url with with different ports" do
assert res == :ok assert res == :ok
assert not called( refute_enqueued(
Publisher.enqueue_one(%{ worker: "Pleroma.Workers.PublisherWorker",
inbox: "https://domain.com/users/nick1/inbox", args: %{
activity_id: note_activity.id "params" => %{
}) inbox: "https://domain.com/users/nick1/inbox",
) activity_id: note_activity.id
}
}
)
end end
test_with_mock "Publishes a non-public activity to non-quarantined instances.", test_with_mock "Publishes a non-public activity to non-quarantined instances.",
@ -345,15 +344,16 @@ test "publish to url with with different ports" do
assert res == :ok assert res == :ok
assert called( assert_enqueued(
Publisher.enqueue_one( worker: "Pleroma.Workers.PublisherWorker",
%{ args: %{
inbox: "https://domain.com/users/nick1/inbox", "params" => %{
activity_id: note_activity.id inbox: "https://domain.com/users/nick1/inbox",
}, activity_id: note_activity.id
priority: 1 }
) },
) priority: 1
)
end end
test_with_mock "Publishes to directly addressed actors with higher priority.", test_with_mock "Publishes to directly addressed actors with higher priority.",
@ -403,12 +403,15 @@ test "publish to url with with different ports" do
res = Publisher.publish(actor, note_activity) res = Publisher.publish(actor, note_activity)
assert res == :ok assert res == :ok
assert called( assert_enqueued(
Publisher.enqueue_one(%{ worker: "Pleroma.Workers.PublisherWorker",
inbox: "https://domain.com/users/nick1/inbox", args: %{
activity_id: note_activity.id "params" => %{
}) inbox: "https://domain.com/users/nick1/inbox",
) activity_id: note_activity.id
}
}
)
end end
test_with_mock "publishes a delete activity to peers who signed fetch requests to the create acitvity/object.", test_with_mock "publishes a delete activity to peers who signed fetch requests to the create acitvity/object.",
@ -452,25 +455,69 @@ test "publish to url with with different ports" do
res = Publisher.publish(actor, delete) res = Publisher.publish(actor, delete)
assert res == :ok assert res == :ok
assert called( assert_enqueued(
Publisher.enqueue_one( worker: "Pleroma.Workers.PublisherWorker",
%{ args: %{
inbox: "https://domain.com/users/nick1/inbox", "params" => %{
activity_id: delete.id inbox: "https://domain.com/users/nick1/inbox",
}, activity_id: delete.id
priority: 1 }
) },
) priority: 1
)
assert called( assert_enqueued(
Publisher.enqueue_one( worker: "Pleroma.Workers.PublisherWorker",
%{ args: %{
inbox: "https://domain2.com/users/nick1/inbox", "params" => %{
activity_id: delete.id inbox: "https://domain2.com/users/nick1/inbox",
}, activity_id: delete.id
priority: 1 }
) },
) priority: 1
)
end end
end end
test "cc in prepared json for a follow request is an empty list" do
user = insert(:user)
remote_user = insert(:user, local: false)
{:ok, _, _, activity} = CommonAPI.follow(remote_user, user)
assert_enqueued(
worker: "Pleroma.Workers.PublisherWorker",
args: %{
"activity_id" => activity.id,
"op" => "publish"
}
)
ObanHelpers.perform_all()
expected_params =
%{
"activity_id" => activity.id,
"inbox" => remote_user.inbox,
"unreachable_since" => nil
}
assert_enqueued(
worker: "Pleroma.Workers.PublisherWorker",
args: %{
"op" => "publish_one",
"params" => expected_params
}
)
# params need to be atom keys for Publisher.prepare_one.
# this is done in the Oban job.
expected_params = Map.new(expected_params, fn {k, v} -> {String.to_atom(k), v} end)
%{json: json} = Publisher.prepare_one(expected_params)
{:ok, decoded} = Jason.decode(json)
assert decoded["cc"] == []
end
end end

View file

@ -922,13 +922,23 @@ test "get statuses by IDs" do
%{id: id1} = insert(:note_activity) %{id: id1} = insert(:note_activity)
%{id: id2} = insert(:note_activity) %{id: id2} = insert(:note_activity)
query_string = "ids[]=#{id1}&ids[]=#{id2}" query_string = "id[]=#{id1}&id[]=#{id2}"
conn = get(conn, "/api/v1/statuses/?#{query_string}") conn = get(conn, "/api/v1/statuses/?#{query_string}")
assert [%{"id" => ^id1}, %{"id" => ^id2}] = assert [%{"id" => ^id1}, %{"id" => ^id2}] =
Enum.sort_by(json_response_and_validate_schema(conn, :ok), & &1["id"]) Enum.sort_by(json_response_and_validate_schema(conn, :ok), & &1["id"])
end end
test "get statuses by IDs falls back to ids[]" do
%{conn: conn} = oauth_access(["read:statuses"])
%{id: id} = insert(:note_activity)
query_string = "ids[]=#{id}"
conn = get(conn, "/api/v1/statuses/?#{query_string}")
assert [%{"id" => ^id}] = json_response_and_validate_schema(conn, 200)
end
describe "getting statuses by ids with restricted unauthenticated for local and remote" do describe "getting statuses by ids with restricted unauthenticated for local and remote" do
setup do: local_and_remote_activities() setup do: local_and_remote_activities()
@ -937,7 +947,7 @@ test "get statuses by IDs" do
setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true) setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true)
test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do
res_conn = get(conn, "/api/v1/statuses?ids[]=#{local.id}&ids[]=#{remote.id}") res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}")
assert json_response_and_validate_schema(res_conn, 200) == [] assert json_response_and_validate_schema(res_conn, 200) == []
end end
@ -945,7 +955,7 @@ test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} d
test "if user is authenticated", %{local: local, remote: remote} do test "if user is authenticated", %{local: local, remote: remote} do
%{conn: conn} = oauth_access(["read"]) %{conn: conn} = oauth_access(["read"])
res_conn = get(conn, "/api/v1/statuses?ids[]=#{local.id}&ids[]=#{remote.id}") res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}")
assert length(json_response_and_validate_schema(res_conn, 200)) == 2 assert length(json_response_and_validate_schema(res_conn, 200)) == 2
end end
@ -957,7 +967,7 @@ test "if user is authenticated", %{local: local, remote: remote} do
setup do: clear_config([:restrict_unauthenticated, :activities, :local], true) setup do: clear_config([:restrict_unauthenticated, :activities, :local], true)
test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do
res_conn = get(conn, "/api/v1/statuses?ids[]=#{local.id}&ids[]=#{remote.id}") res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}")
remote_id = remote.id remote_id = remote.id
assert [%{"id" => ^remote_id}] = json_response_and_validate_schema(res_conn, 200) assert [%{"id" => ^remote_id}] = json_response_and_validate_schema(res_conn, 200)
@ -966,7 +976,7 @@ test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} d
test "if user is authenticated", %{local: local, remote: remote} do test "if user is authenticated", %{local: local, remote: remote} do
%{conn: conn} = oauth_access(["read"]) %{conn: conn} = oauth_access(["read"])
res_conn = get(conn, "/api/v1/statuses?ids[]=#{local.id}&ids[]=#{remote.id}") res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}")
assert length(json_response_and_validate_schema(res_conn, 200)) == 2 assert length(json_response_and_validate_schema(res_conn, 200)) == 2
end end
@ -978,7 +988,7 @@ test "if user is authenticated", %{local: local, remote: remote} do
setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true) setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true)
test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do
res_conn = get(conn, "/api/v1/statuses?ids[]=#{local.id}&ids[]=#{remote.id}") res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}")
local_id = local.id local_id = local.id
assert [%{"id" => ^local_id}] = json_response_and_validate_schema(res_conn, 200) assert [%{"id" => ^local_id}] = json_response_and_validate_schema(res_conn, 200)
@ -987,7 +997,7 @@ test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} d
test "if user is authenticated", %{local: local, remote: remote} do test "if user is authenticated", %{local: local, remote: remote} do
%{conn: conn} = oauth_access(["read"]) %{conn: conn} = oauth_access(["read"])
res_conn = get(conn, "/api/v1/statuses?ids[]=#{local.id}&ids[]=#{remote.id}") res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}")
assert length(json_response_and_validate_schema(res_conn, 200)) == 2 assert length(json_response_and_validate_schema(res_conn, 200)) == 2
end end
@ -2241,7 +2251,7 @@ test "index" do
result = result =
conn conn
|> get("/api/v1/statuses/?ids[]=#{activity.id}") |> get("/api/v1/statuses/?id[]=#{activity.id}")
|> json_response_and_validate_schema(200) |> json_response_and_validate_schema(200)
assert [ assert [
@ -2254,7 +2264,7 @@ test "index" do
result = result =
conn conn
|> get("/api/v1/statuses/?ids[]=#{activity.id}&with_muted=true") |> get("/api/v1/statuses/?id[]=#{activity.id}&with_muted=true")
|> json_response_and_validate_schema(200) |> json_response_and_validate_schema(200)
assert [ assert [

View file

@ -63,7 +63,8 @@ def user_factory(attrs \\ %{}) do
ap_id: ap_id, ap_id: ap_id,
follower_address: ap_id <> "/followers", follower_address: ap_id <> "/followers",
following_address: ap_id <> "/following", following_address: ap_id <> "/following",
featured_address: ap_id <> "/collections/featured" featured_address: ap_id <> "/collections/featured",
inbox: "https://#{base_domain}/inbox"
} }
else else
%{ %{