Skip to content

Commit 597aa4f

Browse files
committed
fix(iam): throw error when statement id is invalid (#34819)
Resolves #34819 by throwing error when PolicyStatement Statement id contains characters not allowed by the API. See docs in https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_sid.html
1 parent 1965014 commit 597aa4f

File tree

15 files changed

+126
-23
lines changed

15 files changed

+126
-23
lines changed
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import * as iam from 'aws-cdk-lib/aws-iam';
2+
import * as cdk from 'aws-cdk-lib';
3+
import * as integ from '@aws-cdk/integ-tests-alpha';
4+
5+
/**
6+
* Integration test to verify that PolicyStatements with valid alphanumeric SIDs
7+
* can be successfully deployed to AWS.
8+
*
9+
* This test validates that the SID validation logic doesn't interfere with
10+
* actual CloudFormation deployment of valid PolicyStatements.
11+
*/
12+
13+
const app = new cdk.App();
14+
const stack = new cdk.Stack(app, 'PolicyStatementSidTest');
15+
16+
const role = new iam.Role(stack, 'TestRole', {
17+
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
18+
inlinePolicies: {
19+
TestPolicy: new iam.PolicyDocument({
20+
statements: [
21+
new iam.PolicyStatement({
22+
sid: 'ValidSid123',
23+
effect: iam.Effect.ALLOW,
24+
actions: ['s3:GetObject'],
25+
resources: ['arn:aws:s3:::example-bucket/*'],
26+
}),
27+
new iam.PolicyStatement({
28+
sid: 'ALLCAPS',
29+
effect: iam.Effect.ALLOW,
30+
actions: ['s3:ListBucket'],
31+
resources: ['arn:aws:s3:::example-bucket'],
32+
}),
33+
new iam.PolicyStatement({
34+
sid: '123456',
35+
effect: iam.Effect.ALLOW,
36+
actions: ['logs:CreateLogGroup'],
37+
resources: ['*'],
38+
}),
39+
new iam.PolicyStatement({
40+
sid: 'abc123DEF',
41+
effect: iam.Effect.ALLOW,
42+
actions: ['logs:CreateLogStream'],
43+
resources: ['*'],
44+
}),
45+
// Test statement without SID (should still work)
46+
new iam.PolicyStatement({
47+
effect: iam.Effect.ALLOW,
48+
actions: ['logs:PutLogEvents'],
49+
resources: ['*'],
50+
}),
51+
],
52+
}),
53+
},
54+
});
55+
56+
const managedPolicy = new iam.ManagedPolicy(stack, 'TestManagedPolicy', {
57+
statements: [
58+
new iam.PolicyStatement({
59+
sid: 'ManagedPolicySid1',
60+
effect: iam.Effect.ALLOW,
61+
actions: ['dynamodb:GetItem'],
62+
resources: ['*'],
63+
}),
64+
],
65+
});
66+
67+
role.addManagedPolicy(managedPolicy);
68+
69+
new integ.IntegTest(app, 'PolicyStatementSidIntegTest', {
70+
testCases: [stack],
71+
});

packages/@aws-cdk/aws-msk-alpha/lib/cluster.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ export class Cluster extends ClusterBase {
631631
this.saslScramAuthenticationKey.addToResourcePolicy(
632632
new iam.PolicyStatement({
633633
sid:
634-
'Allow access through AWS Secrets Manager for all principals in the account that are authorized to use AWS Secrets Manager',
634+
'AllowAccountSecretsManagerKMSOperations',
635635
principals: [new iam.AnyPrincipal()],
636636
actions: [
637637
'kms:Encrypt',

packages/@aws-cdk/aws-msk-alpha/test/__snapshots__/cluster.test.ts.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1030,7 +1030,7 @@ exports[`MSK Cluster created with authentication enabled with combinations of sa
10301030
"AWS": "*",
10311031
},
10321032
"Resource": "*",
1033-
"Sid": "Allow access through AWS Secrets Manager for all principals in the account that are authorized to use AWS Secrets Manager",
1033+
"Sid": "AllowAccountSecretsManagerKMSOperations",
10341034
},
10351035
],
10361036
"Version": "2012-10-17",

packages/@aws-cdk/aws-msk-alpha/test/cluster.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ describe('MSK Cluster', () => {
507507
Effect: 'Allow',
508508
Principal: { AWS: '*' },
509509
Resource: '*',
510-
Sid: 'Allow access through AWS Secrets Manager for all principals in the account that are authorized to use AWS Secrets Manager',
510+
Sid: 'AllowAccountSecretsManagerKMSOperations',
511511
},
512512
],
513513
'Version': '2012-10-17',

packages/@aws-cdk/aws-msk-alpha/test/integ.add-cluster-user.js.snapshot/ScramSecretTestStack.template.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@
469469
"AWS": "*"
470470
},
471471
"Resource": "*",
472-
"Sid": "Allow access through AWS Secrets Manager for all principals in the account that are authorized to use AWS Secrets Manager"
472+
"Sid": "AllowAccountSecretsManagerKMSOperations"
473473
},
474474
{
475475
"Action": [

packages/@aws-cdk/aws-msk-alpha/test/integ.add-cluster-user.js.snapshot/tree.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/aws-msk-alpha/test/integ.add-cluster-user.ts.snapshot/ScramSecretTestStack.template.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@
469469
"AWS": "*"
470470
},
471471
"Resource": "*",
472-
"Sid": "Allow access through AWS Secrets Manager for all principals in the account that are authorized to use AWS Secrets Manager"
472+
"Sid": "AllowAccountSecretsManagerKMSOperations"
473473
},
474474
{
475475
"Action": [

packages/@aws-cdk/aws-msk-alpha/test/integ.add-cluster-user.ts.snapshot/tree.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/aws-msk-alpha/test/integ.cluster-authentication.js.snapshot/aws-cdk-msk-auth-integ.template.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@
529529
"AWS": "*"
530530
},
531531
"Resource": "*",
532-
"Sid": "Allow access through AWS Secrets Manager for all principals in the account that are authorized to use AWS Secrets Manager"
532+
"Sid": "AllowAccountSecretsManagerKMSOperations"
533533
}
534534
],
535535
"Version": "2012-10-17"

packages/@aws-cdk/aws-msk-alpha/test/integ.cluster-authentication.js.snapshot/tree.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)