From d53fb55bb76e06ca71a2d46fb6ce8c7d3e1494b0 Mon Sep 17 00:00:00 2001 From: Sergey Suprunenko Date: Wed, 26 Jun 2019 10:59:27 +0000 Subject: [PATCH] Return correct response when reply to a direct message is not direct itself --- lib/pleroma/web/common_api/common_api.ex | 1 + test/web/common_api/common_api_test.exs | 2 +- .../mastodon_api_controller_test.exs | 335 +++++++++--------- 3 files changed, 171 insertions(+), 167 deletions(-) diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 42b78494d..f8df1e2ea 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -247,6 +247,7 @@ def post(user, %{"status" => status} = data) do res else + {:private_to_public, true} -> {:error, "The message visibility must be direct"} {:error, _} = e -> e e -> {:error, e} end diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 7ff23b63d..e96106f11 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -121,7 +121,7 @@ test "it does not allow replies to direct messages that are not direct messages }) Enum.each(["public", "private", "unlisted"], fn visibility -> - assert {:error, {:private_to_public, _}} = + assert {:error, "The message visibility must be direct"} = CommonAPI.post(user, %{ "status" => "suya..", "visibility" => visibility, diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index 17e723528..03f57dbfa 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -94,56 +94,186 @@ test "the public timeline when public is set to false", %{conn: conn} do |> json_response(403) == %{"error" => "This resource requires authentication."} end - test "posting a status", %{conn: conn} do - user = insert(:user) + describe "posting statuses" do + setup do + user = insert(:user) - idempotency_key = "Pikachu rocks!" + conn = + build_conn() + |> assign(:user, user) - conn_one = - conn - |> assign(:user, user) - |> put_req_header("idempotency-key", idempotency_key) - |> post("/api/v1/statuses", %{ - "status" => "cofe", - "spoiler_text" => "2hu", - "sensitive" => "false" - }) + [conn: conn] + end - {:ok, ttl} = Cachex.ttl(:idempotency_cache, idempotency_key) - # Six hours - assert ttl > :timer.seconds(6 * 60 * 60 - 1) + test "posting a status", %{conn: conn} do + idempotency_key = "Pikachu rocks!" - assert %{"content" => "cofe", "id" => id, "spoiler_text" => "2hu", "sensitive" => false} = - json_response(conn_one, 200) + conn_one = + conn + |> put_req_header("idempotency-key", idempotency_key) + |> post("/api/v1/statuses", %{ + "status" => "cofe", + "spoiler_text" => "2hu", + "sensitive" => "false" + }) - assert Activity.get_by_id(id) + {:ok, ttl} = Cachex.ttl(:idempotency_cache, idempotency_key) + # Six hours + assert ttl > :timer.seconds(6 * 60 * 60 - 1) - conn_two = - conn - |> assign(:user, user) - |> put_req_header("idempotency-key", idempotency_key) - |> post("/api/v1/statuses", %{ - "status" => "cofe", - "spoiler_text" => "2hu", - "sensitive" => "false" - }) + assert %{"content" => "cofe", "id" => id, "spoiler_text" => "2hu", "sensitive" => false} = + json_response(conn_one, 200) - assert %{"id" => second_id} = json_response(conn_two, 200) + assert Activity.get_by_id(id) - assert id == second_id + conn_two = + conn + |> put_req_header("idempotency-key", idempotency_key) + |> post("/api/v1/statuses", %{ + "status" => "cofe", + "spoiler_text" => "2hu", + "sensitive" => "false" + }) - conn_three = - conn - |> assign(:user, user) - |> post("/api/v1/statuses", %{ - "status" => "cofe", - "spoiler_text" => "2hu", - "sensitive" => "false" - }) + assert %{"id" => second_id} = json_response(conn_two, 200) + assert id == second_id - assert %{"id" => third_id} = json_response(conn_three, 200) + conn_three = + conn + |> post("/api/v1/statuses", %{ + "status" => "cofe", + "spoiler_text" => "2hu", + "sensitive" => "false" + }) - refute id == third_id + assert %{"id" => third_id} = json_response(conn_three, 200) + refute id == third_id + end + + test "replying to a status", %{conn: conn} do + user = insert(:user) + {:ok, replied_to} = CommonAPI.post(user, %{"status" => "cofe"}) + + conn = + conn + |> post("/api/v1/statuses", %{"status" => "xD", "in_reply_to_id" => replied_to.id}) + + assert %{"content" => "xD", "id" => id} = json_response(conn, 200) + + activity = Activity.get_by_id(id) + + assert activity.data["context"] == replied_to.data["context"] + assert Activity.get_in_reply_to_activity(activity).id == replied_to.id + end + + test "replying to a direct message with visibility other than direct", %{conn: conn} do + user = insert(:user) + {:ok, replied_to} = CommonAPI.post(user, %{"status" => "suya..", "visibility" => "direct"}) + + Enum.each(["public", "private", "unlisted"], fn visibility -> + conn = + conn + |> post("/api/v1/statuses", %{ + "status" => "@#{user.nickname} hey", + "in_reply_to_id" => replied_to.id, + "visibility" => visibility + }) + + assert json_response(conn, 422) == %{"error" => "The message visibility must be direct"} + end) + end + + test "posting a status with an invalid in_reply_to_id", %{conn: conn} do + conn = + conn + |> post("/api/v1/statuses", %{"status" => "xD", "in_reply_to_id" => ""}) + + assert %{"content" => "xD", "id" => id} = json_response(conn, 200) + assert Activity.get_by_id(id) + end + + test "posting a sensitive status", %{conn: conn} do + conn = + conn + |> post("/api/v1/statuses", %{"status" => "cofe", "sensitive" => true}) + + assert %{"content" => "cofe", "id" => id, "sensitive" => true} = json_response(conn, 200) + assert Activity.get_by_id(id) + end + + test "posting a fake status", %{conn: conn} do + real_conn = + conn + |> post("/api/v1/statuses", %{ + "status" => + "\"Tenshi Eating a Corndog\" is a much discussed concept on /jp/. The significance of it is disputed, so I will focus on one core concept: the symbolism behind it" + }) + + real_status = json_response(real_conn, 200) + + assert real_status + assert Object.get_by_ap_id(real_status["uri"]) + + real_status = + real_status + |> Map.put("id", nil) + |> Map.put("url", nil) + |> Map.put("uri", nil) + |> Map.put("created_at", nil) + |> Kernel.put_in(["pleroma", "conversation_id"], nil) + + fake_conn = + conn + |> post("/api/v1/statuses", %{ + "status" => + "\"Tenshi Eating a Corndog\" is a much discussed concept on /jp/. The significance of it is disputed, so I will focus on one core concept: the symbolism behind it", + "preview" => true + }) + + fake_status = json_response(fake_conn, 200) + + assert fake_status + refute Object.get_by_ap_id(fake_status["uri"]) + + fake_status = + fake_status + |> Map.put("id", nil) + |> Map.put("url", nil) + |> Map.put("uri", nil) + |> Map.put("created_at", nil) + |> Kernel.put_in(["pleroma", "conversation_id"], nil) + + assert real_status == fake_status + end + + test "posting a status with OGP link preview", %{conn: conn} do + Pleroma.Config.put([:rich_media, :enabled], true) + + conn = + conn + |> post("/api/v1/statuses", %{ + "status" => "https://example.com/ogp" + }) + + assert %{"id" => id, "card" => %{"title" => "The Rock"}} = json_response(conn, 200) + assert Activity.get_by_id(id) + Pleroma.Config.put([:rich_media, :enabled], false) + end + + test "posting a direct status", %{conn: conn} do + user2 = insert(:user) + content = "direct cofe @#{user2.nickname}" + + conn = + conn + |> post("api/v1/statuses", %{"status" => content, "visibility" => "direct"}) + + assert %{"id" => id, "visibility" => "direct"} = json_response(conn, 200) + assert activity = Activity.get_by_id(id) + assert activity.recipients == [user2.ap_id, conn.assigns[:user].ap_id] + assert activity.data["to"] == [user2.ap_id] + assert activity.data["cc"] == [] + end end describe "posting polls" do @@ -243,100 +373,6 @@ test "maximum date limit is enforced", %{conn: conn} do end end - test "posting a sensitive status", %{conn: conn} do - user = insert(:user) - - conn = - conn - |> assign(:user, user) - |> post("/api/v1/statuses", %{"status" => "cofe", "sensitive" => true}) - - assert %{"content" => "cofe", "id" => id, "sensitive" => true} = json_response(conn, 200) - assert Activity.get_by_id(id) - end - - test "posting a fake status", %{conn: conn} do - user = insert(:user) - - real_conn = - conn - |> assign(:user, user) - |> post("/api/v1/statuses", %{ - "status" => - "\"Tenshi Eating a Corndog\" is a much discussed concept on /jp/. The significance of it is disputed, so I will focus on one core concept: the symbolism behind it" - }) - - real_status = json_response(real_conn, 200) - - assert real_status - assert Object.get_by_ap_id(real_status["uri"]) - - real_status = - real_status - |> Map.put("id", nil) - |> Map.put("url", nil) - |> Map.put("uri", nil) - |> Map.put("created_at", nil) - |> Kernel.put_in(["pleroma", "conversation_id"], nil) - - fake_conn = - conn - |> assign(:user, user) - |> post("/api/v1/statuses", %{ - "status" => - "\"Tenshi Eating a Corndog\" is a much discussed concept on /jp/. The significance of it is disputed, so I will focus on one core concept: the symbolism behind it", - "preview" => true - }) - - fake_status = json_response(fake_conn, 200) - - assert fake_status - refute Object.get_by_ap_id(fake_status["uri"]) - - fake_status = - fake_status - |> Map.put("id", nil) - |> Map.put("url", nil) - |> Map.put("uri", nil) - |> Map.put("created_at", nil) - |> Kernel.put_in(["pleroma", "conversation_id"], nil) - - assert real_status == fake_status - end - - test "posting a status with OGP link preview", %{conn: conn} do - Pleroma.Config.put([:rich_media, :enabled], true) - user = insert(:user) - - conn = - conn - |> assign(:user, user) - |> post("/api/v1/statuses", %{ - "status" => "https://example.com/ogp" - }) - - assert %{"id" => id, "card" => %{"title" => "The Rock"}} = json_response(conn, 200) - assert Activity.get_by_id(id) - Pleroma.Config.put([:rich_media, :enabled], false) - end - - test "posting a direct status", %{conn: conn} do - user1 = insert(:user) - user2 = insert(:user) - content = "direct cofe @#{user2.nickname}" - - conn = - conn - |> assign(:user, user1) - |> post("api/v1/statuses", %{"status" => content, "visibility" => "direct"}) - - assert %{"id" => id, "visibility" => "direct"} = json_response(conn, 200) - assert activity = Activity.get_by_id(id) - assert activity.recipients == [user2.ap_id, user1.ap_id] - assert activity.data["to"] == [user2.ap_id] - assert activity.data["cc"] == [] - end - test "direct timeline", %{conn: conn} do user_one = insert(:user) user_two = insert(:user) @@ -501,39 +537,6 @@ test "doesn't include DMs from blocked users", %{conn: conn} do assert status["id"] == direct.id end - test "replying to a status", %{conn: conn} do - user = insert(:user) - - {:ok, replied_to} = TwitterAPI.create_status(user, %{"status" => "cofe"}) - - conn = - conn - |> assign(:user, user) - |> post("/api/v1/statuses", %{"status" => "xD", "in_reply_to_id" => replied_to.id}) - - assert %{"content" => "xD", "id" => id} = json_response(conn, 200) - - activity = Activity.get_by_id(id) - - assert activity.data["context"] == replied_to.data["context"] - assert Activity.get_in_reply_to_activity(activity).id == replied_to.id - end - - test "posting a status with an invalid in_reply_to_id", %{conn: conn} do - user = insert(:user) - - conn = - conn - |> assign(:user, user) - |> post("/api/v1/statuses", %{"status" => "xD", "in_reply_to_id" => ""}) - - assert %{"content" => "xD", "id" => id} = json_response(conn, 200) - - activity = Activity.get_by_id(id) - - assert activity - end - test "verify_credentials", %{conn: conn} do user = insert(:user)