Merge branch 'federation-fixes' into 'main'

fix(activitypub): various federation follow & nodeinfo fixes

See merge request framasoft/mobilizon!1516

lib/federation/activity_pub/types/actors.ex

@@ -301,7 +301,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Actors do
        ) do
     %Actor{id: relay_id} = Relay.get_actor()
 
-    unless follower.target_actor.manually_approves_followers or
+    unless follower.target_actor.manually_approves_followers == true or
              follower.target_actor.id == relay_id do
       require Logger
       Logger.debug("Target doesn't manually approves followers, we can accept right away")

lib/federation/node_info.ex

@@ -5,6 +5,7 @@ defmodule Mobilizon.Federation.NodeInfo do
 
   alias Mobilizon.Service.HTTP.WebfingerClient
   require Logger
+  import Mobilizon.Service.HTTP.Utils, only: [is_content_type?: 2]
 
   @application_uri "https://www.w3.org/ns/activitystreams#Application"
   @nodeinfo_rel_2_0 "http://nodeinfo.diaspora.software/ns/schema/2.0"
@@ -20,7 +21,7 @@ defmodule Mobilizon.Federation.NodeInfo do
       {:ok, body} ->
         extract_application_actor(body)
 
-      {:error, :node_info_meta_http_error} ->
+      {:error, _err} ->
         nil
     end
   end
@@ -31,7 +32,9 @@ defmodule Mobilizon.Federation.NodeInfo do
 
     with {:ok, endpoint} when is_binary(endpoint) <- fetch_nodeinfo_details(host),
          :ok <- Logger.debug("Going to get NodeInfo information from URL #{endpoint}"),
-         {:ok, %{body: body, status: code}} when code in 200..299 <- WebfingerClient.get(endpoint) do
+         {:ok, %{body: body, status: code, headers: headers}} when code in 200..299 <-
+           WebfingerClient.get(endpoint),
+         {:ok, body} <- validate_json_response(body, headers) do
       Logger.debug("Found nodeinfo information for domain #{host}")
       {:ok, body}
     else
@@ -58,8 +61,8 @@ defmodule Mobilizon.Federation.NodeInfo do
     prefix = if @env !== :dev, do: "https", else: "http"
 
     case WebfingerClient.get("#{prefix}://#{host}/.well-known/nodeinfo") do
-      {:ok, %{body: body, status: code}} when code in 200..299 ->
+      {:ok, %{body: body, status: code, headers: headers}} when code in 200..299 ->
-        {:ok, body}
+        validate_json_response(body, headers)
 
       err ->
         Logger.debug("Failed to fetch NodeInfo data #{inspect(err)}")
@@ -102,4 +105,19 @@ defmodule Mobilizon.Federation.NodeInfo do
       rel == relation and is_binary(href)
     end)
   end
+
+  @spec validate_json_response(map() | String.t(), list()) ::
+          {:ok, String.t()} | {:error, :bad_content_type | :body_not_json}
+  defp validate_json_response(body, headers) do
+    cond do
+      !is_content_type?(headers, "application/json") ->
+        {:error, :bad_content_type}
+
+      !is_map(body) ->
+        {:error, :body_not_json}
+
+      true ->
+        {:ok, body}
+    end
+  end
 end

lib/graphql/resolvers/admin.ex

@@ -16,6 +16,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
   alias Mobilizon.Reports.{Note, Report}
   alias Mobilizon.Service.Auth.Authenticator
   alias Mobilizon.Service.Statistics
+  alias Mobilizon.Service.Workers.RefreshInstances
   alias Mobilizon.Storage.Page
   alias Mobilizon.Users.User
   alias Mobilizon.Web.Email
@@ -546,6 +547,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Admin do
     case Relay.follow(domain) do
       {:ok, _activity, _follow} ->
         Instances.refresh()
+        RefreshInstances.refresh_instance_actor(domain)
         get_instance(parent, args, resolution)
 
       {:error, :follow_pending} ->

lib/mobilizon/instances/instances.ex

@@ -22,15 +22,16 @@ defmodule Mobilizon.Instances do
 
     order_by_options = Keyword.new([{direction, order_by}])
 
-    subquery =
+    query =
-      Actor
+      Instance
-      |> where(
+      |> join(:left, [i], ia in InstanceActor, on: i.domain == ia.domain)
-        [a],
+      |> join(:left, [_i, ia], a in Actor, on: ia.actor_id == a.id)
-        a.preferred_username == "relay" and a.type == :Application and not is_nil(a.domain)
+      |> join(:left, [_i, _ia, a], f1 in Follower, on: f1.target_actor_id == a.id)
-      )
+      |> join(:left, [_i, _ia, a], f2 in Follower, on: f2.actor_id == a.id)
-      |> join(:left, [a], f1 in Follower, on: f1.target_actor_id == a.id)
+      |> select([i, ia, a, f1, f2], %{
-      |> join(:left, [a], f2 in Follower, on: f2.actor_id == a.id)
+        instance: i,
-      |> select([a, f1, f2], %{
+        instance_actor: ia,
+        actor: a,
         domain: a.domain,
         has_relay: fragment(@is_null_fragment, a.id),
         following: fragment(@is_null_fragment, f2.id),
@@ -38,13 +39,6 @@ defmodule Mobilizon.Instances do
         follower: fragment(@is_null_fragment, f1.id),
         follower_approved: f1.approved
       })
-
-    query =
-      Instance
-      |> join(:left, [i], s in subquery(subquery), on: i.domain == s.domain)
-      |> join(:left, [i], ia in InstanceActor, on: i.domain == ia.domain)
-      |> join(:left, [_i, _s, ia], a in Actor, on: ia.actor_id == a.id)
-      |> select([i, s, ia, a], {i, s, ia, a})
       |> order_by(^order_by_options)
 
     query =
@@ -93,17 +87,17 @@ defmodule Mobilizon.Instances do
     SQL.query!(Repo, "REFRESH MATERIALIZED VIEW instances")
   end
 
-  defp convert_instance_meta(
+  defp convert_instance_meta(%{
-         {instance,
+         instance: instance,
-          %{
+         instance_actor: instance_meta,
-            domain: _domain,
+         actor: instance_actor,
-            follower: follower,
+         domain: _domain,
-            follower_approved: follower_approved,
+         follower: follower,
-            following: following,
+         follower_approved: follower_approved,
-            following_approved: following_approved,
+         following: following,
-            has_relay: has_relay
+         following_approved: following_approved,
-          }, instance_meta, instance_actor}
+         has_relay: has_relay
-       ) do
+       }) do
     instance
     |> Map.put(:follower_status, follow_status(following, following_approved))
     |> Map.put(:followed_status, follow_status(follower, follower_approved))

lib/service/http/utils.ex

@@ -0,0 +1,35 @@
+defmodule Mobilizon.Service.HTTP.Utils do
+  @moduledoc """
+  Utils for HTTP operations
+  """
+
+  @spec get_header(Enum.t(), String.t()) :: String.t() | nil
+  def get_header(headers, key) do
+    key = String.downcase(key)
+
+    case List.keyfind(headers, key, 0) do
+      {^key, value} -> String.downcase(value)
+      nil -> nil
+    end
+  end
+
+  @spec is_content_type?(Enum.t(), String.t() | list(String.t())) :: boolean
+  def is_content_type?(headers, content_type) do
+    headers
+    |> get_header("Content-Type")
+    |> content_type_header_matches(content_type)
+  end
+
+  @spec content_type_header_matches(String.t() | nil, Enum.t()) :: boolean
+  defp content_type_header_matches(header, content_types)
+  defp content_type_header_matches(nil, _content_types), do: false
+
+  defp content_type_header_matches(header, content_type)
+       when is_binary(header) and is_binary(content_type),
+       do: content_type_header_matches(header, [content_type])
+
+  defp content_type_header_matches(header, content_types)
+       when is_binary(header) and is_list(content_types) do
+    Enum.any?(content_types, fn content_type -> String.starts_with?(header, content_type) end)
+  end
+end

lib/service/rich_media/parser.ex

@@ -22,6 +22,7 @@ defmodule Mobilizon.Service.RichMedia.Parser do
   alias Mobilizon.Service.RichMedia.Parsers.Fallback
   alias Plug.Conn.Utils
   require Logger
+  import Mobilizon.Service.HTTP.Utils
 
   defp parsers do
     Mobilizon.Config.get([:rich_media, :parsers])
@@ -74,7 +75,7 @@ defmodule Mobilizon.Service.RichMedia.Parser do
                 opts: @options
               )},
            {:is_html, _response_headers, true} <-
