[#3053] Removed target accessibility checks for OStatus endpoints delegating to RedirectController. Added tests.

This commit is contained in:
Ivan Tashkinov 2020-10-11 22:34:28 +03:00
parent e1eb54d389
commit 89c595b772
4 changed files with 48 additions and 27 deletions

View file

@ -47,6 +47,7 @@ switched to a new configuration mechanism, however it was not officially removed
- Add documented-but-missing chat pagination. - Add documented-but-missing chat pagination.
- Allow sending out emails again. - Allow sending out emails again.
- OStatus / static FE endpoints: fixed inaccessibility for anonymous users on non-federating instances, switched to handling per `:restrict_unauthenticated` setting.
## Unreleased (Patch) ## Unreleased (Patch)

View file

@ -37,11 +37,10 @@ defmodule Pleroma.Web.OStatus.OStatusController do
with id <- Endpoint.url() <> conn.request_path, with id <- Endpoint.url() <> conn.request_path,
{_, %Activity{} = activity} <- {_, %Activity{} = activity} <-
{:activity, Activity.get_create_by_object_ap_id_with_object(id)}, {:activity, Activity.get_create_by_object_ap_id_with_object(id)},
{_, true} <- {:public?, Visibility.is_public?(activity)}, {_, true} <- {:public?, Visibility.is_public?(activity)} do
{_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)} do
redirect(conn, to: "/notice/#{activity.id}") redirect(conn, to: "/notice/#{activity.id}")
else else
reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] -> reason when reason in [{:public?, false}, {:activity, nil}] ->
{:error, :not_found} {:error, :not_found}
e -> e ->
@ -57,11 +56,10 @@ defmodule Pleroma.Web.OStatus.OStatusController do
def activity(conn, _params) do def activity(conn, _params) do
with id <- Endpoint.url() <> conn.request_path, with id <- Endpoint.url() <> conn.request_path,
{_, %Activity{} = activity} <- {:activity, Activity.normalize(id)}, {_, %Activity{} = activity} <- {:activity, Activity.normalize(id)},
{_, true} <- {:public?, Visibility.is_public?(activity)}, {_, true} <- {:public?, Visibility.is_public?(activity)} do
{_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)} do
redirect(conn, to: "/notice/#{activity.id}") redirect(conn, to: "/notice/#{activity.id}")
else else
reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] -> reason when reason in [{:public?, false}, {:activity, nil}] ->
{:error, :not_found} {:error, :not_found}
e -> e ->
@ -72,7 +70,6 @@ defmodule Pleroma.Web.OStatus.OStatusController do
def notice(%{assigns: %{format: format}} = conn, %{"id" => id}) do def notice(%{assigns: %{format: format}} = conn, %{"id" => id}) do
with {_, %Activity{} = activity} <- {:activity, Activity.get_by_id_with_object(id)}, with {_, %Activity{} = activity} <- {:activity, Activity.get_by_id_with_object(id)},
{_, true} <- {:public?, Visibility.is_public?(activity)}, {_, true} <- {:public?, Visibility.is_public?(activity)},
{_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)},
%User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do %User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do
cond do cond do
format in ["json", "activity+json"] -> format in ["json", "activity+json"] ->
@ -100,7 +97,7 @@ defmodule Pleroma.Web.OStatus.OStatusController do
RedirectController.redirector(conn, nil) RedirectController.redirector(conn, nil)
end end
else else
reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] -> reason when reason in [{:public?, false}, {:activity, nil}] ->
conn conn
|> put_status(404) |> put_status(404)
|> RedirectController.redirector(nil, 404) |> RedirectController.redirector(nil, 404)

View file

@ -9,6 +9,18 @@ defmodule Pleroma.Web.Router do
plug(:accepts, ["html"]) plug(:accepts, ["html"])
end end
pipeline :accepts_html_xml do
plug(:accepts, ["html", "xml", "rss", "atom"])
end
pipeline :accepts_html_json do
plug(:accepts, ["html", "activity+json", "json"])
end
pipeline :accepts_html_xml_json do
plug(:accepts, ["html", "xml", "rss", "atom", "activity+json", "json"])
end
pipeline :accepts_xml_rss_atom do pipeline :accepts_xml_rss_atom do
plug(:accepts, ["xml", "rss", "atom"]) plug(:accepts, ["xml", "rss", "atom"])
end end
@ -574,24 +586,10 @@ defmodule Pleroma.Web.Router do
) )
end end
pipeline :ostatus_html_json do
plug(:accepts, ["html", "activity+json", "json"])
plug(Pleroma.Plugs.StaticFEPlug)
end
pipeline :ostatus_html_xml do
plug(:accepts, ["html", "xml", "rss", "atom"])
plug(Pleroma.Plugs.StaticFEPlug)
end
pipeline :ostatus_html_xml_json do
plug(:accepts, ["html", "xml", "rss", "atom", "activity+json", "json"])
plug(Pleroma.Plugs.StaticFEPlug)
end
scope "/", Pleroma.Web do scope "/", Pleroma.Web do
# Note: html format is supported only if static FE is enabled # Note: html format is supported only if static FE is enabled
pipe_through(:ostatus_html_json) # Note: http signature is only considered for json requests (no auth for non-json requests)
pipe_through([:accepts_html_json, :http_signature, Pleroma.Plugs.StaticFEPlug])
get("/objects/:uuid", OStatus.OStatusController, :object) get("/objects/:uuid", OStatus.OStatusController, :object)
get("/activities/:uuid", OStatus.OStatusController, :activity) get("/activities/:uuid", OStatus.OStatusController, :activity)
@ -604,15 +602,17 @@ defmodule Pleroma.Web.Router do
scope "/", Pleroma.Web do scope "/", Pleroma.Web do
# Note: html format is supported only if static FE is enabled # Note: html format is supported only if static FE is enabled
pipe_through(:ostatus_html_xml_json) # Note: http signature is only considered for json requests (no auth for non-json requests)
pipe_through([:accepts_html_xml_json, :http_signature, Pleroma.Plugs.StaticFEPlug])
# Note: for json format responds with user profile (not user feed) # Note: returns user _profile_ for json requests, redirects to user _feed_ for non-json ones
get("/users/:nickname", Feed.UserController, :feed_redirect, as: :user_feed) get("/users/:nickname", Feed.UserController, :feed_redirect, as: :user_feed)
end end
scope "/", Pleroma.Web do scope "/", Pleroma.Web do
# Note: html format is supported only if static FE is enabled # Note: html format is supported only if static FE is enabled
pipe_through(:ostatus_html_xml) pipe_through([:accepts_html_xml, Pleroma.Plugs.StaticFEPlug])
get("/users/:nickname/feed", Feed.UserController, :feed, as: :user_feed) get("/users/:nickname/feed", Feed.UserController, :feed, as: :user_feed)
end end

View file

@ -78,6 +78,18 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do
assert html_response(conn, 200) =~ user.nickname assert html_response(conn, 200) =~ user.nickname
end end
test "returns 404 for local user with `restrict_unauthenticated/profiles/local` setting", %{
conn: conn
} do
clear_config([:restrict_unauthenticated, :profiles, :local], true)
local_user = insert(:user, local: true)
conn
|> get("/users/#{local_user.nickname}")
|> html_response(404)
end
end end
describe "notice html" do describe "notice html" do
@ -200,5 +212,16 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do
assert html_response(conn, 200) =~ "testing a thing!" assert html_response(conn, 200) =~ "testing a thing!"
end end
test "returns 404 for local public activity with `restrict_unauthenticated/activities/local` setting",
%{conn: conn, user: user} do
clear_config([:restrict_unauthenticated, :activities, :local], true)
{:ok, activity} = CommonAPI.post(user, %{status: "testing a thing!"})
conn
|> get("/notice/#{activity.id}")
|> html_response(404)
end
end end
end end