webhooks: Migrate to validate_extract_webhook_http_header.

This is a part of our efforts to close #6213.
This commit is contained in:
Eeshan Garg
2018-04-24 16:24:13 -02:30
committed by Tim Abbott
parent 34d1b0ebf1
commit 9fb9c0d901
6 changed files with 36 additions and 40 deletions

View File

@@ -9,7 +9,8 @@ from django.utils.translation import ugettext as _
from zerver.decorator import api_key_only_webhook_view from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success 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, \ from zerver.lib.webhooks.git import SUBJECT_WITH_BRANCH_TEMPLATE, \
SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \ SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \
get_commits_comment_action_message, get_force_push_commits_event_message, \ 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) return get_subject(payload)
def get_type(request: HttpRequest, payload: Dict[str, Any]) -> str: def get_type(request: HttpRequest, payload: Dict[str, Any]) -> str:
event_key = request.META.get("HTTP_X_EVENT_KEY")
if payload.get('push'): if payload.get('push'):
return 'push' return 'push'
elif payload.get('fork'): elif payload.get('fork'):
@@ -127,11 +127,19 @@ def get_type(request: HttpRequest, payload: Dict[str, Any]) -> str:
return "issue_created" return "issue_created"
elif payload.get('pullrequest'): elif payload.get('pullrequest'):
pull_request_template = 'pull_request_{}' 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<action>.*)$', event_key) action = re.match('pullrequest:(?P<action>.*)$', event_key)
if action: if action:
action = action.group('action') action_group = action.group('action')
if action in PULL_REQUEST_SUPPORTED_ACTIONS: if action_group in PULL_REQUEST_SUPPORTED_ACTIONS:
return pull_request_template.format(action) 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)) raise UnknownTriggerType("We don't support {} event type".format(event_key))
def get_body_based_on_type(type: str) -> Callable[[Dict[str, Any]], Text]: def get_body_based_on_type(type: str) -> Callable[[Dict[str, Any]], Text]:

View File

@@ -8,7 +8,8 @@ from django.http import HttpRequest, HttpResponse
from zerver.decorator import api_key_only_webhook_view from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, JsonableError, has_request_variables from zerver.lib.request import REQ, JsonableError, has_request_variables
from zerver.lib.response import json_success 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, \ from zerver.lib.webhooks.git import CONTENT_MESSAGE_TEMPLATE, \
SUBJECT_WITH_BRANCH_TEMPLATE, SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \ SUBJECT_WITH_BRANCH_TEMPLATE, SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \
get_commits_comment_action_message, get_issue_event_message, \ get_commits_comment_action_message, get_issue_event_message, \
@@ -379,7 +380,7 @@ def api_github_webhook(
return json_success() return json_success()
def get_event(request: HttpRequest, payload: Dict[str, Any], branches: Text) -> Optional[str]: 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': if event == 'pull_request':
action = payload['action'] action = payload['action']
if action in ('opened', 'synchronize', 'reopened', 'edited'): if action in ('opened', 'synchronize', 'reopened', 'edited'):

View File

@@ -7,7 +7,8 @@ from django.http import HttpRequest, HttpResponse
from zerver.decorator import api_key_only_webhook_view from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_success 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, \ from zerver.lib.webhooks.git import EMPTY_SHA, \
SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \ SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \
get_commits_comment_action_message, get_issue_event_message, \ 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]: 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 # if there is no 'action' attribute, then this is a test payload
# and we should ignore it # and we should ignore it
event = request.META.get('HTTP_X_GITLAB_EVENT') event = validate_extract_webhook_http_header(request, 'X_GITLAB_EVENT', 'GitLab')
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
if event in ['Issue Hook', 'Merge Request Hook', 'Wiki Page Hook']: if event in ['Issue Hook', 'Merge Request Hook', 'Wiki Page Hook']:
action = payload['object_attributes'].get('action') action = payload['object_attributes'].get('action')
if action is None: if action is None:

View File

@@ -8,7 +8,8 @@ from django.utils.translation import ugettext as _
from zerver.decorator import api_key_only_webhook_view from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success 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, \ from zerver.lib.webhooks.git import SUBJECT_WITH_BRANCH_TEMPLATE, \
SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, get_create_branch_event_message, \ SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, get_create_branch_event_message, \
get_pull_request_event_message, get_push_commits_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: branches: Optional[Text]=REQ(default=None)) -> HttpResponse:
repo = payload['repository']['name'] 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': if event == 'push':
branch = payload['ref'].replace('refs/heads/', '') branch = payload['ref'].replace('refs/heads/', '')
if branches is not None and branches.find(branch) == -1: if branches is not None and branches.find(branch) == -1:

View File

