Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement GitHub Webhook #3906

Merged
merged 5 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/api/urls/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"),
Expand Down
2 changes: 1 addition & 1 deletion api/app/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions api/integrations/github/helpers.py
Original file line number Diff line number Diff line change
@@ -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
44 changes: 34 additions & 10 deletions api/integrations/github/views.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down Expand Up @@ -107,7 +108,7 @@
@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
Expand All @@ -119,7 +120,7 @@

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)

Check warning on line 123 in api/integrations/github/views.py

View check run for this annotation

Codecov / codecov/patch

api/integrations/github/views.py#L123

Added line #L123 was not covered by tests

repo_owner = query_serializer.validated_data.get("repo_owner")
repo_name = query_serializer.validated_data.get("repo_name")
Expand All @@ -138,13 +139,13 @@
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)

Check warning on line 142 in api/integrations/github/views.py

View check run for this annotation

Codecov / codecov/patch

api/integrations/github/views.py#L142

Added line #L142 was not covered by tests


@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
Expand All @@ -156,7 +157,7 @@

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)

Check warning on line 160 in api/integrations/github/views.py

View check run for this annotation

Codecov / codecov/patch

api/integrations/github/views.py#L160

Added line #L160 was not covered by tests

repo_owner = query_serializer.validated_data.get("repo_owner")
repo_name = query_serializer.validated_data.get("repo_name")
Expand All @@ -176,12 +177,12 @@
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)

Check warning on line 180 in api/integrations/github/views.py

View check run for this annotation

Codecov / codecov/patch

api/integrations/github/views.py#L180

Added line #L180 was not covered by tests


@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(
Expand All @@ -203,4 +204,27 @@
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)

Check warning on line 207 in api/integrations/github/views.py

View check run for this annotation

Codecov / codecov/patch

api/integrations/github/views.py#L207

Added line #L207 was not covered by tests


@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)
133 changes: 133 additions & 0 deletions api/tests/unit/integrations/github/test_unit_github_views.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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()
4 changes: 4 additions & 0 deletions infrastructure/aws/production/ecs-task-definition-web.json
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@
"name": "SSE_AUTHENTICATION_TOKEN",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:SSE_AUTHENTICATION_TOKEN::"
},
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we added these secrete?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet, I don't have the credentials or authorization to do it. Could you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we added it to staging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added instructions in the description on how to update the GitHub App once we have a secret defined and set.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we can't merge it without it, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For another env var added @matthewelwell asked me to do first the PR declaring the env var in the infrastructure folder. And he set the value after that, however, I would say that he has the last word.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense, but we still can't merge it before the secrets are added because the deployment will fail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthewelwell have set the env var secret.

"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"
Expand Down
4 changes: 4 additions & 0 deletions infrastructure/aws/staging/ecs-task-definition-web.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down