Basic authentication support

[beccadax]

Pursuant to #53, this branch adds basic authentication support to StripeEvent.

Basic authentication is supported in the simplest way possible:

1. The module's user sets a StripeEvent.authentication_secret attribute to their desired password.
2. If authentication_secret is set, a new before_action in WebhookController ensures that the request is appropriately authenticated, and returns a 401 otherwise.
3. If authentication_secret is not set, the module behaves as it does currently. All previously-existing tests pass unmodified.

This branch does not add a default way to determine the secret, a deprecation warning if the user doesn't specify a secret, or anything else of that sort. I'm by no means opposed to such things, but since this is my first pull request, I don't feel qualified to make those sorts of design decisions. It does, however, include new tests to ensure this feature is working as intended and a section in the README explaining how to enable it.

Suggestions welcome.

[coveralls]

Coverage remained the same at 100.0% when pulling dc0f962 on brentdax:feature/basic-auth into 742fd9f on integrallis:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 15029c8 on brentdax:feature/basic-auth into 742fd9f on integrallis:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 15029c8 on brentdax:feature/basic-auth into 742fd9f on integrallis:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 15029c8 on brentdax:feature/basic-auth into 742fd9f on integrallis:master.

[rmm5t]

Overall, decent looking pull-request, but could we move this portion to more idiomatic ruby?

Either:

authenticate_or_request_with_http_basic do |username, password|
  password == StripeEvent.authentication_secret
end if StripeEvent.authentication_secret

OR

if StripeEvent.authentication_secret
  authenticate_or_request_with_http_basic do |username, password|
    password == StripeEvent.authentication_secret
  end
end

[coveralls]

Coverage remained the same at 100.0% when pulling ee3a417 on brentdax:feature/basic-auth into 742fd9f on integrallis:master.

[invisiblefunnel]

Overriding webhook in this context reduces duplication but makes the tests a bit harder to reason about. I'd rather repeat request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials('user', secret) in each test for clarity.

[beccadax]

If overriding webhook is a concern, we could always rename the override to webhook_with_secret or even have a separate set_secret call.

[rmm5t]

I agree with @invisiblefunnel here. A webhook_with_secret should suffice and add clarity.

[invisiblefunnel]

👍

[invisiblefunnel]

This is a great start! Thanks for putting it together.

@rmm5t @peterkeen what do you think about specifying a secret vs. authenticate_with_http_basic?

[rmm5t]

@rmm5t @peterkeen what do you think about specifying a secret vs. authenticate_with_http_basic?

Good question. authenticate_with_http_basic certainly gives more flexibility, but I'm not sure what flexibility buys us here. Let's go for simplicity instead and take an opinionated approach about how the replay attack prevention should be handled. SSL/TLS + BasicAuth-Secret == Secure

If we don't keep this simple, it's less likely that people will implement this protective feature. I think that's more important.

Afterall, if we realize this a limitation, it's easier to add more flexibility and maintain the simpler backwards compatibility than remove it later.

[invisiblefunnel]

@rmm5t very good point about simplicity for the end user. Let's proceed with the authentication_secret approach. Things are coming together!

[coveralls]

Coverage remained the same at 100.0% when pulling bed6d16 on brentdax:feature/basic-auth into 742fd9f on integrallis:master.

[coveralls]

Coverage remained the same at 100.0% when pulling bed6d16 on brentdax:feature/basic-auth into 742fd9f on integrallis:master.

[rmm5t]

I think we're close enough to get this merged in. Any objections?

[invisiblefunnel]

Looks good to me. Thank you @brentdax @rmm5t!
@rmm5t I'd also like to add you as an author in the gemspec, sound good?

[rmm5t]

Sweet. Merging in 3...2...1

Great work @brentdax! Thanks for taking the lead on getting this PR going.

@invisiblefunnel Sure thing. I'm glad to handle the version bump and release too if you want to grant gem push rights. My rubygems email is the same as on my github profile. I'll be securing my webhook endpoints to use the new auth secret soon after.

[peterkeen]

Sorry I totally dropped off this thread, but I think this is a great addition to the project. Thanks @brentdax, @rmm5t, and @invisiblefunnel

[invisiblefunnel]

@rmm5t you're all set.

Thanks again everyone!

[rmm5t]

v1.5.0 is in the wild. 🍻

[invisiblefunnel]

Cheers!

[beccadax]

Yeah, I had to update my Gemfile twice in the span of a couple hours as the pull request was merged and then released. Thanks for working with me on this!