Skip to content

add shape headers to CORS expose headers #11650

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Oct 7, 2024

Motivation

This was supposed to be a hackathon project that I forgot to present. This is related to #11587 and #11577.
We can make use of the specs to automatically expose headers as given by the specs for a specific operation, instead of adding them one by one.

This also adds a small fix related to the Allow-Headers, as we were trying to access the response headers instead of the request header. I'm not too sure of the existing behavior here to be honest, as it effectively always adds the headers, so the existing list is not really needed.

We could maybe also always use * for Expose headers (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Expose-Headers), as I'm not sure we have a lot of security issues regarding this, and it might just be way lighter than accessing the specs.

In any case, this additional logic is only executed when the request has a Origin header, so when it's coming from a browser, so it shouldn't impact performance in other cases.

The problem with testing is that not many service operations actually return headers, there is the Lambda case in case of exception, but for this you need to actually call lambda.Invoke and get an execution failure, which isn't too adapted to the test_security.py file...

\cc @dfangl @webdev51

Changes

  • add a new method in the CORS enricher to automatically adds the possible headers from an AWS API operation to the Expose CORS headers
  • fix the allowed headers to get all the headers from the request

@bentsku bentsku added the semver: patch Non-breaking changes which can be included in patch releases label Oct 7, 2024
@bentsku bentsku self-assigned this Oct 7, 2024
Copy link

github-actions bot commented Oct 7, 2024

S3 Image Test Results (AMD64 / ARM64)

  2 files  ±0    2 suites  ±0   3m 24s ⏱️ -4s
423 tests ±0  369 ✅ ±0   54 💤 ±0  0 ❌ ±0 
846 runs  ±0  738 ✅ ±0  108 💤 ±0  0 ❌ ±0 

Results for commit 55a2c8d. ± Comparison against base commit f6d2d07.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Oct 7, 2024

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 40m 22s ⏱️ - 1m 13s
3 487 tests ±0  3 074 ✅ +2  413 💤  - 2  0 ❌ ±0 
3 489 runs  ±0  3 074 ✅ +2  415 💤  - 2  0 ❌ ±0 

Results for commit 55a2c8d. ± Comparison against base commit f6d2d07.

♻️ This comment has been updated with latest results.

@bentsku bentsku added this to the Playground milestone Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant