-
Notifications
You must be signed in to change notification settings - Fork 43
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
Feature Requests: Inject Scopes middleware option #265
Comments
Hi @DellanX - thanks for raising this issue. Thanks for providing the use case as an example; particularly the bit about not being able to do this for relationships via the controller middleware. That convinced me this needs a solution! I'm going to look into this today. My initial feeling is I'll go with a different approach to your PR, but will see how I get on! |
Woah! This looks like option 1! Nice, I'll definitely give it a shot in the next few days. Thanks for implementing this! 😊 |
It works! And it works well! Thanks again for implementing this! I'm ready for this issue to be closed, as I assume this will be in a future release. |
No problem, thanks for confirming! It's a nice new feature. Tagged in |
Rationale
I need to be able to specify OAuth2 scopes middleware by
resource type
,relationship
andaction
.I definitely could easily add by
action
and maybe evenresource type
by using Controller Middleware, but when combining the three, it'd be much easier to add as a route middleware. And that I am probably not the only one needing this.Specifically, I want to be able to add a scopes such as
User.Groups.Show
andUser.Groups.Detach
.With Controller Middleware, I could do the following, but it doesn't enable me to separate scopes such as
User.Groups.Show
andUser.Notifications.Show
The ask
Similar to the way Gates/Policies were handled, it'd be nice if there was a flag we could enable in the route registration to add the scopes middleware. This would allow API clients to be constrained to a subset of the users permitted actions.
Implementation
I am willing to help implement this, but I want to run the idea by y'all first.
Option 1, Flexible but difficult, I could technically implement some kind of fancy action-to-middleware array option, but this would have to be seperate, because the middleware option could already be an array. Thus this option would have to have priority over the middleware setting at:
laravel/src/Routing/RelationshipRegistrar.php
Line 277 in 4f86051
Option 2, build this as a scope-check only feature.
Finally, the middleware needed to implement
scope
would be appended to the middleware option when assigned at:laravel/src/Routing/RelationshipRegistrar.php
Line 277 in 4f86051
Option 3, hybrid option
Use a key-value array as defined in
option 1
Append the middleware as defined in
option 2
This option is probably the best in my opinion, as it has the hidden feature of allowing users to append any middleware for an action.
(Not just scope middleware).
Option 4, flags
What I'd prefer, is an extension of option 2, which would be to facilitate the defining of a default callback in the service provider.
Thus to consume this feature, the developer would just call the
scope()
function without providing a callbackOption 5, ???
I'm always open to more ideas
Circling back
I'm willing to help implement this. Are there any initial concerns or feedback before I get started on developing/PRing this?
The text was updated successfully, but these errors were encountered: