Improve member adding and excluding flow

Allow to exclude a member

Send emails to the member when it's excluded

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

lib/federation/activity_pub/activity_pub.ex

@@ -47,7 +47,7 @@ defmodule Mobilizon.Federation.ActivityPub do
   alias Mobilizon.Storage.Page
 
   alias Mobilizon.Web.Endpoint
-  alias Mobilizon.Web.Email.{Admin, Mailer}
+  alias Mobilizon.Web.Email.{Admin, Group, Mailer}
 
   require Logger
 
@@ -225,7 +225,8 @@ defmodule Mobilizon.Federation.ActivityPub do
       end
 
     with {:ok, activity} <- create_activity(update_data, local),
-         :ok <- maybe_federate(activity) do
+         :ok <- maybe_federate(activity),
+         :ok <- maybe_relay_if_group_activity(activity) do
       {:ok, activity, entity}
     else
       err ->
@@ -240,10 +241,12 @@ defmodule Mobilizon.Federation.ActivityPub do
       case type do
         :join -> reject_join(entity, additional)
         :follow -> reject_follow(entity, additional)
+        :invite -> reject_invite(entity, additional)
       end
 
     with {:ok, activity} <- create_activity(update_data, local),
-         :ok <- maybe_federate(activity) do
+         :ok <- maybe_federate(activity),
+         :ok <- maybe_relay_if_group_activity(activity) do
       {:ok, activity, entity}
     else
       err ->
@@ -376,8 +379,9 @@ defmodule Mobilizon.Federation.ActivityPub do
 
   def leave(object, actor, local \\ true, additional \\ %{})
 
