(aws-lambda): grant invoke twice with different principals

[TRANTANKHOA]

I grant invoke for my lambda to 2 different principles and only one resource policy created.

Reproduction Steps

lambda.grantInvoke(new ServicePrincipal('s3.amazonaws.com').withConditions({
    ArnLike: {
        'aws:SourceArn': aBucket.bucketArn,
    },
    StringEquals: {
        'aws:SourceAccount': this.node.tryGetContext('account'),
    },
}))
lambda.grantInvoke(new ServicePrincipal('s3.amazonaws.com').withConditions({
    ArnLike: {
        'aws:SourceArn': bBucket.bucketArn,
    },
    StringEquals: {
        'aws:SourceAccount': this.node.tryGetContext('account'),
    },
}))

Expect 2 different resource policies created

What actually happened?

Deployment went ok, only first policy created

Statement ID: LambdaInvokeServicePrincipals3amazonawscomF32-78TTVCKWLPHK
Principal: s3.amazonaws.com
Effect: Allow
Action: lambda:InvokeFunction
Conditions: {
 "StringEquals": {
  "AWS:SourceAccount": "111122223333"
 },
 "ArnLike": {
  "AWS:SourceArn": "arn:aws:s3:::a-bucket"
 }
}

Environment

• **CDK CLI Version : 1.115.0
• **Node.js Version: v14.17.3
• **OS: Windows
• **Language (Version): TypeScript

---

This is 🐛 Bug Report

[nija-at]

Acknowledged that this is a bug.

The bug is coming from

aws-cdk/packages/@aws-cdk/aws-lambda/lib/function-base.ts

Lines 309 to 310 in dbe4641

	// Memoize the result so subsequent grantInvoke() calls are idempotent
	let grant = this._invocationGrants[identifier];

The key to the memoization should not just be the identifier, which in this case is s3.amazonaws.com. The fix is to update the identifier to be the "hash" of the Grantable object passed.

The workaround for this is to use lambda.addPermission() instead.

[nija-at]

Since there is a workaround and we have only received one report on this so far, I am unassigning and marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback.

[deepak-sreekumar]

@nija-at Can I take a stab at this?
Proposed solution:
As you suggested, using object-hash and changing the identifier to something like
const identifier = `Invoke${MD5(grantee.grantPrincipal)}`; .

[deepak-sreekumar]

Raised a PR. Please take a look @nija-at @njlynch . Thanks.

[Dzhuneyt]

The PR mentioned above was a bit outdated. I've created a new one using the newer Template.fromStack(stack) syntax for tests as well as using the NodeJS built-in Crypto package. @nija-at @njlynch

[Dzhuneyt]

The PR now integrated green @nija-at @njlynch. In case you wanted to take a look.

[Dzhuneyt]

@nija-at @njlynch - open to any feedback here. I've created a new "clean" PR.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.