search

gr2m / hapi-cors-headers

hapi extension to enable CORS
MIT License
28 stars 11 forks source link

Support HAPI v17 #10

Open frankthelen opened 6 years ago

frankthelen commented 6 years ago

Hi, are there any plans to support HAPI v17? Thanks.

gr2m commented 6 years ago

yeah I see no reason not to :) Can you send a pull request to make the relevant changes? I’ll release it as a breaking release

frankthelen commented 6 years ago

Cool. I try to find some time in the next days. :-)

frankthelen commented 6 years ago

Honestly, I am not event sure if and why this plugin is needed at all. Since Hapi v17 does have CORS support. Well, Hapi v16 did also have CORS support -- so, what is this plugin doing what is not provided by Hapi directly?

frankthelen commented 6 years ago

In the Hapi v17 release notes (https://github.com/hapijs/hapi/issues/3658) it says:

route.options.cors.origin can be set to 'ignore' which provides a CDN-friendly mode that ignores Origin headers and always responds with 'Access-Control-Allow-Origin' set to '*'.

Shouldn't that do the job?

frankthelen commented 6 years ago

I quickly tested with route.options.cors.origin and Hapi v17. Workling just fine. So I don't really see the need for this plugin with Hapi v17. Or am I missing something here?

gr2m commented 6 years ago

I’m not sure, but if it works for you without, I guess it’s fine :) Do you have access-control-allow-origin configured to specific domains? This plugin sets it dynamically based on the origin header. access-control-allow-origin must be set if you send any kind of authentication

frankthelen commented 6 years ago

That's not exactly what it does. You either say route.options.cors.origin = 'ignore' that tells Hapi to always respond with Access-Control-Allow-Origin set to *. Or you set a list of specific domains, e.g., route.options.cors.origin = ['example.com'], even with wildcards, which is the way to go in authentication use cases. See the docs: https://hapijs.com/api#-routeoptionscors. As far as I understand, you cannot configure Hapi out of the box to simply mirror the origin header into Access-Control-Allow-Origin. Not sure if that's really needed. If yes, we can easily update the plugin to support Hapi v17.

frankthelen commented 6 years ago

See https://github.com/gr2m/hapi-cors-headers/pull/11. Better make a new branch at your side and continue from there.