Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Check the guard of logged in user #1313

Closed
Daniyal-Javani opened this issue Dec 31, 2019 · 8 comments
Closed

Check the guard of logged in user #1313

Daniyal-Javani opened this issue Dec 31, 2019 · 8 comments

Comments

@Daniyal-Javani
Copy link

v2 guard lookup currently looks to the model's guard_name property, with fallbacks down to finally the first available guard in your config array of guards (not the one listed as "default"). Null doesn't disable this lookup.
So, wherever you don't specify a guard when doing a permission check, it does the lookup according to that pattern I just described.

Shouldn't it check the guard of logged in user?
like this

function activeGuard()
{
    foreach (array_keys(config('auth.guards')) as $guard) {

        if (auth()->guard($guard)->check()) return $guard;
    }
    return null;
}

How can I check based on the guard of user?

Originally posted by @Daniyal-Javani in #892 (comment)

@Daniyal-Javani Daniyal-Javani changed the title > v2 guard lookup currently looks to the model's guard_name property, with fallbacks down to finally the first available guard in your config array of guards (not the one listed as "default"). Null doesn't disable this lookup. Check the guard of logged in user Dec 31, 2019
@drbyte drbyte added the support label Jan 7, 2020
@drbyte
Copy link
Collaborator

drbyte commented Feb 19, 2020

@Daniyal-Javani I'd like to see a future version of this package honor the guard that the user has been assigned during their login/authentication.

A PR which updates that functionality would be welcome. Ideally with tests.

@musapinar
Copy link
Contributor

musapinar commented Feb 22, 2020

@drbyte
Copy link
Collaborator

drbyte commented Feb 22, 2020

@musapinar Yes, but the next line overrides it with the model's guard, which leaves many people confused about why the package checks against a different guard than the one the user is using (they would say "it checks against the wrong guard").
(See several issues about guards: most are complaining of this factor.)

It's a complex change to make, because it's messing with a matrix of values created by conflating authorization with authentication. I'd rather remove all mention of guards altogether, and let the application implement guard-related rules.
But there's a large user base depending on the guard features of this package, even though they wish it worked in a way that honoured the user's logged-in-guard with more prominence and more consistency.

@drbyte
Copy link
Collaborator

drbyte commented Feb 22, 2020

@musapinar I think the first starting point though is reviewing where it's not working as desired, writing the tests for that, and then getting those tests to pass.

@Daniyal-Javani
Copy link
Author

Daniyal-Javani commented Feb 23, 2020

OK, what about this test
Daniyal-Javani@54dde60
The test will fail (server error) because there is no permission named do_that with web gaurd while its guard is api and the user had logged in with api gaurd.

@musapinar
Copy link
Contributor

@Daniyal-Javani @drbyte
Test looks good to me.

I'm afraid I can't/won't help much on this one as I do not make use of that guard feature at all. I enforce web no matter what since I do not want to duplicate roles/permissions for each guard. (Here is my glorious fix btw : https://i.imgur.com/OO3xe3w.png)

Not sure how elegant the following is, but it passes all tests green :

            ->keys()
            ->filter(function ($guard) {
                return request()->user() ? auth()->guard($guard)->check() : true;
            });

https://github.com/spatie/laravel-permission/blob/master/src/Guard.php#L42
https://i.imgur.com/6dlJmza.png

Good luck.

@Pixy123
Copy link

Pixy123 commented Mar 30, 2020

Hi, when you release this patch? Thank's

@jonasbrenig
Copy link

jonasbrenig commented Apr 22, 2020

If you are looking for a solution that works without this feature:

You can set up a second user Model, such as App\User and App\ApiUser.
Then make sure to use the ApiUser in your auth.php for your api guard.

class ApiUser extends Authenticatable {
	use HasRoles;

	/**
	 * The table associated with the model.
	 *
	 * @var string
	 */
        protected $table = 'users';
    
         /**
	 * The guard that this user should be authenticated with.
	 */
	protected $guard_name = 'api';

}
class User extends Authenticatable {
	use HasRoles;
}
	'guards' => [
		'web' => [
			'driver'   => 'session',
			'provider' => 'users',
		],

		'api' => [
			'driver'   => 'token',
			'provider' => 'apiusers',
			'hash'     => false,
		],
	],
	'providers' => [
		'users' => [
			'driver' => 'eloquent',
			'model'  => App\Models\User::class,
		],

		'apiusers' => [
			'driver' => 'eloquent',
			'model'  => App\Models\ApiUser::class,
		],
	],

I was able to get it working like this, maybe someone will find this helpful.

@spatie spatie locked and limited conversation to collaborators Jul 8, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants