wingrunr21 / griddler-mandrill

Mandrill adapter for Griddler
MIT License
16 stars 49 forks source link

Authenticating webhooks #11

Open beezee opened 9 years ago

beezee commented 9 years ago

Mandrill allows you to authenticate that a webook came from them.

I know I could override the controller and handle it there, but then the adapter I've chosen to use with Griddler starts to leak into other parts of my codebase and it increases the perceived friction of using this adapter.

Personally I'd prefer if the adapter checked an optional config for the key, and attempted to verify request signatures when config is present. I'm happy to open a PR with these changes if you're open to supporting that behavior.

Also, I'm curious what your preferred means of defining that config would be.

wingrunr21 commented 9 years ago

Well, any configuration for this would need to be done on the overarching Griddler config in the initializer. At the moment we'd need to re-open Griddler::Configuration and add the config value. It might be a better idea to allow Griddler to pass any un-consumed configuration values onto the adapters.

As far as the configuration itself, I think anything that responds to call would probably work. This would be called to obtain the expected signature value and compared with the incoming value.

We'd need to define a new exception or two, subclassed from Griddler::Error, within the Griddler::Errors module. One exception for an invalid configuration and another for a bad signature.

In the mean time, can you utilize the headers or raw_headers in your email processing class and run your checks that way?

@calebthompson any thoughts?

beezee commented 9 years ago

Yeah I suppose email processor is more concentrated than trying to overload controller, but it still couples my implementation to an adapter, which is less ideal compared to an optional config.

I'll wait for more feedback before writing any code for this.

wingrunr21 commented 9 years ago

If you are worried about coupling to the adapter in the short term, write your email processor sans-authentication then subclass that email processor and override process. Do your authentication logic then call super. Switching between them would be as simple as updating the Griddler config. That should get you going now and allow a relatively easy upgrade path later.

I am not against adding this to griddler-mandrill but it will take a bit of planning. Right now Griddler doesn't really allow for adapter-specific features.

jufemaiz commented 7 years ago

Did any work commence on this?

If not, I will have a look into it.

Cheers, Joel