spatie/laravel-permission

Implicite bindings are evaluated before role or permission middleware

Ligniperdus opened this issue · 2 comments

Describe the bug
Calling a route behind the Role middleware should always return 403 if the user does not have the required role. However, if you use implicit binding in the route and the requested model instance does not exist, 404 will be returned instead.
This is because the SubstituteBindings middleware is called for the Role middleware by default.
The 404 is the expected result of the SubstituteBindings middleware in this case:

... Laravel will automatically inject the model instance that has an ID matching the corresponding value from the request URI. If a matching model instance is not found in the database, a 404 HTTP response will automatically be generated.

Currently, the bug has the following consequences:

  • Unauthorized requests do not get the expected response
  • Unauthorized users can find out if database entries exist

A possible solution would be to use Sorting Middleware but this must also be in the Using a middleware documentation with the reference to this bug.

Versions
Fresh Laravel Projekt

laravel/framework: 10.25.2
spatie/laravel-permission: 5.11
PHP version: 8.2
Database version: mysql 8

To Reproduce

Generate a new route that uses implicit binding that located behind role middleware:

Route::group(['middleware' => ['role:super-admin']], function () {
    Route::get('/users/{user}', function (User $user) {
        return $user->email;
    });
});

Write a simple test to check if a request returns the expected 403:

public function test_unauthorized_role_middleware_response(): void
{
    $unusedId = User::max('id') + 1;
    $response = $this->get('/users/' . $unusedId);
    $response->assertStatus(403);
}

Expected behavior
Unauthorized request should return 403 if the route is behind the role or permission middleware.

Additional context
Laravel documentation for implicit binding: https://laravel.com/docs/10.x/routing#implicit-binding

Environment:

  • OS: Debian
  • Version: bullseye

I don't see it as a bug, but a note in the documentation would be necessary, like the following:

NOTE: You must add your custom `Middleware` to `$middlewarePriority` in `app/Http/Kernel.php`.

Docs updated