Improve Member federation

Federate the member ID instead of the person ID

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

lib/federation/activity_pub/activity_pub.ex

@@ -480,7 +480,7 @@ defmodule Mobilizon.Federation.ActivityPub do
            "to" => [group_members_url],
            "type" => "Remove",
            "actor" => moderator_url,
-           "object" => member.actor.url,
+           "object" => member.url,
            "origin" => group_url
          },
          {:ok, activity} <- create_activity(remove_data, local),

lib/federation/activity_pub/transmogrifier.ex

@@ -686,8 +686,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
 
     with {:ok, %Actor{id: moderator_id} = moderator} <-
            data |> Utils.get_actor() |> ActivityPub.get_or_fetch_actor_by_url(),
-         {:ok, %Actor{id: person_id}} <-
-           object |> Utils.get_url() |> ActivityPub.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(),
          {:is_admin, {:ok, %Member{role: role}}}
@@ -1084,4 +1083,16 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
         err
     end
   end
+
+  # Before 1.0.4 the object of a "Remove" activity was an actor's URL
+  # instead of the member's URL.
+  # TODO: Remove in 1.2
+  @spec get_remove_object(map() | String.t()) :: {:ok, String.t() | integer()}
+  defp get_remove_object(object) do
+    case object |> Utils.get_url() |> ActivityPub.fetch_object_from_url() do
+      {:ok, %Member{actor: %Actor{id: person_id}}} -> {:ok, person_id}
+      {:ok, %Actor{id: person_id}} -> {:ok, person_id}
+      _ -> {:error, :remove_object_not_found}
+    end
+  end
 end

test/federation/activity_pub/activity_pub_test.exs

@@ -17,7 +17,7 @@ defmodule Mobilizon.Federation.ActivityPubTest do
   alias Mobilizon.Todos.{Todo, TodoList}
 
   alias Mobilizon.Federation.ActivityPub
-  alias Mobilizon.Federation.ActivityPub.Utils
+  alias Mobilizon.Federation.ActivityPub.{Relay, Utils}
   alias Mobilizon.Federation.HTTPSignatures.Signature
   alias Mobilizon.Service.HTTP.ActivityPub.Mock
 
@@ -125,6 +125,11 @@ defmodule Mobilizon.Federation.ActivityPubTest do
         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
@@ -301,6 +306,19 @@ defmodule Mobilizon.Federation.ActivityPubTest do
     end
   end
 
+  describe "remove a member" do
+    test "it creates an remove activity" do
+      group = insert(:group)
+      member = insert(:member, parent: group)
+      moderator = insert(:actor)
+      {:ok, activity, _member} = ActivityPub.remove(member, group, moderator, true)
+      assert activity.data["type"] == "Remove"
+      assert activity.data["actor"] == moderator.url
+      assert activity.data["to"] == [group.members_url]
+      assert activity.data["object"] == member.url
+    end
+  end
+
   describe "create a todo list" do
     @todo_list_title "My TODO-list"