Merge remote-tracking branch 'origin/develop' into fork

Signed-off-by: marcin mikołajczak <git@mkljczk.pl>

changelog.d/ldap-error-logging.change

@@ -0,0 +1 @@
+Improve error logging when LDAP authentication fails.

changelog.d/oban-fetcher-rejected.change

@@ -0,0 +1 @@
+Discard Remote Fetcher jobs which errored due to an MRF rejection

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,

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
 

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
 

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

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},

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))

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}
                })