-  # TODO: If we want to use this for exclusion we need to have an extra field
-  # for the actor that excluded the participant
+  @doc """
+  Leave an event
+  """
   def leave(
         %Event{id: event_id, url: event_url} = _event,
         %Actor{id: actor_id, url: actor_url} = _actor,
@@ -409,10 +413,63 @@ defmodule Mobilizon.Federation.ActivityPub do
     end
   end
 
+  @doc """
+  Leave a group
+  """
+  def leave(
+        %Actor{type: :Group, id: group_id, url: group_url, members_url: group_members_url},
+        %Actor{id: actor_id, url: actor_url},
+        local,
+        _additional
+      ) do
+    with {:member, {:ok, %Member{id: member_id} = member}} <-
+           {:member, Actors.get_member(actor_id, group_id)},
+         {:is_only_admin, false} <-
+           {:is_only_admin, Actors.is_only_administrator?(member_id, group_id)},
+         {:delete, {:ok, %Member{} = member}} <- {:delete, Actors.delete_member(member)},
+         leave_data <- %{
+           "to" => [group_members_url],
+           "cc" => [group_url],
+           "attributedTo" => group_url,
+           "type" => "Leave",
+           "actor" => actor_url,
+           "object" => group_url
+         },
+         {:ok, activity} <- create_activity(leave_data, local),
+         :ok <- maybe_federate(activity),
+         :ok <- maybe_relay_if_group_activity(activity) do
+      {:ok, activity, member}
+    end
+  end
+
+  def remove(
+        %Member{} = member,
+        %Actor{type: :Group, url: group_url, members_url: group_members_url},
+        %Actor{url: moderator_url},
+        local,
+        _additional \\ %{}
+      ) do
+    with {:ok, %Member{id: member_id}} <- Actors.update_member(member, %{role: :rejected}),
+         %Member{} = member <- Actors.get_member(member_id),
+         :ok <- Group.send_notification_to_removed_member(member),
+         remove_data <- %{
+           "to" => [group_members_url],
+           "type" => "Remove",
+           "actor" => moderator_url,
+           "object" => member.actor.url,
+           "origin" => group_url
+         },
+         {:ok, activity} <- create_activity(remove_data, local),
+         :ok <- maybe_federate(activity),
+         :ok <- maybe_relay_if_group_activity(activity) do
+      {:ok, activity, member}
+    end
+  end
+
   @spec invite(Actor.t(), Actor.t(), Actor.t(), boolean, map()) ::
           {:ok, map(), Member.t()} | {:error, :member_not_found}
   def invite(
-        %Actor{url: group_url, id: group_id} = group,
+        %Actor{url: group_url, id: group_id, members_url: members_url} = group,
         %Actor{url: actor_url, id: actor_id} = actor,
         %Actor{url: target_actor_url, id: target_actor_id} = _target_actor,
         local \\ true,
@@ -431,6 +488,7 @@ defmodule Mobilizon.Federation.ActivityPub do
            }),
          invite_data <- %{
            "type" => "Invite",
+           "attributedTo" => group_url,
            "actor" => actor_url,
            "object" => group_url,
            "target" => target_actor_url,
@@ -439,11 +497,12 @@ defmodule Mobilizon.Federation.ActivityPub do
          {:ok, activity} <-
            create_activity(
              invite_data
-             |> Map.merge(%{"to" => [target_actor_url], "cc" => [group_url]})
+             |> Map.merge(%{"to" => [target_actor_url, members_url], "cc" => [group_url]})
              |> Map.merge(additional),
              local
            ),
-         :ok <- maybe_federate(activity) do
+         :ok <- maybe_federate(activity),
+         :ok <- maybe_relay_if_group_activity(activity) do
       {:ok, activity, member}
     end
   end
@@ -806,9 +865,10 @@ defmodule Mobilizon.Federation.ActivityPub do
            Actors.update_member(member, %{role: :member}),
          accept_data <- %{
            "type" => "Accept",
-           "actor" => actor_url,
-           "to" => [inviter.url],
+           "attributedTo" => member.parent.url,
+           "to" => [inviter.url, member.parent.members_url],
            "cc" => [member.parent.url],
+           "actor" => actor_url,
            "object" => member_url,
            "id" => "#{Endpoint.url()}/accept/invite/member/#{member_id}"
          } do
@@ -873,4 +933,26 @@ defmodule Mobilizon.Federation.ActivityPub do
         err
     end
   end
+
+  @spec reject_invite(Member.t(), map()) :: {:ok, Member.t(), Activity.t()} | any
+  defp reject_invite(
+         %Member{invited_by_id: invited_by_id, actor_id: actor_id} = member,
+         _additional
+       ) do
+    with %Actor{} = inviter <- Actors.get_actor(invited_by_id),
+         %Actor{url: actor_url} <- Actors.get_actor(actor_id),
+         {:ok, %Member{url: member_url, id: member_id} = member} <-
+           Actors.delete_member(member),
+         accept_data <- %{
+           "type" => "Reject",
+           "actor" => actor_url,
+           "attributedTo" => member.parent.url,
+           "to" => [inviter.url, member.parent.members_url],
+           "cc" => [member.parent.url],
+           "object" => member_url,
+           "id" => "#{Endpoint.url()}/reject/invite/member/#{member_id}"
+         } do
+      {:ok, member, accept_data}
+    end
+  end
 end

lib/federation/activity_pub/fetcher.ex

@@ -40,7 +40,8 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
            "type" => "Create",
            "to" => data["to"],
            "cc" => data["cc"],
-           "actor" => data["attributedTo"] || data["actor"],
+           "actor" => data["actor"] || data["attributedTo"],
+           "attributedTo" => data["attributedTo"] || data["actor"],
            "object" => data
          } do
       Transmogrifier.handle_incoming(params)
@@ -61,7 +62,8 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
            "type" => "Update",
            "to" => data["to"],
            "cc" => data["cc"],
-           "actor" => data["attributedTo"] || data["actor"],
+           "actor" => data["actor"] || data["attributedTo"],
+           "attributedTo" => data["attributedTo"] || data["actor"],
            "object" => data
          } do
       Transmogrifier.handle_incoming(params)

lib/federation/activity_pub/preloader.ex

@@ -17,7 +17,8 @@ defmodule Mobilizon.Federation.ActivityPub.Preloader do
   def maybe_preload(%Comment{url: url}),
     do: {:ok, Discussions.get_comment_from_url_with_preload!(url)}
 