-             {:is_html, response_headers, is_html(response_headers)} do
+             {:is_html, response_headers, is_html?(response_headers)} do
         body
         |> convert_utf8(response_headers)
         |> maybe_parse()
@@ -107,43 +108,21 @@ defmodule Mobilizon.Service.RichMedia.Parser do
   defp get_data_for_media(response_headers, url) do
     data = %{title: get_filename_from_headers(response_headers) || get_filename_from_url(url)}
 
-    if is_image(response_headers) do
+    if is_image?(response_headers) do
       Map.put(data, :image_remote_url, url)
     else
       data
     end
   end
 
-  @spec is_html(Enum.t()) :: boolean
+  @spec is_html?(Enum.t()) :: boolean
-  def is_html(headers) do
+  defp is_html?(headers) do
-    headers
+    is_content_type?(headers, ["text/html", "application/xhtml"])
-    |> get_header("Content-Type")
-    |> content_type_header_matches(["text/html", "application/xhtml"])
   end
 
-  @spec is_image(Enum.t()) :: boolean
+  @spec is_image?(Enum.t()) :: boolean
-  defp is_image(headers) do
+  defp is_image?(headers) do
-    headers
+    is_content_type?(headers, ["image/"])
-    |> get_header("Content-Type")
-    |> content_type_header_matches(["image/"])
-  end
-
-  @spec content_type_header_matches(String.t() | nil, Enum.t()) :: boolean
-  defp content_type_header_matches(header, content_types)
-  defp content_type_header_matches(nil, _content_types), do: false
-
-  defp content_type_header_matches(header, content_types) when is_binary(header) do
-    Enum.any?(content_types, fn content_type -> String.starts_with?(header, content_type) end)
-  end
-
-  @spec get_header(Enum.t(), String.t()) :: String.t() | nil
-  defp get_header(headers, key) do
-    key = String.downcase(key)
-
-    case List.keyfind(headers, key, 0) do
-      {^key, value} -> String.downcase(value)
-      nil -> nil
-    end
   end
 
   @spec get_filename_from_headers(Enum.t()) :: String.t() | nil

lib/service/workers/refresh_instances.ex

@@ -20,21 +20,16 @@ defmodule Mobilizon.Service.Workers.RefreshInstances do
     Instances.refresh()
 
     Instances.all_domains()
-    |> Enum.each(&refresh_instance_actor/1)
+    |> Enum.each(fn %Instance{domain: domain} -> refresh_instance_actor(domain) end)
   end
 
-  @spec refresh_instance_actor(Instance.t()) ::
+  @spec refresh_instance_actor(String.t() | nil) ::
-          {:ok, Mobilizon.Actors.Actor.t()}
+          {:ok, Mobilizon.Actors.Actor.t()} | {:error, Ecto.Changeset.t()} | {:error, atom}
-          | {:error,
+  def refresh_instance_actor(nil) do
-             ActivityPubActor.make_actor_errors()
-             | Mobilizon.Federation.WebFinger.finger_errors()}
-  def refresh_instance_actor(%Instance{domain: nil}) do
     {:error, :not_remote_instance}
   end
 
-  @spec refresh_instance_actor(Instance.t()) ::
+  def refresh_instance_actor(domain) do
-          {:ok, InstanceActor.t()} | {:error, Ecto.Changeset.t()} | {:error, atom}
-  def refresh_instance_actor(%Instance{domain: domain}) do
     %Actor{url: url} = Relay.get_actor()
     %URI{host: host} = URI.new!(url)
 
