Merge branch 'refactor-ap' into 'master'

Fix some AP issues

See merge request framasoft/mobilizon!904

lib/federation/activity_pub/activity_pub.ex

@@ -39,11 +39,12 @@ defmodule Mobilizon.Federation.ActivityPub do
     Visibility
   }
 
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
+
   alias Mobilizon.Federation.ActivityPub.Types.{Managable, Ownable}
 
-  alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
+  alias Mobilizon.Federation.ActivityStream.Convertible
   alias Mobilizon.Federation.HTTPSignatures.Signature
-  alias Mobilizon.Federation.WebFinger
 
   alias Mobilizon.Service.Notifications.Scheduler
   alias Mobilizon.Storage.Page
@@ -79,7 +80,6 @@ defmodule Mobilizon.Federation.ActivityPub do
           {:ok, struct()} | {:error, any()}
   def fetch_object_from_url(url, options \\ []) do
     Logger.info("Fetching object from url #{url}")
-    force_fetch = Keyword.get(options, :force, false)
 
     with {:not_http, true} <- {:not_http, String.starts_with?(url, "http")},
          {:existing, nil} <-
@@ -99,9 +99,41 @@ defmodule Mobilizon.Federation.ActivityPub do
       Preloader.maybe_preload(entity)
     else
       {:existing, entity} ->
-        Logger.debug("Entity is already existing")
+        handle_existing_entity(url, entity, options)
+
+      e ->
+        Logger.warn("Something failed while fetching url #{inspect(e)}")
+        {:error, e}
+    end
+  end
+
+  @spec handle_existing_entity(String.t(), struct(), Keyword.t()) ::
+          {:ok, struct()}
+          | {:ok, struct()}
+          | {:error, String.t(), struct()}
+          | {:error, String.t()}
+  defp handle_existing_entity(url, entity, options) do
+    Logger.debug("Entity is already existing")
+    Logger.debug("Going to preload an existing entity")
+
+    case refresh_entity(url, entity, options) do
+      {:ok, entity} ->
+        Preloader.maybe_preload(entity)
+
+      {:error, status, entity} ->
+        {:ok, entity} = Preloader.maybe_preload(entity)
+        {:error, status, entity}
+
+      err ->
+        err
+    end
+  end
+
+  @spec refresh_entity(String.t(), struct(), Keyword.t()) ::
+          {:ok, struct()} | {:error, String.t(), struct()} | {:error, String.t()}
+  defp refresh_entity(url, entity, options) do
+    force_fetch = Keyword.get(options, :force, false)
 
-        res =
     if force_fetch and not are_same_origin?(url, Endpoint.url()) do
       Logger.debug("Entity is external and we want a force fetch")
 
@@ -114,63 +146,13 @@ defmodule Mobilizon.Federation.ActivityPub do
 
         {:error, "Not found"} ->
           {:error, "Not found", entity}
+
+        {:error, "Object origin check failed"} ->
+          {:error, "Object origin check failed"}
       end
     else
       {:ok, entity}
     end
