From f5978da67633110acbc6494ff6f07b3f07424779 Mon Sep 17 00:00:00 2001 From: Lain Soykaf Date: Tue, 28 May 2024 14:00:25 +0400 Subject: [PATCH] HTTPSignaturePlugTest: Rewrite to use mox. --- config/test.exs | 4 + lib/pleroma/http_signatures_api.ex | 4 + lib/pleroma/web/plugs/http_signature_plug.ex | 19 +- .../web/plugs/http_signature_plug_test.exs | 219 +++++++++--------- test/support/data_case.ex | 1 + test/support/http_signatures_proxy.ex | 9 + test/support/mocks.ex | 1 + 7 files changed, 144 insertions(+), 113 deletions(-) create mode 100644 lib/pleroma/http_signatures_api.ex create mode 100644 test/support/http_signatures_proxy.ex diff --git a/config/test.exs b/config/test.exs index 6c88ad3c6..0d4c82e0e 100644 --- a/config/test.exs +++ b/config/test.exs @@ -155,6 +155,10 @@ config :pleroma, Pleroma.ScheduledActivity, config_impl: Pleroma.UnstubbedConfig config :pleroma, Pleroma.Web.RichMedia.Helpers, config_impl: Pleroma.StaticStubbedConfigMock config :pleroma, Pleroma.Uploaders.IPFS, config_impl: Pleroma.UnstubbedConfigMock config :pleroma, Pleroma.Web.Plugs.HTTPSecurityPlug, config_impl: Pleroma.StaticStubbedConfigMock +config :pleroma, Pleroma.Web.Plugs.HTTPSignaturePlug, config_impl: Pleroma.StaticStubbedConfigMock + +config :pleroma, Pleroma.Web.Plugs.HTTPSignaturePlug, + http_signatures_impl: Pleroma.StubbedHTTPSignaturesMock peer_module = if String.to_integer(System.otp_release()) >= 25 do diff --git a/lib/pleroma/http_signatures_api.ex b/lib/pleroma/http_signatures_api.ex new file mode 100644 index 000000000..8e73dc98e --- /dev/null +++ b/lib/pleroma/http_signatures_api.ex @@ -0,0 +1,4 @@ +defmodule Pleroma.HTTPSignaturesAPI do + @callback validate_conn(conn :: Plug.Conn.t()) :: boolean + @callback signature_for_conn(conn :: Plug.Conn.t()) :: map +end diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex index 71b2a5f51..6bf2dd432 100644 --- a/lib/pleroma/web/plugs/http_signature_plug.ex +++ b/lib/pleroma/web/plugs/http_signature_plug.ex @@ -8,11 +8,17 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do import Plug.Conn import Phoenix.Controller, only: [get_format: 1, text: 2] - alias Pleroma.Config alias Pleroma.Web.ActivityPub.MRF require Logger + @config_impl Application.compile_env(:pleroma, [__MODULE__, :config_impl], Pleroma.Config) + @http_signatures_impl Application.compile_env( + :pleroma, + [__MODULE__, :http_signatures_impl], + HTTPSignatures + ) + def init(options) do options end @@ -41,7 +47,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do |> put_req_header("(request-target)", request_target) |> put_req_header("@request-target", request_target) - HTTPSignatures.validate_conn(conn) + @http_signatures_impl.validate_conn(conn) end defp validate_signature(conn) do @@ -108,9 +114,9 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do defp maybe_require_signature(%{assigns: %{valid_signature: true}} = conn), do: conn defp maybe_require_signature(%{remote_ip: remote_ip} = conn) do - if Pleroma.Config.get([:activitypub, :authorized_fetch_mode], false) do + if @config_impl.get([:activitypub, :authorized_fetch_mode], false) do exceptions = - Pleroma.Config.get([:activitypub, :authorized_fetch_mode_exceptions], []) + @config_impl.get([:activitypub, :authorized_fetch_mode_exceptions], []) |> Enum.map(&InetHelper.parse_cidr/1) if Enum.any?(exceptions, fn x -> InetCidr.contains?(x, remote_ip) end) do @@ -129,7 +135,8 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do defp maybe_filter_requests(%{halted: true} = conn), do: conn defp maybe_filter_requests(conn) do - if Pleroma.Config.get([:activitypub, :authorized_fetch_mode], false) do + if @config_impl.get([:activitypub, :authorized_fetch_mode], false) and + conn.assigns[:actor_id] do %{host: host} = URI.parse(conn.assigns.actor_id) if MRF.subdomain_match?(rejected_domains(), host) do @@ -145,7 +152,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do end defp rejected_domains do - Config.get([:instance, :rejected_instances]) + @config_impl.get([:instance, :rejected_instances]) |> Pleroma.Web.ActivityPub.MRF.instance_list_from_tuples() |> Pleroma.Web.ActivityPub.MRF.subdomains_regex() end diff --git a/test/pleroma/web/plugs/http_signature_plug_test.exs b/test/pleroma/web/plugs/http_signature_plug_test.exs index b871d956e..5f049dc45 100644 --- a/test/pleroma/web/plugs/http_signature_plug_test.exs +++ b/test/pleroma/web/plugs/http_signature_plug_test.exs @@ -3,89 +3,88 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do - use Pleroma.Web.ConnCase + use Pleroma.Web.ConnCase, async: true alias Pleroma.Web.Plugs.HTTPSignaturePlug + alias Pleroma.StubbedHTTPSignaturesMock, as: HTTPSignaturesMock + alias Pleroma.StaticStubbedConfigMock, as: ConfigMock import Plug.Conn import Phoenix.Controller, only: [put_format: 2] - import Mock + import Mox - test "it call HTTPSignatures to check validity if the actor signed it" do + test "it calls HTTPSignatures to check validity if the actor signed it" do params = %{"actor" => "http://mastodon.example.org/users/admin"} conn = build_conn(:get, "/doesntmattter", params) - with_mock HTTPSignatures, - validate_conn: fn _ -> true end, - signature_for_conn: fn _ -> - %{"keyId" => "http://mastodon.example.org/users/admin#main-key"} - end do - conn = - conn - |> put_req_header( - "signature", - "keyId=\"http://mastodon.example.org/users/admin#main-key" - ) - |> put_format("activity+json") - |> HTTPSignaturePlug.call(%{}) + HTTPSignaturesMock + |> expect(:validate_conn, fn _ -> true end) - assert conn.assigns.valid_signature == true - assert conn.halted == false - assert called(HTTPSignatures.validate_conn(:_)) - end + conn = + conn + |> put_req_header( + "signature", + "keyId=\"http://mastodon.example.org/users/admin#main-key" + ) + |> put_format("activity+json") + |> HTTPSignaturePlug.call(%{}) + + assert conn.assigns.valid_signature == true + assert conn.halted == false end describe "requires a signature when `authorized_fetch_mode` is enabled" do setup do - clear_config([:activitypub, :authorized_fetch_mode], true) - params = %{"actor" => "http://mastodon.example.org/users/admin"} conn = build_conn(:get, "/doesntmattter", params) |> put_format("activity+json") [conn: conn] end - test "when signature header is present", %{conn: conn} do - with_mock HTTPSignatures, - validate_conn: fn _ -> false end, - signature_for_conn: fn _ -> - %{"keyId" => "http://mastodon.example.org/users/admin#main-key"} - end do - conn = - conn - |> put_req_header( - "signature", - "keyId=\"http://mastodon.example.org/users/admin#main-key" - ) - |> HTTPSignaturePlug.call(%{}) + test "when signature header is present", %{conn: orig_conn} do + ConfigMock + |> expect(:get, fn [:activitypub, :authorized_fetch_mode], false -> true end) + |> expect(:get, fn [:activitypub, :authorized_fetch_mode_exceptions], [] -> [] end) - assert conn.assigns.valid_signature == false - assert conn.halted == true - assert conn.status == 401 - assert conn.state == :sent - assert conn.resp_body == "Request not signed" - assert called(HTTPSignatures.validate_conn(:_)) - end + HTTPSignaturesMock + |> expect(:validate_conn, 2, fn _ -> false end) - with_mock HTTPSignatures, - validate_conn: fn _ -> true end, - signature_for_conn: fn _ -> - %{"keyId" => "http://mastodon.example.org/users/admin#main-key"} - end do - conn = - conn - |> put_req_header( - "signature", - "keyId=\"http://mastodon.example.org/users/admin#main-key" - ) - |> HTTPSignaturePlug.call(%{}) + conn = + orig_conn + |> put_req_header( + "signature", + "keyId=\"http://mastodon.example.org/users/admin#main-key" + ) + |> HTTPSignaturePlug.call(%{}) - assert conn.assigns.valid_signature == true - assert conn.halted == false - assert called(HTTPSignatures.validate_conn(:_)) - end + assert conn.assigns.valid_signature == false + assert conn.halted == true + assert conn.status == 401 + assert conn.state == :sent + assert conn.resp_body == "Request not signed" + + ConfigMock + |> expect(:get, fn [:activitypub, :authorized_fetch_mode], false -> true end) + + HTTPSignaturesMock + |> expect(:validate_conn, fn _ -> true end) + + conn = + orig_conn + |> put_req_header( + "signature", + "keyId=\"http://mastodon.example.org/users/admin#main-key" + ) + |> HTTPSignaturePlug.call(%{}) + + assert conn.assigns.valid_signature == true + assert conn.halted == false end test "halts the connection when `signature` header is not present", %{conn: conn} do + ConfigMock + |> expect(:get, fn [:activitypub, :authorized_fetch_mode], false -> true end) + |> expect(:get, fn [:activitypub, :authorized_fetch_mode_exceptions], [] -> [] end) + conn = HTTPSignaturePlug.call(conn, %{}) assert conn.assigns[:valid_signature] == nil assert conn.halted == true @@ -95,65 +94,71 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do end test "exempts specific IPs from `authorized_fetch_mode_exceptions`", %{conn: conn} do - clear_config([:activitypub, :authorized_fetch_mode_exceptions], ["192.168.0.0/24"]) + ConfigMock + |> expect(:get, fn [:activitypub, :authorized_fetch_mode], false -> true end) + |> expect(:get, fn [:activitypub, :authorized_fetch_mode_exceptions], [] -> + ["192.168.0.0/24"] + end) + |> expect(:get, fn [:activitypub, :authorized_fetch_mode], false -> true end) - with_mock HTTPSignatures, validate_conn: fn _ -> false end do - conn = - conn - |> Map.put(:remote_ip, {192, 168, 0, 1}) - |> put_req_header( - "signature", - "keyId=\"http://mastodon.example.org/users/admin#main-key" - ) - |> HTTPSignaturePlug.call(%{}) + HTTPSignaturesMock + |> expect(:validate_conn, 2, fn _ -> false end) - assert conn.remote_ip == {192, 168, 0, 1} - assert conn.halted == false - assert called(HTTPSignatures.validate_conn(:_)) - end - end - end - - test "rejects requests from `rejected_instances` when `authorized_fetch_mode` is enabled" do - clear_config([:activitypub, :authorized_fetch_mode], true) - clear_config([:instance, :rejected_instances], [{"mastodon.example.org", "no reason"}]) - - with_mock HTTPSignatures, - validate_conn: fn _ -> true end, - signature_for_conn: fn _ -> - %{"keyId" => "http://mastodon.example.org/users/admin#main-key"} - end do conn = - build_conn(:get, "/doesntmattter", %{"actor" => "http://mastodon.example.org/users/admin"}) + conn + |> Map.put(:remote_ip, {192, 168, 0, 1}) |> put_req_header( "signature", "keyId=\"http://mastodon.example.org/users/admin#main-key" ) - |> put_format("activity+json") |> HTTPSignaturePlug.call(%{}) - assert conn.assigns.valid_signature == true - assert conn.halted == true - assert called(HTTPSignatures.validate_conn(:_)) - end - - with_mock HTTPSignatures, - validate_conn: fn _ -> true end, - signature_for_conn: fn _ -> - %{"keyId" => "http://allowed.example.org/users/admin#main-key"} - end do - conn = - build_conn(:get, "/doesntmattter", %{"actor" => "http://allowed.example.org/users/admin"}) - |> put_req_header( - "signature", - "keyId=\"http://allowed.example.org/users/admin#main-key" - ) - |> put_format("activity+json") - |> HTTPSignaturePlug.call(%{}) - - assert conn.assigns.valid_signature == true + assert conn.remote_ip == {192, 168, 0, 1} assert conn.halted == false - assert called(HTTPSignatures.validate_conn(:_)) end end + + test "rejects requests from `rejected_instances` when `authorized_fetch_mode` is enabled" do + ConfigMock + |> expect(:get, fn [:activitypub, :authorized_fetch_mode], false -> true end) + |> expect(:get, fn [:instance, :rejected_instances] -> + [{"mastodon.example.org", "no reason"}] + end) + + HTTPSignaturesMock + |> expect(:validate_conn, fn _ -> true end) + + conn = + build_conn(:get, "/doesntmattter", %{"actor" => "http://mastodon.example.org/users/admin"}) + |> put_req_header( + "signature", + "keyId=\"http://mastodon.example.org/users/admin#main-key" + ) + |> put_format("activity+json") + |> HTTPSignaturePlug.call(%{}) + + assert conn.assigns.valid_signature == true + assert conn.halted == true + + ConfigMock + |> expect(:get, fn [:activitypub, :authorized_fetch_mode], false -> true end) + |> expect(:get, fn [:instance, :rejected_instances] -> + [{"mastodon.example.org", "no reason"}] + end) + + HTTPSignaturesMock + |> expect(:validate_conn, fn _ -> true end) + + conn = + build_conn(:get, "/doesntmattter", %{"actor" => "http://allowed.example.org/users/admin"}) + |> put_req_header( + "signature", + "keyId=\"http://allowed.example.org/users/admin#main-key" + ) + |> put_format("activity+json") + |> HTTPSignaturePlug.call(%{}) + + assert conn.assigns.valid_signature == true + assert conn.halted == false + end end diff --git a/test/support/data_case.ex b/test/support/data_case.ex index 14403f0b8..52d4bef1a 100644 --- a/test/support/data_case.ex +++ b/test/support/data_case.ex @@ -116,6 +116,7 @@ defmodule Pleroma.DataCase do Mox.stub_with(Pleroma.Web.FederatorMock, Pleroma.Web.Federator) Mox.stub_with(Pleroma.ConfigMock, Pleroma.Config) Mox.stub_with(Pleroma.StaticStubbedConfigMock, Pleroma.Test.StaticConfig) + Mox.stub_with(Pleroma.StubbedHTTPSignaturesMock, Pleroma.Test.HTTPSignaturesProxy) end def ensure_local_uploader(context) do diff --git a/test/support/http_signatures_proxy.ex b/test/support/http_signatures_proxy.ex new file mode 100644 index 000000000..4c6b39d19 --- /dev/null +++ b/test/support/http_signatures_proxy.ex @@ -0,0 +1,9 @@ +defmodule Pleroma.Test.HTTPSignaturesProxy do + @behaviour Pleroma.HTTPSignaturesAPI + + @impl true + defdelegate validate_conn(conn), to: HTTPSignatures + + @impl true + defdelegate signature_for_conn(conn), to: HTTPSignatures +end diff --git a/test/support/mocks.ex b/test/support/mocks.ex index d906f0e1d..63cbc49ab 100644 --- a/test/support/mocks.ex +++ b/test/support/mocks.ex @@ -28,6 +28,7 @@ Mox.defmock(Pleroma.Web.FederatorMock, for: Pleroma.Web.Federator.Publishing) Mox.defmock(Pleroma.ConfigMock, for: Pleroma.Config.Getting) Mox.defmock(Pleroma.UnstubbedConfigMock, for: Pleroma.Config.Getting) Mox.defmock(Pleroma.StaticStubbedConfigMock, for: Pleroma.Config.Getting) +Mox.defmock(Pleroma.StubbedHTTPSignaturesMock, for: Pleroma.HTTPSignaturesAPI) Mox.defmock(Pleroma.LoggerMock, for: Pleroma.Logging)