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

Policy not registering or instantiated #22847

Closed
axdemelas opened this issue Jan 18, 2018 · 16 comments
Closed

Policy not registering or instantiated #22847

axdemelas opened this issue Jan 18, 2018 · 16 comments

Comments

@axdemelas
Copy link

  • Laravel Version: 5.5.*
  • PHP Version: 7.1.10
  • Database Driver & Version: Mysql

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');        
}
  1. 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.

  1. 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()...
    );
}
  1. 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
Copy link

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

@lagbox
Copy link
Contributor

lagbox commented Jan 18, 2018

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
Copy link
Contributor

LastDragon-ru commented Jan 29, 2018

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
Copy link
Contributor

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

@LastDragon-ru
Copy link
Contributor

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

@jtomek
Copy link

jtomek commented Aug 14, 2019

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.

@florinbotea1693
Copy link

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
Copy link

@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
Copy link

erstefan commented Sep 8, 2020

@MicroDreamIT Have you found a solution to this?

@viewpnt1
Copy link

viewpnt1 commented Sep 8, 2020

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

@erstefan
Copy link

erstefan commented Sep 8, 2020 via email

@Emerica
Copy link

Emerica commented Jan 21, 2022

@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
Copy link

KnudH commented Mar 7, 2022

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
Copy link
Contributor

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

@f21gerb
Copy link

f21gerb commented Oct 21, 2022

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
Copy link

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.

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