Skip to content

Commit

Permalink
Fix incoming payload parsing, cleanup, and add logging
Browse files Browse the repository at this point in the history
  • Loading branch information
@agjohnson
agjohnson committed Oct 5, 2016
1 parent 7de216b commit 54ced28
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 80 deletions.
71 changes: 39 additions & 32 deletions readthedocs/core/views/hooks.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,20 +141,24 @@ def github_build(request):
This will search for projects matching either a stripped down HTTP or SSH
URL. The search is error prone, use the API v2 webhook for new webhooks.
Old webhooks may not have specified the content type to POST with, and
therefore can use ``application/x-www-form-urlencoded`` to pass the JSON
payload. More information on the API docs here:
https://developer.github.com/webhooks/creating/#content-type
"""
if request.method == 'POST':
try:
data = json.loads(request.body)
except (ValueError, TypeError):
log.error('Invalid GitHub webhook payload', exc_info=True)
return HttpResponse('Invalid request', status=400)
http_url = data['repository']['url']
http_search_url = http_url.replace('http://', '').replace('https://', '')
ssh_url = data['repository']['ssh_url']
ssh_search_url = ssh_url.replace('git@', '').replace('.git', '')
try:
if request.META['CONTENT_TYPE'] == 'application/x-www-form-urlencoded':
data = json.loads(request.POST.get('payload'))
else:
data = json.loads(request.body)
http_url = data['repository']['url']
http_search_url = http_url.replace('http://', '').replace('https://', '')
ssh_url = data['repository']['ssh_url']
ssh_search_url = ssh_url.replace('git@', '').replace('.git', '')
branches = [data['ref'].replace('refs/heads/', '')]
except KeyError:
except (ValueError, TypeError, KeyError):
log.error('Invalid GitHub webhook payload', exc_info=True)
return HttpResponse('Invalid request', status=400)
try:
Expand All @@ -178,7 +182,7 @@ def github_build(request):
log.error('Project match not found: url=%s', http_search_url)
return HttpResponseNotFound('Project not found')
else:
return HttpResponse('Method not allowed', status=405)
return HttpResponse('Method not allowed, POST is required', status=405)


@csrf_exempt
Expand All @@ -191,12 +195,12 @@ def gitlab_build(request):
if request.method == 'POST':
try:
data = json.loads(request.body)
except (ValueError, TypeError):
url = data['project']['http_url']
search_url = re.sub(r'^https?://(.*?)(?:\.git|)$', '\\1', url)
branches = [data['ref'].replace('refs/heads/', '')]
except (ValueError, TypeError, KeyError):
log.error('Invalid GitLab webhook payload', exc_info=True)
return HttpResponse('Invalid request', status=400)
url = data['project']['http_url']
search_url = re.sub(r'^https?://(.*?)(?:\.git|)$', '\\1', url)
branches = [data['ref'].replace('refs/heads/', '')]
log.info(
'GitLab webhook search: url=%s branches=%s',
search_url,
Expand All @@ -209,49 +213,52 @@ def gitlab_build(request):
log.error('Project match not found: url=%s', search_url)
return HttpResponseNotFound('Project match not found')
else:
return HttpResponse('Method not allowed', status=405)
return HttpResponse('Method not allowed, POST is required', status=405)


@csrf_exempt
def bitbucket_build(request):
"""Consume webhooks from multiple versions of Bitbucket's API
New webhooks are set up with v2, but v1 webhooks will still point to this
endpoint. There are also "services" that point here and submit
``application/x-www-form-urlencoded`` data.
API v1
https://confluence.atlassian.com/bitbucket/events-resources-296095220.html
API v2
https://confluence.atlassian.com/bitbucket/event-payloads-740262817.html#EventPayloads-Push
Services
https://confluence.atlassian.com/bitbucket/post-service-management-223216518.html
"""
if request.method == 'POST':
try:
data = json.loads(request.body)
except (TypeError, ValueError):
log.error('Invalid Bitbucket webhook payload', exc_info=True)
return HttpResponse('Invalid request', status=400)
if request.META['CONTENT_TYPE'] == 'application/x-www-form-urlencoded':
data = json.loads(request.POST.get('payload'))
else:
data = json.loads(request.body)

version = 2 if request.META.get('HTTP_USER_AGENT') == 'Bitbucket-Webhooks/2.0' else 1
if version == 1:
try:
version = 2 if request.META.get('HTTP_USER_AGENT') == 'Bitbucket-Webhooks/2.0' else 1
if version == 1:
branches = [commit.get('branch', '')
for commit in data['commits']]
repository = data['repository']
search_url = 'bitbucket.org{0}'.format(
repository['absolute_url'].rstrip('/')
)
except KeyError:
log.error('Invalid Bitbucket webhook payload', exc_info=True)
return HttpResponse('Invalid request', status=400)
elif version == 2:
try:
elif version == 2:
changes = data['push']['changes']
branches = [change['new']['name']
for change in changes]
search_url = 'bitbucket.org/{0}'.format(
data['repository']['full_name']
)
except KeyError:
log.error('Invalid Bitbucket webhook payload', exc_info=True)
return HttpResponse('Invalid request', status=400)
except (TypeError, ValueError, KeyError):
log.error('Invalid Bitbucket webhook payload', exc_info=True)
return HttpResponse('Invalid request', status=400)

log.info(
'Bitbucket webhook search: url=%s branches=%s',
search_url,
Expand All @@ -265,7 +272,7 @@ def bitbucket_build(request):
log.error('Project match not found: url=%s', search_url)
return HttpResponseNotFound('Project match not found')
else:
return HttpResponse('Method not allowed', status=405)
return HttpResponse('Method not allowed, POST is required', status=405)


@csrf_exempt
Expand Down
83 changes: 37 additions & 46 deletions readthedocs/restapi/views/integrations.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import logging

from rest_framework import permissions
from rest_framework.views import APIView
from rest_framework.renderers import JSONRenderer
Expand All @@ -13,6 +15,8 @@
from readthedocs.projects.models import Project


log = logging.getLogger(__name__)

GITHUB_PUSH = 'push'
GITLAB_PUSH = 'push'
BITBUCKET_PUSH = 'repo:push'
Expand All @@ -27,23 +31,41 @@ def post(self, request, project_slug, format=None):
try:
project = Project.objects.get(slug=project_slug)
resp = self.handle_webhook(request, project, request.data)
if resp is None:
log.info('Unhandled webhook event')
resp = {'detail': 'Unhandled webhook event'}
except Project.DoesNotExist:
raise Http404('Project does not exist')
return Response(resp)

def handle_webhook(self, request, project, data=None):
"""Handle webhook payload
"""Handle webhook payload"""
raise NotImplementedError

