From 4ec8b57d9c929ecdc7e0ba6684c649b756479c0d Mon Sep 17 00:00:00 2001 From: bergjaak <108292982+bergjaak@users.noreply.github.com> Date: Thu, 2 May 2024 10:51:28 -0400 Subject: [PATCH] feat: support for IAM Identity Center in security diff (#30009) ### 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 https://github.com/cdklabs/awscdk-service-spec/pull/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* --- .../@aws-cdk-testing/cli-integ/lib/aws.ts | 2 + .../cli-integ/resources/cdk-apps/app/app.js | 65 ++++++++ .../tests/cli-integ-tests/cli.integtest.ts | 152 ++++++++++++++++++ 3 files changed, 219 insertions(+) diff --git a/packages/@aws-cdk-testing/cli-integ/lib/aws.ts b/packages/@aws-cdk-testing/cli-integ/lib/aws.ts index 73f8c9b..bcf19f4 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/aws.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/aws.ts @@ -19,6 +19,7 @@ export class AwsClients { public readonly s3: AwsCaller; public readonly ecr: AwsCaller; public readonly ecs: AwsCaller; + public readonly sso: AwsCaller; public readonly sns: AwsCaller; public readonly iam: AwsCaller; public readonly lambda: AwsCaller; @@ -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); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index bdcc445..5683951 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -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'); @@ -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, @@ -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); @@ -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 diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index 8635056..7497881 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -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');