From 1d727cd0691fdedcd78d5ef22c07a1b280531037 Mon Sep 17 00:00:00 2001 From: Karen Konou Date: Tue, 12 Feb 2019 23:25:09 +0100 Subject: [PATCH 1/5] added checks for public url and follower collections --- .../web/activity_pub/mrf/hellthread_policy.ex | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex index 4c6e612b2..9be382972 100644 --- a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex @@ -14,6 +14,23 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do |> Map.put("cc", ["https://www.w3.org/ns/activitystreams#Public"]) end + defp get_recipient_count(message) do + recipients = (message["to"] || []) ++ (message["cc"] || []) + + cond do + Enum.member?(recipients, "https://www.w3.org/ns/activitystreams#Public") && + Enum.find(recipients, &String.ends_with?(&1, "/followers")) -> + length(recipients) - 2 + + Enum.member?(recipients, "https://www.w3.org/ns/activitystreams#Public") || + Enum.find(recipients, &String.ends_with?(&1, "/followers")) -> + length(recipients) - 1 + + true -> + length(recipients) + end + end + @impl true def filter(%{"type" => "Create"} = message) do delist_threshold = Pleroma.Config.get([:mrf_hellthread, :delist_threshold]) @@ -24,13 +41,13 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do Pleroma.Config.get([:mrf_hellthread, :threshold]) ) - recipients = (message["to"] || []) ++ (message["cc"] || []) + recipients = get_recipient_count(message) cond do - length(recipients) > reject_threshold and reject_threshold > 0 -> + recipients > reject_threshold and reject_threshold > 0 -> {:reject, nil} - length(recipients) > delist_threshold and delist_threshold > 0 -> + recipients > delist_threshold and delist_threshold > 0 -> if Enum.member?(message["to"], "https://www.w3.org/ns/activitystreams#Public") or Enum.member?(message["cc"], "https://www.w3.org/ns/activitystreams#Public") do {:ok, delist_message(message)} From bef9b9cb669d6c1081eb83c624af89dbf9a8b9da Mon Sep 17 00:00:00 2001 From: Karen Konou Date: Wed, 13 Feb 2019 16:23:09 +0100 Subject: [PATCH 2/5] refactored code --- .../web/activity_pub/mrf/hellthread_policy.ex | 86 +++++++++++-------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex index 9be382972..1f09b4e66 100644 --- a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex @@ -7,56 +7,66 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do @behaviour Pleroma.Web.ActivityPub.MRF defp delist_message(message) do + delist_threshold = Pleroma.Config.get([:mrf_hellthread, :delist_threshold]) follower_collection = User.get_cached_by_ap_id(message["actor"]).follower_address - message - |> Map.put("to", [follower_collection]) - |> Map.put("cc", ["https://www.w3.org/ns/activitystreams#Public"]) + message = + with {:public, recipients} <- get_recipient_count(message) do + if recipients > delist_threshold and delist_threshold > 0 do + message + |> Map.put("to", [follower_collection]) + |> Map.put("cc", ["https://www.w3.org/ns/activitystreams#Public"]) + else + message + end + else + _ -> message + end + + {:ok, message} end - defp get_recipient_count(message) do - recipients = (message["to"] || []) ++ (message["cc"] || []) - - cond do - Enum.member?(recipients, "https://www.w3.org/ns/activitystreams#Public") && - Enum.find(recipients, &String.ends_with?(&1, "/followers")) -> - length(recipients) - 2 - - Enum.member?(recipients, "https://www.w3.org/ns/activitystreams#Public") || - Enum.find(recipients, &String.ends_with?(&1, "/followers")) -> - length(recipients) - 1 - - true -> - length(recipients) - end - end - - @impl true - def filter(%{"type" => "Create"} = message) do - delist_threshold = Pleroma.Config.get([:mrf_hellthread, :delist_threshold]) - + defp reject_message(message) do reject_threshold = Pleroma.Config.get( [:mrf_hellthread, :reject_threshold], Pleroma.Config.get([:mrf_hellthread, :threshold]) ) - recipients = get_recipient_count(message) - - cond do - recipients > reject_threshold and reject_threshold > 0 -> + with {_, recipients} <- get_recipient_count(message) do + if recipients > reject_threshold and reject_threshold > 0 do {:reject, nil} - - recipients > delist_threshold and delist_threshold > 0 -> - if Enum.member?(message["to"], "https://www.w3.org/ns/activitystreams#Public") or - Enum.member?(message["cc"], "https://www.w3.org/ns/activitystreams#Public") do - {:ok, delist_message(message)} - else - {:ok, message} - end - - true -> + else {:ok, message} + end + end + end + + defp get_recipient_count(message) do + recipients = (message["to"] || []) ++ (message["cc"] || []) + follower_collection = User.get_cached_by_ap_id(message["actor"]).follower_address + + if Enum.member?(recipients, "https://www.w3.org/ns/activitystreams#Public") do + recipients + |> List.delete("https://www.w3.org/ns/activitystreams#Public") + |> List.delete(follower_collection) + + {:public, length(recipients)} + else + recipients + |> List.delete(follower_collection) + + {:not_public, length(recipients)} + end + end + + @impl true + def filter(%{"type" => "Create"} = message) do + with {:ok, message} <- reject_message(message), + {:ok, message} <- delist_message(message) do + {:ok, message} + else + _e -> {:reject, nil} end end From d943c90249e0a598e57a0dbdf41d387b53916092 Mon Sep 17 00:00:00 2001 From: Karen Konou Date: Fri, 15 Feb 2019 12:47:50 +0100 Subject: [PATCH 3/5] Add tests, change default config values, fix a bug --- config/config.exs | 4 +- .../web/activity_pub/mrf/hellthread_policy.ex | 12 +++-- .../mrf/hellthread_policy_test.exs | 50 +++++++++++++++++++ 3 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 test/web/activity_pub/mrf/hellthread_policy_test.exs diff --git a/config/config.exs b/config/config.exs index 5db0ea9aa..e2a239a76 100644 --- a/config/config.exs +++ b/config/config.exs @@ -228,8 +228,8 @@ config :pleroma, :mrf_rejectnonpublic, allow_direct: false config :pleroma, :mrf_hellthread, - delist_threshold: 5, - reject_threshold: 10 + delist_threshold: 10, + reject_threshold: 20 config :pleroma, :mrf_simple, media_removal: [], diff --git a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex index 1f09b4e66..95211c596 100644 --- a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex @@ -47,14 +47,16 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do follower_collection = User.get_cached_by_ap_id(message["actor"]).follower_address if Enum.member?(recipients, "https://www.w3.org/ns/activitystreams#Public") do - recipients - |> List.delete("https://www.w3.org/ns/activitystreams#Public") - |> List.delete(follower_collection) + recipients = + recipients + |> List.delete("https://www.w3.org/ns/activitystreams#Public") + |> List.delete(follower_collection) {:public, length(recipients)} else - recipients - |> List.delete(follower_collection) + recipients = + recipients + |> List.delete(follower_collection) {:not_public, length(recipients)} end diff --git a/test/web/activity_pub/mrf/hellthread_policy_test.exs b/test/web/activity_pub/mrf/hellthread_policy_test.exs new file mode 100644 index 000000000..b5bdd35cc --- /dev/null +++ b/test/web/activity_pub/mrf/hellthread_policy_test.exs @@ -0,0 +1,50 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicyTest do + use Pleroma.DataCase + import Pleroma.Factory + + import Pleroma.Web.ActivityPub.MRF.HellthreadPolicy + + describe "hellthread filter tests" do + setup do + user = insert(:user) + + message = %{ + "actor" => user.ap_id, + "cc" => [user.follower_address], + "type" => "Create", + "to" => [ + "https://www.w3.org/ns/activitystreams#Public", + "https://instace.tld/users/user1", + "https://instace.tld/users/user2", + "https://instace.tld/users/user3" + ] + } + + [user: user, message: message] + end + + test "reject test", %{message: message} do + Pleroma.Config.put([:mrf_hellthread], %{delist_threshold: 0, reject_threshold: 2}) + + {:reject, nil} = filter(message) + end + + test "delist test", %{user: user, message: message} do + Pleroma.Config.put([:mrf_hellthread], %{delist_threshold: 2, reject_threshold: 0}) + + {:ok, message} = filter(message) + assert user.follower_address in message["to"] + assert "https://www.w3.org/ns/activitystreams#Public" in message["cc"] + end + + test "threshold test", %{message: message} do + Pleroma.Config.put([:mrf_hellthread], %{delist_threshold: 0, reject_threshold: 3}) + + {:ok, _} = filter(message) + end + end +end From dca6bee2f7eba1dc366cc65d3087f20678549739 Mon Sep 17 00:00:00 2001 From: Karen Konou Date: Fri, 15 Feb 2019 13:43:14 +0100 Subject: [PATCH 4/5] Rename test, add check for follower collection when delisting --- .../web/activity_pub/mrf/hellthread_policy.ex | 17 +++++++++++------ .../activity_pub/mrf/hellthread_policy_test.exs | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex index 95211c596..1fd7b9c67 100644 --- a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex @@ -10,17 +10,22 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do delist_threshold = Pleroma.Config.get([:mrf_hellthread, :delist_threshold]) follower_collection = User.get_cached_by_ap_id(message["actor"]).follower_address + follower_collection? = Enum.member?(message["to"] ++ message["cc"], follower_collection) + message = - with {:public, recipients} <- get_recipient_count(message) do - if recipients > delist_threshold and delist_threshold > 0 do + case recipients = get_recipient_count(message) do + {:public, _} when follower_collection? and recipients > delist_threshold -> message |> Map.put("to", [follower_collection]) |> Map.put("cc", ["https://www.w3.org/ns/activitystreams#Public"]) - else + + {:public, _} when recipients > delist_threshold -> + message + |> Map.put("to", []) + |> Map.put("cc", ["https://www.w3.org/ns/activitystreams#Public"]) + + _ -> message - end - else - _ -> message end {:ok, message} diff --git a/test/web/activity_pub/mrf/hellthread_policy_test.exs b/test/web/activity_pub/mrf/hellthread_policy_test.exs index b5bdd35cc..ebf9997cd 100644 --- a/test/web/activity_pub/mrf/hellthread_policy_test.exs +++ b/test/web/activity_pub/mrf/hellthread_policy_test.exs @@ -41,7 +41,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicyTest do assert "https://www.w3.org/ns/activitystreams#Public" in message["cc"] end - test "threshold test", %{message: message} do + test "excludes follower collection and public URI from threshold count", %{message: message} do Pleroma.Config.put([:mrf_hellthread], %{delist_threshold: 0, reject_threshold: 3}) {:ok, _} = filter(message) From c2e0a0c8d44f8697160d4597db45c5c4afd0d8a6 Mon Sep 17 00:00:00 2001 From: Karen Konou Date: Fri, 15 Feb 2019 14:05:20 +0100 Subject: [PATCH 5/5] Readd threshold is not 0 check, optmization? --- .../web/activity_pub/mrf/hellthread_policy.ex | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex index 1fd7b9c67..8ab1dd4e5 100644 --- a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex @@ -6,20 +6,20 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do alias Pleroma.User @behaviour Pleroma.Web.ActivityPub.MRF - defp delist_message(message) do - delist_threshold = Pleroma.Config.get([:mrf_hellthread, :delist_threshold]) + defp delist_message(message, threshold) when threshold > 0 do follower_collection = User.get_cached_by_ap_id(message["actor"]).follower_address follower_collection? = Enum.member?(message["to"] ++ message["cc"], follower_collection) message = case recipients = get_recipient_count(message) do - {:public, _} when follower_collection? and recipients > delist_threshold -> + {:public, _} + when follower_collection? and recipients > threshold -> message |> Map.put("to", [follower_collection]) |> Map.put("cc", ["https://www.w3.org/ns/activitystreams#Public"]) - {:public, _} when recipients > delist_threshold -> + {:public, _} when recipients > threshold -> message |> Map.put("to", []) |> Map.put("cc", ["https://www.w3.org/ns/activitystreams#Public"]) @@ -31,15 +31,11 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do {:ok, message} end - defp reject_message(message) do - reject_threshold = - Pleroma.Config.get( - [:mrf_hellthread, :reject_threshold], - Pleroma.Config.get([:mrf_hellthread, :threshold]) - ) + defp delist_message(message, _threshold), do: {:ok, message} + defp reject_message(message, threshold) when threshold > 0 do with {_, recipients} <- get_recipient_count(message) do - if recipients > reject_threshold and reject_threshold > 0 do + if recipients > threshold do {:reject, nil} else {:ok, message} @@ -47,6 +43,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do end end + defp reject_message(message, _threshold), do: {:ok, message} + defp get_recipient_count(message) do recipients = (message["to"] || []) ++ (message["cc"] || []) follower_collection = User.get_cached_by_ap_id(message["actor"]).follower_address @@ -69,8 +67,16 @@ defmodule Pleroma.Web.ActivityPub.MRF.HellthreadPolicy do @impl true def filter(%{"type" => "Create"} = message) do - with {:ok, message} <- reject_message(message), - {:ok, message} <- delist_message(message) do + reject_threshold = + Pleroma.Config.get( + [:mrf_hellthread, :reject_threshold], + Pleroma.Config.get([:mrf_hellthread, :threshold]) + ) + + delist_threshold = Pleroma.Config.get([:mrf_hellthread, :delist_threshold]) + + with {:ok, message} <- reject_message(message, reject_threshold), + {:ok, message} <- delist_message(message, delist_threshold) do {:ok, message} else _e -> {:reject, nil}