laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.22k stars 10.91k forks source link

Policy not registering or instantiated #22847

Closed axdemelas closed 6 years ago

axdemelas commented 6 years ago

Description:

I'm trying to build a basic authorization logic for users by associating a policy class to User model.

Steps To Reproduce:

  1. Inside UserPolicy.php I typed a simple method to test:

    public function view(User $user) 
    {
    dd('called');        
    }
  2. Map and registering the policies in AuthServiceProvider:

    /**
    * The policy mappings for the application.
    *
    * @var array
    */
    protected $policies = [
    \Modules\User\Entities\User::class => \Modules\User\Policies\UserPolicy::class,
    ];
    /**
    * Register any authentication / authorization services.
    *
    * @return void
    */
    public function boot() 
    {
    $this->registerPolicies();
    }

    Following the docs, this is all I need to make things work. The problem is that when I test authorization inside a controller, it seems that the policy class not even was instantiated.

  3. Inside UserController.php:

    /**
    * Display a listing of the resource.
    * @return Response
    */
    public function index() {
    $user = Auth::user();
    
    dd(
        $user->can('view') // Evaluates to "false" without throwing an exception by dd()...
    );
    }
  4. In order to see if it was some inaccuracy in my code, I used the same policy class but now binding it to a Gate and things just worked normally. In AuthServiceProvider.php:

/**
 * Register any authentication / authorization services.
 *
 * @return void
 */
public function boot()
{
    Gate::resource('users', \Modules\User\Policies\UserPolicy::class);
}
  1. Inside UserController.php:
/**
 * Display a listing of the resource.
 * @return Response
 */
public function index() {
    dd(
        Gate::allows('users.view') // "called"
    );
}

What should I try to get Policy classes registered and working?

Dylan-DPC-zz commented 6 years ago

This repo is for bug tracking. Use the forums or slack channel for solving your issue

lagbox commented 6 years ago

User can view what? there is no resource to check. Policies are for resources. You are not passing any resource to the gate to match to a policy. Sounds like you just want an 'ability'(now called 'gates', because Laravel loves to reuse the same naming multiple times for no reason)

$user->can('view', \Modules\User\Entities\User::class); should allow the policy to be used as it knows what resource is being used it can look up the policy. Though not sure what you are doing as the current authenticated user isn't the resource to be authorized against.

LastDragon-ru commented 6 years ago

Unfortunately, seems this is a bug (latest laravel + php 7.1.x) or maybe I should create different bug? In my case, I have policy and resource, but

protected $policies = [
     \User::class => \UserPolicy::class, // Not works
     '\User' =>  \UserPolicy::class, // Works
];

I suppose both should work.

devcircus commented 6 years ago

Like was pointed out, you're not using it correctly. Read the docs carefully, and you'll see what is happening.

LastDragon-ru commented 6 years ago

Yep, in my case the routes config was invalid) Sorry for any inconvenience.

jtomek commented 5 years ago

Ok, I've struggled with policies and this issue in particular for months due to impossible debugging. It's actually worse than debugging queues.

Anyway. I think I have finally figured it out so I will just leave it here for somebody else who might face a similar problem:

Edit: I've stumbled upon some additional issues for Models that consist of multiple words. I spent a couple of hours looking into it, but figured it out now. I added lesson number 2 (a key lesson) because it's really important and it's not mentioned anywhere.

Lesson 1 (a key lesson): In my API resource controllers, I had called my controller methods without the identification of a Model involved, that's a mistake. You must inject the Model too.

Lesson 2 (a key lesson): The naming of your parameters is very important. Make sure you check your route:list and amend your apiResource based on the expected middleware. Example: If your default middleware for your apiResource is: "api,auth:api,can:view,report_page", you need to make sure that your api.php resource route parameters are in line with it:

Route::apiResource('pages', 'ReportPageController')->parameters([
        'pages' => 'report_page'
    ]);

As always, don't forget to clear you routes (php artisan route:clear) in case you cached them.

Note: Don't ask me why I can't pass 'page' as a second parameter into $this->authorizeResource(). See my question at the end.

Lesson 3: Define Gate::before() only once in your app. If you use the Modules package and think that your gate overwrite relates to an instance of AuthServiceProvider in your module, you are wrong. Dumping $ability in your Gate::before functions is a good way to see that this is getting called multiple times.