-  def maybe_preload(%Discussion{} = discussion), do: {:ok, discussion}
+  def maybe_preload(%Discussion{id: discussion_id}),
+    do: {:ok, Discussions.get_discussion(discussion_id)}
 
   def maybe_preload(%Resource{url: url}),
     do: {:ok, Resources.get_resource_by_url_with_preloads(url)}

lib/federation/activity_pub/transmogrifier.ex

@@ -21,6 +21,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
   alias Mobilizon.Federation.ActivityPub.Types.Ownable
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Tombstone
+  alias Mobilizon.Web.Endpoint
   alias Mobilizon.Web.Email.{Group, Participation}
 
   require Logger
@@ -313,7 +314,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
          {:object_not_found, {:ok, activity, object}} <-
            {:object_not_found,
             do_handle_incoming_reject_following(rejected_object, actor) ||
-              do_handle_incoming_reject_join(rejected_object, actor)} do
+              do_handle_incoming_reject_join(rejected_object, actor) ||
+              do_handle_incoming_reject_invite(rejected_object, actor)} do
       {:ok, activity, object}
     else
       {:object_not_found, nil} ->
@@ -341,8 +343,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
          {:ok, %Actor{id: actor_id, suspended: false} = actor} <-
            ActivityPub.get_or_fetch_actor_by_url(actor_url),
          :ok <- Logger.debug("Fetching contained object"),
-         {:ok, entity} <-
-           object |> Utils.get_url() |> fetch_object_optionnally_authenticated(actor),
+         {:ok, entity} <- process_announce_data(object, actor),
          :ok <- eventually_create_share(object, entity, actor_id) do
       {:ok, nil, entity}
     else
@@ -396,6 +397,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
         %{"type" => "Update", "object" => %{"type" => "Note"} = object, "actor" => _actor} =
           update_data
       ) do
+    Logger.info("Handle incoming to update a note")
+
     with actor <- Utils.get_actor(update_data),
          {:ok, %Actor{url: actor_url, suspended: false}} <-
            ActivityPub.get_or_fetch_actor_by_url(actor),
@@ -520,9 +523,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(
-        %{"type" => "Leave", "object" => object, "actor" => actor, "id" => _id} = data
-      ) 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),
          object <- Utils.get_url(object),
@@ -565,6 +566,39 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
     end
   end
 
+  def handle_incoming(
+        %{"type" => "Remove", "actor" => actor, "object" => object, "origin" => origin} = data
+      ) 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(),
+         {:ok, %Actor{id: person_id}} <-
+           object |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url(),
+         {: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}}}
+         when role in [:moderator, :administrator, :creator] <-
+           {:is_admin, Actors.get_member(moderator_id, group_id)},
+         {:is_member, {:ok, %Member{role: role} = member}} when role != :rejected <-
+           {:is_member, Actors.get_member(person_id, group_id)} do
+      ActivityPub.remove(member, group, moderator, false)
+    else
+      {:is_admin, {:ok, %Member{}}} ->
+        Logger.warn(
+          "Person #{inspect(actor)} is not an admin from #{inspect(origin)} and can't remove member #{
+            inspect(object)
+          }"
+        )
+
+        {:error, "Member already removed"}
+
+      {:is_member, {:ok, %Member{role: :rejected}}} ->
+        Logger.warn("Member #{inspect(object)} already removed from #{inspect(origin)}")
+        {:error, "Member already removed"}
+    end
+  end
+
   #
   #  # TODO
   #  # Accept
@@ -761,6 +795,16 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
     end
   end
 
+  defp do_handle_incoming_reject_invite(invite_object, %Actor{} = actor_rejecting) do
+    with {:invite, {:ok, %Member{role: :invited, actor_id: actor_id} = member}} <-
+           {:invite, get_member(invite_object)},
+         {:same_actor, true} <- {:same_actor, actor_rejecting.id === actor_id},
+         {:ok, activity, member} <-
+           ActivityPub.reject(:invite, member, false) do
+      {:ok, activity, member}
+    end
+  end
+
   # If the object has been announced by a group let's use one of our members to fetch it
   @spec fetch_object_optionnally_authenticated(String.t(), Actor.t() | any()) ::
           {:ok, struct()} | {:error, any()}