If a build is triggered from this method, return a JSON response with
the following::
def get_response_push(self, project, branches):
"""Build branches on push events and return API response
Return a JSON response with the following::
{
"build_triggered": true,
"project": "project_name",
"versions": [...]
}
:param project: Project instance
:type project: Project
:param branches: List of branch names to build
:type branches: list(str)
"""
raise NotImplementedError
to_build, not_building = build_branches(project, branches)
if not_building:
log.info('Skipping project branches: project=%s branches=%s',
project, branches)
triggered = True if to_build else False
return {'build_triggered': triggered,
'project': project.slug,
'versions': to_build}


class GitHubWebhookView(WebhookMixin, APIView):
Expand All @@ -60,32 +82,19 @@ class GitHubWebhookView(WebhookMixin, APIView):
"ref": "branch-name",
...
}
If a build is triggered, this will return JSON with the following::
{
"build_triggered": true,
"project": "project_name",
"versions": [...]
}
"""

def handle_webhook(self, request, project, data=None):
# Get event and trigger other webhook events
event = request.META.get('HTTP_X_GITHUB_EVENT', 'push')
webhook_github.send(Project, project=project, data=data, event=event)
# Handle push events and trigger builds
try:
branches = [request.data['ref'].replace('refs/heads/', '')]
except KeyError:
raise ParseError('Parameter "ref" is required')
if event == GITHUB_PUSH:
to_build, not_building = build_branches(project, branches)
triggered = True if to_build else False
return {'build_triggered': triggered,
'project': project.slug,
'versions': to_build}
return {'build_triggered': False}
try:
branches = [request.data['ref'].replace('refs/heads/', '')]
return self.get_response_push(project, branches)
except KeyError:
raise ParseError('Parameter "ref" is required')


class GitLabWebhookView(WebhookMixin, APIView):
Expand All @@ -101,32 +110,19 @@ class GitLabWebhookView(WebhookMixin, APIView):
"ref": "branch-name",
...
}
If a build is triggered, this will return JSON with the following::
{
"build_triggered": true,
"project": "project_name",
"versions": [...]
}
"""

