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

add support for CORS #135

Merged
merged 3 commits into from
Oct 1, 2018
Merged

add support for CORS #135

merged 3 commits into from
Oct 1, 2018

Conversation

antoinerg
Copy link
Collaborator

Adds a --cors option to enable Cross-Origin Resource Sharing (CORS).

This may be out of scope for Orca since it can be handled at the HTTP layer by a downstream server but it was also very simple to add here.

@etpinard
Copy link
Contributor

Very cool. Two comments:

  • Would it better to enable CORS by default? Strictly-speaking this would be a breaking change, so the way you have it here is 👍 for v1. I'm interested in hearing your thoughts on this.
  • This probably deserves a test. An integration test might be nice, but I think an unit similar to this one would suffice.

@antoinerg
Copy link
Collaborator Author

@etpinard I have never seen CORS enabled by default in any HTTP service since it can be a security risk. I think I would leave it off in case we manage sensitive information in Orca down the road.

@etpinard
Copy link
Contributor

etpinard commented Oct 1, 2018

Beautiful.

💃

@antoinerg antoinerg merged commit a0e7314 into master Oct 1, 2018
@antoinerg antoinerg deleted the support-cors branch October 1, 2018 13:39
@etpinard etpinard added this to the v1.2.0 milestone Dec 29, 2018
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.

2 participants