From 6f83ae27aa924db1da2189cf495bd83cb41ba408 Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 9 Dec 2022 19:57:29 +0000 Subject: [PATCH 1/7] extend reject MRF to check if originating instance is blocked --- CHANGELOG.md | 1 + config/config.exs | 3 +- lib/pleroma/config/deprecation_warnings.ex | 3 +- .../web/activity_pub/mrf/simple_policy.ex | 209 ++++++++++++------ .../activity_pub/mrf/simple_policy_test.exs | 80 +++++++ 5 files changed, 224 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07ed6653a..017ec6a8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ 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 ## Changed - MastoAPI: Accept BooleanLike input on `/api/v1/accounts/:id/follow` (fixes follows with mastodon.py) diff --git a/config/config.exs b/config/config.exs index 8e0104400..4d6150634 100644 --- a/config/config.exs +++ b/config/config.exs @@ -391,7 +391,8 @@ accept: [], avatar_removal: [], banner_removal: [], - reject_deletes: [] + reject_deletes: [], + handle_threads: true config :pleroma, :mrf_keyword, reject: [], diff --git a/lib/pleroma/config/deprecation_warnings.ex b/lib/pleroma/config/deprecation_warnings.ex index 8a336c35a..73ef41145 100644 --- a/lib/pleroma/config/deprecation_warnings.ex +++ b/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 diff --git a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex index a59212db4..f7eb0f159 100644 --- a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex +++ b/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} - actor_host == Config.get([Pleroma.Web.Endpoint, :url, :host]) -> {:ok, object} - MRF.subdomain_match?(accepts, actor_host) -> {:ok, object} + accepts == [] -> {:ok, nil} + actor_host == Config.get([Pleroma.Web.Endpoint, :url, :host]) -> {:ok, nil} + 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), - {:ok, object} <- check_reject(actor_info, object), + with {:ok, _} <- check_accept(actor_info), + {: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), - {:ok, object} <- check_reject(actor_info, object), + with {:ok, _} <- check_accept(actor_info), + {: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), - {:ok, object} <- check_reject(uri, object) do + with {:ok, _} <- check_accept(uri), + {: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, - description: - "List of instances to strip media attachments from and the reason for doing so" - }, - %{ - key: :media_nsfw, - label: "Media NSFW", - description: - "List of instances to tag all media as NSFW (sensitive) from and the reason for doing so" - }, - %{ - key: :federated_timeline_removal, - description: - "List of instances to remove from the Federated (aka The Whole Known Network) Timeline and the reason for doing so" - }, - %{ - key: :reject, - description: - "List of instances to reject activities from (except deletes) and the reason for doing so" - }, - %{ - key: :accept, - description: - "List of instances to only accept activities from (except deletes) and the reason for doing so" - }, - %{ - key: :followers_only, - description: - "Force posts from the given instances to be visible by followers only and the reason for doing so" - }, - %{ - key: :report_removal, - description: "List of instances to reject reports from and the reason for doing so" - }, - %{ - key: :avatar_removal, - description: "List of instances to strip avatars from and the reason for doing so" - }, - %{ - key: :banner_removal, - description: "List of instances to strip banners from and the reason for doing so" - }, - %{ - key: :reject_deletes, - description: "List of instances to reject deletions from and the reason for doing so" - } - ] - |> Enum.map(fn setting -> - Map.merge( - setting, + ([ + %{ + key: :media_removal, + description: + "List of instances to strip media attachments from and the reason for doing so" + }, + %{ + key: :media_nsfw, + label: "Media NSFW", + description: + "List of instances to tag all media as NSFW (sensitive) from and the reason for doing so" + }, + %{ + key: :federated_timeline_removal, + description: + "List of instances to remove from the Federated (aka The Whole Known Network) Timeline and the reason for doing so" + }, + %{ + key: :reject, + description: + "List of instances to reject activities from (except deletes) and the reason for doing so" + }, + %{ + key: :accept, + description: + "List of instances to only accept activities from (except deletes) and the reason for doing so" + }, + %{ + key: :followers_only, + description: + "Force posts from the given instances to be visible by followers only and the reason for doing so" + }, + %{ + key: :report_removal, + description: "List of instances to reject reports from and the reason for doing so" + }, + %{ + key: :avatar_removal, + description: "List of instances to strip avatars from and the reason for doing so" + }, + %{ + key: :banner_removal, + description: "List of instances to strip banners from and the reason for doing so" + }, + %{ + key: :reject_deletes, + description: "List of instances to reject deletions from and the reason for doing so" + } + ] + |> Enum.map(fn setting -> + Map.merge( + setting, + %{ + type: {:list, :tuple}, + key_placeholder: "instance", + value_placeholder: "reason", + suggestions: [ + {"example.com", "Some reason"}, + {"*.example.com", "Another reason"} + ] + } + ) + end)) ++ + [ %{ - type: {:list, :tuple}, - key_placeholder: "instance", - value_placeholder: "reason", - suggestions: [{"example.com", "Some reason"}, {"*.example.com", "Another reason"}] + key: :handle_threads, + label: "Apply to entire threads", + type: :boolean, + description: + "Enable to filter replies to threads based from their originating instance, using the reject and accept rules" } - ) - end) + ] } end end diff --git a/test/pleroma/web/activity_pub/mrf/simple_policy_test.exs b/test/pleroma/web/activity_pub/mrf/simple_policy_test.exs index 036573171..875cf8f43 100644 --- a/test/pleroma/web/activity_pub/mrf/simple_policy_test.exs +++ b/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 From 9db4c2429fce2d9f56cc59de64b3d6b62e1a7071 Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 9 Dec 2022 19:59:27 +0000 Subject: [PATCH 2/7] Remove FollowBotPolicy --- CHANGELOG.md | 3 + docs/docs/configuration/cheatsheet.md | 5 - .../web/activity_pub/mrf/follow_bot_policy.ex | 59 -------- .../mrf/follow_bot_policy_test.exs | 126 ------------------ 4 files changed, 3 insertions(+), 190 deletions(-) delete mode 100644 lib/pleroma/web/activity_pub/mrf/follow_bot_policy.ex delete mode 100644 test/pleroma/web/activity_pub/mrf/follow_bot_policy_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 017ec6a8c..4f266d514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ 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 + ## 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. diff --git a/docs/docs/configuration/cheatsheet.md b/docs/docs/configuration/cheatsheet.md index 12c044d67..3c8bbcf84 100644 --- a/docs/docs/configuration/cheatsheet.md +++ b/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 diff --git a/lib/pleroma/web/activity_pub/mrf/follow_bot_policy.ex b/lib/pleroma/web/activity_pub/mrf/follow_bot_policy.ex deleted file mode 100644 index 7cf7de068..000000000 --- a/lib/pleroma/web/activity_pub/mrf/follow_bot_policy.ex +++ /dev/null @@ -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 diff --git a/test/pleroma/web/activity_pub/mrf/follow_bot_policy_test.exs b/test/pleroma/web/activity_pub/mrf/follow_bot_policy_test.exs deleted file mode 100644 index a61562558..000000000 --- a/test/pleroma/web/activity_pub/mrf/follow_bot_policy_test.exs +++ /dev/null @@ -1,126 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2021 Pleroma Authors -# 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 From dcf58a3c532d22bc13505b84a50c2c1ddfdaa960 Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 9 Dec 2022 20:01:38 +0000 Subject: [PATCH 3/7] Do not pass transient undo-y activities through MRF --- CHANGELOG.md | 1 + lib/pleroma/web/activity_pub/mrf.ex | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f266d514..6956912a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## 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. diff --git a/lib/pleroma/web/activity_pub/mrf.ex b/lib/pleroma/web/activity_pub/mrf.ex index 064ffc527..dae6d7f6a 100644 --- a/lib/pleroma/web/activity_pub/mrf.ex +++ b/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 From bc265bfd541ea437481786504c0334444626f06f Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 9 Dec 2022 20:04:48 +0000 Subject: [PATCH 4/7] Underscore unused variable --- lib/pleroma/config/deprecation_warnings.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/config/deprecation_warnings.ex b/lib/pleroma/config/deprecation_warnings.ex index 73ef41145..c213f3ce6 100644 --- a/lib/pleroma/config/deprecation_warnings.ex +++ b/lib/pleroma/config/deprecation_warnings.ex @@ -66,7 +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.filter(fn {_k, v} -> not is_atom(v) end) |> Enum.map(fn {k, v} -> {k, Enum.map(v, fn From f5a315f04cc4e07ff951a2c084525d899291df92 Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 9 Dec 2022 20:13:31 +0000 Subject: [PATCH 5/7] Add URL and code to :not_found errors Ref #355 --- CHANGELOG.md | 1 + lib/pleroma/collections/fetcher.ex | 2 +- lib/pleroma/object/fetcher.ex | 4 ++-- lib/pleroma/web/activity_pub/activity_pub.ex | 2 +- test/pleroma/object/fetcher_test.exs | 6 ++++-- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6956912a7..eef3c53b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - 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) diff --git a/lib/pleroma/collections/fetcher.ex b/lib/pleroma/collections/fetcher.ex index ab69f4b84..a2fcb7794 100644 --- a/lib/pleroma/collections/fetcher.ex +++ b/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} -> diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index a9dfa18e7..cde4e5039 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/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} diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index d700128c0..521c8b852 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/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} diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 71306cdfe..22192d98f 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/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" ) From e49b583147748be73062acc92ea510f6f55a503a Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 9 Dec 2022 20:27:54 +0000 Subject: [PATCH 6/7] mandate published on notes fixes #356 --- CHANGELOG.md | 1 + lib/mix/tasks/pleroma/search/meilisearch.ex | 3 +- .../article_note_page_validator.ex | 2 +- .../mastodon/note-without-published.json | 44 +++++++++++++++++++ .../article_note_page_validator_test.exs | 7 +++ 5 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/mastodon/note-without-published.json diff --git a/CHANGELOG.md b/CHANGELOG.md index eef3c53b8..c0aa002a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Follow/Block/Mute imports now spin off into *n* tasks to avoid the oban timeout - Transient activities recieved from remote servers are no longer persisted in the database - Overhauled static-fe view for logged-out users +- `published` is now a mandatory field in Note objects ## Removed - FollowBotPolicy diff --git a/lib/mix/tasks/pleroma/search/meilisearch.ex b/lib/mix/tasks/pleroma/search/meilisearch.ex index 27a31afcf..caeeb58d5 100644 --- a/lib/mix/tasks/pleroma/search/meilisearch.ex +++ b/lib/mix/tasks/pleroma/search/meilisearch.ex @@ -60,7 +60,8 @@ def run(["index"]) do where: fragment("data->>'type' = 'Note'") and (fragment("data->'to' \\? ?", ^Pleroma.Constants.as_public()) or - fragment("data->'cc' \\? ?", ^Pleroma.Constants.as_public())), + fragment("data->'cc' \\? ?", ^Pleroma.Constants.as_public())) and + fragment("data->>'published' IS NOT NULL"), order_by: [desc: fragment("data->'published'")] ) diff --git a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex index 0d45421e2..0967f557a 100644 --- a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex @@ -172,7 +172,7 @@ def changeset(struct, data) do defp validate_data(data_cng) do data_cng |> validate_inclusion(:type, ["Article", "Note", "Page"]) - |> validate_required([:id, :actor, :attributedTo, :type, :context]) + |> validate_required([:id, :actor, :attributedTo, :type, :context, :published]) |> CommonValidations.validate_any_presence([:cc, :to]) |> CommonValidations.validate_fields_match([:actor, :attributedTo]) |> CommonValidations.validate_actor_presence() diff --git a/test/fixtures/mastodon/note-without-published.json b/test/fixtures/mastodon/note-without-published.json new file mode 100644 index 000000000..c721ecef5 --- /dev/null +++ b/test/fixtures/mastodon/note-without-published.json @@ -0,0 +1,44 @@ +{ + "@context" : [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + { + "Emoji" : "toot:Emoji", + "Hashtag" : "as:Hashtag", + "atomUri" : "ostatus:atomUri", + "conversation" : "ostatus:conversation", + "inReplyToAtomUri" : "ostatus:inReplyToAtomUri", + "manuallyApprovesFollowers" : "as:manuallyApprovesFollowers", + "movedTo" : "as:movedTo", + "ostatus" : "http://ostatus.org#", + "sensitive" : "as:sensitive", + "toot" : "http://joinmastodon.org/ns#" + } + ], + "atomUri" : "http://mastodon.example.org/users/admin/statuses/99541947525187367", + "attachment" : [ + { + "mediaType" : "image/jpeg", + "name" : null, + "type" : "Document", + "url" : "http://mastodon.example.org/system/media_attachments/files/000/000/002/original/334ce029e7bfb920.jpg" + } + ], + "attributedTo" : "http://mastodon.example.org/users/admin", + "cc" : [ + "http://mastodon.example.org/users/admin/followers" + ], + "content" : "

