diff --git a/changelog.d/deps-poison-test-only.skip b/changelog.d/deps-poison-test-only.skip new file mode 100644 index 0000000000..e69de29bb2 diff --git a/changelog.d/ingestion-queue.skip b/changelog.d/ingestion-queue.skip new file mode 100644 index 0000000000..e69de29bb2 diff --git a/changelog.d/ldap-error-logging.change b/changelog.d/ldap-error-logging.change new file mode 100644 index 0000000000..56f0e7fc31 --- /dev/null +++ b/changelog.d/ldap-error-logging.change @@ -0,0 +1 @@ +Improve error logging when LDAP authentication fails. diff --git a/changelog.d/oban-deprecated-discards.skip b/changelog.d/oban-deprecated-discards.skip new file mode 100644 index 0000000000..e69de29bb2 diff --git a/changelog.d/oban-fetcher-rejected.change b/changelog.d/oban-fetcher-rejected.change new file mode 100644 index 0000000000..92e0836479 --- /dev/null +++ b/changelog.d/oban-fetcher-rejected.change @@ -0,0 +1 @@ +Discard Remote Fetcher jobs which errored due to an MRF rejection diff --git a/config/config.exs b/config/config.exs index b3f70ab4c9..b25fc5975b 100644 --- a/config/config.exs +++ b/config/config.exs @@ -597,7 +597,6 @@ activity_expiration: 10, federator_incoming: 5, federator_outgoing: 5, - ingestion_queue: 50, web_push: 50, transmogrifier: 20, notifications: 20, diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index a42b4844e8..90c4274f27 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -123,9 +123,9 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa Logger.error("Publisher failed to inbox #{inbox} with status #{code}") case response do - %{status: 403} -> {:discard, :forbidden} - %{status: 404} -> {:discard, :not_found} - %{status: 410} -> {:discard, :not_found} + %{status: 403} -> {:cancel, :forbidden} + %{status: 404} -> {:cancel, :not_found} + %{status: 410} -> {:cancel, :not_found} _ -> {:error, e} end diff --git a/lib/pleroma/web/auth/ldap_authenticator.ex b/lib/pleroma/web/auth/ldap_authenticator.ex index c2c5eb1e5a..ea5620cf60 100644 --- a/lib/pleroma/web/auth/ldap_authenticator.ex +++ b/lib/pleroma/web/auth/ldap_authenticator.ex @@ -91,7 +91,8 @@ defp bind_user(connection, ldap, name, password) do end error -> - error + Logger.error("Could not bind LDAP user #{name}: #{inspect(error)}") + {:error, {:ldap_bind_error, error}} end end @@ -111,7 +112,8 @@ defp register_user(connection, base, uid, name) do try_register(name, attributes) error -> - error + Logger.error("Couldn't register user because LDAP search failed: #{inspect(error)}") + {:error, {:ldap_search_error, error}} end end diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index ed04c54b2b..6d777ae946 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -13,14 +13,17 @@ def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do {:ok, _object} -> :ok + {:rejected, reason} -> + {:cancel, reason} + {:error, :forbidden} -> - {:discard, :forbidden} + {:cancel, :forbidden} {:error, :not_found} -> - {:discard, :not_found} + {:cancel, :not_found} {:error, :allowed_depth} -> - {:discard, :allowed_depth} + {:cancel, :allowed_depth} {:error, _} = e -> e diff --git a/mix.exs b/mix.exs index 2d3819a3a5..5ec646c477 100644 --- a/mix.exs +++ b/mix.exs @@ -155,7 +155,6 @@ defp deps do {:calendar, "~> 1.0"}, {:cachex, "~> 3.2"}, {:csv, "~> 2.4"}, - {:poison, "~> 3.0", override: true}, {:tesla, "~> 1.11"}, {:castore, "~> 0.1"}, {:cowlib, "~> 2.9", override: true}, @@ -210,6 +209,7 @@ defp deps do ## dev & test {:phoenix_live_reload, "~> 1.3.3", only: :dev}, + {:poison, "~> 3.0", only: :test}, {:ex_doc, "~> 0.22", only: :dev, runtime: false}, {:ex_machina, "~> 2.4", only: :test}, {:credo, "~> 1.6", only: [:dev, :test], runtime: false}, diff --git a/test/pleroma/web/activity_pub/publisher_test.exs b/test/pleroma/web/activity_pub/publisher_test.exs index 150b7bb596..6f48a02279 100644 --- a/test/pleroma/web/activity_pub/publisher_test.exs +++ b/test/pleroma/web/activity_pub/publisher_test.exs @@ -223,7 +223,7 @@ test "publish to url with with different ports" do actor = insert(:user) inbox = "http://404.site/users/nick1/inbox" - assert {:discard, _} = + assert {:cancel, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) assert called(Instances.set_unreachable(inbox)) diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index c30e773d46..2104baab2f 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -39,19 +39,19 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do end test "does not requeue a deleted object" do - assert {:discard, _} = + assert {:cancel, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @deleted_object_one} }) - assert {:discard, _} = + assert {:cancel, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @deleted_object_two} }) end test "does not requeue an unauthorized object" do - assert {:discard, _} = + assert {:cancel, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @unauthorized_object} }) @@ -60,7 +60,7 @@ test "does not requeue an unauthorized object" do test "does not requeue an object that exceeded depth" do clear_config([:instance, :federation_incoming_replies_max_depth], 0) - assert {:discard, _} = + assert {:cancel, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @depth_object, "depth" => 1} })