From 55511b40ddcd6414183759db58eccc2907a7a3a8 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 14 Nov 2023 11:38:12 -0500 Subject: [PATCH] Merge pull request from GHSA-45hq-g76r-46wv Problems: - We were automatically creating an integration when a webhook was received, this integration didn't have a secret set. - The identifier of integrations (or better said, their URL) is really easy to guess. And for those created manually, the URL is the only protection they have. Solutions and improvements: - No longer create integrations automatically. - All integrations are created with a secret attached to it. - Use django's get_random_string to generate the secret, and generate one on save if the integration doesn't have one. - Show the integration's secret in the detail view. - Bitbucket now allows attaching a secret to their webhooks! https://bitbucket.org/blog/enhanced-webhook-security - Use the sha256 signature to validate payloads from github (the sha1 version is deprecated) - This doesn't break the gitea workaround, since they also support using a shared secret and use the same header as github to attach the signature. - Old integrations that don't have a secret attached to them are still valid, but their functionality has been reduced to just trigger builds on existing versions, in order to trigger builds from PRs, and update the default branch, the webhook needs to be re-created with a secret. Ref https://github.com/readthedocs/readthedocs.org/security/advisories/GHSA-45hq-g76r-46wv --- docs/user/guides/setup/git-repo-manual.rst | 20 +- readthedocs/api/v2/views/integrations.py | 169 ++++---- .../0005_change_default_integration_secret.py | 3 - readthedocs/integrations/models.py | 17 +- readthedocs/integrations/utils.py | 13 - readthedocs/oauth/services/bitbucket.py | 2 + readthedocs/oauth/services/github.py | 8 - readthedocs/oauth/services/gitlab.py | 11 - readthedocs/oauth/utils.py | 4 + readthedocs/projects/forms.py | 7 +- readthedocs/rtd_tests/tests/test_api.py | 388 ++++++++++++------ .../rtd_tests/tests/test_integrations.py | 25 +- readthedocs/rtd_tests/tests/test_oauth.py | 12 +- .../projects/integration_webhook_detail.html | 20 +- 14 files changed, 423 insertions(+), 276 deletions(-) diff --git a/docs/user/guides/setup/git-repo-manual.rst b/docs/user/guides/setup/git-repo-manual.rst index d454a965b2c..1901b16f82c 100644 --- a/docs/user/guides/setup/git-repo-manual.rst +++ b/docs/user/guides/setup/git-repo-manual.rst @@ -45,7 +45,7 @@ Read the Docs will sync versions and build your documentation when your Git repo You may need to prepend *https://* to the URL. * For **Content type**, both *application/json* and *application/x-www-form-urlencoded* work - * Leave the **Secrets** field blank + * Fill the **Secret** field with the value from the integration on Read the Docs * Select **Let me select individual events**, and mark **Branch or tag creation**, **Branch or tag deletion**, **Pull requests** and **Pushes** events * Ensure **Active** is enabled; it is by default @@ -55,14 +55,13 @@ Read the Docs will sync versions and build your documentation when your Git repo If you see a Response 200, then the webhook is correctly configured. For a 403 error, it's likely that the Payload URL is incorrect. - .. note:: The webhook token, intended for the GitHub **Secret** field, is not yet implemented. - .. tab:: Bitbucket * Go to the :guilabel:`Settings` > :guilabel:`Webhooks` > :guilabel:`Add webhook` page for your project * For **URL**, use the URL of the integration on Read the Docs, found on the :guilabel:`Admin` > :guilabel:`Integrations` page * Under **Triggers**, **Repository push** should be selected + * Fill the **Secret** field with the value from the integration on Read the Docs * Finish by clicking **Save** .. tab:: GitLab @@ -70,6 +69,7 @@ Read the Docs will sync versions and build your documentation when your Git repo * Go to the :guilabel:`Settings` > :guilabel:`Webhooks` page for your GitLab project * For **URL**, use the URL of the integration on **Read the Docs project**, found on the :guilabel:`Admin` > :guilabel:`Integrations` page + * Fill the **Secret token** field with the value from the integration on Read the Docs * Leave the default **Push events** selected, additionally mark **Tag push events** and **Merge request events**. * Finish by clicking **Add Webhook** @@ -99,7 +99,7 @@ Read the Docs will sync versions and build your documentation when your Git repo * Leave the default **HTTP Method** as POST * For **Content type**, both *application/json* and *application/x-www-form-urlencoded* work - * Leave the **Secret** field blank + * Fill the **Secret** field with the value from the integration on Read the Docs * Select **Choose events**, and mark **Branch or tag creation**, **Branch or tag deletion** and **Push** events * Ensure **Active** is enabled; it is by default @@ -205,10 +205,14 @@ will be performed to ensure the authenticated user is an owner of the project. Payload validation ------------------ -If your project was imported through a connected account, -we create a secret for every integration that offers a way to verify that a webhook request is legitimate. -Currently, `GitHub `__ and `GitLab `__ -offer a way to check this. +All integrations are created with a secret token, +this offers a way to verify that a webhook request is legitimate. + +This validation is done according to each provider: + +- `GitHub `__ +- `GitLab `__ +- `Bitbucket `__ Troubleshooting --------------- diff --git a/readthedocs/api/v2/views/integrations.py b/readthedocs/api/v2/views/integrations.py index cc941a65ce8..9e7a0569537 100644 --- a/readthedocs/api/v2/views/integrations.py +++ b/readthedocs/api/v2/views/integrations.py @@ -8,6 +8,7 @@ import structlog from django.shortcuts import get_object_or_404 +from django.utils.crypto import constant_time_compare from rest_framework import permissions, status from rest_framework.exceptions import NotFound, ParseError from rest_framework.renderers import JSONRenderer @@ -29,8 +30,8 @@ log = structlog.get_logger(__name__) -GITHUB_EVENT_HEADER = "HTTP_X_GITHUB_EVENT" -GITHUB_SIGNATURE_HEADER = "HTTP_X_HUB_SIGNATURE" +GITHUB_EVENT_HEADER = "GitHub-Event" +GITHUB_SIGNATURE_HEADER = "X-Hub-Signature-256" GITHUB_PING = "ping" GITHUB_PUSH = "push" GITHUB_PULL_REQUEST = "pull_request" @@ -46,11 +47,12 @@ GITLAB_MERGE_REQUEST_OPEN = "open" GITLAB_MERGE_REQUEST_REOPEN = "reopen" GITLAB_MERGE_REQUEST_UPDATE = "update" -GITLAB_TOKEN_HEADER = "HTTP_X_GITLAB_TOKEN" +GITLAB_TOKEN_HEADER = "X-GitLab-Token" GITLAB_PUSH = "push" GITLAB_NULL_HASH = "0" * 40 GITLAB_TAG_PUSH = "tag_push" -BITBUCKET_EVENT_HEADER = "HTTP_X_EVENT_KEY" +BITBUCKET_EVENT_HEADER = "X-Event-Key" +BITBUCKET_SIGNATURE_HEADER = "X-Hub-Signature" BITBUCKET_PUSH = "repo:push" @@ -142,35 +144,35 @@ def is_payload_valid(self): """Validates the webhook's payload using the integration's secret.""" return False + @staticmethod + def get_digest(secret, msg): + """Get a HMAC digest of `msg` using `secret`.""" + digest = hmac.new( + secret.encode(), + msg=msg.encode(), + digestmod=hashlib.sha256, + ) + return digest.hexdigest() + def get_integration(self): """ Get or create an inbound webhook to track webhook requests. - We shouldn't need this, but to support legacy webhooks, we can't assume - that a webhook has ever been created on our side. Most providers don't - pass the webhook ID in either, so we default to just finding *any* - integration from the provider. This is not ideal, but the - :py:class:`WebhookView` view solves this by performing a lookup on the - integration instead of guessing. + Most providers don't pass the webhook ID in either, so we default + to just finding *any* integration from the provider. This is not ideal, + but the :py:class:`WebhookView` view solves this by performing a lookup + on the integration instead of guessing. """ # `integration` can be passed in as an argument to `as_view`, as it is # in `WebhookView` if self.integration is not None: return self.integration - try: - integration = Integration.objects.get( - project=self.project, - integration_type=self.integration_type, - ) - except Integration.DoesNotExist: - integration = Integration.objects.create( - project=self.project, - integration_type=self.integration_type, - # If we didn't create the integration, - # we didn't set a secret. - secret=None, - ) - return integration + self.integration = get_object_or_404( + Integration, + project=self.project, + integration_type=self.integration_type, + ) + return self.integration def get_response_push(self, project, branches): """ @@ -293,12 +295,15 @@ def update_default_branch(self, default_branch): In case the user already selected a `default-branch` from the "Advanced settings", it does not override it. + + This action can be performed only if the integration has a secret, + requests from anonymous users are ignored. """ - if not self.project.default_branch: - ( - self.project.versions.filter(slug=LATEST).update( - identifier=default_branch - ) + if self.get_integration().secret and not self.project.default_branch: + # Always check for the machine attribute, since latest can be user created. + # RTD doesn't manage those. + self.project.versions.filter(slug=LATEST, machine=True).update( + identifier=default_branch ) @@ -371,7 +376,7 @@ def is_payload_valid(self): See https://developer.github.com/webhooks/securing/. """ - signature = self.request.META.get(GITHUB_SIGNATURE_HEADER) + signature = self.request.headers.get(GITHUB_SIGNATURE_HEADER) secret = self.get_integration().secret if not secret: log.debug('Skipping payload signature validation.') @@ -379,23 +384,13 @@ def is_payload_valid(self): if not signature: return False msg = self.request.body.decode() - digest = GitHubWebhookView.get_digest(secret, msg) + digest = WebhookMixin.get_digest(secret, msg) result = hmac.compare_digest( - b'sha1=' + digest.encode(), + b"sha256=" + digest.encode(), signature.encode(), ) return result - @staticmethod - def get_digest(secret, msg): - """Get a HMAC digest of `msg` using `secret`.""" - digest = hmac.new( - secret.encode(), - msg=msg.encode(), - digestmod=hashlib.sha1, - ) - return digest.hexdigest() - def handle_webhook(self): """ Handle GitHub webhook events. @@ -429,7 +424,7 @@ def handle_webhook(self): action = self.data.get('action', None) created = self.data.get('created', False) deleted = self.data.get('deleted', False) - event = self.request.META.get(GITHUB_EVENT_HEADER, GITHUB_PUSH) + event = self.request.headers.get(GITHUB_EVENT_HEADER, GITHUB_PUSH) log.bind(webhook_event=event) webhook_github.send( Project, @@ -452,16 +447,20 @@ def handle_webhook(self): log.debug('Triggered sync_versions.') return self.sync_versions_response(self.project) - # Handle pull request events - if self.project.external_builds_enabled and event == GITHUB_PULL_REQUEST: - if ( - action in - [ - GITHUB_PULL_REQUEST_OPENED, - GITHUB_PULL_REQUEST_REOPENED, - GITHUB_PULL_REQUEST_SYNC - ] - ): + integration = self.get_integration() + + # Handle pull request events. + # Requests from anonymous users are ignored. + if ( + integration.secret + and self.project.external_builds_enabled + and event == GITHUB_PULL_REQUEST + ): + if action in [ + GITHUB_PULL_REQUEST_OPENED, + GITHUB_PULL_REQUEST_REOPENED, + GITHUB_PULL_REQUEST_SYNC, + ]: # Trigger a build when PR is opened/reopened/sync return self.get_external_version_response(self.project) @@ -474,7 +473,6 @@ def handle_webhook(self): event == GITHUB_PUSH, (created or deleted), ]): - integration = self.get_integration() events = integration.provider_data.get('events', []) if integration.provider_data else [] # noqa if any([ GITHUB_CREATE in events, @@ -556,14 +554,12 @@ def is_payload_valid(self): It is sent in the request's header. See https://docs.gitlab.com/ee/user/project/integrations/webhooks.html#secret-token. """ - token = self.request.META.get(GITLAB_TOKEN_HEADER) + token = self.request.headers.get(GITLAB_TOKEN_HEADER, "") secret = self.get_integration().secret if not secret: log.debug('Skipping payload signature validation.') return True - if not token: - return False - return token == secret + return constant_time_compare(secret, token) def get_external_version_data(self): """Get commit SHA and merge request number from payload.""" @@ -597,6 +593,8 @@ def handle_webhook(self): event=event, ) + integration = self.get_integration() + # Always update `latest` branch to point to the default branch in the repository # even if the event is not gonna be handled. This helps us to keep our db in sync. default_branch = self.data.get("project", {}).get("default_branch", None) @@ -623,15 +621,16 @@ def handle_webhook(self): except KeyError as exc: raise ParseError('Parameter "ref" is required') from exc - if self.project.external_builds_enabled and event == GITLAB_MERGE_REQUEST: - if ( - action in - [ - GITLAB_MERGE_REQUEST_OPEN, - GITLAB_MERGE_REQUEST_REOPEN, - GITLAB_MERGE_REQUEST_UPDATE - ] - ): + if ( + integration.secret + and self.project.external_builds_enabled + and event == GITLAB_MERGE_REQUEST + ): + if action in [ + GITLAB_MERGE_REQUEST_OPEN, + GITLAB_MERGE_REQUEST_REOPEN, + GITLAB_MERGE_REQUEST_UPDATE, + ]: # Handle open, update, reopen merge_request event. return self.get_external_version_response(self.project) @@ -687,7 +686,7 @@ def handle_webhook(self): it sets the new attribute (null if it is a deletion) and the old attribute (null if it is a creation). """ - event = self.request.META.get(BITBUCKET_EVENT_HEADER, BITBUCKET_PUSH) + event = self.request.headers.get(BITBUCKET_EVENT_HEADER, BITBUCKET_PUSH) log.bind(webhook_event=event) webhook_bitbucket.send( Project, @@ -727,8 +726,27 @@ def handle_webhook(self): return None def is_payload_valid(self): - """Bitbucket doesn't have an option for payload validation.""" - return True + """ + BitBucket use a HMAC hexdigest hash to sign the payload. + + It is sent in the request's header. + + See https://support.atlassian.com/bitbucket-cloud/docs/manage-webhooks/#Secure-webhooks. + """ + signature = self.request.headers.get(BITBUCKET_SIGNATURE_HEADER) + secret = self.get_integration().secret + if not secret: + log.debug("Skipping payload signature validation.") + return True + if not signature: + return False + msg = self.request.body.decode() + digest = WebhookMixin.get_digest(secret, msg) + result = hmac.compare_digest( + b"sha256=" + digest.encode(), + signature.encode(), + ) + return result class IsAuthenticatedOrHasToken(permissions.IsAuthenticated): @@ -783,9 +801,8 @@ def get_project(self, **kwargs): if token: integration = self.get_integration() obj = Project.objects.get(**kwargs) - is_valid = ( - integration.project == obj and - token == getattr(integration, 'token', None) + is_valid = integration.project == obj and constant_time_compare( + token, getattr(integration, "token", "") ) if is_valid: return obj @@ -810,10 +827,10 @@ def handle_webhook(self): def is_payload_valid(self): """ - We can't have payload validation in the generic webhook. + Generic webhooks don't have payload validation. - Since we don't know the system that would trigger the webhook. - We have a token for authentication. + We use basic auth or token auth to validate that the user has access to + the project and integration (get_project() method). """ return True diff --git a/readthedocs/integrations/migrations/0005_change_default_integration_secret.py b/readthedocs/integrations/migrations/0005_change_default_integration_secret.py index 1b105cfa5f8..9964153ab01 100644 --- a/readthedocs/integrations/migrations/0005_change_default_integration_secret.py +++ b/readthedocs/integrations/migrations/0005_change_default_integration_secret.py @@ -1,8 +1,6 @@ # Generated by Django 1.11.16 on 2018-12-10 22:08 from django.db import migrations, models -import readthedocs.integrations.utils - class Migration(migrations.Migration): dependencies = [ @@ -15,7 +13,6 @@ class Migration(migrations.Migration): name="secret", field=models.CharField( blank=True, - default=readthedocs.integrations.utils.get_secret, help_text="Secret used to validate the payload of the webhook", max_length=255, null=True, diff --git a/readthedocs/integrations/models.py b/readthedocs/integrations/models.py index 66455416f8d..f508d0c4c2f 100644 --- a/readthedocs/integrations/models.py +++ b/readthedocs/integrations/models.py @@ -10,6 +10,7 @@ ) from django.contrib.contenttypes.models import ContentType from django.db import models, transaction +from django.utils.crypto import get_random_string from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ from pygments import highlight @@ -20,7 +21,7 @@ from readthedocs.core.fields import default_token from readthedocs.projects.models import Project -from .utils import get_secret, normalize_request_payload +from .utils import normalize_request_payload class HttpExchangeManager(models.Manager): @@ -303,7 +304,6 @@ class Integration(models.Model): max_length=255, blank=True, null=True, - default=get_secret, ) objects = IntegrationQuerySet.as_manager() @@ -311,19 +311,16 @@ class Integration(models.Model): # Integration attributes has_sync = False - def recreate_secret(self): - self.secret = get_secret() - self.save(update_fields=["secret"]) - - def remove_secret(self): - self.secret = None - self.save(update_fields=["secret"]) - def __str__(self): return _("{0} for {1}").format( self.get_integration_type_display(), self.project.name ) + def save(self, *args, **kwargs): + if not self.secret: + self.secret = get_random_string(length=32) + super().save(*args, **kwargs) + class GitHubWebhook(Integration): integration_type_id = Integration.GITHUB_WEBHOOK diff --git a/readthedocs/integrations/utils.py b/readthedocs/integrations/utils.py index b6b08cbdc81..98eb1851567 100644 --- a/readthedocs/integrations/utils.py +++ b/readthedocs/integrations/utils.py @@ -1,8 +1,5 @@ """Integration utility functions.""" -import os - - def normalize_request_payload(request): """ Normalize the request body, hopefully to JSON. @@ -23,13 +20,3 @@ def normalize_request_payload(request): except AttributeError: pass return request_payload - - -def get_secret(size=64): - """ - Get a random string of `size` bytes. - - :param size: Number of bytes - """ - secret = os.urandom(size) - return secret.hex() diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index 52197a477d4..7ffab66f3e8 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -214,6 +214,7 @@ def get_webhook_data(self, project, integration): ), "url": self.get_webhook_url(project, integration), "active": True, + "secret": integration.secret, "events": ["repo:push"], } ) @@ -290,6 +291,7 @@ def setup_webhook(self, project, integration=None): project=project, integration_type=Integration.BITBUCKET_WEBHOOK, ) + data = self.get_webhook_data(project, integration) resp = None log.bind( diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index de58a5bbaf9..fb95492a81d 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -295,9 +295,6 @@ def setup_webhook(self, project, integration=None): integration_type=Integration.GITHUB_WEBHOOK, ) - if not integration.secret: - integration.recreate_secret() - data = self.get_webhook_data(project, integration) url = f"https://api.github.com/repos/{owner}/{repo}/hooks" log.bind( @@ -341,8 +338,6 @@ def setup_webhook(self, project, integration=None): except (RequestException, ValueError): log.exception("GitHub webhook creation failed for project.") - # Always remove the secret and return False if we don't return True above - integration.remove_secret() return (False, resp) def update_webhook(self, project, integration): @@ -357,8 +352,6 @@ def update_webhook(self, project, integration): :rtype: (Bool, Response) """ session = self.get_session() - if not integration.secret: - integration.recreate_secret() data = self.get_webhook_data(project, integration) resp = None @@ -412,7 +405,6 @@ def update_webhook(self, project, integration): except (AttributeError, RequestException, ValueError): log.exception("GitHub webhook update failed for project.") - integration.remove_secret() return (False, resp) def send_build_status(self, build, commit, status): diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index 7f244d3f0b0..1af481193d0 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -382,15 +382,10 @@ def setup_webhook(self, project, integration=None): integration_type=Integration.GITLAB_WEBHOOK, ) - if not integration.secret: - integration.recreate_secret() - repo_id = self._get_repo_id(project) url = f"{self.adapter.provider_base_url}/api/v4/projects/{repo_id}/hooks" if repo_id is None: - # Set the secret to None so that the integration can be used manually. - integration.remove_secret() return (False, resp) log.bind( @@ -426,8 +421,6 @@ def setup_webhook(self, project, integration=None): except Exception: log.exception("GitLab webhook creation failed.") - # Always remove secret and return False if we don't return True above - integration.remove_secret() return (False, resp) def update_webhook(self, project, integration): @@ -459,9 +452,6 @@ def update_webhook(self, project, integration): if repo_id is None: return (False, resp) - if not integration.secret: - integration.recreate_secret() - data = self.get_webhook_data(repo_id, project, integration) log.bind( @@ -504,7 +494,6 @@ def update_webhook(self, project, integration): debug_data=debug_data, ) - integration.remove_secret() return (False, resp) def send_build_status(self, build, commit, status): diff --git a/readthedocs/oauth/utils.py b/readthedocs/oauth/utils.py index e2cc98c30e3..7d527e1a066 100644 --- a/readthedocs/oauth/utils.py +++ b/readthedocs/oauth/utils.py @@ -25,6 +25,10 @@ def update_webhook(project, integration, request=None): if service_cls is None: return None + # TODO: remove after integrations without a secret are removed. + if not integration.secret: + integration.save() + updated = False if project.remote_repository: remote_repository_relations = ( diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index fc812c54a7b..ecdd29f16d5 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -839,8 +839,8 @@ class IntegrationForm(forms.ModelForm): class Meta: model = Integration fields = [ - 'project', - 'integration_type', + "project", + "integration_type", ] def __init__(self, *args, **kwargs): @@ -854,9 +854,6 @@ def clean_project(self): def save(self, commit=True): self.instance = Integration.objects.subclass(self.instance) - # We don't set the secret on the integration - # when it's created via the form. - self.instance.secret = None return super().save(commit) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 3ae6d4752b5..c6e0812070f 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -14,6 +14,8 @@ from readthedocs.api.v2.models import BuildAPIKey from readthedocs.api.v2.views.integrations import ( + BITBUCKET_EVENT_HEADER, + BITBUCKET_SIGNATURE_HEADER, GITHUB_CREATE, GITHUB_DELETE, GITHUB_EVENT_HEADER, @@ -36,6 +38,7 @@ GITLAB_TOKEN_HEADER, GitHubWebhookView, GitLabWebhookView, + WebhookMixin, ) from readthedocs.builds.constants import ( BUILD_STATE_CLONING, @@ -46,7 +49,7 @@ LATEST, ) from readthedocs.builds.models import APIVersion, Build, BuildCommandResult, Version -from readthedocs.integrations.models import Integration +from readthedocs.integrations.models import GenericAPIWebhook, Integration from readthedocs.oauth.models import ( RemoteOrganization, RemoteOrganizationRelation, @@ -65,6 +68,18 @@ from readthedocs.subscriptions.products import RTDProductFeature +def get_signature(integration, payload): + if not isinstance(payload, str): + payload = json.dumps(payload, separators=(",", ":")) + return "sha256=" + WebhookMixin.get_digest( + secret=integration.secret, + # When the test client sends the payload, it doesn't + # separate the json keys with spaces, so when getting + # the digest, we need to remove the spaces. + msg=payload, + ) + + @override_settings(PUBLIC_DOMAIN="readthedocs.io") class APIBuildTests(TestCase): fixtures = ['eric.json', 'test_data.json'] @@ -1689,6 +1704,27 @@ def setUp(self): }, } + self.github_integration = get( + Integration, + project=self.project, + integration_type=Integration.GITHUB_WEBHOOK, + ) + self.gitlab_integration = get( + Integration, + project=self.project, + integration_type=Integration.GITLAB_WEBHOOK, + ) + self.bitbucket_integration = get( + Integration, + project=self.project, + integration_type=Integration.BITBUCKET_WEBHOOK, + ) + self.generic_integration = get( + GenericAPIWebhook, + project=self.project, + integration_type=Integration.API_WEBHOOK, + ) + def test_webhook_skipped_project(self, trigger_build): client = APIClient() self.project.skip = True @@ -1699,7 +1735,12 @@ def test_webhook_skipped_project(self, trigger_build): self.project.slug, ), self.github_payload, - format='json', + format="json", + headers={ + GITHUB_SIGNATURE_HEADER: get_signature( + self.github_integration, self.github_payload + ), + }, ) self.assertDictEqual(response.data, {'detail': 'This project is currently disabled'}) self.assertEqual(response.status_code, status.HTTP_406_NOT_ACCEPTABLE) @@ -1711,12 +1752,17 @@ def test_sync_repository_custom_project_queue(self, sync_repository_task, trigge self.project.build_queue = 'specific-build-queue' self.project.save() - headers = {GITHUB_EVENT_HEADER: GITHUB_CREATE} + headers = { + GITHUB_EVENT_HEADER: GITHUB_CREATE, + GITHUB_SIGNATURE_HEADER: get_signature( + self.github_integration, self.github_payload + ), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), self.github_payload, format='json', - **headers, + headers=headers, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -1739,28 +1785,40 @@ def test_github_webhook_for_branches(self, trigger_build): """GitHub webhook API.""" client = APIClient() + data = {"ref": "master"} client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), - {'ref': 'master'}, - format='json', + data, + format="json", + headers={ + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, data), + }, ) trigger_build.assert_has_calls( [mock.call(version=self.version, project=self.project)], ) + data = {"ref": "non-existent"} client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), - {'ref': 'non-existent'}, - format='json', + data, + format="json", + headers={ + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, data), + }, ) trigger_build.assert_has_calls( [mock.call(version=mock.ANY, project=self.project)], ) + data = {"ref": "refs/heads/master"} client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), - {'ref': 'refs/heads/master'}, - format='json', + data, + format="json", + headers={ + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, data), + }, ) trigger_build.assert_has_calls( [mock.call(version=self.version, project=self.project)], @@ -1769,29 +1827,41 @@ def test_github_webhook_for_branches(self, trigger_build): def test_github_webhook_for_tags(self, trigger_build): """GitHub webhook API.""" client = APIClient() + data = {"ref": "v1.0"} client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), - {'ref': 'v1.0'}, - format='json', + data, + format="json", + headers={ + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, data), + }, ) trigger_build.assert_has_calls( [mock.call(version=self.version_tag, project=self.project)], ) + data = {"ref": "refs/heads/non-existent"} client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), - {'ref': 'refs/heads/non-existent'}, + data, format='json', + headers={ + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, data), + }, ) trigger_build.assert_has_calls( [mock.call(version=mock.ANY, project=self.project)], ) + data = {"ref": "refs/tags/v1.0"} client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), - {'ref': 'refs/tags/v1.0'}, + data, format='json', + headers={ + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, data), + }, ) trigger_build.assert_has_calls( [mock.call(version=self.version_tag, project=self.project)], @@ -1802,12 +1872,15 @@ def test_github_webhook_no_build_on_delete(self, sync_repository_task, trigger_b client = APIClient() payload = {'ref': 'master', 'deleted': True} - headers = {GITHUB_EVENT_HEADER: GITHUB_PUSH} + headers = { + GITHUB_EVENT_HEADER: GITHUB_PUSH, + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, payload), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -1823,12 +1896,17 @@ def test_github_webhook_no_build_on_delete(self, sync_repository_task, trigger_b def test_github_ping_event(self, sync_repository_task, trigger_build): client = APIClient() - headers = {GITHUB_EVENT_HEADER: GITHUB_PING} + headers = { + GITHUB_EVENT_HEADER: GITHUB_PING, + GITHUB_SIGNATURE_HEADER: get_signature( + self.github_integration, self.github_payload + ), + } resp = client.post( "/api/v2/webhook/github/{}/".format(self.project.slug), self.github_payload, format="json", - **headers, + headers=headers, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertDictEqual(resp.data, {"detail": "Webhook configured correctly"}) @@ -1839,12 +1917,17 @@ def test_github_ping_event(self, sync_repository_task, trigger_build): def test_github_create_event(self, sync_repository_task, trigger_build): client = APIClient() - headers = {GITHUB_EVENT_HEADER: GITHUB_CREATE} + headers = { + GITHUB_EVENT_HEADER: GITHUB_CREATE, + GITHUB_SIGNATURE_HEADER: get_signature( + self.github_integration, self.github_payload + ), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), self.github_payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -1860,12 +1943,17 @@ def test_github_create_event(self, sync_repository_task, trigger_build): def test_github_pull_request_opened_event(self, trigger_build, core_trigger_build): client = APIClient() - headers = {GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST} + headers = { + GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST, + GITHUB_SIGNATURE_HEADER: get_signature( + self.github_integration, self.github_pull_request_payload + ), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), self.github_pull_request_payload, format='json', - **headers + headers=headers, ) # get the created external version external_version = self.project.versions( @@ -1892,12 +1980,15 @@ def test_github_pull_request_reopened_event(self, trigger_build, core_trigger_bu payload["action"] = GITHUB_PULL_REQUEST_REOPENED payload["number"] = pull_request_number - headers = {GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST} + headers = { + GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST, + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, payload), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), payload, format='json', - **headers + headers=headers, ) # get the created external version external_version = self.project.versions( @@ -1937,12 +2028,15 @@ def test_github_pull_request_synchronize_event(self, trigger_build, core_trigger payload["action"] = GITHUB_PULL_REQUEST_SYNC payload["number"] = pull_request_number - headers = {GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST} + headers = { + GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST, + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, payload), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), payload, format='json', - **headers + headers=headers, ) # get updated external version external_version = self.project.versions( @@ -1984,12 +2078,15 @@ def test_github_pull_request_closed_event(self, trigger_build, core_trigger_buil payload["number"] = pull_request_number payload["pull_request"]["head"]["sha"] = identifier - headers = {GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST} + headers = { + GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST, + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, payload), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), payload, format='json', - **headers + headers=headers, ) external_version = self.project.versions( manager=EXTERNAL @@ -2014,12 +2111,15 @@ def test_github_pull_request_no_action(self, trigger_build): } } } - headers = {GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST} + headers = { + GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST, + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, payload), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 200) self.assertEqual(resp.data['detail'], 'Unhandled webhook event') @@ -2036,7 +2136,7 @@ def test_github_pull_request_opened_event_invalid_payload(self, trigger_build): '/api/v2/webhook/github/{}/'.format(self.project.slug), payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 400) @@ -2053,7 +2153,7 @@ def test_github_pull_request_closed_event_invalid_payload(self, trigger_build): '/api/v2/webhook/github/{}/'.format(self.project.slug), payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 400) @@ -2062,12 +2162,17 @@ def test_github_pull_request_closed_event_invalid_payload(self, trigger_build): def test_github_delete_event(self, sync_repository_task, trigger_build): client = APIClient() - headers = {GITHUB_EVENT_HEADER: GITHUB_DELETE} + headers = { + GITHUB_EVENT_HEADER: GITHUB_DELETE, + GITHUB_SIGNATURE_HEADER: get_signature( + self.github_integration, self.github_payload + ), + } resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), self.github_payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -2092,23 +2197,25 @@ def test_github_parse_ref(self, trigger_build): def test_github_invalid_webhook(self, trigger_build): """GitHub webhook unhandled event.""" client = APIClient() + payload = {"foo": "bar"} resp = client.post( '/api/v2/webhook/github/{}/'.format(self.project.slug), - {'foo': 'bar'}, + payload, format='json', - HTTP_X_GITHUB_EVENT='issues', + headers={ + GITHUB_EVENT_HEADER: "issues", + GITHUB_SIGNATURE_HEADER: get_signature( + self.github_integration, payload + ), + }, ) self.assertEqual(resp.status_code, 200) self.assertEqual(resp.data['detail'], 'Unhandled webhook event') def test_github_invalid_payload(self, trigger_build): client = APIClient() - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITHUB_WEBHOOK, - ) wrong_signature = '1234' - self.assertNotEqual(integration.secret, wrong_signature) + self.assertNotEqual(self.github_integration.secret, wrong_signature) headers = { GITHUB_EVENT_HEADER: GITHUB_PUSH, GITHUB_SIGNATURE_HEADER: wrong_signature, @@ -2120,7 +2227,7 @@ def test_github_invalid_payload(self, trigger_build): ), self.github_payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 400) self.assertEqual( @@ -2131,17 +2238,13 @@ def test_github_invalid_payload(self, trigger_build): def test_github_valid_payload(self, trigger_build): client = APIClient() payload = '{"ref":"master"}' - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITHUB_WEBHOOK, - ) - digest = GitHubWebhookView.get_digest( - integration.secret, + signature = get_signature( + self.github_integration, payload, ) headers = { GITHUB_EVENT_HEADER: GITHUB_PUSH, - GITHUB_SIGNATURE_HEADER: 'sha1=' + digest, + GITHUB_SIGNATURE_HEADER: signature, } resp = client.post( reverse( @@ -2150,7 +2253,7 @@ def test_github_valid_payload(self, trigger_build): ), json.loads(payload), format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 200) @@ -2160,10 +2263,6 @@ def test_github_empty_signature(self, trigger_build): GITHUB_EVENT_HEADER: GITHUB_PUSH, GITHUB_SIGNATURE_HEADER: '', } - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITHUB_WEBHOOK, - ) resp = client.post( reverse( 'api_webhook_github', @@ -2171,7 +2270,7 @@ def test_github_empty_signature(self, trigger_build): ), self.github_payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 400) self.assertEqual( @@ -2182,12 +2281,7 @@ def test_github_empty_signature(self, trigger_build): def test_github_skip_signature_validation(self, trigger_build): client = APIClient() payload = '{"ref":"master"}' - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITHUB_WEBHOOK, - secret=None, - ) - self.assertFalse(integration.secret) + Integration.objects.filter(pk=self.github_integration.pk).update(secret=None) headers = { GITHUB_EVENT_HEADER: GITHUB_PUSH, GITHUB_SIGNATURE_HEADER: 'skipped', @@ -2199,32 +2293,29 @@ def test_github_skip_signature_validation(self, trigger_build): ), json.loads(payload), format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 200) @mock.patch('readthedocs.core.views.hooks.sync_repository_task', mock.MagicMock()) def test_github_sync_on_push_event(self, trigger_build): """Sync if the webhook doesn't have the create/delete events, but we receive a push event with created/deleted.""" - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITHUB_WEBHOOK, - provider_data={ - 'events': [], - }, - secret=None, - ) + self.github_integration.provider_data = { + "events": [], + } + self.github_integration.save() client = APIClient() - headers = { - GITHUB_EVENT_HEADER: GITHUB_PUSH, - } payload = { 'ref': 'master', 'created': True, 'deleted': False, } + headers = { + GITHUB_EVENT_HEADER: GITHUB_PUSH, + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, payload), + } resp = client.post( reverse( 'api_webhook_github', @@ -2232,35 +2323,32 @@ def test_github_sync_on_push_event(self, trigger_build): ), payload, format='json', - **headers + headers=headers, ) self.assertTrue(resp.json()['versions_synced']) @mock.patch('readthedocs.core.views.hooks.sync_repository_task', mock.MagicMock()) def test_github_dont_trigger_double_sync(self, trigger_build): """Don't trigger a sync twice if the webhook has the create/delete events.""" - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITHUB_WEBHOOK, - provider_data={ - 'events': [ - GITHUB_CREATE, - GITHUB_DELETE, - ], - }, - secret=None, - ) + self.github_integration.provider_data = { + "events": [ + GITHUB_CREATE, + GITHUB_DELETE, + ], + } + self.github_integration.save() client = APIClient() - headers = { - GITHUB_EVENT_HEADER: GITHUB_PUSH, - } payload = { 'ref': 'master', 'created': True, 'deleted': False, } + headers = { + GITHUB_EVENT_HEADER: GITHUB_PUSH, + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, payload), + } resp = client.post( reverse( 'api_webhook_github', @@ -2268,14 +2356,15 @@ def test_github_dont_trigger_double_sync(self, trigger_build): ), payload, format='json', - **headers + headers=headers, ) self.assertFalse(resp.json()['versions_synced']) + payload = {"ref": "master"} headers = { GITHUB_EVENT_HEADER: GITHUB_CREATE, + GITHUB_SIGNATURE_HEADER: get_signature(self.github_integration, payload), } - payload = {'ref': 'master'} resp = client.post( reverse( 'api_webhook_github', @@ -2283,7 +2372,7 @@ def test_github_dont_trigger_double_sync(self, trigger_build): ), payload, format='json', - **headers + headers=headers, ) self.assertTrue(resp.json()['versions_synced']) @@ -2298,10 +2387,14 @@ def test_github_get_external_version_data(self, trigger_build): def test_gitlab_webhook_for_branches(self, trigger_build): """GitLab webhook API.""" client = APIClient() + headers = { + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + } client.post( '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), self.gitlab_payload, format='json', + headers=headers, ) trigger_build.assert_called_with( version=mock.ANY, project=self.project, @@ -2324,10 +2417,14 @@ def test_gitlab_webhook_for_tags(self, trigger_build): object_kind=GITLAB_TAG_PUSH, ref='v1.0', ) + headers = { + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + } client.post( '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), self.gitlab_payload, format='json', + headers=headers, ) trigger_build.assert_called_with( version=self.version_tag, project=self.project, @@ -2341,6 +2438,7 @@ def test_gitlab_webhook_for_tags(self, trigger_build): '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), self.gitlab_payload, format='json', + headers=headers, ) trigger_build.assert_called_with( version=self.version_tag, project=self.project, @@ -2354,6 +2452,7 @@ def test_gitlab_webhook_for_tags(self, trigger_build): '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), self.gitlab_payload, format='json', + headers=headers, ) trigger_build.assert_not_called() @@ -2370,6 +2469,9 @@ def test_gitlab_push_hook_creation( '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), self.gitlab_payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -2394,6 +2496,9 @@ def test_gitlab_push_hook_deletion( '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), self.gitlab_payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -2419,6 +2524,9 @@ def test_gitlab_tag_push_hook_creation( '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), self.gitlab_payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -2444,6 +2552,9 @@ def test_gitlab_tag_push_hook_deletion( '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), self.gitlab_payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -2462,6 +2573,9 @@ def test_gitlab_invalid_webhook(self, trigger_build): '/api/v2/webhook/gitlab/{}/'.format(self.project.slug), {'object_kind': 'pull_request'}, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) self.assertEqual(resp.status_code, 200) self.assertEqual(resp.data['detail'], 'Unhandled webhook event') @@ -2469,11 +2583,7 @@ def test_gitlab_invalid_webhook(self, trigger_build): def test_gitlab_invalid_payload(self, trigger_build): client = APIClient() wrong_secret = '1234' - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITLAB_WEBHOOK, - ) - self.assertNotEqual(integration.secret, wrong_secret) + self.assertNotEqual(self.gitlab_integration.secret, wrong_secret) headers = { GITLAB_TOKEN_HEADER: wrong_secret, } @@ -2484,7 +2594,7 @@ def test_gitlab_invalid_payload(self, trigger_build): ), self.gitlab_payload, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 400) self.assertEqual( @@ -2494,12 +2604,8 @@ def test_gitlab_invalid_payload(self, trigger_build): def test_gitlab_valid_payload(self, trigger_build): client = APIClient() - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITLAB_WEBHOOK, - ) headers = { - GITLAB_TOKEN_HEADER: integration.secret, + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, } resp = client.post( reverse( @@ -2508,16 +2614,12 @@ def test_gitlab_valid_payload(self, trigger_build): ), {'object_kind': 'pull_request'}, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 200) def test_gitlab_empty_token(self, trigger_build): client = APIClient() - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITLAB_WEBHOOK, - ) headers = { GITLAB_TOKEN_HEADER: '', } @@ -2528,7 +2630,7 @@ def test_gitlab_empty_token(self, trigger_build): ), {'object_kind': 'pull_request'}, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 400) self.assertEqual( @@ -2538,12 +2640,7 @@ def test_gitlab_empty_token(self, trigger_build): def test_gitlab_skip_token_validation(self, trigger_build): client = APIClient() - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.GITLAB_WEBHOOK, - secret=None, - ) - self.assertFalse(integration.secret) + Integration.objects.filter(pk=self.gitlab_integration.pk).update(secret=None) headers = { GITLAB_TOKEN_HEADER: 'skipped', } @@ -2554,7 +2651,7 @@ def test_gitlab_skip_token_validation(self, trigger_build): ), {'object_kind': 'pull_request'}, format='json', - **headers + headers=headers, ) self.assertEqual(resp.status_code, 200) @@ -2569,6 +2666,9 @@ def test_gitlab_merge_request_open_event(self, trigger_build, core_trigger_build ), self.gitlab_merge_request_payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) # get the created external version external_version = self.project.versions( @@ -2602,6 +2702,9 @@ def test_gitlab_merge_request_reopen_event(self, trigger_build, core_trigger_bui ), payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) # get the created external version external_version = self.project.versions( @@ -2648,6 +2751,9 @@ def test_gitlab_merge_request_update_event(self, trigger_build, core_trigger_bui ), payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) # get updated external version external_version = self.project.versions( @@ -2696,6 +2802,9 @@ def test_gitlab_merge_request_close_event(self, trigger_build, core_trigger_buil ), payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) external_version = self.project.versions( manager=EXTERNAL @@ -2740,6 +2849,9 @@ def test_gitlab_merge_request_merge_event(self, trigger_build, core_trigger_buil ), payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) external_version = self.project.versions( manager=EXTERNAL @@ -2774,6 +2886,9 @@ def test_gitlab_merge_request_no_action(self, trigger_build): ), payload, format='json', + headers={ + GITLAB_TOKEN_HEADER: self.gitlab_integration.secret, + }, ) self.assertEqual(resp.status_code, 200) self.assertEqual(resp.data['detail'], 'Unhandled webhook event') @@ -2834,6 +2949,11 @@ def test_bitbucket_webhook(self, trigger_build): '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), self.bitbucket_payload, format='json', + headers={ + BITBUCKET_SIGNATURE_HEADER: get_signature( + self.bitbucket_integration, self.bitbucket_payload + ), + }, ) trigger_build.assert_has_calls( [mock.call(version=mock.ANY, project=self.project)], @@ -2882,6 +3002,11 @@ def test_bitbucket_push_hook_creation( '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), self.bitbucket_payload, format='json', + headers={ + BITBUCKET_SIGNATURE_HEADER: get_signature( + self.bitbucket_integration, self.bitbucket_payload + ), + }, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -2903,6 +3028,11 @@ def test_bitbucket_push_hook_deletion( '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), self.bitbucket_payload, format='json', + headers={ + BITBUCKET_SIGNATURE_HEADER: get_signature( + self.bitbucket_integration, self.bitbucket_payload + ), + }, ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertFalse(resp.data['build_triggered']) @@ -2917,9 +3047,17 @@ def test_bitbucket_push_hook_deletion( def test_bitbucket_invalid_webhook(self, trigger_build): """Bitbucket webhook unhandled event.""" client = APIClient() + payload = {"foo": "bar"} resp = client.post( '/api/v2/webhook/bitbucket/{}/'.format(self.project.slug), - {'foo': 'bar'}, format='json', HTTP_X_EVENT_KEY='pull_request', + payload, + format="json", + headers={ + BITBUCKET_EVENT_HEADER: "pull_request", + BITBUCKET_SIGNATURE_HEADER: get_signature( + self.bitbucket_integration, payload + ), + }, ) self.assertEqual(resp.status_code, 200) self.assertEqual(resp.data['detail'], 'Unhandled webhook event') @@ -2939,18 +3077,14 @@ def test_generic_api_fails_without_auth(self, trigger_build): def test_generic_api_respects_token_auth(self, trigger_build): client = APIClient() - integration = Integration.objects.create( - project=self.project, - integration_type=Integration.API_WEBHOOK, - ) - self.assertIsNotNone(integration.token) + self.assertIsNotNone(self.generic_integration.token) resp = client.post( '/api/v2/webhook/{}/{}/'.format( self.project.slug, - integration.pk, + self.generic_integration.pk, ), - {'token': integration.token}, - format='json', + {"token": self.generic_integration.token}, + format="json", ) self.assertEqual(resp.status_code, 200) self.assertTrue(resp.data['build_triggered']) @@ -2958,10 +3092,10 @@ def test_generic_api_respects_token_auth(self, trigger_build): resp = client.post( '/api/v2/webhook/{}/{}/'.format( self.project.slug, - integration.pk, + self.generic_integration.pk, ), - {'token': integration.token, 'branches': 'nonexistent'}, - format='json', + {"token": self.generic_integration.token, "branches": "nonexistent"}, + format="json", ) self.assertEqual(resp.status_code, 200) self.assertFalse(resp.data['build_triggered']) diff --git a/readthedocs/rtd_tests/tests/test_integrations.py b/readthedocs/rtd_tests/tests/test_integrations.py index 4ca41ba0765..76b39141a3d 100644 --- a/readthedocs/rtd_tests/tests/test_integrations.py +++ b/readthedocs/rtd_tests/tests/test_integrations.py @@ -2,12 +2,10 @@ from django.test import TestCase from rest_framework.test import APIClient -from readthedocs.integrations.models import ( - GitHubWebhook, - HttpExchange, - Integration, -) +from readthedocs.api.v2.views.integrations import GITHUB_SIGNATURE_HEADER +from readthedocs.integrations.models import GitHubWebhook, HttpExchange, Integration from readthedocs.projects.models import Project +from readthedocs.rtd_tests.tests.test_api import get_signature class HttpExchangeTests(TestCase): @@ -28,12 +26,16 @@ def test_exchange_json_request_body(self): project=project, integration_type=Integration.GITHUB_WEBHOOK, provider_data="", - secret=None, ) + payload = {"ref": "exchange_json"} + signature = get_signature(integration, payload) resp = client.post( "/api/v2/webhook/github/{}/".format(project.slug), - {"ref": "exchange_json"}, + payload, format="json", + headers={ + GITHUB_SIGNATURE_HEADER: signature, + }, ) exchange = HttpExchange.objects.get(integrations=integration) self.assertEqual( @@ -45,6 +47,7 @@ def test_exchange_json_request_body(self): { "Content-Type": "application/json", "Cookie": "", + "X-Hub-Signature-256": signature, }, ) self.assertEqual( @@ -74,10 +77,15 @@ def test_exchange_form_request_body(self): provider_data="", secret=None, ) + payload = "payload=%7B%22ref%22%3A+%22exchange_form%22%7D" + signature = get_signature(integration, payload) resp = client.post( "/api/v2/webhook/github/{}/".format(project.slug), - "payload=%7B%22ref%22%3A+%22exchange_form%22%7D", + payload, content_type="application/x-www-form-urlencoded", + headers={ + GITHUB_SIGNATURE_HEADER: signature, + }, ) exchange = HttpExchange.objects.get(integrations=integration) self.assertEqual( @@ -89,6 +97,7 @@ def test_exchange_form_request_body(self): { "Content-Type": "application/x-www-form-urlencoded", "Cookie": "", + "X-Hub-Signature-256": signature, }, ) self.assertEqual( diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 90940945b43..0038306655c 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -380,7 +380,7 @@ def test_setup_webhook_404_error(self, session, mock_logger): self.integration.refresh_from_db() self.assertFalse(success) - self.assertIsNone(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.bind.assert_called_with(http_status_code=404) mock_logger.warning.assert_called_with( "GitHub project does not exist or user does not have permissions.", @@ -394,7 +394,7 @@ def test_setup_webhook_value_error(self, session, mock_logger): self.integration.refresh_from_db() - self.assertIsNone(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.bind.assert_called_with( project_slug=self.project.slug, integration_id=self.integration.pk, @@ -450,7 +450,7 @@ def test_update_webhook_value_error(self, session, mock_logger): self.integration.refresh_from_db() - self.assertIsNone(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.exception.assert_called_with( "GitHub webhook update failed for project." ) @@ -1239,7 +1239,7 @@ def test_setup_webhook_404_error(self, session, mock_logger): self.integration.refresh_from_db() self.assertFalse(success) - self.assertIsNone(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.bind.assert_called_with(http_status_code=404) mock_logger.info.assert_called_with( "Gitlab project does not exist or user does not have permissions.", @@ -1253,7 +1253,7 @@ def test_setup_webhook_value_error(self, session, mock_logger): self.integration.refresh_from_db() - self.assertIsNone(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.bind.assert_called_with( project_slug=self.project.slug, integration_id=self.integration.pk, @@ -1317,7 +1317,7 @@ def test_update_webhook_value_error(self, repo_id, session, mock_logger): self.integration.refresh_from_db() - self.assertIsNone(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.bind.assert_called_with( project_slug=self.project.slug, integration_id=self.integration.pk, diff --git a/readthedocs/templates/projects/integration_webhook_detail.html b/readthedocs/templates/projects/integration_webhook_detail.html index 011be85e4ec..d8057b223fe 100644 --- a/readthedocs/templates/projects/integration_webhook_detail.html +++ b/readthedocs/templates/projects/integration_webhook_detail.html @@ -60,7 +60,25 @@ {{ PRODUCTION_DOMAIN }}{{ webhook_url }}

- {% block integration_details %}{% endblock %} + {% block integration_details %} +
+ {% if integration.secret %} +

+ + +

+ {% else %} +

+ {% blocktrans trimmed %} + This integration does not have a secret, + the authenticity of the incoming webhook cannot be verified. + Click on "Resync webhook" to generate a secret. + Read more in our blog post. + {% endblocktrans %} +

+ {% endif %} +
+ {% endblock %}

{% blocktrans trimmed %}