Skip to content

Commit 6c716c6

Browse files
committed
revert: "fix(ses-actions): permissions too wide for S3 action" (#30375)
### Issue # (if applicable) Closes #[30143](#30143). ### Reason for this change Fix the below deployment failure Deployment fails with a Could not write to bucket error: 1:36:13 PM | CREATE_FAILED | AWS::SES::ReceiptRule | TestRuleSetStoreToBucketRule3E41D5CF Could not write to bucket: reprosess3rulestack-testemailstoref58b593c-dxh45g1m3y6b (Service: AmazonSimpleEmailService; Status Code: 400; Error Code: InvalidS3Configuration; Request ID: 817f5520-748b-4bae-b347-ec68df52b675; Proxy: null) This PR reverts the changes introduced in PR #29833 ### Description of changes This PR reverts the change that was made in CDK v2.139.0 to reduce overly broad permissions allocated to SES for the S3 receipt rule action. This resulted in deployment failure where SES is unable to write to s3 bucket. ### Description of how you validated changes Dry-run for integration tests ### 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 9f2bdf7 commit 6c716c6

File tree

8 files changed

+35
-145
lines changed

8 files changed

+35
-145
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-ses-actions/test/integ.actions.js.snapshot/aws-cdk-ses-receipt.assets.json

+2-2
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-ses-actions/test/integ.actions.js.snapshot/aws-cdk-ses-receipt.template.json

+2-28
Original file line numberDiff line numberDiff line change
@@ -86,35 +86,8 @@
8686
"Action": "s3:PutObject",
8787
"Condition": {
8888
"StringEquals": {
89-
"aws:SourceAccount": {
89+
"aws:Referer": {
9090
"Ref": "AWS::AccountId"
91-
},
92-
"aws:SourceArn": {
93-
"Fn::Join": [
94-
"",
95-
[
96-
"arn:",
97-
{
98-
"Ref": "AWS::Partition"
99-
},
100-
":ses:",
101-
{
102-
"Ref": "AWS::Region"
103-
},
104-
":",
105-
{
106-
"Ref": "AWS::AccountId"
107-
},
108-
":receipt-rule-set/",
109-
{
110-
"Ref": "RuleSetE30C6C48"
111-
},
112-
":receipt-rule/",
113-
{
114-
"Ref": "RuleSetFirstRule0A27C8CC"
115-
}
116-
]
117-
]
11891
}
11992
}
12093
},
@@ -313,6 +286,7 @@
313286
}
314287
},
315288
"DependsOn": [
289+
"BucketPolicyE9A3008A",
316290
"FunctionAllowSes1829904A"
317291
]
318292
},

packages/@aws-cdk-testing/framework-integ/test/aws-ses-actions/test/integ.actions.js.snapshot/manifest.json

+3-9
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-ses-actions/test/integ.actions.js.snapshot/tree.json

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

packages/aws-cdk-lib/aws-ses-actions/lib/s3.ts

+22-39
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,32 @@ export interface S3Props {
4242
* a notification to Amazon SNS.
4343
*/
4444
export class S3 implements ses.IReceiptRuleAction {
45-
private rule?: ses.IReceiptRule;
45+
4646
constructor(private readonly props: S3Props) {
4747
}
4848

4949
public bind(rule: ses.IReceiptRule): ses.ReceiptRuleActionConfig {
50-
this.rule = rule;
50+
// Allow SES to write to S3 bucket
51+
// See https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-permissions.html#receiving-email-permissions-s3
52+
const keyPattern = this.props.objectKeyPrefix || '';
53+
const s3Statement = new iam.PolicyStatement({
54+
actions: ['s3:PutObject'],
55+
principals: [new iam.ServicePrincipal('ses.amazonaws.com')],
56+
resources: [this.props.bucket.arnForObjects(`${keyPattern}*`)],
57+
conditions: {
58+
StringEquals: {
59+
'aws:Referer': cdk.Aws.ACCOUNT_ID,
60+
},
61+
},
62+
});
63+
this.props.bucket.addToResourcePolicy(s3Statement);
64+
65+
const policy = this.props.bucket.node.tryFindChild('Policy') as s3.BucketPolicy;
66+
if (policy) { // The bucket could be imported
67+
rule.node.addDependency(policy);
68+
} else {
69+
cdk.Annotations.of(rule).addWarningV2('@aws-cdk/s3:AddBucketPermissions', 'This rule is using a S3 action with an imported bucket. Ensure permission is given to SES to write to that bucket.');
70+
}
5171

5272
// Allow SES to use KMS master key
5373
// See https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-permissions.html#receiving-email-permissions-kms
@@ -79,41 +99,4 @@ export class S3 implements ses.IReceiptRuleAction {
7999
},
80100
};
81101
}
82-
83-
/**
84-
* Generate and apply the receipt rule action statement
85-
*
86-
* @param ruleSet The rule set the rule is being added to
87-
* @internal
88-
*/
89-
public _applyPolicyStatement(receiptRuleSet: ses.IReceiptRuleSet): void {
90-
if (!this.rule) {
91-
throw new Error('Cannot apply policy statement before binding the action to a receipt rule');
92-
}
93-
94-
// Allow SES to write to S3 bucket
95-
// See https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-permissions.html#receiving-email-permissions-s3
96-
const keyPattern = this.props.objectKeyPrefix || '';
97-
const s3Statement = new iam.PolicyStatement({
98-
actions: ['s3:PutObject'],
99-
principals: [new iam.ServicePrincipal('ses.amazonaws.com')],
100-
resources: [this.props.bucket.arnForObjects(`${keyPattern}*`)],
101-
conditions: {
102-
StringEquals: {
103-
'aws:SourceAccount': cdk.Aws.ACCOUNT_ID,
104-
'aws:SourceArn': cdk.Arn.format({
105-
partition: cdk.Aws.PARTITION,
106-
service: 'ses',
107-
region: cdk.Aws.REGION,
108-
account: cdk.Aws.ACCOUNT_ID,
109-
resource: [
110-
`receipt-rule-set/${receiptRuleSet.receiptRuleSetName}`,
111-
`receipt-rule/${this.rule.receiptRuleName}`,
112-
].join(':'),
113-
}),
114-
},
115-
},
116-
});
117-
this.props.bucket.addToResourcePolicy(s3Statement);
118-
}
119102
}

packages/aws-cdk-lib/aws-ses-actions/test/actions.test.ts

+1-18
Original file line numberDiff line numberDiff line change
@@ -190,26 +190,9 @@ test('add s3 action', () => {
190190
Action: 's3:PutObject',
191191
Condition: {
192192
StringEquals: {
193-
'aws:SourceAccount': {
193+
'aws:Referer': {
194194
Ref: 'AWS::AccountId',
195195
},
196-
'aws:SourceArn': {
197-
'Fn::Join': [
198-
'',
199-
[
200-
'arn:',
201-
{ Ref: 'AWS::Partition' },
202-
':ses:',
203-
{ Ref: 'AWS::Region' },
204-
':',
205-
{ Ref: 'AWS::AccountId' },
206-
':receipt-rule-set/',
207-
{ Ref: 'RuleSetE30C6C48' },
208-
':receipt-rule/',
209-
{ Ref: 'RuleSetRule0B1D6BCA' },
210-
],
211-
],
212-
},
213196
},
214197
},
215198
Effect: 'Allow',

packages/aws-cdk-lib/aws-ses/lib/receipt-rule-action.ts

-8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { IReceiptRule } from './receipt-rule';
2-
import { IReceiptRuleSet } from './receipt-rule-set';
32

43
/**
54
* An abstract action for a receipt rule.
@@ -10,13 +9,6 @@ export interface IReceiptRuleAction {
109
*/
1110
bind(receiptRule: IReceiptRule): ReceiptRuleActionConfig;
1211

13-
/**
14-
* Generate and apply the receipt rule action statement
15-
*
16-
* @param ruleSet The rule set the rule is being added to
17-
* @internal
18-
*/
19-
_applyPolicyStatement?(ruleSet: IReceiptRuleSet): void;
2012
}
2113

2214
/**

packages/aws-cdk-lib/aws-ses/lib/receipt-rule.ts

+4-13
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,7 @@ export class ReceiptRule extends Resource implements IReceiptRule {
112112
}
113113

114114
public readonly receiptRuleName: string;
115-
116-
private readonly ruleSet: IReceiptRuleSet;
117-
private readonly actions: IReceiptRuleAction[] = [];
118-
private readonly actionProperties: CfnReceiptRule.ActionProperty[] = [];
115+
private readonly actions = new Array<CfnReceiptRule.ActionProperty>();
119116

120117
constructor(scope: Construct, id: string, props: ReceiptRuleProps) {
121118
super(scope, id, {
@@ -136,7 +133,6 @@ export class ReceiptRule extends Resource implements IReceiptRule {
136133
});
137134

138135
this.receiptRuleName = resource.ref;
139-
this.ruleSet = props.ruleSet;
140136

141137
for (const action of props.actions || []) {
142138
this.addAction(action);
@@ -147,20 +143,15 @@ export class ReceiptRule extends Resource implements IReceiptRule {
147143
* Adds an action to this receipt rule.
148144
*/
149145
public addAction(action: IReceiptRuleAction) {
150-
this.actions.push(action);
151-
this.actionProperties.push(action.bind(this));
146+
this.actions.push(action.bind(this));
152147
}
153148

154149
private renderActions() {
155-
if (this.actionProperties.length === 0) {
150+
if (this.actions.length === 0) {
156151
return undefined;
157152
}
158153

159-
for (const action of this.actions) {
160-
action._applyPolicyStatement?.(this.ruleSet);
161-
}
162-
163-
return this.actionProperties;
154+
return this.actions;
164155
}
165156
}
166157

0 commit comments

Comments
 (0)