-
Notifications
You must be signed in to change notification settings - Fork 485
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
fix(security): Access-Control-Allow-Credentials is all CORS requests #4669
Conversation
Note to reviewers. The diff is showing more deltas than there actually are. This PR is just pulling 3 CORS headers forward into a section that does not depend on the HTTP verb. |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. @@ Coverage Diff @@
## main #4669 +/- ##
=======================================
Coverage 41.84% 41.84%
=======================================
Files 105 105
Lines 9723 9723
=======================================
Hits 4069 4069
Misses 5308 5308
Partials 346 346 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Moved to draft state. Somehow, even through the directives are there, they are not "taking". |
…ests Signed-off-by: Bryon Nevis <bryon.nevis@intel.com>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Ready for review again. Lesson learned. If you use add_header in a block, it forgets about the containing block's add_header directives :-( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Access-Control-Allow-Credentials must be sent for non-preflight requests as well. Note that this PR has TAF approval to send extra headers in order to avoid complicated boolean logic in the nginx rules.
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:
describing the break)Testing Instructions
TAF team will be testing: #4648
New Dependency Instructions (If applicable)