Fix http signature plug tests

lib/pleroma/user/signing_key.ex

@@ -111,7 +111,9 @@ def private_pem_to_public_pem(private_pem) do
   @doc """
   Given a user, return the public key for that user in binary format.
   """
-  def public_key(%User{signing_key: %__MODULE__{public_key: public_key_pem}}) do
+  def public_key(%User{} = user) do
+    case Repo.preload(user, :signing_key) do
+      %User{signing_key: %__MODULE__{public_key: public_key_pem}} ->
         key =
           public_key_pem
           |> :public_key.pem_decode()
@@ -119,6 +121,10 @@ def public_key(%User{signing_key: %__MODULE__{public_key: public_key_pem}}) do
           |> :public_key.pem_entry_decode()
 
         {:ok, key}
+
+      _ ->
+        {:error, "key not found"}
+    end
   end
 
   def public_key(_), do: {:error, "key not found"}
@@ -133,6 +139,7 @@ def public_key_pem(%User{} = user) do
   def public_key_pem(e) do
     {:error, "key not found"}
   end
+
   @spec private_key(User.t()) :: {:ok, binary()} | {:error, String.t()}
   @doc """
   Given a user, return the private key for that user in binary format.

lib/pleroma/web/activity_pub/activity_pub_controller.ex

@@ -60,7 +60,6 @@ defp relay_active?(conn, _) do
     end
   end
 
-
   @doc """
   Render the user's AP data
   WARNING: we cannot actually check if the request has a fragment! so let's play defensively

lib/pleroma/web/plugs/ensure_user_public_key_plug.ex

@@ -12,6 +12,7 @@ def init(options), do: options
 
   def call(conn, _opts) do
     key_id = key_id_from_conn(conn)
+
     unless is_nil(key_id) do
       SigningKey.fetch_remote_key(key_id)
       # now we SHOULD have the user that owns the key locally. maybe.

lib/pleroma/web/plugs/http_signature_plug.ex

