From 5f692a0c1ef3f01229ab5d64ee4625398dbc7ab7 Mon Sep 17 00:00:00 2001
From: Otavio Macedo <otaviomacedo@protonmail.com>
Date: Tue, 8 Mar 2022 15:58:28 +0000
Subject: [PATCH] fix(apigatewayv2-integrations): in case of multiple routes,
 only one execute permission is created (#18716)

When multiple routes are defined for a single lambda integration, only one of
the routes gets permission to execute the function. This is because the
permissions are added when the integration is bound to the route, which happens
only once per integration.

Split the `_bindToRoute` workflow into two parts:

1. The actual bind, followed by the creation of an `HttpIntegration`. We keep
doing this only once per integration.
2. A post-bind step, that happens for every route.

In the case of `HttpLambdaIntegration`, adding the permission has been moved to
the post bind step. All other integrations remain the same.

Fixes https://github.com/aws/aws-cdk/issues/18201.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../lib/http/lambda.ts                        |  4 +-
 .../test/http/lambda.test.ts                  | 37 ++++++++++++++++++-
 .../aws-apigatewayv2/lib/http/integration.ts  | 13 +++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts
index 2417fffe1610d..69019e0c2866a 100644
--- a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts
+++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts
@@ -50,7 +50,7 @@ export class HttpLambdaIntegration extends HttpRouteIntegration {
     this._id = id;
   }
 
-  public bind(options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {
+  protected completeBind(options: HttpRouteIntegrationBindOptions) {
     const route = options.route;
     this.handler.addPermission(`${this._id}-Permission`, {
       scope: options.scope,
@@ -61,7 +61,9 @@ export class HttpLambdaIntegration extends HttpRouteIntegration {
         resourceName: `*/*${route.path ?? ''}`, // empty string in the case of the catch-all route $default
       }),
     });
+  }
 
+  public bind(_: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {
     return {
       type: HttpIntegrationType.AWS_PROXY,
       uri: this.handler.functionArn,
diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/lambda.test.ts b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/lambda.test.ts
index 5c318e1629842..f832921ad995b 100644
--- a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/lambda.test.ts
+++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/lambda.test.ts
@@ -1,4 +1,4 @@
-import { Template } from '@aws-cdk/assertions';
+import { Match, Template } from '@aws-cdk/assertions';
 import { HttpApi, HttpRoute, HttpRouteKey, MappingValue, ParameterMapping, PayloadFormatVersion } from '@aws-cdk/aws-apigatewayv2';
 import { Code, Function, Runtime } from '@aws-cdk/aws-lambda';
 import { App, Stack } from '@aws-cdk/core';
@@ -71,6 +71,41 @@ describe('LambdaProxyIntegration', () => {
 
     expect(() => app.synth()).not.toThrow();
   });
+
+  test('multiple routes for the same lambda integration', () => {
+    const app = new App();
+    const lambdaStack = new Stack(app, 'lambdaStack');
+    const fooFn = fooFunction(lambdaStack, 'Fn');
+
+    const stack = new Stack(app, 'apigwStack');
+    const api = new HttpApi(stack, 'httpApi');
+    const integration = new HttpLambdaIntegration('Integration', fooFn);
+
+    api.addRoutes({
+      path: '/foo',
+      integration,
+    });
+
+    api.addRoutes({
+      path: '/bar',
+      integration,
+    });
+
+    // Make sure we have two permissions -- one for each method -- but a single integration
+    Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', {
+      SourceArn: {
+        'Fn::Join': ['', Match.arrayWith([':execute-api:', '/*/*/foo'])],
+      },
+    });
+
+    Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', {
+      SourceArn: {
+        'Fn::Join': ['', Match.arrayWith([':execute-api:', '/*/*/bar'])],
+      },
+    });
+
+    Template.fromStack(stack).resourceCountIs('AWS::ApiGatewayV2::Integration', 1);
+  });
 });
 
 function fooFunction(stack: Stack, id: string) {
diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts
index 58e9c9a60879a..9667f072d9036 100644
--- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts
+++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts
@@ -332,9 +332,22 @@ export abstract class HttpRouteIntegration {
         credentials: config.credentials,
       });
     }
+    this.completeBind(options);
     return { integrationId: this.integration.integrationId };
   }
 
+  /**
+   * Complete the binding of the integration to the route. In some cases, there is
+   * some additional work to do, such as adding permissions for the API to access
+   * the target. This work is necessary whether the integration has just been
+   * created for this route or it is an existing one, previously created for other
+   * routes. In most cases, however, concrete implementations do not need to
+   * override this method.
+   */
+  protected completeBind(_options: HttpRouteIntegrationBindOptions): void {
+    // no-op by default
+  }
+
   /**
    * Bind this integration to the route.
    */