[5.2] Fix missing middleware parameters when using authorizeResource()

[misenhower]

This fixes an issue with the authorizeResource method in the AuthorizesResources trait. Previously, the middleware would not be properly applied to some of the controller's methods.

As an example, say you have a PostsController class that uses authorizeResource as follows:

class PostsController extends Controller
{
   function __construct()
   {
        $this->authorizeResource(\App\Post::class);
   }
}

Let's say this controller is registered as a resource:

Route::singularResourceParameters();
Route::resource('posts', 'PostsController');

Running php artisan route:list will reveal that the controller's create and edit methods do not have the proper middleware applied:

+--------+-----------+-------------------+---------------+----------------------------------------------+-------------------------+
| Domain | Method    | URI               | Name          | Action                                       | Middleware              |
+--------+-----------+-------------------+---------------+----------------------------------------------+-------------------------+
|        | GET|HEAD  | /                 |               | Closure                                      | web                     |
|        | POST      | posts             | posts.store   | App\Http\Controllers\PostsController@store   | web,can:create,App\Post |
|        | GET|HEAD  | posts             | posts.index   | App\Http\Controllers\PostsController@index   | web,can:view,App\Post   |
|        | GET|HEAD  | posts/create      | posts.create  | App\Http\Controllers\PostsController@create  | web                     |
|        | DELETE    | posts/{post}      | posts.destroy | App\Http\Controllers\PostsController@destroy | web,can:delete,post     |
|        | PUT|PATCH | posts/{post}      | posts.update  | App\Http\Controllers\PostsController@update  | web,can:update,post     |
|        | GET|HEAD  | posts/{post}      | posts.show    | App\Http\Controllers\PostsController@show    | web,can:view,post       |
|        | GET|HEAD  | posts/{post}/edit | posts.edit    | App\Http\Controllers\PostsController@edit    | web                     |
+--------+-----------+-------------------+---------------+----------------------------------------------+-------------------------+

This happens because authorizeResource repeatedly calls $this->middleware() (here), which replaces the entry in the middleware each time it is called (here).

This was missed by the unit tests for this method because the tests only verified the presence of middleware by name -- the tests didn't make sure the middleware was actually being applied to the correct methods.

After applying this fix, the routing table will appear as follows:

+--------+-----------+-------------------+---------------+----------------------------------------------+-------------------------+
| Domain | Method    | URI               | Name          | Action                                       | Middleware              |
+--------+-----------+-------------------+---------------+----------------------------------------------+-------------------------+
|        | GET|HEAD  | /                 |               | Closure                                      | web                     |
|        | POST      | posts             | posts.store   | App\Http\Controllers\PostsController@store   | web,can:create,App\Post |
|        | GET|HEAD  | posts             | posts.index   | App\Http\Controllers\PostsController@index   | web,can:view,App\Post   |
|        | GET|HEAD  | posts/create      | posts.create  | App\Http\Controllers\PostsController@create  | web,can:create,App\Post |
|        | DELETE    | posts/{post}      | posts.destroy | App\Http\Controllers\PostsController@destroy | web,can:delete,post     |
|        | PUT|PATCH | posts/{post}      | posts.update  | App\Http\Controllers\PostsController@update  | web,can:update,post     |
|        | GET|HEAD  | posts/{post}      | posts.show    | App\Http\Controllers\PostsController@show    | web,can:view,post       |
|        | GET|HEAD  | posts/{post}/edit | posts.edit    | App\Http\Controllers\PostsController@edit    | web,can:update,post     |
+--------+-----------+-------------------+---------------+----------------------------------------------+-------------------------+

The same issue appears in Laravel 5.3, but I am guessing the middleware names applied in this method might be changing since the default Policy stub for resources includes methods like viewAny in addition to view.

[JosephSilber]

Overall this looks good to me 👍

Should we maybe add a comment there in the code explaining what's going on? Without knowing about the middleware overriding one another, that code looks little weird.

[JosephSilber]

You may also want to add the method to the error message.

[misenhower]

Great feedback, thanks. After thinking about it I realized it might be better to simulate each request in the test to avoid relying on implementation details of the controller class.

Let me know if you notice any other issues. Thanks!