sebastienheyd / boilerplate

Laravel AdminLTE 3 Boilerplate package with blade components, users, roles and permissions management
MIT License
219 stars 66 forks source link

Menu option ['role' => 'admin'] doesn't work as expected #16

Closed DonDiegoAA closed 5 years ago

DonDiegoAA commented 5 years ago

Hello,

I created a menu that I wanted to make accessible only to the admin role. Here is the code:

<?php

namespace App\Menu;

use Sebastienheyd\Boilerplate\Menu\Builder;

class Configuration
{
    public function make(Builder $menu)
    {
        $menu->add('Configuration', [ 'role' => 'admin', 'icon' => 'cogs' ])
             ->id('configuration')
             ->order(10);

        $menu->addTo('configuration', 'Langues', [ 'route' => 'mayers.languages.index', 'role' => 'admin' ])
             ->activeIfRoute(['mayers.languages.*']);

    }
}

But by logging in with a user whose role is backend_user this menu is visible.

I think the problem comes from Menu/Builder.php, in the add function.

The condition, line 50 if($currentUser->ability (['admin'], [])) { always returns true, , because of the empty array for permissions.

Here is a correction that works for me:

...
         if (isset($options['role']) || isset($options['permission'])) {
            $ability = ['admin'];
            if (isset($options['role'])) {
                $ability = $ability + explode(',', $options['role']);
            }

            $permission = null;  //Set permissions to null rather than an empty array.
            if (isset($options['permission'])) {
                $permission = explode(',', $options['permission']);
            }

            $currentUser = Auth::user();
            if ($currentUser->ability($ability, $permission)) {
                $this->items->push($item);
            }
        } else {
            $this->items->push($item);
        }
...

Another solution is to put the option 'permission' of the menu on a value that does not exist,

$menu->add('Configuration', [ 'permission' => 'fake_admin', 'icon' => 'cogs' ])
             ->id('configuration')
             ->order(10);

but it is not very clean.

By the way, thank you for this great boilerplate.

sebastienheyd commented 5 years ago

Hi,

Changing the empty array by null seems to be the good solution.

Added in 5.8.5