Lesson 4: Make it simple for yourself (if you can). Declare your $policies in your central app/Providers/AuthServiceProvider only. Don't create further AuthServiceProviders in your modules. It's too uncertain in terms of what's happening behind the curtains - But if you use the Modules package and insist on using multiple AuthServiceProviders, don't forget to add them to your module.json and composer.json, and run composer dump, and php artisan module:dump

Lesson 5: Make sure your are extending a Controller that is extending a BaseController because you need the authorizeResource function from there. E.g.: $this->authorizeResource(\Modules\Reports\Entities\Report::class);

Lesson 6: You've probabaly encountered posts about this topic with different ways how to define the protected $policies =[]. Don't define your Policy.php and Model.php as strings. Use VSCode and Intellisense to include the right Model::class and Policy::class. A wrong namespace is a very common problem when you're debugging this. Be sure and peak behind the classes. There are otherwise too many balls in the air.

Lesson 7: Your destroy method in your resource controllers needs to, unlike in a generated controller, inject a Model class as well (public function destroy(Report $report){}).

Lesson 8: Your controller's store method is without an input Model::class. Store maps into a create() policy method. Make sure you don't include a second parameter there (see lesson number 2 and my question below)

// class ReportPolicy

public function create(\App\User $user)
{
        return $user->hasRole('admin');
}

Lesson 9: authorizeResource() doesn't define a Gate for your index method. I solved it with a middleware $this->middleware(['permission:browse reports'])->only('index');

--

Open questions:

(Q) What is the second parameter of $this->authorizeResource(Report::class) for? I haven't been able to figure it out. The documentation states that

"The authorizeResource method accepts the model's class name as its first argument, and the name of the route / request parameter that will contain the model's ID as its second argument:"

So far, it's working only without a second parameter and testing variants of my route name (report, reports) or url (api/v1/reports) result in a 403. I've seen in route:list that the reports.store route name defines the following middleware "can:create,Modules\Reports\Entities\Report". Passing ("Modules\Reports\Entities\Report") into the authorizeResource as th second parameter doesn't work too. I get an error "ReportController::destroy(), 1 passed and exactly 2 expected".

Disclosure: I'm a user of the "nwidart/laravel-modules" package.

florinbotea1693 commented 5 years ago

Ok, I've struggled with policies and this issue in particular for months due to impossible debugging. It's actually worse than debugging queues.

Anyway. I think I have finally figured it out so I will just leave it here for somebody else who might face a similar problem:

Edit: I've stumbled upon some additional issues for Models that consist of multiple words. I spent a couple of hours looking into it, but figured it out now. I added lesson number 2 (a key lesson) because it's really important and it's not mentioned anywhere.

Lesson 1 (a key lesson): In my API resource controllers, I had called my controller methods without the identification of a Model involved, that's a mistake. You must inject the Model too.

  • e.g. public function show($report){}. That's a mistake. Notice the $report
  • You must pass the Model class too "public function show(Report $report)"

Lesson 2 (a key lesson): The naming of your parameters is very important. Make sure you check your route:list and amend your apiResource based on the expected middleware. Example: If your default middleware for your apiResource is: "api,auth:api,can:view,report_page", you need to make sure that your api.php resource route parameters are in line with it:

Route::apiResource('pages', 'ReportPageController')->parameters([
        'pages' => 'report_page'
    ]);

As always, don't forget to clear you routes (php artisan route:clear) in case you cached them.

Note: Don't ask me why I can't pass 'page' as a second parameter into $this->authorizeResource(). See my question at the end.

Lesson 3: Define Gate::before() only once in your app. If you use the Modules package and think that your gate overwrite relates to an instance of AuthServiceProvider in your module, you are wrong. Dumping $ability in your Gate::before functions is a good way to see that this is getting called multiple times.

Lesson 4: Make it simple for yourself (if you can). Declare your $policies in your central app/Providers/AuthServiceProvider only. Don't create further AuthServiceProviders in your modules. It's too uncertain in terms of what's happening behind the curtains - But if you use the Modules package and insist on using multiple AuthServiceProviders, don't forget to add them to your module.json and composer.json, and run composer dump, and php artisan module:dump

