Skip to content

Commit

Permalink
fix(iam): policies added to immutably imported role (#6090)
Browse files Browse the repository at this point in the history
In the refactoring done in #5569, we introduced a bug. The
`ImmutableRole` class correctly ignored policies directly added to it,
but did not ignore policies added via `Grant.addToPrincipal()`.

That's because its `IGrantable#grantPrincipal` field was being used
as the principal to grant to, which was pointing to the wrapped
role instead of the `ImmutableRole` itself.

Fix this oversight and add a test to cement it in.

Fixes #5943.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
rix0rrr and mergify[bot] authored Feb 5, 2020
1 parent 46febad commit f1f5319
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { IRole } from '../role';
export class ImmutableRole implements IRole {
public readonly assumeRoleAction = this.role.assumeRoleAction;
public readonly policyFragment = this.role.policyFragment;
public readonly grantPrincipal = this.role.grantPrincipal;
public readonly grantPrincipal = this;
public readonly roleArn = this.role.roleArn;
public readonly roleName = this.role.roleName;
public readonly node = this.role.node;
Expand Down
32 changes: 27 additions & 5 deletions packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,39 @@ describe('ImmutableRole', () => {
});

test('ignores calls to addToPolicy', () => {
mutableRole.addToPolicy(new iam.PolicyStatement({
immutableRole.addToPolicy(new iam.PolicyStatement({
resources: ['*'],
actions: ['s3:*'],
actions: ['iam:*'],
}));

immutableRole.addToPolicy(new iam.PolicyStatement({
mutableRole.addToPolicy(new iam.PolicyStatement({
resources: ['*'],
actions: ['iam:*'],
actions: ['s3:*'],
}));

expect(stack).toHaveResourceLike('AWS::IAM::Policy', {
expect(stack).toHaveResource('AWS::IAM::Policy', {
"PolicyDocument": {
"Version": "2012-10-17",
"Statement": [
{
"Resource": "*",
"Action": "s3:*",
"Effect": "Allow",
},
],
},
});
});

test('ignores grants', () => {

iam.Grant.addToPrincipal({
grantee: immutableRole,
actions: ['s3:*'],
resourceArns: ['*'],
});

expect(stack).not.toHaveResourceLike('AWS::IAM::Policy', {
"PolicyDocument": {
"Statement": [
{
Expand Down

0 comments on commit f1f5319

Please sign in to comment.