@@ -48,16 +43,17 @@ defmodule Mobilizon.Service.Workers.RefreshInstances do
         end
 
       with instance_metadata <- fetch_instance_metadata(domain),
-           :ok <- Logger.debug("Ready to save instance actor details"),
+           args <- %{
+             domain: domain,
+             actor_id: actor_id,
+             instance_name: get_in(instance_metadata, ["metadata", "nodeName"]),
+             instance_description: get_in(instance_metadata, ["metadata", "nodeDescription"]),
+             software: get_in(instance_metadata, ["software", "name"]),
+             software_version: get_in(instance_metadata, ["software", "version"])
+           },
+           :ok <- Logger.debug("Ready to save instance actor details #{inspect(args)}"),
            {:ok, %InstanceActor{}} <-
-             Instances.create_instance_actor(%{
+             Instances.create_instance_actor(args) do
-               domain: domain,
-               actor_id: actor_id,
-               instance_name: get_in(instance_metadata, ["metadata", "nodeName"]),
-               instance_description: get_in(instance_metadata, ["metadata", "nodeDescription"]),
-               software: get_in(instance_metadata, ["software", "name"]),
-               software_version: get_in(instance_metadata, ["software", "version"])
-             }) do
         Logger.info("Saved instance actor details for domain #{host}")
       else
         err ->

src/components/core/MaterialIcon.vue

@@ -262,6 +262,8 @@ const icons: Record<string, () => Promise<any>> = {
     import(`../../../node_modules/vue-material-design-icons/PencilOutline.vue`),
   Apps: () =>
     import(`../../../node_modules/vue-material-design-icons/Apps.vue`),
+  Server: () =>
+    import(`../../../node_modules/vue-material-design-icons/Server.vue`),
 };
 
 const props = withDefaults(

src/views/Admin/InstancesView.vue

@@ -72,11 +72,11 @@
             name: RouteName.INSTANCE,
             params: { domain: instance.domain },
           }"
-          class="flex items-center mb-2 rounded bg-mbz-yellow-alt-300 hover:bg-mbz-yellow-alt-200 dark:bg-mbz-purple-600 dark:hover:bg-mbz-purple-700 p-4 flex-wrap justify-center gap-x-2 gap-y-3"
+          class="min-w-0 flex items-center mb-2 rounded bg-mbz-yellow-alt-300 hover:bg-mbz-yellow-alt-200 dark:bg-mbz-purple-600 dark:hover:bg-mbz-purple-700 p-4 flex-wrap md:flex-nowrap justify-center gap-x-2 gap-y-3"
           v-for="instance in instances.elements"
           :key="instance.domain"
         >
-          <div class="grow overflow-hidden flex items-center gap-1">
+          <div class="flex-1 overflow-hidden flex items-center gap-1">
             <img
               class="w-12"
               v-if="instance.software === 'Mobilizon'"
@@ -104,7 +104,7 @@
 
             <div class="">
               <h3
-                class="text-lg truncate font-bold text-slate-800 dark:text-slate-100"
+                class="text-lg truncate font-bold line-clamp-1 text-slate-800 dark:text-slate-100"
                 v-if="instance.instanceName"
               >
                 {{ instance.instanceName }}
@@ -115,52 +115,62 @@
               >
                 {{ instance.domain }}
               </h3>
