From 8ec38ffe7b95aeb9207fd8b883cd5a897ee86d34 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Sun, 22 Feb 2015 14:18:26 -0800 Subject: [PATCH 1/9] Add tests --- spec/controllers/webhook_controller_spec.rb | 33 +++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/spec/controllers/webhook_controller_spec.rb b/spec/controllers/webhook_controller_spec.rb index cf137a8..9a45d74 100644 --- a/spec/controllers/webhook_controller_spec.rb +++ b/spec/controllers/webhook_controller_spec.rb @@ -51,4 +51,37 @@ def webhook(params) expect { webhook id: 'evt_charge_succeeded' }.to raise_error(Stripe::StripeError, /testing/) end + + context "with an authentication secret" do + def webhook(secret, params) + if secret + request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials('user', secret) + end + super params + end + + before(:each) { StripeEvent.authentication_secret = "secret" } + after(:each) { StripeEvent.authentication_secret = nil } + + it "rejects requests with no secret" do + stub_event('evt_charge_succeeded') + + webhook nil, id: 'evt_charge_succeeded' + expect(response.code).to eq '401' + end + + it "rejects requests with incorrect secret" do + stub_event('evt_charge_succeeded') + + webhook 'incorrect', id: 'evt_charge_succeeded' + expect(response.code).to eq '401' + end + + it "accepts requests with correct secret" do + stub_event('evt_charge_succeeded') + + webhook 'secret', id: 'evt_charge_succeeded' + expect(response.code).to eq '200' + end + end end From 774dd7622b03e4baafab9ef24dc92b4e2cc9cd7f Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Sun, 22 Feb 2015 14:18:41 -0800 Subject: [PATCH 2/9] Add authentication secret attribute --- lib/stripe_event.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stripe_event.rb b/lib/stripe_event.rb index 71eb6e6..75995ce 100644 --- a/lib/stripe_event.rb +++ b/lib/stripe_event.rb @@ -4,7 +4,7 @@ module StripeEvent class << self - attr_accessor :adapter, :backend, :event_retriever, :namespace + attr_accessor :adapter, :backend, :event_retriever, :namespace, :authentication_secret def configure(&block) raise ArgumentError, "must provide a block" unless block_given? From 50e4ba07f2965e7cad80627e321428f376da5784 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Sun, 22 Feb 2015 14:22:56 -0800 Subject: [PATCH 3/9] Actually test authentication secret --- app/controllers/stripe_event/webhook_controller.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/controllers/stripe_event/webhook_controller.rb b/app/controllers/stripe_event/webhook_controller.rb index 79ac7cc..aac4a94 100644 --- a/app/controllers/stripe_event/webhook_controller.rb +++ b/app/controllers/stripe_event/webhook_controller.rb @@ -1,5 +1,12 @@ module StripeEvent class WebhookController < ActionController::Base + before_action do + StripeEvent.authentication_secret.nil? or \ + authenticate_or_request_with_http_basic do |username, password| + password == StripeEvent.authentication_secret + end + end + def event StripeEvent.instrument(params) head :ok From 665a1851a1251d94229d2a1326830fd018f98aba Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Sun, 22 Feb 2015 14:48:52 -0800 Subject: [PATCH 4/9] Add README section on using basic authentication --- README.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/README.md b/README.md index 41a96da..fec46ff 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,29 @@ StripeEvent.subscribe 'customer.card.' do |event| end ``` +## Securing your webhook endpoint + +StripeEvent automatically fetches events from Stripe to ensure they haven't been forged. However, that doesn't prevent an attacker who knows your endpoint name and an event's ID from forcing your server to process a legitimate event twice. If that event triggers some useful action, like generating a license key or enabling a delinquent account, you could end up giving something the attacker is supposed to pay for away for free. + +To prevent this, StripeEvent supports using HTTP Basic authentication on your webhook endpoint. If only Stripe knows the basic authentication password, this ensures that the request really comes from Stripe. Here's what you do: + +1. Arrange for a secret key to be available in your application's environment variables or `secrets.yml` file. You can generate a suitable secret with the `rake secret` command. (Remember, the `secrets.yml` file shouldn't contain production secrets directly; it should use ERB to include them.) + +2. Configure StripeEvent to require that secret be used as a basic authentication password, using code along the lines of these examples: + +```ruby +# STRIPE_WEBHOOK_SECRET environment variable +StripeEvent.authentication_secret = ENV['STRIPE_WEBHOOK_SECRET'] +# stripe_webhook_secret key in secrets.yml file +StripeEvent.authentication_secret = Rails.application.secrets.stripe_webhook_secret +``` + +3. When you specify your webhook's URL in Stripe's settings, include the secret as a password in the URL, along with any username: + + https://stripe:my-secret-key@myapplication.com/my-webhook-path + +This is only truly secure if your webhook endpoint is accessed over SSL, which Stripe strongly recommends anyway. + ## Configuration If you have built an application that has multiple Stripe accounts--say, each of your customers has their own--you may want to define your own way of retrieving events from Stripe (e.g. perhaps you want to use the [user_id parameter](https://stripe.com/docs/apps/getting-started#webhooks) from the top level to detect the customer for the event, then grab their specific API key). You can do this: From 323e75d44cea590a0a796760514cf02982f85105 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Sun, 22 Feb 2015 14:51:11 -0800 Subject: [PATCH 5/9] (Hopefully) fix URL formatting --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fec46ff..a0bab70 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ StripeEvent.authentication_secret = Rails.application.secrets.stripe_webhook_sec 3. When you specify your webhook's URL in Stripe's settings, include the secret as a password in the URL, along with any username: - https://stripe:my-secret-key@myapplication.com/my-webhook-path + https://stripe:my-secret-key@myapplication.com/my-webhook-path This is only truly secure if your webhook endpoint is accessed over SSL, which Stripe strongly recommends anyway. From dc0f9625c582d2eb4734de0d83f0a784fae3f172 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Sun, 22 Feb 2015 14:52:02 -0800 Subject: [PATCH 6/9] More README formatting fixes --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index a0bab70..0bdcf28 100644 --- a/README.md +++ b/README.md @@ -80,12 +80,12 @@ To prevent this, StripeEvent supports using HTTP Basic authentication on your we 2. Configure StripeEvent to require that secret be used as a basic authentication password, using code along the lines of these examples: -```ruby -# STRIPE_WEBHOOK_SECRET environment variable -StripeEvent.authentication_secret = ENV['STRIPE_WEBHOOK_SECRET'] -# stripe_webhook_secret key in secrets.yml file -StripeEvent.authentication_secret = Rails.application.secrets.stripe_webhook_secret -``` + ```ruby + # STRIPE_WEBHOOK_SECRET environment variable + StripeEvent.authentication_secret = ENV['STRIPE_WEBHOOK_SECRET'] + # stripe_webhook_secret key in secrets.yml file + StripeEvent.authentication_secret = Rails.application.secrets.stripe_webhook_secret + ``` 3. When you specify your webhook's URL in Stripe's settings, include the secret as a password in the URL, along with any username: From 15029c87311156a6d6e16fd30483a2fad45b11a8 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Sun, 22 Feb 2015 15:30:45 -0800 Subject: [PATCH 7/9] Improve Rails 3 compatibility --- app/controllers/stripe_event/webhook_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/stripe_event/webhook_controller.rb b/app/controllers/stripe_event/webhook_controller.rb index aac4a94..dcea6a6 100644 --- a/app/controllers/stripe_event/webhook_controller.rb +++ b/app/controllers/stripe_event/webhook_controller.rb @@ -1,6 +1,6 @@ module StripeEvent class WebhookController < ActionController::Base - before_action do + before_filter do StripeEvent.authentication_secret.nil? or \ authenticate_or_request_with_http_basic do |username, password| password == StripeEvent.authentication_secret From ee3a4174750c0ae6dbcc4058122830e733a6173e Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Mon, 23 Feb 2015 19:32:05 -0800 Subject: [PATCH 8/9] Fix style complaint --- app/controllers/stripe_event/webhook_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/stripe_event/webhook_controller.rb b/app/controllers/stripe_event/webhook_controller.rb index dcea6a6..4b1e4d4 100644 --- a/app/controllers/stripe_event/webhook_controller.rb +++ b/app/controllers/stripe_event/webhook_controller.rb @@ -1,10 +1,11 @@ module StripeEvent class WebhookController < ActionController::Base before_filter do - StripeEvent.authentication_secret.nil? or \ + if StripeEvent.authentication_secret authenticate_or_request_with_http_basic do |username, password| password == StripeEvent.authentication_secret end + end end def event From bed6d16b72743770b374147da5a2f6faf83fcff1 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Tue, 24 Feb 2015 20:59:20 -0800 Subject: [PATCH 9/9] Don't override webhook() in authentication tests Instead, add a webhook_with_secret() method. --- spec/controllers/webhook_controller_spec.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/spec/controllers/webhook_controller_spec.rb b/spec/controllers/webhook_controller_spec.rb index 9a45d74..a1e1628 100644 --- a/spec/controllers/webhook_controller_spec.rb +++ b/spec/controllers/webhook_controller_spec.rb @@ -53,11 +53,9 @@ def webhook(params) end context "with an authentication secret" do - def webhook(secret, params) - if secret - request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials('user', secret) - end - super params + def webhook_with_secret(secret, params) + request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials('user', secret) + webhook params end before(:each) { StripeEvent.authentication_secret = "secret" } @@ -66,21 +64,21 @@ def webhook(secret, params) it "rejects requests with no secret" do stub_event('evt_charge_succeeded') - webhook nil, id: 'evt_charge_succeeded' + webhook id: 'evt_charge_succeeded' expect(response.code).to eq '401' end it "rejects requests with incorrect secret" do stub_event('evt_charge_succeeded') - webhook 'incorrect', id: 'evt_charge_succeeded' + webhook_with_secret 'incorrect', id: 'evt_charge_succeeded' expect(response.code).to eq '401' end it "accepts requests with correct secret" do stub_event('evt_charge_succeeded') - webhook 'secret', id: 'evt_charge_succeeded' + webhook_with_secret 'secret', id: 'evt_charge_succeeded' expect(response.code).to eq '200' end end