Fixed #1765 -- Actually validate signature of stripe webook requests

djangoproject/settings/common.py

@@ -273,6 +273,7 @@
 STRIPE_PUBLISHABLE_KEY = SECRETS.get(
     "stripe_publishable_key", "pk_test_TyB5jcROwK8mlCNrn3dCwW7l"
 )
+STRIPE_ENDPOINT_SECRET = SECRETS.get("stripe_endpoint_secret", "insecure")
 
 # product IDs
 PRODUCTS = {

fundraising/tests/test_views.py

@@ -180,6 +180,18 @@ def test_past_donations_sorted(self):
         )
 
 
+def _stripe_signature_header(data):
+    """
+    Compute the `stripe-signature` header for the given data dict.
+    """
+    timestamp = int(datetime.now().timestamp())
+    payload = f"{timestamp}.{json.dumps(data)}"
+    signature = stripe.WebhookSignature._compute_signature(
+        payload, settings.STRIPE_ENDPOINT_SECRET
+    )
+    return f"t={timestamp},v1={signature}"
+
+
 class TestWebhooks(TestCase):
     def setUp(self):
         self.hero = DjangoHero.objects.create(email="[email protected]")
@@ -196,60 +208,76 @@ def stripe_data(self, filename):
             data = json.load(f)
             return stripe.util.convert_to_stripe_object(data, stripe.api_key, None)
 
-    def post_event(self):
+    def post_event(self, data):
         return self.client.post(
             reverse("fundraising:receive-webhook"),
-            data='{"id": "evt_12345"}',
+            data=json.dumps(data),
             content_type="application/json",
+            headers={
+                "stripe-signature": _stripe_signature_header(data),
+            },
         )
 
-    @patch("stripe.Event.retrieve")
-    def test_record_payment(self, event):
-        event.return_value = self.stripe_data("invoice_succeeded")
-        response = self.post_event()
+    def test_record_payment(self):
+        response = self.post_event(self.stripe_data("invoice_succeeded"))
         self.assertEqual(response.status_code, 201)
         self.assertEqual(self.donation.payment_set.count(), 1)
         payment = self.donation.payment_set.first()
         self.assertEqual(payment.amount, 10)
 
-    @patch("stripe.Event.retrieve")
-    def test_subscription_cancelled(self, event):
-        event.return_value = self.stripe_data("subscription_cancelled")
-        self.post_event()
+    def test_subscription_cancelled(self):
+        self.post_event(self.stripe_data("subscription_cancelled"))
         donation = Donation.objects.get(id=self.donation.id)
         self.assertEqual(donation.stripe_subscription_id, "")
         self.assertEqual(len(mail.outbox), 1)
         expected_url = django_hosts_reverse("fundraising:index")
         self.assertTrue(expected_url in mail.outbox[0].body)
 
-    @patch("stripe.Event.retrieve")
-    def test_payment_failed(self, event):
-        event.return_value = self.stripe_data("payment_failed")
-        self.post_event()
+    def test_payment_failed(self):
+        self.post_event(self.stripe_data("payment_failed"))
         self.assertEqual(len(mail.outbox), 1)
         expected_url = django_hosts_reverse(
             "fundraising:manage-donations", kwargs={"hero": self.hero.id}
         )
         self.assertTrue(expected_url in mail.outbox[0].body)
 
-    @patch("stripe.Event.retrieve")
-    def test_no_such_event(self, event):
-        event.side_effect = stripe.error.InvalidRequestError(
-            message="No such event: evt_12345", param="id"
-        )
-        response = self.post_event()
-        self.assertTrue(response.status_code, 422)
-
-    @patch("stripe.Event.retrieve")
-    def test_empty_object(self, event):
-        event.return_value = self.stripe_data("empty_payment")
-        response = self.post_event()
+    def test_empty_object(self):
+        response = self.post_event(self.stripe_data("empty_payment"))
         self.assertEqual(response.status_code, 422)
 
-    @patch("stripe.Event.retrieve")
-    def test_zero_invoice_amount(self, event):
+    def test_zero_invoice_amount(self):
         """Zero payment amounts don't need to be created."""
-        event.return_value = self.stripe_data("zero_invoice_amount")
-        response = self.post_event()
+        response = self.post_event(self.stripe_data("zero_invoice_amount"))
         self.assertEqual(response.status_code, 201)
         self.assertEqual(self.donation.payment_set.count(), 0)
+
+    def test_missing_signature_header(self):
+        response = self.client.post(
+            reverse("fundraising:receive-webhook"),
+            data=json.dumps({}),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 422)
+
+    def test_invalid_json(self):
+        response = self.client.post(
+            reverse("fundraising:receive-webhook"),
+            data="<invalid>",
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 422)
+
+    def test_invalid_signature(self):
+        response = self.client.post(
+            reverse("fundraising:receive-webhook"),
+            data=json.dumps({}),
+            content_type="application/json",
+            headers={"stripe-signature": "<invalid>"},
+        )
+        self.assertEqual(response.status_code, 422)
+
+    def test_unknown_event_type(self):
+        data = self.stripe_data("zero_invoice_amount")
+        data["type"] = "unknown"
+        response = self.post_event(data)
+        self.assertEqual(response.status_code, 422)

fundraising/views.py

@@ -1,5 +1,4 @@
 import decimal
-import json
 import logging
 
 import stripe
@@ -189,16 +188,13 @@ def cancel_donation(request, hero):
 @csrf_exempt
 def receive_webhook(request):
     try:
-        data = json.loads(request.body.decode())
-    except ValueError:
-        return HttpResponse(422)
-
-    # For security, re-request the event object from Stripe.
-    # TODO: Verify shared secret here?
-    try:
-        event = stripe.Event.retrieve(data["id"])
-    except stripe.error.InvalidRequestError:
-        return HttpResponse(422)
+        event = stripe.Webhook.construct_event(
+            request.body,
+            request.headers["stripe-signature"],
+            settings.STRIPE_ENDPOINT_SECRET,
+        )
+    except (KeyError, ValueError, stripe.error.SignatureVerificationError):
+        return HttpResponse(status=422)
 
     return WebhookHandler(event).handle()
 
@@ -214,7 +210,7 @@ def handle(self):
             "customer.subscription.deleted": self.subscription_cancelled,
             "checkout.session.completed": self.checkout_session_completed,
         }
-        handler = handlers.get(self.event.type, lambda: HttpResponse(422))
+        handler = handlers.get(self.event.type, lambda: HttpResponse(status=422))
         if not self.event.data.object:
             return HttpResponse(status=422)
         return handler()