Make CORS configurable

[chillu]

GraphQL APIs will be consumed by third party apps on other hosts, e.g. sending XHR requests from your custom web app to a centralised SaaS solution running a SilverStripe-powered GraphQL endpoint. This requires Cross Origin Resource Sharing

See https://github.com/colymba/silverstripe-restfulapi/blob/master/doc/RESTfulAPI.md#cors for an example.

[PapaBearNZ]

First draft of patch for this issue is up on my fork https://github.com/PapaBearNZ/silverstripe-graphql

Patch adds Yaml config for CORS to the graphql module. Also adds a check for the pre-flight OPTIONS request, and if CORS is enabled, adds the appropriate headers to the response object - both for the OPTIONS response and for the actual data response.

No unit tests as yet.

Looking for suggestions and comments on improvements.

One concern is the 301 redirect from SS to get around the fact that the module folder and the graphql endpoint are the same name. This redirect breaks CORS pre-flight. I suggest that the module folder be renamed to silverstripe-graphql but I realise that this may not be possible for other reasons of which I'm not aware.

[chillu]

Looks great :) A few comments:

• processCorsConfig() would be better named as getCorsResponse()
• Can you add this to the README please? :)
• Tests - but you've already noted that on the chat, heh
• Needs default YAML config
• Should default to an empty string for Allow-Origin, so we don't inadvertendly open up installs to the world via *
• Access-Control-Allow-Headers should only contain GET and POST. The other REST verbs aren't happed for GraphQL
• The 86400 default should be in the default YAML config, not hardcoded in the logic

I've fixed the 301 redirect, see silverstripe/silverstripe-installer#152

[PapaBearNZ]

Thanks Ingo.

Regarding the allow headers . I think we still need the OPTIONS method as well. Otherwise won't the request bounce?

Understood about the YAML default config and I agree with the suggestion of defaulting to denied rather than open. I used '*' because that's what the REST module used but denied is a safer default. I'll make the appropriate changes and hopefully have a PR ready later today.

Thanks for the work on the redirect issue. I'll update and test that.

Regarding the README - no problem there. However, the docs are getting pretty big. Is this going to be updated and pushed to docs/en?

[PapaBearNZ]

#69 PR submitted.

[tractorcow]

Looks like this was fixed with #69 not sure why it's still open.