-
-        Logger.debug("Going to preload an existing entity")
-
-        case res do
-          {:ok, entity} ->
-            Preloader.maybe_preload(entity)
-
-          {:error, status, entity} ->
-            {:ok, entity} = Preloader.maybe_preload(entity)
-            {:error, status, entity}
-
-          err ->
-            err
-        end
-
-      e ->
-        Logger.warn("Something failed while fetching url #{inspect(e)}")
-        {:error, e}
-    end
-  end
-
-  @doc """
-  Getting an actor from url, eventually creating it if we don't have it locally or if it needs an update
-  """
-  @spec get_or_fetch_actor_by_url(String.t(), boolean) :: {:ok, Actor.t()} | {:error, String.t()}
-  def get_or_fetch_actor_by_url(url, preload \\ false)
-
-  def get_or_fetch_actor_by_url(nil, _preload), do: {:error, "Can't fetch a nil url"}
-
-  def get_or_fetch_actor_by_url("https://www.w3.org/ns/activitystreams#Public", _preload) do
-    with %Actor{url: url} <- Relay.get_actor() do
-      get_or_fetch_actor_by_url(url)
-    end
-  end
-
-  @spec get_or_fetch_actor_by_url(String.t(), boolean()) :: {:ok, Actor.t()} | {:error, any()}
-  def get_or_fetch_actor_by_url(url, preload) do
-    with {:ok, %Actor{} = cached_actor} <- Actors.get_actor_by_url(url, preload),
-         false <- Actors.needs_update?(cached_actor) do
-      {:ok, cached_actor}
-    else
-      _ ->
-        # For tests, see https://github.com/jjh42/mock#not-supported---mocking-internal-function-calls and Mobilizon.Federation.ActivityPubTest
-        case __MODULE__.make_actor_from_url(url, preload) do
-          {:ok, %Actor{} = actor} ->
-            {:ok, actor}
-
-          err ->
-            Logger.warn("Could not fetch by AP id")
-            Logger.debug(inspect(err))
-            {:error, "Could not fetch by AP id"}
-        end
-    end
   end
 
   @doc """
@@ -287,7 +269,8 @@ defmodule Mobilizon.Federation.ActivityPub do
         local \\ true,
         public \\ true
       ) do
-    with {:ok, %Actor{id: object_owner_actor_id}} <- get_or_fetch_actor_by_url(object["actor"]),
+    with {:ok, %Actor{id: object_owner_actor_id}} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(object["actor"]),
          {:ok, %Share{} = _share} <- Share.create(object["id"], actor.id, object_owner_actor_id),
          announce_data <- make_announce_data(actor, object, activity_id, public),
          {:ok, activity} <- create_activity(announce_data, local),
@@ -604,68 +587,6 @@ defmodule Mobilizon.Federation.ActivityPub do
     end
   end
 
-  @doc """
-  Create an actor locally by its URL (AP ID)
-  """
-  @spec make_actor_from_url(String.t(), boolean()) :: {:ok, %Actor{}} | {:error, any()}
-  def make_actor_from_url(url, preload \\ false) do
-    if are_same_origin?(url, Endpoint.url()) do
-      {:error, "Can't make a local actor from URL"}
-    else
-      case fetch_and_prepare_actor_from_url(url) do
-        {:ok, data} ->
-          Actors.upsert_actor(data, preload)
-
-        # Request returned 410
-        {:error, :actor_deleted} ->
-          Logger.info("Actor was deleted")
-          {:error, :actor_deleted}
-
-        {:error, e} ->
-          Logger.warn("Failed to make actor from url")
-          {:error, e}
-
-        e ->
-          Logger.warn("Failed to make actor from url")
-          {:error, e}
-      end
-    end
-  end
-
-  @doc """
-  Find an actor in our local database or call WebFinger to find what's its AP ID is and then fetch it
-  """
-  @spec find_or_make_actor_from_nickname(String.t(), atom() | nil) :: tuple()
-  def find_or_make_actor_from_nickname(nickname, type \\ nil) do
-    case Actors.get_actor_by_name(nickname, type) do
-      %Actor{} = actor ->
-        {:ok, actor}
-
-      nil ->
-        make_actor_from_nickname(nickname)
-    end
-  end
-
-  @spec find_or_make_person_from_nickname(String.t()) :: tuple()
-  def find_or_make_person_from_nickname(nick), do: find_or_make_actor_from_nickname(nick, :Person)
-
-  @spec find_or_make_group_from_nickname(String.t()) :: tuple()
-  def find_or_make_group_from_nickname(nick), do: find_or_make_actor_from_nickname(nick, :Group)
-
-  @doc """
-  Create an actor inside our database from username, using WebFinger to find out its AP ID and then fetch it
-  """
-  @spec make_actor_from_nickname(String.t()) :: {:ok, %Actor{}} | {:error, any()}
-  def make_actor_from_nickname(nickname) do
-    case WebFinger.finger(nickname) do
-      {:ok, url} when is_binary(url) ->
-        make_actor_from_url(url)
-
-      _e ->
-        {:error, "No ActivityPub URL found in WebFinger"}
-    end
-  end
-
   @spec is_create_activity?(Activity.t()) :: boolean
   defp is_create_activity?(%Activity{data: %{"type" => "Create"}}), do: true
   defp is_create_activity?(_), do: false
@@ -783,40 +704,6 @@ defmodule Mobilizon.Federation.ActivityPub do
     )
   end
 
-  # Fetching a remote actor's information through its AP ID
-  @spec fetch_and_prepare_actor_from_url(String.t()) :: {:ok, struct()} | {:error, atom()} | any()
-  defp fetch_and_prepare_actor_from_url(url) do
-    Logger.debug("Fetching and preparing actor from url")
-    Logger.debug(inspect(url))
-
-    res =
-      with {:ok, %{status: 200, body: body}} <-
-             Tesla.get(url,
-               headers: [{"Accept", "application/activity+json"}],
-               follow_redirect: true
-             ),
-           :ok <- Logger.debug("response okay, now decoding json"),
-           {:ok, data} <- Jason.decode(body) do
-        Logger.debug("Got activity+json response at actor's endpoint, now converting data")
-        {:ok, Converter.Actor.as_to_model_data(data)}
-      else
-        # Actor is gone, probably deleted
-        {:ok, %{status: 410}} ->
-          Logger.info("Response HTTP 410")
-          {:error, :actor_deleted}
-
-        {:error, e} ->
-          Logger.warn("Could not decode actor at fetch #{url}, #{inspect(e)}")
-          {:error, e}
-
-        e ->
-          Logger.warn("Could not decode actor at fetch #{url}, #{inspect(e)}")
-          {:error, e}
-      end
-
-    res
-  end
-
   @doc """
   Return all public activities (events & comments) for an actor
   """

lib/federation/activity_pub/actor.ex

@@ -0,0 +1,102 @@
+defmodule Mobilizon.Federation.ActivityPub.Actor do
+  @moduledoc """
+  Module to handle ActivityPub Actor interactions
+  """
+
+  alias Mobilizon.Actors
+  alias Mobilizon.Actors.Actor
+  alias Mobilizon.Federation.ActivityPub.{Fetcher, Relay}
+  alias Mobilizon.Federation.WebFinger
+  alias Mobilizon.Web.Endpoint
+  require Logger
+  import Mobilizon.Federation.ActivityPub.Utils, only: [are_same_origin?: 2]
+
+  @doc """
+  Getting an actor from url, eventually creating it if we don't have it locally or if it needs an update
+  """
+  @spec get_or_fetch_actor_by_url(String.t(), boolean) :: {:ok, Actor.t()} | {:error, String.t()}
+  def get_or_fetch_actor_by_url(url, preload \\ false)
+
+  def get_or_fetch_actor_by_url(nil, _preload), do: {:error, "Can't fetch a nil url"}
+
+  def get_or_fetch_actor_by_url("https://www.w3.org/ns/activitystreams#Public", _preload) do
+    with %Actor{url: url} <- Relay.get_actor() do
+      get_or_fetch_actor_by_url(url)
+    end
+  end
+
+  @spec get_or_fetch_actor_by_url(String.t(), boolean()) :: {:ok, Actor.t()} | {:error, any()}
+  def get_or_fetch_actor_by_url(url, preload) do
+    with {:ok, %Actor{} = cached_actor} <- Actors.get_actor_by_url(url, preload),
+         false <- Actors.needs_update?(cached_actor) do
+      {:ok, cached_actor}
+    else
+      _ ->
+        # For tests, see https://github.com/jjh42/mock#not-supported---mocking-internal-function-calls and Mobilizon.Federation.ActivityPubTest
+        case __MODULE__.make_actor_from_url(url, preload) do
+          {:ok, %Actor{} = actor} ->
+            {:ok, actor}
+
+          {:error, err} ->
+            Logger.debug("Could not fetch by AP id")
+            Logger.debug(inspect(err))
+            {:error, "Could not fetch by AP id"}
+        end
+    end
+  end
+
+  @doc """
+  Create an actor locally by its URL (AP ID)
+  """
+  @spec make_actor_from_url(String.t(), boolean()) :: {:ok, %Actor{}} | {:error, any()}
+  def make_actor_from_url(url, preload \\ false) do
+    if are_same_origin?(url, Endpoint.url()) do
+      {:error, "Can't make a local actor from URL"}
+    else
+      case Fetcher.fetch_and_prepare_actor_from_url(url) do
+        {:ok, data} ->
+          Actors.upsert_actor(data, preload)
+
+        # Request returned 410
+        {:error, :actor_deleted} ->
+          Logger.info("Actor was deleted")
+          {:error, :actor_deleted}
+
+        {:error, e} ->
+          Logger.warn("Failed to make actor from url")
+          {:error, e}
+      end
+    end
+  end
+
+  @doc """
+  Find an actor in our local database or call WebFinger to find what's its AP ID is and then fetch it
+  """
+  @spec find_or_make_actor_from_nickname(String.t(), atom() | nil) :: tuple()
+  def find_or_make_actor_from_nickname(nickname, type \\ nil) do
+    case Actors.get_actor_by_name(nickname, type) do
+      %Actor{} = actor ->
+        {:ok, actor}
+
+      nil ->
+        make_actor_from_nickname(nickname)
+    end
+  end
+
+  @spec find_or_make_group_from_nickname(String.t()) :: tuple()
+  def find_or_make_group_from_nickname(nick), do: find_or_make_actor_from_nickname(nick, :Group)
+
+  @doc """
+  Create an actor inside our database from username, using WebFinger to find out its AP ID and then fetch it
+  """
+  @spec make_actor_from_nickname(String.t()) :: {:ok, %Actor{}} | {:error, any()}
+  def make_actor_from_nickname(nickname) do
+    case WebFinger.finger(nickname) do
+      {:ok, url} when is_binary(url) ->
+        make_actor_from_url(url)
+
+      _e ->
+        {:error, "No ActivityPub URL found in WebFinger"}
+    end
+  end
+end

lib/federation/activity_pub/federator.ex

@@ -13,6 +13,7 @@ defmodule Mobilizon.Federation.ActivityPub.Federator do
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier}
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
 
   require Logger
 
@@ -42,7 +43,8 @@ defmodule Mobilizon.Federation.ActivityPub.Federator do
     Logger.debug(inspect(activity))
     Logger.debug(fn -> "Running publish for #{activity.data["id"]}" end)
 
-    with {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(activity.data["actor"]) do
+    with {:ok, %Actor{} = actor} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(activity.data["actor"]) do
       Logger.info(fn -> "Sending #{activity.data["id"]} out via AP" end)
       ActivityPub.publish(actor, activity)
     end
@@ -61,7 +63,7 @@ defmodule Mobilizon.Federation.ActivityPub.Federator do
 
       e ->
         # Just drop those for now
-        Logger.error("Unhandled activity")
+        Logger.debug("Unhandled activity")
         Logger.debug(inspect(e))
         Logger.debug(Jason.encode!(params))
     end

lib/federation/activity_pub/fetcher.ex

@@ -8,6 +8,7 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
 
   alias Mobilizon.Federation.HTTPSignatures.Signature
   alias Mobilizon.Federation.ActivityPub.{Relay, Transmogrifier}
+  alias Mobilizon.Federation.ActivityStream.Converter.Actor, as: ActorConverter
   alias Mobilizon.Service.HTTP.ActivityPub, as: ActivityPubClient
 
   import Mobilizon.Federation.ActivityPub.Utils,
@@ -30,11 +31,11 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
       {:ok, data}
     else
       {:ok, %Tesla.Env{status: 410}} ->
-        Logger.warn("Resource at #{url} is 410 Gone")
+        Logger.debug("Resource at #{url} is 410 Gone")
         {:error, "Gone"}
 
       {:ok, %Tesla.Env{status: 404}} ->
-        Logger.warn("Resource at #{url} is 404 Gone")
+        Logger.debug("Resource at #{url} is 404 Gone")
         {:error, "Not found"}
 
       {:ok, %Tesla.Env{} = res} ->
@@ -75,7 +76,7 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
   @spec fetch_and_update(String.t(), Keyword.t()) :: {:ok, map(), struct()}
   def fetch_and_update(url, options \\ []) do
     with {:ok, data} when is_map(data) <- fetch(url, options),
-         {:origin_check, true} <- {:origin_check, origin_check?(url, data)},
+         {:origin_check, true} <- {:origin_check, origin_check(url, data)},
          params <- %{
            "type" => "Update",
            "to" => data["to"],
@@ -87,7 +88,6 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
       Transmogrifier.handle_incoming(params)
     else
       {:origin_check, false} ->
-        Logger.warn("Object origin check failed")
         {:error, "Object origin check failed"}
 
       {:error, err} ->
@@ -95,6 +95,53 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
     end
   end
 
+  @doc """
+  Fetching a remote actor's information through its AP ID
+  """
+  @spec fetch_and_prepare_actor_from_url(String.t()) :: {:ok, map()} | {:error, atom()} | any()
+  def fetch_and_prepare_actor_from_url(url) do
+    Logger.debug("Fetching and preparing actor from url")
+    Logger.debug(inspect(url))
+
+    res =
+      with {:ok, %{status: 200, body: body}} <-
+             Tesla.get(url,
+               headers: [{"Accept", "application/activity+json"}],
+               follow_redirect: true
+             ),
+           :ok <- Logger.debug("response okay, now decoding json"),
+           {:ok, data} <- Jason.decode(body) do
+        Logger.debug("Got activity+json response at actor's endpoint, now converting data")
+        {:ok, ActorConverter.as_to_model_data(data)}
+      else
+        # Actor is gone, probably deleted
+        {:ok, %{status: 410}} ->
+          Logger.info("Response HTTP 410")
+          {:error, :actor_deleted}
+
+        {:error, e} ->
+          Logger.warn("Could not decode actor at fetch #{url}, #{inspect(e)}")
+          {:error, e}
+
+        e ->
+          Logger.warn("Could not decode actor at fetch #{url}, #{inspect(e)}")
+          {:error, e}
+      end
+
+    res
+  end
+
+  @spec origin_check(String.t(), map()) :: boolean()
+  defp origin_check(url, data) do
+    if origin_check?(url, data) do
+      true
+    else
+      Sentry.capture_message("Object origin check failed", extra: %{url: url, data: data})
+      Logger.debug("Object origin check failed")
+      false
+    end
+  end
+
   @spec address_invalid(String.t()) :: false | {:error, :invalid_url}
   defp address_invalid(address) do
     with %URI{host: host, scheme: scheme} <- URI.parse(address),

lib/federation/activity_pub/preloader.ex

@@ -12,6 +12,7 @@ defmodule Mobilizon.Federation.ActivityPub.Preloader do
   alias Mobilizon.Resources.Resource
   alias Mobilizon.Tombstone
 
+  @spec maybe_preload(struct()) :: {:ok, struct()} | {:error, struct()}
   def maybe_preload(%Event{url: url}),
     do: {:ok, Events.get_public_event_by_url_with_preload!(url)}
 

lib/federation/activity_pub/refresher.ex

@@ -6,8 +6,8 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
   alias Mobilizon.Actors
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.{Fetcher, Relay, Transmogrifier, Utils}
-  alias Mobilizon.Storage.Repo
   require Logger
 
   @doc """
@@ -32,7 +32,7 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
   end
 
   def refresh_profile(%Actor{type: type, url: url}) when type in [:Person, :Application] do
-    with {:ok, %Actor{outbox_url: outbox_url}} <- ActivityPub.make_actor_from_url(url),
+    with {:ok, %Actor{outbox_url: outbox_url}} <- ActivityPubActor.make_actor_from_url(url),
          :ok <- fetch_collection(outbox_url, Relay.get_actor()) do
       :ok
     end
@@ -50,7 +50,7 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
             discussions_url: discussions_url,
             events_url: events_url
           }} <-
