From e0fad9ddd1e266f707ec6ddc70248e6e81e409f4 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 7 Jul 2020 15:51:42 +0200 Subject: [PATCH] Update Instance Actor when updating instance settings Also fix an issue when publishing activities to followers/group members Signed-off-by: Thomas Citharel --- lib/federation/activity_pub/activity_pub.ex | 20 ++- lib/federation/activity_pub/utils.ex | 7 +- .../activity_stream/converter/actor.ex | 6 +- lib/graphql/resolvers/admin.ex | 77 ++++++--- lib/mobilizon/admin/admin.ex | 4 + lib/mobilizon/config.ex | 19 ++ test/graphql/resolvers/admin_test.exs | 163 ++++++++++++++++++ test/support/factory.ex | 8 + 8 files changed, 263 insertions(+), 41 deletions(-) diff --git a/lib/federation/activity_pub/activity_pub.ex b/lib/federation/activity_pub/activity_pub.ex index d6b489d92..a71c9e262 100644 --- a/lib/federation/activity_pub/activity_pub.ex +++ b/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), diff --git a/lib/federation/activity_pub/utils.ex b/lib/federation/activity_pub/utils.ex index 5e969eb9d..7a8d1d860 100644 --- a/lib/federation/activity_pub/utils.ex +++ b/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 diff --git a/lib/federation/activity_stream/converter/actor.ex b/lib/federation/activity_stream/converter/actor.ex index 950d431b4..39fd5dd3c 100644 --- a/lib/federation/activity_stream/converter/actor.ex +++ b/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. """ diff --git a/lib/graphql/resolvers/admin.ex b/lib/graphql/resolvers/admin.ex index 29cd1470f..5ccb9eea5 100644 --- a/lib/graphql/resolvers/admin.ex +++ b/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,19 +170,20 @@ 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 = - res - |> Enum.map(fn {key, %Setting{value: value}} -> - case value do - "true" -> {key, true} - "false" -> {key, false} - value -> {key, value} - end - end) - |> Enum.into(%{}) - + with {:ok, res} <- Admin.save_settings("instance", args), + res <- + res + |> Enum.map(fn {key, %Setting{value: value}} -> + case value do + "true" -> {key, true} + "false" -> {key, false} + value -> {key, value} + end + end) + |> 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 diff --git a/lib/mobilizon/admin/admin.ex b/lib/mobilizon/admin/admin.ex index f16d2c773..07f1854e4 100644 --- a/lib/mobilizon/admin/admin.ex +++ b/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 diff --git a/lib/mobilizon/config.ex b/lib/mobilizon/config.ex index 77e1ffadb..fbe38aeb1 100644 --- a/lib/mobilizon/config.ex +++ b/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 diff --git a/test/graphql/resolvers/admin_test.exs b/test/graphql/resolvers/admin_test.exs index affb40fb1..8b94e16d0 100644 --- a/test/graphql/resolvers/admin_test.exs +++ b/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 diff --git a/test/support/factory.ex b/test/support/factory.ex index 220e7755d..7a9784fe7 100644 --- a/test/support/factory.ex +++ b/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