Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot assign Role from a specific guard #565

Closed
raftalks opened this issue Nov 18, 2017 · 22 comments

Comments

Projects
None yet
7 participants
@raftalks
Copy link

commented Nov 18, 2017

I have come across this issue where I want to seed in roles and assign them to generated user, and in my case the role is created with a guard name. I am not clear how to assign that role to an instance of user model using assignRole method. Since my default guard is web and not the custom one, I need to assign role from a different guard to the user inside the seeder. I hope I have explained the issue clearly. To explain in detail, here is the tinker output from:

spatie-laravel-permission-error

is there a way to handle this?

@introwit

This comment has been minimized.

Copy link

commented Nov 20, 2017

@raftalks Hi :) We have a good documentation around the use of guards with this package here Hope that helps, let me know if that still doesn't solve your problem :)

@raftalks

This comment has been minimized.

Copy link
Author

commented Nov 20, 2017

@introwit the documentation only shows how to create roles and permissions under multiple guards, but one can't assign a user with a role or permission giving a a specific guard.

@introwit

This comment has been minimized.

Copy link

commented Nov 20, 2017

@raftalks so the role root belongs to the guard console but you want to assign that role root to the User and the User belongs to a different guard than console?

@raftalks

This comment has been minimized.

Copy link
Author

commented Nov 20, 2017

When seeding a user, I am creating the roles under certain guard and don't see any way I can assign the roles for a user instance created when seeding.

When I do try to assign a role, it seems this library is looking for the roles under default guard. This doesn't work well if I have different guards for different resources of an API, where users are not strictly set for a specific guard

@raftalks

This comment has been minimized.

Copy link
Author

commented Nov 20, 2017

@introwit In my current project, I am assuming I can use this library to set roles and permissions by different guards set to different resources. Lets say, I am simply using the same User Providers under each of the custom guards set on my Laravel project.

'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'users',
        ],

        'api' => [
            'driver' => 'passport',
            'provider' => 'users',
        ],
        'console' => [
            'driver' => 'passport',
            'provider' => 'users',
        ],

      // ... some more custom guards
    ],

and then I have route groups with middleware set to work with auth:console, auth:api, etc.

Tell me if this approach is wrong for using custom guard roles and permissions, the idea I had in mind was that I can set separate set of roles and permissions based on which guard the user have used to access the resource.

The issue I am facing is at the time of assigning Roles from a custom guard to a generated user account, such as when using Seeder, it is trying to find the role from the default guard which is web on my application.

am I doing this wrong?

@introwit

This comment has been minimized.

Copy link

commented Nov 20, 2017

@raftalks what I would do in such situation is create different providers for each guard especially when there is authorization to manage too. From the package point of view of assigning roles from a custom guard to a user, it would be nice to have @freekmurze 's thoughts on it :)

@the94air

This comment has been minimized.

Copy link

commented Nov 21, 2017

I had the same problem but I solved it by implementing Role or Permission models to my app and every thing is working fine.
https://github.com/spatie/laravel-permission#extending

@AlexVanderbist

This comment has been minimized.

Copy link
Member

commented Nov 23, 2017

Hi @raftalks, when you try to assign a permission to one of your $usera, the package doesn't know what guard this $user is using. In this case it'll try to look for the permission or role in the first guard (in this case web) and not find the given console permission.

You could solve this (as @introwit suggested) by using different models and different providers for these different types of users. (They could still all extend the same User class.)

@c00p3r

This comment has been minimized.

Copy link

commented Nov 24, 2017

Hi everyone!
Stumbled upon same stuff as @raftalks.
Don't wanna write my own user provider (that's not simple you know...)
And I just can't use another model (because project is already running live)
So any other suggestions? (besides writing own method instead "assignRole" and "getStoredRole")
@the94air could you share your solution, please?
@raftalks have you come up with smthn?

@c00p3r

This comment has been minimized.

Copy link

commented Nov 24, 2017

Here's what I've came up with (don't throw rocks on me):

public function assignRole($roles, string $guard = null)
    {
        $roles = \is_string($roles) ? [$roles] : $roles;
        $guard = $guard ? : $this->getDefaultGuardName();

        $roles = collect($roles)
            ->flatten()
            ->map(function ($role) use ($guard) {
                return $this->getStoredRole($role, $guard);
            })
            ->each(function ($role) {
                $this->ensureModelSharesGuard($role);
            })
            ->all();

        $this->roles()->saveMany($roles);

        $this->forgetCachedPermissions();

        return $this;
    }

    protected function getStoredRole($role, string $guard): Role
    {
        if (\is_string($role)) {
            return app(Role::class)->findByName($role, $guard);
        }

        return $role;
    }

Usage:

$user->assignRole('admin'); // guard will be default
$user->assignRole('admin', 'api'); // explcitly setting the guard

And it kinda works...
Waiting for your comments...

@the94air

This comment has been minimized.

Copy link

commented Nov 24, 2017

At first I got the same exception but after making my own models every thing is working.

<?php
// App\Permission

namespace App;

use Spatie\Permission\Contracts\Permission as PermissionInterface;
use Spatie\Permission\Models\Permission as PermissionModel;
use Illuminate\Database\Eloquent\Model;

class Permission extends PermissionModel implements PermissionInterface
{
    protected $guard_name = 'admin';
}
<?php
// App\Role

namespace App;

use Spatie\Permission\Contracts\Role as RoleInterface;
use Spatie\Permission\Models\Role as RoleModel;
use Illuminate\Database\Eloquent\Model;

class Role extends RoleModel implements RoleInterface
{
    protected $guard_name = 'admin';
}
@c00p3r

This comment has been minimized.

Copy link

commented Nov 27, 2017

@the94air so you're using only one guard 'admin'?
What I'm after is some universal/general solution

@c00p3r

This comment has been minimized.

Copy link

commented Nov 27, 2017

_some code was here_
but I researched and figured out that this thing is done by Laravel auth core already

@the94air

This comment has been minimized.

Copy link

commented Nov 27, 2017

@c00p3r I think that will do the same with multi guard. Is it fixed now?

@c00p3r

This comment has been minimized.

Copy link

commented Nov 27, 2017

@the94air if you use only one guard then you're kinda off-topic here. Because the issue is in that when you use 2 or more guards in your app, you can't assign roles for other guards except default one

@c00p3r

This comment has been minimized.

Copy link

commented Nov 27, 2017

I believe I'll make a PR and see what core dev team say

@c00p3r

This comment has been minimized.

Copy link

commented Nov 28, 2017

@AlexVanderbist u said:

You could solve this by using different models

so I'll need 2 separate DB tables (like api_users, web_users)?

@doghap

This comment has been minimized.

Copy link

commented Dec 1, 2017

i have the same issure here. but add a new function $user->assignRole('admin', 'api'); is not a good solution, because u will meet another problem: middleware. the permission middleware uses the function registerPermissions() in PermissionRegistrar.php. u cant pass your guard name to that function (i think its impossible), so it will use the default guard too. what you need todo is set table, primaryKey attributes and set the foreignkeys to user_id in your relationship functions. just like this.

class User extends Authenticatable
{

   use HasApiTokens, HasRoles;

   protected $table = 'users';

   protected $primaryKey = 'id';

   public function merchant() {
   	return $this->hasOne(Merchant::class, 'user_id');
   }
}

and extend user model:

class MerchantUser extends User {
   protected $guard_name = 'merchant';
}

you maybe want to set your guard's provider's model to MerchantUser in auth.php.
@c00p3r

This comment has been minimized.

Copy link

commented Dec 1, 2017

@doghap you are right
but I solved only the issue with 'assigning' role to guard
NOT checking it via middleware

It is impossible to check role with specific guard in middlewares
BUT it is possible to do that inside your controller methods
so this solution might help a bit to someone

@c00p3r

This comment has been minimized.

Copy link

commented Dec 1, 2017

@doghap making separate models (providers) for each guard is another solution too

@AlexVanderbist

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

It is impossible to check role with specific guard in middlewares
BUT it is possible to do that inside your controller methods
so this solution might help a bit to someone

You could also write a custom middleware that does the same thing as the HasRole middleware but with your custom guard hardcoded in there.

Can this issue be closed?

@c00p3r

This comment has been minimized.

Copy link

commented Dec 6, 2017

I think this should be closed
Especially after reading #479

@freekmurze freekmurze closed this Dec 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.