Allow for controlled event rejection

[peterkeen]

This adds the ability to selectively reject an event. For example, if you want to prevent replay attacks you would set up your event retriever to record every event id that comes in and reject duplicates. Or, if you are still receiving events for connected Stripe accounts that are deactivated in your system, you could drop their events on the floor.

[coveralls]

Coverage remained the same when pulling 8c46a3437ea17da61d3a40c2f1318fce61dc079f on peterkeen:event-retriever-reject into b9c5e7fc34af597ffa0bdc2324ce38468939c157 on integrallis:master.

[rmm5t]

Why raise an error instead of just returning from each webhook subscription?

Also, wouldn't it be much better to centralize this and short-circuit everything in the event_retriever instead of the individual webhook subscriptions? The event_retriever would be better at sniffing for replay attacks or deactivated accounts anyway. It separates concerns much better than making every webhook subscription handle those cases.

In other words, I don't think the WebhookController should know anything about this special case.

Perhaps the implementation could be even simpler too. If the event_retriever returns nil instead of an event object, ignore the event, don't fire callbacks on the event subscriptions, and just let the WebhookController naturally return :ok.

The only thing we'd have to add is a conditional check (if event) when calling backend.instrument. For example:

  backend.instrument(namespace.call(event[:type]), event) if event

Thoughts?

[peterkeen]

Having event_retriever throw the exception was actually my intent but the test doesn't show that at all. I like your design better, but for some reason it seemed more awkward to test at the time. I'll implement the change.

[rmm5t]

Whoops. It looks like we criss-crossed some pull-requests. I just submitted #38. Same implementation, but we went about testing it in a different way. I wanted to also test and ensure that a 200 response code was still getting returned to stripe for these ignored events.

[coveralls]

Coverage remained the same when pulling 0d2cb8b9409e96e56bc9a441bb4a14931880f1dd on peterkeen:event-retriever-reject into b9c5e7fc34af597ffa0bdc2324ce38468939c157 on integrallis:master.

[peterkeen]

No worries. As long as it gets implemented somehow I'm happy :)

On Fri, Jul 11, 2014 at 9:28 AM, Ryan McGeary notifications@github.com wrote:

Whoops. It looks like we criss-crossed some pull-requests. I just submitted #38 https://github.com/integrallis/stripe_event/pull/38. Same implemetation, but we went about testing it in a different way. I wanted to also test and ensure that a 200 response code was still getting returned to stripe for these ignored events.

— Reply to this email directly or view it on GitHub https://github.com/integrallis/stripe_event/pull/36#issuecomment-48729721 .

[invisiblefunnel]

Thanks @peterkeen. Glad to have your input on this gem!