From 62733836f12069d23f2c00f3a6926bfdbb59f314 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 4 Feb 2020 11:07:33 +0100 Subject: [PATCH] fix(iam): policies added to immutably imported role 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. --- .../aws-iam/lib/private/immutable-role.ts | 2 +- .../aws-iam/test/immutable-role.test.ts | 32 ++++++++++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts index d894cc8dbc722..fd70bfa3786d5 100644 --- a/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts +++ b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts @@ -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; diff --git a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts index 7a39b761bbb7f..f736b5a3c8680 100644 --- a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts +++ b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts @@ -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": [ {