Add webhooks support #239
Add webhooks support #239
Conversation
|
@wearemd In case you're interested, I contributed the same PR to sikanhe/stripe-elixir#9. |
|
@jayjun Thanks ! |
|
@jayjun what do you thin about backporting signature verification into the 1.x branch? It's a stable branch and signature verification is an extremely high priority security concern. Otherwise, any SaaS app using StripityStripe is a potential exploitation target. |
|
@lessless I don’t mind but my recent PRs have been languishing in unmerged purgatory. stripe-elixir’s maintainer is far more responsive so I’ve moved across. The same webhooks code is already merged and published to Hex there. |
|
According to the opened issues, I assume that @begedin @gmile and @joshsmith are core team members and hope that they will back that effort up! |
|
@jayjun I pinged guys on the slack and @joshsmith said that he will try to review this PR on the next week. |
|
@lessless No worries, I’ve submitted the pull request that backports webhooks. |
lib/stripe/webhook.ex
Outdated
| def construct_event(payload, signature_header, secret, tolerance \\ @default_tolerance) do | ||
| case verify_header(payload, signature_header, secret, tolerance) do | ||
| :ok -> | ||
| {:ok, Poison.decode!(payload)} |
There was a problem hiding this comment.
The documentation here specifies the function will return an {:ok, %Stripe.Event{}}, but it's actually returning an {:ok, %{}}. The map keys are atoms, but it is not an event struct.
We would have to return {:ok, Stripe.Converter.convert_result(payload)} for the output to match the documentation.
Is there a reason you went with Poison.decode!/1 instead? I ask because it's certainly possible our Stripe.Event struct does not map something it should. If that's the case, in the interest of merging this, I'm ok with you updating the documentation instead, so it specifies it's returning a map, and then submitting an issue if the struct has a problem, which we can work on.
There was a problem hiding this comment.
Sorry about that, I fixed the return value (yes, it must return a Stripe.Event) but got sidetracked at work. Changes pushed now.
I only went with Poison.decode!/1 because other parts of this library do the same. I think that’s a reasonable assumption because invalid JSON from Stripe would be exceptional.
Official Stripe libraries provide a function to verify webhook signatures and convert its payload into Event objects.
This PR adds just one public function,
that can be used in plugs to handle Stripe webhooks, for example.
Also, I added my implementation of constant-time string comparison to prevent timing attacks (as recommended by Stripe's documentation). See
Stripe.Util.secure_equals?/2.