search

evendis / mandrill-rails

Webhook processing and event decoration to make using Mandrill with Rails just that much easier
MIT License
288 stars 36 forks source link

Authenticate Mandrill Webhook Requests #3

Closed zshannon closed 11 years ago

zshannon commented 11 years ago

I added support for authenticating Mandrill's webhook requests using the protocol described here: http://help.mandrill.com/entries/23704122-Authenticating-webhook-requests

I bumped the version number (separate commit) to account for this improvement, and described how to use it in the README.

I did not add tests because I could not readily determine a test that would be sufficiently orthogonal to the functionality.

tardate commented 11 years ago

Thanks for this! I'd always been a bit concerned about the vulnerability of the webhook endpoints but hadn't caught up with the support that Mandrill now have for authenticating webhook requests.

I've merged in and added a very basic spec to verify the sig generation.

I think we'll probably need to change the configuration of the webhook keys, because we may not have the same key for different requests (in one app I have event callbacks using one key, and inbound mail processing using another). I'm going to have a quick look at that now before publishing a new gem version.

tardate commented 11 years ago

@zshannon note my refactor to handle multiple API keys. I also changed the way to configure the keys. Does this work for you? Let me know if you have any comments before I publish a new gem version (I'll probably do that +24hrs unless you have any further input).

zshannon commented 11 years ago

@tardate Handling multiple keys is nice! I would advise changing the helper method to one that ends in '!' to better betray that normal execution will stop if the signature is not verifiable. Also, it would be better if, in the case of multiple keys, keys were tied to specific events. Would you like me to mock that up in a pull request?

tardate commented 10 years ago

@zshannon Sure, happy took look at any other ideas you have. Have you been able to test and prove that this is working for you already?

NB: I initially thought about configuring keys per message type, but then realised it's possible (though not sure how likely) to have more than one key for a message type. I'm also not sure what Mandrill does about batching messages that would normally have different API keys if sent individually. So I took the simple way out and said "just give us all your keys and we'll see which works";-) From a security perspective, I don't believe there is any significant diminution of protection, as it only changes the probability of compromise from 1 in some billions to 2 or 3 in some billions.

zshannon commented 10 years ago

@tardate Have been testing all along in development using Forward, and am running the branch I created this pull request from in production, both with no issues. Agreed on the security implications of leaving the keys without message type scopes. I'll create a new pull request in a moment with the modified syntax I proposed above.