@@ -17,7 +17,7 @@ class GrooveHookTests(WebhookTestCase):
u"```") u"```")
self.send_and_test_stream_message('ticket_started', expected_subject, expected_message, self.send_and_test_stream_message('ticket_started', expected_subject, expected_message,
content_type="application/x-www-form-urlencoded", 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 # This simulates the condition when a ticket
# is assigned to an agent. # is assigned to an agent.
@@ -29,7 +29,7 @@ class GrooveHookTests(WebhookTestCase):
u"```") u"```")
self.send_and_test_stream_message('ticket_assigned_agent_only', expected_subject, expected_message, self.send_and_test_stream_message('ticket_assigned_agent_only', expected_subject, expected_message,
content_type="application/x-www-form-urlencoded", 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 # This simulates the condition when a ticket
# is assigned to an agent in a group. # is assigned to an agent in a group.
@@ -41,7 +41,7 @@ class GrooveHookTests(WebhookTestCase):
u"```") u"```")
self.send_and_test_stream_message('ticket_assigned_agent_and_group', expected_subject, expected_message, self.send_and_test_stream_message('ticket_assigned_agent_and_group', expected_subject, expected_message,
content_type="application/x-www-form-urlencoded", 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 # This simulates the condition when a ticket
# is assigned to a group. # is assigned to a group.
@@ -53,7 +53,7 @@ class GrooveHookTests(WebhookTestCase):
u"```") u"```")
self.send_and_test_stream_message('ticket_assigned_group_only', expected_subject, expected_message, self.send_and_test_stream_message('ticket_assigned_group_only', expected_subject, expected_message,
content_type="application/x-www-form-urlencoded", 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 # This simulates the condition when a ticket
# is assigned to no one. # is assigned to no one.
@@ -61,7 +61,7 @@ class GrooveHookTests(WebhookTestCase):
self.subscribe(self.test_user, self.STREAM_NAME) self.subscribe(self.test_user, self.STREAM_NAME)
result = self.client_post(self.url, self.get_body('ticket_assigned_no_one'), result = self.client_post(self.url, self.get_body('ticket_assigned_no_one'),
content_type="application/x-www-form-urlencoded", content_type="application/x-www-form-urlencoded",
X_GROOVE_EVENT='ticket_assigned') HTTP_X_GROOVE_EVENT='ticket_assigned')
self.assert_json_success(result) self.assert_json_success(result)
# This simulates the notification when an agent replied to a ticket. # This simulates the notification when an agent replied to a ticket.
@@ -74,7 +74,7 @@ class GrooveHookTests(WebhookTestCase):
u"```") u"```")
self.send_and_test_stream_message('agent_replied', expected_subject, expected_message, self.send_and_test_stream_message('agent_replied', expected_subject, expected_message,
content_type="application/x-www-form-urlencoded", 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. # This simulates the condition when a customer replied to a ticket.
def test_groove_customer_replied(self) -> None: def test_groove_customer_replied(self) -> None:
@@ -86,7 +86,7 @@ class GrooveHookTests(WebhookTestCase):
u"```") u"```")
self.send_and_test_stream_message('customer_replied', expected_subject, expected_message, self.send_and_test_stream_message('customer_replied', expected_subject, expected_message,
content_type="application/x-www-form-urlencoded", 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. # This simulates the condition when an agent left a note.
def test_groove_note_added(self) -> None: def test_groove_note_added(self) -> None:
@@ -98,27 +98,21 @@ class GrooveHookTests(WebhookTestCase):
u"```") u"```")
self.send_and_test_stream_message('note_added', expected_subject, expected_message, self.send_and_test_stream_message('note_added', expected_subject, expected_message,
content_type="application/x-ww-form-urlencoded", 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. # This is for other events than specified.
def test_groove_ticket_state_changed(self) -> None: def test_groove_ticket_state_changed(self) -> None:
self.subscribe(self.test_user, self.STREAM_NAME) self.subscribe(self.test_user, self.STREAM_NAME)
result = self.client_post(self.url, self.get_body('ticket_state_changed'), result = self.client_post(self.url, self.get_body('ticket_state_changed'),
content_type="application/x-www-form-urlencoded", 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) 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: def test_groove_malformed_payload(self) -> None:
self.subscribe(self.test_user, self.STREAM_NAME) self.subscribe(self.test_user, self.STREAM_NAME)
result = self.client_post(self.url, self.get_body('malformed_payload'), result = self.client_post(self.url, self.get_body('malformed_payload'),
content_type="application/x-www-form-urlencoded", 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') self.assert_json_error(result, 'Missing required data')
def get_body(self, fixture_name: Text) -> Text: def get_body(self, fixture_name: Text) -> Text:

View File

@@ -9,7 +9,8 @@ import logging
from zerver.decorator import api_key_only_webhook_view from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success 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 from zerver.models import UserProfile
def ticket_started_body(payload: Dict[str, Any]) -> Text: 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 @has_request_variables
def api_groove_webhook(request: HttpRequest, user_profile: UserProfile, def api_groove_webhook(request: HttpRequest, user_profile: UserProfile,
payload: Dict[str, Any]=REQ(argument_type='body')) -> HttpResponse: payload: Dict[str, Any]=REQ(argument_type='body')) -> HttpResponse:
try: event = validate_extract_webhook_http_header(request, 'X_GROOVE_EVENT', 'Groove')
# 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'))
# We listen to several events that are used for notifications. # We listen to several events that are used for notifications.
# Other events are ignored. # Other events are ignored.
if event in EVENTS_FUNCTION_MAPPER: if event in EVENTS_FUNCTION_MAPPER: