CORS allowed origins doesn't work on per-route basis

nino-vrijman commented 4 years ago

Node version: 12.14.0 (also tried on 8.17.0) Sails version (sails): 1.2.4 (issue also exists in 1.0.2) ORM hook version (sails-hook-orm): 2.1.1 Sockets hook version (sails-hook-sockets): 2.0.0 Organics hook version (sails-hook-organics): Grunt hook version (sails-hook-grunt): 4.0.0 Uploads hook version (sails-hook-uploads): DB adapter & version (e.g. sails-mysql@5.55.5): Skipper adapter & version (e.g. skipper-s3@5.55.5):

Issue

I created two client and server repositories in which you should be able to reproduce this issue, see the READMEs on how to run it. I also deployed the client and server on Heroku.

I was trying to implement the allowOrigins CORS setting on a per route basis and I noticed it wasn't working. The Access-Control-Allow-Origin wouldn't be set to the value I specified in the CORS dictionary of a single route which should be possible according to the docs.

I created a simple action in my UserController called test which just returns 'ok' which should only allowed to be called from https://some-domain.com but when I run the client in the repo above (which runs on on Heroku / localhost) I get a 200 response with an Access-Control-Allow-Origin header value of '*' (equal to my global CORS configuration) while I expected it to fail because I set the route to only allow requests from https://some-domain.com (see below).

Implementation

My implementation in config/routes.js:

'GET /test': {
  action: 'user/test',
  cors: {
    allowOrigins: ['https://somedomain.com'],
  },
},

Most related issue's where closed and I couldn't find an answer / solution to my problem.

Workaround

The workaround I used for now is to just include all domains in the global CORS allowOrigins configuration.

Edit: split the reproduction repository up into a separate client and server repository and added links to the apps on Heroku

sailsbot commented 4 years ago

@nino-vrijman Thanks for posting! We'll take a look as soon as possible.

In the mean time, there are a few ways you can help speed things along:

look for a workaround. (Even if it's just temporary, sharing your solution can save someone else a lot of time and effort.)
tell us why this issue is important to you and your team. What are you trying to accomplish? (Submissions with a little bit of human context tend to be easier to understand and faster to resolve.)
make sure you've provided clear instructions on how to reproduce the bug from a clean install.
double-check that you've provided all of the requested version and dependency information. (Some of this info might seem irrelevant at first, like which database adapter you're using, but we ask that you include it anyway. Oftentimes an issue is caused by a confluence of unexpected factors, and it can save everybody a ton of time to know all the details up front.)
read the code of conduct.
if appropriate, ask your business to sponsor your issue. (Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)
let us know if you are using a 3rd party plugin; whether that's a database adapter, a non-standard view engine, or any other dependency maintained by someone other than our core team. (Besides the name of the 3rd party package, it helps to include the exact version you're using. If you're unsure, check out this list of all the core packages we maintain.)

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

svenemtell commented 4 years ago

Any news on this? I have a similar setup and per-route cors doesn't seem to work for me either. My goal is to have cars off globally and just open up for one route.

nino-vrijman commented 4 years ago

Any news on this? I have a similar setup and per-route cors doesn't seem to work for me either. My goal is to have cars off globally and just open up for one route.

You might be able to get this working by configuring CORS globally and then disabling CORS for all other routes except the one you would like to enable it for. This would be extremely inconvenient and cumbersome but it should work 😅

johnabrams7 commented 4 years ago

@nino-vrijman We were able to replicate the issue on localhost and think it could be one of two things:

The default CORS settings (“*”) are applying when they shouldn’t-- i.e. when they’ve been overridden on a per-route basis.
This could be specific to localhost, so we could try recreating the behavior with two different domains such as putting the client and server on Heroku apps and test it out with foobar.herokuapp.com instead of localhost.

@svenemtell Ah for that case, what you're trying to do should work. Do you have further details of what's creating that behaviour (perhaps a repo that reproduces the issue if feasible)?

nino-vrijman commented 4 years ago

@johnabrams7 Thanks for your response!

1. The default CORS settings (“*”) are applying when they shouldn’t-- i.e. when they’ve been overridden on a per-route basis.

This definitely seems to be the case when using the reproduction repository I made.

2. This could be specific to localhost, so we could try recreating the behavior with two different domains such as putting the client and server on Heroku apps and test it out with foobar.herokuapp.com instead of localhost.

I will try test if the issue still remains when running it on a heroku (or another) server in the upcoming days and I will get back to you.

nino-vrijman commented 4 years ago

@johnabrams7 So I hosted both the client and server on Heroku. Unfortunately the issue still persists, when clicking the button on the client I would've expect it to fail with a CORS error saying only request from https://some-domain.com are allowed to be made.

I splitted the reproduction repository into a separate client and server repository.

nino-vrijman commented 4 years ago

Any updates on this?

eashaw commented 4 years ago

Hi @nino-vrijman, unfortunately we are not using CORS in any of our projects right now and we haven't been able to look into this issue further.

balderdashy / sails