diff --git a/templates/zerver/api/webhook-walkthrough.md b/templates/zerver/api/webhook-walkthrough.md index 2dd8ba0ed3..d3694e211b 100644 --- a/templates/zerver/api/webhook-walkthrough.md +++ b/templates/zerver/api/webhook-walkthrough.md @@ -510,3 +510,28 @@ class QuerytestHookTests(WebhookTestCase): You can also override `get_body` if your test data needs to be constructed in an unusual way. For more, see the definition for the base class, `WebhookTestCase` in `zerver/lib/test_classes.py.` + + +### Custom HTTP event-type headers + +Some third-party services set a custom HTTP header to indicate the event type that +generates a particular payload. To extract such headers, we recommend using the +`validate_extract_webhook_http_header` function in `zerver/lib/webhooks/common.py`, +like so: + +``` +event = validate_extract_webhook_http_header(request, header, integration_name) +``` + +`request` is the `HttpRequest` object passed to your main webhook function. `header` +is the name of the custom header you'd like to extract, such as `X_EVENT_KEY`, and +`integration_name` is the name of the third-party service in question, such as +`GitHub`. + +Because such headers are how some integrations indicate the event types of their +payloads, the absence of such a header usually indicates a configuration +issue, where one either entered the URL for a different integration, or happens to +be running an older version of the integration that doesn't set that header. + +If the requisite header is missing, this function sends a PM to the owner of the +webhook bot, notifying them of the missing header. diff --git a/zerver/lib/exceptions.py b/zerver/lib/exceptions.py index 0091a63320..824219ee6f 100644 --- a/zerver/lib/exceptions.py +++ b/zerver/lib/exceptions.py @@ -30,6 +30,7 @@ class ErrorCode(AbstractEnum): BAD_IMAGE = () REALM_UPLOAD_QUOTA = () BAD_NARROW = () + MISSING_HTTP_EVENT_HEADER = () STREAM_DOES_NOT_EXIST = () UNAUTHORIZED_PRINCIPAL = () BAD_EVENT_QUEUE_ID = () diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py index 791571b39e..763f4a9e56 100644 --- a/zerver/lib/webhooks/common.py +++ b/zerver/lib/webhooks/common.py @@ -1,13 +1,40 @@ +from django.conf import settings from django.http import HttpRequest +from django.utils.translation import ugettext as _ from typing import Optional, Text from zerver.lib.actions import check_send_stream_message, \ - check_send_private_message -from zerver.lib.exceptions import StreamDoesNotExistError + check_send_private_message, send_rate_limited_pm_notification_to_bot_owner +from zerver.lib.exceptions import StreamDoesNotExistError, JsonableError, \ + ErrorCode from zerver.lib.request import REQ, has_request_variables -from zerver.models import UserProfile +from zerver.lib.send_email import FromAddress +from zerver.models import UserProfile, get_system_bot +MISSING_EVENT_HEADER_MESSAGE = """ +Hi there! Your bot {bot_name} just sent an HTTP request to {request_path} that +is missing the HTTP {header_name} header. Because this header is how +{integration_name} indicates the event type, this usually indicates a configuration +issue, where you either entered the URL for a different integration, or are running +an older version of the third-party service that doesn't provide that header. +Contact {support_email} if you need help debugging! +""" + +# Django prefixes all custom HTTP headers with `HTTP_` +DJANGO_HTTP_PREFIX = "HTTP_" + +class MissingHTTPEventHeader(JsonableError): + code = ErrorCode.MISSING_HTTP_EVENT_HEADER + data_fields = ['header'] + + def __init__(self, header: Text) -> None: + self.header = header + + @staticmethod + def msg_format() -> str: + return _("Missing the HTTP event header '{header}'") + @has_request_variables def check_send_webhook_message( request: HttpRequest, user_profile: UserProfile, @@ -32,3 +59,21 @@ def check_send_webhook_message( # stream, so we don't need to re-raise it since it clutters up # webhook-errors.log pass + +def validate_extract_webhook_http_header(request: HttpRequest, header: Text, + integration_name: Text) -> Text: + extracted_header = request.META.get(DJANGO_HTTP_PREFIX + header) + if extracted_header is None: + message_body = MISSING_EVENT_HEADER_MESSAGE.format( + bot_name=request.user.full_name, + request_path=request.path, + header_name=header, + integration_name=integration_name, + support_email=FromAddress.SUPPORT, + ) + send_rate_limited_pm_notification_to_bot_owner( + request.user, request.user.realm, message_body) + + raise MissingHTTPEventHeader(header) + + return extracted_header diff --git a/zerver/tests/test_webhooks_common.py b/zerver/tests/test_webhooks_common.py new file mode 100644 index 0000000000..5bc643241e --- /dev/null +++ b/zerver/tests/test_webhooks_common.py @@ -0,0 +1,75 @@ +from typing import Text + +from zerver.lib.test_classes import ZulipTestCase, WebhookTestCase +from zerver.lib.webhooks.common import \ + validate_extract_webhook_http_header, \ + MISSING_EVENT_HEADER_MESSAGE, MissingHTTPEventHeader +from zerver.models import get_user, get_realm +from zerver.lib.send_email import FromAddress +from zerver.lib.test_helpers import HostRequestMock + + +class WebhooksCommonTestCase(ZulipTestCase): + def test_webhook_http_header_header_exists(self) -> None: + webhook_bot = get_user('webhook-bot@zulip.com', get_realm('zulip')) + request = HostRequestMock() + request.META['HTTP_X_CUSTOM_HEADER'] = 'custom_value' + request.user = webhook_bot + + header_value = validate_extract_webhook_http_header(request, 'X_CUSTOM_HEADER', + 'test_webhook') + + self.assertEqual(header_value, 'custom_value') + + def test_webhook_http_header_header_does_not_exist(self) -> None: + webhook_bot = get_user('webhook-bot@zulip.com', get_realm('zulip')) + webhook_bot.last_reminder = None + notification_bot = self.notification_bot() + request = HostRequestMock() + request.user = webhook_bot + request.path = 'some/random/path' + + exception_msg = "Missing the HTTP event header 'X_CUSTOM_HEADER'" + with self.assertRaisesRegex(MissingHTTPEventHeader, exception_msg): + validate_extract_webhook_http_header(request, 'X_CUSTOM_HEADER', + 'test_webhook') + + msg = self.get_last_message() + expected_message = MISSING_EVENT_HEADER_MESSAGE.format( + bot_name=webhook_bot.full_name, + request_path=request.path, + header_name='X_CUSTOM_HEADER', + integration_name='test_webhook', + support_email=FromAddress.SUPPORT + ).rstrip() + self.assertEqual(msg.sender.email, notification_bot.email) + self.assertEqual(msg.content, expected_message) + +class MissingEventHeaderTestCase(WebhookTestCase): + STREAM_NAME = 'groove' + URL_TEMPLATE = '/api/v1/external/groove?stream={stream}&api_key={api_key}' + + # This tests the validate_extract_webhook_http_header function with + # an actual webhook, instead of just making a mock + def test_missing_event_header(self) -> None: + self.subscribe(self.test_user, self.STREAM_NAME) + result = self.client_post(self.url, self.get_body('ticket_state_changed'), + content_type="application/x-www-form-urlencoded") + self.assert_json_error(result, "Missing the HTTP event header 'X_GROOVE_EVENT'") + + webhook_bot = get_user('webhook-bot@zulip.com', get_realm('zulip')) + webhook_bot.last_reminder = None + notification_bot = self.notification_bot() + msg = self.get_last_message() + expected_message = MISSING_EVENT_HEADER_MESSAGE.format( + bot_name=webhook_bot.full_name, + request_path='/api/v1/external/groove', + header_name='X_GROOVE_EVENT', + integration_name='Groove', + support_email=FromAddress.SUPPORT + ).rstrip() + self.assertEqual(msg.sender.email, notification_bot.email) + self.assertEqual(msg.content, expected_message) + + def get_body(self, fixture_name: Text) -> Text: + return self.webhook_fixture_data("groove", fixture_name, file_type="json")