Simplified HTTP signature processing

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

lib/pleroma/web/o_auth/scopes.ex

@@ -61,7 +61,7 @@ def to_string(scopes), do: Enum.join(scopes, " ")
   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
 

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

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 @@ def call(%{assigns: %{valid_signature: true}} = conn, _opts) 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 @@ defp maybe_require_signature(
     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),

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

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

test/pleroma/web/plugs/http_signature_plug_test.exs

@@ -108,10 +108,6 @@ test "and signature is present and incorrect", %{conn: conn} 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 @@ test "and signature is correct", %{conn: conn} 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