fix(lambda): grant invoke twice with different principals (aws#20174)

Supercedes: aws#18748 (which got too messy due to conflicts) Closes aws#15710 ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [X] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
wphilipw · May 23, 2022 · 8785555 · 8785555
1 parent 52e64ac
commit 8785555
Show file tree

Hide file tree

Showing 14 changed files with 116 additions and 22 deletions.
diff --git a/...-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/TestStack.template.json b/...-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/TestStack.template.json
@@ -445,7 +445,7 @@
     "TargetType": "lambda"
    },
    "DependsOn": [
-    "FunInvokeServicePrincipalelasticloadbalancingamazonawscomD2CAC0C4"
+    "FunInvoke2UTWxhlfyqbT5FTn5jvgbLgjFfJwzswGk55DU1HY1CA1AAFB"
    ]
   },
   "FunServiceRole3CC876D7": {
@@ -498,7 +498,7 @@
     "FunServiceRole3CC876D7"
    ]
   },
-  "FunInvokeServicePrincipalelasticloadbalancingamazonawscomD2CAC0C4": {
+  "FunInvoke2UTWxhlfyqbT5FTn5jvgbLgjFfJwzswGk55DU1HY1CA1AAFB": {
    "Type": "AWS::Lambda::Permission",
    "Properties": {
     "Action": "lambda:InvokeFunction",

diff --git a/...ges/@aws-cdk/aws-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/cdk.out b/...ges/@aws-cdk/aws-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/cdk.out
@@ -1 +1 @@
-{"version":"17.0.0"}
+{"version":"18.0.0"}
diff --git a/.../@aws-cdk/aws-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/integ.json b/.../@aws-cdk/aws-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/integ.json
@@ -1,7 +1,7 @@
 {
   "version": "18.0.0",
   "testCases": {
-    "aws-elasticloadbalancingv2-targets/test/integ.lambda-target": {
+    "integ.lambda-target": {
       "stacks": [
         "TestStack"
       ],

diff --git a/...ws-cdk/aws-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/manifest.json b/...ws-cdk/aws-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/manifest.json
@@ -1,5 +1,5 @@
 {
-  "version": "17.0.0",
+  "version": "18.0.0",
   "artifacts": {
     "Tree": {
       "type": "cdk:tree",
@@ -177,10 +177,19 @@
             "data": "FunA2CCED21"
           }
         ],
-        "/TestStack/Fun/InvokeServicePrincipal(elasticloadbalancing.amazonaws.com)": [
+        "/TestStack/Fun/Invoke2UTWxhlfyqbT5FTn--5jvgbLgj+FfJwzswGk55DU1H--Y=": [
           {
             "type": "aws:cdk:logicalId",
-            "data": "FunInvokeServicePrincipalelasticloadbalancingamazonawscomD2CAC0C4"
+            "data": "FunInvoke2UTWxhlfyqbT5FTn5jvgbLgjFfJwzswGk55DU1HY1CA1AAFB"
+          }
+        ],
+        "FunInvokeServicePrincipalelasticloadbalancingamazonawscomD2CAC0C4": [
+          {
+            "type": "aws:cdk:logicalId",
+            "data": "FunInvokeServicePrincipalelasticloadbalancingamazonawscomD2CAC0C4",
+            "trace": [
+              "!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
+            ]
           }
         ]
       },

diff --git a/...s/@aws-cdk/aws-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/tree.json b/...s/@aws-cdk/aws-elasticloadbalancingv2-targets/test/lambda-target.integ.snapshot/tree.json
@@ -850,9 +850,9 @@
                   "version": "0.0.0"
                 }
               },
-              "InvokeServicePrincipal(elasticloadbalancing.amazonaws.com)": {
-                "id": "InvokeServicePrincipal(elasticloadbalancing.amazonaws.com)",
-                "path": "TestStack/Fun/InvokeServicePrincipal(elasticloadbalancing.amazonaws.com)",
+              "Invoke2UTWxhlfyqbT5FTn--5jvgbLgj+FfJwzswGk55DU1H--Y=": {
+                "id": "Invoke2UTWxhlfyqbT5FTn--5jvgbLgj+FfJwzswGk55DU1H--Y=",
+                "path": "TestStack/Fun/Invoke2UTWxhlfyqbT5FTn--5jvgbLgj+FfJwzswGk55DU1H--Y=",
                 "attributes": {
                   "aws:cdk:cloudformation:type": "AWS::Lambda::Permission",
                   "aws:cdk:cloudformation:props": {

diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts
@@ -1,3 +1,4 @@
+import { createHash } from 'crypto';
 import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
 import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
@@ -414,7 +415,13 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC
    * Grant the given identity permissions to invoke this Lambda
    */
   public grantInvoke(grantee: iam.IGrantable): iam.Grant {
-    const identifier = `Invoke${grantee.grantPrincipal}`; // calls the .toString() of the principal
+    const hash = createHash('sha256')
+      .update(JSON.stringify({
+        principal: grantee.grantPrincipal.toString(),
+        conditions: grantee.grantPrincipal.policyFragment.conditions,
+      }), 'utf8')
+      .digest('base64');
+    const identifier = `Invoke${hash}`;
 
     // Memoize the result so subsequent grantInvoke() calls are idempotent
     let grant = this._invocationGrants[identifier];

diff --git a/packages/@aws-cdk/aws-lambda/test/function.test.ts b/packages/@aws-cdk/aws-lambda/test/function.test.ts
@@ -2755,6 +2755,75 @@ describe('function', () => {
       });
     });
   });
+
+  test('called twice for the same service principal but with different conditions', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const fn = new lambda.Function(stack, 'Function', {
+      code: lambda.Code.fromInline('xxx'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.NODEJS_14_X,
+    });
+    const sourceArnA = 'some-arn-a';
+    const sourceArnB = 'some-arn-b';
+    const service = 's3.amazonaws.com';
+    const conditionalPrincipalA = new iam.PrincipalWithConditions(new iam.ServicePrincipal(service), {
+      ArnLike: {
+        'aws:SourceArn': sourceArnA,
+      },
+      StringEquals: {
+        'aws:SourceAccount': stack.account,
+      },
+    });
+    const conditionalPrincipalB = new iam.PrincipalWithConditions(new iam.ServicePrincipal(service), {
+      ArnLike: {
+        'aws:SourceArn': sourceArnB,
+      },
+      StringEquals: {
+        'aws:SourceAccount': stack.account,
+      },
+    });
+
+    // WHEN
+    fn.grantInvoke(conditionalPrincipalA);
+    fn.grantInvoke(conditionalPrincipalB);
+
+    // THEN
+    Template.fromStack(stack).resourceCountIs('AWS::Lambda::Permission', 2);
+    Template.fromStack(stack).hasResource('AWS::Lambda::Permission', {
+      Properties: {
+        Action: 'lambda:InvokeFunction',
+        FunctionName: {
+          'Fn::GetAtt': [
+            'Function76856677',
+            'Arn',
+          ],
+        },
+        Principal: service,
+        SourceAccount: {
+          Ref: 'AWS::AccountId',
+        },
+        SourceArn: sourceArnA,
+      },
+    });
+
+    Template.fromStack(stack).hasResource('AWS::Lambda::Permission', {
+      Properties: {
+        Action: 'lambda:InvokeFunction',
+        FunctionName: {
+          'Fn::GetAtt': [
+            'Function76856677',
+            'Arn',
+          ],
+        },
+        Principal: service,
+        SourceAccount: {
+          Ref: 'AWS::AccountId',
+        },
+        SourceArn: sourceArnB,
+      },
+    });
+  });
 });
 
 test('throws if ephemeral storage size is out of bound', () => {

diff --git a/packages/@aws-cdk/aws-lambda/test/lambda-version.test.ts b/packages/@aws-cdk/aws-lambda/test/lambda-version.test.ts
@@ -1,7 +1,7 @@
 import { Template } from '@aws-cdk/assertions';
+import { testDeprecated } from '@aws-cdk/cdk-build-tools';
 import * as cdk from '@aws-cdk/core';
 import * as lambda from '../lib';
-import { testDeprecated } from '@aws-cdk/cdk-build-tools';
 
 describe('lambda version', () => {
   test('can import a Lambda version by ARN', () => {

diff --git a/packages/@aws-cdk/aws-lambda/test/singleton-lambda.test.ts b/packages/@aws-cdk/aws-lambda/test/singleton-lambda.test.ts
@@ -2,9 +2,9 @@ import { Template } from '@aws-cdk/assertions';
 import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
 import * as s3 from '@aws-cdk/aws-s3';
+import { testDeprecated } from '@aws-cdk/cdk-build-tools';
 import * as cdk from '@aws-cdk/core';
 import * as lambda from '../lib';
-import { testDeprecated } from '@aws-cdk/cdk-build-tools';
 
 describe('singleton lambda', () => {
   test('can add same singleton Lambda multiple times, only instantiated once in template', () => {

diff --git a/...anager/test/lambda-rotation.integ.snapshot/cdk-integ-secret-lambda-rotation.template.json b/...anager/test/lambda-rotation.integ.snapshot/cdk-integ-secret-lambda-rotation.template.json
@@ -266,7 +266,7 @@
     "LambdaServiceRoleA8ED4D3B"
    ]
   },
-  "LambdaInvokeServicePrincipalsecretsmanageramazonawscomB927629E": {
+  "LambdaInvokeN0a2GKfZP0JmDqDEVhhu6A0TUv3NyNbk4YMFKNc69846677": {
    "Type": "AWS::Lambda::Permission",
    "Properties": {
     "Action": "lambda:InvokeFunction",

diff --git a/packages/@aws-cdk/aws-secretsmanager/test/lambda-rotation.integ.snapshot/cdk.out b/packages/@aws-cdk/aws-secretsmanager/test/lambda-rotation.integ.snapshot/cdk.out
@@ -1 +1 @@
-{"version":"17.0.0"}
+{"version":"18.0.0"}
diff --git a/packages/@aws-cdk/aws-secretsmanager/test/lambda-rotation.integ.snapshot/integ.json b/packages/@aws-cdk/aws-secretsmanager/test/lambda-rotation.integ.snapshot/integ.json
@@ -1,7 +1,7 @@
 {
   "version": "18.0.0",
   "testCases": {
-    "aws-secretsmanager/test/integ.lambda-rotation": {
+    "integ.lambda-rotation": {
       "stacks": [
         "cdk-integ-secret-lambda-rotation"
       ],

diff --git a/packages/@aws-cdk/aws-secretsmanager/test/lambda-rotation.integ.snapshot/manifest.json b/packages/@aws-cdk/aws-secretsmanager/test/lambda-rotation.integ.snapshot/manifest.json
@@ -1,5 +1,5 @@
 {
-  "version": "17.0.0",
+  "version": "18.0.0",
   "artifacts": {
     "Tree": {
       "type": "cdk:tree",
@@ -57,10 +57,19 @@
             "data": "LambdaD247545B"
           }
         ],
-        "/cdk-integ-secret-lambda-rotation/Lambda/InvokeServicePrincipal(secretsmanager.amazonaws.com)": [
+        "/cdk-integ-secret-lambda-rotation/Lambda/InvokeN0--a2GKfZP0JmDqDE--Vhhu6+A0TUv3NyNbk4YM+FKNc=": [
           {
             "type": "aws:cdk:logicalId",
-            "data": "LambdaInvokeServicePrincipalsecretsmanageramazonawscomB927629E"
+            "data": "LambdaInvokeN0a2GKfZP0JmDqDEVhhu6A0TUv3NyNbk4YMFKNc69846677"
+          }
+        ],
+        "LambdaInvokeServicePrincipalsecretsmanageramazonawscomB927629E": [
+          {
+            "type": "aws:cdk:logicalId",
+            "data": "LambdaInvokeServicePrincipalsecretsmanageramazonawscomB927629E",
+            "trace": [
+              "!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
+            ]
           }
         ]
       },

diff --git a/packages/@aws-cdk/aws-secretsmanager/test/lambda-rotation.integ.snapshot/tree.json b/packages/@aws-cdk/aws-secretsmanager/test/lambda-rotation.integ.snapshot/tree.json
@@ -394,9 +394,9 @@
                   "version": "0.0.0"
                 }
               },
-              "InvokeServicePrincipal(secretsmanager.amazonaws.com)": {
-                "id": "InvokeServicePrincipal(secretsmanager.amazonaws.com)",
-                "path": "cdk-integ-secret-lambda-rotation/Lambda/InvokeServicePrincipal(secretsmanager.amazonaws.com)",
+              "InvokeN0--a2GKfZP0JmDqDE--Vhhu6+A0TUv3NyNbk4YM+FKNc=": {
+                "id": "InvokeN0--a2GKfZP0JmDqDE--Vhhu6+A0TUv3NyNbk4YM+FKNc=",
+                "path": "cdk-integ-secret-lambda-rotation/Lambda/InvokeN0--a2GKfZP0JmDqDE--Vhhu6+A0TUv3NyNbk4YM+FKNc=",
                 "attributes": {
                   "aws:cdk:cloudformation:type": "AWS::Lambda::Permission",
                   "aws:cdk:cloudformation:props": {