pinax / pinax-stripe-light

a payments Django app for Stripe
MIT License
684 stars 285 forks source link

Anti-pattern when instantiating Webhooks #380

Open alexstewartja opened 6 years ago

alexstewartja commented 6 years ago

PROBLEM STATEMENT

I'm creating an app that makes it seamless for Django REST Framework users to integrate pinax-stripe into their workflow.

While implementing the Webhook View, I find it impossible to create a new Webhook instance, as I'm being greeted with AttributeError: 'Webhook' object has no attribute 'name'.

This is clearly due to the fact that the name attribute is being referenced from the Webhook's __init__ method, yet the only initial parameter available is event.

Even after removing the first two lines (64-65) of discrepant code (shown below), line 81 sustains the AttributeError.

DISCREPANT CODE:

PROPOSED SOLUTION

Expunge the inconsonant lines of code and utilize the type attribute, which is sent on every webhook request from Stripe's servers. Use of the type attribute is an unerring way to map incoming webhooks to their respective Django signal.

So in point form:

And here's a snippet of code (redacted for brevity) that I feel should be the optimal approach to broadcasting a signal, from within a Webhook View, as a result of a successful incoming webhook event:

... event = stripe.Webhook.construct_event(payload=self.request.body, sig_header=self.request.META['HTTP_STRIPE_SIGNATURE'], secret='REDACTED_WEBHOOK_SECRET') ... Webhook(event=event).send_signal() return HttpResponse(status=200)

JUSTIFICATION

As is apparent from the code snippet above, constructing a signed event using the core stripe library means we can subsequently trust all attributes of that event, including type.

blueyed commented 6 years ago

Sounds good to me.

paltman commented 6 years ago

I’m not following you.

Events are created by populating the kind attribute with the type parameter sent in the webhook.

paltman commented 6 years ago

Ok, having come back to this (I know it's a long time overdue) and giving this more thought, what you propose @alexstewartja makes a lot of sense. I think the origination of this pattern stems from the early days fo stripe webhooks when they were NOT signed and I don't believe had the type attribute. A LOT has changed since this was django-stripe-payments and the webhook endpoint was just a public URL that accepted blind JSON posts.