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

added cors support for the express server #6

Closed
wants to merge 1 commit into from
Closed

added cors support for the express server #6

wants to merge 1 commit into from

Conversation

BenjaminDobler
Copy link
Collaborator

While writing a simple test client i noticed that cors support is missing in the express app...

@BenjaminDobler BenjaminDobler changed the title added cars support for the express server added cors support for the express server Jan 20, 2017
@wzr1337
Copy link
Owner

wzr1337 commented Jan 21, 2017

Thanks!

I missed that :)

Let's add some configuration file, where we can read CORS Filter configuration from. Adding it without explicit whitelisting feels like an open door..

I'd also be ok with a command line option for know

wzr1337 added a commit that referenced this pull request Feb 7, 2017
@wzr1337
Copy link
Owner

wzr1337 commented Feb 7, 2017

resloved with ea5208a

@wzr1337 wzr1337 closed this Feb 7, 2017
@codeundkakao
Copy link
Contributor

I think this issue is not fully resolved yet. The 'corsOpts' object is never actually passed on to cors at the moment. This leaves the server open to access from all domains while missing to expose the 'Location' header. Additionally whitelisting with something like '127.0.0.1' would not work as a request's origin header also contains the protocol (e.g. http://) and optionally a port, which then need to be in the response as well.

This should be re-opened and revised.

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.

3 participants