Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authenticating webhooks #11

Open
beezee opened this issue Nov 24, 2014 · 4 comments
Open

Authenticating webhooks #11

beezee opened this issue Nov 24, 2014 · 4 comments
Assignees
Milestone

Comments

@beezee
Copy link

beezee commented Nov 24, 2014

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
Copy link
Owner

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
Copy link
Author

beezee commented Nov 24, 2014

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
Copy link
Owner

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.

@wingrunr21 wingrunr21 added this to the v1.2 milestone Nov 18, 2015
@wingrunr21 wingrunr21 self-assigned this Nov 18, 2015
@jufemaiz
Copy link

Did any work commence on this?

If not, I will have a look into it.

Cheers, Joel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants