Closed y-nk closed 1 year ago
Oooo, could you explain in more detail?
Hi @icebob :) Thanks for answering!
So, as i explained, imho it's confusing that a signature of cors: { origin: "*" }
would end up with a very different result than cors: { origin: ["*"] }
(only thing changing is the array)
This is due to the 1st if case catching cors.origin === '*'
(here) and responding res.setHeader("Access-Control-Allow-Origin", "*")
(value of header being *
).
If the value is an array though, it won't go in the 1st if, rather in the 2nd one else if (this.checkOrigin())
where a value of *
is now considered a regexp wildcard ; for which the response will be res.setHeader("Access-Control-Allow-Origin", origin);
reflecting req's origin.
My recommendation would be:
cors.origin value | expected result |
---|---|
false \| undefined |
no cors header (default should be super secure, not super open) |
true |
cors header reflects origin |
string |
cors header uses string blindly |
(string \| regexp)[] |
if item is string, use === , if item is regexp, use .test() → cors header reflects origin |
if you want i can implement this as part of my current PR #215 instead, and add test on top. Let me know.
@y-nk great, thanks! Please implement it in the PR.
Problem
When setting-up cors, if one would pass:
...it would end up passing here:
https://github.com/moleculerjs/moleculer-web/blob/ad0686a76c8b0e60b1c9a03b9a35f5c4fd31eaf4/src/index.js#L1000-L1001
producing a header such as
Access-Control-Allow-Origin: *
but just wrapping up in an array such as:
...woud end up passing in a different condition:
https://github.com/moleculerjs/moleculer-web/blob/ad0686a76c8b0e60b1c9a03b9a35f5c4fd31eaf4/src/index.js#L1002-L1004
and further
https://github.com/moleculerjs/moleculer-web/blob/ad0686a76c8b0e60b1c9a03b9a35f5c4fd31eaf4/src/index.js#L962-L966
producing a header such as
Access-Control-Allow-Origin: ${origin}
which i find personally very confusing api.
Proposal
Remove the condition which is here: https://github.com/moleculerjs/moleculer-web/blob/ad0686a76c8b0e60b1c9a03b9a35f5c4fd31eaf4/src/index.js#L1000-L1001 in favor of the later one since the result would be similar if not identical.