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 web API #101

Closed
wants to merge 1 commit into from
Closed

Added CORS support for web API #101

wants to merge 1 commit into from

Conversation

chqr1y
Copy link

@chqr1y chqr1y commented Jun 9, 2020

#94

Added CORS support for web API :

  • the default policy is '*' for '/api/*'
  • an optional argument '--origin' is added to restrict the CORS to one origin

@lanmaster53
Copy link
Owner

Did you thoroughly test this? It looks like it should work since there is no authentication. In fact, we don't even need to include the ability to specify an origin yet. It isn't needed unless there is authentication, which requires the ACAC header. Everything should work with the wildcard ACAO header.

@chqr1y
Copy link
Author

chqr1y commented Jun 9, 2020

Yes, I'm confident in my tests.

I agree with your comment, this behavior is little silly.
In the real world, no one's going to 'origin' argument to reactivate the CORS policy.

I think we should choose between these two behaviors:

  • We use the wildcard ACAO header all the time (For an application that runs on localhost, that sounds okay)
  • We set the default policy to deny and the end user will have to whitelist his application with the argument '--origin'.

What do you prefer?

@lanmaster53
Copy link
Owner

For now just wildcard the ACAO header.

@lanmaster53
Copy link
Owner

Also, please change the target branch to staging, not master. Thanks!

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