Skip to content

fix: Remove CloudFront functions#406

Open
jackylamhk wants to merge 2 commits into
getlift:masterfrom
jackylamhk:fix/spa-remove-functions
Open

fix: Remove CloudFront functions#406
jackylamhk wants to merge 2 commits into
getlift:masterfrom
jackylamhk:fix/spa-remove-functions

Conversation

@jackylamhk
Copy link
Copy Markdown

@jackylamhk jackylamhk commented Sep 7, 2024

Fixes #129

  • Use 403/404 error responses to replace the request function
  • Use response headers policy to replace the response function

With these changes, the legacy CloudFront functions used to accomplish SPA redirects and injecting security headers to responses are no longer needed. This also fixes an issue when CloudFront functions limits are reached (mentioned in #350).

@jackylamhk
Copy link
Copy Markdown
Author

Not sure how tests are run, I keep getting this error:

 FAIL  test/unit/permissions.test.ts
  ● Test suite failed to run

    TypeError: Converting circular structure to JSON
        --> starting at object with constructor 'Serverless'
        |     property 'yamlParser' -> object with constructor 'YamlParser'
        --- property 'serverless' closes the circle
        at stringify (<anonymous>)

      at messageParent (node_modules/jest-worker/build/workers/messageParent.js:34:19)

@jackylamhk jackylamhk force-pushed the fix/spa-remove-functions branch from a5e241c to 39fa506 Compare September 7, 2024 19:23
@mnapoli
Copy link
Copy Markdown
Member

mnapoli commented Sep 8, 2024

Thanks for the PR!

Yeah unfortunately tests are broken on master because of changes in serverless framework, haven't found time (or even have an idea) to fix it :/

Use 403/404 error responses to replace the request function

Could you explain this? I'm not sure what the impact is? Will the application behave any differently?

Use response headers policy to replace the response function

Are the same exact security headers added? I.e. are there any practical changes for applications already deployed with Lift?

@jackylamhk
Copy link
Copy Markdown
Author

jackylamhk commented Sep 8, 2024

@mnapoli No worries.

Use 403/404 error responses to replace the request function

Could you explain this? I'm not sure what the impact is? Will the application behave any differently?

SPAs need their origin paths rewritten to /index.html to load the entry point, Lift currently uses a CloudFront function to redirect any paths not ending in a whitelisted extension.

While this works, the function is not necessary - by setting the custom error responses for 404 (not found) and 403 (unauthorized, S3 returns one if the file doesn't exist and s3:ListBucket isn't allowed) we can redirect these to the entry point too.

Use response headers policy to replace the response function

Are the same exact security headers added? I.e. are there any practical changes for applications already deployed with Lift?

Here are the new headers (also linked in docs):

Header Value
Referrer-Policy strict-origin-when-cross-origin
Strict-Transport-Security max-age=31536000
X-Content-Type-Options nosniff
X-Frame-Options SAMEORIGIN
X-XSS-Protection 1; mode=block

And here are the old headers:

Header Value
x-frame-options SAMEORIGIN
x-content-type-options nosniff
x-xss-protection 1; mode=block
strict-transport-security max-age=63072000

So it is slightly different: mainly adding the Referrer-Policy.

I didn't retain the option to remove x-frame-options (security.allowIframe) as folks should use the new responseHeadersPolicy key to create a custom policy with securityHeadersBehavior.contentSecurityPolicy instead - for security, instead of allowing all iframes. I could add it back for backward compatibility, though it might be easier to use custom policies if we keep it.

FYI - this build was tested with a SPA and Serverless deployment.

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.

Move from Cloudfront functions to Response headers for security

2 participants