From 9303267a078ecfb6788df51a8b3ff5fb83f67e8d Mon Sep 17 00:00:00 2001 From: Novak Zaballa <41410593+novakzaballa@users.noreply.github.com> Date: Thu, 9 May 2024 08:57:10 -0400 Subject: [PATCH] feat: Implement GitHub Webhook (#3906) --- api/api/urls/v1.py | 3 + api/app/settings/common.py | 2 +- api/integrations/github/helpers.py | 20 +++ api/integrations/github/views.py | 44 ++++-- .../github/test_unit_github_views.py | 133 ++++++++++++++++++ .../production/ecs-task-definition-web.json | 4 + .../aws/staging/ecs-task-definition-web.json | 4 + 7 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 api/integrations/github/helpers.py diff --git a/api/api/urls/v1.py b/api/api/urls/v1.py index e72ad880530f..088a58cbb03d 100644 --- a/api/api/urls/v1.py +++ b/api/api/urls/v1.py @@ -10,6 +10,7 @@ from environments.identities.views import SDKIdentities from environments.sdk.views import SDKEnvironmentAPIView from features.views import SDKFeatureStates +from integrations.github.views import github_webhook from organisations.views import chargebee_webhook schema_view = get_schema_view( @@ -44,6 +45,8 @@ url(r"^metadata/", include("metadata.urls")), # Chargebee webhooks url(r"cb-webhook/", chargebee_webhook, name="chargebee-webhook"), + # GitHub integration webhook + url(r"github-webhook/", github_webhook, name="github-webhook"), # Client SDK urls url(r"^flags/$", SDKFeatureStates.as_view(), name="flags"), url(r"^identities/$", SDKIdentities.as_view(), name="sdk-identities"), diff --git a/api/app/settings/common.py b/api/app/settings/common.py index cda611bbf039..b965dc8bf8da 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -893,7 +893,7 @@ # GitHub integrations GITHUB_PEM = env.str("GITHUB_PEM", default="") GITHUB_APP_ID: int = env.int("GITHUB_APP_ID", default=0) - +GITHUB_WEBHOOK_SECRET = env.str("GITHUB_WEBHOOK_SECRET", default="") # MailerLite MAILERLITE_BASE_URL = env.str( diff --git a/api/integrations/github/helpers.py b/api/integrations/github/helpers.py new file mode 100644 index 000000000000..3789f00220db --- /dev/null +++ b/api/integrations/github/helpers.py @@ -0,0 +1,20 @@ +import hashlib +import hmac + + +def github_webhook_payload_is_valid( + payload_body: bytes, secret_token: str, signature_header: str +) -> bool: + """Verify that the payload was sent from GitHub by validating SHA256. + Raise and return 403 if not authorized. + """ + if not signature_header: + return False + hash_object = hmac.new( + secret_token.encode("utf-8"), msg=payload_body, digestmod=hashlib.sha1 + ) + expected_signature = "sha1=" + hash_object.hexdigest() + if not hmac.compare_digest(expected_signature, signature_header): + return False + + return True diff --git a/api/integrations/github/views.py b/api/integrations/github/views.py index ddbea03bae0e..c62f49d60300 100644 --- a/api/integrations/github/views.py +++ b/api/integrations/github/views.py @@ -1,19 +1,20 @@ +import json import re from functools import wraps import requests from django.conf import settings from django.db.utils import IntegrityError -from django.http import JsonResponse from rest_framework import status, viewsets from rest_framework.decorators import api_view, permission_classes from rest_framework.exceptions import ValidationError -from rest_framework.permissions import IsAuthenticated +from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.response import Response from integrations.github.client import generate_token from integrations.github.constants import GITHUB_API_URL, GITHUB_API_VERSION from integrations.github.exceptions import DuplicateGitHubIntegration +from integrations.github.helpers import github_webhook_payload_is_valid from integrations.github.models import GithubConfiguration, GithubRepository from integrations.github.permissions import HasPermissionToGithubConfiguration from integrations.github.serializers import ( @@ -107,7 +108,7 @@ def create(self, request, *args, **kwargs): @api_view(["GET"]) @permission_classes([IsAuthenticated, HasPermissionToGithubConfiguration]) @github_auth_required -def fetch_pull_requests(request, organisation_pk): +def fetch_pull_requests(request, organisation_pk) -> Response: organisation = Organisation.objects.get(id=organisation_pk) github_configuration = GithubConfiguration.objects.get( organisation=organisation, deleted_at__isnull=True @@ -119,7 +120,7 @@ def fetch_pull_requests(request, organisation_pk): query_serializer = RepoQuerySerializer(data=request.query_params) if not query_serializer.is_valid(): - return JsonResponse({"error": query_serializer.errors}, status=400) + return Response({"error": query_serializer.errors}, status=400) repo_owner = query_serializer.validated_data.get("repo_owner") repo_name = query_serializer.validated_data.get("repo_name") @@ -138,13 +139,13 @@ def fetch_pull_requests(request, organisation_pk): data = response.json() return Response(data) except requests.RequestException as e: - return JsonResponse({"error": str(e)}, status=500) + return Response({"error": str(e)}, status=500) @api_view(["GET"]) @permission_classes([IsAuthenticated, HasPermissionToGithubConfiguration]) @github_auth_required -def fetch_issues(request, organisation_pk): +def fetch_issues(request, organisation_pk) -> Response: organisation = Organisation.objects.get(id=organisation_pk) github_configuration = GithubConfiguration.objects.get( organisation=organisation, deleted_at__isnull=True @@ -156,7 +157,7 @@ def fetch_issues(request, organisation_pk): query_serializer = RepoQuerySerializer(data=request.query_params) if not query_serializer.is_valid(): - return JsonResponse({"error": query_serializer.errors}, status=400) + return Response({"error": query_serializer.errors}, status=400) repo_owner = query_serializer.validated_data.get("repo_owner") repo_name = query_serializer.validated_data.get("repo_name") @@ -176,12 +177,12 @@ def fetch_issues(request, organisation_pk): filtered_data = [issue for issue in data if "pull_request" not in issue] return Response(filtered_data) except requests.RequestException as e: - return JsonResponse({"error": str(e)}, status=500) + return Response({"error": str(e)}, status=500) @api_view(["GET"]) @permission_classes([IsAuthenticated, GithubIsAdminOrganisation]) -def fetch_repositories(request, organisation_pk: int): +def fetch_repositories(request, organisation_pk: int) -> Response: installation_id = request.GET.get("installation_id") token = generate_token( @@ -203,4 +204,27 @@ def fetch_repositories(request, organisation_pk: int): data = response.json() return Response(data) except requests.RequestException as e: - return JsonResponse({"error": str(e)}, status=500) + return Response({"error": str(e)}, status=500) + + +@api_view(["POST"]) +@permission_classes([AllowAny]) +def github_webhook(request) -> Response: + secret = settings.GITHUB_WEBHOOK_SECRET + signature = request.headers.get("X-Hub-Signature") + github_event = request.headers.get("x-github-event") + payload = request.body + if github_webhook_payload_is_valid( + payload_body=payload, secret_token=secret, signature_header=signature + ): + data = json.loads(payload.decode("utf-8")) + # handle GitHub Webhook "installation" event with action type "deleted" + if github_event == "installation" and data["action"] == "deleted": + GithubConfiguration.objects.filter( + installation_id=data["installation"]["id"] + ).delete() + return Response({"detail": "Event processed"}, status=200) + else: + return Response({"detail": "Event bypassed"}, status=200) + else: + return Response({"error": "Invalid signature"}, status=400) diff --git a/api/tests/unit/integrations/github/test_unit_github_views.py b/api/tests/unit/integrations/github/test_unit_github_views.py index 17a107bbd30c..c0b158a5abfd 100644 --- a/api/tests/unit/integrations/github/test_unit_github_views.py +++ b/api/tests/unit/integrations/github/test_unit_github_views.py @@ -1,4 +1,7 @@ +import json + import pytest +from django.conf import settings from django.urls import reverse from pytest_lazyfixture import lazy_fixture from pytest_mock import MockerFixture @@ -7,9 +10,14 @@ from features.feature_external_resources.models import FeatureExternalResource from integrations.github.models import GithubConfiguration, GithubRepository +from integrations.github.views import github_webhook_payload_is_valid from organisations.models import Organisation from projects.models import Project +WEBHOOK_PAYLOAD = json.dumps({"installation": {"id": 1234567}, "action": "deleted"}) +WEBHOOK_SIGNATURE = "sha1=57a1426e19cdab55dd6d0c191743e2958e50ccaa" +WEBHOOK_SECRET = "secret-key" + def test_get_github_configuration( admin_client_new: APIClient, @@ -416,3 +424,128 @@ def test_cannot_fetch_issues_or_prs_when_does_not_have_permissions( # Then assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_verify_github_webhook_payload() -> None: + # When + result = github_webhook_payload_is_valid( + payload_body=WEBHOOK_PAYLOAD.encode("utf-8"), + secret_token=WEBHOOK_SECRET, + signature_header=WEBHOOK_SIGNATURE, + ) + + # Then + assert result is True + + +def test_verify_github_webhook_payload_returns_false_on_bad_signature() -> None: + # When + result = github_webhook_payload_is_valid( + payload_body=WEBHOOK_PAYLOAD.encode("utf-8"), + secret_token=WEBHOOK_SECRET, + signature_header="sha1=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e18", + ) + + # Then + assert result is False + + +def test_verify_github_webhook_payload_returns_false_on_no_signature_header() -> None: + # When + result = github_webhook_payload_is_valid( + payload_body=WEBHOOK_PAYLOAD.encode("utf-8"), + secret_token=WEBHOOK_SECRET, + signature_header=None, + ) + + # Then + assert result is False + + +def test_github_webhook_delete_installation( + github_configuration: GithubConfiguration, +) -> None: + # Given + settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET + url = reverse("api-v1:github-webhook") + + # When + client = APIClient() + response = client.post( + path=url, + data=WEBHOOK_PAYLOAD, + content_type="application/json", + HTTP_X_HUB_SIGNATURE=WEBHOOK_SIGNATURE, + HTTP_X_GITHUB_EVENT="installation", + ) + + # Then + assert response.status_code == 200 + assert not GithubConfiguration.objects.filter(installation_id=1234567).exists() + + +def test_github_webhook_fails_on_signature_header_missing( + github_configuration: GithubConfiguration, +) -> None: + # Given + settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET + url = reverse("api-v1:github-webhook") + + # When + client = APIClient() + response = client.post( + path=url, + data=WEBHOOK_PAYLOAD, + content_type="application/json", + HTTP_X_GITHUB_EVENT="installation", + ) + + # Then + assert response.status_code == 400 + assert response.json() == {"error": "Invalid signature"} + assert GithubConfiguration.objects.filter(installation_id=1234567).exists() + + +def test_github_webhook_fails_on_bad_signature_header_missing( + github_configuration: GithubConfiguration, +) -> None: + # Given + settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET + url = reverse("api-v1:github-webhook") + + # When + client = APIClient() + response = client.post( + path=url, + data=WEBHOOK_PAYLOAD, + content_type="application/json", + HTTP_X_HUB_SIGNATURE="sha1=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e18", + HTTP_X_GITHUB_EVENT="installation", + ) + + # Then + assert response.status_code == 400 + assert GithubConfiguration.objects.filter(installation_id=1234567).exists() + assert response.json() == {"error": "Invalid signature"} + + +def test_github_webhook_bypass_event( + github_configuration: GithubConfiguration, +) -> None: + # Given + settings.GITHUB_WEBHOOK_SECRET = WEBHOOK_SECRET + url = reverse("api-v1:github-webhook") + + # When + client = APIClient() + response = client.post( + path=url, + data=WEBHOOK_PAYLOAD, + content_type="application/json", + HTTP_X_HUB_SIGNATURE=WEBHOOK_SIGNATURE, + HTTP_X_GITHUB_EVENT="installation_repositories", + ) + + # Then + assert response.status_code == 200 + assert GithubConfiguration.objects.filter(installation_id=1234567).exists() diff --git a/infrastructure/aws/production/ecs-task-definition-web.json b/infrastructure/aws/production/ecs-task-definition-web.json index fb73502b8fa9..e98f5bce901e 100644 --- a/infrastructure/aws/production/ecs-task-definition-web.json +++ b/infrastructure/aws/production/ecs-task-definition-web.json @@ -245,6 +245,10 @@ "name": "SSE_AUTHENTICATION_TOKEN", "valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:SSE_AUTHENTICATION_TOKEN::" }, + { + "name": "GITHUB_WEBHOOK_SECRET", + "valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:GITHUB_WEBHOOK_SECRET::" + }, { "name": "GITHUB_PEM", "valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:GITHUB_PEM-E1Ot8p" diff --git a/infrastructure/aws/staging/ecs-task-definition-web.json b/infrastructure/aws/staging/ecs-task-definition-web.json index 25fbffd70827..a7edca9cc62c 100644 --- a/infrastructure/aws/staging/ecs-task-definition-web.json +++ b/infrastructure/aws/staging/ecs-task-definition-web.json @@ -241,6 +241,10 @@ "name": "SSE_AUTHENTICATION_TOKEN", "valueFrom": "arn:aws:secretsmanager:eu-west-2:302456015006:secret:ECS-API-heAdoB:SSE_AUTHENTICATION_TOKEN::" }, + { + "name": "GITHUB_WEBHOOK_SECRET", + "valueFrom": "arn:aws:secretsmanager:eu-west-2:302456015006:secret:ECS-API-heAdoB:GITHUB_WEBHOOK_SECRET::" + }, { "name": "GITHUB_PEM", "valueFrom": "arn:aws:secretsmanager:eu-west-2:302456015006:secret:GITHUB_PEM-Bfoaql"