From d472bafec19cee269e7c943bafae7c805785acd7 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 15:54:14 -0500 Subject: Mark instances as unreachable when returning a 403 from an object fetch This is a definite sign the instance is blocked and they are enforcing authorized_fetch --- test/pleroma/object/fetcher_test.exs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'test') diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 53c9277d6..80272946c 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -25,6 +25,9 @@ defmodule Pleroma.Object.FetcherTest do %{method: :get, url: "https://mastodon.example.org/users/userisgone404"} -> %Tesla.Env{status: 404} + %{method: :get, url: "https://octodon.social/users/cwebber/statuses/111647596861000656"} -> + %Tesla.Env{status: 403} + %{ method: :get, url: @@ -233,6 +236,18 @@ defmodule Pleroma.Object.FetcherTest do ) end + test "handle HTTP 403 response" do + object_id = "https://octodon.social/users/cwebber/statuses/111647596861000656" + Instances.set_reachable(object_id) + + assert Instances.reachable?(object_id) + + assert {:error, "Object fetch has been denied"} == + Fetcher.fetch_object_from_id(object_id) + + refute Instances.reachable?(object_id) + end + test "it can fetch pleroma polls with attachments" do {:ok, object} = Fetcher.fetch_object_from_id("https://patch.cx/objects/tesla_mock/poll_attachment") -- cgit v1.2.3 From 73c4c6d7de6d33c68cf663e65df8525ce8eef4f5 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 17:12:58 -0500 Subject: Revert "Mark instances as unreachable when returning a 403 from an object fetch" This reverts commit d472bafec19cee269e7c943bafae7c805785acd7. --- test/pleroma/object/fetcher_test.exs | 15 --------------- 1 file changed, 15 deletions(-) (limited to 'test') diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 80272946c..53c9277d6 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -25,9 +25,6 @@ defmodule Pleroma.Object.FetcherTest do %{method: :get, url: "https://mastodon.example.org/users/userisgone404"} -> %Tesla.Env{status: 404} - %{method: :get, url: "https://octodon.social/users/cwebber/statuses/111647596861000656"} -> - %Tesla.Env{status: 403} - %{ method: :get, url: @@ -236,18 +233,6 @@ defmodule Pleroma.Object.FetcherTest do ) end - test "handle HTTP 403 response" do - object_id = "https://octodon.social/users/cwebber/statuses/111647596861000656" - Instances.set_reachable(object_id) - - assert Instances.reachable?(object_id) - - assert {:error, "Object fetch has been denied"} == - Fetcher.fetch_object_from_id(object_id) - - refute Instances.reachable?(object_id) - end - test "it can fetch pleroma polls with attachments" do {:ok, object} = Fetcher.fetch_object_from_id("https://patch.cx/objects/tesla_mock/poll_attachment") -- cgit v1.2.3 From be0cca9afd8edf1e550f71f17914607c995b64f8 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 16:06:10 -0500 Subject: RemoteFetcherWorker Oban job tests --- .../pleroma/workers/remote_fetcher_worker_test.exs | 67 ++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 test/pleroma/workers/remote_fetcher_worker_test.exs (limited to 'test') diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs new file mode 100644 index 000000000..d8cb52f52 --- /dev/null +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -0,0 +1,67 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2023 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.RemoteFetcherWorkerTest do + use Pleroma.DataCase + use Oban.Testing, repo: Pleroma.Repo + + alias Pleroma.Instances + alias Pleroma.Workers.RemoteFetcherWorker + + @deleted_object_one "https://deleted-404.example.com/" + @deleted_object_two "https://deleted-410.example.com/" + @unauthorized_object "https://unauthorized.example.com/" + @unreachable_object "https://unreachable.example.com/" + + describe "it does not" do + setup do + Tesla.Mock.mock(fn + %{method: :get, url: @deleted_object_one} -> + %Tesla.Env{ + status: 404 + } + + %{method: :get, url: @deleted_object_two} -> + %Tesla.Env{ + status: 410 + } + + %{method: :get, url: @unauthorized_object} -> + %Tesla.Env{ + status: 403 + } + end) + end + + test "requeue a deleted object" do + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @deleted_object_one} + }) + + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @deleted_object_two} + }) + end + + test "requeue an unauthorized object" do + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @unauthorized_object} + }) + end + + test "fetch an unreachable instance" do + Instances.set_consistently_unreachable(@unreachable_object) + + refute Instances.reachable?(@unreachable_object) + + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @unreachable_object} + }) + end + end +end -- cgit v1.2.3 From becb070603bc65d5302698a2b6cf5f89b3d5c1f0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 20:47:18 -0500 Subject: Conslidate log messages for object fetcher failures and leverage Logger.metadata --- test/pleroma/web/activity_pub/transmogrifier_test.exs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'test') diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index 9c5983347..a49e459a6 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -132,7 +132,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert {:ok, activity} = Transmogrifier.handle_incoming(message) object = Object.normalize(activity) assert [%{"type" => "Mention"}, %{"type" => "Link"}] = object.data["tag"] - end) =~ "Error while fetching" + end) =~ "Object rejected while fetching" end test "it accepts quote posts" do @@ -410,7 +410,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert capture_log(fn -> {:error, _} = Transmogrifier.handle_incoming(data) - end) =~ "Object containment failed" + end) =~ "Object rejected while fetching" end test "it rejects activities which reference objects that have an incorrect attribution (variant 1)" do @@ -425,7 +425,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert capture_log(fn -> {:error, _} = Transmogrifier.handle_incoming(data) - end) =~ "Object containment failed" + end) =~ "Object rejected while fetching" end test "it rejects activities which reference objects that have an incorrect attribution (variant 2)" do @@ -440,7 +440,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert capture_log(fn -> {:error, _} = Transmogrifier.handle_incoming(data) - end) =~ "Object containment failed" + end) =~ "Object rejected while fetching" end end -- cgit v1.2.3 From a2708f7fe31c6009b0c9954e5de3b74c45f9818f Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 21:57:47 -0500 Subject: Leverage existing atoms as return errors for the object fetcher --- test/pleroma/object/fetcher_test.exs | 4 ++-- test/pleroma/web/twitter_api/remote_follow_controller_test.exs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 53c9277d6..24f114c00 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -220,14 +220,14 @@ defmodule Pleroma.Object.FetcherTest do end test "handle HTTP 410 Gone response" do - assert {:error, "Object has been deleted"} == + assert {:error, :not_found} == Fetcher.fetch_and_contain_remote_object_from_id( "https://mastodon.example.org/users/userisgone" ) end test "handle HTTP 404 response" do - assert {:error, "Object has been deleted"} == + assert {:error, :not_found} == Fetcher.fetch_and_contain_remote_object_from_id( "https://mastodon.example.org/users/userisgone404" ) diff --git a/test/pleroma/web/twitter_api/remote_follow_controller_test.exs b/test/pleroma/web/twitter_api/remote_follow_controller_test.exs index 41f8ebcd7..c6ecb53f4 100644 --- a/test/pleroma/web/twitter_api/remote_follow_controller_test.exs +++ b/test/pleroma/web/twitter_api/remote_follow_controller_test.exs @@ -137,7 +137,7 @@ defmodule Pleroma.Web.TwitterAPI.RemoteFollowControllerTest do |> html_response(200) assert response =~ "Error fetching user" - end) =~ "Object has been deleted" + end) =~ ":not_found" end end -- cgit v1.2.3 From ad0a5deb67f454b0529a4faf72399cd9ecc9c0e6 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 22:28:41 -0500 Subject: Prevent requeuing Remote Fetcher jobs that exceed thread depth --- test/pleroma/object/fetcher_test.exs | 3 +-- test/pleroma/workers/remote_fetcher_worker_test.exs | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 24f114c00..6f21452a7 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -101,8 +101,7 @@ defmodule Pleroma.Object.FetcherTest do test "it returns thread depth exceeded error if thread depth is exceeded" do clear_config([:instance, :federation_incoming_replies_max_depth], 0) - assert {:error, "Max thread distance exceeded."} = - Fetcher.fetch_object_from_id(@ap_id, depth: 1) + assert {:error, :allowed_depth} = Fetcher.fetch_object_from_id(@ap_id, depth: 1) end test "it fetches object if max thread depth is restricted to 0 and depth is not specified" do diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index d8cb52f52..df7d77ea0 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -13,6 +13,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do @deleted_object_two "https://deleted-410.example.com/" @unauthorized_object "https://unauthorized.example.com/" @unreachable_object "https://unreachable.example.com/" + @depth_object "https://depth.example.com/" describe "it does not" do setup do @@ -31,6 +32,11 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do %Tesla.Env{ status: 403 } + + %{method: :get, url: @depth_object} -> + %Tesla.Env{ + status: 200 + } end) end @@ -63,5 +69,14 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do args: %{"op" => "fetch_remote", "id" => @unreachable_object} }) end + + test "requeue an object that exceeded depth" do + clear_config([:instance, :federation_incoming_replies_max_depth], 0) + + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @depth_object, "depth" => 1} + }) + end end end -- cgit v1.2.3 From a6fd251e440ec3c6734f6b084c8a69f442bcebb0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 22:37:06 -0500 Subject: Improve test descriptions --- test/pleroma/workers/remote_fetcher_worker_test.exs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'test') diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index df7d77ea0..84899cc5f 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -15,7 +15,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do @unreachable_object "https://unreachable.example.com/" @depth_object "https://depth.example.com/" - describe "it does not" do + describe "RemoteFetcherWorker" do setup do Tesla.Mock.mock(fn %{method: :get, url: @deleted_object_one} -> @@ -40,7 +40,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do end) end - test "requeue a deleted object" do + test "does not requeue a deleted object" do assert {:cancel, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @deleted_object_one} @@ -52,14 +52,14 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do }) end - test "requeue an unauthorized object" do + test "does not requeue an unauthorized object" do assert {:cancel, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @unauthorized_object} }) end - test "fetch an unreachable instance" do + test "does not fetch an unreachable instance" do Instances.set_consistently_unreachable(@unreachable_object) refute Instances.reachable?(@unreachable_object) @@ -70,7 +70,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do }) end - test "requeue an object that exceeded depth" do + test "does not requeue an object that exceeded depth" do clear_config([:instance, :federation_incoming_replies_max_depth], 0) assert {:cancel, _} = -- cgit v1.2.3 From f17f92105bff555d2d372ff2ec053fe40fa1b41b Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 28 Dec 2023 23:52:05 -0500 Subject: Oban jobs should be discarded on permanent errors --- test/pleroma/workers/remote_fetcher_worker_test.exs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'test') diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index 84899cc5f..493b10fdc 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -41,19 +41,19 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do end test "does not requeue a deleted object" do - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @deleted_object_one} }) - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @deleted_object_two} }) end test "does not requeue an unauthorized object" do - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @unauthorized_object} }) @@ -64,7 +64,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do refute Instances.reachable?(@unreachable_object) - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @unreachable_object} }) @@ -73,7 +73,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do test "does not requeue an object that exceeded depth" do clear_config([:instance, :federation_incoming_replies_max_depth], 0) - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @depth_object, "depth" => 1} }) -- cgit v1.2.3 From 12c052551bcd6b7871ccde5b9228315b89f45e01 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sun, 14 Jan 2024 13:23:17 -0500 Subject: Allow the Remote Fetcher to attempt fetching an unreachable instance --- test/pleroma/workers/remote_fetcher_worker_test.exs | 13 ------------- 1 file changed, 13 deletions(-) (limited to 'test') diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index 493b10fdc..c30e773d4 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -6,13 +6,11 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do use Pleroma.DataCase use Oban.Testing, repo: Pleroma.Repo - alias Pleroma.Instances alias Pleroma.Workers.RemoteFetcherWorker @deleted_object_one "https://deleted-404.example.com/" @deleted_object_two "https://deleted-410.example.com/" @unauthorized_object "https://unauthorized.example.com/" - @unreachable_object "https://unreachable.example.com/" @depth_object "https://depth.example.com/" describe "RemoteFetcherWorker" do @@ -59,17 +57,6 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do }) end - test "does not fetch an unreachable instance" do - Instances.set_consistently_unreachable(@unreachable_object) - - refute Instances.reachable?(@unreachable_object) - - assert {:discard, _} = - RemoteFetcherWorker.perform(%Oban.Job{ - args: %{"op" => "fetch_remote", "id" => @unreachable_object} - }) - end - test "does not requeue an object that exceeded depth" do clear_config([:instance, :federation_incoming_replies_max_depth], 0) -- cgit v1.2.3