-              <p
+              <div>
-                v-if="instance.instanceName"
+                <div class="flex flex-wrap gap-x-2 gap-y-1">
-                class="inline-flex gap-2 text-slate-700 dark:text-slate-300"
+                  <p
-              >
+                    v-if="instance.instanceName"
-                <span class="capitalize" v-if="instance.software">{{
+                    class="min-w-0 inline-flex gap-1 truncate text-slate-700 dark:text-slate-300"
-                  instance.software
+                  >
-                }}</span>
+                    <o-icon icon="web" />
-                -
+                    <span>{{ instance.domain }}</span>
-                <span>{{ instance.domain }}</span>
+                  </p>
-              </p>
+                  <p
-              <p
+                    v-if="instance.software"
-                v-else-if="instance.software"
+                    class="capitalize text-slate-700 dark:text-slate-300 inline-flex gap-1"
-                class="capitalize text-slate-700 dark:text-slate-300"
+                  >
-              >
+                    <o-icon icon="server" />
-                {{ instance.software }}
+                    {{ instance.software }}
-              </p>
+                  </p>
-              <span
+                </div>
-                class="text-sm"
+                <div>
-                v-if="instance.followedStatus === InstanceFollowStatus.APPROVED"
+                  <p
-              >
+                    class="inline-flex gap-1 text-slate-700 dark:text-slate-300"
-                <o-icon icon="inbox-arrow-down" />
+                    v-if="
-                {{ t("Followed") }}</span
+                      instance.followedStatus === InstanceFollowStatus.APPROVED
-              >
+                    "
-              <span
+                  >
-                class="text-sm"
+                    <o-icon icon="inbox-arrow-down" />
-                v-else-if="
+                    {{ t("Followed") }}
-                  instance.followedStatus === InstanceFollowStatus.PENDING
+                  </p>
-                "
+                  <p
-              >
+                    class="inline-flex gap-1 text-slate-700 dark:text-slate-300"
-                <o-icon icon="inbox-arrow-down" />
+                    v-else-if="
-                {{ t("Followed, pending response") }}</span
+                      instance.followedStatus === InstanceFollowStatus.PENDING
-              >
+                    "
-              <span
+                  >
-                class="text-sm"
+                    <o-icon icon="inbox-arrow-down" />
-                v-if="instance.followerStatus == InstanceFollowStatus.APPROVED"
+                    {{ t("Followed, pending response") }}
-              >
+                  </p>
-                <o-icon icon="inbox-arrow-up" />
+                  <p
-                {{ t("Follows us") }}</span
+                    class="inline-flex gap-1 text-slate-700 dark:text-slate-300"
-              >
+                    v-if="
-              <span
+                      instance.followerStatus == InstanceFollowStatus.APPROVED
-                class="text-sm"
+                    "
-                v-if="instance.followerStatus == InstanceFollowStatus.PENDING"
+                  >
-              >
+                    <o-icon icon="inbox-arrow-up" />
-                <o-icon icon="inbox-arrow-up" />
+                    {{ t("Follows us") }}
-                {{ t("Follows us, pending approval") }}</span
+                  </p>
-              >
+                  <p
+                    class="inline-flex gap-1 text-slate-700 dark:text-slate-300"
+                    v-else-if="
+                      instance.followerStatus == InstanceFollowStatus.PENDING
+                    "
+                  >
+                    <o-icon icon="inbox-arrow-up" />
+                    {{ t("Follows us, pending approval") }}
+                  </p>
+                </div>
+              </div>
             </div>
           </div>
           <div class="flex-none flex gap-3 ltr:ml-3 rtl:mr-3">

test/federation/node_info_test.exs

@@ -24,7 +24,12 @@ defmodule Mobilizon.Federation.NodeInfoTest do
           url: "https://event-federation.eu/.well-known/nodeinfo"
         },
         _opts ->
-          {:ok, %Tesla.Env{status: 200, body: nodeinfo_data}}
+          {:ok,
+           %Tesla.Env{
+             status: 200,
+             body: nodeinfo_data,
+             headers: [{"content-type", "application/json"}]
+           }}
       end)
 
       assert "https://event-federation.eu/actor-relay" ==
@@ -76,7 +81,12 @@ defmodule Mobilizon.Federation.NodeInfoTest do
           url: "https://mobilizon.fr/.well-known/nodeinfo"
         },
         _opts ->
-          {:ok, %Tesla.Env{status: 200, body: nodeinfo_end_point_data}}
+          {:ok,
+           %Tesla.Env{
+             status: 200,
+             body: nodeinfo_end_point_data,
+             headers: [{"content-type", "application/json"}]
+           }}
       end)
 
       WebfingerClientMock
