-
Notifications
You must be signed in to change notification settings - Fork 403
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(event_handler): disable allow-credentials header when origin allow_origin is * #4638
Conversation
Access-Control-Allow-Origin
not returning *
Access-Control-Allow-Origin
not returning *
Access-Control-Allow-Origin
not returning *
Starting the review. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4638 +/- ##
===========================================
- Coverage 96.46% 96.42% -0.04%
===========================================
Files 223 223
Lines 10735 10744 +9
Branches 1998 2001 +3
===========================================
+ Hits 10355 10360 +5
- Misses 268 270 +2
- Partials 112 114 +2 ☔ View full report in Codecov by Sentry. |
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.
Hi @sthulb! Thanks for working on this.
I'm doing some exploratory tests with different scenarios to see if we're not missing anything. I just left a comment for your review, please see if you agree or not.
Quality Gate passedIssues Measures |
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.
I tested with several scenarios and all the resolvers and everything worked perfectly! Thanks so much for another great work @sthulb!
btw, I don't think we need e2e tests, functional are covering what we need to test.
Issue number: #4589
Summary
Ensures CORS behaviour is correct.
Changes
CORSConfig
Origin
header asACA-Origin
regardless*
, disablesACA-Credentials
CORSConfig
hasallowed_origins
Origin
header asACA-Origin
regardlessACA-Origin
CORSConfig
is set with domains and*
inallowed_origins
Origin
header asACA-Origin
regardless*
and disablesACA-Credentials
The disabling of
Access-Control-Allow-Credentials
to prevent server-side credentials being returned to non-named origins.User experience
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.