From e73c404427e7ba431030ae552ec45674e2924dbc Mon Sep 17 00:00:00 2001 From: Jens Alm Date: Thu, 30 May 2013 10:52:13 +0200 Subject: [PATCH 1/5] Added support for signed webhooks See http://help.mandrill.com/entries/23704122-Authenticating-webhook-request s --- .gitignore | 1 + djrill/tests/test_mandrill_webhook.py | 34 +++++++++++++++++++++++ djrill/views.py | 39 ++++++++++++++++++++++++++- docs/history.rst | 1 + docs/usage/webhooks.rst | 11 +++++++- 5 files changed, 84 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index c7de6dd..eb75239 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +.idea/ .DS_Store ._* *.pyc diff --git a/djrill/tests/test_mandrill_webhook.py b/djrill/tests/test_mandrill_webhook.py index c4fd707..39fafee 100644 --- a/djrill/tests/test_mandrill_webhook.py +++ b/djrill/tests/test_mandrill_webhook.py @@ -1,3 +1,6 @@ +from base64 import b64encode +import hashlib +import hmac import json from django.test import TestCase @@ -39,6 +42,37 @@ class DjrillWebhookSecretMixinTests(TestCase): self.assertEqual(response.status_code, 200) +class DjrillWebhookSignatureMixinTests(TestCase): + """ + Test mixin used in optional Mandrill webhook signature support + """ + + def setUp(self): + settings.DJRILL_WEBHOOK_SECRET = 'abc123' + settings.DJRILL_WEBHOOK_SIGNATURE_KEY = "signature" + settings.DJRILL_WEBHOOK_URL = "/webhook/?secret=abc123" + + def test_incorrect_settings(self): + del settings.DJRILL_WEBHOOK_URL + with self.assertRaises(ImproperlyConfigured): + self.client.post('/webhook/?secret=abc123') + settings.DJRILL_WEBHOOK_URL = "/webhook/?secret=abc123" + + def test_unauthorized(self): + settings.DJRILL_WEBHOOK_SIGNATURE_KEY = "anothersignature" + response = self.client.post(settings.DJRILL_WEBHOOK_URL) + self.assertEqual(response.status_code, 403) + + def test_signature(self): + signature = hmac.new(key=settings.DJRILL_WEBHOOK_SIGNATURE_KEY, msg = settings.DJRILL_WEBHOOK_URL+"mandrill_events[]", digestmod=hashlib.sha1) + hash_string = b64encode(signature.digest()) + response = self.client.post('/webhook/?secret=abc123', data={"mandrill_events":"[]"}, **{"X-Mandrill-Signature" : hash_string}) + self.assertEqual(response.status_code, 200) + + def tearDown(self): + del settings.DJRILL_WEBHOOK_SIGNATURE_KEY + del settings.DJRILL_WEBHOOK_URL + class DjrillWebhookViewTests(TestCase): """ Test optional Mandrill webhook view diff --git a/djrill/views.py b/djrill/views.py index 8a420cf..87f449a 100644 --- a/djrill/views.py +++ b/djrill/views.py @@ -1,9 +1,13 @@ +from base64 import b64encode +import hashlib +import hmac import json from django import forms from django.conf import settings from django.contrib import messages from django.core.exceptions import ImproperlyConfigured +from django.core.urlresolvers import reverse from django.views.generic import TemplateView, View from django.http import HttpResponse from django.utils.decorators import method_decorator @@ -100,6 +104,39 @@ class DjrillWebhookSecretMixin(object): return super(DjrillWebhookSecretMixin, self).dispatch( request, *args, **kwargs) +class DjrillWebhookSignatureMixin(object): + + @method_decorator(csrf_exempt) + def dispatch(self, request, *args, **kwargs): + + signature_key = getattr(settings, 'DJRILL_WEBHOOK_SIGNATURE_KEY', None) + + if signature_key and request.method == "POST": + + # Make webhook url an explicit setting to make sure that this is the exact same string + # that the user entered in Mandrill + post_string = getattr(settings, "DJRILL_WEBHOOK_URL", None) + if post_string is None: + raise ImproperlyConfigured( + "You have set DJRILL_WEBHOOK_SIGNATURE_KEY, but haven't set DJRILL_WEBHOOK_URL in the settings file.") + + signature = request.META.get("X-Mandrill-Signature", None) + if not signature: + return HttpResponse(status=403, content=u"X-Mandrill-Signature not set") + + # The querydict is a bit special, see https://docs.djangoproject.com/en/dev/ref/request-response/#querydict-objects + # Mandrill needs it to be sorted and added to the hash + post_lists = sorted(request.POST.lists()) + for value_list in post_lists: + for item in value_list[1]: + post_string += u"%s%s" % (value_list[0],item) + + hash_string = b64encode(hmac.new(key=signature_key, msg=post_string, digestmod=hashlib.sha1).digest()) + if signature != hash_string: + return HttpResponse(status=403, content=u"Signature doesn't match") + + return super(DjrillWebhookSignatureMixin, self).dispatch( + request, *args, **kwargs) class DjrillIndexView(DjrillApiMixin, TemplateView): template_name = "djrill/status.html" @@ -161,7 +198,7 @@ class DjrillUrlListView(DjrillAdminMedia, DjrillApiMixin, return self.render_to_response(context) -class DjrillWebhookView(DjrillWebhookSecretMixin, View): +class DjrillWebhookView(DjrillWebhookSecretMixin,DjrillWebhookSignatureMixin, View): def head(self, request, *args, **kwargs): return HttpResponse() diff --git a/docs/history.rst b/docs/history.rst index b3ce8e7..b04785a 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -3,6 +3,7 @@ Release Notes Version 0.6 (development): +* Support for signed webhooks Version 0.5: diff --git a/docs/usage/webhooks.rst b/docs/usage/webhooks.rst index 8874cf5..0cde41d 100644 --- a/docs/usage/webhooks.rst +++ b/docs/usage/webhooks.rst @@ -29,10 +29,13 @@ Your code can connect to this signal for further processing. app and Mandrill. Djrill will verify calls to your webhook, and will reject calls without the correct key. + * You can, optionally include the two settings :setting:`DJRILL_WEBHOOK_SIGNATURE_KEY` + and :setting:`DJRILL_WEBHOOK_URL` to enforce webhook signature checking + .. _Mandrill webhooks: http://help.mandrill.com/entries/21738186-Introduction-to-Webhooks .. _securing webhooks: http://apidocs.mailchimp.com/webhooks/#securing-webhooks - +.. _webhook signatures: http://help.mandrill.com/entries/23704122-Authenticating-webhook-requests .. _webhooks-config: @@ -97,6 +100,12 @@ the url config in step 2. And if you'd like to change the *name* of the "secret" query string parameter, you can set :setting:`DJRILL_WEBHOOK_SECRET_NAME` in your :file:`settings.py`. +For extra security, Mandrill provides a signature in the request header +X-Mandrill-Signature. If you want to verify this signature, you need to provide +the settings :setting:`DJRILL_WEBHOOK_SIGNATURE_KEY` with the webhook-specific +signature key that can be found in the Mandrill admin panel and +:setting:`DJRILL_WEBHOOK_URL` where you should enter the exact URL, including +that you entered in Mandrill when creating the webhook. .. _webhooks control panel: https://mandrillapp.com/settings/webhooks .. _inbound settings: https://mandrillapp.com/inbound From bbfaf2c8d8ce6b8d45a933e37157d3daa1b82f3b Mon Sep 17 00:00:00 2001 From: Jens Alm Date: Thu, 30 May 2013 10:57:34 +0200 Subject: [PATCH 2/5] Fixed python 3.2 unicode issues --- djrill/views.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/djrill/views.py b/djrill/views.py index 87f449a..63cf390 100644 --- a/djrill/views.py +++ b/djrill/views.py @@ -122,18 +122,18 @@ class DjrillWebhookSignatureMixin(object): signature = request.META.get("X-Mandrill-Signature", None) if not signature: - return HttpResponse(status=403, content=u"X-Mandrill-Signature not set") + return HttpResponse(status=403, content="X-Mandrill-Signature not set") # The querydict is a bit special, see https://docs.djangoproject.com/en/dev/ref/request-response/#querydict-objects # Mandrill needs it to be sorted and added to the hash post_lists = sorted(request.POST.lists()) for value_list in post_lists: for item in value_list[1]: - post_string += u"%s%s" % (value_list[0],item) + post_string += "%s%s" % (value_list[0],item) hash_string = b64encode(hmac.new(key=signature_key, msg=post_string, digestmod=hashlib.sha1).digest()) if signature != hash_string: - return HttpResponse(status=403, content=u"Signature doesn't match") + return HttpResponse(status=403, content="Signature doesn't match") return super(DjrillWebhookSignatureMixin, self).dispatch( request, *args, **kwargs) From 4e81e5d5e83141ca386a27b20d2e61297a098cd2 Mon Sep 17 00:00:00 2001 From: Jens Alm Date: Thu, 30 May 2013 11:21:05 +0200 Subject: [PATCH 3/5] Added byte/str compatibility for python 3 --- djrill/compat.py | 10 ++++++++++ djrill/tests/test_mandrill_webhook.py | 3 ++- djrill/views.py | 6 ++---- 3 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 djrill/compat.py diff --git a/djrill/compat.py b/djrill/compat.py new file mode 100644 index 0000000..e389065 --- /dev/null +++ b/djrill/compat.py @@ -0,0 +1,10 @@ +# For python 3 compatibility, see http://python3porting.com/problems.html#nicer-solutions +import sys + +if sys.version < '3': + def b(x): + return x +else: + import codecs + def b(x): + return codecs.latin_1_encode(x)[0] \ No newline at end of file diff --git a/djrill/tests/test_mandrill_webhook.py b/djrill/tests/test_mandrill_webhook.py index 39fafee..1b3943c 100644 --- a/djrill/tests/test_mandrill_webhook.py +++ b/djrill/tests/test_mandrill_webhook.py @@ -7,6 +7,7 @@ from django.test import TestCase from django.core.exceptions import ImproperlyConfigured from django.conf import settings +from ..compat import b from ..signals import webhook_event @@ -64,7 +65,7 @@ class DjrillWebhookSignatureMixinTests(TestCase): self.assertEqual(response.status_code, 403) def test_signature(self): - signature = hmac.new(key=settings.DJRILL_WEBHOOK_SIGNATURE_KEY, msg = settings.DJRILL_WEBHOOK_URL+"mandrill_events[]", digestmod=hashlib.sha1) + signature = hmac.new(key=b(settings.DJRILL_WEBHOOK_SIGNATURE_KEY), msg = b(settings.DJRILL_WEBHOOK_URL+"mandrill_events[]"), digestmod=hashlib.sha1) hash_string = b64encode(signature.digest()) response = self.client.post('/webhook/?secret=abc123', data={"mandrill_events":"[]"}, **{"X-Mandrill-Signature" : hash_string}) self.assertEqual(response.status_code, 200) diff --git a/djrill/views.py b/djrill/views.py index 63cf390..04f132d 100644 --- a/djrill/views.py +++ b/djrill/views.py @@ -2,12 +2,10 @@ from base64 import b64encode import hashlib import hmac import json - from django import forms from django.conf import settings from django.contrib import messages from django.core.exceptions import ImproperlyConfigured -from django.core.urlresolvers import reverse from django.views.generic import TemplateView, View from django.http import HttpResponse from django.utils.decorators import method_decorator @@ -16,7 +14,7 @@ from django.views.decorators.csrf import csrf_exempt import requests from djrill import MANDRILL_API_URL, signals - +from .compat import b class DjrillAdminMedia(object): def _media(self): @@ -131,7 +129,7 @@ class DjrillWebhookSignatureMixin(object): for item in value_list[1]: post_string += "%s%s" % (value_list[0],item) - hash_string = b64encode(hmac.new(key=signature_key, msg=post_string, digestmod=hashlib.sha1).digest()) + hash_string = b64encode(hmac.new(key=b(signature_key), msg=b(post_string), digestmod=hashlib.sha1).digest()) if signature != hash_string: return HttpResponse(status=403, content="Signature doesn't match") From a0da0ea7138ba80f96727c2258a50b33a4bf31b5 Mon Sep 17 00:00:00 2001 From: Jens Alm Date: Thu, 30 May 2013 13:10:58 +0200 Subject: [PATCH 4/5] Actually handling the correct headers --- djrill/tests/test_mandrill_webhook.py | 2 +- djrill/views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/djrill/tests/test_mandrill_webhook.py b/djrill/tests/test_mandrill_webhook.py index 1b3943c..d2969e4 100644 --- a/djrill/tests/test_mandrill_webhook.py +++ b/djrill/tests/test_mandrill_webhook.py @@ -67,7 +67,7 @@ class DjrillWebhookSignatureMixinTests(TestCase): def test_signature(self): signature = hmac.new(key=b(settings.DJRILL_WEBHOOK_SIGNATURE_KEY), msg = b(settings.DJRILL_WEBHOOK_URL+"mandrill_events[]"), digestmod=hashlib.sha1) hash_string = b64encode(signature.digest()) - response = self.client.post('/webhook/?secret=abc123', data={"mandrill_events":"[]"}, **{"X-Mandrill-Signature" : hash_string}) + response = self.client.post('/webhook/?secret=abc123', data={"mandrill_events":"[]"}, **{"HTTP_X_MANDRILL_SIGNATURE" : hash_string}) self.assertEqual(response.status_code, 200) def tearDown(self): diff --git a/djrill/views.py b/djrill/views.py index 04f132d..eb60320 100644 --- a/djrill/views.py +++ b/djrill/views.py @@ -118,7 +118,7 @@ class DjrillWebhookSignatureMixin(object): raise ImproperlyConfigured( "You have set DJRILL_WEBHOOK_SIGNATURE_KEY, but haven't set DJRILL_WEBHOOK_URL in the settings file.") - signature = request.META.get("X-Mandrill-Signature", None) + signature = request.META.get("HTTP_X_MANDRILL_SIGNATURE", None) if not signature: return HttpResponse(status=403, content="X-Mandrill-Signature not set") From 807f38a24016715ac360df80a60df2e7b3e731d1 Mon Sep 17 00:00:00 2001 From: Jens Alm Date: Sun, 2 Jun 2013 11:11:37 +0200 Subject: [PATCH 5/5] Cleaned up .gitignor, PEP-8 issues and documentation --- .gitignore | 1 - djrill/compat.py | 1 + djrill/views.py | 7 +++++-- docs/usage/webhooks.rst | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index eb75239..c7de6dd 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ -.idea/ .DS_Store ._* *.pyc diff --git a/djrill/compat.py b/djrill/compat.py index e389065..9c22d16 100644 --- a/djrill/compat.py +++ b/djrill/compat.py @@ -6,5 +6,6 @@ if sys.version < '3': return x else: import codecs + def b(x): return codecs.latin_1_encode(x)[0] \ No newline at end of file diff --git a/djrill/views.py b/djrill/views.py index eb60320..3e76b85 100644 --- a/djrill/views.py +++ b/djrill/views.py @@ -16,6 +16,7 @@ import requests from djrill import MANDRILL_API_URL, signals from .compat import b + class DjrillAdminMedia(object): def _media(self): js = ["js/core.js", "js/jquery.min.js", "js/jquery.init.js"] @@ -102,6 +103,7 @@ class DjrillWebhookSecretMixin(object): return super(DjrillWebhookSecretMixin, self).dispatch( request, *args, **kwargs) + class DjrillWebhookSignatureMixin(object): @method_decorator(csrf_exempt) @@ -127,7 +129,7 @@ class DjrillWebhookSignatureMixin(object): post_lists = sorted(request.POST.lists()) for value_list in post_lists: for item in value_list[1]: - post_string += "%s%s" % (value_list[0],item) + post_string += "%s%s" % (value_list[0], item) hash_string = b64encode(hmac.new(key=b(signature_key), msg=b(post_string), digestmod=hashlib.sha1).digest()) if signature != hash_string: @@ -136,6 +138,7 @@ class DjrillWebhookSignatureMixin(object): return super(DjrillWebhookSignatureMixin, self).dispatch( request, *args, **kwargs) + class DjrillIndexView(DjrillApiMixin, TemplateView): template_name = "djrill/status.html" @@ -196,7 +199,7 @@ class DjrillUrlListView(DjrillAdminMedia, DjrillApiMixin, return self.render_to_response(context) -class DjrillWebhookView(DjrillWebhookSecretMixin,DjrillWebhookSignatureMixin, View): +class DjrillWebhookView(DjrillWebhookSecretMixin, DjrillWebhookSignatureMixin, View): def head(self, request, *args, **kwargs): return HttpResponse() diff --git a/docs/usage/webhooks.rst b/docs/usage/webhooks.rst index 0cde41d..a5cf750 100644 --- a/docs/usage/webhooks.rst +++ b/docs/usage/webhooks.rst @@ -30,12 +30,12 @@ Your code can connect to this signal for further processing. reject calls without the correct key. * You can, optionally include the two settings :setting:`DJRILL_WEBHOOK_SIGNATURE_KEY` - and :setting:`DJRILL_WEBHOOK_URL` to enforce webhook signature checking + and :setting:`DJRILL_WEBHOOK_URL` to enforce `webhook signature`_ checking .. _Mandrill webhooks: http://help.mandrill.com/entries/21738186-Introduction-to-Webhooks .. _securing webhooks: http://apidocs.mailchimp.com/webhooks/#securing-webhooks -.. _webhook signatures: http://help.mandrill.com/entries/23704122-Authenticating-webhook-requests +.. _webhook signature: http://help.mandrill.com/entries/23704122-Authenticating-webhook-requests .. _webhooks-config: