subzerocloud / postgrest-starter-kit

Starter Kit and tooling for authoring REST API backends with PostgREST
MIT License
744 stars 71 forks source link

Duplicate CORS headers for POST requests #53

Open toubsen opened 5 years ago

toubsen commented 5 years ago

Communication with the REST API for POST requests will put duplicated CORS (Access-Control-*) headers into any response.

When using the REST API endpoints from a JS app in a browser (I used react-admin as a sample), this behavoiur leads to the following error in a browser console:

Access to fetch at 'http://localhost:8080/rest/rpc/login'
from origin 'http://localhost:3001' has been blocked by CORS policy:
The 'Access-Control-Allow-Origin' header contains multiple values '*, http://localhost:3001',
but only one is allowed. Have the server send the header with a valid value,
or, if an opaque response serves your needs,
set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

Test case with RPC login endpoint (same behaviour for resources):

curl -D- 'http://localhost:8080/rest/rpc/login' \
    -H 'Origin: http://localhost:3001' \
    -H 'content-type: application/json' \
    -H 'accept: application/vnd.pgrst.object+json' \
    --data-binary '{"email":"alice@email.com","password":"pass"}'

Example output:

[...]
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, POST, PATCH, DELETE, OPTIONS
Access-Control-Allow-Credentials: true
[...]
Access-Control-Allow-Origin: http://localhost:3001
Access-Control-Allow-Credentials: true
Access-Control-Expose-Headers: Content-Encoding, Content-Location, Content-Range, Content-Type, Date, Location, Server, Transfer-Encoding, Range-Unit
[...]

IMHO the current cors.conf is not needed at all, as PostgREST will always add CORS headers on its own, (also for OPTIONS requests).

My fix was to comment / delete everything inside openresty/nginx/conf/includes/http/server/locations/rest/cors.conf, which makes everything work perfectly.

The only use case where these headers might be needed, is when using the subzero.jwt_session_cookie module (with subzero-starter-kit) as this module will not add the headers itself.

Maybe I'm overlooking something additional, which might still be dependent on the LUA injection of these headers (you put them there for a reason I guess). A better options to fix it might be to only add these headers by LUA in a response hook, if they're not already there in the response.

ruslantalpa commented 5 years ago

my guess is the conf file was created before postgrest started replying with those headers, i'll look into it, maybe indeed this file needs removing.

on a side note, you really really want to avoid those headers in the first place (both in dev and production, each situation has different solutions). check this issue out https://github.com/subzerocloud/subzero-starter-kit/issues/12

toubsen commented 5 years ago

Thanks for the pointer - this is pretty helpful for the admin backend use case.

If you think of situations, where you want to open up your API to web apps on different domains (e.g. web service usage, maybe anonymous role only), you would like to have proper CORS headers.

mapmeld commented 4 years ago

I see this was addressed in a recent commit, but I still get this error when using fetch() to make a POST request to my project ( https://mggg-states.subzero.cloud ) from localhost or any other domain outside of subzero.cloud - any advice?