spatie / laravel-permission

Associate users with roles and permissions
https://spatie.be/docs/laravel-permission
MIT License
12k stars 1.75k forks source link

Multiple guards are independent from the Auth::guard() which is used for login? #1515

Closed thoresuenert closed 3 years ago

thoresuenert commented 4 years ago

I wrap my head around the multiple guard usage and I am confused. I thought initially I have to duplicate all permissions for both guards, but as long as I use the Same Model for a Guard it doesn't matter.

When I have the base setup with multiple guards as follows:

// config/auth.php
'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'users',
        ],

        'api' => [
            'driver' => 'passport',
            'provider' => 'users',
            'hash' => false,
        ],
    ],
// App\User
class user extends Authenticated
{
  public $guard_name = 'web';
}

This package don't care which guard I used to login. In this scenario the web guard is used for roles and permissions in both scenarios:

Am I right?

PS: that's feels a bit odd :D we use this package several years.

drbyte commented 4 years ago

Would the merge of #1384 solve your confusion? Would merging it break anything in your application?

thoresuenert commented 4 years ago

@drbyte I would argue that all of the automatism and magic does more harm than good.

Especially in the testing department: We spend way to much time to work around all those guard issues. Let me organise my permission and roles and give me full control.

In the #1384 the auth check is way to deep in the application code for my taste. It would be enough to pass the logged in guard to the checkPermissionTo function at define time:

https://github.com/spatie/laravel-permission/blob/782af0045cf95ae9eaf8d96b338349e1572a4cd2/src/PermissionRegistrar.php#L88-L92

right now it would break my application because we had a bad understanding of the guard name. We hardcoded everything to 'api' to work around error messages in tests, because we thought the gaurd_name is bound to the logged in guard etc.

Arguments Against using the logged in guard

// config/auth.php
'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'users',
        ],

        'api' => [
            'driver' => 'passport',
            'provider' => 'users',
            'hash' => false,
        ],

    'admin' => [
            'driver' => 'session',
            'provider' => 'admins',
        ],

    ],

We have an API where we need to use technical users which will get the same set of permissions as the web user because our business logic relies on them.

In a scenario of three guards we need all of the permissions for every guard we use in our application, that is hard to handle and build a nice manageable ui for.

In the current implementation where the guard_name is set on model level we can do the following:

// App\User
$guard_name = 'web'
// App\Admin
$guard_name = 'admin'

and have a good separation for the admin panel and the application (how I planned it out yesterday this will play nicely with Laravel Nova and nova specific policies).

In this setup without logged in guard functionality I am able to use the web guard set of permissions for the web and api guard. That makes logical sense because in the most cases the api will share a fair amount of role/permissions with the web app.

Conclusion In the current implementation the name guard is misleading, it is kind of a bundle/module what so ever which is aligned with a model which can be used with the same guard name. I prefer the current solution right now, because I found the solution for my problem without righting a new package :D

Edit My Point will be much clearer if we talk about $guard_name = user,admin and use nova guard instead of the admin guard.

spatie-bot commented 3 years ago

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.