diff --git a/lib/federation/activity_pub/actions/update.ex b/lib/federation/activity_pub/actions/update.ex
index 1306934b6..9c0f860ac 100644
--- a/lib/federation/activity_pub/actions/update.ex
+++ b/lib/federation/activity_pub/actions/update.ex
@@ -28,7 +28,7 @@ defmodule Mobilizon.Federation.ActivityPub.Actions.Update do
Logger.debug("updating an activity")
Logger.debug(inspect(args))
- case Managable.update(old_entity, args, additional) do
+ case Managable.update(old_entity, args, Map.put(additional, :local, local)) do
{:ok, entity, update_data} ->
{:ok, activity} = create_activity(update_data, local)
maybe_federate(activity)
diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex
index 2d904ad1a..bac97bee6 100644
--- a/lib/federation/activity_pub/transmogrifier.ex
+++ b/lib/federation/activity_pub/transmogrifier.ex
@@ -407,6 +407,13 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
Actions.Update.update(old_actor, object_data, false, %{updater_actor: author}) do
{:ok, activity, new_actor}
else
+ {:error, :update_not_allowed} ->
+ Logger.warn("Activity tried to update an actor that's local or not a group",
+ activity: params
+ )
+
+ :error
+
e ->
Sentry.capture_message("Error while handling an Update activity",
extra: %{params: params}
diff --git a/lib/federation/activity_pub/types/actors.ex b/lib/federation/activity_pub/types/actors.ex
index 99fdd859c..48aa34fd6 100644
--- a/lib/federation/activity_pub/types/actors.ex
+++ b/lib/federation/activity_pub/types/actors.ex
@@ -45,25 +45,30 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Actors do
def update(%Actor{} = old_actor, args, additional) do
updater_actor = Map.get(args, :updater_actor) || Map.get(additional, :updater_actor)
- case Actors.update_actor(old_actor, args) do
- {:ok, %Actor{} = new_actor} ->
- GroupActivity.insert_activity(new_actor,
- subject: "group_updated",
- old_group: old_actor,
- updater_actor: updater_actor
- )
+ if Map.get(additional, :local, false) == true or not match?(%Actor{domain: nil}, old_actor) or
+ match?(%Actor{type: :Group}, old_actor) do
+ case Actors.update_actor(old_actor, args) do
+ {:ok, %Actor{} = new_actor} ->
+ GroupActivity.insert_activity(new_actor,
+ subject: "group_updated",
+ old_group: old_actor,
+ updater_actor: updater_actor
+ )
- actor_as_data = Convertible.model_to_as(new_actor)
- Cachex.del(:activity_pub, "actor_#{new_actor.preferred_username}")
- audience = Audience.get_audience(new_actor)
+ actor_as_data = Convertible.model_to_as(new_actor)
+ Cachex.del(:activity_pub, "actor_#{new_actor.preferred_username}")
+ audience = Audience.get_audience(new_actor)
- additional = Map.merge(additional, %{"actor" => (updater_actor || old_actor).url})
+ additional = Map.merge(additional, %{"actor" => (updater_actor || old_actor).url})
- update_data = make_update_data(actor_as_data, Map.merge(audience, additional))
- {:ok, new_actor, update_data}
+ update_data = make_update_data(actor_as_data, Map.merge(audience, additional))
+ {:ok, new_actor, update_data}
- {:error, %Ecto.Changeset{} = err} ->
- {:error, err}
+ {:error, %Ecto.Changeset{} = err} ->
+ {:error, err}
+ end
+ else
+ {:error, :update_not_allowed}
end
end
diff --git a/test/federation/activity_pub/activity_pub_test.exs b/test/federation/activity_pub/activity_pub_test.exs
index 80f452142..70e7ce8d5 100644
--- a/test/federation/activity_pub/activity_pub_test.exs
+++ b/test/federation/activity_pub/activity_pub_test.exs
@@ -195,7 +195,7 @@ defmodule Mobilizon.Federation.ActivityPubTest do
actor = insert(:actor)
actor_data = %{summary: @updated_actor_summary}
- {:ok, update, _} = Actions.Update.update(actor, actor_data, false)
+ {:ok, update, _} = Actions.Update.update(actor, actor_data, true)
assert update.data["actor"] == actor.url
assert update.data["to"] == [@activity_pub_public_audience]
diff --git a/test/federation/activity_pub/transmogrifier/update_test.exs b/test/federation/activity_pub/transmogrifier/update_test.exs
index a5724ff60..b8411df20 100644
--- a/test/federation/activity_pub/transmogrifier/update_test.exs
+++ b/test/federation/activity_pub/transmogrifier/update_test.exs
@@ -3,12 +3,13 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.UpdateTest do
use Oban.Testing, repo: Mobilizon.Storage.Repo
import Mobilizon.Factory
import Mox
+ import ExUnit.CaptureLog
alias Mobilizon.{Actors, Events, Posts}
alias Mobilizon.Actors.{Actor, Member}
alias Mobilizon.Events.Event
alias Mobilizon.Posts.Post
- alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier}
+ alias Mobilizon.Federation.ActivityPub.{Activity, Relay, Transmogrifier}
alias Mobilizon.Federation.ActivityStream.Convertible
alias Mobilizon.Service.HTTP.ActivityPub.Mock
@@ -50,6 +51,29 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.UpdateTest do
assert actor.summary == "<p>Some bio</p>"
end
+ test "it fails for incoming update activies on local actors" do
+ %Actor{url: relay_actor_url} = Relay.get_actor()
+
+ update_data = File.read!("test/fixtures/mastodon-update.json") |> Jason.decode!()
+
+ object =
+ update_data["object"]
+ |> Map.put("actor", relay_actor_url)
+ |> Map.put("id", relay_actor_url)
+
+ update_data =
+ update_data
+ |> Map.put("actor", relay_actor_url)
+ |> Map.put("object", object)
+
+ assert capture_log([level: :warn], fn ->
+ :error = Transmogrifier.handle_incoming(update_data)
+ end) =~ "[warning] Activity tried to update an actor that's local or not a group"
+
+ {:ok, %Actor{keys: keys}} = Actors.get_actor_by_url(relay_actor_url)
+ assert Regex.match?(~r/BEGIN RSA PRIVATE KEY/, keys)
+ end
+
test "it works for incoming update activities on events" do
data = File.read!("test/fixtures/mobilizon-post-activity.json") |> Jason.decode!()