From 177341b4912f213a67c2ce3a59abd093193437ce Mon Sep 17 00:00:00 2001
From: Thomas Citharel <tcit@tcit.fr>
Date: Thu, 4 Feb 2021 12:28:53 +0100
Subject: [PATCH] Fix events & posts not being sent to group followers

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
---
 lib/federation/activity_pub/activity_pub.ex   |  2 +
 lib/federation/activity_pub/audience.ex       | 76 +++++++++++++------
 lib/federation/activity_pub/types/posts.ex    | 23 ++----
 lib/federation/activity_pub/utils.ex          |  6 +-
 .../activity_stream/converter/event.ex        | 18 +++--
 .../activity_stream/converter/post.ex         | 12 ++-
 6 files changed, 89 insertions(+), 48 deletions(-)

diff --git a/lib/federation/activity_pub/activity_pub.ex b/lib/federation/activity_pub/activity_pub.ex
index cf7a9e54c..7aec827c2 100644
--- a/lib/federation/activity_pub/activity_pub.ex
+++ b/lib/federation/activity_pub/activity_pub.ex
@@ -710,6 +710,8 @@ defmodule Mobilizon.Federation.ActivityPub do
       Relay.publish(activity)
     end
 
+    recipients = Enum.uniq(recipients)
+
     {recipients, followers} = convert_followers_in_recipients(recipients)
 
     {recipients, members} = convert_members_in_recipients(recipients)
diff --git a/lib/federation/activity_pub/audience.ex b/lib/federation/activity_pub/audience.ex
index d374a8abc..1e491cb3a 100644
--- a/lib/federation/activity_pub/audience.ex
+++ b/lib/federation/activity_pub/audience.ex
@@ -7,6 +7,7 @@ defmodule Mobilizon.Federation.ActivityPub.Audience do
   alias Mobilizon.Actors.{Actor, Member}
   alias Mobilizon.Discussions.{Comment, Discussion}
   alias Mobilizon.Events.{Event, Participant}
+  alias Mobilizon.Posts.Post
   alias Mobilizon.Share
   alias Mobilizon.Storage.Repo
 
@@ -64,10 +65,6 @@ defmodule Mobilizon.Federation.ActivityPub.Audience do
     {mentions, []}
   end
 