-           ActivityPub.make_actor_from_url(group_url),
+           ActivityPubActor.make_actor_from_url(group_url),
          :ok <- fetch_collection(outbox_url, on_behalf_of),
          :ok <- fetch_collection(members_url, on_behalf_of),
          :ok <- fetch_collection(resources_url, on_behalf_of),
@@ -60,9 +60,23 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
          :ok <- fetch_collection(events_url, on_behalf_of) do
       :ok
     else
+      {:error, err} ->
+        Logger.error("Error while refreshing a group")
+
+        Sentry.capture_message("Error while refreshing a group",
+          extra: %{group_url: group_url}
+        )
+
+        Logger.debug(inspect(err))
+
       err ->
         Logger.error("Error while refreshing a group")
-        Logger.error(inspect(err))
+
+        Sentry.capture_message("Error while refreshing a group",
+          extra: %{group_url: group_url}
+        )
+
+        Logger.debug(inspect(err))
     end
   end
 
@@ -96,14 +110,11 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
     end
   end
 
-  @spec refresh_all_external_groups :: any()
+  @spec refresh_all_external_groups :: :ok
   def refresh_all_external_groups do
-    Repo.transaction(fn ->
+    Actors.list_external_groups()
-      Actors.list_external_groups_for_stream()
+    |> Enum.filter(&Actors.needs_update?/1)
-      |> Stream.filter(&Actors.needs_update?/1)
+    |> Enum.each(&refresh_profile/1)
-      |> Stream.map(&refresh_profile/1)
-      |> Stream.run()
-    end)
   end
 
   defp process_collection(%{"type" => type, "orderedItems" => items}, _on_behalf_of)
@@ -122,6 +133,14 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
     :ok
   end
 
+  # Lemmy uses an OrderedCollection with the items property
+  defp process_collection(%{"type" => type, "items" => items} = collection, on_behalf_of)
+       when type in ["OrderedCollection", "OrderedCollectionPage"] do
+    collection
+    |> Map.put("orderedItems", items)
+    |> process_collection(on_behalf_of)
+  end
+
   defp process_collection(%{"type" => "OrderedCollection", "first" => first}, on_behalf_of)
        when is_map(first),
        do: process_collection(first, on_behalf_of)
@@ -150,6 +169,11 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
     Transmogrifier.handle_incoming(data)
   end
 
+  # If we're handling an announce activity
+  defp handling_element(%{"type" => "Announce"} = data) do
+    handling_element(get_in(data, ["object"]))
+  end
+
   # If we're handling directly an object
   defp handling_element(data) when is_map(data) do
     object = get_in(data, ["object"])

lib/federation/activity_pub/relay.ex

@@ -13,6 +13,7 @@ defmodule Mobilizon.Federation.ActivityPub.Relay do
 
   alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.{Activity, Refresher, Transmogrifier}
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.WebFinger
 
   alias Mobilizon.GraphQL.API.Follows
@@ -37,7 +38,8 @@ defmodule Mobilizon.Federation.ActivityPub.Relay do
   def follow(address) do
     with {:ok, target_instance} <- fetch_actor(address),
          %Actor{} = local_actor <- get_actor(),
-         {:ok, %Actor{} = target_actor} <- ActivityPub.get_or_fetch_actor_by_url(target_instance),
+         {:ok, %Actor{} = target_actor} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(target_instance),
          {:ok, activity, follow} <- Follows.follow(local_actor, target_actor) do
       Logger.info("Relay: followed instance #{target_instance}; id=#{activity.data["id"]}")
       {:ok, activity, follow}
@@ -56,7 +58,8 @@ defmodule Mobilizon.Federation.ActivityPub.Relay do
   def unfollow(address) do
     with {:ok, target_instance} <- fetch_actor(address),
          %Actor{} = local_actor <- get_actor(),
-         {:ok, %Actor{} = target_actor} <- ActivityPub.get_or_fetch_actor_by_url(target_instance),
+         {:ok, %Actor{} = target_actor} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(target_instance),
          {:ok, activity, follow} <- Follows.unfollow(local_actor, target_actor) do
       Logger.info("Relay: unfollowed instance #{target_instance}: id=#{activity.data["id"]}")
       {:ok, activity, follow}
@@ -73,7 +76,8 @@ defmodule Mobilizon.Federation.ActivityPub.Relay do
 
     with {:ok, target_instance} <- fetch_actor(address),
          %Actor{} = local_actor <- get_actor(),
-         {:ok, %Actor{} = target_actor} <- ActivityPub.get_or_fetch_actor_by_url(target_instance),
+         {:ok, %Actor{} = target_actor} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(target_instance),
          {:ok, activity, follow} <- Follows.accept(target_actor, local_actor) do
       {:ok, activity, follow}
     end
@@ -84,7 +88,8 @@ defmodule Mobilizon.Federation.ActivityPub.Relay do
 
     with {:ok, target_instance} <- fetch_actor(address),
          %Actor{} = local_actor <- get_actor(),
-         {:ok, %Actor{} = target_actor} <- ActivityPub.get_or_fetch_actor_by_url(target_instance),
+         {:ok, %Actor{} = target_actor} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(target_instance),
          {:ok, activity, follow} <- Follows.reject(target_actor, local_actor) do
       {:ok, activity, follow}
     end
@@ -94,7 +99,8 @@ defmodule Mobilizon.Federation.ActivityPub.Relay do
     Logger.debug("We're trying to refresh a remote instance")
 
     with {:ok, target_instance} <- fetch_actor(address),
-         {:ok, %Actor{} = target_actor} <- ActivityPub.get_or_fetch_actor_by_url(target_instance) do
+         {:ok, %Actor{} = target_actor} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(target_instance) do
       Refresher.refresh_profile(target_actor)
     end
   end

lib/federation/activity_pub/transmogrifier.ex

@@ -18,6 +18,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
 
   alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.{Activity, Refresher, Relay, Utils}
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.Types.Ownable
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Tombstone
@@ -117,12 +118,13 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
 
   def handle_incoming(%{
         "type" => "Create",
-        "object" => %{"type" => "Group", "id" => group_url} = _object
+        "object" => %{"type" => type, "id" => actor_url} = _object
-      }) do
+      })
-    Logger.info("Handle incoming to create a group")
+      when type in ["Group", "Person", "Actor"] do
+    Logger.info("Handle incoming to create an actor")
 
-    with {:ok, %Actor{} = group} <- ActivityPub.get_or_fetch_actor_by_url(group_url) do
+    with {:ok, %Actor{} = actor} <- ActivityPubActor.get_or_fetch_actor_by_url(actor_url) do
-      {:ok, nil, group}
+      {:ok, nil, actor}
     end
   end
 
@@ -201,8 +203,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
   def handle_incoming(
         %{"type" => "Follow", "object" => followed, "actor" => follower, "id" => id} = _data
       ) do
-    with {:ok, %Actor{} = followed} <- ActivityPub.get_or_fetch_actor_by_url(followed, true),
+    with {:ok, %Actor{} = followed} <- ActivityPubActor.get_or_fetch_actor_by_url(followed, true),
-         {:ok, %Actor{} = follower} <- ActivityPub.get_or_fetch_actor_by_url(follower),
+         {:ok, %Actor{} = follower} <- ActivityPubActor.get_or_fetch_actor_by_url(follower),
          {:ok, activity, object} <- ActivityPub.follow(follower, followed, id, false) do
       {:ok, activity, object}
     else
@@ -221,7 +223,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
 
     with {:existing_todo_list, nil} <-
            {:existing_todo_list, Todos.get_todo_list_by_url(object_url)},
-         {:ok, %Actor{url: actor_url}} <- ActivityPub.get_or_fetch_actor_by_url(actor_url),
+         {:ok, %Actor{url: actor_url}} <- ActivityPubActor.get_or_fetch_actor_by_url(actor_url),
          object_data when is_map(object_data) <-
            object |> Converter.TodoList.as_to_model_data(),
          {:ok, %Activity{} = activity, %TodoList{} = todo_list} <-
@@ -295,7 +297,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
         } = data
       ) do
     with actor_url <- Utils.get_actor(data),
-         {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(actor_url),
+         {:ok, %Actor{} = actor} <- ActivityPubActor.get_or_fetch_actor_by_url(actor_url),
          {:object_not_found, {:ok, %Activity{} = activity, object}} <-
            {:object_not_found,
             do_handle_incoming_accept_following(accepted_object, actor) ||
@@ -328,7 +330,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
         %{"type" => "Reject", "object" => rejected_object, "actor" => _actor, "id" => id} = data
       ) do
     with actor_url <- Utils.get_actor(data),
-         {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(actor_url),
+         {:ok, %Actor{} = actor} <- ActivityPubActor.get_or_fetch_actor_by_url(actor_url),
          {:object_not_found, {:ok, activity, object}} <-
            {:object_not_found,
             do_handle_incoming_reject_following(rejected_object, actor) ||
@@ -359,7 +361,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       ) do
     with actor_url <- Utils.get_actor(data),
          {:ok, %Actor{id: actor_id, suspended: false} = actor} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor_url),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor_url),
          :ok <- Logger.debug("Fetching contained object"),
          {:ok, entity} <- process_announce_data(object, actor),
          :ok <- eventually_create_share(object, entity, actor_id) do
@@ -371,14 +373,16 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(%{
+  def handle_incoming(
+        %{
           "type" => "Update",
           "object" => %{"type" => object_type} = object,
           "actor" => _actor_id
-      })
+        } = params
+      )
       when object_type in ["Person", "Group", "Application", "Service", "Organization"] do
     with {:ok, %Actor{suspended: false} = old_actor} <-
-           ActivityPub.get_or_fetch_actor_by_url(object["id"]),
+           ActivityPubActor.get_or_fetch_actor_by_url(object["id"]),
          object_data <-
            object |> Converter.Actor.as_to_model_data(),
          {:ok, %Activity{} = activity, %Actor{} = new_actor} <-
