From 9f2993044098ced1585c8886c16da19dd046b5fd Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 23 Nov 2019 22:55:41 +0300 Subject: [PATCH 1/3] fetcher: move local object checking into a reusable function --- lib/pleroma/object.ex | 4 ++++ lib/pleroma/object/fetcher.ex | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index cde0eddd9..b4ed3a9b2 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -255,4 +255,8 @@ defmodule Pleroma.Object do |> Object.change(%{data: Map.merge(data || %{}, attrs)}) |> Repo.update() end + + def local?(%Object{data: %{"id" => id}}) do + String.starts_with?(id, Pleroma.Web.base_url() <> "/") + end end diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 9a9a46550..4d71c91a8 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -49,7 +49,7 @@ defmodule Pleroma.Object.Fetcher do end def refetch_object(%Object{data: %{"id" => id}} = object) do - with {:local, false} <- {:local, String.starts_with?(id, Pleroma.Web.base_url() <> "/")}, + with {:local, false} <- {:local, Object.local?(object)}, {:ok, data} <- fetch_and_contain_remote_object_from_id(id), {:ok, object} <- reinject_object(object, data) do {:ok, object} From 02f7383891ff0a8dd17f00d6d00ec3495116e38a Mon Sep 17 00:00:00 2001 From: rinpatch Date: Mon, 25 Nov 2019 17:19:33 +0300 Subject: [PATCH 2/3] ActivityPub controller: do not render remote users --- .../web/activity_pub/activity_pub_controller.ex | 3 ++- .../activity_pub/activity_pub_controller_test.exs | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index b2cd965fe..dec5da0d3 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -45,7 +45,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do end def user(conn, %{"nickname" => nickname}) do - with %User{} = user <- User.get_cached_by_nickname(nickname), + with %User{local: true} = user <- User.get_cached_by_nickname(nickname), {:ok, user} <- User.ensure_keys_present(user) do conn |> put_resp_content_type("application/activity+json") @@ -53,6 +53,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do |> render("user.json", %{user: user}) else nil -> {:error, :not_found} + %{local: false} -> {:error, :not_found} end end diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index a5414c521..1aa73d75c 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/test/web/activity_pub/activity_pub_controller_test.exs @@ -110,6 +110,19 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do assert json_response(conn, 200) == UserView.render("user.json", %{user: user}) end + + test "it returns 404 for remote users", %{ + conn: conn + } do + user = insert(:user, local: false, nickname: "remoteuser@example.com") + + conn = + conn + |> put_req_header("accept", "application/json") + |> get("/users/#{user.nickname}.json") + + assert json_response(conn, 404) + end end describe "/object/:uuid" do From 4b10804f21b80b74f9f9f85c9ecd6e1ec791255d Mon Sep 17 00:00:00 2001 From: rinpatch Date: Mon, 25 Nov 2019 17:55:17 +0300 Subject: [PATCH 3/3] 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 @@ defmodule Pleroma.Web.OStatus.OStatusController 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 @@ defmodule Pleroma.Web.OStatus.OStatusController do 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 @@ defmodule Pleroma.Web.OStatus.OStatusController 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 @@ defmodule Pleroma.Web.OStatus.OStatusController 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 @@ defmodule Pleroma.Web.OStatus.OStatusController 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 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest 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 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest 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 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest 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 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest 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)