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

Enable CORS configuration #3809

Merged
merged 2 commits into from Apr 2, 2019
Merged

Enable CORS configuration #3809

merged 2 commits into from Apr 2, 2019

Conversation

dtomcej
Copy link
Contributor

@dtomcej dtomcej commented Aug 21, 2018

What does this PR do?

Enables dynamic CORS configuration, based on (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin) and (https://www.w3.org/TR/cors/#access-control-allow-origin-response-header)

Motivation

Security is good. CORs are good. CORs security headers are better.

Also:
Fixes #3686

More

  • Added/updated tests
    • Preflight Tests
    • Request Tests
    • Response Tests
  • Added/updated documentation

Additional Notes

Due to the complicated nature of CORs, I need some more eyes on this PR to see if I have handled the logic correctly...

@dtomcej
Copy link
Contributor Author

dtomcej commented Oct 5, 2018

This PR is still a WIP, but due to some large configuration PRs that are going to be merged, I will close this, and Re-Open in the future.

@srstsavage
Copy link

I do hope you pick this back up when the timing is right. Looks promising!

@dtomcej
Copy link
Contributor Author

dtomcej commented Dec 27, 2018

Now that the config PRs are merged, reopening this PR!

@dtomcej
Copy link
Contributor Author

dtomcej commented Jan 15, 2019

Yay! Tests Passing!

@ldez ldez moved this from In progress to In review in v2 Mar 21, 2019
@dtomcej dtomcej changed the title Enable CORs configuration Enable CORS configuration Mar 21, 2019
@dtomcej dtomcej force-pushed the issue-3686 branch 2 times, most recently from df78c9f to e386dd2 Compare March 25, 2019 13:28
@dtomcej dtomcej force-pushed the issue-3686 branch 2 times, most recently from 965b7a5 to 12a6303 Compare March 28, 2019 16:59
@dtomcej dtomcej changed the base branch from master to v2.0 March 28, 2019 16:59
Copy link
Contributor

@geraldcroes geraldcroes left a comment

Choose a reason for hiding this comment

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

🎈 LGTM 🎈

@dtomcej dtomcej force-pushed the issue-3686 branch 2 times, most recently from f6dc661 to 63c38e6 Compare April 1, 2019 15:33
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jbdoumenjou jbdoumenjou left a comment

Choose a reason for hiding this comment

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

LGTM

dtomcej and others added 2 commits April 2, 2019 08:02
@Wirone
Copy link

Wirone commented Oct 17, 2019

@dtomcej how allowed origins can be handled while using origin-list-or-null? I would like to add ACCESS-CONTROL-ALLOW-ORIGIN for specified origins only, for other do not allow CORS. I didn't find example for it, is it possible?

@jeff1985
Copy link

jeff1985 commented Dec 4, 2019

@dtomcej I also dont understand how the origin-list is supposed to work? Because there is no config option for a whitelist, the effect of "origin-list-or-null" will be the same as "*", as any domain will be allowed to send requests. Am I overlooking something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v2
Done
Development

Successfully merging this pull request may close these issues.

None yet