From: Ariadne Conill <ariadne@dereferenced.org>
Date: Thu, 18 Jul 2019 15:06:58 +0000 (+0000)
Subject: http signature plug: remove redundant checks handled by HTTPSignatures library
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=88d064d80e4a3272a2a7101089b5f924fd175866;p=akkoma

http signature plug: remove redundant checks handled by HTTPSignatures library

the redundant checks assumed a POST request, which will not work for signed GETs.
this check was originally needed because the HTTPSignatures adapter assumed that
the requests were also POST requests.  but now, the adapter has been corrected.
---

diff --git a/lib/pleroma/plugs/http_signature.ex b/lib/pleroma/plugs/http_signature.ex
index e2874c469..d87fa52fa 100644
--- a/lib/pleroma/plugs/http_signature.ex
+++ b/lib/pleroma/plugs/http_signature.ex
@@ -3,7 +3,6 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
-  alias Pleroma.Web.ActivityPub.Utils
   import Plug.Conn
   require Logger
 
@@ -16,38 +15,30 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
   end
 
   def call(conn, _opts) do
-    user = Utils.get_ap_id(conn.params["actor"])
-    Logger.debug("Checking sig for #{user}")
     [signature | _] = get_req_header(conn, "signature")
 
-    cond do
-      signature && String.contains?(signature, user) ->
-        # set (request-target) header to the appropriate value
-        # we also replace the digest header with the one we computed
-        conn =
-          conn
-          |> put_req_header(
-            "(request-target)",
-            String.downcase("#{conn.method}") <> " #{conn.request_path}"
-          )
-
-        conn =
-          if conn.assigns[:digest] do
-            conn
-            |> put_req_header("digest", conn.assigns[:digest])
-          else
-            conn
-          end
-
-        assign(conn, :valid_signature, HTTPSignatures.validate_conn(conn))
+    if signature do
+      # set (request-target) header to the appropriate value
+      # we also replace the digest header with the one we computed
+      conn =
+        conn
+        |> put_req_header(
+          "(request-target)",
+          String.downcase("#{conn.method}") <> " #{conn.request_path}"
+        )
 
-      signature ->
-        Logger.debug("Signature not from actor")
-        assign(conn, :valid_signature, false)
+      conn =
+        if conn.assigns[:digest] do
+          conn
+          |> put_req_header("digest", conn.assigns[:digest])
+        else
+          conn
+        end
 
-      true ->
-        Logger.debug("No signature header!")
-        conn
+      assign(conn, :valid_signature, HTTPSignatures.validate_conn(conn))
+    else
+      Logger.debug("No signature header!")
+      conn
     end
   end
 end
diff --git a/test/plugs/http_signature_plug_test.exs b/test/plugs/http_signature_plug_test.exs
index efd811df7..d6fd9ea81 100644
--- a/test/plugs/http_signature_plug_test.exs
+++ b/test/plugs/http_signature_plug_test.exs
@@ -26,22 +26,4 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlugTest do
       assert called(HTTPSignatures.validate_conn(:_))
     end
   end
-
-  test "bails out early if the signature isn't by the activity actor" do
-    params = %{"actor" => "https://mst3k.interlinked.me/users/luciferMysticus"}
-    conn = build_conn(:get, "/doesntmattter", params)
-
-    with_mock HTTPSignatures, validate_conn: fn _ -> false end do
-      conn =
-        conn
-        |> put_req_header(
-          "signature",
-          "keyId=\"http://mastodon.example.org/users/admin#main-key"
-        )
-        |> HTTPSignaturePlug.call(%{})
-
-      assert conn.assigns.valid_signature == false
-      refute called(HTTPSignatures.validate_conn(:_))
-    end
-  end
 end