Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(aws-s3-cloudfront): Recognize when client specifies enforceSSL: false #559

Merged
merged 4 commits into from
Dec 28, 2021
+1,116 −1,012
Merged

fix(aws-s3-cloudfront): Recognize when client specifies enforceSSL: false #559

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b941911
added condition to omit HTTPS only requests
mickychetta Dec 23, 2021
47ac4b3
added tests for enforce when bucketProp is empty and when enforceSSL …
mickychetta Dec 23, 2021
5c5e128
updated integ tests enforceSSL bucket policy
mickychetta Dec 25, 2021
57eb8d1
updated viperlight ignore
mickychetta Dec 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,10 @@ export function buildS3Bucket(scope: Construct,

customBucketProps = props.bucketProps ? overrideProps(customBucketProps, props.bucketProps) : customBucketProps;

const s3Bucket: s3.Bucket = new s3.Bucket(scope, _bucketId, customBucketProps);
// Set enforceSSL to always false to prevent duplicate policy statement
mickychetta marked this conversation as resolved.
Show resolved Hide resolved
const s3Bucket: s3.Bucket = new s3.Bucket(scope, _bucketId, { ...customBucketProps, enforceSSL: false });

// Apple secure bucket policy through appleSecureBucketPolicy function and not with enforceSSL prop
mickychetta marked this conversation as resolved.
Show resolved Hide resolved
if (customBucketProps.enforceSSL !== false) {
applySecureBucketPolicy(s3Bucket);
}
Expand Down
81 changes: 58 additions & 23 deletions source/patterns/@aws-solutions-constructs/core/test/s3-bucket.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ test('Test bucket policy that only accepts SSL requests only', () => {
PolicyDocument: {
Statement: [
{
Action: "s3:*",
Action: "*",
Condition: {
Bool: {
"aws:SecureTransport": "false"
Expand All @@ -179,12 +179,6 @@ test('Test bucket policy that only accepts SSL requests only', () => {
AWS: "*"
},
Resource: [
{
"Fn::GetAtt": [
"testbucketS3Bucket87F6BFFC",
"Arn"
]
},
{
"Fn::Join": [
"",
Expand All @@ -198,9 +192,34 @@ test('Test bucket policy that only accepts SSL requests only', () => {
"/*"
]
]
},
{
"Fn::GetAtt": [
"testbucketS3Bucket87F6BFFC",
"Arn"
]
}
]
],
Sid: "HttpsOnly"
},
],
Version: "2012-10-17"
}
});
});

test('Test bucket policy that accepts any requests', () => {
const stack = new Stack();

defaults.buildS3Bucket(stack, {
bucketProps: {
enforceSSL: false
}
}, 'test-bucket');

expect(stack).not.toHaveResource("AWS::S3::BucketPolicy", {
PolicyDocument: {
Statement: [
{
Action: "*",
Condition: {
Expand Down Expand Up @@ -242,20 +261,16 @@ test('Test bucket policy that only accepts SSL requests only', () => {
});
});

test('Test bucket policy that accepts any requests', () => {
test('Test enforcing SSL when bucketProps is not provided', () => {
const stack = new Stack();

defaults.buildS3Bucket(stack, {
bucketProps: {
enforceSSL: false
}
}, 'test-bucket');
defaults.buildS3Bucket(stack, {}, 'test-bucket');

expect(stack).not.toHaveResource("AWS::S3::BucketPolicy", {
expect(stack).toHaveResource("AWS::S3::BucketPolicy", {
PolicyDocument: {
Statement: [
{
Action: "s3:*",
Action: "*",
Condition: {
Bool: {
"aws:SecureTransport": "false"
Expand All @@ -266,12 +281,6 @@ test('Test bucket policy that accepts any requests', () => {
AWS: "*"
},
Resource: [
{
"Fn::GetAtt": [
"testbucketS3Bucket87F6BFFC",
"Arn"
]
},
{
"Fn::Join": [
"",
Expand All @@ -285,9 +294,35 @@ test('Test bucket policy that accepts any requests', () => {
"/*"
]
]
},
{
"Fn::GetAtt": [
"testbucketS3Bucket87F6BFFC",
"Arn"
]
}
]
],
Sid: "HttpsOnly"
},
],
Version: "2012-10-17"
}
});
});

test('Test enforcing SSL when bucketProps is provided and enforceSSL is not set', () => {
const stack = new Stack();

defaults.buildS3Bucket(stack, {
bucketProps: {
versioned: false,
publicReadAccess: false
}
}, 'test-bucket');

expect(stack).toHaveResource("AWS::S3::BucketPolicy", {
PolicyDocument: {
Statement: [
{
Action: "*",
Condition: {
Expand Down