From bd87f7f7552363b4e0fd89f8ff102c8ddd8c75cb Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Mon, 19 Feb 2024 19:16:23 -0500 Subject: [PATCH] add feadback from colifran --- .../aws-sns/test/integ.sns-topic-policy.ts | 5 ++-- packages/aws-cdk-lib/aws-sns/README.md | 10 ++++---- .../aws-cdk-lib/aws-sns/lib/topic-base.ts | 25 ++++++------------- packages/aws-cdk-lib/aws-sns/lib/topic.ts | 11 ++++++++ packages/aws-cdk-lib/aws-sns/test/sns.test.ts | 8 +++--- 5 files changed, 30 insertions(+), 29 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-topic-policy.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-topic-policy.ts index b5e8dfef4ced9..9aa9b9c73b58b 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-topic-policy.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-topic-policy.ts @@ -32,15 +32,14 @@ new TopicPolicy(stack, 'TopicPolicy', { const topicAddPolicy = new Topic(stack, 'TopicAddPolicy', { topicName: 'topicAddPolicy', displayName: 'topicDisplayNameAddPolicy', + enforceSSL: true, }); topicAddPolicy.addToResourcePolicy(new PolicyStatement({ principals: [new ServicePrincipal('s3.amazonaws.com')], actions: ['sns:Publish'], resources: [topicAddPolicy.topicArn], -}), { - enforceSSL: true, -}); +})); new IntegTest(app, 'SNSTopicPolicyInteg', { testCases: [stack], diff --git a/packages/aws-cdk-lib/aws-sns/README.md b/packages/aws-cdk-lib/aws-sns/README.md index 4f9e32f294886..0db9e79a0938e 100644 --- a/packages/aws-cdk-lib/aws-sns/README.md +++ b/packages/aws-cdk-lib/aws-sns/README.md @@ -232,18 +232,18 @@ const topicPolicy = new sns.TopicPolicy(this, 'Policy', { }); ``` -Similiarly for `addToResourcePolicy`, you can enforce SSL by setting the `enforceSSL` flag: +Similiarly you can enforce SSL by setting the `enforceSSL` flag on the topic: ```ts -const topic = new sns.Topic(this, 'TopicAddPolicy'); +const topic = new sns.Topic(this, 'TopicAddPolicy', { + enforceSSL: true, +}); topic.addToResourcePolicy(new iam.PolicyStatement({ principals: [new iam.ServicePrincipal('s3.amazonaws.com')], actions: ['sns:Publish'], resources: [topic.topicArn], -}), { - enforceSSL: true, -}); +})); ``` ## Delivery status logging diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic-base.ts b/packages/aws-cdk-lib/aws-sns/lib/topic-base.ts index 3f9d042eeca00..7e91e89038a6c 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/topic-base.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/topic-base.ts @@ -51,7 +51,7 @@ export interface ITopic extends IResource, notifications.INotificationRuleTarget * will be automatically created upon the first call to `addToResourcePolicy`. If * the topic is imported (`Topic.import`), then this is a no-op. */ - addToResourcePolicy(statement: iam.PolicyStatement, addToResourcePolicyProps?: AddToResourcePolicyProps): iam.AddToResourcePolicyResult; + addToResourcePolicy(statement: iam.PolicyStatement): iam.AddToResourcePolicyResult; /** * Grant topic publishing permissions to the given identity @@ -59,20 +59,6 @@ export interface ITopic extends IResource, notifications.INotificationRuleTarget grantPublish(identity: iam.IGrantable): iam.Grant; } -/** - * Properties for adding a resource policy - */ -export interface AddToResourcePolicyProps { - /** - * Adds a statement to enforce encryption of data in transit when publishing to the topic. - * - * For more information, see https://docs.aws.amazon.com/sns/latest/dg/sns-security-best-practices.html#enforce-encryption-data-in-transit. - * - * @default false - */ - readonly enforceSSL?: boolean; -} - /** * Either a new or imported Topic */ @@ -92,6 +78,11 @@ export abstract class TopicBase extends Resource implements ITopic { */ protected abstract readonly autoCreatePolicy: boolean; + /** + * Adds a statement to enforce encryption of data in transit when publishing to the topic. + */ + protected enforceSSL?: boolean; + private policy?: TopicPolicy; constructor(scope: Construct, id: string, props: ResourceProps = {}) { @@ -139,7 +130,7 @@ export abstract class TopicBase extends Resource implements ITopic { * will be automatically created upon the first call to `addToResourcePolicy`. If * the topic is imported (`Topic.import`), then this is a no-op. */ - public addToResourcePolicy(statement: iam.PolicyStatement, addToResourcePolicyProps?: AddToResourcePolicyProps): iam.AddToResourcePolicyResult { + public addToResourcePolicy(statement: iam.PolicyStatement): iam.AddToResourcePolicyResult { if (!this.policy && this.autoCreatePolicy) { this.policy = new TopicPolicy(this, 'Policy', { topics: [this] }); } @@ -147,7 +138,7 @@ export abstract class TopicBase extends Resource implements ITopic { if (this.policy) { this.policy.document.addStatements(statement); - if (addToResourcePolicyProps?.enforceSSL) { + if (this.enforceSSL) { this.policy.document.addStatements(this.createSSLPolicyDocument()); } return { statementAdded: true, policyDependable: this.policy }; diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic.ts b/packages/aws-cdk-lib/aws-sns/lib/topic.ts index bd55859014bcc..341548644e70d 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/topic.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/topic.ts @@ -67,6 +67,15 @@ export interface TopicProps { * @default - do not archive messages */ readonly messageRetentionPeriodInDays?: number; + + /** + * Adds a statement to enforce encryption of data in transit when publishing to the topic. + * + * For more information, see https://docs.aws.amazon.com/sns/latest/dg/sns-security-best-practices.html#enforce-encryption-data-in-transit. + * + * @default false + */ + readonly enforceSSL?: boolean; } /** @@ -174,6 +183,8 @@ export class Topic extends TopicBase { physicalName: props.topicName, }); + this.enforceSSL = props.enforceSSL; + if (props.contentBasedDeduplication && !props.fifo) { throw new Error('Content based deduplication can only be enabled for FIFO SNS topics.'); } diff --git a/packages/aws-cdk-lib/aws-sns/test/sns.test.ts b/packages/aws-cdk-lib/aws-sns/test/sns.test.ts index 25da0e0388f0e..e1d13ab34dd6b 100644 --- a/packages/aws-cdk-lib/aws-sns/test/sns.test.ts +++ b/packages/aws-cdk-lib/aws-sns/test/sns.test.ts @@ -189,16 +189,16 @@ describe('Topic', () => { test('can add a policy to the topic enforcing ssl', () => { // GIVEN const stack = new cdk.Stack(); - const topic = new sns.Topic(stack, 'Topic'); + const topic = new sns.Topic(stack, 'Topic', { + enforceSSL: true, + }); // WHEN topic.addToResourcePolicy(new iam.PolicyStatement({ resources: ['*'], actions: ['sns:*'], principals: [new iam.ArnPrincipal('arn')], - }), { - enforceSSL: true, - }); + })); // THEN Template.fromStack(stack).hasResourceProperties('AWS::SNS::TopicPolicy', {