diff --git a/djstripe/checks.py b/djstripe/checks.py index 16eef51d1c..0f69ccc47e 100644 --- a/djstripe/checks.py +++ b/djstripe/checks.py @@ -143,6 +143,40 @@ def check_webhook_secret(app_configs=None, **kwargs): return messages +@checks.register("djstripe") +def check_webhook_validation(app_configs=None, **kwargs): + """ + Check that DJSTRIPE_WEBHOOK_VALIDATION is valid + """ + from . import settings as djstripe_settings + + messages = [] + + validation_options = ("verify_signature", "retrieve_event") + + if djstripe_settings.WEBHOOK_VALIDATION is None: + messages.append(checks.Warning( + "Webhook validation is disabled, this is a security risk if the webhook view is enabled", + hint="Set DJSTRIPE_WEBHOOK_VALIDATION to one of {}".format(", ".join(validation_options)), + id="djstripe.W004" + )) + elif djstripe_settings.WEBHOOK_VALIDATION == "verify_signature": + if not djstripe_settings.WEBHOOK_SECRET: + messages.append(checks.Critical( + "DJSTRIPE_WEBHOOK_VALIDATION='verify_signature' but DJSTRIPE_WEBHOOK_SECRET is not set", + hint="Set DJSTRIPE_WEBHOOK_SECRET or set DJSTRIPE_WEBHOOK_VALIDATION='retrieve_event'", + id="djstripe.C006" + )) + elif djstripe_settings.WEBHOOK_VALIDATION not in validation_options: + messages.append(checks.Critical( + "DJSTRIPE_WEBHOOK_VALIDATION is invalid", + hint="Set DJSTRIPE_WEBHOOK_VALIDATION to one of {} or None".format(", ".join(validation_options)), + id="djstripe.C007" + )) + + return messages + + @checks.register("djstripe") def check_subscriber_key_length(app_configs=None, **kwargs): """ diff --git a/djstripe/models/webhooks.py b/djstripe/models/webhooks.py index 54f547aa0a..a81e155dce 100644 --- a/djstripe/models/webhooks.py +++ b/djstripe/models/webhooks.py @@ -145,8 +145,10 @@ def validate(self, api_key=None): logger.info("Test webhook received: {}".format(local_data)) return False - # If the DJSTRIPE_WEBHOOK_SECRET setting is set, use signature verification - if djstripe_settings.WEBHOOK_SECRET: + if djstripe_settings.WEBHOOK_VALIDATION is None: + # validation disabled + return True + elif djstripe_settings.WEBHOOK_VALIDATION == "verify_signature" and djstripe_settings.WEBHOOK_SECRET: try: stripe.WebhookSignature.verify_header( self.body, diff --git a/djstripe/settings.py b/djstripe/settings.py index 36d92356ee..b383ff10ab 100644 --- a/djstripe/settings.py +++ b/djstripe/settings.py @@ -70,6 +70,7 @@ def _get_idempotency_key(object_type, action, livemode): DJSTRIPE_WEBHOOK_URL = getattr(settings, "DJSTRIPE_WEBHOOK_URL", r"^webhook/$") WEBHOOK_TOLERANCE = getattr(settings, "DJSTRIPE_WEBHOOK_TOLERANCE", stripe.Webhook.DEFAULT_TOLERANCE) +WEBHOOK_VALIDATION = getattr(settings, "DJSTRIPE_WEBHOOK_VALIDATION", "verify_signature") WEBHOOK_SECRET = getattr(settings, "DJSTRIPE_WEBHOOK_SECRET", "") # Webhook event callbacks allow an application to take control of what happens diff --git a/docs/reference/settings.rst b/docs/reference/settings.rst index f968cc19df..3436fd17db 100644 --- a/docs/reference/settings.rst +++ b/docs/reference/settings.rst @@ -208,6 +208,9 @@ If this is set to a non-empty value, webhook signatures will be verified. .. _Learn more about webhook signature verification: https://stripe.com/docs/webhooks/signatures +DJSTRIPE_WEBHOOK_VALIDATION= (="verify_signature") +================================================== + This setting controls which type of validation is done on webhooks. Value can be ``"verify_signature"`` for signature verification (recommended default), ``"retrieve_event"`` for event retrieval (makes an extra HTTP diff --git a/runtests.py b/runtests.py index 165a92baf0..f7a7e5799f 100644 --- a/runtests.py +++ b/runtests.py @@ -147,7 +147,8 @@ def run_test_suite(args): ), DJSTRIPE_USE_NATIVE_JSONFIELD=os.environ.get("USE_NATIVE_JSONFIELD", "") == "1", DJSTRIPE_SUBSCRIPTION_REDIRECT="test_url_subscribe", - DJSTRIPE_WEBHOOK_VALIDATION="retrieve_event", + DJSTRIPE_WEBHOOK_VALIDATION="verify_signature", + DJSTRIPE_WEBHOOK_SECRET="whsec_XXXXX", ) # Avoid AppRegistryNotReady exception diff --git a/tests/test_webhooks.py b/tests/test_webhooks.py index 7808ba33d4..1ce81bc97a 100644 --- a/tests/test_webhooks.py +++ b/tests/test_webhooks.py @@ -10,6 +10,7 @@ import warnings from collections import defaultdict from copy import deepcopy +from importlib import reload from unittest.mock import Mock, PropertyMock, call, patch from django.test import TestCase, override_settings @@ -31,6 +32,10 @@ def mock_webhook_handler(webhook_event_trigger): class TestWebhook(TestCase): + + def tearDown(self): + reload(djstripe_settings) + def _send_event(self, event_data): return Client().post( reverse("djstripe:webhook"), @@ -44,51 +49,81 @@ def test_webhook_test_event(self): self.assertEqual(resp.status_code, 200) self.assertFalse(Event.objects.filter(id=TEST_EVENT_ID).exists()) + @override_settings(DJSTRIPE_WEBHOOK_VALIDATION="retrieve_event") @patch("stripe.Transfer.retrieve", return_value=deepcopy(FAKE_TRANSFER)) @patch("stripe.Event.retrieve", return_value=deepcopy(FAKE_EVENT_TRANSFER_CREATED)) def test_webhook_retrieve_event_fail(self, event_retrieve_mock, transfer_retrieve_mock): + reload(djstripe_settings) + invalid_event = deepcopy(FAKE_EVENT_TRANSFER_CREATED) invalid_event["id"] = "evt_invalid" invalid_event["data"]["valid"] = "not really" - djstripe_settings.WEBHOOK_SECRET = "" resp = self._send_event(invalid_event) self.assertEqual(resp.status_code, 400) self.assertFalse(Event.objects.filter(id="evt_invalid").exists()) + @override_settings(DJSTRIPE_WEBHOOK_VALIDATION="retrieve_event") @patch("stripe.Transfer.retrieve", return_value=deepcopy(FAKE_TRANSFER)) @patch("stripe.Event.retrieve", return_value=deepcopy(FAKE_EVENT_TRANSFER_CREATED)) def test_webhook_retrieve_event_pass(self, event_retrieve_mock, transfer_retrieve_mock): - djstripe_settings.WEBHOOK_SECRET = "" + reload(djstripe_settings) + resp = self._send_event(FAKE_EVENT_TRANSFER_CREATED) self.assertEqual(resp.status_code, 200) + event_retrieve_mock.assert_called_once_with( + api_key=djstripe_settings.STRIPE_SECRET_KEY, + id=FAKE_EVENT_TRANSFER_CREATED["id"] + ) - @override_settings(DJSTRIPE_WEBHOOK_VALIDATION="verify_signature") + @override_settings(DJSTRIPE_WEBHOOK_VALIDATION="verify_signature", DJSTRIPE_WEBHOOK_SECRET="whsec_XXXXX") @patch("stripe.Transfer.retrieve", return_value=deepcopy(FAKE_TRANSFER)) @patch("stripe.Event.retrieve", return_value=deepcopy(FAKE_EVENT_TRANSFER_CREATED)) def test_webhook_invalid_verify_signature_fail(self, event_retrieve_mock, transfer_retrieve_mock): + reload(djstripe_settings) + invalid_event = deepcopy(FAKE_EVENT_TRANSFER_CREATED) invalid_event["id"] = "evt_invalid" invalid_event["data"]["valid"] = "not really" - djstripe_settings.WEBHOOK_SECRET = "whsec_XXXXX" resp = self._send_event(invalid_event) self.assertEqual(resp.status_code, 400) self.assertFalse(Event.objects.filter(id="evt_invalid").exists()) + @override_settings(DJSTRIPE_WEBHOOK_VALIDATION="verify_signature", DJSTRIPE_WEBHOOK_SECRET="whsec_XXXXX") + @patch("stripe.WebhookSignature.verify_header", return_value=True) @patch("stripe.Transfer.retrieve", return_value=deepcopy(FAKE_TRANSFER)) @patch("stripe.Event.retrieve", return_value=deepcopy(FAKE_EVENT_TRANSFER_CREATED)) - @patch("stripe.WebhookSignature.verify_header", return_value=True) def test_webhook_verify_signature_pass(self, event_retrieve_mock, transfer_retrieve_mock, verify_signature_mock): - djstripe_settings.WEBHOOK_SECRET = "whsec_XXXXX" + reload(djstripe_settings) + resp = self._send_event(FAKE_EVENT_TRANSFER_CREATED) self.assertEqual(resp.status_code, 200) self.assertFalse(Event.objects.filter(id="evt_invalid").exists()) verify_signature_mock.called_once_with(FAKE_EVENT_TRANSFER_CREATED, {}) + event_retrieve_mock.assert_not_called() + + @override_settings(DJSTRIPE_WEBHOOK_VALIDATION=None) + @patch("stripe.WebhookSignature.verify_header") + @patch("stripe.Transfer.retrieve", return_value=deepcopy(FAKE_TRANSFER)) + @patch("stripe.Event.retrieve", return_value=deepcopy(FAKE_EVENT_TRANSFER_CREATED)) + def test_webhook_no_validation_pass(self, event_retrieve_mock, transfer_retrieve_mock, verify_signature_mock): + reload(djstripe_settings) + + invalid_event = deepcopy(FAKE_EVENT_TRANSFER_CREATED) + invalid_event["id"] = "evt_invalid" + invalid_event["data"]["valid"] = "not really" + + resp = self._send_event(invalid_event) + + self.assertEqual(resp.status_code, 200) + self.assertTrue(Event.objects.filter(id="evt_invalid").exists()) + event_retrieve_mock.assert_not_called() + verify_signature_mock.assert_not_called() def test_webhook_no_signature(self): self.assertEqual(WebhookEventTrigger.objects.count(), 0)