From 4bce13fa2f9c9c234f8cf8d03e150f3478ff7483 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 4 Mar 2020 18:09:06 +0100 Subject: [PATCH 1/2] MastodonController: Return 404 errors correctly. --- lib/pleroma/web/common_api/common_api.ex | 17 ++++--- .../controllers/status_controller.ex | 3 ++ .../controllers/status_controller_test.exs | 47 ++++++++++++++----- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 027b3dc30..091011c6b 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -70,20 +70,21 @@ def reject_follow_request(follower, followed) do end def delete(activity_id, user) do - with %Activity{data: %{"object" => _}} = activity <- - Activity.get_by_id_with_object(activity_id), + with {_, %Activity{data: %{"object" => _}} = activity} <- + {:find_activity, Activity.get_by_id_with_object(activity_id)}, %Object{} = object <- Object.normalize(activity), true <- User.superuser?(user) || user.ap_id == object.data["actor"], {:ok, _} <- unpin(activity_id, user), {:ok, delete} <- ActivityPub.delete(object) do {:ok, delete} else + {:find_activity, _} -> {:error, :not_found} _ -> {:error, dgettext("errors", "Could not delete")} end end def repeat(id_or_ap_id, user, params \\ %{}) do - with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), + with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)}, object <- Object.normalize(activity), announce_activity <- Utils.get_existing_announce(user.ap_id, object), public <- public_announce?(object, params) do @@ -93,21 +94,23 @@ def repeat(id_or_ap_id, user, params \\ %{}) do ActivityPub.announce(user, object, nil, true, public) end else + {:find_activity, _} -> {:error, :not_found} _ -> {:error, dgettext("errors", "Could not repeat")} end end def unrepeat(id_or_ap_id, user) do - with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id) do + with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do object = Object.normalize(activity) ActivityPub.unannounce(user, object) else + {:find_activity, _} -> {:error, :not_found} _ -> {:error, dgettext("errors", "Could not unrepeat")} end end def favorite(id_or_ap_id, user) do - with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), + with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)}, object <- Object.normalize(activity), like_activity <- Utils.get_existing_like(user.ap_id, object) do if like_activity do @@ -116,15 +119,17 @@ def favorite(id_or_ap_id, user) do ActivityPub.like(user, object) end else + {:find_activity, _} -> {:error, :not_found} _ -> {:error, dgettext("errors", "Could not favorite")} end end def unfavorite(id_or_ap_id, user) do - with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id) do + with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do object = Object.normalize(activity) ActivityPub.unlike(user, object) else + {:find_activity, _} -> {:error, :not_found} _ -> {:error, dgettext("errors", "Could not unfavorite")} end end diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index b0048102f..5c90065f6 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -175,6 +175,8 @@ def show(%{assigns: %{user: user}} = conn, %{"id" => id}) do for: user, with_direct_conversation_id: true ) + else + _ -> {:error, :not_found} end end @@ -183,6 +185,7 @@ def delete(%{assigns: %{user: user}} = conn, %{"id" => id}) do with {:ok, %Activity{}} <- CommonAPI.delete(id, user) do json(conn, %{}) else + {:error, :not_found} = e -> e _e -> render_error(conn, :forbidden, "Can't delete this post") end end diff --git a/test/web/mastodon_api/controllers/status_controller_test.exs b/test/web/mastodon_api/controllers/status_controller_test.exs index 9c2ceda5d..fbf63f608 100644 --- a/test/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/web/mastodon_api/controllers/status_controller_test.exs @@ -476,6 +476,15 @@ test "get a status" do assert id == to_string(activity.id) end + test "getting a status that doesn't exist returns 404" do + %{conn: conn} = oauth_access(["read:statuses"]) + activity = insert(:note_activity) + + conn = get(conn, "/api/v1/statuses/#{String.downcase(activity.id)}") + + assert json_response(conn, 404) == %{"error" => "Record not found"} + end + test "get a direct status" do %{user: user, conn: conn} = oauth_access(["read:statuses"]) other_user = insert(:user) @@ -520,6 +529,18 @@ test "when you created it" do refute Activity.get_by_id(activity.id) end + test "when it doesn't exist" do + %{user: author, conn: conn} = oauth_access(["write:statuses"]) + activity = insert(:note_activity, user: author) + + conn = + conn + |> assign(:user, author) + |> delete("/api/v1/statuses/#{String.downcase(activity.id)}") + + assert %{"error" => "Record not found"} == json_response(conn, 404) + end + test "when you didn't create it" do %{conn: conn} = oauth_access(["write:statuses"]) activity = insert(:note_activity) @@ -574,6 +595,14 @@ test "reblogs and returns the reblogged status", %{conn: conn} do assert to_string(activity.id) == id end + test "returns 404 if the reblogged status doesn't exist", %{conn: conn} do + activity = insert(:note_activity) + + conn = post(conn, "/api/v1/statuses/#{String.downcase(activity.id)}/reblog") + + assert %{"error" => "Record not found"} = json_response(conn, 404) + end + test "reblogs privately and returns the reblogged status", %{conn: conn} do activity = insert(:note_activity) @@ -626,12 +655,6 @@ test "reblogged status for another user" do assert to_string(activity.id) == id end - - test "returns 400 error when activity is not exist", %{conn: conn} do - conn = post(conn, "/api/v1/statuses/foo/reblog") - - assert json_response(conn, 400) == %{"error" => "Could not repeat"} - end end describe "unreblogging" do @@ -649,10 +672,10 @@ test "unreblogs and returns the unreblogged status", %{user: user, conn: conn} d assert to_string(activity.id) == id end - test "returns 400 error when activity is not exist", %{conn: conn} do + test "returns 404 error when activity does not exist", %{conn: conn} do conn = post(conn, "/api/v1/statuses/foo/unreblog") - assert json_response(conn, 400) == %{"error" => "Could not unrepeat"} + assert json_response(conn, 404) == %{"error" => "Record not found"} end end @@ -677,10 +700,10 @@ test "favoriting twice will just return 200", %{conn: conn} do assert post(conn, "/api/v1/statuses/#{activity.id}/favourite") |> json_response(200) end - test "returns 400 error for a wrong id", %{conn: conn} do + test "returns 404 error for a wrong id", %{conn: conn} do conn = post(conn, "/api/v1/statuses/1/favourite") - assert json_response(conn, 400) == %{"error" => "Could not favorite"} + assert json_response(conn, 404) == %{"error" => "Record not found"} end end @@ -700,10 +723,10 @@ test "unfavorites a status and returns it", %{user: user, conn: conn} do assert to_string(activity.id) == id end - test "returns 400 error for a wrong id", %{conn: conn} do + test "returns 404 error for a wrong id", %{conn: conn} do conn = post(conn, "/api/v1/statuses/1/unfavourite") - assert json_response(conn, 400) == %{"error" => "Could not unfavorite"} + assert json_response(conn, 404) == %{"error" => "Record not found"} end end From f1750b46585528ebcabc12b35ab19727b2738585 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 5 Mar 2020 12:42:02 +0100 Subject: [PATCH 2/2] Admin API tests: Fix wrong test. --- test/web/admin_api/admin_api_controller_test.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 45b22ea24..8009d4386 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -1880,10 +1880,10 @@ test "deletes status", %{conn: conn, id: id, admin: admin} do "@#{admin.nickname} deleted status ##{id}" end - test "returns error when status is not exist", %{conn: conn} do + test "returns 404 when the status does not exist", %{conn: conn} do conn = delete(conn, "/api/pleroma/admin/statuses/test") - assert json_response(conn, :bad_request) == "Could not delete" + assert json_response(conn, :not_found) == "Not found" end end