@@ -386,7 +390,11 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       {:ok, activity, new_actor}
     else
       e ->
-        Logger.error(inspect(e))
+        Sentry.capture_message("Error while handling an Update activity",
+          extra: %{params: params}
+        )
+
+        Logger.debug(inspect(e))
         :error
     end
   end
@@ -397,7 +405,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       ) do
     with actor <- Utils.get_actor(update_data),
          {:ok, %Actor{url: actor_url, suspended: false} = actor} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor),
          {:ok, %Event{} = old_event} <-
            object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(),
          object_data <- Converter.Event.as_to_model_data(object),
@@ -422,7 +430,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
 
     with actor <- Utils.get_actor(update_data),
          {:ok, %Actor{url: actor_url, suspended: false}} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor),
          {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, update_data)},
          object_data <- Converter.Comment.as_to_model_data(object),
          {:ok, old_entity} <- object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(),
@@ -442,7 +450,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       ) do
     with actor <- Utils.get_actor(update_data),
          {:ok, %Actor{url: actor_url, suspended: false} = actor} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor),
          {:ok, %Post{} = old_post} <-
            object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(),
          object_data <- Converter.Post.as_to_model_data(object),
@@ -470,7 +478,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       when type in ["ResourceCollection", "Document"] do
     with actor <- Utils.get_actor(update_data),
          {:ok, %Actor{url: actor_url, suspended: false}} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor),
          {:ok, %Resource{} = old_resource} <-
            object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(),
          object_data <- Converter.Resource.as_to_model_data(object),
@@ -495,7 +503,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
 
     with actor <- Utils.get_actor(update_data),
          {:ok, %Actor{url: actor_url, suspended: false} = actor} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor),
          {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, update_data)},
          object_data <- Converter.Member.as_to_model_data(object),
          {:ok, old_entity} <- object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(),
@@ -537,7 +545,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
         } = data
       ) do
     with actor <- Utils.get_actor(data),
-         {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(actor),
+         {:ok, %Actor{} = actor} <- ActivityPubActor.get_or_fetch_actor_by_url(actor),
          {:ok, object} <- fetch_obj_helper_as_activity_streams(object_id),
          {:ok, activity, object} <-
            ActivityPub.unannounce(actor, object, id, cancelled_activity_id, false) do
@@ -555,8 +563,9 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
           "id" => id
         } = _data
       ) do
-    with {:ok, %Actor{domain: nil} = followed} <- ActivityPub.get_or_fetch_actor_by_url(followed),
+    with {:ok, %Actor{domain: nil} = followed} <-
-         {:ok, %Actor{} = follower} <- ActivityPub.get_or_fetch_actor_by_url(follower),
+           ActivityPubActor.get_or_fetch_actor_by_url(followed),
+         {:ok, %Actor{} = follower} <- ActivityPubActor.get_or_fetch_actor_by_url(follower),
          {:ok, activity, object} <- ActivityPub.unfollow(follower, followed, id, false) do
       {:ok, activity, object}
     else
@@ -572,7 +581,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
         %{"type" => "Delete", "object" => object, "actor" => _actor, "id" => _id} = data
       ) do
     with actor_url <- Utils.get_actor(data),
-         {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(actor_url),
+         {: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} <-
@@ -586,8 +596,25 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
         Logger.warn("Object origin check failed")
         :error
 
+      {:actor, {:error, "Could not fetch by AP id"}} ->
+        {:error, :unknown_actor}
+
+      {:error, e} ->
+        Logger.debug(inspect(e))
+
+        # 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
     end
   end
@@ -598,7 +625,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       when type in ["ResourceCollection", "Document"] do
     with actor <- Utils.get_actor(data),
          {:ok, %Actor{url: actor_url, suspended: false} = actor} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor),
          {:ok, %Resource{} = old_resource} <-
            object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(),
          object_data <- Converter.Resource.as_to_model_data(object),
@@ -610,7 +637,12 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       {:ok, activity, new_resource}
     else
       e ->
-        Logger.error(inspect(e))
+        Logger.debug(inspect(e))
+
+        Sentry.capture_message("Error while handling an Move activity",
+          extra: %{data: data}
+        )
+
         :error
     end
   end
@@ -625,7 +657,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       ) do
     with actor <- Utils.get_actor(data),
          {:ok, %Actor{url: _actor_url, suspended: false} = actor} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor),
          object <- Utils.get_url(object),
          {:ok, object} <- ActivityPub.fetch_object_from_url(object),
          {:ok, activity, object} <-
@@ -643,7 +675,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
 
   def handle_incoming(%{"type" => "Leave", "object" => object, "actor" => actor} = data) do
     with actor <- Utils.get_actor(data),
