Standard Webhooks: Take 3.

lib/webhoox/authentication/standard_webhook.ex

@@ -8,10 +8,23 @@ defmodule Webhoox.Authentication.StandardWebhook do
   @secret_prefix "whsec_"
   @signature_identifier "v1"
   @tolerance 5 * 60
-  @now :os.system_time(:second)
+
+  @doc """
+  Verify a Standard Webhook given a Plug.Conn, payload and secret
+  """
+  @spec verify(Plug.Conn.t(), map(), binary()) :: boolean()
+  def verify(conn, payload, @secret_prefix <> encoded_secret) do
+    secret = Base.decode64!(encoded_secret)
+
+    verify_signature(conn, payload, secret)
+  end
 
   def verify(conn, payload, secret) do
-    required_headers?(conn)
+    verify_signature(conn, payload, secret)
+  end
+
+  defp verify_signature(conn, payload, secret) do
+    validate_headers(conn)
 
     [id] = get_req_header(conn, "webhook-id")
     [timestamp] = get_req_header(conn, "webhook-timestamp")
@@ -21,10 +34,10 @@ defmodule Webhoox.Authentication.StandardWebhook do
       sign(id, String.to_integer(timestamp), payload, secret)
       |> split_signature_from_identifier()
 
-    verify_signatures(signatures, signed_signature)
+    valid_signatures?(signatures, signed_signature)
   end
 
-  defp required_headers?(%{req_headers: req_headers}) do
+  defp validate_headers(%{req_headers: req_headers}) do
     required_headers = ["webhook-id", "webhook-timestamp", "webhook-signature"]
     filtered_headers = filter_headers(req_headers, required_headers)
 
@@ -52,9 +65,9 @@ defmodule Webhoox.Authentication.StandardWebhook do
     |> Enum.join(", ")
   end
 
-  defp verify_signatures([], _signed_signature), do: false
+  defp valid_signatures?([], _signature), do: false
 
-  defp verify_signatures(signatures, signature) when signature >= 1 do
+  defp valid_signatures?(signatures, signature) when signature >= 1 do
     signatures
     |> Enum.map(&split_signature_from_identifier/1)
     |> Enum.any?(&Plug.Crypto.secure_compare(&1, signature))
@@ -66,6 +79,31 @@ defmodule Webhoox.Authentication.StandardWebhook do
     |> List.last()
   end
 
+  def validate_timestamp(timestamp) do
+    now = :os.system_time(:second)
+
+    cond do
+      is_integer(timestamp) and timestamp > now + @tolerance ->
+        raise ArgumentError, message: "Message timestamp too new"
+
+      is_integer(timestamp) and timestamp < now - @tolerance ->
+        raise ArgumentError, message: "Message timestamp too old"
+
+      true ->
+        :ok
+    end
+  end
+
+  @doc """
+  Sign a Standard Webhook given an id, timestamp, payload and secret
+  """
+  @spec sign(
+          id :: String.t(),
+          timestamp :: integer(),
+          payload :: map(),
+          secret :: binary()
+        ) ::
+          String.t()
   def sign(id, _timestamp, _payload, _secret) when not is_binary(id) do
     raise ArgumentError, message: "Message id must be a string"
   end
@@ -74,16 +112,6 @@ defmodule Webhoox.Authentication.StandardWebhook do
     raise ArgumentError, message: "Message timestamp must be an integer"
   end
 
-  def sign(_id, timestamp, _payload, _secret)
-      when is_integer(timestamp) and timestamp < @now - @tolerance do
-    raise ArgumentError, message: "Message timestamp too old"
-  end
-
-  def sign(_id, timestamp, _payload, _secret)
-      when is_integer(timestamp) and timestamp > @now + @tolerance do
-    raise ArgumentError, message: "Message timestamp too new"
-  end
-
   def sign(_id, _timestamp, payload, _secret) when not is_map(payload) do
     raise ArgumentError, message: "Message payload must be a map"
   end
@@ -103,6 +131,8 @@ defmodule Webhoox.Authentication.StandardWebhook do
   end
 
   defp sign_with_version(id, timestamp, payload, secret) do
+    validate_timestamp(timestamp)
+
     signature =
       to_sign(id, timestamp, payload)
       |> sign_and_encode(secret)

test/authentication/standard_webhook_test.exs

@@ -27,14 +27,14 @@ defmodule Webhoox.Authentication.StandardWebhookTest do
 
     test "raises error when message timestamp is too old" do
       assert_raise ArgumentError, "Message timestamp too old", fn ->
-        timestamp = :os.system_time(:second) - @tolerance - 5000
+        timestamp = :os.system_time(:second) - @tolerance - 1
         Authentication.sign(@id, timestamp, @payload, @secret)
       end
     end
 
     test "raises error when message timestamp is too new" do
       assert_raise ArgumentError, "Message timestamp too new", fn ->
-        timestamp = :os.system_time(:second) + @tolerance + 5000
+        timestamp = :os.system_time(:second) + @tolerance + 1
         Authentication.sign(@id, timestamp, @payload, @secret)
       end
     end
@@ -79,12 +79,24 @@ defmodule Webhoox.Authentication.StandardWebhookTest do
       {:ok, signature: signature}
     end
 
-    test "return true when valid signature", %{signature: signature} do
+    test "return true when valid encoded_secret and signature", %{signature: signature} do
+      conn = setup_webhook(signature)
+
+      assert Authentication.verify(conn, @payload, @encoded_secret)
+    end
+
+    test "return true when valid secret and signature", %{signature: signature} do
       conn = setup_webhook(signature)
 
       assert Authentication.verify(conn, @payload, @secret)
     end
 
+    test "return false when valid secret and invalid signature" do
+      conn = setup_webhook("invalid signature")
+
+      assert false == Authentication.verify(conn, @payload, @secret)
+    end
+
     test "raises error when missing all required headers" do
       connection = conn(:post, "/_incoming", @payload)