Allow custom CORS for the 2.x API

Added link to the CORS module EventSubscriber.

Thinking more about this.. it seems like adding an EventSubscriber on the Kernel Response seems a bit heavy handed, especially if we need to load consumer entities and check their CORS configuration on every request. It seems like we did the same basic thing in 1.x though: the config was loaded from a single variable farm_access_allow_origin and headers were added to all requests via hook_init().

.... ah well of course! Because hook_init was removed from D8, an EventSubscriber is the new approach: https://www.drupal.org/node/2013014

Also just reviewing the issued that added this support to 8.2 because I was curious how it was done...: this commit from #1869548: Opt-in CORS support

Turns out they used a 3rd party middleware: https://github.com/asm89/stack-cors The middleware is always added to the container, but removed in a compiler pass if cors.config.enabled !== true (nicely explained in this comment, also has a link to some Drupal middleware docs).

A couple interesting things:

@catch commented:

Also the distinction from kernel request events (which I also don't like for a lot of the things they get used for - those are the new hook_init()).

and commented:

I have a general concern that middlewares are becoming the new hook_boot(). The usage here is OK, but I have a list as long as my arm of horrible things done in hook_boot() in 7.x and earlier, and the more middlewares we get in core, the more likely it seems we'll get them used as an antipattern in contrib and custom code.

So both approaches have the potential to run on every request and slow things down... doesn't seem like we have a great way around this.

especially if we need to load consumer entities and check their CORS configuration on every request

Instead of this, maybe we update a separate simple config value (or use a simple cache key/value...?) whenever consumer entities are saved and aggregate all the CORS config at that time. Then we just load the cached config value on each request.

@pcambra also mentioned that with this proposed approach we should consider what happens if the site's default.services.yml cors.config is enabled... should farm_api override that value? Or try and merge all of the headers?

It seems that the CORS module approach overrides the Access-Control headers.

Are consumers being loaded on every request already by Simple OAuth?

Good point.. this is true.

I would imagine they only need to be loaded if a request that contains an OAuth token is being processed?

That logic would probably cover most of our use cases. A CORS request could be coming from another tab in an authenticated browsing session too though.

But the general concept of *when* we add the CORS headers is something we could consider more.. the CORS logic in 1.x is pretty simple: it just checks if the request's origin is set, and if it is an "allowed origin", then we always allow the same headers and allow *all* methods. I have this same logic working in 2.x, just need to add some tests.

Wanted to point out too that Drupal core allows each origin to have specific Headers and Methods configured for it. The contrib cors module also allowed origins to only be configured for certain path(s), although that doesn't seem to be possible anymore. I can see why these would be useful... at the least, configuring the allowed Methods might be a nice feature for us to support.

If we do get into supporting these advanced CORS features I think we should consider using the asm89/stack-cors library to handle the logic & setting the correct headers on responses. Main reason being that "it attempts to implement the W3C Recommendation for cross-origin resource sharing" so we don't need to re-invent the wheel ;-)

This is the same library Drupal core is using - they use it as stack middleware, but it can also be used as a library: https://github.com/asm89/stack-cors#example-using-the-library

That said - the simplest approach should be fine for now :-)

I have it working here with tests and a simple documentation update: https://github.com/paul121/farmOS/tree/2.x-consumer_allowed_origins

NOTE: this change will conflict with #3207716: Use BundleFieldDefinition for additional Consumer fields. It might be easier to merge the other bundle fields change first, then rebase this branch on top since it adds another bundle field.