Allow closure does not get called when Route Model Binding is enabled

[TUNER88]

As soon I start using Route Model Binding allow closure does not get called and user get access to the resource. If I disable Route Model Binding everything works fine.

// app/Providers/RouteServiceProvider.php
public function boot(Router $router)
{
    parent::boot($router);

    $router->model('users', 'App\User');
}

if ($user->hasRole('admin')) {
    // works fine 
    // $authority->allow('manage', 'App\User'); 

    // closure does not get called
    $authority->allow('manage', 'App\User', function ($self, $user) {
        return false;
    });

} else {
    $authority->allow('read', 'all');
}

Any ideas how to fix it?
I'm using Laravel 5.

[tortuetorche]

Hi @TUNER88,

I don't use Route Model Binding, but in your code, can you check if
$authority->deny('manage', 'App\User'); works as expected?
Because, I think, but I'm not sure, that the Route Model Binding skip all authorizations rules of the Authority-Controller package.

Cheers,
Tortue Torche

[TUNER88]

@tortuetorche
$authority->deny('manage', 'App\User'); works as expected.

After hours of debugging, if found that the problem is in $this->loadResource() method.

If i disable Route Model Binding and dump $this->params I get following output:

[▼
  "id" => "1"
  "action" => "show"
  "controller" => "users"
]

$this->loadedInstance()returns true and the resource could be loaded in $this->loadResourceInstance()

If I enable Route Model Bindling, $this->params looks like this:

[▼
  "users" => User {#198 ▶}
  "action" => "show"
  "controller" => "users"
]

$this->loadResource() do nothing if Model Binding is enabled.

The User ResourceInstance is already injected, id is no more available, any idea how to handle this?
Maybe pass something to $this->loadAndAuthorizeResource();?

Thx for your support

[tortuetorche]

Your debugging informations are really useful.
I need to refactor the Parameters class to handle params who contains objects and get the id of these objects. Like I do for string or numeric params.

[tortuetorche]

And I think this issue also occurs with Laravel 4.

[TUNER88]

@tortuetorche any idea how I can temporary fix in my project, until you update your package?
Its really nice package, i would like continue using it, but disabling Model Binding in whole project is impossible at this moment.

[tortuetorche]

Of course, You can edit the vendor/efficiently/authority-controller/src/Efficiently/AuthorityController/Parameters.php file at line 46 and add the code below:

if (is_a($lastRouteParamKey, '\Eloquent')) {
    $lastRouteParamKey = $lastRouteParamKey->getKey();
}

Warning: Untested code.

Then, You should clear compiled files, with this command:
php artisan clear-compiled

[tortuetorche]

I'll try to fix this issue and add tests in a couple of days.

[TUNER88]

@tortuetorche thanks for your advice!
Unfortunately I had no success with provided code.

I have forked your repository and patched the code, works fine with one model binding.
If you are interested, you can look at my commit.

Now I can pass model key in the constructor:
$this->loadAndAuthorizeResource(['modelBindingKey' => 'users']);

Am looking forward to new version.

[tortuetorche]

Many thanks @TUNER88, I don't know yet if I'm going to use your code or find another solution but you have done a good job 👍

[TUNER88]

@tortuetorche thx, this is not the best solution, that's why does not have submitted a merge request.
Multiple model bindings are not supported by current solution, its not so good for nested resources.

[tortuetorche]

You're right

[TUNER88]

@tortuetorche Any progress?

[tortuetorche]

Hi @TUNER88,

Any progress?

Sorry for the delay. Like I said, I don't use Route Model Binding and I don't think it's a very often used feature, so this issue wasn't a priority for me.
But it should be fixed now 😎

Can you update this package in your application and test again your authorization rules, please?

Cheers,
Tortue Torche

[tortuetorche]

Hi @TUNER88,

I have backported the fix for authority-controller 1.2
If you have still this bug, feel free to reopen this issue.

Cheers,
Tortue Torche