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(iam): Groups are erroneously accepted as the Principal of a policy (#11479) #12549

Merged

Conversation

swar8080
Copy link
Contributor

closes #11479

Hello, picking this up as my first issue to learn about how CDK works:

AWS mentions that iam groups can't be principals in policies. If a developer doesn't know this, they get an error during deployment. This change will throw an error to help them catch the issue during synthesis time.

Note that this doesn't handle the code path where an arn string is specified as the principal, but the method already documents that a group would be invalid.

Let me know if there's a cleaner way to fix this.

@gitpod-io
Copy link

gitpod-io bot commented Jan 17, 2021

@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Jan 17, 2021
@swar8080 swar8080 changed the title feat(aws-iam): throw an error if a Group is used as the Principal of a policy (#11479) fix(aws-iam): throw an error if a Group is used as the Principal of a policy (#11479) Jan 17, 2021
@rix0rrr rix0rrr changed the title fix(aws-iam): throw an error if a Group is used as the Principal of a policy (#11479) fix(iam): Groups are erroneously accepted as the Principal of a policy (#11479) Jan 18, 2021
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

This will do. Thanks!

@mergify
Copy link
Contributor

mergify bot commented Jan 18, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 1d92bbe
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Jan 18, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit c9b0859 into aws:master Jan 18, 2021
mohanrajendran pushed a commit to mohanrajendran/aws-cdk that referenced this pull request Jan 24, 2021
aws#11479) (aws#12549)

closes aws#11479
----
Hello, picking this up as my first issue to learn about how CDK works:

[AWS mentions](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_principal.html) that iam groups can't be principals in policies. If a developer doesn't know this, they get an error during deployment. This change will throw an error to help them catch the issue during synthesis time.

Note that this doesn't handle the code path where an arn string is specified as the principal, but the method already documents that a group would be invalid.

Let me know if there's a cleaner way to fix this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iam] Throw early error on usage of Group in AssumeRolePolicy
3 participants