-         {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(actor),
+         {:ok, %Actor{} = actor} <- ActivityPubActor.get_or_fetch_actor_by_url(actor),
          object <- Utils.get_url(object),
          {:ok, object} <- ActivityPub.fetch_object_from_url(object),
          {:ok, activity, object} <- ActivityPub.leave(object, actor, false) do
@@ -673,10 +705,10 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
     Logger.info("Handle incoming to invite someone")
 
     with {:ok, %Actor{} = actor} <-
-           data |> Utils.get_actor() |> ActivityPub.get_or_fetch_actor_by_url(),
+           data |> Utils.get_actor() |> ActivityPubActor.get_or_fetch_actor_by_url(),
          {:ok, object} <- object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(),
          {:ok, %Actor{} = target} <-
-           target |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url(),
+           target |> Utils.get_url() |> ActivityPubActor.get_or_fetch_actor_by_url(),
          {:ok, activity, %Member{} = member} <-
            ActivityPub.invite(object, actor, target, false, %{url: id}) do
       {:ok, activity, member}
@@ -689,10 +721,10 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
     Logger.info("Handle incoming to remove a member from a group")
 
     with {:ok, %Actor{id: moderator_id} = moderator} <-
-           data |> Utils.get_actor() |> ActivityPub.get_or_fetch_actor_by_url(),
+           data |> Utils.get_actor() |> ActivityPubActor.get_or_fetch_actor_by_url(),
          {:ok, person_id} <- get_remove_object(object),
          {:ok, %Actor{type: :Group, id: group_id} = group} <-
-           origin |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url(),
+           origin |> Utils.get_url() |> ActivityPubActor.get_or_fetch_actor_by_url(),
          {:is_admin, {:ok, %Member{role: role}}}
          when role in [:moderator, :administrator, :creator] <-
            {:is_admin, Actors.get_member(moderator_id, group_id)},
@@ -741,6 +773,11 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
   def handle_incoming(object) do
     Logger.info("Handing something with type #{object["type"]} not supported")
     Logger.debug(inspect(object))
+
+    Sentry.capture_message("Handing something with type #{object["type"]} not supported",
+      extra: %{object: object}
+    )
+
     {:error, :not_supported}
   end
 

lib/federation/activity_pub/utils.ex

@@ -14,6 +14,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
 
   alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.{Activity, Federator, Relay}
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.Types.Ownable
   alias Mobilizon.Federation.ActivityStream.Converter
   alias Mobilizon.Federation.HTTPSignatures
@@ -175,7 +176,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
   @spec remote_actors(list(String.t())) :: list(Actor.t())
   def remote_actors(recipients) do
     recipients
-    |> Enum.map(fn url -> ActivityPub.get_or_fetch_actor_by_url(url) end)
+    |> Enum.map(fn url -> ActivityPubActor.get_or_fetch_actor_by_url(url) end)
     |> Enum.map(fn {status, actor} ->
       case status do
         :ok ->
@@ -259,7 +260,9 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
     are_same_origin?(id, actor)
   end
 
-  def origin_check?(_id, %{"type" => type} = _params) when type in ["Actor", "Group"], do: true
+  def origin_check?(id, %{"type" => type, "id" => actor_id} = _params)
+      when type in ["Actor", "Person", "Group"],
+      do: id == actor_id
 
   def origin_check?(_id, %{"actor" => nil} = _args), do: false
 
@@ -701,4 +704,42 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
       true
     end
   end
+
+  @spec label_in_collection?(any(), any()) :: boolean()
+  defp label_in_collection?(url, coll) when is_binary(coll), do: url == coll
+  defp label_in_collection?(url, coll) when is_list(coll), do: url in coll
+  defp label_in_collection?(_, _), do: false
+
+  @spec label_in_message?(String.t(), map()) :: boolean()
+  def label_in_message?(label, params),
+    do:
+      [params["to"], params["cc"], params["bto"], params["bcc"]]
+      |> Enum.any?(&label_in_collection?(label, &1))
+
+  @spec unaddressed_message?(map()) :: boolean()
+  def unaddressed_message?(params),
+    do:
+      [params["to"], params["cc"], params["bto"], params["bcc"]]
+      |> Enum.all?(&is_nil(&1))
+
+  @spec recipient_in_message(Actor.t(), Actor.t(), map()) :: boolean()
+  def recipient_in_message(%Actor{url: url} = _recipient, %Actor{} = _actor, params),
+    do: label_in_message?(url, params) || unaddressed_message?(params)
+
+  defp extract_list(target) when is_binary(target), do: [target]
+  defp extract_list(lst) when is_list(lst), do: lst
+  defp extract_list(_), do: []
+
+  def maybe_splice_recipient(url, params) do
+    need_splice? =
+      !label_in_collection?(url, params["to"]) &&
+        !label_in_collection?(url, params["cc"])
+
+    if need_splice? do
+      cc_list = extract_list(params["cc"])
+      Map.put(params, "cc", [url | cc_list])
+    else
+      params
+    end
+  end
 end

lib/federation/activity_stream/converter/discussion.ex

@@ -8,7 +8,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Discussion do
 
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Discussions.Discussion
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Federation.ActivityStream.Converter.Discussion, as: DiscussionConverter
   alias Mobilizon.Storage.Repo
@@ -49,10 +49,10 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Discussion do
   def as_to_model_data(%{"type" => "Note", "name" => name} = object) when not is_nil(name) do
     with creator_url <- Map.get(object, "actor"),
          {:ok, %Actor{id: creator_id, suspended: false}} <-
-           ActivityPub.get_or_fetch_actor_by_url(creator_url),
+           ActivityPubActor.get_or_fetch_actor_by_url(creator_url),
          actor_url <- Map.get(object, "attributedTo"),
          {:ok, %Actor{id: actor_id, suspended: false}} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor_url) do
+           ActivityPubActor.get_or_fetch_actor_by_url(actor_url) do
       %{
         title: name,
         actor_id: actor_id,

lib/federation/activity_stream/converter/flag.ex

@@ -14,7 +14,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Flag do
   alias Mobilizon.Events.Event
   alias Mobilizon.Reports.Report
 
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.Relay
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
 
@@ -65,10 +65,11 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Flag do
 
   @spec as_to_model(map) :: map
   def as_to_model(%{"object" => objects} = object) do
-    with {:ok, %Actor{} = reporter} <- ActivityPub.get_or_fetch_actor_by_url(object["actor"]),
+    with {:ok, %Actor{} = reporter} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(object["actor"]),
          %Actor{} = reported <-
            Enum.reduce_while(objects, nil, fn url, _ ->
-             case ActivityPub.get_or_fetch_actor_by_url(url) do
+             case ActivityPubActor.get_or_fetch_actor_by_url(url) do
                {:ok, %Actor{} = actor} ->
                  {:halt, actor}
 

lib/federation/activity_stream/converter/member.ex

@@ -8,7 +8,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Member do
 
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Actors.Member, as: MemberModel
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.Utils
 
   alias Mobilizon.Federation.ActivityStream.Convertible
@@ -53,5 +53,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Member do
 
   @spec get_actor(String.t() | map() | nil) :: {:ok, Actor.t()} | {:error, String.t()}
   defp get_actor(nil), do: {:error, "nil property found for actor data"}
-  defp get_actor(actor), do: actor |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url()
+
+  defp get_actor(actor),
+    do: actor |> Utils.get_url() |> ActivityPubActor.get_or_fetch_actor_by_url()
 end

lib/federation/activity_stream/converter/post.ex

@@ -6,7 +6,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
   internal one, and back.
   """
   alias Mobilizon.Actors.Actor
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.{Audience, Utils}
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Federation.ActivityStream.Converter.Media, as: MediaConverter
@@ -91,7 +91,9 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
 
   @spec get_actor(String.t() | map() | nil) :: {:ok, Actor.t()} | {:error, String.t()}
   defp get_actor(nil), do: {:error, "nil property found for actor data"}
-  defp get_actor(actor), do: actor |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url()
+
+  defp get_actor(actor),
+    do: actor |> Utils.get_url() |> ActivityPubActor.get_or_fetch_actor_by_url()
 
   defp to_date(nil), do: nil
   defp to_date(%DateTime{} = date), do: DateTime.to_iso8601(date)

lib/federation/activity_stream/converter/resource.ex

@@ -7,6 +7,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Resource do
   """
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.Utils
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Resources
@@ -88,7 +89,9 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Resource do
 
   @spec get_actor(String.t() | map() | nil) :: {:ok, Actor.t()} | {:error, String.t()}
   defp get_actor(nil), do: {:error, "nil property found for actor data"}
-  defp get_actor(actor), do: actor |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url()
+
+  defp get_actor(actor),
+    do: actor |> Utils.get_url() |> ActivityPubActor.get_or_fetch_actor_by_url()
 
   defp get_context(%Resource{parent_id: nil, actor: %Actor{resources_url: resources_url}}),
     do: resources_url

lib/federation/activity_stream/converter/todo.ex

@@ -7,6 +7,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Todo do
   """
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Todos
   alias Mobilizon.Todos.{Todo, TodoList}
@@ -51,7 +52,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Todo do
         %{"type" => "Todo", "actor" => actor_url, "todoList" => todo_list_url} = object
       ) do
     with {:ok, %Actor{id: creator_id} = _creator} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor_url),
+           ActivityPubActor.get_or_fetch_actor_by_url(actor_url),
          {:todo_list, %TodoList{id: todo_list_id}} <-
            {:todo_list, Todos.get_todo_list_by_url(todo_list_url)} do
       %{

lib/federation/activity_stream/converter/todo_list.ex

@@ -6,7 +6,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.TodoList do
   internal one, and back.
   """
   alias Mobilizon.Actors.Actor
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Todos.TodoList
 
@@ -39,7 +39,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.TodoList do
   @impl Converter
   @spec as_to_model_data(map) :: {:ok, map} | {:error, any()}
   def as_to_model_data(%{"type" => "TodoList", "actor" => actor_url} = object) do
-    case ActivityPub.get_or_fetch_actor_by_url(actor_url) do
+    case ActivityPubActor.get_or_fetch_actor_by_url(actor_url) do
       {:ok, %Actor{type: :Group, id: group_id} = _group} ->
         %{
           title: object["name"],

lib/federation/activity_stream/converter/utils.ex

@@ -10,7 +10,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Utils do
   alias Mobilizon.Mention
   alias Mobilizon.Storage.Repo
 
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityStream.Converter.Media, as: MediaConverter
 
   alias Mobilizon.Web.Endpoint
@@ -114,7 +114,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Utils do
   @spec create_mention(map(), list()) :: list()
   defp create_mention(mention, acc) when is_map(mention) do
     with true <- mention["type"] == "Mention",
-         {:ok, %Actor{id: actor_id}} <- ActivityPub.get_or_fetch_actor_by_url(mention["href"]) do
+         {:ok, %Actor{id: actor_id}} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(mention["href"]) do
       acc ++ [%{actor_id: actor_id}]
     else
       _err ->
@@ -169,7 +170,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Utils do
   @spec fetch_actor(String.t()) :: Actor.t()
   defp fetch_actor(actor_url) do
     with {:ok, %Actor{suspended: false} = actor} <-
-           ActivityPub.get_or_fetch_actor_by_url(actor_url) do
+           ActivityPubActor.get_or_fetch_actor_by_url(actor_url) do
       actor
     end
   end

lib/federation/http_signatures/signature.ex

@@ -12,7 +12,7 @@ defmodule Mobilizon.Federation.HTTPSignatures.Signature do
 
   alias Mobilizon.Actors.Actor
 
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
 
   require Logger
 
@@ -50,9 +50,10 @@ defmodule Mobilizon.Federation.HTTPSignatures.Signature do
 
   # Gets a public key for a given ActivityPub actor ID (url).
   @spec get_public_key_for_url(String.t()) ::
-          {:ok, String.t()} | {:error, :actor_fetch_error | :pem_decode_error}
+          {:ok, String.t()}
+          | {:error, :actor_fetch_error | :pem_decode_error | :actor_not_fetchable}
   defp get_public_key_for_url(url) do
-    with {:ok, %Actor{keys: keys}} <- ActivityPub.get_or_fetch_actor_by_url(url),
+    with {:ok, %Actor{keys: keys}} <- ActivityPubActor.get_or_fetch_actor_by_url(url),
          {:ok, public_key} <- prepare_public_key(keys) do
       {:ok, public_key}
     else
@@ -61,8 +62,16 @@ defmodule Mobilizon.Federation.HTTPSignatures.Signature do
 
         {:error, :pem_decode_error}
 
-      _ ->
+      {:error, "Could not fetch by AP id"} ->
+        {:error, :actor_not_fetchable}
+
+      err ->
+        Sentry.capture_message("Unable to fetch actor, so no keys for you",
+          extra: %{url: url}
+        )
+
         Logger.error("Unable to fetch actor, so no keys for you")
+        Logger.error(inspect(err))
 
         {:error, :actor_fetch_error}
     end
@@ -74,9 +83,6 @@ defmodule Mobilizon.Federation.HTTPSignatures.Signature do
          :ok <- Logger.debug("Fetching public key for #{actor_id}"),
          {:ok, public_key} <- get_public_key_for_url(actor_id) do
       {:ok, public_key}
-    else
-      e ->
-        {:error, e}
     end
   end
 
@@ -84,12 +90,9 @@ defmodule Mobilizon.Federation.HTTPSignatures.Signature do
     with %{"keyId" => kid} <- HTTPSignatures.signature_for_conn(conn),
          actor_id <- key_id_to_actor_url(kid),
          :ok <- Logger.debug("Refetching public key for #{actor_id}"),
-         {:ok, _actor} <- ActivityPub.make_actor_from_url(actor_id),
+         {:ok, _actor} <- ActivityPubActor.make_actor_from_url(actor_id),
          {:ok, public_key} <- get_public_key_for_url(actor_id) do
       {:ok, public_key}
-    else
-      e ->
-        {:error, e}
     end
   end
 

lib/federation/web_finger/web_finger.ex

@@ -10,7 +10,7 @@ defmodule Mobilizon.Federation.WebFinger do
 
   alias Mobilizon.Actors
   alias Mobilizon.Actors.Actor
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.WebFinger.XmlBuilder
   alias Mobilizon.Service.HTTP.{HostMetaClient, WebfingerClient}
   alias Mobilizon.Web.Endpoint
@@ -56,7 +56,7 @@ defmodule Mobilizon.Federation.WebFinger do
       {:ok, represent_actor(actor, "JSON")}
     else
       _e ->
-        case ActivityPub.get_or_fetch_actor_by_url(resource) do
+        case ActivityPubActor.get_or_fetch_actor_by_url(resource) do
           {:ok, %Actor{} = actor} when not is_nil(actor) ->
             {:ok, represent_actor(actor, "JSON")}
 

lib/graphql/api/search.ex

@@ -10,6 +10,7 @@ defmodule Mobilizon.GraphQL.API.Search do
   alias Mobilizon.Storage.Page
 
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
 
   require Logger
 
@@ -92,7 +93,7 @@ defmodule Mobilizon.GraphQL.API.Search do
   # If the search string is an username
   @spec process_from_username(String.t()) :: Page.t()
   defp process_from_username(search) do
-    case ActivityPub.find_or_make_actor_from_nickname(search) do
+    case ActivityPubActor.find_or_make_actor_from_nickname(search) do
       {:ok, actor} ->
         %Page{total: 1, elements: [actor]}
 

lib/graphql/resolvers/group.ex

@@ -7,6 +7,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Group do
   alias Mobilizon.{Actors, Events, Users}
   alias Mobilizon.Actors.{Actor, Member}
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.GraphQL.API
   alias Mobilizon.Users.User
   alias Mobilizon.Web.Upload
@@ -27,7 +28,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Group do
         }
       ) do
     with {:group, {:ok, %Actor{id: group_id, suspended: false} = group}} <-
-           {:group, ActivityPub.find_or_make_group_from_nickname(name)},
+           {:group, ActivityPubActor.find_or_make_group_from_nickname(name)},
          {:actor, %Actor{id: actor_id} = _actor} <- {:actor, Users.get_actor_for_user(user)},
          {:member, true} <- {:member, Actors.is_member?(actor_id, group_id)} do
       {:ok, group}
@@ -45,7 +46,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Group do
 
   def find_group(_parent, %{preferred_username: name}, _resolution) do
     with {:ok, %Actor{suspended: false} = actor} <-
-           ActivityPub.find_or_make_group_from_nickname(name),
+           ActivityPubActor.find_or_make_group_from_nickname(name),
          %Actor{} = actor <- restrict_fields_for_non_member_request(actor) do
       {:ok, actor}
     else

lib/graphql/resolvers/member.ex

@@ -7,6 +7,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Member do
   alias Mobilizon.{Actors, Users}
   alias Mobilizon.Actors.{Actor, Member}
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.Refresher
   alias Mobilizon.Storage.Page
   alias Mobilizon.Users.User
@@ -70,7 +71,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Member do
            target_actor_username |> String.trim() |> String.trim_leading("@"),
          {:target_actor_username, {:ok, %Actor{id: target_actor_id} = target_actor}} <-
            {:target_actor_username,
-            ActivityPub.find_or_make_actor_from_nickname(target_actor_username)},
+            ActivityPubActor.find_or_make_actor_from_nickname(target_actor_username)},
          {:existant, true} <-
            {:existant, check_member_not_existant_or_rejected(target_actor_id, group.id)},
          {:ok, _activity, %Member{} = member} <- ActivityPub.invite(group, actor, target_actor) do

lib/graphql/resolvers/person.ex

@@ -13,6 +13,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Person do
   import Mobilizon.Web.Gettext
 
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   require Logger
 
   alias Mobilizon.Web.Upload
@@ -39,7 +40,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Person do
         context: %{current_user: %User{} = user}
       }) do
     with {:ok, %Actor{id: actor_id} = actor} <-
