From 88d064d80e4a3272a2a7101089b5f924fd175866 Mon Sep 17 00:00:00 2001
From: Ariadne Conill <ariadne@dereferenced.org>
Date: Thu, 18 Jul 2019 15:06:58 +0000
Subject: [PATCH] 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.
---
 lib/pleroma/plugs/http_signature.ex     | 51 ++++++++++---------------
 test/plugs/http_signature_plug_test.exs | 18 ---------
 2 files changed, 21 insertions(+), 48 deletions(-)

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 @@ def call(%{assigns: %{valid_signature: true}} = conn, _opts) 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))
-
-      signature ->
-        Logger.debug("Signature not from actor")
-        assign(conn, :valid_signature, false)
-
-      true ->
-        Logger.debug("No signature header!")
+    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}"
+        )
+
+      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))
+    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 @@ test "it call HTTPSignatures to check validity if the actor sighed it" 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