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

Adding multiple principals role using assumeRolePolicy.addStatements override conditions #3227

Closed
1 task done
NetaNir opened this issue Jul 7, 2019 · 0 comments · Fixed by #7703
Closed
1 task done
Assignees
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management bug This issue is a bug. p1

Comments

@NetaNir
Copy link
Contributor

NetaNir commented Jul 7, 2019

  • I'm submitting a ...

    • 🪲 bug report
  • What is the current behavior?

When Adding multiple conditional principals to the same PolicyStatment which has same conditions key will result in only one of the conditions:

if (role.assumeRolePolicy) { 
  role.assumeRolePolicy.addStatements(
    new PolicyStatement({
      principals: [
        new ServicePrincipal('myService.amazon.com', {
          conditions: {
            StringEquals: {
              hairColor: 'blond',
              pet: 'cat'
            }
          }
        }),
        new ServicePrincipal('yourservice.amazon.com', {
          conditions: {
            StringEquals: {
              hairColor: 'black'
            }
          }
        })
      ]
    }));
  } 

will result in this template:

        "AssumeRolePolicyDocument": {
          "Statement": [
            {
              "Action": "sts:AssumeRole",
              "Effect": "Allow",
              "Principal": {
                "Service": "sqs.amazonaws.com"
              }
            },
            {
              "Condition": {
                "StringEquals": {
                  "hairColor": "black"
                }
              },
              "Effect": "Allow",
              "Principal": {
                "Service": [
                  "myService.amazon.com",
                  "yourservice.amazon.com"
                ]
              }
            }
          ],
          "Version": "2012-10-17"
        }

This is because the conditions are added to a map in the statement which causes the first added role conditions to be override:

  public addConditions(conditions: {[key: string]: any}) {
    Object.keys(conditions).map(key => {
      this.addCondition(key, conditions[key]);
    });
  }

Adding each principal in it's own statement results in the correct behavior, If this is the expected behavior maybe it will be better to not allow multiple conditioned principals add to a single statement (like in the composite principals)

  • Please tell us about your environment:

    • CDK CLI Version: 1.0
    • Module Version: 1.0
@NetaNir NetaNir added the needs-triage This issue or PR still needs to be triaged. label Jul 7, 2019
@RomainMuller RomainMuller added bug This issue is a bug. security @aws-cdk/aws-iam Related to AWS Identity and Access Management and removed needs-triage This issue or PR still needs to be triaged. labels Jul 23, 2019
@rix0rrr rix0rrr added p1 and removed SECURITY labels Oct 24, 2019
rix0rrr added a commit that referenced this issue Apr 30, 2020
Principals can come with implicit conditions, which is useful for
representing principals that can't be expressed with a single field
in IAM.

However, you should not be allowed to use principals with mixed
conditions in a single statement, because IAM can't properly
express the implied OR there.

Fixes #3227.
@mergify mergify bot closed this as completed in #7703 May 5, 2020
mergify bot pushed a commit that referenced this issue May 5, 2020
Principals can come with implicit conditions, which is useful for
representing principals that can't be expressed with a single field
in IAM.

However, you should not be allowed to use principals with mixed
conditions in a single statement, because IAM can't properly
express the implied OR there.

Fixes #3227.
karupanerura pushed a commit to karupanerura/aws-cdk that referenced this issue May 7, 2020
Principals can come with implicit conditions, which is useful for
representing principals that can't be expressed with a single field
in IAM.

However, you should not be allowed to use principals with mixed
conditions in a single statement, because IAM can't properly
express the implied OR there.

Fixes aws#3227.
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 bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants