Skip to content

Commit aec09d0

Browse files
committed
fix(cloudformation-diff): show PermissionSet as principal in IAM statement changes
1 parent 1ec3310 commit aec09d0

File tree

2 files changed

+32
-4
lines changed

2 files changed

+32
-4
lines changed

packages/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,15 @@ export class IamChanges {
287287
break;
288288
case PropertyScrutinyType.InlineResourcePolicy:
289289
// Any PolicyDocument on a resource (including AssumeRolePolicyDocument)
290-
this.statements.addOld(...this.readResourceStatements(propertyChange.oldValue, propertyChange.resourceLogicalId));
291-
this.statements.addNew(...this.readResourceStatements(propertyChange.newValue, propertyChange.resourceLogicalId));
290+
// Special-case AWS::SSO::PermissionSet as a pseudo-principal in the IAM statement changes output.
291+
if (propertyChange.resourceType === 'AWS::SSO::PermissionSet') {
292+
this.statements.addOld(...this.readPermissionSetInlinePolicy(propertyChange.oldValue, propertyChange.resourceLogicalId));
293+
this.statements.addNew(...this.readPermissionSetInlinePolicy(propertyChange.newValue, propertyChange.resourceLogicalId));
294+
} else {
295+
// Existing behaviour for all other resources
296+
this.statements.addOld(...this.readResourceStatements(propertyChange.oldValue, propertyChange.resourceLogicalId));
297+
this.statements.addNew(...this.readResourceStatements(propertyChange.newValue, propertyChange.resourceLogicalId));
298+
}
292299
break;
293300
case PropertyScrutinyType.ManagedPolicies:
294301
// Just a list of managed policies
@@ -413,6 +420,27 @@ export class IamChanges {
413420
})];
414421
}
415422

423+
private readPermissionSetInlinePolicy(policy: any, logicalId: string): Statement[] {
424+
if (policy === undefined) {
425+
return [];
426+
}
427+
428+
// For PermissionSet inline policies:
429+
// - Resource: still defaulted to the PermissionSet ARN when wildcarded
430+
// - Principal: a pseudo-principal that identifies the PermissionSet
431+
const appliesToResource = '${' + logicalId + '.Arn}';
432+
const appliesToPrincipal = 'AWS:${' + logicalId + '}';
433+
434+
const statements = parseStatements(renderIntrinsics(policy.Statement));
435+
436+
// Keeping the existing behaviour for Resource…
437+
defaultResource(appliesToResource, statements);
438+
// …and additionally injecting a pseudo-principal for readability
439+
defaultPrincipal(appliesToPrincipal, statements);
440+
441+
return statements;
442+
}
443+
416444
private readResourceStatements(policy: any, logicalId: string): Statement[] {
417445
if (policy === undefined) {
418446
return [];

packages/@aws-cdk/cloudformation-diff/test/iam/detect-changes.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ test('can summarize negative ssoPermissionSet changes with PermissionsBoundary.C
564564
'${MySsoPermissionSet.Arn}',
565565
'Allow',
566566
'iam:CreateServiceLinkedRole',
567-
'',
567+
'AWS:${MySsoPermissionSet}',
568568
'',
569569
].map(s => chalk.red(s)),
570570
],
@@ -609,7 +609,7 @@ test('can summarize ssoPermissionSet changes with PermissionsBoundary.CustomerMa
609609
'${MySsoPermissionSet.Arn}',
610610
'Allow',
611611
'iam:CreateServiceLinkedRole',
612-
'',
612+
'AWS:${MySsoPermissionSet}',
613613
'',
614614
].map(s => chalk.green(s)),
615615
],

0 commit comments

Comments
 (0)