@@ -86,7 +96,12 @@ defmodule Mobilizon.Federation.NodeInfoTest do
           url: "https://mobilizon.fr/.well-known/nodeinfo/2.1"
         },
         _opts ->
-          {:ok, %Tesla.Env{status: 200, body: nodeinfo_data}}
+          {:ok,
+           %Tesla.Env{
+             status: 200,
+             body: nodeinfo_data,
+             headers: [{"content-type", "application/json"}]
+           }}
       end)
 
       assert {:ok, data} = NodeInfo.nodeinfo("mobilizon.fr")
@@ -107,7 +122,12 @@ defmodule Mobilizon.Federation.NodeInfoTest do
           url: "https://event-federation.eu/.well-known/nodeinfo"
         },
         _opts ->
-          {:ok, %Tesla.Env{status: 200, body: nodeinfo_end_point_data}}
+          {:ok,
+           %Tesla.Env{
+             status: 200,
+             body: nodeinfo_end_point_data,
+             headers: [{"content-type", "application/json"}]
+           }}
       end)
 
       WebfingerClientMock
@@ -117,7 +137,12 @@ defmodule Mobilizon.Federation.NodeInfoTest do
           url: "https://event-federation.eu/wp-json/activitypub/1.0/nodeinfo"
         },
         _opts ->
-          {:ok, %Tesla.Env{status: 200, body: nodeinfo_wp_data}}
+          {:ok,
+           %Tesla.Env{
+             status: 200,
+             body: nodeinfo_wp_data,
+             headers: [{"content-type", "application/json"}]
+           }}
       end)
 
       assert {:ok, data} = NodeInfo.nodeinfo("event-federation.eu")
@@ -138,7 +163,12 @@ defmodule Mobilizon.Federation.NodeInfoTest do
           url: "https://somewhere.tld/.well-known/nodeinfo"
         },
         _opts ->
-          {:ok, %Tesla.Env{status: 200, body: nodeinfo_end_point_data}}
+          {:ok,
+           %Tesla.Env{
+             status: 200,
+             body: nodeinfo_end_point_data,
+             headers: [{"content-type", "application/json"}]
+           }}
       end)
 
       assert {:error, :no_node_info_endpoint_found} = NodeInfo.nodeinfo("somewhere.tld")
@@ -169,7 +199,12 @@ defmodule Mobilizon.Federation.NodeInfoTest do
           url: "https://mobilizon.fr/.well-known/nodeinfo"
         },
         _opts ->
-          {:ok, %Tesla.Env{status: 200, body: nodeinfo_end_point_data}}
+          {:ok,
+           %Tesla.Env{
+             status: 200,
+             body: nodeinfo_end_point_data,
+             headers: [{"content-type", "application/json"}]
+           }}
       end)
 
       WebfingerClientMock

test/service/workers/refresh_instances_test.exs

@@ -5,7 +5,6 @@ defmodule Mobilizon.Service.Workers.RefreshInstancesTest do
 
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Federation.ActivityPub.Relay
-  alias Mobilizon.Instances.Instance
   alias Mobilizon.Service.Workers.RefreshInstances
 
   use Mobilizon.DataCase
@@ -14,7 +13,7 @@ defmodule Mobilizon.Service.Workers.RefreshInstancesTest do
     test "unless if local actor" do
       # relay = Mobilizon.Web.Relay.get_actor()
       assert {:error, :not_remote_instance} ==
-               RefreshInstances.refresh_instance_actor(%Instance{domain: nil})
+               RefreshInstances.refresh_instance_actor(nil)
     end
 
     test "unless if local relay actor" do
@@ -22,7 +21,7 @@ defmodule Mobilizon.Service.Workers.RefreshInstancesTest do
       %URI{host: domain} = URI.new!(url)
 
       assert {:error, :not_remote_instance} ==
-               RefreshInstances.refresh_instance_actor(%Instance{domain: domain})
+               RefreshInstances.refresh_instance_actor(domain)
     end
   end
 end