-  #  def get_addressed_actors(_, to) when is_list(to) do
-  #    Actors.get(to)
-  #  end
-
   def get_addressed_actors(mentioned_users, _), do: mentioned_users
 
   def calculate_to_and_cc_from_mentions(
@@ -79,9 +76,8 @@ defmodule Mobilizon.Federation.ActivityPub.Audience do
   end
 
   def calculate_to_and_cc_from_mentions(%Comment{} = comment) do
-    with mentioned_actors <- Enum.map(comment.mentions, &process_mention/1),
-         addressed_actors <- get_addressed_actors(mentioned_actors, nil),
-         {to, cc} <- get_to_and_cc(comment.actor, addressed_actors, comment.visibility),
+    with {to, cc} <-
+           extract_actors_from_mentions(comment.mentions, comment.actor, comment.visibility),
          {to, cc} <- {Enum.uniq(to ++ add_in_reply_to(comment.in_reply_to_comment)), cc},
          {to, cc} <- {Enum.uniq(to ++ add_event_author(comment.event)), cc},
          {to, cc} <-
@@ -101,32 +97,45 @@ defmodule Mobilizon.Federation.ActivityPub.Audience do
     end
   end
 
-  def calculate_to_and_cc_from_mentions(%Event{
-        attributed_to: %Actor{members_url: members_url},
-        visibility: visibility
-      }) do
+  def calculate_to_and_cc_from_mentions(
+        %Event{
+          attributed_to: %Actor{members_url: members_url},
+          visibility: visibility
+        } = event
+      ) do
+    %{"to" => to, "cc" => cc} = extract_actors_from_event(event)
+
     case visibility do
       :public ->
-        %{"to" => [members_url, @ap_public], "cc" => []}
+        %{"to" => [@ap_public, members_url] ++ to, "cc" => [] ++ cc}
 
       :unlisted ->
-        %{"to" => [members_url], "cc" => [@ap_public]}
+        %{"to" => [members_url] ++ to, "cc" => [@ap_public] ++ cc}
 
       :private ->
+        # Private is restricted to only the members
         %{"to" => [members_url], "cc" => []}
     end
   end
 
   def calculate_to_and_cc_from_mentions(%Event{} = event) do
-    with mentioned_actors <- Enum.map(event.mentions, &process_mention/1),
-         addressed_actors <- get_addressed_actors(mentioned_actors, nil),
-         {to, cc} <- get_to_and_cc(event.organizer_actor, addressed_actors, event.visibility),
-         {to, cc} <-
-           {to,
-            Enum.uniq(
-              cc ++ add_comments_authors(event.comments) ++ add_shares_actors_followers(event.url)
-            )} do
-      %{"to" => to, "cc" => cc}
+    extract_actors_from_event(event)
+  end
+
+  def calculate_to_and_cc_from_mentions(%Post{
+        attributed_to: %Actor{members_url: members_url},
+        visibility: visibility
+      }) do
+    case visibility do
+      :public ->
+        %{"to" => [@ap_public, members_url], "cc" => []}
+
+      :unlisted ->
+        %{"to" => [members_url], "cc" => [@ap_public]}
+
+      :private ->
+        # Private is restricted to only the members
+        %{"to" => [members_url], "cc" => []}
     end
   end
 
@@ -211,4 +220,27 @@ defmodule Mobilizon.Federation.ActivityPub.Audience do
       url
     end
   end
+
+  @spec extract_actors_from_mentions(list(), Actor.t(), atom()) :: {list(), list()}
+  defp extract_actors_from_mentions(mentions, actor, visibility) do
+    with mentioned_actors <- Enum.map(mentions, &process_mention/1),
+         addressed_actors <- get_addressed_actors(mentioned_actors, nil) do
+      get_to_and_cc(actor, addressed_actors, visibility)
+    end
+  end
+
+  defp extract_actors_from_event(%Event{} = event) do
+    with {to, cc} <-
+           extract_actors_from_mentions(event.mentions, event.organizer_actor, event.visibility),
+         {to, cc} <-
+           {to,
+            Enum.uniq(
+              cc ++ add_comments_authors(event.comments) ++ add_shares_actors_followers(event.url)
+            )} do
+      %{"to" => to, "cc" => cc}
+    else
+      _ ->
+        %{"to" => [], "cc" => []}
+    end
+  end
 end
diff --git a/lib/federation/activity_pub/types/posts.ex b/lib/federation/activity_pub/types/posts.ex
index bddfaf475..22b145a66 100644
--- a/lib/federation/activity_pub/types/posts.ex
+++ b/lib/federation/activity_pub/types/posts.ex
@@ -2,6 +2,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Posts do
   @moduledoc false
   alias Mobilizon.{Actors, Posts, Tombstone}
   alias Mobilizon.Actors.Actor
+  alias Mobilizon.Federation.ActivityPub.Audience
   alias Mobilizon.Federation.ActivityPub.Types.Entity
   alias Mobilizon.Federation.ActivityStream.Converter.Utils, as: ConverterUtils
   alias Mobilizon.Federation.ActivityStream.Convertible
@@ -19,15 +20,11 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Posts do
          {:ok, %Post{attributed_to_id: group_id, author_id: creator_id} = post} <-
            Posts.create_post(args),
          {:ok, %Actor{} = group} <- Actors.get_group_by_actor_id(group_id),
-         %Actor{url: creator_url} = creator <- Actors.get_actor(creator_id),
+         %Actor{} = creator <- Actors.get_actor(creator_id),
          post_as_data <-
            Convertible.model_to_as(%{post | attributed_to: group, author: creator}),
-         audience <- %{
-           "to" => [group.members_url],
-           "cc" => [],
-           "actor" => creator_url,
-           "attributedTo" => [creator_url]
-         } do
+         audience <-
+           Audience.calculate_to_and_cc_from_mentions(post) do
       create_data = make_create_data(post_as_data, Map.merge(audience, additional))
 
       {:ok, post, create_data}
@@ -44,16 +41,12 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Posts do
          {:ok, %Post{attributed_to_id: group_id, author_id: creator_id} = post} <-
            Posts.update_post(post, args),
          {:ok, true} <- Cachex.del(:activity_pub, "post_#{post.slug}"),
-         {:ok, %Actor{url: group_url} = group} <- Actors.get_group_by_actor_id(group_id),
-         %Actor{url: creator_url} = creator <- Actors.get_actor(creator_id),
+         {:ok, %Actor{} = group} <- Actors.get_group_by_actor_id(group_id),
+         %Actor{} = creator <- Actors.get_actor(creator_id),
          post_as_data <-
            Convertible.model_to_as(%{post | attributed_to: group, author: creator}),
-         audience <- %{
-           "to" => [group.members_url],
-           "cc" => [],
-           "actor" => creator_url,
-           "attributedTo" => [group_url]
-         } do
+         audience <-
+           Audience.calculate_to_and_cc_from_mentions(post) do
       update_data = make_update_data(post_as_data, Map.merge(audience, additional))
 
       {:ok, post, update_data}
diff --git a/lib/federation/activity_pub/utils.ex b/lib/federation/activity_pub/utils.ex
index 32855aff7..6d39f3034 100644
--- a/lib/federation/activity_pub/utils.ex
+++ b/lib/federation/activity_pub/utils.ex
@@ -127,7 +127,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
   def maybe_relay_if_group_activity(activity, attributed_to \\ nil)
 
   def maybe_relay_if_group_activity(
-        %Activity{local: false, data: %{"object" => object}},
+        %Activity{data: %{"object" => object}},
         _attributed_to
       )
       when is_map(object) do
@@ -136,7 +136,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
 
   # When doing a delete the object is just an AP ID string, so we pass the attributed_to actor as well
   def maybe_relay_if_group_activity(
-        %Activity{local: false, data: %{"object" => object}},
+        %Activity{data: %{"object" => object}},
         %Actor{url: attributed_to_url}
       )
       when is_binary(object) do
@@ -421,7 +421,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
          ["https://www.w3.org/ns/activitystreams#Public"]}
       else
         if actor_type == :Group do
