Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

Why was group-based middleware support removed? #517

Open
kirkbushell opened this issue Feb 3, 2021 · 15 comments
Open

Why was group-based middleware support removed? #517

kirkbushell opened this issue Feb 3, 2021 · 15 comments

Comments

@kirkbushell
Copy link

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
Copy link

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
Copy link

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

@ftrudeau-pelcro
Copy link

Would appreciate feedback @barryvdh

@barryvdh
Copy link
Member

barryvdh commented Mar 6, 2021

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

@kirkbushell
Copy link
Author

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
Copy link

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
Copy link
Member

barryvdh commented Mar 9, 2021

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
Copy link

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
Copy link
Member

barryvdh commented Mar 9, 2021

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

@ftrudeau-pelcro
Copy link

ftrudeau-pelcro commented Mar 9, 2021

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
Copy link
Member

barryvdh commented Mar 9, 2021

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; #395

@ftrudeau-pelcro
Copy link

Any progress here @barryvdh ?

@hbouhadji
Copy link

I had the same issue, and according to #514 you can do something like

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

in the config/cors.php.

tested and working for me

@kirkbushell
Copy link
Author

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

@hbouhadji
Copy link

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" ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants