Gate authorization always goes to 403 error with explicit model binding

[f-liva]

Laravel 9.18.0 PHP 8.1.0

Authorization by policy of edit and update actions of resource type routes using explicit model binding always goes to 403 error because the model resolved by the explicit model binding is not passed as the underlying \Illuminate\Auth\Access\Gate::authorize arguments.

This is the configuration needed to reproduce the anomaly:

routes/web.php

Route::resource('centers', UserController::class);
Route::resource('admins', UserController::class);
Route::resource('users', UserController::class);

app/Providers/RouteServiceProvider.php

public function boot() {
    Route::model('center', User::class);
    Route::model('admin', User::class);

    ...
}

app/Http/Controllers/UserController.php

public function __construct()
{
    $this->authorizeResource(User::class);
}

app/Policies/UserPolicy.php

namespace App\Policies;

use App\Models\User;
use Illuminate\Auth\Access\HandlesAuthorization;

class UserPolicy
{
    use HandlesAuthorization;

    /**
     * Determine whether the user can view any models.
     *
     * @param  \App\Models\User  $user
     * @return \Illuminate\Auth\Access\Response|bool
     */
    public function viewAny(User $user)
    {
        return true;
    }

    /**
     * Determine whether the user can view the model.
     *
     * @param  \App\Models\User  $user
     * @param  \App\Models\User  $model
     * @return \Illuminate\Auth\Access\Response|bool
     */
    public function view(User $user, User $model)
    {
        return true;
    }

    /**
     * Determine whether the user can create models.
     *
     * @param  \App\Models\User  $user
     * @return \Illuminate\Auth\Access\Response|bool
     */
    public function create(User $user)
    {
        return true;
    }

    /**
     * Determine whether the user can update the model.
     *
     * @param  \App\Models\User  $user
     * @param  \App\Models\User  $model
     * @return \Illuminate\Auth\Access\Response|bool
     */
    public function update(User $user, User $model)
    {
        return true;
    }

    /**
     * Determine whether the user can delete the model.
     *
     * @param  \App\Models\User  $user
     * @param  \App\Models\User  $model
     * @return \Illuminate\Auth\Access\Response|bool
     */
    public function delete(User $user, User $model)
    {
        return true;
    }

    /**
     * Determine whether the user can restore the model.
     *
     * @param  \App\Models\User  $user
     * @param  \App\Models\User  $model
     * @return \Illuminate\Auth\Access\Response|bool
     */
    public function restore(User $user, User $model)
    {
        return true;
    }

    /**
     * Determine whether the user can permanently delete the model.
     *
     * @param  \App\Models\User  $user
     * @param  \App\Models\User  $model
     * @return \Illuminate\Auth\Access\Response|bool
     */
    public function forceDelete(User $user, User $model)
    {
        return true;
    }
}

At this point, opening from browser the same User resource from the three different routes, here is a dump of the $arguments parameter passed to the call of \Illuminate\Auth\Access::authorize during policy verification:

/users/1/edit ✅

array:4 [▼
  "ability" => "update"
  "result" => true
  "user" => 1
  "arguments" => "[0 => Object(App\Models\User)]"
]

The edit view page opens correctly.

/centers/1/edit ❌

array:4 [▼
  "ability" => "update"
  "result" => null
  "user" => 1
  "arguments" => "[0 => null]"
]

The edit view page opens with 403 error.

/admins/1/edit ❌

array:4 [▼
  "ability" => "update"
  "result" => null
  "user" => 1
  "arguments" => "[0 => null]"
]

The edit view page opens with 403 error.

[driesvints]

Heya, thanks for reporting.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as separate commits on the main/master branch and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Please do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

[rodrigopedra]

@f-liva if you run php artisan route:list you will see routes from your centers resource will expect a {center} parameter, and routes from your admins resource will expect a {admin} parameter.

For example, the centers resource will register these routes, among others:

GET http://localhost/centers/{center}
PUT http://localhost/centers/{center}
DELETE http://localhost/centers/{center}

When you use the AuthorizesRequests@authorizeResource it will then register these middlewares:

GET http://localhost/centers/{center}
==> can:view,user

PUT http://localhost/centers/{center}
==> can:update,user

DELETE http://localhost/centers/{center}
==> can:delete,user

But when the Authorize middleware process those abilities it won't find a parameter named user on those routes, and as such will return Unauthorized. Which is expected behavior.

In you case, as you are sharing the same controller, it will also not execute as you would expect

For example:

1. user navigates to http://localhost/centers/{center}
2. SubstituteBindings will use explicit route model binding to resolve the {center} parameter to an User model instance
3. Route dispatcher, when resolving the controller method, won't find a parameter named $user, so the container will provide an empty User instance
4. The resolved User model instance from the {center} parameter is never used and is ignored.

This happens as when resolving the Controller method related to the route, the Container tries to resolve the method parameter by their names, and not by their types.

So SubstituteBindings will provide a variable like this to the container: ['center'=> <User instance>], but when resolving the method the container will look for a $user variable, as you are reusing your UserController across resources, and as no parameter named user was provided to the container, and as it can resolve User, it will instantiate a brand new User instance to the controller.

Both issues can be resolved by mapping the resource parameter upon registration:

<?php // ./routes/web.php

Route::resource('centers', UserController::class)->parameters(['centers' => 'user']);
Route::resource('admins', UserController::class)->parameters(['admins' => 'user']);
Route::resource('users', UserController::class);

This should solve both your Policy issue (which you already noticed), and your binding issue (still unnoticed).

This behavior is decribed in the docs here:

https://laravel.com/docs/9.x/controllers#restful-naming-resource-route-parameters

The code snippet available in the link above is analog to your use case.

Hope this helps =)

[driesvints]

Thanks @rodrigopedra

[mateusjunges]

@driesvints check this repo https://github.com/mateusjunges/bug-report-42982

• Clone the project

• Configure the database and .env file

• Run migrate and seed

• Go to http://localhost:8000/

It will redirect you to http://127.0.0.1:8000/users/3 and return the correct user.

Then, if you change the url to http://127.0.0.1:8000/centers/3, you will get a 403 unauthorized response.

[mateusjunges]

Oops, not needed anymore 😅

[f-liva]

@f-liva if you run php artisan route:list you will see routes from your centers resource will expect a {center} parameter, and routes from your admins resource will expect a {admin} parameter.

For example, the centers resource will register these routes, among others:

GET http://localhost/centers/{center}
PUT http://localhost/centers/{center}
DELETE http://localhost/centers/{center}

When you use the AuthorizesRequests@authorizeResource it will then register these middlewares:

GET http://localhost/centers/{center}
==> can:view,user

PUT http://localhost/centers/{center}
==> can:update,user

DELETE http://localhost/centers/{center}
==> can:delete,user

But when the Authorize middleware process those abilities it won't find a parameter named user on those routes, and as such will return Unauthorized. Which is expected behavior.

In you case, as you are sharing the same controller, it will also not execute as you would expect

For example:

1. user navigates to http://localhost/centers/{center}
2. SubstituteBindings will use explicit route model binding to resolve the {center} parameter to an User model instance
3. Route dispatcher, when resolving the controller method, won't find a parameter named $user, so the container will provide an empty User instance
4. The resolved User model instance from the {center} parameter is never used and is ignored.

This happens as when resolving the Controller method related to the route, the Container tries to resolve the method parameter by their names, and not by their types.

So SubstituteBindings will provide a variable like this to the container: ['center'=> <User instance>], but when resolving the method the container will look for a $user variable, as you are reusing your UserController across resources, and as no parameter named user was provided to the container, and as it can resolve User, it will instantiate a brand new User instance to the controller.

Both issues can be resolved by mapping the resource parameter upon registration:

<?php // ./routes/web.php

Route::resource('centers', UserController::class)->parameters(['centers' => 'user']);
Route::resource('admins', UserController::class)->parameters(['admins' => 'user']);
Route::resource('users', UserController::class);

This should solve both your Policy issue (which you already noticed), and your binding issue (still unnoticed).

This behavior is decribed in the docs here:

https://laravel.com/docs/9.x/controllers#restful-naming-resource-route-parameters

The code snippet available in the link above is analog to your use case.

Hope this helps =)

EPIC, thank you!

I thought it was an internal Laravel bug, so it's not that great!