Skip to content

Commit

Permalink
Expose Stripe::Webhook.compute_signature (#915)
Browse files Browse the repository at this point in the history
Exposes the `.compute_signature` method, which may be useful when
testing webhook signing in test suites.

I change the API slightly so that a caller isn't forced to do as much
string mangling, and to match the one that we already have in stripe-go:

``` go
func ComputeSignature(t time.Time, payload []byte, secret string) []byte {
```

Add basic documentation and test case. I also change a few things around
so that we send `Time` objects around more often where applicable, and
don't change then to Unix integers until the last moment that we need
to.

The one other alternative API I considered is this one, which would
default the timestamp to the current time to allow the method to be
called with one fewer arg:

``` ruby
def self.compute_signature(payload, secret: timestamp: Time.now)
```

I decided against it in the end though because it does remove some
explicitness, and it's not a big deal to just pass in `Time.now`,
especially given that this is not expected to be a commonly used method.

Fixes #912.
  • Loading branch information
brandur committed Apr 24, 2020
1 parent 1b68611 commit e117c9f
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 12 deletions.
28 changes: 21 additions & 7 deletions lib/stripe/webhook.rb
Expand Up @@ -24,18 +24,28 @@ def self.construct_event(payload, sig_header, secret,
module Signature
EXPECTED_SCHEME = "v1"

def self.compute_signature(payload, secret)
OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload)
# Computes a webhook signature given a time (probably the current time),
# a payload, and a signing secret.
def self.compute_signature(timestamp, payload, secret)
raise ArgumentError, "timestamp should be an instance of Time" \
unless timestamp.is_a?(Time)
raise ArgumentError, "payload should be a string" \
unless payload.is_a?(String)
raise ArgumentError, "secret should be a string" \
unless secret.is_a?(String)

timestamped_payload = "#{timestamp.to_i}.#{payload}"
OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret,
timestamped_payload)
end
private_class_method :compute_signature

# Extracts the timestamp and the signature(s) with the desired scheme
# from the header
def self.get_timestamp_and_signatures(header, scheme)
list_items = header.split(/,\s*/).map { |i| i.split("=", 2) }
timestamp = Integer(list_items.select { |i| i[0] == "t" }[0][1])
signatures = list_items.select { |i| i[0] == scheme }.map { |i| i[1] }
[timestamp, signatures]
[Time.at(timestamp), signatures]
end
private_class_method :get_timestamp_and_signatures

Expand All @@ -53,6 +63,11 @@ def self.verify_header(payload, header, secret, tolerance: nil)
begin
timestamp, signatures =
get_timestamp_and_signatures(header, EXPECTED_SCHEME)

# TODO: Try to knock over this blanket rescue as it can unintentionally
# swallow many valid errors. Instead, try to validate an incoming
# header one piece at a time, and error with a known exception class if
# any part is found to be invalid. Rescue that class here.
rescue StandardError
raise SignatureVerificationError.new(
"Unable to extract timestamp and signatures from header",
Expand All @@ -67,16 +82,15 @@ def self.verify_header(payload, header, secret, tolerance: nil)
)
end

signed_payload = "#{timestamp}.#{payload}"
expected_sig = compute_signature(signed_payload, secret)
expected_sig = compute_signature(timestamp, payload, secret)
unless signatures.any? { |s| Util.secure_compare(expected_sig, s) }
raise SignatureVerificationError.new(
"No signatures found matching the expected signature for payload",
header, http_body: payload
)
end

if tolerance && timestamp < Time.now.to_f - tolerance
if tolerance && timestamp < Time.now - tolerance
raise SignatureVerificationError.new(
"Timestamp outside the tolerance zone (#{Time.at(timestamp)})",
header, http_body: payload
Expand Down
27 changes: 22 additions & 5 deletions test/stripe/webhook_test.rb
Expand Up @@ -13,12 +13,29 @@ class WebhookTest < Test::Unit::TestCase
SECRET = "whsec_test_secret"

def generate_header(opts = {})
opts[:timestamp] ||= Time.now.to_i
opts[:timestamp] ||= Time.now
opts[:payload] ||= EVENT_PAYLOAD
opts[:secret] ||= SECRET
opts[:scheme] ||= Stripe::Webhook::Signature::EXPECTED_SCHEME
opts[:signature] ||= Stripe::Webhook::Signature.send(:compute_signature, "#{opts[:timestamp]}.#{opts[:payload]}", opts[:secret])
"t=#{opts[:timestamp]},#{opts[:scheme]}=#{opts[:signature]}"
opts[:signature] ||= Stripe::Webhook::Signature.compute_signature(
opts[:timestamp],
opts[:payload],
opts[:secret]
)
"t=#{opts[:timestamp].to_i},#{opts[:scheme]}=#{opts[:signature]}"
end

context ".compute_signature" do
should "compute a signature which can then be verified" do
timestamp = Time.now
signature = Stripe::Webhook::Signature.compute_signature(
timestamp,
EVENT_PAYLOAD,
SECRET
)
header = generate_header(timestamp: timestamp, signature: signature)
assert(Stripe::Webhook::Signature.verify_header(EVENT_PAYLOAD, header, SECRET))
end
end

context ".construct_event" do
Expand Down Expand Up @@ -70,7 +87,7 @@ def generate_header(opts = {})
end

should "raise a SignatureVerificationError when the timestamp is not within the tolerance" do
header = generate_header(timestamp: Time.now.to_i - 15)
header = generate_header(timestamp: Time.now - 15)
e = assert_raises(Stripe::SignatureVerificationError) do
Stripe::Webhook::Signature.verify_header(EVENT_PAYLOAD, header, SECRET, tolerance: 10)
end
Expand All @@ -88,7 +105,7 @@ def generate_header(opts = {})
end

should "return true when the header contains a valid signature and the timestamp is off but no tolerance is provided" do
header = generate_header(timestamp: 12_345)
header = generate_header(timestamp: Time.at(12_345))
assert(Stripe::Webhook::Signature.verify_header(EVENT_PAYLOAD, header, SECRET))
end
end
Expand Down

0 comments on commit e117c9f

Please sign in to comment.