yeah.

", + "conversation" : "tag:mastodon.example.org,2018-02-17:objectId=59:objectType=Conversation", + "id" : "http://mastodon.example.org/users/admin/statuses/99541947525187367", + "inReplyTo" : null, + "inReplyToAtomUri" : null, + "sensitive" : false, + "summary" : null, + "tag" : [], + "to" : [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "type" : "Note", + "url" : "http://mastodon.example.org/@admin/99541947525187367" +} diff --git a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs index 5b95ebc51..0ddc3c76d 100644 --- a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs +++ b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs @@ -28,6 +28,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidatorTest "to" => [user.follower_address], "cc" => [], "content" => "Hellow this is content.", + "published" => "2021-01-01T00:00:00Z", "context" => "xxx", "summary" => "a post" } @@ -65,6 +66,12 @@ test "a note with a remote replies collection should validate", _ do ArticleNotePageValidator.cast_and_validate(note) end + test "a note without a published field should not validate", _ do + insert(:user, %{ap_id: "http://mastodon.example.org/users/admin"}) + note = Jason.decode!(File.read!("test/fixtures/mastodon/note-without-published.json")) + %{valid?: false} = ArticleNotePageValidator.cast_and_validate(note) + end + test "a note with an attachment should work", _ do insert(:user, %{ap_id: "https://owncast.localhost.localdomain/federation/user/streamer"}) From 739ed14f54eea3bf6e5ddabdf3fa7f60b9618aff Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 9 Dec 2022 20:59:26 +0000 Subject: [PATCH 7/7] Revert "mandate published on notes" This reverts commit e49b583147748be73062acc92ea510f6f55a503a. --- CHANGELOG.md | 1 - lib/mix/tasks/pleroma/search/meilisearch.ex | 3 +- .../article_note_page_validator.ex | 2 +- .../mastodon/note-without-published.json | 44 ------------------- .../article_note_page_validator_test.exs | 7 --- 5 files changed, 2 insertions(+), 55 deletions(-) delete mode 100644 test/fixtures/mastodon/note-without-published.json diff --git a/CHANGELOG.md b/CHANGELOG.md index c0aa002a1..eef3c53b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Follow/Block/Mute imports now spin off into *n* tasks to avoid the oban timeout - Transient activities recieved from remote servers are no longer persisted in the database - Overhauled static-fe view for logged-out users -- `published` is now a mandatory field in Note objects ## Removed - FollowBotPolicy diff --git a/lib/mix/tasks/pleroma/search/meilisearch.ex b/lib/mix/tasks/pleroma/search/meilisearch.ex index caeeb58d5..27a31afcf 100644 --- a/lib/mix/tasks/pleroma/search/meilisearch.ex +++ b/lib/mix/tasks/pleroma/search/meilisearch.ex @@ -60,8 +60,7 @@ def run(["index"]) do where: fragment("data->>'type' = 'Note'") and (fragment("data->'to' \\? ?", ^Pleroma.Constants.as_public()) or - fragment("data->'cc' \\? ?", ^Pleroma.Constants.as_public())) and - fragment("data->>'published' IS NOT NULL"), + fragment("data->'cc' \\? ?", ^Pleroma.Constants.as_public())), order_by: [desc: fragment("data->'published'")] ) diff --git a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex index 0967f557a..0d45421e2 100644 --- a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex @@ -172,7 +172,7 @@ def changeset(struct, data) do defp validate_data(data_cng) do data_cng |> validate_inclusion(:type, ["Article", "Note", "Page"]) - |> validate_required([:id, :actor, :attributedTo, :type, :context, :published]) + |> validate_required([:id, :actor, :attributedTo, :type, :context]) |> CommonValidations.validate_any_presence([:cc, :to]) |> CommonValidations.validate_fields_match([:actor, :attributedTo]) |> CommonValidations.validate_actor_presence() diff --git a/test/fixtures/mastodon/note-without-published.json b/test/fixtures/mastodon/note-without-published.json deleted file mode 100644 index c721ecef5..000000000 --- a/test/fixtures/mastodon/note-without-published.json +++ /dev/null @@ -1,44 +0,0 @@ -{ - "@context" : [ - "https://www.w3.org/ns/activitystreams", - "https://w3id.org/security/v1", - { - "Emoji" : "toot:Emoji", - "Hashtag" : "as:Hashtag", - "atomUri" : "ostatus:atomUri", - "conversation" : "ostatus:conversation", - "inReplyToAtomUri" : "ostatus:inReplyToAtomUri", - "manuallyApprovesFollowers" : "as:manuallyApprovesFollowers", - "movedTo" : "as:movedTo", - "ostatus" : "http://ostatus.org#", - "sensitive" : "as:sensitive", - "toot" : "http://joinmastodon.org/ns#" - } - ], - "atomUri" : "http://mastodon.example.org/users/admin/statuses/99541947525187367", - "attachment" : [ - { - "mediaType" : "image/jpeg", - "name" : null, - "type" : "Document", - "url" : "http://mastodon.example.org/system/media_attachments/files/000/000/002/original/334ce029e7bfb920.jpg" - } - ], - "attributedTo" : "http://mastodon.example.org/users/admin", - "cc" : [ - "http://mastodon.example.org/users/admin/followers" - ], - "content" : "

