laravel / ideas

Issues board used for Laravel internals discussions.
939 stars 28 forks source link

Can't pass a class name to a policy when providing additional context #1933

Open Sheco opened 4 years ago

Sheco commented 4 years ago

Description:

According to the docs on https://laravel.com/docs/6.x/authorization#supplying-additional-context

A policy can receive additional parameters like this:

public function update(User $user, Post $post, int $category)

This doesn't work correctly with methods without models when specifying the class name as a string (as suggested in other sections called "Actions That Don't Require Models"), but it does work when passing a new empty instance of this same class.

Steps To Reproduce:

  1. Create a new Laravel app
  2. Create a Post model
  3. Create a PostPolicy policy
  4. Create the following policy method:
    public function create(User $user, \App\Post $post, int $category) {  
        echo "Post: $post->id, category: $category\n";                          
        return true;                                                      
    } 
  1. Try authorizing this method with an empty instance to see it working correctly:
>>> Gate::forUser(new App\User)->authorize('create', [new App\Post, 1])
Post: , category: 1
=> Illuminate\Auth\Access\Response {#3004}
  1. Try authorizing this method with a class name, to see it fail:
>>> Gate::forUser(new App\User)->authorize('create', [App\Post::class, 1])
TypeError: Argument 2 passed to App/Policies/TestPolicy::create() must be an instance of App/Test or null, int given, called in /tmp/test/vendor/laravel/framework/src/Illuminate/Auth/Access/Gate.php on line 706
driesvints commented 4 years ago

I think this simply isn't supported atm. The example in the docs also highlights that you should pass an instance rather than a class name.

I've moved this issue to the ideas repo instead. Feel free to send in a PR to support this 👍

SanderMuller commented 4 years ago

I have this use case as well, and would love to see this added

dcaswel commented 4 years ago

This would be nice to have

iraklisg commented 3 years ago

:+1: for this idea. It could be proved very useful in cases of multi-tenancy