Merge branch 'features/validators-apc2s' into 'develop'

AP C2S: Remove restrictions and make it go through pipeline

See merge request pleroma/pleroma!3203

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - HTTPSecurityPlug now sends a response header to opt out of Google's FLoC (Federated Learning of Cohorts) targeted advertising.
 - Email address is now returned if requesting user is the owner of the user account so it can be exposed in client and FE user settings UIs.
 - Improved Twittercard and OpenGraph meta tag generation including thumbnails and image dimension metadata when available.
+- ActivityPub Client-to-Server(C2S): Limitation on the type of Activity/Object are lifted as they are now passed through ObjectValidators
 
 ### Added
 

lib/pleroma/activity.ex

@@ -292,7 +292,8 @@ def get_in_reply_to_activity(%Activity{} = activity) do
     get_in_reply_to_activity_from_object(Object.normalize(activity, fetch: false))
   end
 
-  def normalize(obj) when is_map(obj), do: get_by_ap_id_with_object(obj["id"])
+  def normalize(%Activity{data: %{"id" => ap_id}}), do: get_by_ap_id_with_object(ap_id)
+  def normalize(%{"id" => ap_id}), do: get_by_ap_id_with_object(ap_id)
   def normalize(ap_id) when is_binary(ap_id), do: get_by_ap_id_with_object(ap_id)
   def normalize(_), do: nil
 

lib/pleroma/web/activity_pub/activity_pub_controller.ex

@@ -11,7 +11,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
   alias Pleroma.Object.Fetcher
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
-  alias Pleroma.Web.ActivityPub.Builder
   alias Pleroma.Web.ActivityPub.InternalFetchActor
   alias Pleroma.Web.ActivityPub.ObjectView
   alias Pleroma.Web.ActivityPub.Pipeline
