From df2c184bc034c3dd60215291e296b6df1a4cec98 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sun, 14 Nov 2021 16:26:51 +0100 Subject: [PATCH] Refactor transmogrifier Delete to avoid spoofed Delete being accepted Signed-off-by: Thomas Citharel --- lib/federation/activity_pub/permission.ex | 3 +- lib/federation/activity_pub/transmogrifier.ex | 50 +++++++------------ 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/lib/federation/activity_pub/permission.ex b/lib/federation/activity_pub/permission.ex index 83b910f6..30925528 100644 --- a/lib/federation/activity_pub/permission.ex +++ b/lib/federation/activity_pub/permission.ex @@ -101,7 +101,8 @@ defmodule Mobilizon.Federation.ActivityPub.Permission do false end else - true + # Object is not owned by a group + false end end diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex index b41ab49e..df8ebf38 100644 --- a/lib/federation/activity_pub/transmogrifier.ex +++ b/lib/federation/activity_pub/transmogrifier.ex @@ -602,42 +602,28 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do ) do Logger.info("Handle incoming to delete an object") - with actor_url <- Utils.get_actor(data), - {:actor, {:ok, %Actor{} = actor}} <- - {:actor, ActivityPubActor.get_or_fetch_actor_by_url(actor_url)}, - object_id <- Utils.get_url(object), - {:ok, object} <- is_group_object_gone(object_id), - {:origin_check, true} <- - {:origin_check, - Utils.origin_check_from_id?(actor_url, object_id) || - Permission.can_delete_group_object?(actor, object)}, - {:ok, activity, object} <- Actions.Delete.delete(object, actor, false) do - {:ok, activity, object} - else - {:origin_check, false} -> - Logger.warn("Object origin check failed") - :error + actor_url = Utils.get_actor(data) + object_id = Utils.get_url(object) - {:actor, {:error, _err}} -> + case ActivityPubActor.get_or_fetch_actor_by_url(actor_url) do + {:error, _err} -> {:error, :unknown_actor} - {:error, e} -> - Logger.debug(inspect(e)) + {:ok, %Actor{} = actor} -> + case is_group_object_gone(object_id) do + {:ok, object} -> + if Utils.origin_check_from_id?(actor_url, object_id) || + Permission.can_delete_group_object?(actor, object) do + Actions.Delete.delete(object, actor, false) + else + Logger.warn("Object origin check failed") + :error + end - # Sentry.capture_message("Error while handling a Delete activity", - # extra: %{data: data} - # ) - - :error - - e -> - Logger.error(inspect(e)) - - # Sentry.capture_message("Error while handling a Delete activity", - # extra: %{data: data} - # ) - - :error + {:error, err} -> + Logger.debug(inspect(err)) + {:error, err} + end end end