def handle_webhook(self, request, project, data=None):
# Get event and trigger other webhook events
event = data.get('object_kind', GITLAB_PUSH)
webhook_gitlab.send(Project, project=project, data=data, event=event)
# Handle push events and trigger builds
try:
branches = [request.data['ref'].replace('refs/heads/', '')]
except KeyError:
raise ParseError('Parameter "ref" is required')
if event == GITLAB_PUSH:
to_build, not_building = build_branches(project, branches)
triggered = True if to_build else False
return {'build_triggered': triggered,
'project': project.slug,
'versions': to_build}
return {'build_triggered': False}
try:
branches = [request.data['ref'].replace('refs/heads/', '')]
return self.get_response_push(project, branches)
except KeyError:
raise ParseError('Parameter "ref" is required')


class BitbucketWebhookView(WebhookMixin, APIView):
Expand Down Expand Up @@ -162,11 +158,6 @@ def handle_webhook(self, request, project, data=None):
changes = data['push']['changes']
branches = [change['new']['name']
for change in changes]
return self.get_response_push(project, branches)
except KeyError:
raise ParseError('Invalid request')
to_build, not_building = build_branches(project, branches)
triggered = True if to_build else False
return {'build_triggered': triggered,
'project': project.slug,
'versions': to_build}
return {'build_triggered': False}
35 changes: 35 additions & 0 deletions readthedocs/rtd_tests/tests/test_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,18 @@ def test_github_webhook(self, trigger_build):
mock.call(force=True, version=mock.ANY, project=self.project)
])

def test_github_invalid_webhook(self, trigger_build):
"""GitHub webhook unhandled event"""
client = APIClient()
resp = client.post(
'/api/v2/webhook/github/{0}/'.format(self.project.slug),
{'foo': 'bar'},
format='json',
HTTP_X_GITHUB_EVENT='pull_request',
)
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp.data['detail'], 'Unhandled webhook event')

def test_gitlab_webhook(self, trigger_build):
"""GitLab webhook API"""
client = APIClient()
Expand All @@ -344,6 +356,17 @@ def test_gitlab_webhook(self, trigger_build):
mock.call(force=True, version=mock.ANY, project=self.project)
])

def test_gitlab_invalid_webhook(self, trigger_build):
"""GitLab webhook unhandled event"""
client = APIClient()
resp = client.post(
'/api/v2/webhook/gitlab/{0}/'.format(self.project.slug),
{'object_kind': 'pull_request'},
format='json',
)
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp.data['detail'], 'Unhandled webhook event')

def test_bitbucket_webhook(self, trigger_build):
"""Bitbucket webhook API"""
client = APIClient()
Expand Down Expand Up @@ -379,3 +402,15 @@ def test_bitbucket_webhook(self, trigger_build):
trigger_build.assert_has_calls([
mock.call(force=True, version=mock.ANY, project=self.project)
])

def test_bitbucket_invalid_webhook(self, trigger_build):
"""Bitbucket webhook unhandled event"""
client = APIClient()
resp = client.post(
'/api/v2/webhook/bitbucket/{0}/'.format(self.project.slug),
{'foo': 'bar'},
format='json',
HTTP_X_EVENT_KEY='pull_request'
)
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp.data['detail'], 'Unhandled webhook event')
27 changes: 25 additions & 2 deletions readthedocs/rtd_tests/tests/test_post_commit_hooks.py
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from django.test import TestCase
import json
import logging
import mock
from urllib import urlencode

import mock
from django_dynamic_fixture import get
from django.test import TestCase

from readthedocs.builds.models import Version
from readthedocs.projects.models import Project
Expand Down Expand Up @@ -209,6 +210,17 @@ def setUp(self):
}
}

def test_post_types(self):
"""Ensure various POST formats"""
r = self.client.post('/github/',
data=json.dumps(self.payload),
content_type='application/json')
self.assertEqual(r.status_code, 200)
r = self.client.post('/github/',
data=urlencode({'payload': json.dumps(self.payload)}),
content_type='application/x-www-form-urlencoded')
self.assertEqual(r.status_code, 200)

def test_github_upper_case_repo(self):
"""
Test the github post commit hook will build properly with upper case
Expand Down Expand Up @@ -404,6 +416,17 @@ def setUp(self):
"user": "marcus"
}

def test_post_types(self):
"""Ensure various POST formats"""
r = self.client.post('/bitbucket/',
data=json.dumps(self.hg_payload),
content_type='application/json')
self.assertEqual(r.status_code, 200)
r = self.client.post('/bitbucket/',
data=urlencode({'payload': json.dumps(self.hg_payload)}),
content_type='application/x-www-form-urlencoded')
self.assertEqual(r.status_code, 200)

def test_bitbucket_post_commit(self):
r = self.client.post('/bitbucket/', data=json.dumps(self.hg_payload),
content_type='application/json')
Expand Down

0 comments on commit 54ced28

Please sign in to comment.