Create activity handling: Flip it and reverse it

Both objects and create activities will now go through the common
pipeline and will be validated. Objects are now created as a side
effect of the Create activity, rolling back a transaction if it's
not possible to insert the object.

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, []}

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),

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}

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: %{})

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, meta \\ []) do
-  def cast_and_validate(data) 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

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 <https://pleroma.social/>
+# 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

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)

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)
+    with {:ok, _object, _meta} <- handle_object_creation(meta[:object_data], meta) do
-
-    {:ok, _object} = handle_object_creation(object)
-
       Notification.create_notifications(activity)
-
       {:ok, activity, meta}
+    else
+      e -> Repo.rollback(e)
+    end
   end
 
   # Nothing to do
@@ -45,7 +50,8 @@ def handle(object, meta) do
     {:ok, object, meta}
   end
 
-  def handle_object_creation(%{data: %{"type" => "ChatMessage"}} = object) do
+  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"]))
 
@@ -56,7 +62,8 @@ def handle_object_creation(%{data: %{"type" => "ChatMessage"}} = object) do
         end
       end)
 
-    {:ok, object}
+      {:ok, object, meta}
+    end
   end
 
   # Nothing to do

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 ->

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}

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

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 <https://pleroma.social/>
+# 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 <script>alert('foo')</script>"
+    assert {:ok, "hey look xss alert(&#39;foo&#39;)"} == SafeText.cast(text)
+  end
+
+  test "errors for non-text" do
+    assert :error == SafeText.cast(1)
+  end
+end

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)

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