Update Instance Actor when updating instance settings

Also fix an issue when publishing activities to followers/group members

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

lib/federation/activity_pub/activity_pub.ex

@@ -720,7 +720,7 @@ defmodule Mobilizon.Federation.ActivityPub do
   """
   # credo:disable-for-lines:47
   @spec publish(Actor.t(), Activity.t()) :: :ok
-  def publish(actor, activity) do
+  def publish(actor, %Activity{recipients: recipients} = activity) do
     Logger.debug("Publishing an activity")
     Logger.debug(inspect(activity))
 
@@ -733,24 +733,26 @@ defmodule Mobilizon.Federation.ActivityPub do
       Relay.publish(activity)
     end
 
-    followers =
+    {recipients, followers} =
       if actor.followers_url in activity.recipients do
-        Actors.list_external_followers_for_actor(actor)
+        {Enum.filter(recipients, fn recipient -> recipient != actor.followers_url end),
+         Actors.list_external_followers_for_actor(actor)}
       else
-        []
+        {recipients, []}
       end
 
     # If we want to send to all members of the group, because this server is the one the group is on
-    members =
+    {recipients, members} =
       if is_announce_activity?(activity) and actor.type == :Group and
            actor.members_url in activity.recipients and is_nil(actor.domain) do
-        Actors.list_external_members_for_group(actor)
+        {Enum.filter(recipients, fn recipient -> recipient != actor.members_url end),
+         Actors.list_external_members_for_group(actor)}
       else
-        []
+        {recipients, []}
       end
 
     remote_inboxes =
-      (remote_actors(activity) ++ followers ++ members)
+      (remote_actors(recipients) ++ followers ++ members)
       |> Enum.map(fn follower -> follower.shared_inbox_url || follower.inbox_url end)
       |> Enum.uniq()
 
@@ -1045,7 +1047,7 @@ defmodule Mobilizon.Federation.ActivityPub do
     end
   end
 
-  @spec update_actor(Todo.t(), map, map) :: {:ok, Todo.t(), Activity.t()} | any
+  @spec update_todo(Todo.t(), map, map) :: {:ok, Todo.t(), Activity.t()} | any
   defp update_todo(%Todo{} = old_todo, args, additional) do
     with {:ok, %Todo{todo_list_id: todo_list_id} = todo} <- Todos.update_todo(old_todo, args),
          %TodoList{actor_id: group_id} = todo_list <- Todos.get_todo_list(todo_list_id),

lib/federation/activity_pub/utils.ex

@@ -114,10 +114,9 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
 
   def maybe_federate(_), do: :ok
 
-  def remote_actors(%{data: %{"to" => to} = data}) do
-    to = to ++ (data["cc"] || [])
-
-    to
+  @spec remote_actors(list(String.t())) :: list(Actor.t())
+  def remote_actors(recipients) do
+    recipients
     |> Enum.map(fn url -> ActivityPub.get_or_fetch_actor_by_url(url) end)
     |> Enum.map(fn {status, actor} ->
       case status do

lib/federation/activity_stream/converter/actor.ex

@@ -21,12 +21,14 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Actor do
     defdelegate model_to_as(actor), to: ActorConverter
   end
 
+  @allowed_types ["Application", "Group", "Organization", "Person", "Service"]
+
   @doc """
   Converts an AP object data to our internal data structure.
   """
   @impl Converter
   @spec as_to_model_data(map()) :: {:ok, map()}
-  def as_to_model_data(data) do
+  def as_to_model_data(%{"type" => type} = data) when type in @allowed_types do
     avatar =
       data["icon"]["url"] &&
         %{
@@ -62,6 +64,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Actor do
     }
   end
 
+  def as_to_model_data(_), do: :error
+
   @doc """
   Convert an actor struct to an ActivityStream representation.
   """

lib/graphql/resolvers/admin.ex

@@ -11,6 +11,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
   alias Mobilizon.Config
   alias Mobilizon.Conversations.Comment
   alias Mobilizon.Events.Event
+  alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.Relay
   alias Mobilizon.Reports.{Note, Report}
   alias Mobilizon.Service.Statistics
@@ -158,21 +159,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
         context: %{current_user: %User{role: role}}
       })
       when is_admin(role) do
-    {:ok,
-     %{
-       instance_description: Config.instance_description(),
-       instance_long_description: Config.instance_long_description(),
-       instance_name: Config.instance_name(),
-       registrations_open: Config.instance_registrations_open?(),
-       contact: Config.contact(),
-       instance_terms: Config.instance_terms(),
-       instance_terms_type: Config.instance_terms_type(),
-       instance_terms_url: Config.instance_terms_url(),
-       instance_privacy_policy: Config.instance_privacy(),
-       instance_privacy_policy_type: Config.instance_privacy_type(),
-       instance_privacy_policy_url: Config.instance_privacy_url(),
-       instance_rules: Config.instance_rules()
-     }}
+    {:ok, Config.admin_settings()}
   end
 
   def get_settings(_parent, _args, _resolution) do
@@ -183,8 +170,8 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
         context: %{current_user: %User{role: role}}
       })
       when is_admin(role) do
-    with {:ok, res} <- Admin.save_settings("instance", args) do
-      res =
+    with {:ok, res} <- Admin.save_settings("instance", args),
+         res <-
            res
            |> Enum.map(fn {key, %Setting{value: value}} ->
              case value do
@@ -193,9 +180,10 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
                value -> {key, value}
              end
            end)
-        |> Enum.into(%{})
-
+           |> Enum.into(%{}),
+         :ok <- eventually_update_instance_actor(res) do
       Config.clear_config_cache()
+      Cachex.put(:config, :admin_config, res)
 
       {:ok, res}
     end
@@ -284,4 +272,39 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
         {:error, err}
     end
   end
+
+  @spec eventually_update_instance_actor(map()) :: :ok
+  defp eventually_update_instance_actor(admin_setting_args) do
+    args = %{}
+    new_instance_description = Map.get(admin_setting_args, :instance_description)
+    new_instance_name = Map.get(admin_setting_args, :instance_name)
+
+    %{
+      instance_description: old_instance_description,
+      instance_name: old_instance_name
+    } = Config.admin_settings()
+
+    args =
+      if not is_nil(new_instance_description) &&
+           new_instance_description != old_instance_description,
+         do: Map.put(args, :summary, new_instance_description),
+         else: args
+
+    args =
+      if not is_nil(new_instance_name) && new_instance_name != old_instance_name,
+        do: Map.put(args, :name, new_instance_name),
+        else: args
+
+    with {:changes, true} <- {:changes, args != %{}},
+         %Actor{} = instance_actor <- Relay.get_actor(),
+         {:ok, _activity, _actor} <- ActivityPub.update(:actor, instance_actor, args, true) do
+      :ok
+    else
+      {:changes, false} ->
+        :ok
+
+      err ->
+        err
+    end
+  end
 end

lib/mobilizon/admin/admin.ex

@@ -99,6 +99,10 @@ defmodule Mobilizon.Admin do
     |> Repo.transaction()
   end
 
+  def clear_settings(group) do
+    Setting |> where([s], s.group == ^group) |> Repo.delete_all()
+  end
+
   defp do_save_setting(transaction, _group, args) when args == %{}, do: transaction
 
   defp do_save_setting(transaction, group, args) do

lib/mobilizon/config.ex

@@ -231,6 +231,7 @@ defmodule Mobilizon.Config do
 
   def anonymous_actor_id, do: get_cached_value(:anonymous_actor_id)
   def relay_actor_id, do: get_cached_value(:relay_actor_id)
+  def admin_settings, do: get_cached_value(:admin_config)
 
   @spec get(module | atom) :: any
   def get(key), do: get(key, nil)
@@ -300,6 +301,24 @@ defmodule Mobilizon.Config do
     end
   end
 
+  @spec create_cache(atom()) :: map()
+  defp create_cache(:admin_config) do
+    %{
+      instance_description: instance_description(),
+      instance_long_description: instance_long_description(),
+      instance_name: instance_name(),
+      registrations_open: instance_registrations_open?(),
+      contact: contact(),
+      instance_terms: instance_terms(),
+      instance_terms_type: instance_terms_type(),
+      instance_terms_url: instance_terms_url(),
+      instance_privacy_policy: instance_privacy(),
+      instance_privacy_policy_type: instance_privacy_type(),
+      instance_privacy_policy_url: instance_privacy_url(),
+      instance_rules: instance_rules()
+    }
+  end
+
   def clear_config_cache do
     Cachex.clear(:config)
   end

test/graphql/resolvers/admin_test.exs

@@ -217,4 +217,167 @@ defmodule Mobilizon.GraphQL.Resolvers.AdminTest do
              }
     end
   end
+
+  @admin_settings_fragment """
+  fragment adminSettingsFragment on AdminSettings {
+    instanceName
+    instanceDescription
+    instanceLongDescription
+    contact
+    instanceTerms
+    instanceTermsType
+    instanceTermsUrl
+    instancePrivacyPolicy
+    instancePrivacyPolicyType
+    instancePrivacyPolicyUrl
+    instanceRules
+    registrationsOpen
+  }
+  """
+
+  describe "Resolver: Get the instance admin settings" do
+    @admin_settings_query """
+    query {
+      adminSettings {
+        ...adminSettingsFragment
+      }
+    }
+    #{@admin_settings_fragment}
+    """
+
+    setup %{conn: conn} do
+      Cachex.clear(:config)
+      [conn: conn]
+    end
+
+    test "from config files", %{conn: conn} do
+      admin = insert(:user, role: :administrator)
+
+      res =
+        conn
+        |> auth_conn(admin)
+        |> AbsintheHelpers.graphql_query(query: @admin_settings_query)
+
+      assert res["data"]["adminSettings"]["instanceName"] ==
+               Application.get_env(:mobilizon, :instance)[:name]
+
+      assert res["data"]["adminSettings"]["registrationsOpen"] ==
+               Application.get_env(:mobilizon, :instance)[:registrations_open]
+    end
+
+    @instance_name "My Awesome Instance"
+    test "from DB", %{conn: conn} do
+      admin = insert(:user, role: :administrator)
+      insert(:admin_setting, group: "instance", name: "instance_name", value: @instance_name)
+      insert(:admin_setting, group: "instance", name: "registrations_open", value: "false")
+
+      res =
+        conn
+        |> auth_conn(admin)
+        |> AbsintheHelpers.graphql_query(query: @admin_settings_query)
+
+      assert res["data"]["adminSettings"]["instanceName"] == @instance_name
+
+      assert res["data"]["adminSettings"]["registrationsOpen"] == false
+    end
+
+    test "unless user isn't admin", %{conn: conn} do
+      admin = insert(:user)
+
+      res =
+        conn
+        |> auth_conn(admin)
+        |> AbsintheHelpers.graphql_query(query: @admin_settings_query)
+
+      assert hd(res["errors"])["message"] ==
+               "You need to be logged-in and an administrator to access admin settings"
+    end
+  end
+
+  describe "Resolver: Update the instance admin settings" do
+    setup %{conn: conn} do
+      Cachex.clear(:config)
+      [conn: conn]
+    end
+
+    @update_instance_admin_settings_mutation """
+    mutation SaveAdminSettings(
+      $instanceName: String
+      $instanceDescription: String
+      $instanceLongDescription: String
+      $contact: String
+      $instanceTerms: String
+      $instanceTermsType: InstanceTermsType
+      $instanceTermsUrl: String
+      $instancePrivacyPolicy: String
+      $instancePrivacyPolicyType: InstancePrivacyType
+      $instancePrivacyPolicyUrl: String
+      $instanceRules: String
+      $registrationsOpen: Boolean
+      ) {
+      saveAdminSettings(
+        instanceName: $instanceName
+        instanceDescription: $instanceDescription
+        instanceLongDescription: $instanceLongDescription
+        contact: $contact
+        instanceTerms: $instanceTerms
+        instanceTermsType: $instanceTermsType
+        instanceTermsUrl: $instanceTermsUrl
+        instancePrivacyPolicy: $instancePrivacyPolicy
+        instancePrivacyPolicyType: $instancePrivacyPolicyType
+        instancePrivacyPolicyUrl: $instancePrivacyPolicyUrl
+        instanceRules: $instanceRules
+        registrationsOpen: $registrationsOpen
+      ) {
+        ...adminSettingsFragment
+      }
+    }
+    #{@admin_settings_fragment}
+    """
+
+    @new_instance_name "new Instance Name"
+
+    test "does the setting update and updates instance actor as well", %{conn: conn} do
+      admin = insert(:user, role: :administrator)
+
+      res =
+        conn
+        |> auth_conn(admin)
+        |> AbsintheHelpers.graphql_query(query: @admin_settings_query)
+
+      assert res["data"]["adminSettings"]["instanceName"] ==
+               Application.get_env(:mobilizon, :instance)[:name]
+
+      assert res["data"]["adminSettings"]["registrationsOpen"] ==
+               Application.get_env(:mobilizon, :instance)[:registrations_open]
+
+      res =
+        conn
+        |> auth_conn(admin)
+        |> AbsintheHelpers.graphql_query(
+          query: @update_instance_admin_settings_mutation,
+          variables: %{"instanceName" => @new_instance_name, "registrationsOpen" => false}
+        )
+
+      assert res["data"]["saveAdminSettings"]["instanceName"] == @new_instance_name
+      assert res["data"]["saveAdminSettings"]["registrationsOpen"] == false
+
+      assert %Actor{name: @new_instance_name} = Relay.get_actor()
+    end
+
+    test "unless user isn't admin", %{conn: conn} do
+      admin = insert(:user)
+
+      res =
+        conn
+        |> auth_conn(admin)
+        |> AbsintheHelpers.graphql_query(
+          query: @update_instance_admin_settings_mutation,
+          variables: %{"instanceName" => @new_instance_name, "registrationsOpen" => false}
+        )
+
+      assert hd(res["errors"])["message"] ==
+               "You need to be logged-in and an administrator to save admin settings"
+    end
+  end
 end

test/support/factory.ex

@@ -316,4 +316,12 @@ defmodule Mobilizon.Factory do
       path: "/#{title}"
     }
   end
+
+  def admin_setting_factory do
+    %Mobilizon.Admin.Setting{
+      group: sequence("group"),
+      name: sequence("name"),
+      value: sequence("value")
+    }
+  end
 end