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

fix(lambda): granting multiple conditional principals is not correctly configured #16782

Closed

Conversation

deepak-sreekumar
Copy link

If grantInvoke() is called twice for the same service principal but with different conditions, only one resource policy was getting created. The reason was a conditional skip on policy creation based only on the principal (eg: s3.amazonaws.com ) instead of checking the whole grantable object.

Adding a more robust check based on the object hash to handle the cases similar to the conditional principal.

fixes #15710

@gitpod-io
Copy link

gitpod-io bot commented Oct 4, 2021

@peterwoodworth peterwoodworth changed the title fix(aws-lambda): grant invoke with conditional principals fix(aws-lambda): grant invoke with conditional principals Oct 21, 2021
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Oct 21, 2021
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 1d0c893
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@nija-at nija-at changed the title fix(aws-lambda): grant invoke with conditional principals fix(lambda): granting multiple conditional principals is not correctly configured Nov 5, 2021
@nija-at nija-at assigned rix0rrr and unassigned nija-at Nov 5, 2021
@nija-at
Copy link
Contributor

nija-at commented Nov 5, 2021

Assigning to @rix0rrr since he's more familiar with the intricacies in IAM grants.

@nija-at nija-at removed their assignment Nov 5, 2021
Comment on lines +114 to +116
"bundledDependencies": [
"object-hash"
],
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to do crypto without having to use this dependency.

Nodejs' crypto library should suffice - https://nodejs.org/api/crypto.html#cryptocreatecipherivalgorithm-key-iv-options

@rix0rrr rix0rrr added bug This issue is a bug. p2 and removed @aws-cdk/aws-lambda Related to AWS Lambda labels Mar 4, 2022
@aws-cdk-automation
Copy link
Collaborator

This PR has been in BUILD FAILING for 21 days, and looks abandoned. It will be closed in 10 days if no further commits are pushed to it.

@github-actions
Copy link

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@github-actions github-actions bot added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Apr 20, 2022
@github-actions github-actions bot closed this Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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