Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webhook: export ComputeSignature #602

Closed
wants to merge 1 commit into from
Closed

webhook: export ComputeSignature #602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 9 additions & 9 deletions webhook/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

const (
// Signatures older than this will be rejected by ConstructEvent
// DefaultTolerance signatures older than this will be rejected by ConstructEvent
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix here, but could you make this a sensible sentence? Like DefaultTolerance is the amount of drift in time that's allowed by ConstructEven as it's validating a signature.

DefaultTolerance time.Duration = 300 * time.Second
signingVersion string = "v1"
)
Expand All @@ -27,10 +27,10 @@ var (
ErrNoValidSignature error = errors.New("Webhook had no valid signature")
)

// Computes a webhook signature using Stripe's v1 signing method. See
// ComputeSignature computes a webhook signature using Stripe's v1 signing method. See
// https://stripe.com/docs/webhooks#signatures
func computeSignature(t time.Time, payload []byte, secret string) []byte {
mac := hmac.New(sha256.New, []byte(secret))
func ComputeSignature(t time.Time, payload, secret []byte) []byte {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for trying to follow the advice I was giving in #497!

Contrary to what I said there though, I now see that unfortunately changing the signature to []byte ends up breaking compatibility on a lot of these already-exported functions. It's also still not perfectly consistent because header in some of the functions below is still a string.

Sorry for the reversal, but for those reasons, I think it actually might be better to leave secret here as a string, at least for the time being. Would you mind changing it back?

mac := hmac.New(sha256.New, secret)
mac.Write([]byte(fmt.Sprintf("%d", t.Unix())))
mac.Write([]byte("."))
mac.Write(payload)
Expand Down Expand Up @@ -95,7 +95,7 @@ func parseSignatureHeader(header string) (*signedHeader, error) {
// your signing secret from the Stripe dashboard:
// https://dashboard.stripe.com/webhooks
//
func ConstructEvent(payload []byte, header string, secret string) (stripe.Event, error) {
func ConstructEvent(payload []byte, header string, secret []byte) (stripe.Event, error) {
return ConstructEventWithTolerance(payload, header, secret, DefaultTolerance)
}

Expand All @@ -109,7 +109,7 @@ func ConstructEvent(payload []byte, header string, secret string) (stripe.Event,
// your signing secret from the Stripe dashboard:
// https://dashboard.stripe.com/webhooks
//
func ConstructEventWithTolerance(payload []byte, header string, secret string, tolerance time.Duration) (stripe.Event, error) {
func ConstructEventWithTolerance(payload []byte, header string, secret []byte, tolerance time.Duration) (stripe.Event, error) {
return constructEvent(payload, header, secret, tolerance, true)
}

Expand All @@ -122,11 +122,11 @@ func ConstructEventWithTolerance(payload []byte, header string, secret string, t
// your signing secret from the Stripe dashboard:
// https://dashboard.stripe.com/webhooks
//
func ConstructEventIgnoringTolerance(payload []byte, header string, secret string) (stripe.Event, error) {
func ConstructEventIgnoringTolerance(payload []byte, header string, secret []byte) (stripe.Event, error) {
return constructEvent(payload, header, secret, 0*time.Second, false)
}

func constructEvent(payload []byte, sigHeader string, secret string, tolerance time.Duration, enforceTolerance bool) (stripe.Event, error) {
func constructEvent(payload []byte, sigHeader string, secret []byte, tolerance time.Duration, enforceTolerance bool) (stripe.Event, error) {
e := stripe.Event{}

if err := json.Unmarshal(payload, &e); err != nil {
Expand All @@ -138,7 +138,7 @@ func constructEvent(payload []byte, sigHeader string, secret string, tolerance t
return e, err
}

expectedSignature := computeSignature(header.timestamp, payload, secret)
expectedSignature := ComputeSignature(header.timestamp, payload, secret)
expiredTimestamp := time.Since(header.timestamp) > tolerance
if enforceTolerance && expiredTimestamp {
return e, ErrTooOld
Expand Down
2 changes: 1 addition & 1 deletion webhook/client_handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func Example() {
}

// Pass the request body & Stripe-Signature header to ConstructEvent, along with the webhook signing key
event, err := webhook.ConstructEvent(body, req.Header.Get("Stripe-Signature"), "whsec_DaLRHCRs35vEXqOE8uTEAXGLGUOnyaFf")
event, err := webhook.ConstructEvent(body, req.Header.Get("Stripe-Signature"), []byte("whsec_DaLRHCRs35vEXqOE8uTEAXGLGUOnyaFf"))

if err != nil {
w.WriteHeader(http.StatusBadRequest) // Return a 400 error on a bad signature
Expand Down
8 changes: 4 additions & 4 deletions webhook/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ var testPayload = []byte(`{
"id": "evt_test_webhook",
"object": "event"
}`)
var testSecret = "whsec_test_secret"
var testSecret = []byte("whsec_test_secret")

type SignedPayload struct {
timestamp time.Time
payload []byte
secret string
secret []byte
scheme string
signature []byte
header string
Expand All @@ -34,7 +34,7 @@ func newSignedPayload(options ...func(*SignedPayload)) *SignedPayload {
}

if signedPayload.signature == nil {
signedPayload.signature = computeSignature(signedPayload.timestamp, signedPayload.payload, signedPayload.secret)
signedPayload.signature = ComputeSignature(signedPayload.timestamp, signedPayload.payload, signedPayload.secret)
}
signedPayload.header = generateHeader(*signedPayload)
return signedPayload
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestTokenNew(t *testing.T) {

p = newSignedPayload()
p2 := newSignedPayload(func(p *SignedPayload) {
p.secret = testSecret + "_rolled_key"
p.secret = append(testSecret, []byte("_rolled_key")...)
})
headerWithRolledKey := p.header + ",v1=" + p2.hexSignature()
if p.hexSignature() == p2.hexSignature() {
Expand Down