@@ -139,12 +139,17 @@ defp maybe_require_signature(
   defp maybe_require_signature(conn), do: conn
 
   defp signature_host(conn) do
-    with %{"keyId" => kid} <- HTTPSignatures.signature_for_conn(conn),
+    with {:key_id, %{"keyId" => kid}} <- {:key_id, HTTPSignatures.signature_for_conn(conn)},
-         {:ok, actor_id} <- Signature.key_id_to_actor_id(kid) do
+         {:actor_id, {:ok, actor_id}} <- {:actor_id, Signature.key_id_to_actor_id(kid)} do
       actor_id
     else
-      e ->
+      {:key_id, e} ->
-        {:error, e}
+        Logger.error("Failed to extract key_id from signature: #{inspect(e)}")
+        nil
+
+      {:actor_id, e} ->
+        Logger.error("Failed to extract actor_id from signature: #{inspect(e)}")
+        nil
     end
   end
 end

test/pleroma/web/activity_pub/activity_pub_controller_test.exs

@@ -609,7 +609,9 @@ test "it clears `unreachable` federation status of the sender", %{conn: conn} do
       data = File.read!("test/fixtures/mastodon-post-activity.json") |> Jason.decode!()
 
       sender_url = data["actor"]
-      sender = insert(:user, ap_id: data["actor"])
+
+      sender =
+        insert(:user, ap_id: data["actor"])
         |> with_signing_key()
 
       Instances.set_consistently_unreachable(sender_url)
@@ -976,7 +978,9 @@ test "it accepts announces with to as string instead of array", %{conn: conn} do
       user = insert(:user)
 
       {:ok, post} = CommonAPI.post(user, %{status: "hey"})
-      announcer = insert(:user, local: false)
+
+      announcer =
+        insert(:user, local: false)
         |> with_signing_key()
 
       data = %{
@@ -1007,7 +1011,9 @@ test "it accepts messages from actors that are followed by the user", %{
       data: data
     } do
       recipient = insert(:user)
-      actor = insert(:user, %{ap_id: "http://mastodon.example.org/users/actor"})
+
+      actor =
+        insert(:user, %{ap_id: "http://mastodon.example.org/users/actor"})
         |> with_signing_key()
 
       {:ok, recipient, actor} = User.follow(recipient, actor)
@@ -1061,8 +1067,10 @@ test "it returns a note activity in a collection", %{conn: conn} do
     end
 
     test "it clears `unreachable` federation status of the sender", %{conn: conn, data: data} do
-      user = insert(:user)
+      user =
+        insert(:user)
         |> with_signing_key()
+
       data = Map.put(data, "bcc", [user.ap_id])
 
       sender_host = URI.parse(data["actor"]).host
@@ -1085,7 +1093,6 @@ test "it removes all follower collections but actor's", %{conn: conn} do
       [actor, recipient] = insert_pair(:user)
       actor = with_signing_key(actor)
 
-
       to = [
         recipient.ap_id,
         recipient.follower_address,
@@ -1149,8 +1156,11 @@ test "it requires authentication", %{conn: conn} do
     @tag capture_log: true
     test "forwarded report", %{conn: conn} do
       admin = insert(:user, is_admin: true)
-      actor = insert(:user, local: false)
+
+      actor =
+        insert(:user, local: false)
         |> with_signing_key()
+
       remote_domain = URI.parse(actor.ap_id).host
       reported_user = insert(:user)
 

test/pleroma/web/plugs/http_signature_plug_test.exs

@@ -14,6 +14,15 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do
   import Phoenix.Controller, only: [put_format: 2]
   import Mock
 
+  setup do
+    user =
+      :user
+      |> insert(%{ ap_id: "http://mastodon.example.org/users/admin" })
+      |> with_signing_key(%{ key_id: "http://mastodon.example.org/users/admin#main-key" })
+
+    {:ok, %{user: user}}
+  end
+
   setup_with_mocks([
     {HTTPSignatures, [],
      [
@@ -46,15 +55,15 @@ defp submit_to_plug(host, method, path) do
     |> HTTPSignaturePlug.call(%{})
   end
 
-  test "it call HTTPSignatures to check validity if the actor signed it" do
+  test "it call HTTPSignatures to check validity if the actor signed it", %{user: user} do
-    params = %{"actor" => "http://mastodon.example.org/users/admin"}
+    params = %{"actor" => user.ap_id}
     conn = build_conn(:get, "/doesntmattter", params)
 
     conn =
       conn
       |> put_req_header(
         "signature",
-        "keyId=\"http://mastodon.example.org/users/admin#main-key"
+        "keyId=\"#{user.signing_key.key_id}\""
       )
       |> put_format("activity+json")
       |> HTTPSignaturePlug.call(%{})

test/support/factory.ex

@@ -88,14 +88,18 @@ def user_factory(attrs \\ %{}) do
       end
 
     attrs = Map.delete(attrs, :domain)
+
     user
     |> Map.put(:raw_bio, user.bio)
     |> Map.merge(urls)
     |> merge_attributes(attrs)
   end
 
-  def with_signing_key(%User{} = user) do
+  def with_signing_key(%User{} = user, attrs \\ %{}) do
+
     signing_key = build(:signing_key, %{user: user, key_id: "#{user.ap_id}#main-key"})
+    |> merge_attributes(attrs)
+
     insert(signing_key)
     %{user | signing_key: signing_key}
   end
@@ -104,6 +108,7 @@ def signing_key_factory(attrs \\ %{}) do
     pem = Enum.random(@rsa_keys)
     user = attrs[:user] || insert(:user)
     {:ok, public_key} = Pleroma.User.SigningKey.private_pem_to_public_pem(pem)
+
     %Pleroma.User.SigningKey{
       user_id: user.id,
       public_key: public_key,