From 3d6d7de267be1fad2d2221bdfbd72c5c10b16397 Mon Sep 17 00:00:00 2001 From: Chase Pursley Date: Wed, 27 Dec 2023 07:53:43 -0500 Subject: [PATCH] Standard Webhooks: Take 3. --- .../authentication/standard_webhook.ex | 62 ++++++++++++++----- test/authentication/standard_webhook_test.exs | 18 +++++- 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/lib/webhoox/authentication/standard_webhook.ex b/lib/webhoox/authentication/standard_webhook.ex index e0ce13b..4699273 100644 --- a/lib/webhoox/authentication/standard_webhook.ex +++ b/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) diff --git a/test/authentication/standard_webhook_test.exs b/test/authentication/standard_webhook_test.exs index 2e95a30..15aefac 100644 --- a/test/authentication/standard_webhook_test.exs +++ b/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)