From 6f567e0850de5fbac5164d0c68489dbe374a7a3f Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Mar 2017 13:22:59 -0400 Subject: [PATCH] Add tests for build web hooks endpoint --- buildtrigger/bitbuckethandler.py | 3 ++ buildtrigger/githubhandler.py | 10 +++-- buildtrigger/gitlabhandler.py | 4 +- buildtrigger/test/test_githubhandler.py | 7 ++-- buildtrigger/test/test_gitlabhandler.py | 2 +- endpoints/webhooks.py | 1 + test/test_endpoints.py | 49 ++++++++++++++++++++++++- 7 files changed, 64 insertions(+), 12 deletions(-) diff --git a/buildtrigger/bitbuckethandler.py b/buildtrigger/bitbuckethandler.py index 84818d61c..b1bad253c 100644 --- a/buildtrigger/bitbuckethandler.py +++ b/buildtrigger/bitbuckethandler.py @@ -517,6 +517,9 @@ class BitbucketBuildTrigger(BuildTriggerHandler): def handle_trigger_request(self, request): payload = request.get_json() + if payload is None: + raise InvalidPayloadException('Missing payload') + logger.debug('Got BitBucket request: %s', payload) repository = self._get_repository_client() diff --git a/buildtrigger/githubhandler.py b/buildtrigger/githubhandler.py index 072af6e03..1c2169e2d 100644 --- a/buildtrigger/githubhandler.py +++ b/buildtrigger/githubhandler.py @@ -530,6 +530,8 @@ class GithubBuildTrigger(BuildTriggerHandler): def handle_trigger_request(self, request): # Check the payload to see if we should skip it based on the lack of a head_commit. payload = request.get_json() + if payload is None: + raise InvalidPayloadException('Missing payload') # This is for GitHub's probing/testing. if 'zen' in payload: @@ -537,16 +539,16 @@ class GithubBuildTrigger(BuildTriggerHandler): # Lookup the default branch for the repository. if 'repository' not in payload: - raise ValidationRequestException("Missing 'repository' on request") + raise InvalidPayloadException("Missing 'repository' on request") if 'owner' not in payload['repository']: - raise ValidationRequestException("Missing 'owner' on repository") + raise InvalidPayloadException("Missing 'owner' on repository") if 'name' not in payload['repository']['owner']: - raise ValidationRequestException("Missing owner 'name' on repository") + raise InvalidPayloadException("Missing owner 'name' on repository") if 'name' not in payload['repository']: - raise ValidationRequestException("Missing 'name' on repository") + raise InvalidPayloadException("Missing 'name' on repository") default_branch = None lookup_user = None diff --git a/buildtrigger/gitlabhandler.py b/buildtrigger/gitlabhandler.py index 522f51329..f34d2ddd9 100644 --- a/buildtrigger/gitlabhandler.py +++ b/buildtrigger/gitlabhandler.py @@ -514,7 +514,7 @@ class GitLabBuildTrigger(BuildTriggerHandler): def handle_trigger_request(self, request): payload = request.get_json() if not payload: - raise SkipRequestException() + raise InvalidPayloadException() logger.debug('GitLab trigger payload %s', payload) @@ -523,7 +523,7 @@ class GitLabBuildTrigger(BuildTriggerHandler): repo = gl_client.getproject(self.config['build_source']) if repo is False: logger.debug('Skipping GitLab build; project %s not found', self.config['build_source']) - raise SkipRequestException() + raise InvalidPayloadException() default_branch = repo['default_branch'] metadata = get_transformed_webhook_payload(payload, default_branch=default_branch, diff --git a/buildtrigger/test/test_githubhandler.py b/buildtrigger/test/test_githubhandler.py index 4ca91ece4..db236048c 100644 --- a/buildtrigger/test/test_githubhandler.py +++ b/buildtrigger/test/test_githubhandler.py @@ -2,7 +2,8 @@ import json import pytest from buildtrigger.test.githubmock import get_github_trigger -from buildtrigger.triggerutil import SkipRequestException, ValidationRequestException +from buildtrigger.triggerutil import (SkipRequestException, ValidationRequestException, + InvalidPayloadException) from endpoints.building import PreparedBuild from util.morecollections import AttrDict @@ -14,8 +15,8 @@ def github_trigger(): @pytest.mark.parametrize('payload, expected_error, expected_message', [ ('{"zen": true}', SkipRequestException, ""), - ('{}', ValidationRequestException, "Missing 'repository' on request"), - ('{"repository": "foo"}', ValidationRequestException, "Missing 'owner' on repository"), + ('{}', InvalidPayloadException, "Missing 'repository' on request"), + ('{"repository": "foo"}', InvalidPayloadException, "Missing 'owner' on repository"), # Valid payload: ('''{ diff --git a/buildtrigger/test/test_gitlabhandler.py b/buildtrigger/test/test_gitlabhandler.py index d20a690be..a74f5fea2 100644 --- a/buildtrigger/test/test_gitlabhandler.py +++ b/buildtrigger/test/test_gitlabhandler.py @@ -36,7 +36,7 @@ def test_lookup_user(email, expected_response, gitlab_trigger): @pytest.mark.parametrize('payload, expected_error, expected_message', [ - ('{}', SkipRequestException, ''), + ('{}', InvalidPayloadException, ''), # Valid payload: ('''{ diff --git a/endpoints/webhooks.py b/endpoints/webhooks.py index 79a3e58d1..83c205fbf 100644 --- a/endpoints/webhooks.py +++ b/endpoints/webhooks.py @@ -83,6 +83,7 @@ def build_trigger_webhook(trigger_uuid, **kwargs): namespace = trigger.repository.namespace_user.username repository = trigger.repository.name permission = ModifyRepositoryPermission(namespace, repository) + if permission.can(): handler = BuildTriggerHandler.get_handler(trigger) diff --git a/test/test_endpoints.py b/test/test_endpoints.py index 88da7270a..a87f09377 100644 --- a/test/test_endpoints.py +++ b/test/test_endpoints.py @@ -24,6 +24,7 @@ from endpoints.api.user import Signin from endpoints.keyserver import jwk_with_kid from endpoints.csrf import OAUTH_CSRF_TOKEN_NAME from endpoints.web import web as web_bp +from endpoints.webhooks import webhooks as webhooks_bp from initdb import setup_database_for_testing, finished_database_for_testing from test.helpers import assert_action_logged @@ -33,6 +34,12 @@ except ValueError: # This blueprint was already registered pass +try: + app.register_blueprint(webhooks_bp, url_prefix='/webhooks') +except ValueError: + # This blueprint was already registered + pass + try: app.register_blueprint(keyserver.key_server, url_prefix='') except ValueError: @@ -101,14 +108,20 @@ class EndpointTestCase(unittest.TestCase): self.assertEquals(rv.status_code, expected_code) return rv.data - def postResponse(self, resource_name, headers=None, form=None, with_csrf=True, expected_code=200, **kwargs): + def postResponse(self, resource_name, headers=None, data=None, form=None, with_csrf=True, expected_code=200, **kwargs): headers = headers or {} form = form or {} url = url_for(resource_name, **kwargs) if with_csrf: url = EndpointTestCase._add_csrf(url) - rv = self.app.post(url, headers=headers, data=form) + post_data = None + if form: + post_data = form + elif data: + post_data = py_json.dumps(data) + + rv = self.app.post(url, headers=headers, data=post_data) if expected_code is not None: self.assertEquals(rv.status_code, expected_code) @@ -121,6 +134,38 @@ class EndpointTestCase(unittest.TestCase): self.assertEquals(rv.status_code, 200) +class WebhookEndpointTestCase(EndpointTestCase): + def test_invalid_build_trigger_webhook(self): + self.postResponse('webhooks.build_trigger_webhook', trigger_uuid='invalidtrigger', + expected_code=404) + + def test_valid_build_trigger_webhook_invalid_auth(self): + trigger = list(model.build.list_build_triggers('devtable', 'building'))[0] + self.postResponse('webhooks.build_trigger_webhook', trigger_uuid=trigger.uuid, + expected_code=403) + + def test_valid_build_trigger_webhook_cookie_auth(self): + self.login('devtable', 'password') + + # Cookie auth is not supported, so this should 403 + trigger = list(model.build.list_build_triggers('devtable', 'building'))[0] + self.postResponse('webhooks.build_trigger_webhook', trigger_uuid=trigger.uuid, + expected_code=403) + + def test_valid_build_trigger_webhook_missing_payload(self): + auth_header = 'Basic %s' % (base64.b64encode('devtable:password')) + trigger = list(model.build.list_build_triggers('devtable', 'building'))[0] + self.postResponse('webhooks.build_trigger_webhook', trigger_uuid=trigger.uuid, + expected_code=400, headers={'Authorization': auth_header}) + + def test_valid_build_trigger_webhook_invalid_payload(self): + auth_header = 'Basic %s' % (base64.b64encode('devtable:password')) + trigger = list(model.build.list_build_triggers('devtable', 'building'))[0] + self.postResponse('webhooks.build_trigger_webhook', trigger_uuid=trigger.uuid, + expected_code=400, + headers={'Authorization': auth_header, 'Content-Type': 'application/json'}, + data={'invalid': 'payload'}) + class WebEndpointTestCase(EndpointTestCase): def test_index(self): self.getResponse('web.index')