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