commit: 22d49993d9ab35ff5d5276a6a1f4aec96d03b7f3
parent eff7f9892dbb042a447fd83e1ce5e9cb69941cf6
Author: rinpatch <rinpatch@sdf.org>
Date: Thu, 17 Sep 2020 12:13:36 +0000
Merge branch 'bugfix/mrf-ingestion' into 'develop'
Bugfix: MRF and Pipeline Ingestion
See merge request pleroma/secteam/pleroma!15
Diffstat:
12 files changed, 124 insertions(+), 53 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,17 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
+## unreleased-patch - ???
+
+### Security
+
+- Fix most MRF rules either crashing or not being applied to objects passed into the Common Pipeline (ChatMessage, Question, Answer, Audio, Event)
+
+### Fixed
+
+- Welcome Chat messages preventing user registration with MRF Simple Policy applied to the local instance
+- Mastodon API: the public timeline returning an error when the `reply_visibility` parameter is set to `self` for an unauthenticated user
+
## [2.1.1] - 2020-09-08
### Security
diff --git a/lib/pleroma/web/activity_pub/mrf.ex b/lib/pleroma/web/activity_pub/mrf.ex
@@ -5,16 +5,34 @@
defmodule Pleroma.Web.ActivityPub.MRF do
@callback filter(Map.t()) :: {:ok | :reject, Map.t()}
- def filter(policies, %{} = object) do
+ def filter(policies, %{} = message) do
policies
- |> Enum.reduce({:ok, object}, fn
- policy, {:ok, object} -> policy.filter(object)
+ |> Enum.reduce({:ok, message}, fn
+ policy, {:ok, message} -> policy.filter(message)
_, error -> error
end)
end
def filter(%{} = object), do: get_policies() |> filter(object)
+ def pipeline_filter(%{} = message, meta) do
+ object = meta[:object_data]
+ ap_id = message["object"]
+
+ if object && ap_id do
+ with {:ok, message} <- filter(Map.put(message, "object", object)) do
+ meta = Keyword.put(meta, :object_data, message["object"])
+ {:ok, Map.put(message, "object", ap_id), meta}
+ else
+ {err, message} -> {err, message, meta}
+ end
+ else
+ {err, message} = filter(message)
+
+ {err, message, meta}
+ end
+ end
+
def get_policies do
Pleroma.Config.get([:mrf, :policies], []) |> get_policies()
end
diff --git a/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex b/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex
@@ -20,9 +20,17 @@ defmodule Pleroma.Web.ActivityPub.MRF.KeywordPolicy do
String.match?(string, pattern)
end
- defp check_reject(%{"object" => %{"content" => content, "summary" => summary}} = message) do
+ defp object_payload(%{} = object) do
+ [object["content"], object["summary"], object["name"]]
+ |> Enum.filter(& &1)
+ |> Enum.join("\n")
+ end
+
+ defp check_reject(%{"object" => %{} = object} = message) do
+ payload = object_payload(object)
+
if Enum.any?(Pleroma.Config.get([:mrf_keyword, :reject]), fn pattern ->
- string_matches?(content, pattern) or string_matches?(summary, pattern)
+ string_matches?(payload, pattern)
end) do
{:reject, "[KeywordPolicy] Matches with rejected keyword"}
else
@@ -30,12 +38,12 @@ defmodule Pleroma.Web.ActivityPub.MRF.KeywordPolicy do
end
end
- defp check_ftl_removal(
- %{"to" => to, "object" => %{"content" => content, "summary" => summary}} = message
- ) do
+ defp check_ftl_removal(%{"to" => to, "object" => %{} = object} = message) do
+ payload = object_payload(object)
+
if Pleroma.Constants.as_public() in to and
Enum.any?(Pleroma.Config.get([:mrf_keyword, :federated_timeline_removal]), fn pattern ->
- string_matches?(content, pattern) or string_matches?(summary, pattern)
+ string_matches?(payload, pattern)
end) do
to = List.delete(to, Pleroma.Constants.as_public())
cc = [Pleroma.Constants.as_public() | message["cc"] || []]
@@ -51,35 +59,24 @@ defmodule Pleroma.Web.ActivityPub.MRF.KeywordPolicy do
end
end
- defp check_replace(%{"object" => %{"content" => content, "summary" => summary}} = message) do
- content =
- if is_binary(content) do
- content
- else
- ""
- end
-
- summary =
- if is_binary(summary) do
- summary
- else
- ""
- end
-
- {content, summary} =
- Enum.reduce(
- Pleroma.Config.get([:mrf_keyword, :replace]),
- {content, summary},
- fn {pattern, replacement}, {content_acc, summary_acc} ->
- {String.replace(content_acc, pattern, replacement),
- String.replace(summary_acc, pattern, replacement)}
- end
- )
-
- {:ok,
- message
- |> put_in(["object", "content"], content)
- |> put_in(["object", "summary"], summary)}
+ defp check_replace(%{"object" => %{} = object} = message) do
+ object =
+ ["content", "name", "summary"]
+ |> Enum.filter(fn field -> Map.has_key?(object, field) && object[field] end)
+ |> Enum.reduce(object, fn field, object ->
+ data =
+ Enum.reduce(
+ Pleroma.Config.get([:mrf_keyword, :replace]),
+ object[field],
+ fn {pat, repl}, acc -> String.replace(acc, pat, repl) end
+ )
+
+ Map.put(object, field, data)
+ end)
+
+ message = Map.put(message, "object", object)
+
+ {:ok, message}
end
@impl true
diff --git a/lib/pleroma/web/activity_pub/mrf/subchain_policy.ex b/lib/pleroma/web/activity_pub/mrf/subchain_policy.ex
@@ -28,8 +28,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.SubchainPolicy do
}"
)
- subchain
- |> MRF.filter(message)
+ MRF.filter(subchain, message)
else
_e -> {:ok, message}
end
diff --git a/lib/pleroma/web/activity_pub/pipeline.ex b/lib/pleroma/web/activity_pub/pipeline.ex
@@ -26,13 +26,17 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do
{:error, e} ->
{:error, e}
+
+ {:reject, e} ->
+ {:reject, e}
end
end
def do_common_pipeline(object, meta) do
with {_, {:ok, validated_object, meta}} <-
{:validate_object, ObjectValidator.validate(object, meta)},
- {_, {:ok, mrfd_object}} <- {:mrf_object, MRF.filter(validated_object)},
+ {_, {:ok, mrfd_object, meta}} <-
+ {:mrf_object, MRF.pipeline_filter(validated_object, meta)},
{_, {:ok, activity, meta}} <-
{:persist_object, ActivityPub.persist(mrfd_object, meta)},
{_, {:ok, activity, meta}} <-
@@ -40,7 +44,7 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do
{_, {:ok, _}} <- {:federation, maybe_federate(activity, meta)} do
{:ok, activity, meta}
else
- {:mrf_object, {:reject, _}} -> {:ok, nil, meta}
+ {:mrf_object, {:reject, message, _}} -> {:reject, message}
e -> {:error, e}
end
end
diff --git a/lib/pleroma/web/api_spec/operations/chat_operation.ex b/lib/pleroma/web/api_spec/operations/chat_operation.ex
@@ -184,7 +184,8 @@ defmodule Pleroma.Web.ApiSpec.ChatOperation do
"application/json",
ChatMessage
),
- 400 => Operation.response("Bad Request", "application/json", ApiError)
+ 400 => Operation.response("Bad Request", "application/json", ApiError),
+ 422 => Operation.response("MRF Rejection", "application/json", ApiError)
},
security: [
%{
diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex
@@ -55,7 +55,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do
"application/json",
%Schema{oneOf: [Status, ScheduledStatus]}
),
- 422 => Operation.response("Bad Request", "application/json", ApiError)
+ 422 => Operation.response("Bad Request / MRF Rejection", "application/json", ApiError)
}
}
end
diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex
@@ -49,6 +49,9 @@ defmodule Pleroma.Web.CommonAPI do
local: true
)} do
{:ok, activity}
+ else
+ {:common_pipeline, {:reject, _} = e} -> e
+ e -> e
end
end
diff --git a/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex b/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex
@@ -90,6 +90,16 @@ defmodule Pleroma.Web.PleromaAPI.ChatController do
conn
|> put_view(MessageReferenceView)
|> render("show.json", chat_message_reference: cm_ref)
+ else
+ {:reject, message} ->
+ conn
+ |> put_status(:unprocessable_entity)
+ |> json(%{error: message})
+
+ {:error, message} ->
+ conn
+ |> put_status(:bad_request)
+ |> json(%{error: message})
end
end
diff --git a/test/web/activity_pub/pipeline_test.exs b/test/web/activity_pub/pipeline_test.exs
@@ -26,7 +26,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do
{
Pleroma.Web.ActivityPub.MRF,
[],
- [filter: fn o -> {:ok, o} end]
+ [pipeline_filter: fn o, m -> {:ok, o, m} end]
},
{
Pleroma.Web.ActivityPub.ActivityPub,
@@ -51,7 +51,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do
Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta)
assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta))
- assert_called(Pleroma.Web.ActivityPub.MRF.filter(activity))
+ assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta))
assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta))
assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta))
refute called(Pleroma.Web.Federator.publish(activity))
@@ -68,7 +68,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do
{
Pleroma.Web.ActivityPub.MRF,
[],
- [filter: fn o -> {:ok, o} end]
+ [pipeline_filter: fn o, m -> {:ok, o, m} end]
},
{
Pleroma.Web.ActivityPub.ActivityPub,
@@ -93,7 +93,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do
Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta)
assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta))
- assert_called(Pleroma.Web.ActivityPub.MRF.filter(activity))
+ assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta))
assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta))
assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta))
assert_called(Pleroma.Web.Federator.publish(activity))
@@ -109,7 +109,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do
{
Pleroma.Web.ActivityPub.MRF,
[],
- [filter: fn o -> {:ok, o} end]
+ [pipeline_filter: fn o, m -> {:ok, o, m} end]
},
{
Pleroma.Web.ActivityPub.ActivityPub,
@@ -131,7 +131,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do
Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta)
assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta))
- assert_called(Pleroma.Web.ActivityPub.MRF.filter(activity))
+ assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta))
assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta))
assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta))
end
@@ -148,7 +148,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do
{
Pleroma.Web.ActivityPub.MRF,
[],
- [filter: fn o -> {:ok, o} end]
+ [pipeline_filter: fn o, m -> {:ok, o, m} end]
},
{
Pleroma.Web.ActivityPub.ActivityPub,
@@ -170,7 +170,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do
Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta)
assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta))
- assert_called(Pleroma.Web.ActivityPub.MRF.filter(activity))
+ assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta))
assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta))
assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta))
end
diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs
@@ -213,6 +213,17 @@ defmodule Pleroma.Web.CommonAPITest do
assert message == :content_too_long
end
+
+ test "it reject messages via MRF" do
+ clear_config([:mrf_keyword, :reject], ["GNO"])
+ clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.KeywordPolicy])
+
+ author = insert(:user)
+ recipient = insert(:user)
+
+ assert {:reject, "[KeywordPolicy] Matches with rejected keyword"} ==
+ CommonAPI.post_chat_message(author, recipient, "GNO/Linux")
+ end
end
describe "unblocking" do
diff --git a/test/web/pleroma_api/controllers/chat_controller_test.exs b/test/web/pleroma_api/controllers/chat_controller_test.exs
@@ -100,7 +100,7 @@ defmodule Pleroma.Web.PleromaAPI.ChatControllerTest do
|> post("/api/v1/pleroma/chats/#{chat.id}/messages")
|> json_response_and_validate_schema(400)
- assert result
+ assert %{"error" => "no_content"} == result
end
test "it works with an attachment", %{conn: conn, user: user} do
@@ -126,6 +126,23 @@ defmodule Pleroma.Web.PleromaAPI.ChatControllerTest do
assert result["attachment"]
end
+
+ test "gets MRF reason when rejected", %{conn: conn, user: user} do
+ clear_config([:mrf_keyword, :reject], ["GNO"])
+ clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.KeywordPolicy])
+
+ other_user = insert(:user)
+
+ {:ok, chat} = Chat.get_or_create(user.id, other_user.ap_id)
+
+ result =
+ conn
+ |> put_req_header("content-type", "application/json")
+ |> post("/api/v1/pleroma/chats/#{chat.id}/messages", %{"content" => "GNO/Linux"})
+ |> json_response_and_validate_schema(422)
+
+ assert %{"error" => "[KeywordPolicy] Matches with rejected keyword"} == result
+ end
end
describe "DELETE /api/v1/pleroma/chats/:id/messages/:message_id" do
