Skip to content
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

Closed
DellanX opened this issue Feb 5, 2024 · 5 comments
Closed

Feature Requests: Inject Scopes middleware option #265

DellanX opened this issue Feb 5, 2024 · 5 comments
Assignees

Comments

@DellanX
Copy link

DellanX commented Feb 5, 2024

Rationale

I need to be able to specify OAuth2 scopes middleware by resource type, relationship and action.
I definitely could easily add by action and maybe even resource 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 and User.Groups.Detach.
With Controller Middleware, I could do the following, but it doesn't enable me to separate scopes such as User.Groups.Show and User.Notifications.Show

    public function __construct(\Illuminate\Http\Request $request)
    {
        $resourceType = 'users';
        foreach(['index', 'show', 'store', 'update', 'destroy'] as $action) {
            $this->middleware("scopes:{$resourceType}.{$action}")->only($action);
        }
        // NOTE: Does not work, as it'd apply the same scope to all relationships. 
        // foreach(['attachRelationship', 'showRelated', 'showRelationship', 'updateRelationship', 'detachRelationship'] as $action) {
        //     $this->middleware("scopes:{$resourceType}.{$action}")->only($action);
        // }
    }

 

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:

    $action['middleware'] = $options['middleware'];

  • Option 2, build this as a scope-check only feature.

    1. A Scope middleware can be defined in the config file.
    2. On relationships, to enable scope checking, the developer defines a callback to generate the scope name(s).
          $server->resource('users', JsonApiController::class)->only('index', 'show', 'delete')->relationships(function (Relationships $relationships) {
              $relationships->hasMany('notifications')->scope(function (string $model, string $related, string $action) {
                  return "{$model}.{$related}.{$action}";
              });;
              $relationships->hasMany('unread-notifications');
              $relationships->hasMany('sessions');
          });

    Finally, the middleware needed to implement scope would be appended to the middleware option when assigned at:

    $action['middleware'] = $options['middleware'];

  • 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 callback

  • Option 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?

@lindyhopchris
Copy link
Contributor

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!

@lindyhopchris
Copy link
Contributor

@DellanX I've implemented this on #268

Could you give it a go? Instructions on the PR as to how to use it. To install:

composer require "laravel-json-api/laravel:dev-feature/route-action-middleware"

And let me know how you get on?

@lindyhopchris lindyhopchris self-assigned this Feb 10, 2024
@DellanX
Copy link
Author

DellanX commented Feb 10, 2024

Woah! This looks like option 1! Nice, I'll definitely give it a shot in the next few days. Thanks for implementing this! 😊

@DellanX
Copy link
Author

DellanX commented Feb 14, 2024

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.

@lindyhopchris
Copy link
Contributor

No problem, thanks for confirming! It's a nice new feature. Tagged in v3.3.0.

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

No branches or pull requests

2 participants