diff --git a/zerver/webhooks/bitbucket2/view.py b/zerver/webhooks/bitbucket2/view.py index 92577e2b71..31632db0c5 100644 --- a/zerver/webhooks/bitbucket2/view.py +++ b/zerver/webhooks/bitbucket2/view.py @@ -9,7 +9,8 @@ from django.utils.translation import ugettext as _ from zerver.decorator import api_key_only_webhook_view from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_error, json_success -from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.webhooks.common import check_send_webhook_message, \ + validate_extract_webhook_http_header from zerver.lib.webhooks.git import SUBJECT_WITH_BRANCH_TEMPLATE, \ SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \ get_commits_comment_action_message, get_force_push_commits_event_message, \ @@ -110,7 +111,6 @@ def get_subject_based_on_type(payload: Dict[str, Any], type: str) -> Text: return get_subject(payload) def get_type(request: HttpRequest, payload: Dict[str, Any]) -> str: - event_key = request.META.get("HTTP_X_EVENT_KEY") if payload.get('push'): return 'push' elif payload.get('fork'): @@ -127,11 +127,19 @@ def get_type(request: HttpRequest, payload: Dict[str, Any]) -> str: return "issue_created" elif payload.get('pullrequest'): pull_request_template = 'pull_request_{}' + # Note that we only need the HTTP header to determine pullrequest events. + # We rely on the payload itself to determine the other ones. + event_key = validate_extract_webhook_http_header(request, "X_EVENT_KEY", "BitBucket") action = re.match('pullrequest:(?P.*)$', event_key) if action: - action = action.group('action') - if action in PULL_REQUEST_SUPPORTED_ACTIONS: - return pull_request_template.format(action) + action_group = action.group('action') + if action_group in PULL_REQUEST_SUPPORTED_ACTIONS: + return pull_request_template.format(action_group) + else: + event_key = validate_extract_webhook_http_header(request, "X_EVENT_KEY", "BitBucket") + if event_key == 'repo:updated': + return event_key + raise UnknownTriggerType("We don't support {} event type".format(event_key)) def get_body_based_on_type(type: str) -> Callable[[Dict[str, Any]], Text]: diff --git a/zerver/webhooks/github/view.py b/zerver/webhooks/github/view.py index b3be3a60d7..ffb51d6f10 100644 --- a/zerver/webhooks/github/view.py +++ b/zerver/webhooks/github/view.py @@ -8,7 +8,8 @@ from django.http import HttpRequest, HttpResponse from zerver.decorator import api_key_only_webhook_view from zerver.lib.request import REQ, JsonableError, has_request_variables from zerver.lib.response import json_success -from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.webhooks.common import check_send_webhook_message, \ + validate_extract_webhook_http_header from zerver.lib.webhooks.git import CONTENT_MESSAGE_TEMPLATE, \ SUBJECT_WITH_BRANCH_TEMPLATE, SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \ get_commits_comment_action_message, get_issue_event_message, \ @@ -379,7 +380,7 @@ def api_github_webhook( return json_success() def get_event(request: HttpRequest, payload: Dict[str, Any], branches: Text) -> Optional[str]: - event = request.META['HTTP_X_GITHUB_EVENT'] + event = validate_extract_webhook_http_header(request, 'X_GITHUB_EVENT', 'GitHub') if event == 'pull_request': action = payload['action'] if action in ('opened', 'synchronize', 'reopened', 'edited'): diff --git a/zerver/webhooks/gitlab/view.py b/zerver/webhooks/gitlab/view.py index 0cf5c6391b..1329a40dcd 100644 --- a/zerver/webhooks/gitlab/view.py +++ b/zerver/webhooks/gitlab/view.py @@ -7,7 +7,8 @@ from django.http import HttpRequest, HttpResponse from zerver.decorator import api_key_only_webhook_view from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success -from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.webhooks.common import check_send_webhook_message, \ + validate_extract_webhook_http_header from zerver.lib.webhooks.git import EMPTY_SHA, \ SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \ get_commits_comment_action_message, get_issue_event_message, \ @@ -332,13 +333,7 @@ def get_subject_based_on_event(event: str, payload: Dict[str, Any]) -> Text: def get_event(request: HttpRequest, payload: Dict[str, Any], branches: Optional[Text]) -> Optional[str]: # if there is no 'action' attribute, then this is a test payload # and we should ignore it - event = request.META.get('HTTP_X_GITLAB_EVENT') - if event is None: - # Suppress events from old versions of GitLab that don't set - # this header. TODO: We probably want a more generic solution - # to this category of issue that throws a 40x error to the - # user, e.g. some sort of InvalidWebhookRequest exception. - return None + event = validate_extract_webhook_http_header(request, 'X_GITLAB_EVENT', 'GitLab') if event in ['Issue Hook', 'Merge Request Hook', 'Wiki Page Hook']: action = payload['object_attributes'].get('action') if action is None: diff --git a/zerver/webhooks/gogs/view.py b/zerver/webhooks/gogs/view.py index b349304d2d..157b130bd2 100644 --- a/zerver/webhooks/gogs/view.py +++ b/zerver/webhooks/gogs/view.py @@ -8,7 +8,8 @@ from django.utils.translation import ugettext as _ from zerver.decorator import api_key_only_webhook_view from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_error, json_success -from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.webhooks.common import check_send_webhook_message, \ + validate_extract_webhook_http_header from zerver.lib.webhooks.git import SUBJECT_WITH_BRANCH_TEMPLATE, \ SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, get_create_branch_event_message, \ get_pull_request_event_message, get_push_commits_event_message @@ -66,7 +67,7 @@ def api_gogs_webhook(request: HttpRequest, user_profile: UserProfile, branches: Optional[Text]=REQ(default=None)) -> HttpResponse: repo = payload['repository']['name'] - event = request.META['HTTP_X_GOGS_EVENT'] + event = validate_extract_webhook_http_header(request, 'X_GOGS_EVENT', 'Gogs') if event == 'push': branch = payload['ref'].replace('refs/heads/', '') if branches is not None and branches.find(branch) == -1: diff --git a/zerver/webhooks/groove/tests.py b/zerver/webhooks/groove/tests.py index 2540b7dc47..1996d1e953 100644 --- a/zerver/webhooks/groove/tests.py +++ b/zerver/webhooks/groove/tests.py @@ -17,7 +17,7 @@ class GrooveHookTests(WebhookTestCase): u"```") self.send_and_test_stream_message('ticket_started', expected_subject, expected_message, content_type="application/x-www-form-urlencoded", - X_GROOVE_EVENT="ticket_started") + HTTP_X_GROOVE_EVENT="ticket_started") # This simulates the condition when a ticket # is assigned to an agent. @@ -29,7 +29,7 @@ class GrooveHookTests(WebhookTestCase): u"```") self.send_and_test_stream_message('ticket_assigned_agent_only', expected_subject, expected_message, content_type="application/x-www-form-urlencoded", - X_GROOVE_EVENT="ticket_assigned") + HTTP_X_GROOVE_EVENT="ticket_assigned") # This simulates the condition when a ticket # is assigned to an agent in a group. @@ -41,7 +41,7 @@ class GrooveHookTests(WebhookTestCase): u"```") self.send_and_test_stream_message('ticket_assigned_agent_and_group', expected_subject, expected_message, content_type="application/x-www-form-urlencoded", - X_GROOVE_EVENT="ticket_assigned") + HTTP_X_GROOVE_EVENT="ticket_assigned") # This simulates the condition when a ticket # is assigned to a group. @@ -53,7 +53,7 @@ class GrooveHookTests(WebhookTestCase): u"```") self.send_and_test_stream_message('ticket_assigned_group_only', expected_subject, expected_message, content_type="application/x-www-form-urlencoded", - X_GROOVE_EVENT="ticket_assigned") + HTTP_X_GROOVE_EVENT="ticket_assigned") # This simulates the condition when a ticket # is assigned to no one. @@ -61,7 +61,7 @@ class GrooveHookTests(WebhookTestCase): self.subscribe(self.test_user, self.STREAM_NAME) result = self.client_post(self.url, self.get_body('ticket_assigned_no_one'), content_type="application/x-www-form-urlencoded", - X_GROOVE_EVENT='ticket_assigned') + HTTP_X_GROOVE_EVENT='ticket_assigned') self.assert_json_success(result) # This simulates the notification when an agent replied to a ticket. @@ -74,7 +74,7 @@ class GrooveHookTests(WebhookTestCase): u"```") self.send_and_test_stream_message('agent_replied', expected_subject, expected_message, content_type="application/x-www-form-urlencoded", - X_GROOVE_EVENT="agent_replied") + HTTP_X_GROOVE_EVENT="agent_replied") # This simulates the condition when a customer replied to a ticket. def test_groove_customer_replied(self) -> None: @@ -86,7 +86,7 @@ class GrooveHookTests(WebhookTestCase): u"```") self.send_and_test_stream_message('customer_replied', expected_subject, expected_message, content_type="application/x-www-form-urlencoded", - X_GROOVE_EVENT="customer_replied") + HTTP_X_GROOVE_EVENT="customer_replied") # This simulates the condition when an agent left a note. def test_groove_note_added(self) -> None: @@ -98,27 +98,21 @@ class GrooveHookTests(WebhookTestCase): u"```") self.send_and_test_stream_message('note_added', expected_subject, expected_message, content_type="application/x-ww-form-urlencoded", - X_GROOVE_EVENT="note_added") + HTTP_X_GROOVE_EVENT="note_added") # This is for other events than specified. def test_groove_ticket_state_changed(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", - X_GROOVE_EVENT='ticket_state_changed') + HTTP_X_GROOVE_EVENT='ticket_state_changed') self.assert_json_success(result) - def test_groove_header_missing(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 event header') - def test_groove_malformed_payload(self) -> None: self.subscribe(self.test_user, self.STREAM_NAME) result = self.client_post(self.url, self.get_body('malformed_payload'), content_type="application/x-www-form-urlencoded", - X_GROOVE_EVENT='ticket_started') + HTTP_X_GROOVE_EVENT='ticket_started') self.assert_json_error(result, 'Missing required data') def get_body(self, fixture_name: Text) -> Text: diff --git a/zerver/webhooks/groove/view.py b/zerver/webhooks/groove/view.py index 042adf97b1..d0281726c2 100644 --- a/zerver/webhooks/groove/view.py +++ b/zerver/webhooks/groove/view.py @@ -9,7 +9,8 @@ import logging from zerver.decorator import api_key_only_webhook_view from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_error, json_success -from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.webhooks.common import check_send_webhook_message, \ + validate_extract_webhook_http_header from zerver.models import UserProfile def ticket_started_body(payload: Dict[str, Any]) -> Text: @@ -79,12 +80,8 @@ def note_added_body(payload: Dict[str, Any]) -> Text: @has_request_variables def api_groove_webhook(request: HttpRequest, user_profile: UserProfile, payload: Dict[str, Any]=REQ(argument_type='body')) -> HttpResponse: - try: - # The event identifier is stored in the X_GROOVE_EVENT header. - event = request.META['X_GROOVE_EVENT'] - except KeyError: - logging.error('No header with the Groove payload') - return json_error(_('Missing event header')) + event = validate_extract_webhook_http_header(request, 'X_GROOVE_EVENT', 'Groove') + # We listen to several events that are used for notifications. # Other events are ignored. if event in EVENTS_FUNCTION_MAPPER: