Add verification to Stripe webhook endpoint

app/controllers/stripe/webhooks_controller.rb

@@ -3,26 +3,35 @@ require 'stripe/webhook_handler'
 module Stripe
   class WebhooksController < BaseController
     protect_from_forgery except: :create
+    before_filter :verify_webhook
 
     # POST /stripe/webhook
     def create
-      # TODO is there a sensible way to confirm this webhook call is actually from Stripe?
-      event = Event.construct_from(params)
-      handler = WebhookHandler.new(event)
+      handler = WebhookHandler.new(@event)
       result = handler.handle
 
-      render nothing: true, status: status_mappings[result]
+      render nothing: true, status: status_mappings[result] || 200
     end
 
     private
 
+    def verify_webhook
+      payload = request.raw_post
+      signature = request.headers["HTTP_STRIPE_SIGNATURE"]
+      @event = Webhook.construct_event(payload, signature, Stripe.endpoint_secret)
+    rescue JSON::ParserError
+      render nothing: true, status: 400
+    rescue Stripe::SignatureVerificationError
+      render nothing: true, status: 401
+    end
+
     # Stripe interprets a 4xx or 3xx response as a failure to receive the webhook,
-    # and will stop sending events if too many of these are returned.
+    # and will stop sending events if too many of either of these are returned.
     def status_mappings
       {
-        success: 200,
-        failure: 202,
-        silent_fail: 204
+        success: 200, # The event was handled successfully
+        unknown: 202, # The event was of an unknown type
+        ignored: 204  # No action was taken in response to the event
       }
     end
   end

config/application.yml.example

@@ -29,8 +29,9 @@ CURRENCY: AUD
 
 # Stripe Connect details for instance account
 # Find these under 'API keys' and 'Connect' in your Stripe account dashboard -> Account Settings
-# Under 'Connect', the Redirect URI should be set to https://YOUR_SERVER_URL/stripe/callback (e.g. https://openfoodnetwork.org.uk/stripe/connect)
+# Under 'Connect', the Redirect URI should be set to https://YOUR_SERVER_URL/admin/stripe_accounts/connect_callback (e.g. https://openfoodnetwork.org.uk/admin/stripe_accounts/connect_callback)
 # Under 'Webhooks', you should set up a Connect endpoint pointing to https://YOUR_SERVER_URL/stripe/webhooks e.g. (https://openfoodnetwork.org.uk/stripe/webhooks)
 # STRIPE_INSTANCE_SECRET_KEY: "sk_test_xxxxxx" # This can be a test key or a live key
 # STRIPE_INSTANCE_PUBLISHABLE_KEY: "pk_test_xxxx" # This can be a test key or a live key
 # STRIPE_CLIENT_ID: "ca_xxxx" # This can be a development ID or a production ID
+# STRIPE_ENDPOINT_SECRET: "whsec_xxxx"

config/initializers/stripe.rb

@@ -1,13 +1,14 @@
-# Add the :publishable_key property, to allow us to access this
-# property from the object, rather than calling from ENV directly.
+# Add some additional properties, to allow us to access these
+# properties from the object, rather than calling from ENV directly.
 # This is mostly useful for stubbing when testing, but also feels
 # a bit cleaner than accessing keys in different ways.
 module Stripe
   class << self
-    attr_accessor :publishable_key
+    attr_accessor :publishable_key, :endpoint_secret
   end
 end
 
 Stripe.api_key = ENV['STRIPE_INSTANCE_SECRET_KEY']
 Stripe.publishable_key = ENV['STRIPE_INSTANCE_PUBLISHABLE_KEY']
 Stripe.client_id = ENV['STRIPE_CLIENT_ID']
+Stripe.endpoint_secret = ENV['STRIPE_ENDPOINT_SECRET']

spec/controllers/stripe/webhooks_controller_spec.rb

@@ -13,34 +13,62 @@ describe Stripe::WebhooksController do
       }
     end
 
-    context "when an event with an unknown type is received" do
-      it "responds with a 202" do
+    context "when invalid json is provided" do
+      before do
+        allow(Stripe::Webhook).to receive(:construct_event).and_raise JSON::ParserError, "parsing failed"
+      end
+
+      it "responds with a 400" do
         post 'create', params
-        expect(response.status).to eq 202
+        expect(response.status).to eq 400
       end
     end
 
-    describe "when an account.application.deauthorized event is received" do
-      let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") }
+    context "when event signature verification fails" do
       before do
-        params["type"] = "account.application.deauthorized"
+        allow(Stripe::Webhook).to receive(:construct_event).and_raise Stripe::SignatureVerificationError.new("verfication failed", "header")
       end
 
-      context "when the stripe_account id on the event does not match any known accounts" do
-        it "doesn't delete any Stripe accounts, responds with 204" do
+      it "responds with a 401" do
+        post 'create', params
+        expect(response.status).to eq 401
+      end
+    end
+
+    context "when event signature verification succeeds" do
+      before do
+        allow(Stripe::Webhook).to receive(:construct_event) { Stripe::Event.construct_from(params) }
+      end
+
+      context "when an event with an unknown type is received" do
+        it "responds with a 202" do
           post 'create', params
-          expect(response.status).to eq 204
-          expect(StripeAccount.all).to include stripe_account
+          expect(response.status).to eq 202
         end
       end
 
-      context "when the stripe_account id on the event matches a known account" do
-        before { params["account"] = "webhook_id" }
+      describe "when an account.application.deauthorized event is received" do
+        let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") }
+        before do
+          params["type"] = "account.application.deauthorized"
+        end
 
-        it "deletes Stripe accounts in response to a webhook" do
-          post 'create', params
-          expect(response.status).to eq 200
-          expect(StripeAccount.all).not_to include stripe_account
+        context "when the stripe_account id on the event does not match any known accounts" do
+          it "doesn't delete any Stripe accounts, responds with 204" do
+            post 'create', params
+            expect(response.status).to eq 204
+            expect(StripeAccount.all).to include stripe_account
+          end
+        end
+
+        context "when the stripe_account id on the event matches a known account" do
+          before { params["account"] = "webhook_id" }
+
+          it "deletes Stripe accounts in response to a webhook" do
+            post 'create', params
+            expect(response.status).to eq 200
+            expect(StripeAccount.all).not_to include stripe_account
+          end
         end
       end
     end