From 6da783b84d628a4cc71408e26e6834d029d7299d Mon Sep 17 00:00:00 2001 From: Floatingghost Date: Fri, 28 Jun 2024 03:22:11 +0100 Subject: [PATCH] Fix http signature plug tests --- lib/pleroma/user/signing_key.ex | 21 ++++++++---- .../activity_pub/activity_pub_controller.ex | 1 - .../web/plugs/ensure_user_public_key_plug.ex | 1 + lib/pleroma/web/plugs/http_signature_plug.ex | 13 +++++--- .../activity_pub_controller_test.exs | 32 ++++++++++++------- .../web/plugs/http_signature_plug_test.exs | 15 +++++++-- test/support/factory.ex | 7 +++- 7 files changed, 63 insertions(+), 27 deletions(-) diff --git a/lib/pleroma/user/signing_key.ex b/lib/pleroma/user/signing_key.ex index 1568b5487..3d9b476f0 100644 --- a/lib/pleroma/user/signing_key.ex +++ b/lib/pleroma/user/signing_key.ex @@ -111,14 +111,20 @@ 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 - key = - public_key_pem - |> :public_key.pem_decode() - |> hd() - |> :public_key.pem_entry_decode() + 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() + |> hd() + |> :public_key.pem_entry_decode() - {:ok, key} + {: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. diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index dff3e1f8e..9c6b3655d 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/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 diff --git a/lib/pleroma/web/plugs/ensure_user_public_key_plug.ex b/lib/pleroma/web/plugs/ensure_user_public_key_plug.ex index baebcec29..c59053854 100644 --- a/lib/pleroma/web/plugs/ensure_user_public_key_plug.ex +++ b/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. diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex index e3ae99636..06527cada 100644 --- a/lib/pleroma/web/plugs/http_signature_plug.ex +++ b/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), - {:ok, actor_id} <- Signature.key_id_to_actor_id(kid) do + with {:key_id, %{"keyId" => kid}} <- {:key_id, HTTPSignatures.signature_for_conn(conn)}, + {:actor_id, {:ok, actor_id}} <- {:actor_id, Signature.key_id_to_actor_id(kid)} do actor_id else - e -> - {:error, e} + {:key_id, 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 diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 26b6f8e47..d67b59ef2 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -609,8 +609,10 @@ 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"]) - |> with_signing_key() + + sender = + insert(:user, ap_id: data["actor"]) + |> with_signing_key() Instances.set_consistently_unreachable(sender_url) refute Instances.reachable?(sender_url) @@ -976,8 +978,10 @@ 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) - |> with_signing_key() + + announcer = + insert(:user, local: false) + |> with_signing_key() data = %{ "@context" => "https://www.w3.org/ns/activitystreams", @@ -1007,8 +1011,10 @@ 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"}) - |> with_signing_key() + + 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) - |> with_signing_key() + 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) - |> with_signing_key() + + actor = + insert(:user, local: false) + |> with_signing_key() + remote_domain = URI.parse(actor.ap_id).host reported_user = insert(:user) diff --git a/test/pleroma/web/plugs/http_signature_plug_test.exs b/test/pleroma/web/plugs/http_signature_plug_test.exs index 0a602424d..7f623bfca 100644 --- a/test/pleroma/web/plugs/http_signature_plug_test.exs +++ b/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 - params = %{"actor" => "http://mastodon.example.org/users/admin"} + test "it call HTTPSignatures to check validity if the actor signed it", %{user: user} do + 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(%{}) diff --git a/test/support/factory.ex b/test/support/factory.ex index 6689cc5d4..1f54c5b7a 100644 --- a/test/support/factory.ex +++ b/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,