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.

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

Introduce CloudFront secure headers [blocked - don't merge] #82

Closed
wants to merge 1 commit into from

Conversation

matteofigus
Copy link
Member

@matteofigus matteofigus commented Mar 19, 2020

Opening this PR as incomplete (no tests and docs yet) because I wanted to show some progress and discuss some items.

Before this approach I tried:

  1. Setting headers just in S3: not possible because of the custom headers
  2. Spiking Amplify Console: it's quite great but depends on a repo. Should create a codecommit repo and populate it via the build which is nasty

Another idea I didn't try was to spin API Gateway with CF backed release on top of S3?

Things I didn't like about this approach (which is still the simplest and doesn't require using new services):

  1. It requires the L@E to be in us-east-1 which I need to do with a Custom Resource 🤦‍♂
  2. Deletion is weird, when you delete the CloudFront trigger is ok, but when you delete the lambda it fails because it's auto replicated and CF trigger disassociation is apparently kind of async (even if it takes its own 20m as usual). According to the docs, it may take a couple of hours. So, we would need to add in the docs that deleting that lambda is manual after a couple of hours from the stack deletion 🤦‍♂ 🤕

If we are ok, I can add tests and clean this up, or we can just close.
Another thing, should we make this optional?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@with_logging
@helper.delete
def delete(event, context):
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we not clean it up too?

Copy link
Member Author

@matteofigus matteofigus Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol read the PR description ^

Copy link
Contributor

@ctd ctd Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, you mightn't be able to delete it anyway.. I don't know if this has changed since 2017 but: hashicorp/terraform-provider-aws#1721

response = event['Records'][0]['cf']['response']
h = response['headers']
h['strict-transport-security'] = [{ 'key': 'Strict-Transport-Security', 'value': 'max-age=63072000; includeSubdomains; preload'}]
h['content-security-policy'] = [{ 'key': 'Content-Security-Policy', 'value': "default-src 'none'; img-src 'self'; script-src 'self'; style-src 'self'; object-src 'none'"}]
Copy link
Member Author

@matteofigus matteofigus Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this is a sample header, in case, I need to work out the right combination to ensure the website works (whitelist here all the dependencies such as the cognito, glue and s3 urls).

@matteofigus matteofigus changed the title Introduce CloudFront secure headers [wip - don't merge yet] Introduce CloudFront secure headers [blocked - don't merge] Apr 20, 2020
@matteofigus matteofigus deleted the cloudfront-headers branch May 27, 2020 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants