Merge pull request 'Magical patches' (#357) from magical-patches into develop

Reviewed-on: AkkomaGang/akkoma#357

CHANGELOG.md

@@ -13,6 +13,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Regular task to prune local transient activities
 - Task to manually run the transient prune job (pleroma.database prune\_task)
 - Ability to follow hashtags
+- Option to extend `reject` in MRF-Simple to apply to entire threads, where the originating instance is rejected
+- Extra information to failed HTTP requests
 
 ## Changed
 - MastoAPI: Accept BooleanLike input on `/api/v1/accounts/:id/follow` (fixes follows with mastodon.py)
@@ -22,6 +24,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Transient activities recieved from remote servers are no longer persisted in the database
 - Overhauled static-fe view for logged-out users
 
+## Removed
+- FollowBotPolicy
+- Passing of undo/block into MRF
+
 ## Upgrade Notes
 - If you have an old instance, you will probably want to run `mix pleroma.database prune_task` in the foreground to catch it up with the history of your instance.
 

config/config.exs

@@ -391,7 +391,8 @@
   accept: [],
   avatar_removal: [],
   banner_removal: [],
-  reject_deletes: []
+  reject_deletes: [],
+  handle_threads: true
 
 config :pleroma, :mrf_keyword,
   reject: [],

docs/docs/configuration/cheatsheet.md

@@ -221,11 +221,6 @@ Notes:
 - The hashtags in the configuration do not have a leading `#`.
 - This MRF Policy is always enabled, if you want to disable it you have to set empty lists
 
-#### :mrf_follow_bot
-
-* `follower_nickname`: The name of the bot account to use for following newly discovered users. Using `followbot` or similar is strongly suggested.
-
-
 ### :activitypub
 * `unfollow_blocked`: Whether blocks result in people getting unfollowed
 * `outgoing_blocks`: Whether to federate blocks to other instances

lib/pleroma/collections/fetcher.ex

@@ -68,7 +68,7 @@ defp fetch_page_items(id, items \\ []) do
           items
         end
       else
-        {:error, "Object has been deleted"} ->
+        {:error, {"Object has been deleted", _, _}} ->
           items
 
         {:error, error} ->

lib/pleroma/config/deprecation_warnings.ex

@@ -25,7 +25,7 @@ defmodule Pleroma.Config.DeprecationWarnings do
   def check_simple_policy_tuples do
     has_strings =
       Config.get([:mrf_simple])
-      |> Enum.any?(fn {_, v} -> Enum.any?(v, &is_binary/1) end)
+      |> Enum.any?(fn {_, v} -> is_list(v) and Enum.any?(v, &is_binary/1) end)
 
     if has_strings do
       Logger.warn("""
@@ -66,6 +66,7 @@ def check_simple_policy_tuples do
 
       new_config =
         Config.get([:mrf_simple])
+        |> Enum.filter(fn {_k, v} -> not is_atom(v) end)
         |> Enum.map(fn {k, v} ->
           {k,
            Enum.map(v, fn

lib/pleroma/object/fetcher.ex

@@ -180,7 +180,7 @@ def fetch_object_from_id!(id, options \\ []) do
       {:error, %Tesla.Mock.Error{}} ->
         nil
 
-      {:error, "Object has been deleted"} ->
+      {:error, {"Object has been deleted", _id, _code}} ->
         nil
 
       {:reject, reason} ->
@@ -284,7 +284,7 @@ defp get_object(id) do
         end
 
       {:ok, %{status: code}} when code in [404, 410] ->
-        {:error, "Object has been deleted"}
+        {:error, {"Object has been deleted", id, code}}
 
       {:error, e} ->
         {:error, e}

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -1711,7 +1711,7 @@ def fetch_and_prepare_user_from_ap_id(ap_id, additional \\ []) do
       {:ok, maybe_update_follow_information(data)}
     else
       # If this has been deleted, only log a debug and not an error
-      {:error, "Object has been deleted" = e} ->
+      {:error, {"Object has been deleted" = e, _, _}} ->
         Logger.debug("Could not decode user at fetch #{ap_id}, #{inspect(e)}")
         {:error, e}
 

lib/pleroma/web/activity_pub/mrf.ex

@@ -63,6 +63,12 @@ defmodule Pleroma.Web.ActivityPub.MRF do
 
   @required_description_keys [:key, :related_policy]
 
+  def filter_one(policy, %{"type" => type} = message)
+      when type in ["Undo", "Block", "Delete"] and
+             policy != Pleroma.Web.ActivityPub.MRF.SimplePolicy do
+    {:ok, message}
+  end
+
   def filter_one(policy, message) do
     should_plug_history? =
       if function_exported?(policy, :history_awareness, 0) do

lib/pleroma/web/activity_pub/mrf/follow_bot_policy.ex

@@ -1,59 +0,0 @@
-defmodule Pleroma.Web.ActivityPub.MRF.FollowBotPolicy do
-  @behaviour Pleroma.Web.ActivityPub.MRF.Policy
-  alias Pleroma.Config
-  alias Pleroma.User
-  alias Pleroma.Web.CommonAPI
-
-  require Logger
-
-  @impl true
-  def filter(message) do
-    with follower_nickname <- Config.get([:mrf_follow_bot, :follower_nickname]),
-         %User{actor_type: "Service"} = follower <-
-           User.get_cached_by_nickname(follower_nickname),
-         %{"type" => "Create", "object" => %{"type" => "Note"}} <- message do
-      try_follow(follower, message)
-    else
-      nil ->
-        Logger.warn(
-          "#{__MODULE__} skipped because of missing `:mrf_follow_bot, :follower_nickname` configuration, the :follower_nickname
-            account does not exist, or the account is not correctly configured as a bot."
-        )
-
-        {:ok, message}
-
-      _ ->
-        {:ok, message}
-    end
-  end
-
-  defp try_follow(follower, message) do
-    to = Map.get(message, "to", [])
-    cc = Map.get(message, "cc", [])
-    actor = [message["actor"]]
-
-    Enum.concat([to, cc, actor])
-    |> List.flatten()
-    |> Enum.uniq()
-    |> User.get_all_by_ap_id()
-    |> Enum.each(fn user ->
-      with false <- user.local,
-           false <- User.following?(follower, user),
-           false <- User.locked?(user),
-           false <- (user.bio || "") |> String.downcase() |> String.contains?("nobot") do
-        Logger.debug(
-          "#{__MODULE__}: Follow request from #{follower.nickname} to #{user.nickname}"
-        )
-
-        CommonAPI.follow(follower, user)
-      end
-    end)
-
-    {:ok, message}
-  end
-
-  @impl true
-  def describe do
-    {:ok, %{}}
-  end
-end

lib/pleroma/web/activity_pub/mrf/simple_policy.ex

@@ -13,20 +13,20 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicy do
 
   require Pleroma.Constants
 
-  defp check_accept(%{host: actor_host} = _actor_info, object) do
+  defp check_accept(%{host: actor_host} = _actor_info) do
     accepts =
       instance_list(:accept)
       |> MRF.subdomains_regex()
 
     cond do
-      accepts == [] -> {:ok, object}
+      accepts == [] -> {:ok, nil}
-      actor_host == Config.get([Pleroma.Web.Endpoint, :url, :host]) -> {:ok, object}
+      actor_host == Config.get([Pleroma.Web.Endpoint, :url, :host]) -> {:ok, nil}
-      MRF.subdomain_match?(accepts, actor_host) -> {:ok, object}
+      MRF.subdomain_match?(accepts, actor_host) -> {:ok, nil}
       true -> {:reject, "[SimplePolicy] host not in accept list"}
     end
   end
 
-  defp check_reject(%{host: actor_host} = _actor_info, object) do
+  defp check_reject(%{host: actor_host} = _actor_info) do
     rejects =
       instance_list(:reject)
       |> MRF.subdomains_regex()
@@ -34,7 +34,7 @@ defp check_reject(%{host: actor_host} = _actor_info, object) do
     if MRF.subdomain_match?(rejects, actor_host) do
       {:reject, "[SimplePolicy] host in reject list"}
     else
-      {:ok, object}
+      {:ok, nil}
     end
   end
 
@@ -178,6 +178,55 @@ defp check_banner_removal(%{host: actor_host} = _actor_info, %{"image" => _image
 
   defp check_banner_removal(_actor_info, object), do: {:ok, object}
 
+  defp extract_context_uri(%{"conversation" => "tag:" <> rest}) do
+    rest
+    |> String.split(",", parts: 2, trim: true)
+    |> hd()
+    |> case do
+      nil -> nil
+      hostname -> URI.parse("//" <> hostname)
+    end
+  end
+
+  defp extract_context_uri(%{"context" => "http" <> _ = context}), do: URI.parse(context)
+
+  defp extract_context_uri(_), do: nil
+
+  defp check_context(activity) do
+    uri = extract_context_uri(activity)
+
+    with {:uri, true} <- {:uri, Kernel.match?(%URI{}, uri)},
+         {:ok, _} <- check_accept(uri),
+         {:ok, _} <- check_reject(uri) do
+      {:ok, activity}
+    else
+      # Can't check.
+      {:uri, false} -> {:ok, activity}
+      {:reject, nil} -> {:reject, "[SimplePolicy]"}
+      {:reject, _} = e -> e
+      _ -> {:reject, "[SimplePolicy]"}
+    end
+  end
+
+  defp check_reply_to(%{"object" => %{"inReplyTo" => in_reply_to}} = activity) do
+    with {:ok, _} <- filter(in_reply_to) do
+      {:ok, activity}
+    end
+  end
+
+  defp check_reply_to(activity), do: {:ok, activity}
+
+  defp maybe_check_thread(activity) do
+    if Config.get([:mrf_simple, :handle_threads], true) do
+      with {:ok, _} <- check_context(activity),
+           {:ok, _} <- check_reply_to(activity) do
+        {:ok, activity}
+      end
+    else
+      {:ok, activity}
+    end
+  end
+
   defp check_object(%{"object" => object} = activity) do
     with {:ok, _object} <- filter(object) do
       {:ok, activity}
@@ -210,13 +259,14 @@ def filter(%{"type" => "Delete", "actor" => actor} = object) do
   def filter(%{"actor" => actor} = object) do
     actor_info = URI.parse(actor)
 
-    with {:ok, object} <- check_accept(actor_info, object),
+    with {:ok, _} <- check_accept(actor_info),
-         {:ok, object} <- check_reject(actor_info, object),
+         {:ok, _} <- check_reject(actor_info),
          {:ok, object} <- check_media_removal(actor_info, object),
          {:ok, object} <- check_media_nsfw(actor_info, object),
          {:ok, object} <- check_ftl_removal(actor_info, object),
          {:ok, object} <- check_followers_only(actor_info, object),
          {:ok, object} <- check_report_removal(actor_info, object),
+         {:ok, object} <- maybe_check_thread(object),
          {:ok, object} <- check_object(object) do
       {:ok, object}
     else
@@ -230,8 +280,8 @@ def filter(%{"id" => actor, "type" => obj_type} = object)
       when obj_type in ["Application", "Group", "Organization", "Person", "Service"] do
     actor_info = URI.parse(actor)
 
-    with {:ok, object} <- check_accept(actor_info, object),
+    with {:ok, _} <- check_accept(actor_info),
-         {:ok, object} <- check_reject(actor_info, object),
+         {:ok, _} <- check_reject(actor_info),
          {:ok, object} <- check_avatar_removal(actor_info, object),
          {:ok, object} <- check_banner_removal(actor_info, object) do
       {:ok, object}
@@ -242,11 +292,17 @@ def filter(%{"id" => actor, "type" => obj_type} = object)
     end
   end
 
+  def filter(%{"id" => id} = object) do
+    with {:ok, _} <- filter(id) do
+      {:ok, object}
+    end
+  end
+
   def filter(object) when is_binary(object) do
     uri = URI.parse(object)
 
-    with {:ok, object} <- check_accept(uri, object),
+    with {:ok, _} <- check_accept(uri),
-         {:ok, object} <- check_reject(uri, object) do
+         {:ok, _} <- check_reject(uri) do
       {:ok, object}
     else
       {:reject, nil} -> {:reject, "[SimplePolicy]"}
@@ -288,6 +344,7 @@ def describe do
 
     mrf_simple_excluded =
       Config.get(:mrf_simple)
+      |> Enum.filter(fn {_, v} -> is_list(v) end)
       |> Enum.map(fn {rule, instances} ->
         {rule, Enum.reject(instances, fn {host, _} -> host in exclusions end)}
       end)
@@ -332,66 +389,78 @@ def config_description do
       label: "MRF Simple",
       description: "Simple ingress policies",
       children:
-        [
+        ([
-          %{
+           %{
-            key: :media_removal,
+             key: :media_removal,
-            description:
+             description:
-              "List of instances to strip media attachments from and the reason for doing so"
+               "List of instances to strip media attachments from and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :media_nsfw,
+             key: :media_nsfw,
-            label: "Media NSFW",
+             label: "Media NSFW",
-            description:
+             description:
-              "List of instances to tag all media as NSFW (sensitive) from and the reason for doing so"
+               "List of instances to tag all media as NSFW (sensitive) from and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :federated_timeline_removal,
+             key: :federated_timeline_removal,
-            description:
+             description:
-              "List of instances to remove from the Federated (aka The Whole Known Network) Timeline and the reason for doing so"
+               "List of instances to remove from the Federated (aka The Whole Known Network) Timeline and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :reject,
+             key: :reject,
-            description:
+             description:
-              "List of instances to reject activities from (except deletes) and the reason for doing so"
+               "List of instances to reject activities from (except deletes) and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :accept,
+             key: :accept,
-            description:
+             description:
-              "List of instances to only accept activities from (except deletes) and the reason for doing so"
+               "List of instances to only accept activities from (except deletes) and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :followers_only,
+             key: :followers_only,
-            description:
+             description:
-              "Force posts from the given instances to be visible by followers only and the reason for doing so"
+               "Force posts from the given instances to be visible by followers only and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :report_removal,
+             key: :report_removal,
-            description: "List of instances to reject reports from and the reason for doing so"
+             description: "List of instances to reject reports from and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :avatar_removal,
+             key: :avatar_removal,
-            description: "List of instances to strip avatars from and the reason for doing so"
+             description: "List of instances to strip avatars from and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :banner_removal,
+             key: :banner_removal,
-            description: "List of instances to strip banners from and the reason for doing so"
+             description: "List of instances to strip banners from and the reason for doing so"
-          },
+           },
-          %{
+           %{
-            key: :reject_deletes,
+             key: :reject_deletes,
-            description: "List of instances to reject deletions from and the reason for doing so"
+             description: "List of instances to reject deletions from and the reason for doing so"
-          }
+           }
-        ]
+         ]
-        |> Enum.map(fn setting ->
+         |> Enum.map(fn setting ->
-          Map.merge(
+           Map.merge(
-            setting,
+             setting,
+             %{
+               type: {:list, :tuple},
+               key_placeholder: "instance",
+               value_placeholder: "reason",
+               suggestions: [
+                 {"example.com", "Some reason"},
+                 {"*.example.com", "Another reason"}
+               ]
+             }
+           )
+         end)) ++
+          [
             %{
-              type: {:list, :tuple},
+              key: :handle_threads,
-              key_placeholder: "instance",
+              label: "Apply to entire threads",
-              value_placeholder: "reason",
+              type: :boolean,
-              suggestions: [{"example.com", "Some reason"}, {"*.example.com", "Another reason"}]
+              description:
+                "Enable to filter replies to threads based from their originating instance, using the reject and accept rules"
             }
-          )
+          ]
-        end)
     }
   end
 end

test/pleroma/object/fetcher_test.exs

@@ -216,14 +216,16 @@ test "all objects with fake directions are rejected by the object fetcher" do
     end
 
     test "handle HTTP 410 Gone response" do
-      assert {:error, "Object has been deleted"} ==
+      assert {:error,
+              {"Object has been deleted", "https://mastodon.example.org/users/userisgone", 410}} ==
                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,
+              {"Object has been deleted", "https://mastodon.example.org/users/userisgone404", 404}} ==
                Fetcher.fetch_and_contain_remote_object_from_id(
                  "https://mastodon.example.org/users/userisgone404"
                )

test/pleroma/web/activity_pub/mrf/follow_bot_policy_test.exs

@@ -1,126 +0,0 @@
-# Pleroma: A lightweight social networking server
-# Copyright © 2017-2021 Pleroma Authors <https://pleroma.social/>
-# SPDX-License-Identifier: AGPL-3.0-only
-
-defmodule Pleroma.Web.ActivityPub.MRF.FollowBotPolicyTest do
-  use Pleroma.DataCase, async: true
-
-  alias Pleroma.User
-  alias Pleroma.Web.ActivityPub.MRF.FollowBotPolicy
-
-  import Pleroma.Factory
-
-  describe "FollowBotPolicy" do
-    test "follows remote users" do
-      bot = insert(:user, actor_type: "Service")
-      remote_user = insert(:user, local: false)
-      clear_config([:mrf_follow_bot, :follower_nickname], bot.nickname)
-
-      message = %{
-        "@context" => "https://www.w3.org/ns/activitystreams",
-        "to" => [remote_user.follower_address],
-        "cc" => ["https://www.w3.org/ns/activitystreams#Public"],
-        "type" => "Create",
-        "object" => %{
-          "content" => "Test post",
-          "type" => "Note",
-          "attributedTo" => remote_user.ap_id,
-          "inReplyTo" => nil
-        },
-        "actor" => remote_user.ap_id
-      }
-
-      refute User.following?(bot, remote_user)
-
-      assert User.get_follow_requests(remote_user) |> length == 0
-
-      FollowBotPolicy.filter(message)
-
-      assert User.get_follow_requests(remote_user) |> length == 1
-    end
-
-    test "does not follow users with #nobot in bio" do
-      bot = insert(:user, actor_type: "Service")
-      remote_user = insert(:user, %{local: false, bio: "go away bots! #nobot"})
-      clear_config([:mrf_follow_bot, :follower_nickname], bot.nickname)
-
-      message = %{
-        "@context" => "https://www.w3.org/ns/activitystreams",
-        "to" => [remote_user.follower_address],
-        "cc" => ["https://www.w3.org/ns/activitystreams#Public"],
-        "type" => "Create",
-        "object" => %{
-          "content" => "I don't like follow bots",
-          "type" => "Note",
-          "attributedTo" => remote_user.ap_id,
-          "inReplyTo" => nil
-        },
-        "actor" => remote_user.ap_id
-      }
-
-      refute User.following?(bot, remote_user)
-
-      assert User.get_follow_requests(remote_user) |> length == 0
-
-      FollowBotPolicy.filter(message)
-
-      assert User.get_follow_requests(remote_user) |> length == 0
-    end
-
-    test "does not follow local users" do
-      bot = insert(:user, actor_type: "Service")
-      local_user = insert(:user, local: true)
-      clear_config([:mrf_follow_bot, :follower_nickname], bot.nickname)
-
-      message = %{
-        "@context" => "https://www.w3.org/ns/activitystreams",
-        "to" => [local_user.follower_address],
-        "cc" => ["https://www.w3.org/ns/activitystreams#Public"],
-        "type" => "Create",
-        "object" => %{
-          "content" => "Hi I'm a local user",
-          "type" => "Note",
-          "attributedTo" => local_user.ap_id,
-          "inReplyTo" => nil
-        },
-        "actor" => local_user.ap_id
-      }
-
-      refute User.following?(bot, local_user)
-
-      assert User.get_follow_requests(local_user) |> length == 0
-
-      FollowBotPolicy.filter(message)
-
-      assert User.get_follow_requests(local_user) |> length == 0
-    end
-
-    test "does not follow users requiring follower approval" do
-      bot = insert(:user, actor_type: "Service")
-      remote_user = insert(:user, %{local: false, is_locked: true})
-      clear_config([:mrf_follow_bot, :follower_nickname], bot.nickname)
-
-      message = %{
-        "@context" => "https://www.w3.org/ns/activitystreams",
-        "to" => [remote_user.follower_address],
-        "cc" => ["https://www.w3.org/ns/activitystreams#Public"],
-        "type" => "Create",
-        "object" => %{
-          "content" => "I don't like randos following me",
-          "type" => "Note",
-          "attributedTo" => remote_user.ap_id,
-          "inReplyTo" => nil
-        },
-        "actor" => remote_user.ap_id
-      }
-
-      refute User.following?(bot, remote_user)
-
-      assert User.get_follow_requests(remote_user) |> length == 0
-
-      FollowBotPolicy.filter(message)
-
-      assert User.get_follow_requests(remote_user) |> length == 0
-    end
-  end
-end

test/pleroma/web/activity_pub/mrf/simple_policy_test.exs

@@ -356,6 +356,86 @@ test "reject by URI object" do
 
       assert {:reject, _} = SimplePolicy.filter(announce)
     end
+
+    test "accept by matching context URI if :handle_threads is disabled" do
+      clear_config([:mrf_simple, :reject], [{"blocked.tld", ""}])
+      clear_config([:mrf_simple, :handle_threads], false)
+
+      remote_message =
+        build_remote_message()
+        |> Map.put("context", "https://blocked.tld/contexts/abc")
+
+      assert {:ok, _} = SimplePolicy.filter(remote_message)
+    end
+
+    test "accept by matching conversation field if :handle_threads is disabled" do
+      clear_config([:mrf_simple, :reject], [{"blocked.tld", ""}])
+      clear_config([:mrf_simple, :handle_threads], false)
+
+      remote_message =
+        build_remote_message()
+        |> Map.put(
+          "conversation",
+          "tag:blocked.tld,1997-06-25:objectId=12345:objectType=Conversation"
+        )
+
+      assert {:ok, _} = SimplePolicy.filter(remote_message)
+    end
+
+    test "accept by matching reply ID if :handle_threads is disabled" do
+      clear_config([:mrf_simple, :reject], [{"blocked.tld", ""}])
+      clear_config([:mrf_simple, :handle_threads], false)
+
+      remote_message =
+        build_remote_message()
+        |> Map.put("type", "Create")
+        |> Map.put("object", %{
+          "type" => "Note",
+          "inReplyTo" => "https://blocked.tld/objects/1"
+        })
+
+      assert {:ok, _} = SimplePolicy.filter(remote_message)
+    end
+
+    test "reject by matching context URI if :handle_threads is enabled" do
+      clear_config([:mrf_simple, :reject], [{"blocked.tld", ""}])
+      clear_config([:mrf_simple, :handle_threads], true)
+
+      remote_message =
+        build_remote_message()
+        |> Map.put("context", "https://blocked.tld/contexts/abc")
+
+      assert {:reject, _} = SimplePolicy.filter(remote_message)
+    end
+
+    test "reject by matching conversation field if :handle_threads is enabled" do
+      clear_config([:mrf_simple, :reject], [{"blocked.tld", ""}])
+      clear_config([:mrf_simple, :handle_threads], true)
+
+      remote_message =
+        build_remote_message()
+        |> Map.put(
+          "conversation",
+          "tag:blocked.tld,1997-06-25:objectId=12345:objectType=Conversation"
+        )
+
+      assert {:reject, _} = SimplePolicy.filter(remote_message)
+    end
+
+    test "reject by matching reply ID if :handle_threads is enabled" do
+      clear_config([:mrf_simple, :reject], [{"blocked.tld", ""}])
+      clear_config([:mrf_simple, :handle_threads], true)
+
+      remote_message =
+        build_remote_message()
+        |> Map.put("type", "Create")
+        |> Map.put("object", %{
+          "type" => "Note",
+          "inReplyTo" => "https://blocked.tld/objects/1"
+        })
+
+      assert {:reject, _} = SimplePolicy.filter(remote_message)
+    end
   end
 
   describe "when :followers_only" do