diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex
index 73e19bf97..d96c12440 100644
--- a/lib/pleroma/notification.ex
+++ b/lib/pleroma/notification.ex
@@ -275,7 +275,7 @@ def dismiss(%{id: user_id} = _user, id) do
end
def create_notifications(%Activity{data: %{"to" => _, "type" => "Create"}} = activity) do
- object = Object.normalize(activity)
+ object = Object.normalize(activity, false)
if object && object.data["type"] == "Answer" do
{:ok, []}
diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
index 69ac06f6b..ecb13d76a 100644
--- a/lib/pleroma/web/activity_pub/activity_pub.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -126,7 +126,14 @@ def increase_poll_votes_if_vote(%{
def increase_poll_votes_if_vote(_create_data), do: :noop
+ @object_types ["ChatMessage"]
@spec persist(map(), keyword()) :: {:ok, Activity.t() | Object.t()}
+ def persist(%{"type" => type} = object, meta) when type in @object_types do
+ with {:ok, object} <- Object.create(object) do
+ {:ok, object, meta}
+ end
+ end
+
def persist(object, meta) do
with local <- Keyword.fetch!(meta, :local),
{recipients, _, _} <- get_recipients(object),
diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex
index 03db681ec..a4da9242a 100644
--- a/lib/pleroma/web/activity_pub/object_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validator.ex
@@ -23,7 +23,7 @@ def validate(%{"type" => "Like"} = object, meta) do
object
|> LikeValidator.cast_and_validate()
|> Ecto.Changeset.apply_action(:insert) do
- object = stringify_keys(object |> Map.from_struct())
+ object = stringify_keys(object)
{:ok, object, meta}
end
end
@@ -41,7 +41,7 @@ def validate(%{"type" => "ChatMessage"} = object, meta) do
def validate(%{"type" => "Create"} = object, meta) do
with {:ok, object} <-
object
- |> CreateChatMessageValidator.cast_and_validate()
+ |> CreateChatMessageValidator.cast_and_validate(meta)
|> Ecto.Changeset.apply_action(:insert) do
object = stringify_keys(object)
{:ok, object, meta}
diff --git a/lib/pleroma/web/activity_pub/object_validators/chat_message_validator.ex b/lib/pleroma/web/activity_pub/object_validators/chat_message_validator.ex
index f07045d9d..e87c1ac2e 100644
--- a/lib/pleroma/web/activity_pub/object_validators/chat_message_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validators/chat_message_validator.ex
@@ -18,7 +18,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ChatMessageValidator do
field(:id, Types.ObjectID, primary_key: true)
field(:to, Types.Recipients, default: [])
field(:type, :string)
- field(:content, :string)
+ field(:content, Types.SafeText)
field(:actor, Types.ObjectID)
field(:published, Types.DateTime)
field(:emoji, :map, default: %{})
diff --git a/lib/pleroma/web/activity_pub/object_validators/create_chat_message_validator.ex b/lib/pleroma/web/activity_pub/object_validators/create_chat_message_validator.ex
index ce52d5623..21c7a5ba4 100644
--- a/lib/pleroma/web/activity_pub/object_validators/create_chat_message_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validators/create_chat_message_validator.ex
@@ -33,8 +33,31 @@ def cast_data(data) do
cast(%__MODULE__{}, data, __schema__(:fields))
end
- # No validation yet
- def cast_and_validate(data) do
+ def cast_and_validate(data, meta \\ []) do
cast_data(data)
+ |> validate_data(meta)
+ end
+
+ def validate_data(cng, meta \\ []) do
+ cng
+ |> validate_required([:id, :actor, :to, :type, :object])
+ |> validate_inclusion(:type, ["Create"])
+ |> validate_recipients_match(meta)
+ end
+
+ def validate_recipients_match(cng, meta) do
+ object_recipients = meta[:object_data]["to"] || []
+
+ cng
+ |> validate_change(:to, fn :to, recipients ->
+ activity_set = MapSet.new(recipients)
+ object_set = MapSet.new(object_recipients)
+
+ if MapSet.equal?(activity_set, object_set) do
+ []
+ else
+ [{:to, "Recipients don't match with object recipients"}]
+ end
+ end)
end
end
diff --git a/lib/pleroma/web/activity_pub/object_validators/types/safe_text.ex b/lib/pleroma/web/activity_pub/object_validators/types/safe_text.ex
new file mode 100644
index 000000000..822e8d2c1
--- /dev/null
+++ b/lib/pleroma/web/activity_pub/object_validators/types/safe_text.ex
@@ -0,0 +1,25 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.ActivityPub.ObjectValidators.Types.SafeText do
+ use Ecto.Type
+
+ alias Pleroma.HTML
+
+ def type, do: :string
+
+ def cast(str) when is_binary(str) do
+ {:ok, HTML.strip_tags(str)}
+ end
+
+ def cast(_), do: :error
+
+ def dump(data) do
+ {:ok, data}
+ end
+
+ def load(data) do
+ {:ok, data}
+ end
+end
diff --git a/lib/pleroma/web/activity_pub/pipeline.ex b/lib/pleroma/web/activity_pub/pipeline.ex
index 7ccee54c9..4213ba751 100644
--- a/lib/pleroma/web/activity_pub/pipeline.ex
+++ b/lib/pleroma/web/activity_pub/pipeline.ex
@@ -4,20 +4,22 @@
defmodule Pleroma.Web.ActivityPub.Pipeline do
alias Pleroma.Activity
+ alias Pleroma.Object
alias Pleroma.Web.ActivityPub.ActivityPub
alias Pleroma.Web.ActivityPub.MRF
alias Pleroma.Web.ActivityPub.ObjectValidator
alias Pleroma.Web.ActivityPub.SideEffects
alias Pleroma.Web.Federator
- @spec common_pipeline(map(), keyword()) :: {:ok, Activity.t(), keyword()} | {:error, any()}
+ @spec common_pipeline(map(), keyword()) ::
+ {:ok, Activity.t() | Object.t(), keyword()} | {:error, any()}
def 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, %Activity{} = activity, meta}} <-
+ {_, {:ok, activity, meta}} <-
{:persist_object, ActivityPub.persist(mrfd_object, meta)},
- {_, {:ok, %Activity{} = activity, meta}} <-
+ {_, {:ok, activity, meta}} <-
{:execute_side_effects, SideEffects.handle(activity, meta)},
{_, {:ok, _}} <- {:federation, maybe_federate(activity, meta)} do
{:ok, activity, meta}
@@ -27,7 +29,9 @@ def common_pipeline(object, meta) do
end
end
- defp maybe_federate(activity, meta) do
+ defp maybe_federate(%Object{}, _), do: {:ok, :not_federated}
+
+ defp maybe_federate(%Activity{} = activity, meta) do
with {:ok, local} <- Keyword.fetch(meta, :local) do
if local do
Federator.publish(activity)
diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex
index a2b4da8d6..794a46267 100644
--- a/lib/pleroma/web/activity_pub/side_effects.ex
+++ b/lib/pleroma/web/activity_pub/side_effects.ex
@@ -8,7 +8,9 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
alias Pleroma.Chat
alias Pleroma.Notification
alias Pleroma.Object
+ alias Pleroma.Repo
alias Pleroma.User
+ alias Pleroma.Web.ActivityPub.Pipeline
alias Pleroma.Web.ActivityPub.Utils
def handle(object, meta \\ [])
@@ -30,14 +32,17 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do
result
end
+ # Tasks this handles
+ # - Actually create object
+ # - Rollback if we couldn't create it
+ # - Set up notifications
def handle(%{data: %{"type" => "Create"}} = activity, meta) do
- object = Object.normalize(activity, false)
-
- {:ok, _object} = handle_object_creation(object)
-
- Notification.create_notifications(activity)
-
- {:ok, activity, meta}
+ with {:ok, _object, _meta} <- handle_object_creation(meta[:object_data], meta) do
+ Notification.create_notifications(activity)
+ {:ok, activity, meta}
+ else
+ e -> Repo.rollback(e)
+ end
end
# Nothing to do
@@ -45,18 +50,20 @@ def handle(object, meta) do
{:ok, object, meta}
end
- def handle_object_creation(%{data: %{"type" => "ChatMessage"}} = object) do
- actor = User.get_cached_by_ap_id(object.data["actor"])
- recipient = User.get_cached_by_ap_id(hd(object.data["to"]))
+ def handle_object_creation(%{"type" => "ChatMessage"} = object, meta) do
+ with {:ok, object, meta} <- Pipeline.common_pipeline(object, meta) do
+ actor = User.get_cached_by_ap_id(object.data["actor"])
+ recipient = User.get_cached_by_ap_id(hd(object.data["to"]))
- [[actor, recipient], [recipient, actor]]
- |> Enum.each(fn [user, other_user] ->
- if user.local do
- Chat.bump_or_create(user.id, other_user.ap_id)
- end
- end)
+ [[actor, recipient], [recipient, actor]]
+ |> Enum.each(fn [user, other_user] ->
+ if user.local do
+ Chat.bump_or_create(user.id, other_user.ap_id)
+ end
+ end)
- {:ok, object}
+ {:ok, object, meta}
+ end
end
# Nothing to do
diff --git a/lib/pleroma/web/activity_pub/transmogrifier/chat_message_handling.ex b/lib/pleroma/web/activity_pub/transmogrifier/chat_message_handling.ex
index cfe3b767b..043d847d1 100644
--- a/lib/pleroma/web/activity_pub/transmogrifier/chat_message_handling.ex
+++ b/lib/pleroma/web/activity_pub/transmogrifier/chat_message_handling.ex
@@ -3,31 +3,39 @@
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.ActivityPub.Transmogrifier.ChatMessageHandling do
- alias Pleroma.Object
+ alias Pleroma.Repo
alias Pleroma.Web.ActivityPub.ObjectValidator
alias Pleroma.Web.ActivityPub.ObjectValidators.ChatMessageValidator
alias Pleroma.Web.ActivityPub.ObjectValidators.CreateChatMessageValidator
alias Pleroma.Web.ActivityPub.Pipeline
def handle_incoming(
- %{"type" => "Create", "object" => %{"type" => "ChatMessage"} = object_data} = data,
+ %{"type" => "Create", "object" => %{"type" => "ChatMessage"}} = data,
_options
) do
+ # Create has to be run inside a transaction because the object is created as a side effect.
+ # If this does not work, we need to roll back creating the activity.
+ case Repo.transaction(fn -> do_handle_incoming(data) end) do
+ {:ok, value} ->
+ value
+
+ {:error, e} ->
+ {:error, e}
+ end
+ end
+
+ def do_handle_incoming(
+ %{"type" => "Create", "object" => %{"type" => "ChatMessage"} = object_data} = data
+ ) do
with {_, {:ok, cast_data_sym}} <-
{:casting_data, data |> CreateChatMessageValidator.cast_and_apply()},
cast_data = ObjectValidator.stringify_keys(cast_data_sym),
{_, {:ok, object_cast_data_sym}} <-
{:casting_object_data, object_data |> ChatMessageValidator.cast_and_apply()},
object_cast_data = ObjectValidator.stringify_keys(object_cast_data_sym),
- # For now, just strip HTML
- stripped_content = Pleroma.HTML.strip_tags(object_cast_data["content"]),
- object_cast_data = object_cast_data |> Map.put("content", stripped_content),
- {_, true} <- {:to_fields_match, cast_data["to"] == object_cast_data["to"]},
- {_, {:ok, validated_object, _meta}} <-
- {:validate_object, ObjectValidator.validate(object_cast_data, %{})},
- {_, {:ok, _created_object}} <- {:persist_object, Object.create(validated_object)},
{_, {:ok, activity, _meta}} <-
- {:common_pipeline, Pipeline.common_pipeline(cast_data, local: false)} do
+ {:common_pipeline,
+ Pipeline.common_pipeline(cast_data, local: false, object_data: object_cast_data)} do
{:ok, activity}
else
e ->
diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex
index 5eb221668..c39d1cee6 100644
--- a/lib/pleroma/web/common_api/common_api.ex
+++ b/lib/pleroma/web/common_api/common_api.ex
@@ -38,13 +38,15 @@ def post_chat_message(%User{} = user, %User{} = recipient, content) do
recipient.ap_id,
content |> Formatter.html_escape("text/plain")
)},
- {_, {:ok, chat_message_object}} <-
- {:create_object, Object.create(chat_message_data)},
{_, {:ok, create_activity_data, _meta}} <-
{:build_create_activity,
- Builder.create(user, chat_message_object.data["id"], [recipient.ap_id])},
+ Builder.create(user, chat_message_data["id"], [recipient.ap_id])},
{_, {:ok, %Activity{} = activity, _meta}} <-
- {:common_pipeline, Pipeline.common_pipeline(create_activity_data, local: true)} do
+ {:common_pipeline,
+ Pipeline.common_pipeline(create_activity_data,
+ local: true,
+ object_data: chat_message_data
+ )} do
{:ok, activity}
else
{:content_length, false} -> {:error, :content_too_long}
diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex
index 945e63e22..4afdf80af 100644
--- a/lib/pleroma/web/common_api/utils.ex
+++ b/lib/pleroma/web/common_api/utils.ex
@@ -425,7 +425,7 @@ def maybe_notify_mentioned_recipients(
%Activity{data: %{"to" => _to, "type" => type} = data} = activity
)
when type == "Create" do
- object = Object.normalize(activity)
+ object = Object.normalize(activity, false)
object_data =
cond do
diff --git a/test/web/activity_pub/object_validators/types/safe_text_test.exs b/test/web/activity_pub/object_validators/types/safe_text_test.exs
new file mode 100644
index 000000000..59ed0a1fe
--- /dev/null
+++ b/test/web/activity_pub/object_validators/types/safe_text_test.exs
@@ -0,0 +1,23 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.ActivityPub.ObjectValidators.Types.SafeTextTest do
+ use Pleroma.DataCase
+
+ alias Pleroma.Web.ActivityPub.ObjectValidators.Types.SafeText
+
+ test "it lets normal text go through" do
+ text = "hey how are you"
+ assert {:ok, text} == SafeText.cast(text)
+ end
+
+ test "it removes html tags from text" do
+ text = "hey look xss "
+ assert {:ok, "hey look xss alert('foo')"} == SafeText.cast(text)
+ end
+
+ test "errors for non-text" do
+ assert :error == SafeText.cast(1)
+ end
+end
diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs
index 2889a577c..19abac6a6 100644
--- a/test/web/activity_pub/side_effects_test.exs
+++ b/test/web/activity_pub/side_effects_test.exs
@@ -47,14 +47,14 @@ test "notifies the recipient" do
recipient = insert(:user, local: true)
{:ok, chat_message_data, _meta} = Builder.chat_message(author, recipient.ap_id, "hey")
- {:ok, chat_message_object} = Object.create(chat_message_data)
{:ok, create_activity_data, _meta} =
- Builder.create(author, chat_message_object.data["id"], [recipient.ap_id])
+ Builder.create(author, chat_message_data["id"], [recipient.ap_id])
{:ok, create_activity, _meta} = ActivityPub.persist(create_activity_data, local: false)
- {:ok, _create_activity, _meta} = SideEffects.handle(create_activity)
+ {:ok, _create_activity, _meta} =
+ SideEffects.handle(create_activity, local: false, object_data: chat_message_data)
assert Repo.get_by(Notification, user_id: recipient.id, activity_id: create_activity.id)
end
@@ -64,14 +64,17 @@ test "it creates a Chat for the local users and bumps the unread count" do
recipient = insert(:user, local: true)
{:ok, chat_message_data, _meta} = Builder.chat_message(author, recipient.ap_id, "hey")
- {:ok, chat_message_object} = Object.create(chat_message_data)
{:ok, create_activity_data, _meta} =
- Builder.create(author, chat_message_object.data["id"], [recipient.ap_id])
+ Builder.create(author, chat_message_data["id"], [recipient.ap_id])
{:ok, create_activity, _meta} = ActivityPub.persist(create_activity_data, local: false)
- {:ok, _create_activity, _meta} = SideEffects.handle(create_activity)
+ {:ok, _create_activity, _meta} =
+ SideEffects.handle(create_activity, local: false, object_data: chat_message_data)
+
+ # An object is created
+ assert Object.get_by_ap_id(chat_message_data["id"])
# The remote user won't get a chat
chat = Chat.get(author.id, recipient.ap_id)
@@ -85,14 +88,14 @@ test "it creates a Chat for the local users and bumps the unread count" do
recipient = insert(:user, local: true)
{:ok, chat_message_data, _meta} = Builder.chat_message(author, recipient.ap_id, "hey")
- {:ok, chat_message_object} = Object.create(chat_message_data)
{:ok, create_activity_data, _meta} =
- Builder.create(author, chat_message_object.data["id"], [recipient.ap_id])
+ Builder.create(author, chat_message_data["id"], [recipient.ap_id])
{:ok, create_activity, _meta} = ActivityPub.persist(create_activity_data, local: false)
- {:ok, _create_activity, _meta} = SideEffects.handle(create_activity)
+ {:ok, _create_activity, _meta} =
+ SideEffects.handle(create_activity, local: false, object_data: chat_message_data)
# Both users are local and get the chat
chat = Chat.get(author.id, recipient.ap_id)
diff --git a/test/web/activity_pub/transmogrifier/chat_message_test.exs b/test/web/activity_pub/transmogrifier/chat_message_test.exs
index a63a31e6e..ceaee614c 100644
--- a/test/web/activity_pub/transmogrifier/chat_message_test.exs
+++ b/test/web/activity_pub/transmogrifier/chat_message_test.exs
@@ -55,7 +55,8 @@ test "it rejects messages where the `to` field of activity and object don't matc
data
|> Map.put("to", author.ap_id)
- {:error, _} = Transmogrifier.handle_incoming(data)
+ assert match?({:error, _}, Transmogrifier.handle_incoming(data))
+ refute Object.get_by_ap_id(data["object"]["id"])
end
test "it inserts it and creates a chat" do