From f43f33e3078385084136295d2a3320efa6cb4134 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 19 Dec 2023 13:53:10 -0500 Subject: [PATCH] Return a 400 from a bad delivery attempt to the inbox This stops the backend from generating 500 errors from these events. --- changelog.d/bad_inbox_request.change | 1 + .../web/activity_pub/activity_pub_controller.ex | 9 +++++++-- .../activity_pub_controller_test.exs | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 changelog.d/bad_inbox_request.change diff --git a/changelog.d/bad_inbox_request.change b/changelog.d/bad_inbox_request.change new file mode 100644 index 000000000..b81f60638 --- /dev/null +++ b/changelog.d/bad_inbox_request.change @@ -0,0 +1 @@ +Invalid activities delivered to the inbox will be rejected with a 400 Bad Request diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 3b2193ca3..b1a118160 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -273,12 +273,17 @@ def outbox(conn, %{"nickname" => nickname}) do end def inbox(%{assigns: %{valid_signature: true}} = conn, %{"nickname" => nickname} = params) do - with %User{} = recipient <- User.get_cached_by_nickname(nickname), - {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(params["actor"]), + with %User{is_active: true} = recipient <- User.get_cached_by_nickname(nickname), + {:ok, %User{is_active: true} = actor} <- User.get_or_fetch_by_ap_id(params["actor"]), true <- Utils.recipient_in_message(recipient, actor, params), params <- Utils.maybe_splice_recipient(recipient.ap_id, params) do Federator.incoming_ap_doc(params) json(conn, "ok") + else + _ -> + conn + |> put_status(:bad_request) + |> json("Invalid request.") end end diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 0dc61c2e5..069736925 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -895,6 +895,23 @@ test "it inserts an incoming activity into the database", %{conn: conn, data: da assert Activity.get_by_ap_id(data["id"]) end + test "it rejects an invalid incoming activity", %{conn: conn, data: data} do + user = insert(:user, is_active: false) + + data = + data + |> Map.put("bcc", [user.ap_id]) + |> Kernel.put_in(["object", "bcc"], [user.ap_id]) + + conn = + conn + |> assign(:valid_signature, true) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{user.nickname}/inbox", data) + + assert "Invalid request." == json_response(conn, 400) + end + test "it accepts messages with to as string instead of array", %{conn: conn, data: data} do user = insert(:user)