fruitcake / laravel-cors

Adds CORS (Cross-Origin Resource Sharing) headers support in your Laravel application
MIT License
6.27k stars 613 forks source link

unexpected response headers #519

Open hrodenburg opened 3 years ago

hrodenburg commented 3 years ago

Hi,

I'm trying to implement CORS in my Laravel application. At the moment, the correct headers are sent by the Nginx webserver, but implementing this in the application makes more sense I think.

However, I get some unexpected response headers, and cannot understand why that should be correct.

My (testing) config:

'paths' => ['api/*'],
'allowed_methods' => ['GET', 'POST', 'OPTIONS'],
'allowed_origins' => ['*'],
'allowed_origins_patterns' => [],
'allowed_headers' => ['Authorization', '*'],
'exposed_headers' => [],
'max_age' => 0,
'supports_credentials' => false,

When sending a request with or without an Origin header present, the response headers always contains Access-Control-Allow-Origin: *. But why? A request without an Origin header should not return this header, and a request with a specific Origin value should return this value as value of the Access-Control-Allow-Origin key, at least to my understanding. When setting supports_credentials to true, it makes a bit more sense. When an Origin header is specified, it returns the value as expected (the value is set to the value specified in the Origin header). But when the request Origin header is not set, and empty Access-Control-Allow-Origin is returned (no value). This can't be right I think?

Another issue is that Access-Control-Allow-Headers are never set in the response. According to the config, this should get managed by allowed_headers, but that does not seem to work at all. I tried * or a specific string, but no response header is set. I'm aware that this package is merely only a package for https://github.com/asm89/stack-cors, so if it makes more sense to ask these questions over there, please let me know (but I will have to test the original package to make it behavious the same).

Please let me know if this makes any sense, or that it is just me missing something obvious...

Thanks

barryvdh commented 3 years ago

If supports credentials is false, the is valid and does not need to return the actual host. By adding the header always, it means that the response can be cached with a reverse proxy. With credentials, the is not valid and replaced with the actual host.

The Allow headers should be added I think, otherwise that might be a bug. But I think there are tests for that?

hrodenburg commented 3 years ago

Thanks for your reply Barry, much appreciated!

If supports credentials is false, the * is valid and does not need to return the actual host. By adding the header always, it means that the response can be cached with a reverse proxy. Right, caching! That never crossed my mind. I did notice the "Vary" header, which should've give me a hint though. Thanks!

The Allow headers should be added I think, otherwise that might be a bug. But I think there are tests for that? I'm not sure about the tests, but I could not make it work. I will try to find some time to investigate this further.

barryvdh commented 3 years ago

The Access-Control-Allow-Headers should be added to the Preflight response, not the actual response; https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers As far as I can tell they should be added: https://github.com/fruitcake/laravel-cors/blob/4f92f55ba2d19874e36c8dc56a4fbdf4ba86af28/tests/GlobalMiddlewareTest.php#L141-L167