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(webhook): Enable authorizer assignment to webhook #4000

Conversation

SSW-SCIENTIFIC
Copy link
Contributor

We want to add a lambda authenticator to webhook API endpoint to restrict requests by source IP address. To achieve this, the following two options are possible:

  1. add attributes to pass authorizer resource information to this module,
  2. or simply ignores changes related to authorizer of aws_apigatewayv2_route resource,

However, 1. requires all informations to create aws_apigatewayv2_authorizer and aws_lambda_permission, this is little bit need consideration. Option 2. is, of cource, needs resouce update by hand, however, the very simple to achieve by ignoring attribute changes.

This PR aims to implement option 2.

We want to add a lambda authenticator to webhook API endpoint to restrict requests by source IP address.
To achieve this, the following two options are possible:
1. add attributes to pass authorizer resource information to this module,
2. or simply ignores changes related to authorizer of `aws_apigatewayv2_route` resource,
However, 1. requires all informations to create `aws_apigatewayv2_authorizer` and `aws_lambda_permission`, this is little bit need consideration.
Option 2. is, of cource, needs resouce update by hand, however, the very simple to achieve by ignoring attribute changes.

This PR aims to implement option 2.
@npalm
Copy link
Collaborator

npalm commented Jul 17, 2024

Maybe great if you even can share an example how to attach the authroizer. I digged in some time ago but at that time it seems not a real option with the version of the gateway we are using. The only option I saw was an authorizer lambda to run the checks.

@npalm npalm self-requested a review July 17, 2024 15:50
@SSW-SCIENTIFIC
Copy link
Contributor Author

SSW-SCIENTIFIC commented Jul 17, 2024

@npalm
Yes, you are right, only we can add lambda authorizer for API Gateway v2.

I show the example below:

  1. create lambda for authorizer as follows:
import * as process from "node:process";

import { APIGatewayRequestSimpleAuthorizerHandlerV2 } from "aws-lambda";
import { SSMClient, GetParameterCommand } from "@aws-sdk/client-ssm";
import { containsCidr } from "cidr-tools";

console.log("Loading function");

export const handler: APIGatewayRequestSimpleAuthorizerHandlerV2 = async (event) => {
  const allowlist = await fetchIPAllowList();

  if (containsCidr(allowlist, event.requestContext.http.sourceIp)) {
    return { isAuthorized: true };
  }

  return { isAuthorized: false };
};


const client = new SSMClient();

const fetchIPAllowList = async () => {
  const result = await client.send(
    new GetParameterCommand({
      Name: process.env.IP_ALLOWLIST_PARAMETER_NAME,
    }),
  );
  return JSON.parse(result.Parameter?.Value || "[]") as unknown as string[];
};

and create lambda resouce:

module "authorizer" {
  source = "terraform-aws-modules/lambda/aws"

  publish = true

  function_name = "source-ip-address-authorizer"
  description   = "Lambda Authorizer to protect API Gateway."
  handler       = "index.handler"
  runtime       = "nodejs20.x"

  environment_variables = {
    "IP_ALLOWLIST_PARAMETER_NAME" = module.ip_allowlist.ssm_parameter_name
  }

  allowed_triggers = {
    api-gateway = {
      service = "apigateway"
    }
  }

  attach_policy_json = true
  policy_json        = data.aws_iam_policy_document.authorizer.json

  source_path = "./authorizer/dist/index.mjs"
}

module "ip_allowlist" {
  source = "terraform-aws-modules/ssm-parameter/aws"

  name   = "/services/public-api/ip-allowlist"
  values = var.ip_allowlist
}

data "aws_iam_policy_document" "authorizer" {
  statement {
    effect    = "Allow"
    actions   = ["ssm:GetParameter"]
    resources = [module.ip_allowlist.ssm_parameter_arn]
  }
}
  1. create lambda authorizer resouce for Terraform AWS GitHub Runner:
resource "aws_apigatewayv2_authorizer" "authorizer" {
  authorizer_type                   = "REQUEST"
  api_id                            = module.github_runners.webhook.gateway.id
  authorizer_uri                    = module.authorizer.lambda_function_invoke_arn
  name                              = "source-ip-address-authorizer"
  authorizer_payload_format_version = "2.0"
  enable_simple_responses           = true
}

Here module.github_runners is (in my case) multi-runner module of terraform-aws-github-runner.

  1. modify aws_apigatewayv2_route.webhook resouce manually, for example, by Web UI to attach lambda authorizer:
 resource "aws_apigatewayv2_route" "webhook" {
   api_id    = aws_apigatewayv2_api.webhook.id
   route_key = "POST /${local.webhook_endpoint}"
   target    = "integrations/${aws_apigatewayv2_integration.webhook.id}"

+  authorizer_type = "CUSTOM"
+  authorizer_id   = aws_apigatewayv2_authorizer.authorizer.id
 }

Thank you.

@npalm
Copy link
Collaborator

npalm commented Jul 30, 2024

I will catch-up with the PR this week was on leave last 1.5 week.

@npalm
Copy link
Collaborator

npalm commented Jul 31, 2024

As long we have no authorizer part of the module this change is fine. I hope to refactor the module soon to split the webhook in two parts. One responsible for accepting the event from GitHub including checks of the signature and possible IP allow lists. Once accpeted put the event on the eventbridge (AWS) and next let the a dispatcher put in on a queue for a scale-up functions.

This change woudl allow anyone to use easier own streams of the job events, and even more events for any other logic. Including metrics.

Copy link
Collaborator

@npalm npalm left a comment

Choose a reason for hiding this comment

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

Tiny suggestion.

modules/webhook/main.tf Outdated Show resolved Hide resolved
@npalm npalm merged commit b78ccde into github-aws-runners:main Aug 1, 2024
41 checks passed
npalm pushed a commit that referenced this pull request Aug 1, 2024
🤖 I have created a release *beep* *boop*
---


##
[5.13.0](philips-labs/terraform-aws-github-runner@v5.12.2...v5.13.0)
(2024-08-01)


### Features

* **webhook:** Enable authorizer assignment to webhook
([#4000](https://github.com/philips-labs/terraform-aws-github-runner/issues/4000))
([b78ccde](philips-labs/terraform-aws-github-runner@b78ccde))
@SSW-SCIENTIFIC


### Bug Fixes

* add warnings to log for GitHub rate limits
([#3988](https://github.com/philips-labs/terraform-aws-github-runner/issues/3988))
([2ed0b29](philips-labs/terraform-aws-github-runner@2ed0b29))
* bump node dependencies and cleanup
([#4020](https://github.com/philips-labs/terraform-aws-github-runner/issues/4020))
([221958b](philips-labs/terraform-aws-github-runner@221958b))
* **lambda:** bump the aws group across 1 directory with 5 updates
([#4005](https://github.com/philips-labs/terraform-aws-github-runner/issues/4005))
([4ca422d](philips-labs/terraform-aws-github-runner@4ca422d))
* **lambda:** bump the aws group across 1 directory with 5 updates
([#4017](https://github.com/philips-labs/terraform-aws-github-runner/issues/4017))
([0cd6a85](philips-labs/terraform-aws-github-runner@0cd6a85))
* mark github_app variable as sensitive
([#4013](https://github.com/philips-labs/terraform-aws-github-runner/issues/4013))
([08be669](philips-labs/terraform-aws-github-runner@08be669))
@jizi

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>
@SSW-SCIENTIFIC SSW-SCIENTIFIC deleted the feature/ignore-changes-related-to-authorizer-for-webhook branch August 1, 2024 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants