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

Access-Control-Allow-Origin should be explicitly set #712

Closed
tatsuya-ogawa opened this issue Nov 6, 2016 · 11 comments
Closed

Access-Control-Allow-Origin should be explicitly set #712

tatsuya-ogawa opened this issue Nov 6, 2016 · 11 comments
Assignees

Comments

@tatsuya-ogawa
Copy link

The Access-Control-Allow-Origin value should not be wildcard.
So set the origin explicitly.
Otherwise, the chrome ajax requests not works.

@paganotoni
Copy link
Contributor

@vishr thoughts on this one ?

@vishr
Copy link
Member

vishr commented Nov 11, 2016

@tatsuya-ogawa We need more details on this issue.

@tatsuya-ogawa
Copy link
Author

tatsuya-ogawa commented Nov 11, 2016

@vishr @apaganobeleno
When we use cors middleware with setting AllowOrigins "*", then the echo server reply "Access-Control-Allow-Origin: *".
But this method can not coexist with the method using credential.
So I thought Access-Control-Allow-Origin should be explicitly set.

This is my commit.
https://github.com/tatsuya-ogawa/echo/commit/2e9df5bc5f717cd71d9c4cc3b273d8fe5ee35f2d
This commit can not pass tests.
Since the expected value of assert contains a wildcard testcase.

This is related stackoverflow issues
http://stackoverflow.com/questions/19743396/cors-cannot-use-wildcard-in-access-control-allow-origin-when-credentials-flag-i/19744754#19744754

@paganotoni
Copy link
Contributor

@tatsuya-ogawa @vishr please correct me if i'm wrong but Echo allows to configure the "Access-Control-Allow-Origin" domains when you use CORSWithConfig() function.

By default it sets the CORS with Access-Control-Allow-Origin set to '*' inside DefaultCORSConfig , but i think i will be hard to know which domain to put there by default other than *, anyways for production deployments i would recommend to user the CORSWithConfig version instead of the default one.

@tatsuya-ogawa
Copy link
Author

@apaganobeleno
CORSWithConfig do not provide the way of the dynamic Access-Control-Allow-Origin handling,
so we can not realize CORS with wildcard.

Since other languages, frameworks provides the dynamic Access-Control-Allow-Origin handling,
I thought it is better to implement it.

@vishr
Copy link
Member

vishr commented Nov 12, 2016

@tatsuya-ogawa Please verify if this is fixed with the recent changes to v2 and v3.

@tatsuya-ogawa
Copy link
Author

@vishr
v2 and v3 are same.
AllowOrigins is same []string.
In order to realize dynamic setting, it must have a function field.

@vishr
Copy link
Member

vishr commented Nov 13, 2016

@tatsuya-ogawa How about setting allowedOrigin to origin if it is passed in the request header, otherwise use *? I am not sure about the specs but lets double check before making this change.

@vishr
Copy link
Member

vishr commented Nov 13, 2016

But this method can not coexist with the method using credential.

What credentials are you talking about?

@vishr vishr self-assigned this Nov 13, 2016
@tatsuya-ogawa
Copy link
Author

tatsuya-ogawa commented Nov 13, 2016

@vishr
I'm sorry I did not tell it exactly,Credentials is Access-Control-Allow-Credentials.

@tatsuya-ogawa
Copy link
Author

tatsuya-ogawa commented Nov 13, 2016

setting allowedOrigin to origin if it is passed in the request header

I think so too.
It seems that gin-gonic/gin implementation is like that.
If AllowOrigins value is *, then response's Access-Control-Allow-Credentials become request header's location.

@vishr vishr closed this as completed in 3ee7895 Nov 13, 2016
vishr added a commit that referenced this issue Nov 13, 2016
Signed-off-by: Vishal Rana <[email protected]>
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

No branches or pull requests

3 participants