-
Notifications
You must be signed in to change notification settings - Fork 46
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 #85
Enable CORS #85
Conversation
abbycabs
commented
Aug 14, 2015
- http://enable-cors.org/server_expressjs.html
This will let us start working on #72 |
@guahanweb - could use a review here if you're familiar w/ express! |
@@ -54,6 +54,12 @@ module.exports = function (badgerService) { | |||
state: 'none' | |||
}); | |||
|
|||
app.use(function (req, res, next) { | |||
res.header('Access-Control-Allow-Origin', '*'); | |||
res.header('Access-Control-Allow-Headers', 'Origin, X-Requested-With, Content-Type, Accept'); |
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.
The only thing to consider is whether there are ever going to be additional headers that may be sent from the client. Things like Cache-Control
or other browser headers that may be injected?
This is a pretty generic "catch-all" that is good for general CORS. Just be sure your client isn't attaching any unexpected headers.
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.
This is a good point - we should def revisit when we add a CDN.
cr:gh - LGTM, just consider the header comment. If you're confident with those, I'd say merge it! |
Thanks @guahanweb :) |