-          {[actor.members_url], []}
+          {[actor.followers_url, actor.members_url], []}
         else
           {[actor.followers_url], []}
         end
diff --git a/lib/federation/activity_stream/converter/event.ex b/lib/federation/activity_stream/converter/event.ex
index b415b4e8a..a45200b5a 100644
--- a/lib/federation/activity_stream/converter/event.ex
+++ b/lib/federation/activity_stream/converter/event.ex
@@ -94,18 +94,13 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do
     to =
       if event.visibility == :public,
         do: ["https://www.w3.org/ns/activitystreams#Public"],
-        else: [event.organizer_actor.followers_url]
+        else: [attributed_to_or_default(event).followers_url]
 
     %{
       "type" => "Event",
       "to" => to,
       "cc" => [],
-      "attributedTo" =>
-        if(is_nil(event.attributed_to) or not Ecto.assoc_loaded?(event.attributed_to),
-          do: nil,
-          else: event.attributed_to.url
-        ) ||
-          event.organizer_actor.url,
+      "attributedTo" => attributed_to_or_default(event).url,
       "name" => event.title,
       "actor" =>
         if(Ecto.assoc_loaded?(event.organizer_actor), do: event.organizer_actor.url, else: nil),
@@ -135,6 +130,15 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do
     |> maybe_add_inline_media(event)
   end
 
+  @spec attributed_to_or_default(Event.t()) :: Actor.t()
+  defp attributed_to_or_default(event) do
+    if(is_nil(event.attributed_to) or not Ecto.assoc_loaded?(event.attributed_to),
+      do: nil,
+      else: event.attributed_to
+    ) ||
+      event.organizer_actor
+  end
+
   # Get only elements that we have in EventOptions
   @spec get_options(map) :: map
   defp get_options(object) do
diff --git a/lib/federation/activity_stream/converter/post.ex b/lib/federation/activity_stream/converter/post.ex
index 7798a3a8c..45c7ad5d9 100644
--- a/lib/federation/activity_stream/converter/post.ex
+++ b/lib/federation/activity_stream/converter/post.ex
@@ -34,10 +34,20 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
   @impl Converter
   @spec model_to_as(Post.t()) :: map
   def model_to_as(
-        %Post{author: %Actor{url: actor_url}, attributed_to: %Actor{url: creator_url}} = post
+        %Post{
+          author: %Actor{url: actor_url},
+          attributed_to: %Actor{url: creator_url, followers_url: followers_url}
+        } = post
       ) do
+    to =
+      if post.visibility == :public,
+        do: ["https://www.w3.org/ns/activitystreams#Public"],
+        else: [followers_url]
+
     %{
       "type" => "Article",
+      "to" => to,
+      "cc" => [],
       "actor" => actor_url,
       "id" => post.url,
       "name" => post.title,