Don't sign fetches to instance actor when refreshing their keys

Closes #963

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

lib/federation/activity_pub/actor.ex

@@ -14,37 +14,38 @@ defmodule Mobilizon.Federation.ActivityPub.Actor do
   @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(url :: String.t(), preload :: boolean()) ::
+  @spec get_or_fetch_actor_by_url(url :: String.t(), options :: Keyword.t()) ::
           {:ok, Actor.t()}
           | {:error, make_actor_errors}
           | {:error, :no_internal_relay_actor}
           | {:error, :url_nil}
-  def get_or_fetch_actor_by_url(url, preload \\ false)
+  def get_or_fetch_actor_by_url(url, options \\ [])
-
-  def get_or_fetch_actor_by_url(nil, _preload), do: {:error, :url_nil}
 
   def get_or_fetch_actor_by_url("https://www.w3.org/ns/activitystreams#Public", _preload) do
     %Actor{url: url} = Relay.get_actor()
     get_or_fetch_actor_by_url(url)
   end
 
-  def get_or_fetch_actor_by_url(url, preload) do
+  def get_or_fetch_actor_by_url(url, options) when is_binary(url) and is_list(options) do
     Logger.debug("Getting or fetching actor by URL #{url}")
+    preload = Keyword.get(options, :preload, false)
 
     case Actors.get_actor_by_url(url, preload) do
       {:ok, %Actor{} = cached_actor} ->
         if Actors.needs_update?(cached_actor) do
-          __MODULE__.make_actor_from_url(url, preload: preload)
+          __MODULE__.make_actor_from_url(url, options)
         else
           {:ok, cached_actor}
         end
 
       {:error, :actor_not_found} ->
         # For tests, see https://github.com/jjh42/mock#not-supported---mocking-internal-function-calls and Mobilizon.Federation.ActivityPubTest
-        __MODULE__.make_actor_from_url(url, preload: preload)
+        __MODULE__.make_actor_from_url(url, options)
     end
   end
 
+  def get_or_fetch_actor_by_url(nil, _preload), do: {:error, :url_nil}
+
   @type make_actor_errors :: Fetcher.fetch_actor_errors() | :actor_is_local
 
   @doc """

lib/federation/activity_pub/transmogrifier.ex

@@ -210,7 +210,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
   def handle_incoming(
         %{"type" => "Follow", "object" => followed, "actor" => follower, "id" => id} = _data
       ) do
-    with {:ok, %Actor{} = followed} <- ActivityPubActor.get_or_fetch_actor_by_url(followed, true),
+    with {:ok, %Actor{} = followed} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(followed, preload: true),
          {:ok, %Actor{} = follower} <- ActivityPubActor.get_or_fetch_actor_by_url(follower),
          {:ok, activity, object} <-
            Actions.Follow.follow(follower, followed, id, false) do

lib/federation/http_signatures/signature.ex

@@ -52,7 +52,8 @@ defmodule Mobilizon.Federation.HTTPSignatures.Signature do
           {:ok, String.t()}
           | {:error, :actor_not_found | :pem_decode_error}
   defp get_public_key_for_url(url) do
-    with {:ok, %Actor{} = actor} <- ActivityPubActor.get_or_fetch_actor_by_url(url) do
+    with {:ok, %Actor{} = actor} <-
+           ActivityPubActor.get_or_fetch_actor_by_url(url, ignore_sign_object_fetches: true) do
       get_actor_public_key(actor)
     end
   end

test/federation/activity_pub/actor_test.exs

@@ -126,7 +126,7 @@ defmodule Mobilizon.Federation.ActivityPub.ActorTest do
            needs_update?: fn _ -> false end
          ]},
         {ActivityPubActor, [:passthrough],
-         make_actor_from_url: fn @actor_url, preload: false ->
+         make_actor_from_url: fn @actor_url, [] ->
            {:ok,
             %Actor{
               preferred_username: "tcit",
@@ -138,7 +138,7 @@ defmodule Mobilizon.Federation.ActivityPub.ActorTest do
                  ActivityPubActor.get_or_fetch_actor_by_url(@actor_url)
 
         assert_called(Actors.needs_update?(:_))
-        refute called(ActivityPubActor.make_actor_from_url(@actor_url, preload: false))
+        refute called(ActivityPubActor.make_actor_from_url(@actor_url, []))
       end
 
       # Fetch doesn't use cache if Actors.needs_update? returns true
@@ -155,7 +155,7 @@ defmodule Mobilizon.Federation.ActivityPub.ActorTest do
            needs_update?: fn _ -> true end
          ]},
         {ActivityPubActor, [:passthrough],
-         make_actor_from_url: fn @actor_url, preload: false ->
+         make_actor_from_url: fn @actor_url, [] ->
            {:ok,
             %Actor{
               preferred_username: "tcit",
@@ -169,7 +169,7 @@ defmodule Mobilizon.Federation.ActivityPub.ActorTest do
         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, preload: false))
+        assert_called(ActivityPubActor.make_actor_from_url(@actor_url, []))
       end
     end
 
@@ -182,7 +182,7 @@ defmodule Mobilizon.Federation.ActivityPub.ActorTest do
 
       assert match?(
                {:error, :actor_deleted},
-               ActivityPubActor.make_actor_from_url(@actor_url, preload: false)
+               ActivityPubActor.make_actor_from_url(@actor_url, [])
              )
     end