From 015f9258a9bd1430ab079f449b118b664c3b9664 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Tue, 16 Jun 2020 14:48:46 +0200
Subject: [PATCH 01/12] Transmogrifier: Extract user update handling tests.

---
 .../user_update_handling_test.exs             | 154 +++++++++++++++++
 test/web/activity_pub/transmogrifier_test.exs | 156 ------------------
 2 files changed, 154 insertions(+), 156 deletions(-)
 create mode 100644 test/web/activity_pub/transmogrifier/user_update_handling_test.exs

diff --git a/test/web/activity_pub/transmogrifier/user_update_handling_test.exs b/test/web/activity_pub/transmogrifier/user_update_handling_test.exs
new file mode 100644
index 000000000..8e5d3b883
--- /dev/null
+++ b/test/web/activity_pub/transmogrifier/user_update_handling_test.exs
@@ -0,0 +1,154 @@
+# 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.Transmogrifier.UserUpdateHandlingTest do
+  use Pleroma.DataCase
+
+  alias Pleroma.Activity
+  alias Pleroma.User
+  alias Pleroma.Web.ActivityPub.Transmogrifier
+
+  import Pleroma.Factory
+
+  test "it works for incoming update activities" do
+    user = insert(:user, local: false)
+
+    update_data = File.read!("test/fixtures/mastodon-update.json") |> Poison.decode!()
+
+    object =
+      update_data["object"]
+      |> Map.put("actor", user.ap_id)
+      |> Map.put("id", user.ap_id)
+
+    update_data =
+      update_data
+      |> Map.put("actor", user.ap_id)
+      |> Map.put("object", object)
+
+    {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(update_data)
+
+    assert data["id"] == update_data["id"]
+
+    user = User.get_cached_by_ap_id(data["actor"])
+    assert user.name == "gargle"
+
+    assert user.avatar["url"] == [
+             %{
+               "href" =>
+                 "https://cd.niu.moe/accounts/avatars/000/033/323/original/fd7f8ae0b3ffedc9.jpeg"
+             }
+           ]
+
+    assert user.banner["url"] == [
+             %{
+               "href" =>
+                 "https://cd.niu.moe/accounts/headers/000/033/323/original/850b3448fa5fd477.png"
+             }
+           ]
+
+    assert user.bio == "<p>Some bio</p>"
+  end
+
+  test "it works with alsoKnownAs" do
+    %{ap_id: actor} = insert(:user, local: false)
+
+    assert User.get_cached_by_ap_id(actor).also_known_as == []
+
+    {:ok, _activity} =
+      "test/fixtures/mastodon-update.json"
+      |> File.read!()
+      |> Poison.decode!()
+      |> Map.put("actor", actor)
+      |> Map.update!("object", fn object ->
+        object
+        |> Map.put("actor", actor)
+        |> Map.put("id", actor)
+        |> Map.put("alsoKnownAs", [
+          "http://mastodon.example.org/users/foo",
+          "http://example.org/users/bar"
+        ])
+      end)
+      |> Transmogrifier.handle_incoming()
+
+    assert User.get_cached_by_ap_id(actor).also_known_as == [
+             "http://mastodon.example.org/users/foo",
+             "http://example.org/users/bar"
+           ]
+  end
+
+  test "it works with custom profile fields" do
+    user = insert(:user, local: false)
+
+    assert user.fields == []
+
+    update_data = File.read!("test/fixtures/mastodon-update.json") |> Poison.decode!()
+
+    object =
+      update_data["object"]
+      |> Map.put("actor", user.ap_id)
+      |> Map.put("id", user.ap_id)
+
+    update_data =
+      update_data
+      |> Map.put("actor", user.ap_id)
+      |> Map.put("object", object)
+
+    {:ok, _update_activity} = Transmogrifier.handle_incoming(update_data)
+
+    user = User.get_cached_by_ap_id(user.ap_id)
+
+    assert user.fields == [
+             %{"name" => "foo", "value" => "updated"},
+             %{"name" => "foo1", "value" => "updated"}
+           ]
+
+    Pleroma.Config.put([:instance, :max_remote_account_fields], 2)
+
+    update_data =
+      put_in(update_data, ["object", "attachment"], [
+        %{"name" => "foo", "type" => "PropertyValue", "value" => "bar"},
+        %{"name" => "foo11", "type" => "PropertyValue", "value" => "bar11"},
+        %{"name" => "foo22", "type" => "PropertyValue", "value" => "bar22"}
+      ])
+
+    {:ok, _} = Transmogrifier.handle_incoming(update_data)
+
+    user = User.get_cached_by_ap_id(user.ap_id)
+
+    assert user.fields == [
+             %{"name" => "foo", "value" => "updated"},
+             %{"name" => "foo1", "value" => "updated"}
+           ]
+
+    update_data = put_in(update_data, ["object", "attachment"], [])
+
+    {:ok, _} = Transmogrifier.handle_incoming(update_data)
+
+    user = User.get_cached_by_ap_id(user.ap_id)
+
+    assert user.fields == []
+  end
+
+  test "it works for incoming update activities which lock the account" do
+    user = insert(:user, local: false)
+
+    update_data = File.read!("test/fixtures/mastodon-update.json") |> Poison.decode!()
+
+    object =
+      update_data["object"]
+      |> Map.put("actor", user.ap_id)
+      |> Map.put("id", user.ap_id)
+      |> Map.put("manuallyApprovesFollowers", true)
+
+    update_data =
+      update_data
+      |> Map.put("actor", user.ap_id)
+      |> Map.put("object", object)
+
+    {:ok, %Activity{local: false}} = Transmogrifier.handle_incoming(update_data)
+
+    user = User.get_cached_by_ap_id(user.ap_id)
+    assert user.locked == true
+  end
+end
diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs
index 94d8552e8..b542bb7b8 100644
--- a/test/web/activity_pub/transmogrifier_test.exs
+++ b/test/web/activity_pub/transmogrifier_test.exs
@@ -401,162 +401,6 @@ test "it strips internal reactions" do
       refute Map.has_key?(object_data, "reaction_count")
     end
 
-    test "it works for incoming update activities" do
-      data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!()
-
-      {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data)
-      update_data = File.read!("test/fixtures/mastodon-update.json") |> Poison.decode!()
-
-      object =
-        update_data["object"]
-        |> Map.put("actor", data["actor"])
-        |> Map.put("id", data["actor"])
-
-      update_data =
-        update_data
-        |> Map.put("actor", data["actor"])
-        |> Map.put("object", object)
-
-      {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(update_data)
-
-      assert data["id"] == update_data["id"]
-
-      user = User.get_cached_by_ap_id(data["actor"])
-      assert user.name == "gargle"
-
-      assert user.avatar["url"] == [
-               %{
-                 "href" =>
-                   "https://cd.niu.moe/accounts/avatars/000/033/323/original/fd7f8ae0b3ffedc9.jpeg"
-               }
-             ]
-
-      assert user.banner["url"] == [
-               %{
-                 "href" =>
-                   "https://cd.niu.moe/accounts/headers/000/033/323/original/850b3448fa5fd477.png"
-               }
-             ]
-
-      assert user.bio == "<p>Some bio</p>"
-    end
-
-    test "it works with alsoKnownAs" do
-      {:ok, %Activity{data: %{"actor" => actor}}} =
-        "test/fixtures/mastodon-post-activity.json"
-        |> File.read!()
-        |> Poison.decode!()
-        |> Transmogrifier.handle_incoming()
-
-      assert User.get_cached_by_ap_id(actor).also_known_as == ["http://example.org/users/foo"]
-
-      {:ok, _activity} =
-        "test/fixtures/mastodon-update.json"
-        |> File.read!()
-        |> Poison.decode!()
-        |> Map.put("actor", actor)
-        |> Map.update!("object", fn object ->
-          object
-          |> Map.put("actor", actor)
-          |> Map.put("id", actor)
-          |> Map.put("alsoKnownAs", [
-            "http://mastodon.example.org/users/foo",
-            "http://example.org/users/bar"
-          ])
-        end)
-        |> Transmogrifier.handle_incoming()
-
-      assert User.get_cached_by_ap_id(actor).also_known_as == [
-               "http://mastodon.example.org/users/foo",
-               "http://example.org/users/bar"
-             ]
-    end
-
-    test "it works with custom profile fields" do
-      {:ok, activity} =
-        "test/fixtures/mastodon-post-activity.json"
-        |> File.read!()
-        |> Poison.decode!()
-        |> Transmogrifier.handle_incoming()
-
-      user = User.get_cached_by_ap_id(activity.actor)
-
-      assert user.fields == [
-               %{"name" => "foo", "value" => "bar"},
-               %{"name" => "foo1", "value" => "bar1"}
-             ]
-
-      update_data = File.read!("test/fixtures/mastodon-update.json") |> Poison.decode!()
-
-      object =
-        update_data["object"]
-        |> Map.put("actor", user.ap_id)
-        |> Map.put("id", user.ap_id)
-
-      update_data =
-        update_data
-        |> Map.put("actor", user.ap_id)
-        |> Map.put("object", object)
-
-      {:ok, _update_activity} = Transmogrifier.handle_incoming(update_data)
-
-      user = User.get_cached_by_ap_id(user.ap_id)
-
-      assert user.fields == [
-               %{"name" => "foo", "value" => "updated"},
-               %{"name" => "foo1", "value" => "updated"}
-             ]
-
-      Pleroma.Config.put([:instance, :max_remote_account_fields], 2)
-
-      update_data =
-        put_in(update_data, ["object", "attachment"], [
-          %{"name" => "foo", "type" => "PropertyValue", "value" => "bar"},
-          %{"name" => "foo11", "type" => "PropertyValue", "value" => "bar11"},
-          %{"name" => "foo22", "type" => "PropertyValue", "value" => "bar22"}
-        ])
-
-      {:ok, _} = Transmogrifier.handle_incoming(update_data)
-
-      user = User.get_cached_by_ap_id(user.ap_id)
-
-      assert user.fields == [
-               %{"name" => "foo", "value" => "updated"},
-               %{"name" => "foo1", "value" => "updated"}
-             ]
-
-      update_data = put_in(update_data, ["object", "attachment"], [])
-
-      {:ok, _} = Transmogrifier.handle_incoming(update_data)
-
-      user = User.get_cached_by_ap_id(user.ap_id)
-
-      assert user.fields == []
-    end
-
-    test "it works for incoming update activities which lock the account" do
-      data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!()
-
-      {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data)
-      update_data = File.read!("test/fixtures/mastodon-update.json") |> Poison.decode!()
-
-      object =
-        update_data["object"]
-        |> Map.put("actor", data["actor"])
-        |> Map.put("id", data["actor"])
-        |> Map.put("manuallyApprovesFollowers", true)
-
-      update_data =
-        update_data
-        |> Map.put("actor", data["actor"])
-        |> Map.put("object", object)
-
-      {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(update_data)
-
-      user = User.get_cached_by_ap_id(data["actor"])
-      assert user.locked == true
-    end
-
     test "it works for incomming unfollows with an existing follow" do
       user = insert(:user)
 

From abdb540d450b5e68ea452f78d865d63bca764a49 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Fri, 19 Jun 2020 15:30:30 +0200
Subject: [PATCH 02/12] ObjectValidators: Add basic UpdateValidator.

---
 lib/pleroma/web/activity_pub/builder.ex       | 15 +++++++
 .../web/activity_pub/object_validator.ex      | 11 +++++
 .../object_validators/update_validator.ex     | 43 +++++++++++++++++++
 .../activity_pub/object_validator_test.exs    | 20 +++++++++
 4 files changed, 89 insertions(+)
 create mode 100644 lib/pleroma/web/activity_pub/object_validators/update_validator.ex

diff --git a/lib/pleroma/web/activity_pub/builder.ex b/lib/pleroma/web/activity_pub/builder.ex
index 1aac62c69..135a5c431 100644
--- a/lib/pleroma/web/activity_pub/builder.ex
+++ b/lib/pleroma/web/activity_pub/builder.ex
@@ -123,6 +123,21 @@ def like(actor, object) do
     end
   end
 
+  # Retricted to user updates for now, always public
+  @spec update(User.t(), Object.t()) :: {:ok, map(), keyword()}
+  def update(actor, object) do
+    to = [Pleroma.Constants.as_public(), actor.follower_address]
+
+    {:ok,
+     %{
+       "id" => Utils.generate_activity_id(),
+       "type" => "Update",
+       "actor" => actor.ap_id,
+       "object" => object,
+       "to" => to
+     }, []}
+  end
+
   @spec announce(User.t(), Object.t(), keyword()) :: {:ok, map(), keyword()}
   def announce(actor, object, options \\ []) do
     public? = Keyword.get(options, :public, false)
diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex
index 6a83a2c33..804a9d06e 100644
--- a/lib/pleroma/web/activity_pub/object_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validator.ex
@@ -19,10 +19,21 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
   alias Pleroma.Web.ActivityPub.ObjectValidators.EmojiReactValidator
   alias Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator
   alias Pleroma.Web.ActivityPub.ObjectValidators.UndoValidator
+  alias Pleroma.Web.ActivityPub.ObjectValidators.UpdateValidator
 
   @spec validate(map(), keyword()) :: {:ok, map(), keyword()} | {:error, any()}
   def validate(object, meta)
 
+  def validate(%{"type" => "Update"} = object, meta) do
+    with {:ok, object} <-
+           object
+           |> UpdateValidator.cast_and_validate()
+           |> Ecto.Changeset.apply_action(:insert) do
+      object = stringify_keys(object)
+      {:ok, object, meta}
+    end
+  end
+
   def validate(%{"type" => "Undo"} = object, meta) do
     with {:ok, object} <-
            object
diff --git a/lib/pleroma/web/activity_pub/object_validators/update_validator.ex b/lib/pleroma/web/activity_pub/object_validators/update_validator.ex
new file mode 100644
index 000000000..94d72491b
--- /dev/null
+++ b/lib/pleroma/web/activity_pub/object_validators/update_validator.ex
@@ -0,0 +1,43 @@
+# 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.UpdateValidator do
+  use Ecto.Schema
+
+  alias Pleroma.EctoType.ActivityPub.ObjectValidators
+
+  import Ecto.Changeset
+  import Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations
+
+  @primary_key false
+
+  embedded_schema do
+    field(:id, ObjectValidators.ObjectID, primary_key: true)
+    field(:type, :string)
+    field(:actor, ObjectValidators.ObjectID)
+    field(:to, ObjectValidators.Recipients, default: [])
+    field(:cc, ObjectValidators.Recipients, default: [])
+    # In this case, we save the full object in this activity instead of just a
+    # reference, so we can always see what was actually changed by this.
+    field(:object, :map)
+  end
+
+  def cast_data(data) do
+    %__MODULE__{}
+    |> cast(data, __schema__(:fields))
+  end
+
+  def validate_data(cng) do
+    cng
+    |> validate_required([:id, :type, :actor, :to, :cc, :object])
+    |> validate_inclusion(:type, ["Update"])
+    |> validate_actor_presence()
+  end
+
+  def cast_and_validate(data) do
+    data
+    |> cast_data
+    |> validate_data
+  end
+end
diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs
index 31224abe0..adb56092d 100644
--- a/test/web/activity_pub/object_validator_test.exs
+++ b/test/web/activity_pub/object_validator_test.exs
@@ -622,4 +622,24 @@ test "returns an error if the actor can't announce the object", %{
       assert {:actor, {"can not announce this object publicly", []}} in cng.errors
     end
   end
+
+  describe "updates" do
+    setup do
+      user = insert(:user)
+
+      object = %{
+        "id" => user.ap_id,
+        "name" => "A new name",
+        "summary" => "A new bio"
+      }
+
+      {:ok, valid_update, []} = Builder.update(user, object)
+
+      %{user: user, valid_update: valid_update}
+    end
+
+    test "validates a basic object", %{valid_update: valid_update} do
+      assert {:ok, _update, []} = ObjectValidator.validate(valid_update, [])
+    end
+  end
 end

From 75670a99e46a09f9bddc0959c680c2cb173e1f3b Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Fri, 19 Jun 2020 16:38:57 +0200
Subject: [PATCH 03/12] UpdateValidator: Only allow updates from the user
 themselves.

---
 .../object_validators/update_validator.ex        | 16 ++++++++++++++++
 test/web/activity_pub/object_validator_test.exs  | 12 ++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/lib/pleroma/web/activity_pub/object_validators/update_validator.ex b/lib/pleroma/web/activity_pub/object_validators/update_validator.ex
index 94d72491b..b4ba5ede0 100644
--- a/lib/pleroma/web/activity_pub/object_validators/update_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validators/update_validator.ex
@@ -33,6 +33,7 @@ def validate_data(cng) do
     |> validate_required([:id, :type, :actor, :to, :cc, :object])
     |> validate_inclusion(:type, ["Update"])
     |> validate_actor_presence()
+    |> validate_updating_rights()
   end
 
   def cast_and_validate(data) do
@@ -40,4 +41,19 @@ def cast_and_validate(data) do
     |> cast_data
     |> validate_data
   end
+
+  # For now we only support updating users, and here the rule is easy:
+  # object id == actor id
+  def validate_updating_rights(cng) do
+    with actor = get_field(cng, :actor),
+         object = get_field(cng, :object),
+         {:ok, object_id} <- ObjectValidators.ObjectID.cast(object),
+         true <- actor == object_id do
+      cng
+    else
+      _e ->
+        cng
+        |> add_error(:object, "Can't be updated by this actor")
+    end
+  end
 end
diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs
index adb56092d..770a8dcf8 100644
--- a/test/web/activity_pub/object_validator_test.exs
+++ b/test/web/activity_pub/object_validator_test.exs
@@ -641,5 +641,17 @@ test "returns an error if the actor can't announce the object", %{
     test "validates a basic object", %{valid_update: valid_update} do
       assert {:ok, _update, []} = ObjectValidator.validate(valid_update, [])
     end
+
+    test "returns an error if the object can't be updated by the actor", %{
+      valid_update: valid_update
+    } do
+      other_user = insert(:user)
+
+      update =
+        valid_update
+        |> Map.put("actor", other_user.ap_id)
+
+      assert {:error, _cng} = ObjectValidator.validate(update, [])
+    end
   end
 end

From 31a4d42ce0470d74417279a855192294650cff97 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Mon, 22 Jun 2020 13:15:37 +0200
Subject: [PATCH 04/12] SideEffects: Handle user updating.

---
 lib/pleroma/web/activity_pub/side_effects.ex | 12 ++++++++++++
 test/web/activity_pub/side_effects_test.exs  | 16 ++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex
index 1a1cc675c..09fd7d7c9 100644
--- a/lib/pleroma/web/activity_pub/side_effects.ex
+++ b/lib/pleroma/web/activity_pub/side_effects.ex
@@ -20,6 +20,18 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
 
   def handle(object, meta \\ [])
 
+  # Tasks this handles:
+  # Update the user
+  def handle(%{data: %{"type" => "Update", "object" => updated_object}} = object, meta) do
+    {:ok, new_user_data} = ActivityPub.user_data_from_user_object(updated_object)
+
+    User.get_by_ap_id(updated_object["id"])
+    |> User.remote_user_changeset(new_user_data)
+    |> User.update_and_set_cache()
+
+    {:ok, object, meta}
+  end
+
   # Tasks this handles:
   # - Add like to object
   # - Set up notification
diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs
index 6bbbaae87..1d7c2736b 100644
--- a/test/web/activity_pub/side_effects_test.exs
+++ b/test/web/activity_pub/side_effects_test.exs
@@ -64,6 +64,22 @@ test "it streams out notifications and streams" do
     end
   end
 
+  describe "update users" do
+    setup do
+      user = insert(:user)
+      {:ok, update_data, []} = Builder.update(user, %{"id" => user.ap_id, "name" => "new name!"})
+      {:ok, update, _meta} = ActivityPub.persist(update_data, local: true)
+
+      %{user: user, update_data: update_data, update: update}
+    end
+
+    test "it updates the user", %{user: user, update: update} do
+      {:ok, _, _} = SideEffects.handle(update)
+      user = User.get_by_id(user.id)
+      assert user.name == "new name!"
+    end
+  end
+
   describe "delete objects" do
     setup do
       user = insert(:user)

From 9438f83f83305f101b9fed65f68a5b9fd622bcbb Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Mon, 22 Jun 2020 13:16:05 +0200
Subject: [PATCH 05/12] Transmogrifier: Handle `Update` with the pipeline.

---
 .../web/activity_pub/transmogrifier.ex        | 33 +++----------------
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex
index 851f474b8..8165218ee 100644
--- a/lib/pleroma/web/activity_pub/transmogrifier.ex
+++ b/lib/pleroma/web/activity_pub/transmogrifier.ex
@@ -684,35 +684,12 @@ def handle_incoming(%{"type" => type} = data, _options)
   end
 
   def handle_incoming(
-        %{"type" => "Update", "object" => %{"type" => object_type} = object, "actor" => actor_id} =
-          data,
+        %{"type" => "Update"} = data,
         _options
-      )
-      when object_type in [
-             "Person",
-             "Application",
-             "Service",
-             "Organization"
-           ] do
-    with %User{ap_id: ^actor_id} = actor <- User.get_cached_by_ap_id(object["id"]) do
-      {:ok, new_user_data} = ActivityPub.user_data_from_user_object(object)
-
-      actor
-      |> User.remote_user_changeset(new_user_data)
-      |> User.update_and_set_cache()
-
-      ActivityPub.update(%{
-        local: false,
-        to: data["to"] || [],
-        cc: data["cc"] || [],
-        object: object,
-        actor: actor_id,
-        activity_id: data["id"]
-      })
-    else
-      e ->
-        Logger.error(e)
-        :error
+      ) do
+    with {:ok, %User{}} <- ObjectValidator.fetch_actor(data),
+         {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do
+      {:ok, activity}
     end
   end
 

From 1e7ca2443011f65aa766c3ddd5cd1203e79db50b Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Mon, 22 Jun 2020 13:23:21 +0200
Subject: [PATCH 06/12] Update Handling Test: Fix for re-used update ids.

---
 .../transmogrifier/user_update_handling_test.exs         | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/test/web/activity_pub/transmogrifier/user_update_handling_test.exs b/test/web/activity_pub/transmogrifier/user_update_handling_test.exs
index 8e5d3b883..64636656c 100644
--- a/test/web/activity_pub/transmogrifier/user_update_handling_test.exs
+++ b/test/web/activity_pub/transmogrifier/user_update_handling_test.exs
@@ -106,11 +106,13 @@ test "it works with custom profile fields" do
     Pleroma.Config.put([:instance, :max_remote_account_fields], 2)
 
     update_data =
-      put_in(update_data, ["object", "attachment"], [
+      update_data
+      |> put_in(["object", "attachment"], [
         %{"name" => "foo", "type" => "PropertyValue", "value" => "bar"},
         %{"name" => "foo11", "type" => "PropertyValue", "value" => "bar11"},
         %{"name" => "foo22", "type" => "PropertyValue", "value" => "bar22"}
       ])
+      |> Map.put("id", update_data["id"] <> ".")
 
     {:ok, _} = Transmogrifier.handle_incoming(update_data)
 
@@ -121,7 +123,10 @@ test "it works with custom profile fields" do
              %{"name" => "foo1", "value" => "updated"}
            ]
 
-    update_data = put_in(update_data, ["object", "attachment"], [])
+    update_data =
+      update_data
+      |> put_in(["object", "attachment"], [])
+      |> Map.put("id", update_data["id"] <> ".")
 
     {:ok, _} = Transmogrifier.handle_incoming(update_data)
 

From e785cd5caeab2c610f12a9071cade31a6b4549a4 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Mon, 22 Jun 2020 13:59:45 +0200
Subject: [PATCH 07/12] ActivityPub: Remove `update` and switch to pipeline.

---
 lib/pleroma/web/activity_pub/activity_pub.ex  | 22 --------
 lib/pleroma/web/activity_pub/side_effects.ex  | 18 +++++--
 .../controllers/account_controller.ex         | 53 +++++++++++--------
 test/web/activity_pub/activity_pub_test.exs   | 46 ----------------
 test/web/activity_pub/side_effects_test.exs   |  9 ++++
 5 files changed, 52 insertions(+), 96 deletions(-)

diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
index 3e4f3ad30..4cc9fe16c 100644
--- a/lib/pleroma/web/activity_pub/activity_pub.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -321,28 +321,6 @@ defp accept_or_reject(type, %{to: to, actor: actor, object: object} = params) do
     end
   end
 
-  @spec update(map()) :: {:ok, Activity.t()} | {:error, any()}
-  def update(%{to: to, cc: cc, actor: actor, object: object} = params) do
-    local = !(params[:local] == false)
-    activity_id = params[:activity_id]
-
-    data =
-      %{
-        "to" => to,
-        "cc" => cc,
-        "type" => "Update",
-        "actor" => actor,
-        "object" => object
-      }
-      |> Maps.put_if_present("id", activity_id)
-
-    with {:ok, activity} <- insert(data, local),
-         _ <- notify_and_stream(activity),
-         :ok <- maybe_federate(activity) do
-      {:ok, activity}
-    end
-  end
-
   @spec follow(User.t(), User.t(), String.t() | nil, boolean(), keyword()) ::
           {:ok, Activity.t()} | {:error, any()}
   def follow(follower, followed, activity_id \\ nil, local \\ true, opts \\ []) do
diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex
index 09fd7d7c9..de143b8f0 100644
--- a/lib/pleroma/web/activity_pub/side_effects.ex
+++ b/lib/pleroma/web/activity_pub/side_effects.ex
@@ -21,13 +21,21 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
   def handle(object, meta \\ [])
 
   # Tasks this handles:
-  # Update the user
+  # - Update the user
+  #
+  # For a local user, we also get a changeset with the full information, so we
+  # can update non-federating, non-activitypub settings as well.
   def handle(%{data: %{"type" => "Update", "object" => updated_object}} = object, meta) do
-    {:ok, new_user_data} = ActivityPub.user_data_from_user_object(updated_object)
+    if changeset = Keyword.get(meta, :user_update_changeset) do
+      changeset
+      |> User.update_and_set_cache()
+    else
+      {:ok, new_user_data} = ActivityPub.user_data_from_user_object(updated_object)
 
-    User.get_by_ap_id(updated_object["id"])
-    |> User.remote_user_changeset(new_user_data)
-    |> User.update_and_set_cache()
+      User.get_by_ap_id(updated_object["id"])
+      |> User.remote_user_changeset(new_user_data)
+      |> User.update_and_set_cache()
+    end
 
     {:ok, object, meta}
   end
diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex
index c38c2b895..f0499621a 100644
--- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex
@@ -20,6 +20,8 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   alias Pleroma.Plugs.RateLimiter
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
+  alias Pleroma.Web.ActivityPub.Pipeline
+  alias Pleroma.Web.ActivityPub.Builder
   alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.MastodonAPI.ListView
   alias Pleroma.Web.MastodonAPI.MastodonAPI
@@ -179,34 +181,39 @@ def update_credentials(%{assigns: %{user: user}, body_params: params} = conn, _p
       |> Maps.put_if_present(:default_scope, params["source"]["privacy"])
       |> Maps.put_if_present(:actor_type, params[:actor_type])
 
-    changeset = User.update_changeset(user, user_params)
-
-    with {:ok, user} <- User.update_and_set_cache(changeset) do
-      user
-      |> build_update_activity_params()
-      |> ActivityPub.update()
-
-      render(conn, "show.json", user: user, for: user, with_pleroma_settings: true)
+    # What happens here:
+    #
+    # We want to update the user through the pipeline, but the ActivityPub
+    # update information is not quite enough for this, because this also
+    # contains local settings that don't federate and don't even appear
+    # in the Update activity. 
+    # 
+    # So we first build the normal local changeset, then apply it to the
+    # user data, but don't persist it. With this, we generate the object
+    # data for our update activity. We feed this and the changeset as meta
+    # inforation into the pipeline, where they will be properly updated and
+    # federated.
+    with changeset <- User.update_changeset(user, user_params),
+         {:ok, unpersisted_user} <- Ecto.Changeset.apply_action(changeset, :update),
+         updated_object <-
+           Pleroma.Web.ActivityPub.UserView.render("user.json", user: user)
+           |> Map.delete("@context"),
+         {:ok, update_data, []} <- Builder.update(user, updated_object),
+         {:ok, _update, _} <-
+           Pipeline.common_pipeline(update_data,
+             local: true,
+             user_update_changeset: changeset
+           ) do
+      render(conn, "show.json",
+        user: unpersisted_user,
+        for: unpersisted_user,
+        with_pleroma_settings: true
+      )
     else
       _e -> render_error(conn, :forbidden, "Invalid request")
     end
   end
 
-  # Hotfix, handling will be redone with the pipeline
-  defp build_update_activity_params(user) do
-    object =
-      Pleroma.Web.ActivityPub.UserView.render("user.json", user: user)
-      |> Map.delete("@context")
-
-    %{
-      local: true,
-      to: [user.follower_address],
-      cc: [],
-      object: object,
-      actor: user.ap_id
-    }
-  end
-
   defp normalize_fields_attributes(fields) do
     if Enum.all?(fields, &is_tuple/1) do
       Enum.map(fields, fn {_, v} -> v end)
diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs
index 7693f6400..ce35c9605 100644
--- a/test/web/activity_pub/activity_pub_test.exs
+++ b/test/web/activity_pub/activity_pub_test.exs
@@ -1092,52 +1092,6 @@ test "it filters broken threads" do
     end
   end
 
-  describe "update" do
-    setup do: clear_config([:instance, :max_pinned_statuses])
-
-    test "it creates an update activity with the new user data" do
-      user = insert(:user)
-      {:ok, user} = User.ensure_keys_present(user)
-      user_data = Pleroma.Web.ActivityPub.UserView.render("user.json", %{user: user})
-
-      {:ok, update} =
-        ActivityPub.update(%{
-          actor: user_data["id"],
-          to: [user.follower_address],
-          cc: [],
-          object: user_data
-        })
-
-      assert update.data["actor"] == user.ap_id
-      assert update.data["to"] == [user.follower_address]
-      assert embedded_object = update.data["object"]
-      assert embedded_object["id"] == user_data["id"]
-      assert embedded_object["type"] == user_data["type"]
-    end
-  end
-
-  test "returned pinned statuses" do
-    Config.put([:instance, :max_pinned_statuses], 3)
-    user = insert(:user)
-
-    {:ok, activity_one} = CommonAPI.post(user, %{status: "HI!!!"})
-    {:ok, activity_two} = CommonAPI.post(user, %{status: "HI!!!"})
-    {:ok, activity_three} = CommonAPI.post(user, %{status: "HI!!!"})
-
-    CommonAPI.pin(activity_one.id, user)
-    user = refresh_record(user)
-
-    CommonAPI.pin(activity_two.id, user)
-    user = refresh_record(user)
-
-    CommonAPI.pin(activity_three.id, user)
-    user = refresh_record(user)
-
-    activities = ActivityPub.fetch_user_activities(user, nil, %{pinned: true})
-
-    assert 3 = length(activities)
-  end
-
   describe "flag/1" do
     setup do
       reporter = insert(:user)
diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs
index 1d7c2736b..12c9ef1da 100644
--- a/test/web/activity_pub/side_effects_test.exs
+++ b/test/web/activity_pub/side_effects_test.exs
@@ -78,6 +78,15 @@ test "it updates the user", %{user: user, update: update} do
       user = User.get_by_id(user.id)
       assert user.name == "new name!"
     end
+
+    test "it uses a given changeset to update", %{user: user, update: update} do
+      changeset = Ecto.Changeset.change(user, %{default_scope: "direct"})
+
+      assert user.default_scope == "public"
+      {:ok, _, _} = SideEffects.handle(update, user_update_changeset: changeset)
+      user = User.get_by_id(user.id)
+      assert user.default_scope == "direct"
+    end
   end
 
   describe "delete objects" do

From b05f795326b77edd881ffea2c004d7ca0ddd7df9 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Mon, 22 Jun 2020 14:02:29 +0200
Subject: [PATCH 08/12] Credo fixes

---
 .../web/mastodon_api/controllers/account_controller.ex      | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex
index f0499621a..d4605c518 100644
--- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex
@@ -20,8 +20,8 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   alias Pleroma.Plugs.RateLimiter
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
-  alias Pleroma.Web.ActivityPub.Pipeline
   alias Pleroma.Web.ActivityPub.Builder
+  alias Pleroma.Web.ActivityPub.Pipeline
   alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.MastodonAPI.ListView
   alias Pleroma.Web.MastodonAPI.MastodonAPI
@@ -186,8 +186,8 @@ def update_credentials(%{assigns: %{user: user}, body_params: params} = conn, _p
     # We want to update the user through the pipeline, but the ActivityPub
     # update information is not quite enough for this, because this also
     # contains local settings that don't federate and don't even appear
-    # in the Update activity. 
-    # 
+    # in the Update activity.
+    #
     # So we first build the normal local changeset, then apply it to the
     # user data, but don't persist it. With this, we generate the object
     # data for our update activity. We feed this and the changeset as meta

From 2737809bbf249696d06d4a351837a405d79d47e3 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Tue, 23 Jun 2020 11:03:32 +0200
Subject: [PATCH 09/12] An act of desperation.

---
 test/web/activity_pub/activity_pub_controller_test.exs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs
index e490a5744..6ea50fd96 100644
--- a/test/web/activity_pub/activity_pub_controller_test.exs
+++ b/test/web/activity_pub/activity_pub_controller_test.exs
@@ -665,7 +665,7 @@ test "it accepts announces with to as string instead of array", %{conn: conn} do
         |> post("/users/#{user.nickname}/inbox", data)
 
       assert "ok" == json_response(conn, 200)
-      ObanHelpers.perform(all_enqueued(worker: ReceiverWorker))
+      ObanHelpers.perform_all()
       %Activity{} = activity = Activity.get_by_ap_id(data["id"])
       assert "https://www.w3.org/ns/activitystreams#Public" in activity.recipients
     end

From d93e01137b0682dd97b95b848f7b8656de89e3cf Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Tue, 23 Jun 2020 11:43:20 +0200
Subject: [PATCH 10/12] ActivityPubControllerTest: Testing changes.

---
 .../web/activity_pub/activity_pub_controller_test.exs | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs
index 6ea50fd96..e5f801b22 100644
--- a/test/web/activity_pub/activity_pub_controller_test.exs
+++ b/test/web/activity_pub/activity_pub_controller_test.exs
@@ -648,11 +648,14 @@ test "it accepts messages with bcc as string instead of array", %{conn: conn, da
     test "it accepts announces with to as string instead of array", %{conn: conn} do
       user = insert(:user)
 
+      {:ok, post} = CommonAPI.post(user, %{status: "hey"})
+      announcer = insert(:user, local: false)
+
       data = %{
         "@context" => "https://www.w3.org/ns/activitystreams",
-        "actor" => "http://mastodon.example.org/users/admin",
-        "id" => "http://mastodon.example.org/users/admin/statuses/19512778738411822/activity",
-        "object" => "https://mastodon.social/users/emelie/statuses/101849165031453009",
+        "actor" => announcer.ap_id,
+        "id" => "#{announcer.ap_id}/statuses/19512778738411822/activity",
+        "object" => post.data["object"],
         "to" => "https://www.w3.org/ns/activitystreams#Public",
         "cc" => [user.ap_id],
         "type" => "Announce"
@@ -665,7 +668,7 @@ test "it accepts announces with to as string instead of array", %{conn: conn} do
         |> post("/users/#{user.nickname}/inbox", data)
 
       assert "ok" == json_response(conn, 200)
-      ObanHelpers.perform_all()
+      ObanHelpers.perform(all_enqueued(worker: ReceiverWorker))
       %Activity{} = activity = Activity.get_by_ap_id(data["id"])
       assert "https://www.w3.org/ns/activitystreams#Public" in activity.recipients
     end

From adc199c6a8932f893bc1098acbf222e64cdb07d9 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Tue, 23 Jun 2020 12:04:51 +0200
Subject: [PATCH 11/12] ActivityPubControllerTest: Capture error log

---
 test/web/activity_pub/activity_pub_controller_test.exs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs
index e5f801b22..e722f7c04 100644
--- a/test/web/activity_pub/activity_pub_controller_test.exs
+++ b/test/web/activity_pub/activity_pub_controller_test.exs
@@ -536,6 +536,7 @@ test "accept follow activity", %{conn: conn} do
       assert_receive {:mix_shell, :info, ["relay.mastodon.host"]}
     end
 
+    @tag capture_log: true
     test "without valid signature, " <>
            "it only accepts Create activities and requires enabled federation",
          %{conn: conn} do

From aee815b478aea5d74959c5a445c6c5d87f25168e Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Tue, 23 Jun 2020 12:37:05 +0200
Subject: [PATCH 12/12] ObjectValidator: Clarify type of object.

---
 lib/pleroma/web/activity_pub/object_validator.ex | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex
index 804a9d06e..2c657b467 100644
--- a/lib/pleroma/web/activity_pub/object_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validator.ex
@@ -24,13 +24,13 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
   @spec validate(map(), keyword()) :: {:ok, map(), keyword()} | {:error, any()}
   def validate(object, meta)
 
-  def validate(%{"type" => "Update"} = object, meta) do
-    with {:ok, object} <-
-           object
+  def validate(%{"type" => "Update"} = update_activity, meta) do
+    with {:ok, update_activity} <-
+           update_activity
            |> UpdateValidator.cast_and_validate()
            |> Ecto.Changeset.apply_action(:insert) do
-      object = stringify_keys(object)
-      {:ok, object, meta}
+      update_activity = stringify_keys(update_activity)
+      {:ok, update_activity, meta}
     end
   end