search

silverstripe / silverstripe-graphql

Serves Silverstripe data as GraphQL representations
BSD 3-Clause "New" or "Revised" License
52 stars 61 forks source link

FIX: Don't 403 error when origin is on CORS-disallowed (fixes #519) #527

Closed kinglozzer closed 1 year ago

kinglozzer commented 1 year ago

This will no longer output a 403 error if origin is invalid - CORS headers are to protect things client side, not server side, so there’s no security benefit in triggering this error.

The Access-Control-Allow-Origin is now only output if the origin is valid. For valid origins, that matches the current behaviour. For invalid origins, this is a behavioural change (it used to 403) but this new behaviour will correctly trigger preflight errors:

Access to XMLHttpRequest at 'http://my-graphql-server.test/' from origin 'http://my-origin.test' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource.

One other minor behavioural change here - if developer has opted to allow all origins, this will now explicitly output Access-Control-Allow-Origin: * instead of Access-Control-Allow-Origin: <current origin>

Issue

kinglozzer commented 1 year ago

bump

GuySartorelli commented 1 year ago

The issue attached to this PR is in our "backlog with PRs" column and will be refined in a future backlog refinement session. We're still pretty busy getting the CMS 5 release ready so we haven't had a chance to look at this properly yet.

emteknetnz commented 1 year ago

@kinglozzer Let me know if you're going to continue with this PR, otherwise I'll close this one for now

kinglozzer commented 1 year ago

Not likely to have time soon 😔