From 293628fb241e765f33bcbf20b876f19537af7fd4 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Wed, 10 Dec 2025 14:31:22 +0100 Subject: [PATCH] MastoAPI/CommonAPI: Return 404 when post not visible to user Akkoma patches returned 403 and some of my previous commits returned 422. This unifies the errors returned to 404 "Record not found", gaslighting user just like we do for other endpoints and how Mastodon does it. --- .../api_spec/operations/status_operation.ex | 7 +- lib/pleroma/web/common_api.ex | 16 +++- lib/pleroma/web/common_api/activity_draft.ex | 2 +- .../controllers/status_controller.ex | 5 ++ .../controllers/status_controller_test.exs | 86 +++++++++---------- 5 files changed, 66 insertions(+), 50 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex index e7dd5d9f5..b894a2787 100644 --- a/lib/pleroma/web/api_spec/operations/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/status_operation.ex @@ -178,6 +178,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do parameters: [id_param()], responses: %{ 200 => status_response(), + 400 => Operation.response("Error", "application/json", ApiError), 404 => Operation.response("Not Found", "application/json", ApiError) } } @@ -246,7 +247,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do 404 => Operation.response("Not found", "application/json", %Schema{ allOf: [ApiError], - title: "Unprocessable Entity", + title: "Not Found", example: %{ "error" => "Record not found" } @@ -340,7 +341,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do 400 => Operation.response("Error", "application/json", ApiError), 404 => Operation.response( - "Unprocessable Entity", + "Not Found", "application/json", %Schema{ allOf: [ApiError], @@ -366,7 +367,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do 400 => Operation.response("Error", "application/json", ApiError), 404 => Operation.response( - "Error", + "Not Found", "application/json", %Schema{ allOf: [ApiError], diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index fc2b0a180..b53fee6f6 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -258,7 +258,7 @@ defmodule Pleroma.Web.CommonAPI do {:ok, _} = res -> res - {:error, reason} = res when reason in [:not_found, :forbidden] -> + {:error, :not_found} = res -> res {:error, e} -> @@ -280,7 +280,7 @@ defmodule Pleroma.Web.CommonAPI do {:error, :not_found} {:visible, _} -> - {:error, :forbidden} + {:error, :not_found} {:common_pipeline, {:error, {:validate, {:error, changeset}}}} = e -> if {:object, {"already liked by this actor", []}} in changeset.errors do @@ -539,6 +539,14 @@ defmodule Pleroma.Web.CommonAPI do defp activity_belongs_to_actor(%{actor: actor}, actor), do: true defp activity_belongs_to_actor(_, _), do: {:error, :ownership_error} + defp activity_visible_to_actor(activity, %User{} = user) do + if Visibility.visible_for_user?(activity, user) do + true + else + {:error, :visibility_error} + end + end + defp object_type_is_allowed_for_pin(%{data: %{"type" => type}}) do with false <- type in ["Note", "Article", "Question"] do {:error, :not_allowed} @@ -553,7 +561,11 @@ defmodule Pleroma.Web.CommonAPI do @spec unpin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def unpin(id, user) do + # Order of visibility/belonging matters for MastoAPI responses. + # post not visible -> 404 + # post visible, not owned -> 422 with %Activity{} = activity <- create_activity_by_id(id), + true <- activity_visible_to_actor(activity, user), true <- activity_belongs_to_actor(activity, user.ap_id), {:ok, unpin_data, _} <- Builder.unpin(user, activity.object), {:ok, _unpin, _} <- diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex index 759fa302c..f520d5775 100644 --- a/lib/pleroma/web/common_api/activity_draft.ex +++ b/lib/pleroma/web/common_api/activity_draft.ex @@ -154,7 +154,7 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do add_error(draft, dgettext("errors", "Cannot reply to a deleted status")) false -> - add_error(draft, dgettext("errors", "Replying to a status that is not visibile to user")) + add_error(draft, dgettext("errors", "Record not found")) {:type, type} -> add_error( diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index b8207158d..ea20c0ded 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -417,6 +417,11 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do with {:ok, activity} <- CommonAPI.unpin(ap_id_or_id, user) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) else + # Order matters, if status is not owned by user and is not visible to user + # return 404 just like other endpoints + {:error, :visibility_error} -> + {:error, :not_found, "Record not found"} + {:error, :ownership_error} -> {:error, :unprocessable_entity, "Someone else's status cannot be unpinned"} diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index d90029bdb..ad012cfa0 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -1597,22 +1597,17 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do {:ok, activity} = CommonAPI.post(stranger, %{status: "it can eternal lie", visibility: "private"}) - resp = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/#{activity.id}/favourite") - |> json_response_and_validate_schema(403) - - assert match?(%{"error" => _}, resp) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/favourite") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end test "returns 404 error for a wrong id", %{conn: conn} do - conn = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/1/favourite") - - assert json_response_and_validate_schema(conn, 404) == %{"error" => "Record not found"} + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/1/favourite") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end end @@ -1639,13 +1634,10 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do activity = insert(:note_activity) # using base post ID - resp = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/#{activity.id}/unfavourite") - |> json_response(400) - - assert match?(%{"error" => "Could not unfavorite"}, resp) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unfavourite") + |> json_response_and_validate_schema(400) == %{"error" => "Could not unfavorite"} end test "can't unfavourite other user's favs", %{conn: conn} do @@ -1655,13 +1647,10 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do {:ok, _} = CommonAPI.favorite(activity.id, other) # using base post ID - resp = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/#{activity.id}/unfavourite") - |> json_response(400) - - assert match?(%{"error" => "Could not unfavorite"}, resp) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unfavourite") + |> json_response_and_validate_schema(400) == %{"error" => "Could not unfavorite"} end test "can't unfavourite other user's favs using their activity", %{conn: conn} do @@ -1670,13 +1659,10 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do other = insert(:user) {:ok, fav_activity} = CommonAPI.favorite(activity.id, other) # some APIs (used to) take IDs of any activity type, make sure this fails one way or another - resp = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/#{fav_activity.id}/unfavourite") - |> json_response_and_validate_schema(404) - - assert match?(%{"error" => _}, resp) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{fav_activity.id}/unfavourite") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end test "returns 404 error for a wrong id", %{conn: conn} do @@ -1722,7 +1708,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do |> json_response(403) == %{"error" => "Invalid credentials."} end - test "/pin: returns 400 error when activity is not public", %{conn: conn, user: user} do + test "/pin: returns 422 error when activity is not public", %{conn: conn, user: user} do {:ok, dm} = CommonAPI.post(user, %{status: "test", visibility: "direct"}) conn = @@ -1769,8 +1755,23 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end + test "/unpin: returns 404 error when activity not visible to user", %{user: user} do + %{conn: conn, user: stranger} = oauth_access(["write:accounts"]) + {:ok, activity} = CommonAPI.post(user, %{status: "yumi", visibility: "private"}) + + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, stranger) + + assert conn + |> assign(:user, stranger) + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unpin") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} + end + test "/unpin: returns 422 error when activity not owned by user", %{activity: activity} do - %{conn: conn} = oauth_access(["write:accounts"]) + %{conn: conn, user: user} = oauth_access(["write:accounts"]) + + assert Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, user) assert conn |> put_req_header("content-type", "application/json") @@ -2169,6 +2170,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do test "fails when base post not visible to current user", %{user: user} do other_user = insert(:user, local: true) + %{conn: conn} = oauth_access(["read:accounts"]) {:ok, activity} = CommonAPI.post(user, %{ @@ -2176,14 +2178,10 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do visibility: "private" }) - resp = - build_conn() - |> assign(:user, other_user) - |> assign(:token, insert(:oauth_token, user: other_user, scopes: ["read:accounts"])) - |> get("/api/v1/statuses/#{activity.id}/favourited_by") - |> json_response_and_validate_schema(404) - - assert match?(%{"error" => _}, resp) + assert conn + |> assign(:user, other_user) + |> get("/api/v1/statuses/#{activity.id}/favourited_by") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end test "returns empty array when :show_reactions is disabled", %{conn: conn, activity: activity} do