Simplified HTTP signature processing

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 73346617e65598f6865f8581a0266d062deb6f36..6b99a0e3c3341613c8c7fa5dcb2c4739df25f864 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Non-admin users now cannot register `admin` scope tokens (not security-critical, they didn't work before, but you _could_ create them)
   - Admin scopes will be dropped on create
 - Rich media will now backoff for 20 minutes after a failure
+- Simplified HTTP signature processing
 
 ### Fixed 
 - /api/v1/accounts/lookup will now respect restrict\_unauthenticated

diff --git a/lib/pleroma/web/o_auth/scopes.ex b/lib/pleroma/web/o_auth/scopes.ex
index d5e7c29d678666e017f479244271b8a9501eb293..344ecd631d9f5ed075079f8e0d06ff24e87e4e8b 100644 (file)
--- a/lib/pleroma/web/o_auth/scopes.ex
+++ b/lib/pleroma/web/o_auth/scopes.ex
@@ -61,7 +61,7 @@ defmodule Pleroma.Web.OAuth.Scopes do
   def validate(blank_scopes, _app_scopes, _user) when blank_scopes in [nil, []],
     do: {:error, :missing_scopes}
 
-  def validate(scopes, app_scopes, %Pleroma.User{is_admin: is_admin}) do
+  def validate(scopes, app_scopes, _user) do
     validate_scopes_are_supported(scopes, app_scopes)
   end
 

diff --git a/lib/pleroma/web/plugs/ensure_http_signature_plug.ex b/lib/pleroma/web/plugs/ensure_http_signature_plug.ex
new file mode 100644 (file)
index 0000000..c75501a
--- /dev/null
+++ b/lib/pleroma/web/plugs/ensure_http_signature_plug.ex
@@ -0,0 +1,31 @@
+# Akkoma: Magically expressive social media
+# Copyright © 2022-2022 Akkoma Authors <https://akkoma.dev/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.Plugs.EnsureHTTPSignaturePlug do
+  @moduledoc """
+  Ensures HTTP signature has been validated by previous plugs on ActivityPub requests.
+  """
+  import Plug.Conn
+  import Phoenix.Controller, only: [get_format: 1, text: 2]
+
+  alias Pleroma.Config
+
+  def init(options) do
+    options
+  end
+
+  def call(%{assigns: %{valid_signature: true}} = conn, _), do: conn
+
+  def call(conn, _) do
+    with true <- get_format(conn) in ["json", "activity+json"],
+         true <- Config.get([:activitypub, :authorized_fetch_mode], true) do
+      conn
+      |> put_status(:unauthorized)
+      |> text("Request not signed")
+      |> halt()
+    else
+      _ -> conn
+    end
+  end
+end

diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex
index 5ed3235e2e32985c4d0b736326d185396160dc08..4ffaa6e982a142f59f896205c425f8a7d5cf591d 100644 (file)
--- a/lib/pleroma/web/plugs/http_signature_plug.ex
+++ b/lib/pleroma/web/plugs/http_signature_plug.ex
@@ -4,7 +4,7 @@
 
 defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
   import Plug.Conn
-  import Phoenix.Controller, only: [get_format: 1, text: 2]
+  import Phoenix.Controller, only: [get_format: 1]
   alias Pleroma.Activity
   alias Pleroma.Web.Router
   alias Pleroma.Signature
@@ -22,7 +22,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
   end
 
   def call(conn, _opts) do
-    if get_format(conn) == "activity+json" do
+    if get_format(conn) in ["json", "activity+json"] do
       conn
       |> maybe_assign_valid_signature()
       |> maybe_require_signature()
@@ -113,18 +113,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
     conn
   end
 
-  defp maybe_require_signature(%{assigns: %{valid_signature: true}} = conn), do: conn
-
-  defp maybe_require_signature(conn) do
-    if Pleroma.Config.get([:activitypub, :authorized_fetch_mode], false) do
-      conn
-      |> put_status(:unauthorized)
-      |> text("Request not signed")
-      |> halt()
-    else
-      conn
-    end
-  end
+  defp maybe_require_signature(conn), do: conn
 
   defp signature_host(conn) do
     with %{"keyId" => kid} <- HTTPSignatures.signature_for_conn(conn),

diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex
index f47041b0b8cc1eb1757072e8ce511fb658ea1070..f984ad59822d19bbbdaa2cafae0b21cd3bf7b80f 100644 (file)
--- a/lib/pleroma/web/router.ex
+++ b/lib/pleroma/web/router.ex
@@ -147,6 +147,7 @@ defmodule Pleroma.Web.Router do
   pipeline :http_signature do
     plug(Pleroma.Web.Plugs.HTTPSignaturePlug)
     plug(Pleroma.Web.Plugs.MappedSignatureToIdentityPlug)
+    plug(Pleroma.Web.Plugs.EnsureHTTPSignaturePlug)
   end
 
   pipeline :static_fe do

diff --git a/test/pleroma/web/plugs/ensure_http_signature_plug_test.exs b/test/pleroma/web/plugs/ensure_http_signature_plug_test.exs
new file mode 100644 (file)
index 0000000..35dabbe
--- /dev/null
+++ b/test/pleroma/web/plugs/ensure_http_signature_plug_test.exs
@@ -0,0 +1,68 @@
+# Akkoma: Magically expressive social media
+# Copyright © 2022-2022 Akkoma Authors <https://akkoma.dev/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.Plugs.EnsureHTTPSignaturePlugTest do
+  use Pleroma.Web.ConnCase
+  alias Pleroma.Web.Plugs.EnsureHTTPSignaturePlug
+
+  import Plug.Conn
+  import Phoenix.Controller, only: [put_format: 2]
+
+  import Pleroma.Tests.Helpers, only: [clear_config: 2]
+
+  describe "requires a signature when `authorized_fetch_mode` is enabled" do
+    setup do
+      clear_config([:activitypub, :authorized_fetch_mode], true)
+
+      conn =
+        build_conn(:get, "/doesntmatter")
+        |> put_format("activity+json")
+
+      [conn: conn]
+    end
+
+    test "and signature has been set as invalid", %{conn: conn} do
+      conn =
+        conn
+        |> assign(:valid_signature, false)
+        |> EnsureHTTPSignaturePlug.call(%{})
+
+      assert conn.halted == true
+      assert conn.status == 401
+      assert conn.state == :sent
+      assert conn.resp_body == "Request not signed"
+    end
+
+    test "and signature has been set as valid", %{conn: conn} do
+      conn =
+        conn
+        |> assign(:valid_signature, true)
+        |> EnsureHTTPSignaturePlug.call(%{})
+
+      assert conn.halted == false
+    end
+
+    test "does nothing for non-ActivityPub content types", %{conn: conn} do
+      conn =
+        conn
+        |> assign(:valid_signature, false)
+        |> put_format("html")
+        |> EnsureHTTPSignaturePlug.call(%{})
+
+      assert conn.halted == false
+    end
+  end
+
+  test "does nothing on invalid signature when `authorized_fetch_mode` is disabled" do
+    clear_config([:activitypub, :authorized_fetch_mode], false)
+
+    conn =
+      build_conn(:get, "/doesntmatter")
+      |> put_format("activity+json")
+      |> assign(:valid_signature, false)
+      |> EnsureHTTPSignaturePlug.call(%{})
+
+    assert conn.halted == false
+  end
+end

diff --git a/test/pleroma/web/plugs/http_signature_plug_test.exs b/test/pleroma/web/plugs/http_signature_plug_test.exs
index 49e0b980823184d13de35f597a1d12a764dc1ecf..34d0dc00e2461947ade2ec02c3c605b03109142a 100644 (file)
--- a/test/pleroma/web/plugs/http_signature_plug_test.exs
+++ b/test/pleroma/web/plugs/http_signature_plug_test.exs
@@ -108,10 +108,6 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do
         |> HTTPSignaturePlug.call(%{})
 
       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
 
@@ -125,17 +121,12 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do
         |> HTTPSignaturePlug.call(%{})
 
       assert conn.assigns.valid_signature == true
-      assert conn.halted == false
       assert called(HTTPSignatures.validate_conn(:_))
     end
 
     test "and halts the connection when `signature` header is not present", %{conn: conn} do
       conn = HTTPSignaturePlug.call(conn, %{})
       assert conn.assigns[:valid_signature] == nil
-      assert conn.halted == true
-      assert conn.status == 401
-      assert conn.state == :sent
-      assert conn.resp_body == "Request not signed"
     end
   end