@@ -787,17 +831,24 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
     :ok
   end
 
+  # Comment initiates a whole discussion only if it has full title
   @spec is_data_for_comment_or_discussion?(map()) :: boolean()
   defp is_data_for_comment_or_discussion?(object_data) do
-    (not Map.has_key?(object_data, :title) or
-       is_nil(object_data.title) or object_data.title == "") and
+    is_data_a_discussion_initialization?(object_data) and
       is_nil(object_data.discussion_id)
   end
 
+  # Comment initiates a whole discussion only if it has full title
+  @spec is_data_for_comment_or_discussion?(map()) :: boolean()
+  defp is_data_a_discussion_initialization?(object_data) do
+    not Map.has_key?(object_data, :title) or
+      is_nil(object_data.title) or object_data.title == ""
+  end
+
   # Comment and conversations have different attributes for actor and groups
   defp transform_object_data_for_discussion(object_data) do
     # Basic comment
-    if is_data_for_comment_or_discussion?(object_data) do
+    if is_data_a_discussion_initialization?(object_data) do
       object_data
     else
       # Conversation
@@ -880,4 +931,16 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
       {:ok, Convertible.model_to_as(object)}
     end
   end
+
+  # Otherwise we need to fetch what's at the URL (this is possible only for objects, not activities)
+  defp process_announce_data(%{"id" => url}, %Actor{} = actor),
+    do: process_announce_data(url, actor)
+
+  defp process_announce_data(url, %Actor{} = actor) do
+    if Utils.are_same_origin?(url, Endpoint.url()) do
+      ActivityPub.fetch_object_from_url(url, force: false)
+    else
+      fetch_object_optionnally_authenticated(url, actor)
+    end
+  end
 end

lib/federation/activity_pub/types/comments.ex

@@ -36,7 +36,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Comments do
   @impl Entity
   @spec update(Comment.t(), map(), map()) :: {:ok, Comment.t(), Activity.t()} | any()
   def update(%Comment{} = old_comment, args, additional) do
-    with args <- prepare_args_for_comment(args),
+    with args <- prepare_args_for_comment_update(args),
          {:ok, %Comment{} = new_comment} <- Discussions.update_comment(old_comment, args),
          {:ok, true} <- Cachex.del(:activity_pub, "comment_#{new_comment.uuid}"),
          comment_as_data <- Convertible.model_to_as(new_comment),
@@ -125,6 +125,20 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Comments do
     end
   end
 
+  defp prepare_args_for_comment_update(args) do
+    with {text, mentions, tags} <-
+           APIUtils.make_content_html(
+             args |> Map.get(:text, "") |> String.trim(),
+             # Can't put additional tags on a comment
+             [],
+             "text/html"
+           ),
+         tags <- ConverterUtils.fetch_tags(tags),
+         mentions <- Map.get(args, :mentions, []) ++ ConverterUtils.fetch_mentions(mentions) do
+      Map.merge(args, %{text: text, mentions: mentions, tags: tags})
+    end
+  end
+
   @spec handle_event_for_comment(String.t() | integer() | nil) :: Event.t() | nil
   defp handle_event_for_comment(event_id) when not is_nil(event_id) do
     case Events.get_event_with_preload(event_id) do

lib/federation/activity_pub/utils.ex

@@ -145,7 +145,10 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
 
   def maybe_relay_if_group_activity(_, _), do: :ok
 
-  defp do_maybe_relay_if_group_activity(object, attributed_to) when not is_nil(attributed_to) do
+  defp do_maybe_relay_if_group_activity(object, attributed_to) when is_list(attributed_to),
+    do: do_maybe_relay_if_group_activity(object, hd(attributed_to))
+
+  defp do_maybe_relay_if_group_activity(object, attributed_to) when is_binary(attributed_to) do
     id = "#{Endpoint.url()}/announces/#{Ecto.UUID.generate()}"
 
     case Actors.get_local_group_by_url(attributed_to) do