Lesson 5: Make sure your are extending a Controller that is extending a BaseController because you need the authorizeResource function from there. E.g.: $this->authorizeResource(\Modules\Reports\Entities\Report::class);

Lesson 6: You've probabaly encountered posts about this topic with different ways how to define the protected $policies =[]. Don't define your Policy.php and Model.php as strings. Use VSCode and Intellisense to include the right Model::class and Policy::class. A wrong namespace is a very common problem when you're debugging this. Be sure and peak behind the classes. There are otherwise too many balls in the air.

Lesson 7: Your destroy method in your resource controllers needs to, unlike in a generated controller, inject a Model class as well (public function destroy(Report $report){}).

Lesson 8: Your controller's store method is without an input Model::class. Store maps into a create() policy method. Make sure you don't include a second parameter there (see lesson number 2 and my question below)

// class ReportPolicy

public function create(\App\User $user)
{
        return $user->hasRole('admin');
}

Lesson 9: authorizeResource() doesn't define a Gate for your index method. I solved it with a middleware $this->middleware(['permission:browse reports'])->only('index');

--

Open questions:

(Q) What is the second parameter of $this->authorizeResource(Report::class) for? I haven't been able to figure it out. The documentation states that

"The authorizeResource method accepts the model's class name as its first argument, and the name of the route / request parameter that will contain the model's ID as its second argument:"

So far, it's working only without a second parameter and testing variants of my route name (report, reports) or url (api/v1/reports) result in a 403. I've seen in route:list that the reports.store route name defines the following middleware "can:create,Modules\Reports\Entities\Report". Passing ("Modules\Reports\Entities\Report") into the authorizeResource as th second parameter doesn't work too. I get an error "ReportController::destroy(), 1 passed and exactly 2 expected".

Disclosure: I'm a user of the "nwidart/laravel-modules" package.

2.5: authorizeResource(\App\Post::class, 'post') and controller method update($post_id) will give a 403 error without reaching ever the policy. authorizeResource second parameter name should match the vaiable name passed in controller method as model identifier.

MicroDreamIT commented 4 years ago

@florinbotea1693 I am using resource route and policy, I am using $this->authorizeResource(Warehouse::class, 'warehouse'); in construct. The problem I am facing its only accessing to index method on Policy but not to other method like update, destroy which have model $model parameter. What is the problem?

erstefan commented 4 years ago

@MicroDreamIT Have you found a solution to this?

viewpnt1 commented 4 years ago

@erstefan dont name parameter $model, name it exactly like defined in authorizeResource. For @MicroDreamIT's example it should be update(Warehouse $warehouse)

erstefan commented 4 years ago

Thanks man!

On Tue, 8 Sep 2020 at 12:32, Milan Markovic notifications@github.com wrote:

@erstefan https://github.com/erstefan dont name parameter $model, name it exactly like defined in authorizeResource. For @MicroDreamIT https://github.com/MicroDreamIT's example it should be update(Warehouse $warehouse)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/laravel/framework/issues/22847#issuecomment-688808176, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACIAPKGWT664RM22SP4HEODSEYI4JANCNFSM4EMNMDPQ .

Emerica commented 2 years ago

@viewpnt1 Appreciated, saved me another wasted day trying to figure out why I was getting a 403 because I shortened the variable for readability.

KnudH commented 2 years ago

That costs me some hours before i found this issue and another hour before i got this fixed.

I think instead of let everybody search and try and error for hours it would be more handsome to clarify the docs about this. Its really hard to understand the docs at the moment i think.

BoilingSoup commented 1 year ago

authorizeResource is still not clear at all, and impossible to debug without stumbling into this thread by luck

f21gerb commented 1 year ago

Lesson 1 (a key lesson): In my API resource controllers, I had called my controller methods without the identification of a Model involved, that's a mistake. You must inject the Model too.

@jtomek thank you, this saved me a big headache.

jhomoreira commented 7 months ago

You should name the route with the same name as the model. For example, if your model is called 'post,' your route should also be named 'post.' This is because the authorizeResource method treats the second parameter as optional in case your route has a different name. However, it doesn't take into account that the 'show' ability expects the ID, so it doesn't work for the 'show' function.