From 92753b0cd9cfcdc5edb64a5e55ad27f73079f9e0 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 29 Jan 2019 13:12:28 +0300 Subject: [PATCH] [#534] Made federation push sender be determined basing on content instead of `referer` header. Updated tests. --- .../plugs/set_requester_reachable_plug.ex | 16 -------------- lib/pleroma/reverse_proxy.ex | 3 +-- lib/pleroma/web/activity_pub/activity_pub.ex | 3 +-- .../activity_pub/activity_pub_controller.ex | 11 +++++++++- lib/pleroma/web/ostatus/ostatus.ex | 3 +++ lib/pleroma/web/ostatus/ostatus_controller.ex | 1 - lib/pleroma/web/salmon/salmon.ex | 5 +---- lib/pleroma/web/websub/websub.ex | 3 +-- lib/pleroma/web/websub/websub_controller.ex | 2 -- .../activity_pub_controller_test.exs | 16 ++++++-------- test/web/instances/instances_test.exs | 18 +++++++++++++++ .../delete_handling_test.exs | 7 ++++++ test/web/ostatus/ostatus_controller_test.exs | 20 +---------------- test/web/ostatus/ostatus_test.exs | 18 ++++++++++++++- test/web/websub/websub_controller_test.exs | 22 +------------------ 15 files changed, 68 insertions(+), 80 deletions(-) delete mode 100644 lib/pleroma/plugs/set_requester_reachable_plug.ex diff --git a/lib/pleroma/plugs/set_requester_reachable_plug.ex b/lib/pleroma/plugs/set_requester_reachable_plug.ex deleted file mode 100644 index 88551be70..000000000 --- a/lib/pleroma/plugs/set_requester_reachable_plug.ex +++ /dev/null @@ -1,16 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2019 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.Plugs.SetRequesterReachablePlug do - import Plug.Conn - - def init(_), do: [] - - def call(%Plug.Conn{} = conn, _) do - with [referer] <- get_req_header(conn, "referer"), - do: Pleroma.Instances.set_reachable(referer) - - conn - end -end diff --git a/lib/pleroma/reverse_proxy.ex b/lib/pleroma/reverse_proxy.ex index d8b17212b..a25b5ea4e 100644 --- a/lib/pleroma/reverse_proxy.ex +++ b/lib/pleroma/reverse_proxy.ex @@ -3,8 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.ReverseProxy do - @keep_req_headers ~w(accept user-agent accept-encoding cache-control if-modified-since if-unmodified-since) ++ - ~w(if-none-match if-range range referer) + @keep_req_headers ~w(accept user-agent accept-encoding cache-control if-modified-since if-unmodified-since if-none-match if-range range) @resp_cache_headers ~w(etag date last-modified cache-control) @keep_resp_headers @resp_cache_headers ++ ~w(content-type content-disposition content-encoding content-range accept-ranges vary) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 22c7824fa..4016808e8 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -784,8 +784,7 @@ def publish_one(%{inbox: inbox, json: json, actor: actor, id: id}) do [ {"Content-Type", "application/activity+json"}, {"signature", signature}, - {"digest", digest}, - {"referer", Pleroma.Web.Endpoint.url()} + {"digest", digest} ] ) do Instances.set_reachable(inbox) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index fadb038a2..4dea6ab83 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -18,7 +18,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do action_fallback(:errors) plug(Pleroma.Web.FederatingPlug when action in [:inbox, :relay]) - plug(Pleroma.Web.Plugs.SetRequesterReachablePlug when action in [:inbox]) + plug(:set_requester_reachable when action in [:inbox]) plug(:relay_active? when action in [:relay]) def relay_active?(conn, _) do @@ -291,4 +291,13 @@ def errors(conn, _e) do |> put_status(500) |> json("error") end + + defp set_requester_reachable(%Plug.Conn{} = conn, _) do + with actor <- conn.params["actor"], + true <- is_binary(actor) do + Pleroma.Instances.set_reachable(actor) + end + + conn + end end diff --git a/lib/pleroma/web/ostatus/ostatus.ex b/lib/pleroma/web/ostatus/ostatus.ex index a3155b79d..a20ca17bb 100644 --- a/lib/pleroma/web/ostatus/ostatus.ex +++ b/lib/pleroma/web/ostatus/ostatus.ex @@ -48,6 +48,9 @@ def remote_follow_path do def handle_incoming(xml_string) do with doc when doc != :error <- parse_document(xml_string) do + with {:ok, actor_user} <- find_make_or_update_user(doc), + do: Pleroma.Instances.set_reachable(actor_user.ap_id) + entries = :xmerl_xpath.string('//entry', doc) activities = diff --git a/lib/pleroma/web/ostatus/ostatus_controller.ex b/lib/pleroma/web/ostatus/ostatus_controller.ex index 9392a97f0..302ff38a4 100644 --- a/lib/pleroma/web/ostatus/ostatus_controller.ex +++ b/lib/pleroma/web/ostatus/ostatus_controller.ex @@ -14,7 +14,6 @@ defmodule Pleroma.Web.OStatus.OStatusController do alias Pleroma.Web.ActivityPub.ActivityPub plug(Pleroma.Web.FederatingPlug when action in [:salmon_incoming]) - plug(Pleroma.Web.Plugs.SetRequesterReachablePlug when action in [:salmon_incoming]) action_fallback(:errors) diff --git a/lib/pleroma/web/salmon/salmon.ex b/lib/pleroma/web/salmon/salmon.ex index e96455423..07ca42a5f 100644 --- a/lib/pleroma/web/salmon/salmon.ex +++ b/lib/pleroma/web/salmon/salmon.ex @@ -172,10 +172,7 @@ def send_to_user(url, feed, poster) when is_binary(url) do poster.( url, feed, - [ - {"Content-Type", "application/magic-envelope+xml"}, - {"referer", Pleroma.Web.Endpoint.url()} - ] + [{"Content-Type", "application/magic-envelope+xml"}] ) do Instances.set_reachable(url) Logger.debug(fn -> "Pushed to #{url}, code #{code}" end) diff --git a/lib/pleroma/web/websub/websub.ex b/lib/pleroma/web/websub/websub.ex index abe148270..8f7d53b03 100644 --- a/lib/pleroma/web/websub/websub.ex +++ b/lib/pleroma/web/websub/websub.ex @@ -278,8 +278,7 @@ def publish_one(%{xml: xml, topic: topic, callback: callback, secret: secret}) d xml, [ {"Content-Type", "application/atom+xml"}, - {"X-Hub-Signature", "sha1=#{signature}"}, - {"referer", Pleroma.Web.Endpoint.url()} + {"X-Hub-Signature", "sha1=#{signature}"} ] ) do Instances.set_reachable(callback) diff --git a/lib/pleroma/web/websub/websub_controller.ex b/lib/pleroma/web/websub/websub_controller.ex index 9da7e70a1..a92dfe87b 100644 --- a/lib/pleroma/web/websub/websub_controller.ex +++ b/lib/pleroma/web/websub/websub_controller.ex @@ -20,8 +20,6 @@ defmodule Pleroma.Web.Websub.WebsubController do ] ) - plug(Pleroma.Web.Plugs.SetRequesterReachablePlug when action in [:websub_incoming]) - def websub_subscription_request(conn, %{"nickname" => nickname} = params) do user = User.get_cached_by_nickname(nickname) diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index eca5c134d..d3dd160dd 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/test/web/activity_pub/activity_pub_controller_test.exs @@ -145,17 +145,16 @@ test "it inserts an incoming activity into the database", %{conn: conn} do end test "it clears `unreachable` federation status of the sender", %{conn: conn} do - sender_url = "https://pleroma.soykaf.com" + data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!() + + sender_url = data["actor"] Instances.set_consistently_unreachable(sender_url) refute Instances.reachable?(sender_url) - data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!() - conn = conn |> assign(:valid_signature, true) |> put_req_header("content-type", "application/activity+json") - |> put_req_header("referer", sender_url) |> post("/inbox", data) assert "ok" == json_response(conn, 200) @@ -210,10 +209,6 @@ test "it returns a note activity in a collection", %{conn: conn} do end test "it clears `unreachable` federation status of the sender", %{conn: conn} do - sender_host = "pleroma.soykaf.com" - Instances.set_consistently_unreachable(sender_host) - refute Instances.reachable?(sender_host) - user = insert(:user) data = @@ -221,11 +216,14 @@ test "it clears `unreachable` federation status of the sender", %{conn: conn} do |> Poison.decode!() |> Map.put("bcc", [user.ap_id]) + sender_host = URI.parse(data["actor"]).host + Instances.set_consistently_unreachable(sender_host) + refute Instances.reachable?(sender_host) + conn = conn |> assign(:valid_signature, true) |> put_req_header("content-type", "application/activity+json") - |> put_req_header("referer", "https://#{sender_host}") |> post("/users/#{user.nickname}/inbox", data) assert "ok" == json_response(conn, 200) diff --git a/test/web/instances/instances_test.exs b/test/web/instances/instances_test.exs index a2fdf1019..adb8560a7 100644 --- a/test/web/instances/instances_test.exs +++ b/test/web/instances/instances_test.exs @@ -39,6 +39,11 @@ test "returns `true` for host / url marked unreachable for less than `reachabili assert Instances.reachable?(url) assert Instances.reachable?(URI.parse(url).host) end + + test "returns true on non-binary input" do + assert Instances.reachable?(nil) + assert Instances.reachable?(1) + end end describe "filter_reachable/1" do @@ -71,6 +76,19 @@ test "keeps reachable url or host reachable" do Instances.set_reachable(url) assert Instances.reachable?(url) end + + test "returns error status on non-binary input" do + assert {:error, _} = Instances.set_reachable(nil) + assert {:error, _} = Instances.set_reachable(1) + end + end + + # Note: implementation-specific (e.g. Instance) details of set_unreachable/1 should be tested in implementation-specific tests + describe "set_unreachable/1" do + test "returns error status on non-binary input" do + assert {:error, _} = Instances.set_unreachable(nil) + assert {:error, _} = Instances.set_unreachable(1) + end end describe "set_consistently_unreachable/1" do diff --git a/test/web/ostatus/incoming_documents/delete_handling_test.exs b/test/web/ostatus/incoming_documents/delete_handling_test.exs index c8fbff6cc..d97cd79f4 100644 --- a/test/web/ostatus/incoming_documents/delete_handling_test.exs +++ b/test/web/ostatus/incoming_documents/delete_handling_test.exs @@ -2,9 +2,16 @@ defmodule Pleroma.Web.OStatus.DeleteHandlingTest do use Pleroma.DataCase import Pleroma.Factory + import Tesla.Mock + alias Pleroma.{Repo, Activity, Object} alias Pleroma.Web.OStatus + setup do + mock(fn env -> apply(HttpRequestMock, :request, [env]) end) + :ok + end + describe "deletions" do test "it removes the mentioned activity" do note = insert(:note_activity) diff --git a/test/web/ostatus/ostatus_controller_test.exs b/test/web/ostatus/ostatus_controller_test.exs index cba12b3f7..3145ca9a1 100644 --- a/test/web/ostatus/ostatus_controller_test.exs +++ b/test/web/ostatus/ostatus_controller_test.exs @@ -5,7 +5,7 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do use Pleroma.Web.ConnCase import Pleroma.Factory - alias Pleroma.{User, Repo, Object, Instances} + alias Pleroma.{User, Repo, Object} alias Pleroma.Web.CommonAPI alias Pleroma.Web.OStatus.ActivityRepresenter @@ -59,24 +59,6 @@ test "decodes a salmon with a changed magic key", %{conn: conn} do assert response(conn, 200) end - - test "it clears `unreachable` federation status of the sender", %{conn: conn} do - sender_url = "https://pleroma.soykaf.com" - Instances.set_consistently_unreachable(sender_url) - refute Instances.reachable?(sender_url) - - user = insert(:user) - salmon = File.read!("test/fixtures/salmon.xml") - - conn = - conn - |> put_req_header("content-type", "application/atom+xml") - |> put_req_header("referer", sender_url) - |> post("/users/#{user.nickname}/salmon", salmon) - - assert response(conn, 200) - assert Instances.reachable?(sender_url) - end end test "gets a feed", %{conn: conn} do diff --git a/test/web/ostatus/ostatus_test.exs b/test/web/ostatus/ostatus_test.exs index 403cc7095..0c63dd84d 100644 --- a/test/web/ostatus/ostatus_test.exs +++ b/test/web/ostatus/ostatus_test.exs @@ -6,7 +6,7 @@ defmodule Pleroma.Web.OStatusTest do use Pleroma.DataCase alias Pleroma.Web.OStatus alias Pleroma.Web.XML - alias Pleroma.{Object, Repo, User, Activity} + alias Pleroma.{Object, Repo, User, Activity, Instances} import Pleroma.Factory import ExUnit.CaptureLog @@ -311,6 +311,22 @@ test "handle incoming unfollows with existing follow" do refute User.following?(follower, followed) end + test "it clears `unreachable` federation status of the sender" do + incoming_reaction_xml = File.read!("test/fixtures/share-gs.xml") + doc = XML.parse_document(incoming_reaction_xml) + actor_uri = XML.string_from_xpath("//author/uri[1]", doc) + reacted_to_author_uri = XML.string_from_xpath("//author/uri[2]", doc) + + Instances.set_consistently_unreachable(actor_uri) + Instances.set_consistently_unreachable(reacted_to_author_uri) + refute Instances.reachable?(actor_uri) + refute Instances.reachable?(reacted_to_author_uri) + + {:ok, _} = OStatus.handle_incoming(incoming_reaction_xml) + assert Instances.reachable?(actor_uri) + refute Instances.reachable?(reacted_to_author_uri) + end + describe "new remote user creation" do test "returns local users" do local_user = insert(:user) diff --git a/test/web/websub/websub_controller_test.exs b/test/web/websub/websub_controller_test.exs index cb19d6fe6..6492df2a0 100644 --- a/test/web/websub/websub_controller_test.exs +++ b/test/web/websub/websub_controller_test.exs @@ -6,7 +6,7 @@ defmodule Pleroma.Web.Websub.WebsubControllerTest do use Pleroma.Web.ConnCase import Pleroma.Factory alias Pleroma.Web.Websub.WebsubClientSubscription - alias Pleroma.{Repo, Activity, Instances} + alias Pleroma.{Repo, Activity} alias Pleroma.Web.Websub test "websub subscription request", %{conn: conn} do @@ -82,25 +82,5 @@ test "rejects incoming feed updates with the wrong signature", %{conn: conn} do assert length(Repo.all(Activity)) == 0 end - - test "it clears `unreachable` federation status of the sender", %{conn: conn} do - sender_url = "https://pleroma.soykaf.com" - Instances.set_consistently_unreachable(sender_url) - refute Instances.reachable?(sender_url) - - websub = insert(:websub_client_subscription) - doc = "some stuff" - signature = Websub.sign(websub.secret, doc) - - conn = - conn - |> put_req_header("x-hub-signature", "sha1=" <> signature) - |> put_req_header("content-type", "application/atom+xml") - |> put_req_header("referer", sender_url) - |> post("/push/subscriptions/#{websub.id}", doc) - - assert response(conn, 200) == "OK" - assert Instances.reachable?(sender_url) - end end end