GrafiteInc / CMS

Decoupled CMS for any Laravel app, gain control of: pages, blogs, galleries, events, images, custom modules and more.
https://cms.grafite.ca
MIT License
495 stars 104 forks source link

CMS breaks spatie/laravel-permission roles. #182

Closed davisriska closed 5 years ago

davisriska commented 5 years ago

Describe the bug CMS package breaks the https://github.com/spatie/laravel-permission roles. The users always has all possible roles in the system.

To Reproduce Steps to reproduce the behavior:

  1. Setup /spatie/laravel-permission with some roles and permissions.
  2. Setup cms with https://docs.grafite.ca/cms/#complex-setup
  3. Dump authorized user roles.

Expected behavior Should only show the assigned roles.

Additional context I expect it's something to do with the builder, but I'm not really sure. As it requires you to set it up to use the role functionality, but I haven't done anything like that. I have been ripping my hair out trying to figure out whats wrong. Any help appreciated.

EDIT: As a side note, permissions actually work fine, it's the role part that doesn't work as it for some reason links all the roles to a user.

EDIT 2: After even more debugging I have found out that this seems to only happens on the home page.

mlantz commented 5 years ago

In general the CMS should have nothing to do with any roles or permissions. If you use the simple setup (the command line one) then it will potentially overwrite files and code to handle permissions etc. To access the CMS you need to have an auth setup to handle cms it can be as simple as

Gate::define('cms', function ($user) {
    return (bool) $user;
});

You'd have to customize this definition to work with Spatie's roles and permissions but to be honest I don't know them, and am not sure how you set roles to a user.

davisriska commented 5 years ago

In general the CMS should have nothing to do with any roles or permissions. If you use the simple setup (the command line one) then it will potentially overwrite files and code to handle permissions etc. To access the CMS you need to have an auth setup to handle cms it can be as simple as

Gate::define('cms', function ($user) {
    return (bool) $user;
});

You'd have to customize this definition to work with Spatie's roles and permissions but to be honest I don't know them, and am not sure how you set roles to a user.

As I stated above, I used the complex setup, and thats why its so weird. Nothing was overwritten as far as I know. All that happens is that all the roles get loaded for a user on the homepage.

When installing cms it breaks the @hasanyrole blade directive too it seems. I checked multiple times by uninstalling it and installing it to check.

davisriska commented 5 years ago

Right now I have figured out that this happens if permissions get checked inside a model scope before anywhere else. And when permissions get checked in the scope, the query is without the related user so all of the roles are loaded. I will at a later date figure out what's up with that and give further feedback on it. As I still think that it is in someway related to CMS, It would be nice to leave this issue as is.

mlantz commented 5 years ago

Thanks @davisriska, I'll try adding the Spatie package to my test app and see if I can replicate this in the meanwhile.

capricorn05 commented 5 years ago

Have similar problem. Any update

davisriska commented 5 years ago

Have similar problem. Any update

Hey, for now you can try a fix I did - I made a middleware that checks for a random permission, which in return loads in all the permissions for user and caches that. `

   namespace App\Http\Middleware;

use Closure;
use Illuminate\Support\Facades\Auth;

class PermissionBugFix {
    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request $request
     * @param  \Closure                 $next
     *
     * @return mixed
     */
    public function handle($request, Closure $next) {

        Auth::user() && Auth::user()->can('manage users'); // This is a stupid fix for a stupid bug

        return $next($request);
    }
}

`

Change the can part with a permission you have.

capricorn05 commented 5 years ago

Have problem where @can etc. Not getting picked up in blade also. Will this resolve this issue also

davisriska commented 5 years ago

Have problem where @can etc. Not getting picked up in blade also. Will this resolve this issue also

No, sadly that won't fix it. You should be able to use @can tho. I couldn't use the @hasRole blade specifically.

capricorn05 commented 5 years ago

This is a real serious security risk. Won't be using grafite at the moment.

mlantz commented 5 years ago

Hey @davisriska could you remove the 'cms-language', 'cms-analytics' from the cms routes? I think I found the issue with the Blade directives that I can fix but I'm wondering if those middlewares are breaking things @capricorn05 how about you?

davisriska commented 5 years ago

@mlantz Well, I tried removing the middleware ('cms-language', 'cms-analytics'), it didnt fix the role/permission issue and it didn't fix the blade template problem either.

mlantz commented 5 years ago

So I've spent nearly a full day debugging this, or at least attempting to and I discovered this link https://github.com/spatie/laravel-permission/issues/278 it looks like other people had issues with things related to Blade, as did other packages. Based on thorough testing I'm unable to resolve what the root issue is.

I was able to get a user and their roles and permissions in a fresh build of an app with the CMS, but I wasn't able to get the Blade components of the permissions package to work correctly with the CMS either. I'll make a note in the documentation, about the spate/laravel-permissions package.

That being said, the CMS package works fine with the core auth components and the standard blade components of @can and other authorization elements of the Laravel Framework. The fact that its doesn't play well with a third party auth component doesn't incure that the CMS package itself is a security risk, its simply a concern if you're using spatie/laravel-permissions.

davisriska commented 5 years ago

Thanks for your time and input @mlantz!