From 1d252ca4122bfc9c09cc43d90f4e136eba0929a3 Mon Sep 17 00:00:00 2001 From: medmunds Date: Tue, 6 Nov 2018 18:10:59 -0800 Subject: [PATCH] Mailgun: better errors for misconfigured webhooks Detect cases where inbound or tracking webhook url has been configured in wrong Mailgun webhook. See #129. --- anymail/webhooks/mailgun.py | 23 +++++++++++++++++++++- tests/test_mailgun_inbound.py | 36 +++++++++++++++++++++++++++++++++- tests/test_mailgun_webhooks.py | 14 +++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/anymail/webhooks/mailgun.py b/anymail/webhooks/mailgun.py index 5506ad4..ec4ab51 100644 --- a/anymail/webhooks/mailgun.py +++ b/anymail/webhooks/mailgun.py @@ -7,7 +7,7 @@ from django.utils.crypto import constant_time_compare from django.utils.timezone import utc from .base import AnymailBaseWebhookView -from ..exceptions import AnymailWebhookValidationFailure, AnymailInvalidAddress +from ..exceptions import AnymailConfigurationError, AnymailWebhookValidationFailure, AnymailInvalidAddress from ..inbound import AnymailInboundMessage from ..signals import inbound, tracking, AnymailInboundEvent, AnymailTrackingEvent, EventType, RejectReason from ..utils import get_anymail_setting, combine, querydict_getfirst, parse_single_address @@ -200,6 +200,12 @@ class MailgunTrackingWebhookView(MailgunBaseWebhookView): # to avoid potential conflicting user-data. esp_event.getfirst = querydict_getfirst.__get__(esp_event) + if 'event' not in esp_event and 'sender' in esp_event: + # Inbound events don't (currently) have an event field + raise AnymailConfigurationError( + "You seem to have set Mailgun's *inbound* route " + "to Anymail's Mailgun *tracking* webhook URL.") + event_type = self.legacy_event_types.get(esp_event.getfirst('event'), EventType.UNKNOWN) timestamp = datetime.fromtimestamp(int(esp_event['timestamp']), tz=utc) # use *last* value of timestamp # Message-Id is not documented for every event, but seems to always be included. @@ -319,12 +325,27 @@ class MailgunInboundWebhookView(MailgunBaseWebhookView): signal = inbound def parse_events(self, request): + if request.content_type == "application/json": + esp_event = json.loads(request.body.decode('utf-8')) + event_type = esp_event.get('event-data', {}).get('event', '') + raise AnymailConfigurationError( + "You seem to have set Mailgun's *%s tracking* webhook " + "to Anymail's Mailgun *inbound* webhook URL. " + "(Or Mailgun has changed inbound events to use json.)" + % event_type) return [self.esp_to_anymail_event(request)] def esp_to_anymail_event(self, request): # Inbound uses the entire Django request as esp_event, because we need POST and FILES. # Note that request.POST is case-sensitive (unlike email.message.Message headers). esp_event = request + + if request.POST.get('event', 'inbound') != 'inbound': + # (Legacy) tracking event + raise AnymailConfigurationError( + "You seem to have set Mailgun's *%s tracking* webhook " + "to Anymail's Mailgun *inbound* webhook URL." % request.POST['event']) + if 'body-mime' in request.POST: # Raw-MIME message = AnymailInboundMessage.parse_raw_mime(request.POST['body-mime']) diff --git a/tests/test_mailgun_inbound.py b/tests/test_mailgun_inbound.py index 5540a06..8dc00d0 100644 --- a/tests/test_mailgun_inbound.py +++ b/tests/test_mailgun_inbound.py @@ -7,11 +7,14 @@ from django.test import override_settings from django.utils.timezone import utc from mock import ANY +from anymail.exceptions import AnymailConfigurationError from anymail.inbound import AnymailInboundMessage from anymail.signals import AnymailInboundEvent from anymail.webhooks.mailgun import MailgunInboundWebhookView -from .test_mailgun_webhooks import TEST_API_KEY, mailgun_sign_legacy_payload, querydict_to_postdict +from .test_mailgun_webhooks import ( + TEST_API_KEY, mailgun_sign_payload, + mailgun_sign_legacy_payload, querydict_to_postdict) from .utils import sample_image_content, sample_email_content from .webhook_cases import WebhookTestCase @@ -174,3 +177,34 @@ class MailgunInboundTestCase(WebhookTestCase): self.assertEqual(message.subject, 'Raw MIME test') self.assertEqual(message.text, u"It's a body\N{HORIZONTAL ELLIPSIS}\n") self.assertEqual(message.html, u"""
It's a body\N{HORIZONTAL ELLIPSIS}
\n""") + + def test_misconfigured_tracking(self): + raw_event = mailgun_sign_payload({ + "event-data": { + "event": "clicked", + "timestamp": 1534109600.089676, + "recipient": "recipient@example.com", + "url": "https://example.com/test" + } + }) + with self.assertRaisesMessage( + AnymailConfigurationError, + "You seem to have set Mailgun's *clicked tracking* webhook" + " to Anymail's Mailgun *inbound* webhook URL." + ): + self.client.post('/anymail/mailgun/inbound/', + data=json.dumps(raw_event), content_type='application/json') + + def test_misconfigured_tracking_legacy(self): + raw_event = mailgun_sign_legacy_payload({ + 'domain': 'example.com', + 'message-headers': '[]', + 'recipient': 'recipient@example.com', + 'event': 'delivered', + }) + with self.assertRaisesMessage( + AnymailConfigurationError, + "You seem to have set Mailgun's *delivered tracking* webhook" + " to Anymail's Mailgun *inbound* webhook URL." + ): + self.client.post('/anymail/mailgun/inbound/', data=raw_event) diff --git a/tests/test_mailgun_webhooks.py b/tests/test_mailgun_webhooks.py index 7aa898c..e8d3254 100644 --- a/tests/test_mailgun_webhooks.py +++ b/tests/test_mailgun_webhooks.py @@ -8,6 +8,7 @@ from django.test import override_settings from django.utils.timezone import utc from mock import ANY +from anymail.exceptions import AnymailConfigurationError from anymail.signals import AnymailTrackingEvent from anymail.webhooks.mailgun import MailgunTrackingWebhookView @@ -698,3 +699,16 @@ class MailgunLegacyTestCase(WebhookTestCase): kwargs = self.assert_handler_called_once_with(self.tracking_handler) event = kwargs['event'] self.assertEqual(event.tags, ["tag1", "tag2"]) + + def test_misconfigured_inbound(self): + raw_event = mailgun_sign_legacy_payload({ + 'recipient': 'test@inbound.example.com', + 'sender': 'envelope-from@example.org', + 'message-headers': '[]', + 'body-plain': 'Test body plain', + 'body-html': '
Test body html
', + }) + + errmsg = "You seem to have set Mailgun's *inbound* route to Anymail's Mailgun *tracking* webhook URL." + with self.assertRaisesMessage(AnymailConfigurationError, errmsg): + self.client.post('/anymail/mailgun/tracking/', data=raw_event)