Skip to content

Commit

Permalink
feat: support for IAM Identity Center in security diff (#30009)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)

Closes #29835 

### Reason for this change

IAM Identity Center resources were ignored in the security diff

### Description of changes

* Adds the  IAM Identity Center resources to CDK diff
* fixes not presenting property changes when a resource is removed from the template

### Description of how you validated changes

* Added unit tests and integration tests.
* Ran the integration tests that mention cdk diff (`bin/run-suite -a cli-integ-tests -t 'cdk diff'`):
```
Test Suites: 2 skipped, 1 passed, 1 of 3 total
Tests:       90 skipped, 13 passed, 103 total
Snapshots:   0 total
Time:        312.397 s
Ran all test suites with tests matching "cdk diff": 
```

### Dependent PRs

* Before this change can be merged, this change cdklabs/awscdk-service-spec#1052 must be merged.

### Checklist
- [Y] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
bergjaak authored May 2, 2024
1 parent bb7e4d8 commit 0a3cb94
Show file tree
Hide file tree
Showing 9 changed files with 940 additions and 7 deletions.
2 changes: 2 additions & 0 deletions packages/@aws-cdk-testing/cli-integ/lib/aws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export class AwsClients {
public readonly s3: AwsCaller<AWS.S3>;
public readonly ecr: AwsCaller<AWS.ECR>;
public readonly ecs: AwsCaller<AWS.ECS>;
public readonly sso: AwsCaller<AWS.SSO>;
public readonly sns: AwsCaller<AWS.SNS>;
public readonly iam: AwsCaller<AWS.IAM>;
public readonly lambda: AwsCaller<AWS.Lambda>;
Expand All @@ -36,6 +37,7 @@ export class AwsClients {
this.s3 = makeAwsCaller(AWS.S3, this.config);
this.ecr = makeAwsCaller(AWS.ECR, this.config);
this.ecs = makeAwsCaller(AWS.ECS, this.config);
this.sso = makeAwsCaller(AWS.SSO, this.config);
this.sns = makeAwsCaller(AWS.SNS, this.config);
this.iam = makeAwsCaller(AWS.IAM, this.config);
this.lambda = makeAwsCaller(AWS.Lambda, this.config);
Expand Down
65 changes: 65 additions & 0 deletions packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') {
var sns = require('@aws-cdk/aws-sns');
var sqs = require('@aws-cdk/aws-sqs');
var lambda = require('@aws-cdk/aws-lambda');
var sso = require('@aws-cdk/aws-sso');
var docker = require('@aws-cdk/aws-ecr-assets');
} else {
var cdk = require('aws-cdk-lib');
Expand All @@ -19,6 +20,7 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') {
LegacyStackSynthesizer,
aws_ec2: ec2,
aws_ecs: ecs,
aws_sso: sso,
aws_s3: s3,
aws_ssm: ssm,
aws_iam: iam,
Expand Down Expand Up @@ -68,6 +70,62 @@ class YourStack extends cdk.Stack {
}
}

class SsoPermissionSetNoPolicy extends Stack {
constructor(scope, id) {
super(scope, id);

new sso.CfnPermissionSet(this, "permission-set-without-managed-policy", {
instanceArn: 'arn:aws:sso:::instance/testvalue',
name: 'testName',
permissionsBoundary: { customerManagedPolicyReference: { name: 'why', path: '/how/' }},
})
}
}

class SsoPermissionSetManagedPolicy extends Stack {
constructor(scope, id) {
super(scope, id);
new sso.CfnPermissionSet(this, "permission-set-with-managed-policy", {
managedPolicies: ['arn:aws:iam::aws:policy/administratoraccess'],
customerManagedPolicyReferences: [{ name: 'forSSO' }],
permissionsBoundary: { managedPolicyArn: 'arn:aws:iam::aws:policy/AdministratorAccess' },
instanceArn: 'arn:aws:sso:::instance/testvalue',
name: 'niceWork',
})
}
}

class SsoAssignment extends Stack {
constructor(scope, id) {
super(scope, id);
new sso.CfnAssignment(this, "assignment", {
instanceArn: 'arn:aws:sso:::instance/testvalue',
permissionSetArn: 'arn:aws:sso:::testvalue',
principalId: '11111111-2222-3333-4444-test',
principalType: 'USER',
targetId: '111111111111',
targetType: 'AWS_ACCOUNT'
});
}
}

class SsoInstanceAccessControlConfig extends Stack {
constructor(scope, id) {
super(scope, id);
new sso.CfnInstanceAccessControlAttributeConfiguration(this, 'instanceAccessControlConfig', {
instanceArn: 'arn:aws:sso:::instance/testvalue',
accessControlAttributes: [
{ key: 'first', value: { source: ['a'] } },
{ key: 'second', value: { source: ['b'] } },
{ key: 'third', value: { source: ['c'] } },
{ key: 'fourth', value: { source: ['d'] } },
{ key: 'fifth', value: { source: ['e'] } },
{ key: 'sixth', value: { source: ['f'] } },
]
})
}
}

class ListMultipleDependentStack extends Stack {
constructor(scope, id) {
super(scope, id);
Expand Down Expand Up @@ -591,6 +649,13 @@ switch (stackSet) {
new EcsHotswapStack(app, `${stackPrefix}-ecs-hotswap`);
new DockerStack(app, `${stackPrefix}-docker`);
new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`);

// SSO stacks
new SsoInstanceAccessControlConfig(app, `${stackPrefix}-sso-access-control`);
new SsoAssignment(app, `${stackPrefix}-sso-assignment`);
new SsoPermissionSetManagedPolicy(app, `${stackPrefix}-sso-perm-set-with-managed-policy`);
new SsoPermissionSetNoPolicy(app, `${stackPrefix}-sso-perm-set-without-managed-policy`);

const failed = new FailedStack(app, `${stackPrefix}-failed`)

// A stack that depends on the failed stack -- used to test that '-e' does not deploy the failing stack
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,158 @@ integTest('cdk diff --fail with multiple stack exits with if any of the stacks c
await expect(fixture.cdk(['diff', '--fail', fixture.fullStackName('test-1'), fixture.fullStackName('test-2')])).rejects.toThrow('exited with error');
}));

integTest('cdk diff --security-only successfully outputs sso-permission-set-without-managed-policy information', withDefaultFixture(async (fixture) => {
const diff = await fixture.cdk(
['diff', '--security-only', fixture.fullStackName('sso-perm-set-without-managed-policy')],
);
`┌───┬──────────────────────────────────────────┬──────────────────────────────────┬────────────────────┬───────────────────────────────────┬─────────────────────────────────┐
│ │ Resource │ InstanceArn │ PermissionSet name │ PermissionsBoundary │ CustomerManagedPolicyReferences │
├───┼──────────────────────────────────────────┼──────────────────────────────────┼────────────────────┼───────────────────────────────────┼─────────────────────────────────┤
│ + │\${permission-set-without-managed-policy} │ arn:aws:sso:::instance/testvalue │ testName │ CustomerManagedPolicyReference: { │ │
│ │ │ │ │ Name: why, Path: /how/ │ │
│ │ │ │ │ } │ │
`;
expect(diff).toContain('Resource');
expect(diff).toContain('permission-set-without-managed-policy');

expect(diff).toContain('InstanceArn');
expect(diff).toContain('arn:aws:sso:::instance/testvalue');

expect(diff).toContain('PermissionSet name');
expect(diff).toContain('testName');

expect(diff).toContain('PermissionsBoundary');
expect(diff).toContain('CustomerManagedPolicyReference: {');
expect(diff).toContain('Name: why, Path: /how/');
expect(diff).toContain('}');

expect(diff).toContain('CustomerManagedPolicyReferences');
}));

integTest('cdk diff --security-only successfully outputs sso-permission-set-with-managed-policy information', withDefaultFixture(async (fixture) => {
const diff = await fixture.cdk(
['diff', '--security-only', fixture.fullStackName('sso-perm-set-with-managed-policy')],
);
`┌───┬──────────────────────────────────────────┬──────────────────────────────────┬────────────────────┬───────────────────────────────────────────────────────────────┬─────────────────────────────────┐
│ │ Resource │ InstanceArn │ PermissionSet name │ PermissionsBoundary │ CustomerManagedPolicyReferences │
├───┼──────────────────────────────────────────┼──────────────────────────────────┼────────────────────┼───────────────────────────────────────────────────────────────┼─────────────────────────────────┤
│ + │\${permission-set-with-managed-policy} │ arn:aws:sso:::instance/testvalue │ niceWork │ ManagedPolicyArn: arn:aws:iam::aws:policy/AdministratorAccess │ Name: forSSO, Path: │
`;

expect(diff).toContain('Resource');
expect(diff).toContain('permission-set-with-managed-policy');

expect(diff).toContain('InstanceArn');
expect(diff).toContain('arn:aws:sso:::instance/testvalue');

expect(diff).toContain('PermissionSet name');
expect(diff).toContain('niceWork');

expect(diff).toContain('PermissionsBoundary');
expect(diff).toContain('ManagedPolicyArn: arn:aws:iam::aws:policy/AdministratorAccess');

expect(diff).toContain('CustomerManagedPolicyReferences');
expect(diff).toContain('Name: forSSO, Path:');
}));

integTest('cdk diff --security-only successfully outputs sso-assignment information', withDefaultFixture(async (fixture) => {
const diff = await fixture.cdk(
['diff', '--security-only', fixture.fullStackName('sso-assignment')],
);
`┌───┬───────────────┬──────────────────────────────────┬─────────────────────────┬──────────────────────────────┬───────────────┬──────────────┬─────────────┐
│ │ Resource │ InstanceArn │ PermissionSetArn │ PrincipalId │ PrincipalType │ TargetId │ TargetType │
├───┼───────────────┼──────────────────────────────────┼─────────────────────────┼──────────────────────────────┼───────────────┼──────────────┼─────────────┤
│ + │\${assignment} │ arn:aws:sso:::instance/testvalue │ arn:aws:sso:::testvalue │ 11111111-2222-3333-4444-test │ USER │ 111111111111 │ AWS_ACCOUNT │
└───┴───────────────┴──────────────────────────────────┴─────────────────────────┴──────────────────────────────┴───────────────┴──────────────┴─────────────┘
`;
expect(diff).toContain('Resource');
expect(diff).toContain('assignment');

expect(diff).toContain('InstanceArn');
expect(diff).toContain('arn:aws:sso:::instance/testvalue');

expect(diff).toContain('PermissionSetArn');
expect(diff).toContain('arn:aws:sso:::testvalue');

expect(diff).toContain('PrincipalId');
expect(diff).toContain('11111111-2222-3333-4444-test');

expect(diff).toContain('PrincipalType');
expect(diff).toContain('USER');

expect(diff).toContain('TargetId');
expect(diff).toContain('111111111111');

expect(diff).toContain('TargetType');
expect(diff).toContain('AWS_ACCOUNT');
}));

integTest('cdk diff --security-only successfully outputs sso-access-control information', withDefaultFixture(async (fixture) => {
const diff = await fixture.cdk(
['diff', '--security-only', fixture.fullStackName('sso-access-control')],
);
`┌───┬────────────────────────────────┬────────────────────────┬─────────────────────────────────┐
│ │ Resource │ InstanceArn │ AccessControlAttributes │
├───┼────────────────────────────────┼────────────────────────┼─────────────────────────────────┤
│ + │\${instanceAccessControlConfig} │ arn:aws:test:testvalue │ Key: first, Values: [a] │
│ │ │ │ Key: second, Values: [b] │
│ │ │ │ Key: third, Values: [c] │
│ │ │ │ Key: fourth, Values: [d] │
│ │ │ │ Key: fifth, Values: [e] │
│ │ │ │ Key: sixth, Values: [f] │
└───┴────────────────────────────────┴────────────────────────┴─────────────────────────────────┘
`;
expect(diff).toContain('Resource');
expect(diff).toContain('instanceAccessControlConfig');

expect(diff).toContain('InstanceArn');
expect(diff).toContain('arn:aws:sso:::instance/testvalue');

expect(diff).toContain('AccessControlAttributes');
expect(diff).toContain('Key: first, Values: [a]');
expect(diff).toContain('Key: second, Values: [b]');
expect(diff).toContain('Key: third, Values: [c]');
expect(diff).toContain('Key: fourth, Values: [d]');
expect(diff).toContain('Key: fifth, Values: [e]');
expect(diff).toContain('Key: sixth, Values: [f]');
}));

integTest('cdk diff --security-only --fail exits when security diff for sso access control config', withDefaultFixture(async (fixture) => {
await expect(
fixture.cdk(
['diff', '--security-only', '--fail', fixture.fullStackName('sso-access-control')],
),
).rejects
.toThrow('exited with error');
}));

integTest('cdk diff --security-only --fail exits when security diff for sso-perm-set-without-managed-policy', withDefaultFixture(async (fixture) => {
await expect(
fixture.cdk(
['diff', '--security-only', '--fail', fixture.fullStackName('sso-perm-set-without-managed-policy')],
),
).rejects
.toThrow('exited with error');
}));

integTest('cdk diff --security-only --fail exits when security diff for sso-perm-set-with-managed-policy', withDefaultFixture(async (fixture) => {
await expect(
fixture.cdk(
['diff', '--security-only', '--fail', fixture.fullStackName('sso-perm-set-with-managed-policy')],
),
).rejects
.toThrow('exited with error');
}));

integTest('cdk diff --security-only --fail exits when security diff for sso-assignment', withDefaultFixture(async (fixture) => {
await expect(
fixture.cdk(
['diff', '--security-only', '--fail', fixture.fullStackName('sso-assignment')],
),
).rejects
.toThrow('exited with error');
}));

integTest('cdk diff --security-only --fail exits when security changes are present', withDefaultFixture(async (fixture) => {
const stackName = 'iam-test';
await expect(fixture.cdk(['diff', '--security-only', '--fail', fixture.fullStackName(stackName)])).rejects.toThrow('exited with error');
Expand Down
6 changes: 4 additions & 2 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,13 @@ export class TemplateDiff implements ITemplateDiff {
continue;
}

if (!resourceChange.newResourceType) {
if (!resourceChange.resourceType) {
// We use resourceChange.resourceType to loadResourceModel so that we can inspect the
// properties of a resource even after the resource is removed from the template.
continue;
}

const newTypeProps = loadResourceModel(resourceChange.newResourceType)?.properties || {};
const newTypeProps = loadResourceModel(resourceChange.resourceType)?.properties || {};
for (const [propertyName, prop] of Object.entries(newTypeProps)) {
const propScrutinyType = prop.scrutinizable || PropertyScrutinyType.None;
if (scrutinyTypes.includes(propScrutinyType)) {
Expand Down
13 changes: 13 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/lib/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,19 @@ class Formatter {
this.printSectionHeader('IAM Policy Changes');
this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarizeManagedPolicies()), this.stream.columns));
}

if (changes.ssoPermissionSets.hasChanges || changes.ssoInstanceACAConfigs.hasChanges || changes.ssoAssignments.hasChanges) {
this.printSectionHeader('IAM Identity Center Changes');
if (changes.ssoPermissionSets.hasChanges) {
this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarizeSsoPermissionSets()), this.stream.columns));
}
if (changes.ssoInstanceACAConfigs.hasChanges) {
this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarizeSsoInstanceACAConfigs()), this.stream.columns));
}
if (changes.ssoAssignments.hasChanges) {
this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarizeSsoAssignments()), this.stream.columns));
}
}
}

public formatSecurityGroupChanges(changes: SecurityGroupChanges) {
Expand Down
Loading

0 comments on commit 0a3cb94

Please sign in to comment.