From 4b10804f21b80b74f9f9f85c9ecd6e1ec791255d Mon Sep 17 00:00:00 2001 From: rinpatch Date: Mon, 25 Nov 2019 17:55:17 +0300 Subject: [PATCH] OStatus controller: don't serve json at /notice/, redirect instead --- lib/pleroma/web/ostatus/ostatus_controller.ex | 46 ++++------- test/web/ostatus/ostatus_controller_test.exs | 79 ++++--------------- 2 files changed, 28 insertions(+), 97 deletions(-) diff --git a/lib/pleroma/web/ostatus/ostatus_controller.ex b/lib/pleroma/web/ostatus/ostatus_controller.ex index 12a7c2365..01ec7941e 100644 --- a/lib/pleroma/web/ostatus/ostatus_controller.ex +++ b/lib/pleroma/web/ostatus/ostatus_controller.ex @@ -11,7 +11,6 @@ defmodule Pleroma.Web.OStatus.OStatusController do alias Pleroma.Plugs.RateLimiter alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPubController - alias Pleroma.Web.ActivityPub.ObjectView alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.Endpoint alias Pleroma.Web.Metadata.PlayerView @@ -38,11 +37,9 @@ def object(%{assigns: %{format: format}} = conn, %{"uuid" => uuid}) do with id <- o_status_url(conn, :object, uuid), {_, %Activity{} = activity} <- {:activity, Activity.get_create_by_object_ap_id_with_object(id)}, - {_, true} <- {:public?, Visibility.is_public?(activity)}, - %User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do + {_, true} <- {:public?, Visibility.is_public?(activity)} do case format do - "html" -> redirect(conn, to: "/notice/#{activity.id}") - _ -> represent_activity(conn, nil, activity, user) + _ -> redirect(conn, to: "/notice/#{activity.id}") end else reason when reason in [{:public?, false}, {:activity, nil}] -> @@ -61,11 +58,9 @@ def activity(%{assigns: %{format: format}} = conn, %{"uuid" => _uuid}) def activity(%{assigns: %{format: format}} = conn, %{"uuid" => uuid}) do with id <- o_status_url(conn, :activity, uuid), {_, %Activity{} = activity} <- {:activity, Activity.normalize(id)}, - {_, true} <- {:public?, Visibility.is_public?(activity)}, - %User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do + {_, true} <- {:public?, Visibility.is_public?(activity)} do case format do - "html" -> redirect(conn, to: "/notice/#{activity.id}") - _ -> represent_activity(conn, format, activity, user) + _ -> redirect(conn, to: "/notice/#{activity.id}") end else reason when reason in [{:public?, false}, {:activity, nil}] -> @@ -81,7 +76,15 @@ def notice(%{assigns: %{format: format}} = conn, %{"id" => id}) do {_, true} <- {:public?, Visibility.is_public?(activity)}, %User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do cond do - format == "html" && activity.data["type"] == "Create" -> + format in ["json", "activity+json"] -> + if activity.local do + %{data: %{"id" => redirect_url}} = Object.normalize(activity) + redirect(conn, external: redirect_url) + else + {:error, :not_found} + end + + activity.data["type"] == "Create" -> %Object{} = object = Object.normalize(activity) RedirectController.redirector_with_meta( @@ -94,11 +97,8 @@ def notice(%{assigns: %{format: format}} = conn, %{"id" => id}) do } ) - format == "html" -> - RedirectController.redirector(conn, nil) - true -> - represent_activity(conn, format, activity, user) + RedirectController.redirector(conn, nil) end else reason when reason in [{:public?, false}, {:activity, nil}] -> @@ -135,24 +135,6 @@ def notice_player(conn, %{"id" => id}) do end end - defp represent_activity( - conn, - "activity+json", - %Activity{data: %{"type" => "Create"}} = activity, - _user - ) do - object = Object.normalize(activity) - - conn - |> put_resp_header("content-type", "application/activity+json") - |> put_view(ObjectView) - |> render("object.json", %{object: object}) - end - - defp represent_activity(_conn, _, _, _) do - {:error, :not_found} - end - def errors(conn, {:error, :not_found}) do render_error(conn, :not_found, "Not found") end diff --git a/test/web/ostatus/ostatus_controller_test.exs b/test/web/ostatus/ostatus_controller_test.exs index 37b7b62f5..50235dfef 100644 --- a/test/web/ostatus/ostatus_controller_test.exs +++ b/test/web/ostatus/ostatus_controller_test.exs @@ -35,23 +35,6 @@ test "redirects to /notice/id for html format", %{conn: conn} do assert redirected_to(conn) == "/notice/#{note_activity.id}" end - test "500s when user not found", %{conn: conn} do - note_activity = insert(:note_activity) - object = Object.normalize(note_activity) - user = User.get_cached_by_ap_id(note_activity.data["actor"]) - User.invalidate_cache(user) - Pleroma.Repo.delete(user) - [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, object.data["id"])) - url = "/objects/#{uuid}" - - conn = - conn - |> put_req_header("accept", "application/xml") - |> get(url) - - assert response(conn, 500) == ~S({"error":"Something went wrong"}) - end - test "404s on private objects", %{conn: conn} do note_activity = insert(:direct_note_activity) object = Object.normalize(note_activity) @@ -82,21 +65,6 @@ test "redirects to /notice/id for html format", %{conn: conn} do assert redirected_to(conn) == "/notice/#{note_activity.id}" end - test "505s when user not found", %{conn: conn} do - note_activity = insert(:note_activity) - [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"])) - user = User.get_cached_by_ap_id(note_activity.data["actor"]) - User.invalidate_cache(user) - Pleroma.Repo.delete(user) - - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/activities/#{uuid}") - - assert response(conn, 500) == ~S({"error":"Something went wrong"}) - end - test "404s on private activities", %{conn: conn} do note_activity = insert(:direct_note_activity) [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"])) @@ -127,21 +95,28 @@ test "gets an activity in AS2 format", %{conn: conn} do end describe "GET notice/2" do - test "gets a notice in xml format", %{conn: conn} do + test "redirects to a proper object URL when json requested and the object is local", %{ + conn: conn + } do note_activity = insert(:note_activity) + expected_redirect_url = Object.normalize(note_activity).data["id"] - conn - |> get("/notice/#{note_activity.id}") - |> response(200) + redirect_url = + conn + |> put_req_header("accept", "application/activity+json") + |> get("/notice/#{note_activity.id}") + |> redirected_to() + + assert redirect_url == expected_redirect_url end - test "gets a notice in AS2 format", %{conn: conn} do - note_activity = insert(:note_activity) + test "returns a 404 on remote notice when json requested", %{conn: conn} do + note_activity = insert(:note_activity, local: false) conn |> put_req_header("accept", "application/activity+json") |> get("/notice/#{note_activity.id}") - |> json_response(200) + |> response(404) end test "500s when actor not found", %{conn: conn} do @@ -157,32 +132,6 @@ test "500s when actor not found", %{conn: conn} do assert response(conn, 500) == ~S({"error":"Something went wrong"}) end - test "only gets a notice in AS2 format for Create messages", %{conn: conn} do - note_activity = insert(:note_activity) - url = "/notice/#{note_activity.id}" - - conn = - conn - |> put_req_header("accept", "application/activity+json") - |> get(url) - - assert json_response(conn, 200) - - user = insert(:user) - - {:ok, like_activity, _} = CommonAPI.favorite(note_activity.id, user) - url = "/notice/#{like_activity.id}" - - assert like_activity.data["type"] == "Like" - - conn = - build_conn() - |> put_req_header("accept", "application/activity+json") - |> get(url) - - assert response(conn, 404) - end - test "render html for redirect for html format", %{conn: conn} do note_activity = insert(:note_activity)