Why run request handlers "successfully" if the request fails CORS?

[wearhere]

If a request fails CORS as specified by this module's configuration, the module will not set the CORS headers on the response. However, it will still call next, causing the usual request handler to run. This can be a problem if the request is expensive to handle, or has side effects.

What is the thinking behind this? It seems reasonable to terminate the request immediately, or at least call next with an error, if it's going to fail client-side due to the lack of CORS headers.

Here is a small project demonstrating this issue: https://github.com/mixmaxhq/cors-response-tester.

[dougwilson]

This is how the CORS specification (http://www.w3.org/TR/cors/) is written.

[dougwilson]

Essentially CORS is written as a form of "double-opt-in" such that a server that has no idea what CORS is is not vulnerable to anything. Your example route would still be vulnerable with a simple <img src='http://localhost:9999/something-expensive'> since no Origin header is sent at all, so CORS does not even come into play. It is important to understand what the exact types of protections the CORS specification provides and does not provide; this module simply implements the specification as outlined by the W3C :)

[dougwilson]

I forgot to mention, that if you do want to implement this, you can do so by passing a function to the origin option and providing an Error object to the callback when you don't allow the origin, causing the Express error handing stack to take over instead of invoking your route. Do keep in mind that you are not getting some kind of security mechanism out of this, as can be demoed by my <img> tag example, though.

[wearhere]

Thanks for the quick and thorough response @dougwilson. I don't see why you would want to "stop" at the CORS specification—what's the use of responding if the response can't be used?—but that's your call. Easy to fix in user-space / another package.

Do keep in mind that you are not getting some kind of security mechanism out of this, as can be demoed by my <img> tag example, though.

At least for my use case, the origin callback can simply return an Error if an origin header is missing, i.e. I'm securing routes that are only designed to be called via AJAX.

[dougwilson]

Hi @wearhere you were asking, technically why the default behavior was the way it was, which is that the default is closest to the specification. We provide mechanisms to differ from the spec as I provided above for your use-case, so I hope that helps.

The CORS specification was never designed as a security mechanism to prevent routes from being called; it was only designed as a mechanism to allow a hole to be added to the cross-origin rules of browser user agents. The specification is designed such that a "CORS failure" should look exactly as if the server has no idea what CORS is--in which case the request will still go through.

I would suggest bring this up to the W3C if you would like implementers to implement it differently, and I would be happy to do so; this really isn't the forum to get changes progressed through the specification :)

[wearhere]

Ah, @dougwilson your defaults make sense to me. What I wonder is, would you be open to a PR that added a setting to simplify the "have the origin callback return an Error" approach? Like,

cors({
  // Modelled after `preflightContinue` / `optionsSuccessStatus`.
  mismatchContinue: false, // defaults to `true`
  mismatchStatus: 400 // the default
})

and then if the origin didn't match (here, and if the origin failed some new checking here) and !mismatchContinue, the middleware would res.status(mismatchStatus).end().

[dougwilson]

HI @wearhere sure, feel free to make that PR. I'm a little confused, because it won't even work for your use-case; a missing Origin header won't be a mismatch at all, just FYI. Also, I think the line references you gave are not at all where those check should go, because they would only apply to the dynamic origin function, not all the others like a static origin string, array, etc.

module would res.status(mismatchStatus).end()

Please be aware that this module supports all Node.js and not is Express-only, so you need to use only the Node.js API https://nodejs.org/dist/latest-v6.x/docs/api/http.html#http_class_http_serverresponse

[wearhere]

Here's my PR @dougwilson: #110 Thanks for your consideration!