[proposal] ROADMAP.md: Add CORS proposal to backlog

[ghost]

I'm not entirely familiar with the submission process. If you notice something missing, or the issue does not follow the guidelines completely, I'll submit a corrected version as soon as possible.

Summary

Sails CORS hooks are implemented in a way that differs a bit from the formal specification and can cause some undesired effects in certain enviroments. The primary cause are some blank headers set by Sails that should not be present as far as the aforementioned specification is concerned. While it doesn't matter in most cases, it may prevent other software that sits in from of the Node.js server from setting their own headers unless the CORS hook is disabled. In order to get a "sane" behavior, as well as minimizing the information one "attacker" may be able to obtain from a response, the removal of the blank headers is suggested in all cases but those explicitly required to be compliant with the present conventions regarding request processing. I.e., Sails method to clear CORS headers should effectively remove the headers themselves, not set them to '', and request processing should be terminated in case no matching origin for the request is found, instead of sending Access-Control-Allow-Origin: ''.

The proposal also suggests 2 new additions:

1. The first one leverages the use of the "Vary" header to allow Sails CORS to work with some reverse proxys used for caching. Without that header, a reverse proxy as Varnish, may cache a single version of the response, allowing only a single origin to access the representation of the response. If two or more origins are allowed, then only the origin present in the first request is going to be cached, effectively denying the access from requests from any of the other hosts. If the request is not allowed, then no hosts will be allowed until the request expires from cache. Higher security levels preventing the request from being processed such as those returning status codes 400, 500, etc. shouldn't be a real concern, as those won't be normally cached.
2. The second one would allow the usage of Access-Control-Allow-Origin: * in the response in case of public resources without access checks. This would allow caching of resources even when the Vary header is not supported.

Both options would be disabled by default, and their usage would be completely optional.

Required changes

The changes described in the previous section affect core elements of Sails, and can not be implemented extending it. On top of that, they would require either changes in the test suite ( the first change in particular), or new tests to cover the new functionality (the second and third changes).

The affected files, without any particular order would be:

CORS hook

• clear-headers.js => Replace the usage of res.set for res.removeHeader
• to-prepare-send-headers.js => Minor code changes

Tests

• test/integration/hook.cors_csrf.test.js => Modification of assertions of the headers from denied requests. Inclussion of new tests.

Backend generator, from sails-generate-backend

• templates/config/cors.js => Addition of the new configuration options, if considered relevant.

Documentation

Update the relevant information in the documentation, providing examples, if required.

---

I'd like to ask for forgiveness in case someone finds the present description "a bit rude". English is not my first language and I had trouble to phrase some of the contents in this proposal. It was in no way my intention to offend any of the contributors behind Sails, or to disrespect them or the project itself. I lack experience contributing to other open source projects and did it with the sole purpose of trying be of helpful in regards to the issue at hand.

[sailsbot]

Thanks for posting, @Yn5an3! We'll look into this ASAP.

[ghost]

The proposal relates to issue 3593

[sgress454]

Looks great @Yn5an3, thanks!

[sgress454]

Going to go ahead and merge this, seems a fair proposal.

[sgress454]

Update: CORS support in Sails 1.0 is much more inline with standard practices, including the exclusion of headers rather than setting them to empty string, the use of the vary header, and not reflecting origins when * is used.