Search code examples
laravellaravel-5modelnestednested-resources

Laravel nested resource security


On a project I'm currently working on, I have nested resources like this

route()->resources('user', 'UserController');
route()->resources('user.place', 'PostController');
route()->resources('user.place.picture', 'PictureController');

Everything is working fine, I pass along each time the id of the user, etc...

The trick is when you come to a place, if this is yours I show buttons to edit and make some actions. So I check if the $place.user_id == $user.id to show the buttons.

After playing a little, since to access for example a place of mine I have the following URL : /user/1/place/2 for the Place.show.

I'm the owner so the buttons show and I can modify but if another user (user_id 2 for example) use the same URL to see my place he can't modify anything but can see the place.

What I found is that if this user use the same URL but change the user_id to mine then he can have full access to the place and modify it.

So to protect against this I added another condition : Auth::user()->id == $user->id

What I'm wondering is if there was a solution to prevent this behavior, because if I have to protect every method of a nested resources it's becoming long and ugly to implement. Also when I come to the next level user.place.picture. I think I need to add another layer of security each time I go down. $picture->place_id == $place->id and so on.

So, when I print a link so the user can click on it with the right user, place, picture ID then nothing prevent or check if one id has been modified. If the last one is modified, there's no problem since it'll retrieve another element. But if we modify the parent's id it can be dangerous, especially when I grant some access along the way.

Hope there's a simple solution that I missed !


Solution

  • First of all, I don't think it's necessary for you to perform this check at all;

    $place.user_id == $user.id

    Because as you have discovered, the $user which is derived from the URL can easily be manipulated and thus that guard has no value. You've added a check to make sure the currently logged in User is the owner of the Place but that should be your only check, you don't need both of them.

    If I understand correctly, this sounds like a perfect use case for using Policies in Laravel. The documentation is quite comprehensive but I'll try and sum it up for your specific use case:

    • Generate a Policy, e.g. PlacePolicy
    • For your use case, In this policy, you would create an update() method which accepted as parameters, the User (the logged in User) and Place (the place they are trying to access)
    • The update() method on the PlacePolicy can now determine, by whatever means, if the User can update the Place

    So, it might look like this;

    <?php
    
    namespace App\Policies;
    
    use App\Place;
    use App\User;
    
    class PlacePolicy
    {
        /**
         * Determine if the given place can be updated by the User.
         *
         * @param  \App\User  $user
         * @param  \App\Place  $place
         * @return bool
         */
        public function update(User $user, Place $place)
        {
            return $place->user_id == $user->id;
        }
    }
    

    This is a very simple check - but you can essentially do absolutely anything here to determine whether or not the given User can perform an action on another model.

    NOTE: You will need to Register this Policy in a ServiceProvider, please see the Documentation linked above on how to do this

    With this Policy in place, you can take advantage of the array of helpful tools Laravel provides. For example, you can utilize Blades can directives to do something like this:

    @can('update', $place)
        < SHOW THE EDIT BUTTON >
    @endcan
    

    You can also apply Policy classes via Middleware to prevent the page being accessed in the first place:

    Route::get('/user/{user}/place/{place}' .....)->middleware('can:update,place');
    

    Hopefully this is enough to get you on your way! :)