Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions stripe/_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import time
from collections import OrderedDict
from hashlib import sha256
from typing import Optional

# Used for global variables
import stripe # noqa: IMP101
Expand Down Expand Up @@ -39,6 +40,41 @@ def construct_event(
)
return event

@staticmethod
def generate_test_header_string(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Did you consider Webhook.generate_test_header_string? stripe-node puts the helper on the primary webhooks object. I'm curious why you chose WebhookSignature instead of Webhook

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that Webhook is the better home. My original reasoning was internal cohesion (co-locating with _compute_signature and verify_header), but that's outweighed by:

  • Cross-SDK consistency. stripe-node, stripe-go, and stripe-ruby all put their test header helper on the primary webhooks object, so a user hopping between SDKs knows where to look.
  • Discovery via use case. The helper exists to feed Webhook.construct_event in tests; sitting next to it is a stronger signal than sitting next to the primitive it wraps.
  • Public-surface symmetry. Webhook is the user-facing entry point; WebhookSignature is closer to plumbing. A testing helper belongs on the surface being tested.

Moved it to Webhook.generate_test_header_string, with the implementation still delegating to WebhookSignature._compute_signature internally. Tests updated accordingly. Thanks for pushing back on this.

payload: str,
secret: str,
timestamp: Optional[int] = None,
scheme: Optional[str] = None,
signature: Optional[str] = None,
) -> str:
"""
Generates a value for the `Stripe-Signature` header that can be used
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring uses a mix of markdown and RST syntax. I would suggest sticking to markdown to be consistent with the rest of the code base. You can specify parameters as:

Args:
    payload: ...
    secret: ...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switched to a markdown Args: block. Thanks for the catch!

when testing code that calls `Webhook.construct_event` or
`WebhookSignature.verify_header`. Mirrors `generateTestHeaderString`
from stripe-node.

Args:
payload: The webhook payload to sign, as a string.
secret: The webhook signing secret (`whsec_...`).
timestamp: Unix timestamp to embed in the header. Defaults to
the current time.
scheme: Signature scheme. Defaults to
`WebhookSignature.EXPECTED_SCHEME`.
signature: Pre-computed signature to embed in the header. If
omitted, a signature is computed from `payload` and `secret`.
"""
if timestamp is None:
timestamp = int(time.time())
if scheme is None:
scheme = WebhookSignature.EXPECTED_SCHEME
if signature is None:
signed_payload = "%d.%s" % (timestamp, payload)
signature = WebhookSignature._compute_signature(
signed_payload, secret
)
return "t=%d,%s=%s" % (timestamp, scheme, signature)


class WebhookSignature(object):
EXPECTED_SCHEME = "v1"
Expand Down
70 changes: 58 additions & 12 deletions tests/test_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,13 @@


def generate_header(**kwargs):
timestamp = kwargs.get("timestamp", int(time.time()))
payload = kwargs.get("payload", DUMMY_WEBHOOK_PAYLOAD)
secret = kwargs.get("secret", DUMMY_WEBHOOK_SECRET)
scheme = kwargs.get("scheme", stripe.WebhookSignature.EXPECTED_SCHEME)
signature = kwargs.get("signature", None)
if signature is None:
payload_to_sign = "%d.%s" % (timestamp, payload)
signature = stripe.WebhookSignature._compute_signature(
payload_to_sign, secret
)
header = "t=%d,%s=%s" % (timestamp, scheme, signature)
return header
return stripe.Webhook.generate_test_header_string(
payload=kwargs.get("payload", DUMMY_WEBHOOK_PAYLOAD),
secret=kwargs.get("secret", DUMMY_WEBHOOK_SECRET),
timestamp=kwargs.get("timestamp"),
scheme=kwargs.get("scheme"),
signature=kwargs.get("signature"),
)


class TestWebhook(object):
Expand Down Expand Up @@ -149,6 +144,57 @@ def test_timestamp_off_but_no_tolerance(self):
)


class TestGenerateTestHeaderString(object):
def test_uses_defaults_when_optional_args_omitted(self):
before = int(time.time())
header = stripe.Webhook.generate_test_header_string(
payload=DUMMY_WEBHOOK_PAYLOAD, secret=DUMMY_WEBHOOK_SECRET
)
after = int(time.time())

parts = dict(item.split("=", 1) for item in header.split(","))
assert before <= int(parts["t"]) <= after
assert "v1" in parts

def test_header_verifies_round_trip(self):
header = stripe.Webhook.generate_test_header_string(
payload=DUMMY_WEBHOOK_PAYLOAD, secret=DUMMY_WEBHOOK_SECRET
)
assert stripe.WebhookSignature.verify_header(
DUMMY_WEBHOOK_PAYLOAD,
header,
DUMMY_WEBHOOK_SECRET,
tolerance=10,
)

def test_honors_custom_timestamp_and_scheme(self):
header = stripe.Webhook.generate_test_header_string(
payload=DUMMY_WEBHOOK_PAYLOAD,
secret=DUMMY_WEBHOOK_SECRET,
timestamp=12345,
scheme="v0",
)
assert header.startswith("t=12345,v0=")

def test_uses_provided_signature_verbatim(self):
header = stripe.Webhook.generate_test_header_string(
payload=DUMMY_WEBHOOK_PAYLOAD,
secret=DUMMY_WEBHOOK_SECRET,
timestamp=12345,
signature="deadbeef",
)
assert header == "t=12345,v1=deadbeef"

def test_bad_secret_fails_verification(self):
header = stripe.Webhook.generate_test_header_string(
payload=DUMMY_WEBHOOK_PAYLOAD, secret="whsec_wrong"
)
with pytest.raises(SignatureVerificationError):
stripe.WebhookSignature.verify_header(
DUMMY_WEBHOOK_PAYLOAD, header, DUMMY_WEBHOOK_SECRET
)


class TestStripeClientConstructEvent(object):
def test_construct_event(self, stripe_mock_stripe_client):
header = generate_header()
Expand Down