@@ -358,15 +361,14 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
 
   def make_announce_data(
         %Actor{} = actor,
-        %{"id" => url, "type" => type, "actor" => object_actor_url} = _object,
+        %{"actor" => object_actor_url} = object,
         activity_id,
         public
-      )
-      when type in ["Note", "Event", "ResourceCollection", "Document", "Todo"] do
+      ) do
     do_make_announce_data(
       actor,
       object_actor_url,
-      url,
+      object,
       activity_id,
       public
     )
@@ -375,7 +377,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
   defp do_make_announce_data(
          %Actor{type: actor_type} = actor,
          object_actor_url,
-         object_url,
+         object,
          activity_id,
          public
        ) do
@@ -394,7 +396,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
     data = %{
       "type" => "Announce",
       "actor" => actor.url,
-      "object" => object_url,
+      "object" => object,
       "to" => to,
       "cc" => cc
     }

lib/federation/activity_stream/converter/comment.ex

@@ -68,10 +68,18 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Comment do
         tags: tags,
         mentions: mentions,
         local: is_nil(actor_domain),
-        visibility: if(Visibility.is_public?(object), do: :public, else: :private)
+        visibility: if(Visibility.is_public?(object), do: :public, else: :private),
+        published_at: object["published"]
       }
 
-      maybe_fetch_parent_object(object, data)
+      Logger.debug("Converted object before fetching parents")
+      Logger.debug(inspect(data))
+
+      data = maybe_fetch_parent_object(object, data)
+
+      Logger.debug("Converted object after fetching parents")
+      Logger.debug(inspect(data))
+      data
     else
       {:ok, %Actor{suspended: true}} ->
         :error
@@ -98,7 +106,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Comment do
           comment.actor.url,
       "uuid" => comment.uuid,
       "id" => comment.url,
-      "tag" => build_mentions(comment.mentions) ++ build_tags(comment.tags)
+      "tag" => build_mentions(comment.mentions) ++ build_tags(comment.tags),
+      "published" => comment.published_at |> DateTime.to_iso8601()
     }
 
     object =

lib/federation/activity_stream/converter/post.ex

@@ -35,7 +35,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
       "name" => post.title,
       "content" => post.body,
       "attributedTo" => creator_url,
-      "published" => post.publish_at || post.inserted_at
+      "published" => (post.publish_at || post.inserted_at) |> DateTime.to_iso8601()
     }
   end
 

lib/federation/activity_stream/converter/resource.ex

@@ -36,7 +36,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Resource do
       "name" => resource.title,
       "summary" => resource.summary,
       "context" => get_context(resource),
-      "attributedTo" => actor_url
+      "attributedTo" => actor_url,
+      "published" => resource.published_at |> DateTime.to_iso8601()
     }
 
     case type do
@@ -65,7 +66,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Resource do
         url: object["id"],
         actor_id: actor_id,
         creator_id: creator_id,
-        parent_id: parent_id
+        parent_id: parent_id,
+        published_at: object["published"]
       }
 
       case type do

lib/federation/activity_stream/converter/todo.ex

@@ -37,7 +37,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Todo do
       "id" => todo.url,
       "name" => todo.title,
       "status" => todo.status,
-      "todoList" => todo_list_url
+      "todoList" => todo_list_url,
+      "published" => todo.published_at |> DateTime.to_iso8601()
     }
   end
 
@@ -58,7 +59,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Todo do
         status: object["status"],
         url: object["id"],
         todo_list_id: todo_list_id,
-        creator_id: creator_id
+        creator_id: creator_id,
+        published_at: object["published"]
       }
     else
       {:todo_list, nil} ->

lib/federation/activity_stream/converter/todo_list.ex

@@ -28,7 +28,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.TodoList do
       "type" => "TodoList",
       "actor" => group_url,
       "id" => todo_list.url,
-      "name" => todo_list.title
+      "name" => todo_list.title,
+      "published" => todo_list.published_at |> DateTime.to_iso8601()
     }
   end
 
@@ -43,7 +44,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.TodoList do
         %{
           title: object["name"],
           url: object["id"],
-          actor_id: group_id
+          actor_id: group_id,
+          published_at: object["published"]
         }
 
       _ ->