spatie/laravel-permission

withoutPermission scope from HasPermissions returns users from outside the team

countless-integers opened this issue · 2 comments

Describe the bug
Applying withoutPermission scope from HasPermissions trait returns users from outside the team.

Versions
You can use composer show to get the version numbers of:

  • spatie/laravel-permission package version: 6.4
  • laravel/framework package version: 10.48.2

PHP version: 8.2

Database version: MySQL 8.0

To Reproduce
Steps to reproduce the behavior:

Permission::create(['name' => 'comment.create']);
setPermissionsTeamId(1);
$user1->givePermissionTo('comment.create');
setPermissionsTeamId(2);
$user2->givePermissionTo('comment.create');
setPermissionsTeamId(1);
$users = User::withoutPermission('comment.delete')->get();
// return $user1 and $user2

Expected behavior
If teams feature is enabled I'd expect to only see member of the team in the result.

Additional context
I think this is the result of inverse whereHas constraint. Team id is in the query SQL, but it's in the subquery, so NOT EXISTS(__subquery__ ... team_id = 1) which "matches" all users. I guess it's how eloquent does it, but it's counter-intuitive with teams feature enabled.

Environment (please complete the following information, because it helps us investigate better):

  • OS: macos
  • Version Sonoma

You need to have your own relationship between user and team
This package does not provide you with that part, you must implement it

$users = User::withoutPermission('comment.delete')
    ->whereHas('YOUR_TEAM_RELATION', fn($q) => $q->where('id',  getPermissionsTeamId()))->get();

You need to have your own relationship between user and team This package does not provide you with that part, you must implement it

Fair enough, just that Users::permission() (and hence withoutPermission) includes the team id in the query, it's just that the way the query is constructed make the former look like it works in team scope and the latter doesn't. Anyway, thanks for the suggestion, I can overwrite the trait scope to tack on that extra whereHas constraint to make it work for my app.