thedevdojo / voyager

Voyager - The Missing Laravel Admin
https://voyager.devdojo.com
MIT License
11.78k stars 2.67k forks source link

MenuItemPolicy shouldn't use $slug to check browse permission #2986

Closed MrCrayon closed 6 years ago

MrCrayon commented 6 years ago

Description:

When using a model with a composite name like table name _voyagerhooks permission is going to be browse_voyager_hooks but permission checked using $slug is going to be browse_voyager-hooks. This will return true because not found.

        // If permission doesn't exist, we can't check it!
        if (!self::$permissions->contains('key', 'browse_'.$slug)) {
            return true;
        }

Steps To Reproduce:

Create a bread model with a composite name remove browse permission Menu item still appears

Expected result:

Menu Item should not be shown

MrCrayon commented 6 years ago

I think bug was introduced with #2832 because this will always fail:

if ($str = self::$datatypes->get($slug)) {
    $slug = $str->name;
}

This should solve the problem:

        if (self::$datatypes == null) {
-            self::$datatypes = DataType::all();
+            self::$datatypes = DataType::all()->keyBy('slug');
        }
emptynick commented 6 years ago

Fixed in https://github.com/the-control-group/voyager/pull/2988

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have further questions please ask in our Slack group.