Skip to content

Commit 6df9bfe

Browse files
authored
fix(iam): adding organization id pattern verification (#33555)
### Issue # Closes #32756 ### Reason for this change The original issue was related to over permissive s3 permissions. Which originally was being caused by what seems to be something related to an undefined `iam.OrgranizationPrincipal` being allowed. However when using 2.178.2, I'm not seeing this particular issue, but the policy that is generated could still be incorrectly created by leaving a blank string. `iam.OrgranizationPrincipal('')` This can be avoided with a simple check. Although this is not a golden solution since it's not able to check if that organization exists, but for the use case it's better than nothing. ### Description of changes Adding a regex check that matches the Organization ID regex pattern in the docs; https://docs.aws.amazon.com/organizations/latest/APIReference/API_Organization.html ``` if (!organizationId.match(/^o-[a-z0-9]{10,32}$/)) { throw new Error(`Expected Organization ID must match regex pattern ^o-[a-z0-9]{10,32}$, received ${organizationId}`); } ``` ### Description of how you validated changes Added a test for bad names ``` test('throw error when Organization ID does not match regex pattern', () => { // GIVEN const shortOrgId = 'o-shortname'; const noOOrgName = 'no-o-name'; const longOrgName = 'o-thisnameistoooooooooooooooooolong'; // THEN expect(() => new iam.OrganizationPrincipal(shortOrgId)).toThrow(`Expected Organization ID must match regex pattern ^o-[a-z0-9]{10,32}$, received ${shortOrgId}`); expect(() => new iam.OrganizationPrincipal(noOOrgName)).toThrow(`Expected Organization ID must match regex pattern ^o-[a-z0-9]{10,32}$, received ${noOOrgName}`); expect(() => new iam.OrganizationPrincipal(longOrgName)).toThrow(`Expected Organization ID must match regex pattern ^o-[a-z0-9]{10,32}$, received ${longOrgName}`); }); ``` ### Checklist - [X] 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*
1 parent a6bfe3c commit 6df9bfe

File tree

13 files changed

+356
-55
lines changed

13 files changed

+356
-55
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.role.js.snapshot/cdk.out

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.role.js.snapshot/integ-iam-role-1.assets.json

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.role.js.snapshot/integ-iam-role-1.template.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@
105105
"Action": "sts:AssumeRole",
106106
"Condition": {
107107
"StringEquals": {
108-
"aws:PrincipalOrgID": "o-1234"
108+
"aws:PrincipalOrgID": "o-12345abcde"
109109
}
110110
},
111111
"Effect": "Allow",

packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.role.js.snapshot/integ.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.role.js.snapshot/integiamroleDefaultTestDeployAssert48737E31.assets.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.role.js.snapshot/manifest.json

+172-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)