-           ActivityPub.find_or_make_actor_from_nickname(preferred_username),
+           ActivityPubActor.find_or_make_actor_from_nickname(preferred_username),
          {:own, {:is_owned, _}} <- {:own, User.owns_actor(user, actor_id)} do
       {:ok, actor}
     else

lib/mix/tasks/mobilizon/actors/refresh.ex

@@ -4,7 +4,7 @@ defmodule Mix.Tasks.Mobilizon.Actors.Refresh do
   """
   use Mix.Task
   alias Mobilizon.Actors.Actor
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Storage.Repo
   import Ecto.Query
   import Mix.Tasks.Mobilizon.Common
@@ -65,7 +65,7 @@ defmodule Mix.Tasks.Mobilizon.Actors.Refresh do
   def run([preferred_username]) do
     start_mobilizon()
 
-    case ActivityPub.make_actor_from_nickname(preferred_username) do
+    case ActivityPubActor.make_actor_from_nickname(preferred_username) do
       {:ok, %Actor{}} ->
         shell_info("""
         Actor #{preferred_username} refreshed
@@ -89,7 +89,7 @@ defmodule Mix.Tasks.Mobilizon.Actors.Refresh do
 
   @spec make_actor(String.t(), boolean()) :: any()
   defp make_actor(username, verbose) do
-    ActivityPub.make_actor_from_nickname(username)
+    ActivityPubActor.make_actor_from_nickname(username)
   rescue
     _ ->
       if verbose do

lib/mobilizon/actors/actors.ex

@@ -374,12 +374,22 @@ defmodule Mobilizon.Actors do
 
       {:error, remove, error, _} when remove in [:remove_banner, :remove_avatar] ->
         Logger.error("Error while deleting actor's banner or avatar")
-        Logger.error(inspect(error, pretty: true))
+
+        Sentry.capture_message("Error while deleting actor's banner or avatar",
+          extra: %{err: error}
+        )
+
+        Logger.debug(inspect(error, pretty: true))
         {:error, error}
 
       err ->
         Logger.error("Unknown error while deleting actor")
-        Logger.error(inspect(err, pretty: true))
+
+        Sentry.capture_message("Error while deleting actor's banner or avatar",
+          extra: %{err: err}
+        )
+
+        Logger.debug(inspect(err, pretty: true))
         {:error, err}
     end
   end
@@ -652,10 +662,11 @@ defmodule Mobilizon.Actors do
   @doc """
   Lists the groups.
   """
-  @spec list_groups_for_stream :: Enum.t()
+  @spec list_external_groups(non_neg_integer()) :: list(Actor.t())
-  def list_external_groups_for_stream do
+  def list_external_groups(limit \\ 100) when limit > 0 do
     external_groups_query()
