-
Notifications
You must be signed in to change notification settings - Fork 11.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[10.x] Add arguments to the signed middleware to ignore properties #46987
[10.x] Add arguments to the signed middleware to ignore properties #46987
Conversation
Hi there, I guess this is an acceptable solution. Only issue I see is when you would want to ignore the Can we maybe update the factory-like static methods to include ignored parameters?
|
That's a nice idea, let me do some testing here! |
@erikgaal added the implementation that you suggested, thanks! 😉 💪 |
Awesome!! |
The middleware already ships in the skeleton though? You don't have to extend or do anything manually. |
@taylorotwell this is in cases like in the example where multiple routes will have different ignore parameters, in this case, it's needed to create new middleware classes to extend and add the needed parameters in the ignore/except. With this PR the current behavior remains the same if people want to use it with the ignore/except property but it will also provide the other way to use it with the list of properties in the middleware definition. |
I have not better solution but But every alternative I can think of ( Best alternative I can think of is a fluent |
I'll re-open this so @taylorotwell can have another look. |
@WendellAdriel one thought I have with this is should the passed ignore arguments override the default "except" ones totally or should they merge with them? |
@taylorotwell that's a really good question. |
@WendellAdriel I guess I can see it both ways. You may define the things you always want to ignore in the middleware. If you add additional ones on route registration I would potentially expect those (the things you want to sometimes ignore) to be merged in with the things you always want to ignore. |
@taylorotwell tomorrow morning my time I’ll update the PR with that then! |
…s ignore/except list
…l/laravel-framework into signature-middleware-params
@taylorotwell I updated the PR with some changes to allow it to merge the arguments passed in the middleware with the ignore/except property and added a new test case for that |
Thanks! |
I’m happy to help!!! 💪 |
…aravel#46987) * 10.x - Add arguments to the signed middleware to ignore properties * Fix code style * Add possibility to pass ignore parameters to the static methods * Fix code style * formatting * Update middleware to merge arguments passed to middleware to the class ignore/except list --------- Co-authored-by: Taylor Otwell <taylor@laravel.com>
Why
Based on this tweet from Claudio Dekker. This allows passing properties to the
signed
middleware to ignore properties instead of extending it and adding the keys to the$ignore
property.Also, this adds the possibility to pass the ignore parameters to the static methods:
relative
andabsolute
.Pros of this implementation
Result
Middleware
Static Methods