search

fruitcake / laravel-cors

Adds CORS (Cross-Origin Resource Sharing) headers support in your Laravel application
MIT License
6.27k stars 615 forks source link

Why was group-based middleware support removed? #517

Open kirkbushell opened 3 years ago

kirkbushell commented 3 years ago

I currently have a use-case where I have an api.domain not /api setup. And I do not want to have to open up my entire application just to support the api requests.

Is there a way to support this currently, perhaps by filtering by domain somehow? If not, I'm really curious why group-based middleware support was removed =\ It would have handled my use-case perfectly.

ftrudeau-pelcro commented 3 years ago

I concur.

We have multiple different sites poking our API endpoints. A series of middleware must run in sequence (configured as a group of middlewares), that detects the site, amongst other things. Only after some middlewares run that HandleCors should run, as it requires context to, for instance, dynamically set the allowed_origins.

What is the alternative, if any ?

ftrudeau-pelcro commented 3 years ago

Hey @barryvdh, any plan to reinstate support for group-based middleware any time soon ? Thanks for the package btw ;)

ftrudeau-pelcro commented 3 years ago

Would appreciate feedback @barryvdh

barryvdh commented 3 years ago

I think it would work in a group, but you want different options?

kirkbushell commented 3 years ago

I think it would work in a group, but you want different options?

What kind of options? This used to be part of group middleware, which allows you to group routes together nicely for CORS support, which we can no longer do.

ftrudeau-pelcro commented 3 years ago

What kind of options? This used to be part of group middleware, which allows you to group routes together nicely for CORS support, which we can no longer do.

I concur.

barryvdh commented 3 years ago

What kind of error do you het when using it as a group? If you just add to a group and set the path to allow *?

ftrudeau-pelcro commented 3 years ago

What kind of error do you het when using it as a group? If you just add to a group and set the path to allow *?

Just quoting the documentation of latest release:

Group middleware is no longer supported, use the global middleware

barryvdh commented 3 years ago

Yeah it's not officially supported/tested, but I think it should work. Except error handling mostly.

ftrudeau-pelcro commented 3 years ago

Fair enough, please update documentation on how to implement the way it used to work using group middleware (or send it here), and @kirkbushell and I can test on our respective projects. But from my previous tests, if I remember correctly, the package was simply not behaving correctly when applied to group middleware. In other words: not working at all, without exceptions / errors.

barryvdh commented 3 years ago

I think it was a matter of adding the middleware to the group instead of the base middleware, but will check. I had a test, but will see if I can skip some tests so we can make the rest work; https://github.com/fruitcake/laravel-cors/pull/395

ftrudeau-pelcro commented 3 years ago

Any progress here @barryvdh ?

hbouhadji commented 2 years ago

I had the same issue, and according to https://github.com/fruitcake/laravel-cors/pull/514 you can do something like

'paths' => [
    'api.domain' => ['*']
]

in the config/cors.php.

tested and working for me

kirkbushell commented 2 years ago

It definitely doesn't work at the group level. @barryvdh would be good to get some clarification on this.

hbouhadji commented 2 years ago

yeah you still need to put it in global but why do you need to apply it on a group if you can filter by domain with the "hack" ?