-    |> Repo.stream()
+    |> limit(^limit)
+    |> Repo.all()
   end
 
   @doc """

lib/mobilizon/addresses/address.ex

@@ -63,6 +63,7 @@ defmodule Mobilizon.Addresses.Address do
     |> cast(attrs, @attrs)
     |> set_url()
     |> validate_required(@required_attrs)
+    |> unique_constraint(:url, name: :addresses_url_index)
   end
 
   @spec set_url(Ecto.Changeset.t()) :: Ecto.Changeset.t()

lib/web/controllers/activity_pub_controller.ex

@@ -10,7 +10,8 @@ defmodule Mobilizon.Web.ActivityPubController do
   alias Mobilizon.Actors.{Actor, Member}
 
   alias Mobilizon.Federation.ActivityPub
-  alias Mobilizon.Federation.ActivityPub.Federator
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
+  alias Mobilizon.Federation.ActivityPub.{Federator, Utils}
 
   alias Mobilizon.Web.ActivityPub.ActorView
   alias Mobilizon.Web.Cache
@@ -105,7 +106,17 @@ defmodule Mobilizon.Web.ActivityPubController do
     actor_collection(conn, "outbox", args)
   end
 
-  # TODO: Ensure that this inbox is a recipient of the message
+  def inbox(%{assigns: %{valid_signature: true}} = conn, %{"name" => preferred_username} = params) do
+    with %Actor{url: recipient_url} = recipient <-
+           Actors.get_local_actor_by_name(preferred_username),
+         {:ok, %Actor{} = actor} <- ActivityPubActor.get_or_fetch_actor_by_url(params["actor"]),
+         true <- Utils.recipient_in_message(recipient, actor, params),
+         params <- Utils.maybe_splice_recipient(recipient_url, params) do
+      Federator.enqueue(:incoming_ap_doc, params)
+      json(conn, "ok")
+    end
+  end
+
   def inbox(%{assigns: %{valid_signature: true}} = conn, params) do
     Logger.debug("Got something with valid signature inside inbox")
     Federator.enqueue(:incoming_ap_doc, params)
@@ -114,7 +125,7 @@ defmodule Mobilizon.Web.ActivityPubController do
 
   # only accept relayed Creates
   def inbox(conn, %{"type" => "Create"} = params) do
-    Logger.info(
+    Logger.debug(
       "Signature missing or not from author, relayed Create message, fetching object from source"
     )
 
@@ -126,8 +137,9 @@ defmodule Mobilizon.Web.ActivityPubController do
   def inbox(conn, params) do
     headers = Enum.into(conn.req_headers, %{})
 
-    if String.contains?(headers["signature"], params["actor"]) do
+    if headers["signature"] && params["actor"] &&
-      Logger.error(
+         String.contains?(headers["signature"], params["actor"]) do
+      Logger.debug(
         "Signature validation error for: #{params["actor"]}, make sure you are forwarding the HTTP Host header!"
       )
 

lib/web/plugs/mapped_signature_to_identity.ex

@@ -12,7 +12,7 @@ defmodule Mobilizon.Web.Plugs.MappedSignatureToIdentity do
 
   alias Mobilizon.Actors.Actor
 
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.Utils
   alias Mobilizon.Federation.HTTPSignatures.Signature
 
@@ -34,7 +34,7 @@ defmodule Mobilizon.Web.Plugs.MappedSignatureToIdentity do
   @spec actor_from_key_id(Plug.Conn.t()) :: Actor.t() | nil
   defp actor_from_key_id(conn) do
     with key_actor_id when is_binary(key_actor_id) <- key_id_from_conn(conn),
-         {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(key_actor_id) do
+         {:ok, %Actor{} = actor} <- ActivityPubActor.get_or_fetch_actor_by_url(key_actor_id) do
       actor
     else
       _ ->

lib/web/views/error_view.ex

@@ -47,6 +47,10 @@ defmodule Mobilizon.Web.ErrorView do
     %{msg: "Not acceptable"}
   end
 
+  def render("500.json", assigns) do
+    render("500.html", assigns)
+  end
+
   def render("500.html", assigns) do
     Mobilizon.Config.instance_config()
     |> Keyword.get(:default_language, "en")

priv/repo/migrations/20210422140923_cleanup_addresses.exs

@@ -0,0 +1,75 @@
+defmodule Mobilizon.Storage.Repo.Migrations.CleanupAddresses do
+  use Ecto.Migration
+
+  def up do
+    # Make sure we don't have any duplicate addresses
+    rows = fetch_bad_rows()
+    Enum.each(rows, &process_row/1)
+
+    flush()
+  end
+
+  def down do
+    # No way down
+  end
+
+  defp fetch_bad_rows() do
+    %Postgrex.Result{rows: rows} =
+      Ecto.Adapters.SQL.query!(
+        Mobilizon.Storage.Repo,
+        "SELECT * FROM (
+          SELECT id, url,
+          ROW_NUMBER() OVER(PARTITION BY url ORDER BY id asc) AS Row
+          FROM addresses
+        ) dups
+        WHERE dups.Row > 1;"
+      )
+
+    rows
+  end
+
+  defp process_row([id, url, _row]) do
+    first_id = find_first_address_id(url)
+
+    if id != first_id do
+      repair_events(id, first_id)
+      repair_actors(id, first_id)
+      delete_row(id)
+    end
+  end
+
+  defp find_first_address_id(url) do
+    %Postgrex.Result{rows: [[id]]} =
+      Ecto.Adapters.SQL.query!(
+        Mobilizon.Storage.Repo,
+        "SELECT id FROM addresses WHERE url = $1 order by id asc limit 1",
+        [url]
+      )
+
+    id
+  end
+
+  defp repair_events(id, first_id) do
+    Ecto.Adapters.SQL.query!(
+      Mobilizon.Storage.Repo,
+      "UPDATE events SET physical_address_id = $1 WHERE physical_address_id = $2",
+      [first_id, id]
+    )
+  end
+
+  defp repair_actors(id, first_id) do
+    Ecto.Adapters.SQL.query!(
+      Mobilizon.Storage.Repo,
+      "UPDATE actors SET physical_address_id = $1 WHERE physical_address_id = $2",
+      [first_id, id]
+    )
+  end
+
+  defp delete_row(id) do
+    Ecto.Adapters.SQL.query!(
+      Mobilizon.Storage.Repo,
+      "DELETE FROM addresses WHERE id = $1",
+      [id]
+    )
+  end
+end

priv/repo/migrations/20210422155913_add_index_to_addresses.exs

@@ -0,0 +1,11 @@
+defmodule Mobilizon.Storage.Repo.Migrations.AddIndexToAddresses do
+  use Ecto.Migration
+
+  def up do
+    create_if_not_exists(unique_index("addresses", [:url]))
+  end
+
+  def down do
+    drop_if_exists(index("addresses", [:url]))
+  end
+end

test/federation/activity_pub/activity_pub_test.exs

@@ -11,13 +11,12 @@ defmodule Mobilizon.Federation.ActivityPubTest do
   import Mox
   import Mobilizon.Factory
 
-  alias Mobilizon.{Actors, Discussions, Events}
+  alias Mobilizon.{Discussions, Events}
-  alias Mobilizon.Actors.Actor
   alias Mobilizon.Resources.Resource
   alias Mobilizon.Todos.{Todo, TodoList}
 
   alias Mobilizon.Federation.ActivityPub
-  alias Mobilizon.Federation.ActivityPub.{Relay, Utils}
+  alias Mobilizon.Federation.ActivityPub.Utils
   alias Mobilizon.Federation.HTTPSignatures.Signature
   alias Mobilizon.Service.HTTP.ActivityPub.Mock
 
@@ -40,116 +39,6 @@ defmodule Mobilizon.Federation.ActivityPubTest do
     end
   end
 
-  describe "fetching actor from its url" do
-    test "returns an actor from nickname" do
-      use_cassette "activity_pub/fetch_tcit@framapiaf.org" do
-        assert {:ok,
-                %Actor{preferred_username: "tcit", domain: "framapiaf.org", visibility: :public} =
-                  _actor} = ActivityPub.make_actor_from_nickname("tcit@framapiaf.org")
-      end
-
-      use_cassette "activity_pub/fetch_tcit@framapiaf.org_not_discoverable" do
-        assert {:ok,
-                %Actor{preferred_username: "tcit", domain: "framapiaf.org", visibility: :unlisted} =
-                  _actor} = ActivityPub.make_actor_from_nickname("tcit@framapiaf.org")
-      end
-    end
-
-    @actor_url "https://framapiaf.org/users/tcit"
-    test "returns an actor from url" do
-      # Initial fetch
-      use_cassette "activity_pub/fetch_framapiaf.org_users_tcit" do
-        # Unlisted because discoverable is not present in the JSON payload
-        assert {:ok,
-                %Actor{preferred_username: "tcit", domain: "framapiaf.org", visibility: :unlisted}} =
-                 ActivityPub.get_or_fetch_actor_by_url(@actor_url)
-      end
-
-      # Fetch uses cache if Actors.needs_update? returns false
-      with_mocks([
-        {Actors, [:passthrough],
-         [
-           get_actor_by_url: fn @actor_url, false ->
-             {:ok,
-              %Actor{
-                preferred_username: "tcit",
-                domain: "framapiaf.org"
-              }}
-           end,
-           needs_update?: fn _ -> false end
-         ]},
-        {ActivityPub, [:passthrough],
-         make_actor_from_url: fn @actor_url, false ->
-           {:ok,
-            %Actor{
-              preferred_username: "tcit",
-              domain: "framapiaf.org"
-            }}
-         end}
-      ]) do
-        assert {:ok, %Actor{preferred_username: "tcit", domain: "framapiaf.org"}} =
-                 ActivityPub.get_or_fetch_actor_by_url(@actor_url)
-
-        assert_called(Actors.needs_update?(:_))
-        refute called(ActivityPub.make_actor_from_url(@actor_url, false))
-      end
-
-      # Fetch doesn't use cache if Actors.needs_update? returns true
-      with_mocks([
-        {Actors, [:passthrough],
-         [
-           get_actor_by_url: fn @actor_url, false ->
-             {:ok,
-              %Actor{
-                preferred_username: "tcit",
-                domain: "framapiaf.org"
-              }}
-           end,
-           needs_update?: fn _ -> true end
-         ]},
-        {ActivityPub, [:passthrough],
-         make_actor_from_url: fn @actor_url, false ->
-           {:ok,
-            %Actor{
-              preferred_username: "tcit",
-              domain: "framapiaf.org"
-            }}
-         end}
-      ]) do
-        assert {:ok, %Actor{preferred_username: "tcit", domain: "framapiaf.org"}} =
-                 ActivityPub.get_or_fetch_actor_by_url(@actor_url)
-
-        assert_called(ActivityPub.get_or_fetch_actor_by_url(@actor_url))
-        assert_called(Actors.get_actor_by_url(@actor_url, false))
-        assert_called(Actors.needs_update?(:_))
-        assert_called(ActivityPub.make_actor_from_url(@actor_url, false))
-      end
-    end
-
-    @public_url "https://www.w3.org/ns/activitystreams#Public"
-    test "activitystreams#Public uri returns Relay actor" do
-      assert ActivityPub.get_or_fetch_actor_by_url(@public_url) == {:ok, Relay.get_actor()}
-    end
-  end
-
-  describe "create activities" do
-    #    test "removes doubled 'to' recipients" do
-    #      actor = insert(:actor)
-    #
-    #      {:ok, activity, _} =
-    #        ActivityPub.create(%{
-    #          to: ["user1", "user1", "user2"],
-    #          actor: actor,
-    #          context: "",
-    #          object: %{}
-    #        })
-    #
-    #      assert activity.data["to"] == ["user1", "user2"]
-    #      assert activity.actor == actor.url
-    #      assert activity.recipients == ["user1", "user2"]
-    #    end
-  end
-
   describe "fetching an" do
     test "object by url" do
       url = "https://framapiaf.org/users/Framasoft/statuses/102093631881522097"

test/federation/activity_pub/actor_test.exs

@@ -0,0 +1,120 @@
+defmodule Mobilizon.Federation.ActivityPub.ActorTest do
+  use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney
+  use Mobilizon.DataCase
+
+  import Mock
+
+  alias Mobilizon.Actors
+  alias Mobilizon.Actors.Actor
+
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
+  alias Mobilizon.Federation.ActivityPub.{Fetcher, Relay}
+
+  describe "fetching actor from its url" do
+    test "returns an actor from nickname" do
+      use_cassette "activity_pub/fetch_tcit@framapiaf.org" do
+        assert {:ok,
+                %Actor{preferred_username: "tcit", domain: "framapiaf.org", visibility: :public} =
+                  _actor} = ActivityPubActor.make_actor_from_nickname("tcit@framapiaf.org")
+      end
+
+      use_cassette "activity_pub/fetch_tcit@framapiaf.org_not_discoverable" do
+        assert {:ok,
+                %Actor{preferred_username: "tcit", domain: "framapiaf.org", visibility: :unlisted} =
+                  _actor} = ActivityPubActor.make_actor_from_nickname("tcit@framapiaf.org")
+      end
+    end
+
+    @actor_url "https://framapiaf.org/users/tcit"
+    test "returns an actor from url" do
+      # Initial fetch
+      use_cassette "activity_pub/fetch_framapiaf.org_users_tcit" do
+        # Unlisted because discoverable is not present in the JSON payload
+        assert {:ok,
+                %Actor{preferred_username: "tcit", domain: "framapiaf.org", visibility: :unlisted}} =
+                 ActivityPubActor.get_or_fetch_actor_by_url(@actor_url)
+      end
+
+      # Fetch uses cache if Actors.needs_update? returns false
+      with_mocks([
+        {Actors, [:passthrough],
+         [
+           get_actor_by_url: fn @actor_url, false ->
+             {:ok,
+              %Actor{
+                preferred_username: "tcit",
+                domain: "framapiaf.org"
+              }}
+           end,
+           needs_update?: fn _ -> false end
+         ]},
+        {ActivityPubActor, [:passthrough],
+         make_actor_from_url: fn @actor_url, false ->
+           {:ok,
+            %Actor{
+              preferred_username: "tcit",
+              domain: "framapiaf.org"
+            }}
+         end}
+      ]) do
+        assert {:ok, %Actor{preferred_username: "tcit", domain: "framapiaf.org"}} =
+                 ActivityPubActor.get_or_fetch_actor_by_url(@actor_url)
+
+        assert_called(Actors.needs_update?(:_))
+        refute called(ActivityPubActor.make_actor_from_url(@actor_url, false))
+      end
+
+      # Fetch doesn't use cache if Actors.needs_update? returns true
+      with_mocks([
+        {Actors, [:passthrough],
+         [
+           get_actor_by_url: fn @actor_url, false ->
+             {:ok,
+              %Actor{
+                preferred_username: "tcit",
+                domain: "framapiaf.org"
+              }}
+           end,
+           needs_update?: fn _ -> true end
+         ]},
+        {ActivityPubActor, [:passthrough],
+         make_actor_from_url: fn @actor_url, false ->
+           {:ok,
+            %Actor{
+              preferred_username: "tcit",
+              domain: "framapiaf.org"
+            }}
+         end}
+      ]) do
+        assert {:ok, %Actor{preferred_username: "tcit", domain: "framapiaf.org"}} =
+                 ActivityPubActor.get_or_fetch_actor_by_url(@actor_url)
+
+        assert_called(ActivityPubActor.get_or_fetch_actor_by_url(@actor_url))
+        assert_called(Actors.get_actor_by_url(@actor_url, false))
+        assert_called(Actors.needs_update?(:_))
+        assert_called(ActivityPubActor.make_actor_from_url(@actor_url, false))
+      end
+    end
+
+    test "handles remote actor being deleted" do
+      with_mocks([
+        {Fetcher, [:passthrough],
+         fetch_and_prepare_actor_from_url: fn @actor_url ->
+           {:error, :actor_deleted}
+         end}
+      ]) do
+        assert match?(
+                 {:error, :actor_deleted},
+                 ActivityPubActor.make_actor_from_url(@actor_url, false)
+               )
+
+        assert_called(Fetcher.fetch_and_prepare_actor_from_url(@actor_url))
+      end
+    end
+
+    @public_url "https://www.w3.org/ns/activitystreams#Public"
+    test "activitystreams#Public uri returns Relay actor" do
+      assert ActivityPubActor.get_or_fetch_actor_by_url(@public_url) == {:ok, Relay.get_actor()}
+    end
+  end
+end

test/federation/activity_pub/transmogrifier/delete_test.exs

@@ -3,7 +3,6 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.DeleteTest do
   use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney
   use Oban.Testing, repo: Mobilizon.Storage.Repo
   import Mobilizon.Factory
-  import ExUnit.CaptureLog
   import Mox
 
   alias Mobilizon.{Actors, Discussions, Events, Posts, Resources}
@@ -78,7 +77,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.DeleteTest do
         data
         |> Map.put("object", object)
 
-      :error = Transmogrifier.handle_incoming(data)
+      {:error, :unknown_actor} = Transmogrifier.handle_incoming(data)
 
       assert Discussions.get_comment_from_url(comment.url)
     end
@@ -119,13 +118,14 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.DeleteTest do
 
     test "it fails for incoming actor deletes with spoofed origin" do
       %{url: url} = insert(:actor)
-      deleted_actor_url = "https://framapiaf.org/users/admin"
 
       data =
         File.read!("test/fixtures/mastodon-delete-user.json")
         |> Jason.decode!()
         |> Map.put("actor", url)
 
+      deleted_actor_url = "https://framapiaf.org/users/admin"
+
       deleted_actor_data =
         File.read!("test/fixtures/mastodon-actor.json")
         |> Jason.decode!()
@@ -137,9 +137,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.DeleteTest do
           {:ok, %Tesla.Env{status: 200, body: deleted_actor_data}}
       end)
 
-      assert capture_log(fn ->
       assert :error == Transmogrifier.handle_incoming(data)
-             end) =~ "Object origin check failed"
 
       assert Actors.get_actor_by_url(url)
     end

test/federation/activity_pub/transmogrifier/posts_test.exs

@@ -7,7 +7,6 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.PostsTest do
   alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier}
   alias Mobilizon.Federation.ActivityStream.Convertible
   alias Mobilizon.Posts.Post
-  alias Mobilizon.Service.HTTP.ActivityPub.Mock
 
   describe "handle incoming posts" do
     setup :verify_on_exit!

test/federation/activity_pub/transmogrifier_test.exs

@@ -21,6 +21,7 @@ defmodule Mobilizon.Federation.ActivityPub.TransmogrifierTest do
   alias Mobilizon.Todos.{Todo, TodoList}
 
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.Utils
   alias Mobilizon.Federation.ActivityPub.{Activity, Relay, Transmogrifier}
   alias Mobilizon.Federation.ActivityStream.Convertible
@@ -89,7 +90,7 @@ defmodule Mobilizon.Federation.ActivityPub.TransmogrifierTest do
           preferred_username: "member"
         )
 
-      with_mock ActivityPub, [:passthrough],
+      with_mock ActivityPubActor, [:passthrough],
         get_or_fetch_actor_by_url: fn url ->
           case url do
             ^group_url -> {:ok, group}
@@ -168,7 +169,7 @@ defmodule Mobilizon.Federation.ActivityPub.TransmogrifierTest do
       group = insert(:group, domain: "morebilizon.com", url: @mobilizon_group_url)
       %Actor{url: actor_url} = actor = insert(:actor)
 
-      with_mock ActivityPub, [:passthrough],
+      with_mock ActivityPubActor, [:passthrough],
         get_or_fetch_actor_by_url: fn url ->
           case url do
             @mobilizon_group_url -> {:ok, group}
@@ -198,7 +199,7 @@ defmodule Mobilizon.Federation.ActivityPub.TransmogrifierTest do
     test "it accepts incoming todo lists and handles group being not found" do
       %Actor{url: actor_url} = actor = insert(:actor)
 
-      with_mock ActivityPub, [:passthrough],
+      with_mock ActivityPubActor, [:passthrough],
         get_or_fetch_actor_by_url: fn url ->
           case url do
             @mobilizon_group_url -> {:error, "Not found"}
@@ -274,7 +275,7 @@ defmodule Mobilizon.Federation.ActivityPub.TransmogrifierTest do
       group = insert(:group, domain: "morebilizon.com", url: @mobilizon_group_url)
       %Actor{url: actor_url} = actor = insert(:actor)
 
-      with_mock ActivityPub, [:passthrough],
+      with_mock ActivityPubActor, [:passthrough],
         get_or_fetch_actor_by_url: fn url ->
           case url do
             @mobilizon_group_url -> {:ok, group}
@@ -304,7 +305,7 @@ defmodule Mobilizon.Federation.ActivityPub.TransmogrifierTest do
     test "it accepts incoming todo lists and handles group being not found" do
       %Actor{url: actor_url} = actor = insert(:actor)
 
-      with_mock ActivityPub, [:passthrough],
+      with_mock ActivityPubActor, [:passthrough],
         get_or_fetch_actor_by_url: fn url ->
           case url do
             @mobilizon_group_url -> {:error, "Not found"}

test/graphql/api/search_test.exs

@@ -12,14 +12,15 @@ defmodule Mobilizon.GraphQL.API.SearchTest do
   alias Mobilizon.GraphQL.API.Search
 
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
 
   test "search an user by username" do
-    with_mock ActivityPub,
+    with_mock ActivityPubActor,
       find_or_make_actor_from_nickname: fn "toto@domain.tld" -> {:ok, %Actor{id: 42}} end do
       assert {:ok, %Page{total: 1, elements: [%Actor{id: 42}]}} ==
                Search.search_actors(%{term: "toto@domain.tld"}, 1, 10, :Person)
 
-      assert_called(ActivityPub.find_or_make_actor_from_nickname("toto@domain.tld"))
+      assert_called(ActivityPubActor.find_or_make_actor_from_nickname("toto@domain.tld"))
     end
   end
 

test/mobilizon/actors/actors_test.exs

@@ -13,7 +13,7 @@ defmodule Mobilizon.ActorsTest do
   alias Mobilizon.Service.Workers
   alias Mobilizon.Storage.Page
 
-  alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
 
   alias Mobilizon.Web.Upload.Uploader
 
@@ -106,7 +106,7 @@ defmodule Mobilizon.ActorsTest do
            preferred_username: preferred_username,
            domain: domain,
            avatar: %FileModel{name: picture_name} = _picture
-         } = _actor} = ActivityPub.get_or_fetch_actor_by_url(@remote_account_url)
+         } = _actor} = ActivityPubActor.get_or_fetch_actor_by_url(@remote_account_url)
 
         assert picture_name == "a28c50ce5f2b13fd.jpg"
 
@@ -156,7 +156,8 @@ defmodule Mobilizon.ActorsTest do
 
     test "get_actor_by_name_with_preload!/1 returns the remote actor with its organized events" do
       use_cassette "actors/remote_actor_mastodon_tcit" do
-        with {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(@remote_account_url) do
+        with {:ok, %Actor{} = actor} <-
+               ActivityPubActor.get_or_fetch_actor_by_url(@remote_account_url) do
           assert Actors.get_actor_by_name_with_preload(
                    "#{actor.preferred_username}@#{actor.domain}"
                  ).organized_events == []
@@ -186,7 +187,7 @@ defmodule Mobilizon.ActorsTest do
          %{actor: %Actor{id: actor_id}} do
       use_cassette "actors/remote_actor_mastodon_tcit" do
         with {:ok, %Actor{id: actor2_id}} <-
-               ActivityPub.get_or_fetch_actor_by_url(@remote_account_url) do
+               ActivityPubActor.get_or_fetch_actor_by_url(@remote_account_url) do
           %Page{total: 2, elements: actors} =
             Actors.build_actors_by_username_or_name_page("tcit",
               actor_type: [:Person],