@@ -403,83 +402,90 @@ def read_inbox(%{assigns: %{user: %User{nickname: as_nickname}}} = conn, %{
     |> json(err)
   end
 
-  defp handle_user_activity(
+  defp fix_user_message(%User{ap_id: actor}, %{"type" => "Create", "object" => object} = activity)
-         %User{} = user,
+       when is_map(object) do
-         %{"type" => "Create", "object" => %{"type" => "Note"} = object} = params
+    length =
-       ) do
+      [object["content"], object["summary"], object["name"]]
-    content = if is_binary(object["content"]), do: object["content"], else: ""
+      |> Enum.filter(&is_binary(&1))
-    name = if is_binary(object["name"]), do: object["name"], else: ""
+      |> Enum.join("")
-    summary = if is_binary(object["summary"]), do: object["summary"], else: ""
+      |> String.length()
-    length = String.length(content <> name <> summary)
 
-    if length > Pleroma.Config.get([:instance, :limit]) do
+    limit = Pleroma.Config.get([:instance, :limit])
-      {:error, dgettext("errors", "Note is over the character limit")}
+
-    else
+    if length < limit do
       object =
         object
-        |> Map.merge(Map.take(params, ["to", "cc"]))
+        |> Transmogrifier.strip_internal_fields()
-        |> Map.put("attributedTo", user.ap_id)
+        |> Map.put("attributedTo", actor)
-        |> Transmogrifier.fix_object()
+        |> Map.put("actor", actor)
+        |> Map.put("id", Utils.generate_object_id())
 
-      ActivityPub.create(%{
+      {:ok, Map.put(activity, "object", object)}
-        to: params["to"],
-        actor: user,
-        context: object["context"],
-        object: object,
-        additional: Map.take(params, ["cc"])
-      })
-    end
-  end
-
-  defp handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do
-    with %Object{} = object <- Object.normalize(params["object"], fetch: false),
-         true <- user.is_moderator || user.ap_id == object.data["actor"],
-         {:ok, delete_data, _} <- Builder.delete(user, object.data["id"]),
-         {:ok, delete, _} <- Pipeline.common_pipeline(delete_data, local: true) do
-      {:ok, delete}
     else
-      _ -> {:error, dgettext("errors", "Can't delete object")}
+      {:error,
+       dgettext(
+         "errors",
+         "Character limit (%{limit} characters) exceeded, contains %{length} characters",
+         limit: limit,
+         length: length
+       )}
     end
   end
 
-  defp handle_user_activity(%User{} = user, %{"type" => "Like"} = params) do
+  defp fix_user_message(
-    with %Object{} = object <- Object.normalize(params["object"], fetch: false),
+         %User{ap_id: actor} = user,
-         {_, {:ok, like_object, meta}} <- {:build_object, Builder.like(user, object)},
+         %{"type" => "Delete", "object" => object} = activity
-         {_, {:ok, %Activity{} = activity, _meta}} <-
+       ) do
-           {:common_pipeline,
+    with {_, %Object{data: object_data}} <- {:normalize, Object.normalize(object, fetch: false)},
-            Pipeline.common_pipeline(like_object, Keyword.put(meta, :local, true))} do
+         {_, true} <- {:permission, user.is_moderator || actor == object_data["actor"]} do
       {:ok, activity}
     else
-      _ -> {:error, dgettext("errors", "Can't like object")}
+      {:normalize, _} ->
+        {:error, "No such object found"}
+
+      {:permission, _} ->
+        {:forbidden, "You can't delete this object"}
     end
   end
 
-  defp handle_user_activity(_, _) do
+  defp fix_user_message(%User{}, activity) do
-    {:error, dgettext("errors", "Unhandled activity type")}
+    {:ok, activity}
   end
 
   def update_outbox(
-        %{assigns: %{user: %User{nickname: nickname} = user}} = conn,
+        %{assigns: %{user: %User{nickname: nickname, ap_id: actor} = user}} = conn,
         %{"nickname" => nickname} = params
       ) do
-    actor = user.ap_id
-
     params =
       params
-      |> Map.drop(["id"])
+      |> Map.drop(["nickname"])
+      |> Map.put("id", Utils.generate_activity_id())
       |> Map.put("actor", actor)
-      |> Transmogrifier.fix_addressing()
 
-    with {:ok, %Activity{} = activity} <- handle_user_activity(user, params) do
+    with {:ok, params} <- fix_user_message(user, params),
+         {:ok, activity, _} <- Pipeline.common_pipeline(params, local: true),
+         %Activity{data: activity_data} <- Activity.normalize(activity) do
       conn
       |> put_status(:created)
-      |> put_resp_header("location", activity.data["id"])
+      |> put_resp_header("location", activity_data["id"])
-      |> json(activity.data)
+      |> json(activity_data)
     else
+      {:forbidden, message} ->
+        conn
+        |> put_status(:forbidden)
+        |> json(message)
+
       {:error, message} ->
         conn
         |> put_status(:bad_request)
         |> json(message)
+
+      e ->
+        Logger.warn(fn -> "AP C2S: #{inspect(e)}" end)
+
+        conn
+        |> put_status(:bad_request)
+        |> json("Bad Request")
     end
   end
 

lib/pleroma/web/activity_pub/object_validator.ex

@@ -175,6 +175,8 @@ def validate(%{"type" => type} = object, meta) when type in ~w(Add Remove) do
     end
   end
 
+  def validate(o, m), do: {:error, {:validator_not_set, {o, m}}}
+
   def cast_and_apply(%{"type" => "ChatMessage"} = object) do
     ChatMessageValidator.cast_and_apply(object)
   end

test/pleroma/web/activity_pub/activity_pub_controller_test.exs

@@ -1334,9 +1334,12 @@ test "It returns poll Answers when authenticated", %{conn: conn} do
         activity: %{
           "@context" => "https://www.w3.org/ns/activitystreams",
           "type" => "Create",
-          "object" => %{"type" => "Note", "content" => "AP C2S test"},
+          "object" => %{
-          "to" => "https://www.w3.org/ns/activitystreams#Public",
+            "type" => "Note",
-          "cc" => []
+            "content" => "AP C2S test",
+            "to" => "https://www.w3.org/ns/activitystreams#Public",
+            "cc" => []
+          }
         }
       ]
     end
@@ -1442,19 +1445,19 @@ test "it erects a tombstone when receiving a delete activity", %{conn: conn} do
       user = User.get_cached_by_ap_id(note_activity.data["actor"])
 
       data = %{
-        type: "Delete",
+        "type" => "Delete",
-        object: %{
+        "object" => %{
-          id: note_object.data["id"]
+          "id" => note_object.data["id"]
         }
       }
 
-      conn =
+      result =
         conn
         |> assign(:user, user)
         |> put_req_header("content-type", "application/activity+json")
         |> post("/users/#{user.nickname}/outbox", data)
+        |> json_response(201)
 
-      result = json_response(conn, 201)
       assert Activity.get_by_ap_id(result["id"])
 
       assert object = Object.get_by_ap_id(note_object.data["id"])
@@ -1479,7 +1482,7 @@ test "it rejects delete activity of object from other actor", %{conn: conn} do
         |> put_req_header("content-type", "application/activity+json")
         |> post("/users/#{user.nickname}/outbox", data)
 
-      assert json_response(conn, 400)
+      assert json_response(conn, 403)
     end
 
     test "it increases like count when receiving a like action", %{conn: conn} do
@@ -1557,7 +1560,7 @@ test "Character limitation", %{conn: conn, activity: activity} do
         |> post("/users/#{user.nickname}/outbox", activity)
         |> json_response(400)
 
-      assert result == "Note is over the character limit"
+      assert result == "Character limit (5 characters) exceeded, contains 11 characters"
     end
   end
 
@@ -1934,10 +1937,10 @@ test "POST /api/ap/upload_media", %{conn: conn} do
         "object" => %{
           "type" => "Note",
           "content" => "AP C2S test, attachment",
-          "attachment" => [object]
+          "attachment" => [object],
-        },
+          "to" => "https://www.w3.org/ns/activitystreams#Public",
-        "to" => "https://www.w3.org/ns/activitystreams#Public",
+          "cc" => []
-        "cc" => []
+        }
       }
 
       activity_response =