Refactor and test Mobilizon.Federation.ActivityPub.Utils.get_actor/1

Raise exception when object contains no actor. Friendica seems to send an Update activity with no actor

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

lib/federation/activity_pub/utils.ex

@@ -21,6 +21,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
   require Logger
 
   @actor_types ["Group", "Person", "Application"]
+  @all_actor_types @actor_types ++ ["Organization", "Service"]
 
   # Wraps an object into an activity
   @spec create_activity(map(), boolean()) :: {:ok, Activity.t()}
@@ -286,24 +287,51 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
     actor
   end
 
-  def get_actor(%{"actor" => actor}) when is_list(actor) do
-    if is_binary(Enum.at(actor, 0)) do
-      Enum.at(actor, 0)
-    else
-      actor
-      |> Enum.find(fn %{"type" => type} -> type in ["Person", "Service", "Application"] end)
-      |> Map.get("id")
+  def get_actor(%{"actor" => [actor | tail] = actor_list} = object)
+      when is_list(actor_list) and length(actor_list) > 0 do
+    res =
+      try do
+        object
+        |> Map.put("actor", actor)
+        |> get_actor()
+      rescue
+        ArgumentError -> nil
+      end
+
+    case res do
+      id when is_binary(id) ->
+        id
+
+      _ ->
+        object
+        |> Map.put("actor", tail)
+        |> get_actor()
     end
   end
 
-  def get_actor(%{"actor" => %{"id" => id}}) when is_binary(id) do
+  def get_actor(%{"actor" => %{"id" => id, "type" => type}})
+      when is_binary(id) and type in @all_actor_types do
     id
   end
 
-  def get_actor(%{"actor" => nil, "attributedTo" => actor}) when not is_nil(actor) do
+  def get_actor(%{"actor" => _, "attributedTo" => actor}) when not is_nil(actor) do
     get_actor(%{"actor" => actor})
   end
 
+  def get_actor(%{"actor" => %{"id" => id, "type" => type}})
+      when is_binary(id) do
+    raise ArgumentError,
+      message: "Object contains an actor object with invalid type: #{inspect(type)}"
+  end
+
+  def get_actor(%{"actor" => nil, "attributedTo" => nil}) do
+    raise ArgumentError, message: "Object contains both actor and attributedTo fields being null"
+  end
+
+  def get_actor(%{"actor" => _}) do
+    raise ArgumentError, message: "Object contains not actor information"
+  end
+
   @doc """
   Checks that an incoming AP object's actor matches the domain it came from.
 

test/federation/activity_pub/utils_test.exs

@@ -59,4 +59,83 @@ defmodule Mobilizon.Federation.ActivityPub.UtilsTest do
              })
     end
   end
+
+  describe "get_actor/1" do
+    test "with a string" do
+      assert Utils.get_actor(%{"actor" => "https://somewhere.tld/@someone"}) ==
+               "https://somewhere.tld/@someone"
+    end
+
+    test "with an object" do
+      assert Utils.get_actor(%{
+               "actor" => %{"id" => "https://somewhere.tld/@someone", "type" => "Person"}
+             }) ==
+               "https://somewhere.tld/@someone"
+    end
+
+    test "with an invalid object" do
+      assert_raise ArgumentError,
+                   "Object contains an actor object with invalid type: \"Else\"",
+                   fn ->
+                     Utils.get_actor(%{
+                       "actor" => %{"id" => "https://somewhere.tld/@someone", "type" => "Else"}
+                     })
+                   end
+    end
+
+    test "with a list" do
+      assert Utils.get_actor(%{
+               "actor" => ["https://somewhere.tld/@someone", "https://somewhere.else/@other"]
+             }) ==
+               "https://somewhere.tld/@someone"
+    end
+
+    test "with a list of objects" do
+      assert Utils.get_actor(%{
+               "actor" => [
+                 %{"type" => "Person", "id" => "https://somewhere.tld/@someone"},
+                 "https://somewhere.else/@other"
+               ]
+             }) ==
+               "https://somewhere.tld/@someone"
+    end
+
+    test "with a list of objects containing an invalid one" do
+      assert Utils.get_actor(%{
+               "actor" => [
+                 %{"type" => "Else", "id" => "https://somewhere.tld/@someone"},
+                 "https://somewhere.else/@other"
+               ]
+             }) ==
+               "https://somewhere.else/@other"
+    end
+
+    test "with an empty list" do
+      assert_raise ArgumentError,
+                   "Object contains not actor information",
+                   fn ->
+                     Utils.get_actor(%{
+                       "actor" => []
+                     })
+                   end
+    end
+
+    test "fallbacks to attributed_to" do
+      assert Utils.get_actor(%{
+               "actor" => nil,
+               "attributedTo" => "https://somewhere.tld/@someone"
+             }) == "https://somewhere.tld/@someone"
+    end
+
+    test "with no actor information" do
+      assert_raise ArgumentError,
+                   "Object contains both actor and attributedTo fields being null",
+                   fn ->
+                     Utils.get_actor(%{
+                       "actor" => nil,
+                       "attributedTo" => nil
+                     })
+                   end
+    end
+  end
 end