yeah.

", - "conversation" : "tag:mastodon.example.org,2018-02-17:objectId=59:objectType=Conversation", - "id" : "http://mastodon.example.org/users/admin/statuses/99541947525187367", - "inReplyTo" : null, - "inReplyToAtomUri" : null, - "sensitive" : false, - "summary" : null, - "tag" : [], - "to" : [ - "https://www.w3.org/ns/activitystreams#Public" - ], - "type" : "Note", - "url" : "http://mastodon.example.org/@admin/99541947525187367" -} diff --git a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs index 0ddc3c76d..5b95ebc51 100644 --- a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs +++ b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs @@ -28,7 +28,6 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidatorTest "to" => [user.follower_address], "cc" => [], "content" => "Hellow this is content.", - "published" => "2021-01-01T00:00:00Z", "context" => "xxx", "summary" => "a post" } @@ -66,12 +65,6 @@ test "a note with a remote replies collection should validate", _ do ArticleNotePageValidator.cast_and_validate(note) end - test "a note without a published field should not validate", _ do - insert(:user, %{ap_id: "http://mastodon.example.org/users/admin"}) - note = Jason.decode!(File.read!("test/fixtures/mastodon/note-without-published.json")) - %{valid?: false} = ArticleNotePageValidator.cast_and_validate(note) - end - test "a note with an attachment should work", _ do insert(:user, %{ap_id: "https://owncast.localhost.localdomain/federation/user/streamer"})