spatie / laravel-permission

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

Permissions are not updated once assigned a role #2725

Open manstie opened 3 weeks ago

manstie commented 3 weeks ago

Description

I was writing tests and came across this quirk.

Assuming you've given permission for a role to do a thing, this test still fails:

        $role = Role::find(1);
        $this->assertFalse($user->hasPermissionTo('do a thing'));
        $this->assertFalse($user->hasRole(1));
        $user->assignRole($role);
        $this->assertTrue($user->hasRole(1));

        $this->assertTrue($role->hasPermissionTo('do a thing'));
        $this->assertTrue($user->hasPermissionTo('do a thing')); // fails here, same thing with `->can('do a thing');`

But it works if you don't check permissions before assigning the role:

        $role = Role::find(1);
        // Works now that this is commented out
        // $this->assertFalse($user->hasPermissionTo('do a thing'));
        $this->assertFalse($user->hasRole(1));
        $user->assignRole($role);
        $this->assertTrue($user->hasRole(1));

        $this->assertTrue($role->hasPermissionTo('do a thing'));
        $this->assertTrue($user->hasPermissionTo('do a thing')); // works fine

I assume this is some caching issue.

Steps To Reproduce

In the same function:

  1. Check a user for permissions
  2. Assign a role
  3. Check user has permission in that role
  4. Fail

Example Application

https://github.com/manstie/laravel-permissions-example

Version of spatie/laravel-permission package:

6.9

Version of laravel/framework package:

11.9

PHP version:

8.2

Database engine and version:

No response

OS: Windows/Mac/Linux version:

No response

drbyte commented 3 weeks ago

I would expect that calling ->assignRole() would clear any associated cached relations (It does when I use Tinker).

Checking for a permission will load the permissions relation on the model, but then we destroy and reload that when roles/permissions are changed for that model. That said, Laravel lets you force a refresh: After you assign the role, call $user = $user->fresh();

Quick test in Tinker using some data similar to what's in the docs:

$user = User::first();
$role = Role::find(1); // 'Writer', with one permission: 'edit articles'
$before = $user->hasPermissionTo('edit articles');
$user->assignRole($role);
$afterRole =$user->hasRole(1);
$after = $user->hasPermissionTo('edit articles');

dd(compact('before', 'after'));
array:2 [
  "before" => false
  "after" => true
]
manstie commented 3 weeks ago

It may be worth adding that I am using the "teams" feature, and I get the same results in tinker, even with $user = $user->fresh(): image

drbyte commented 3 weeks ago

This is isolated to your tests? If so, can you update the Issue Title to include that factor.

This package's test suite does extensive testing of adding and checking roles and permissions to ensure there are no caching issues, etc.

If you can create a simple fresh app (the Docs have a section on creating a new app and pushing it to a public github repo) that recreates this specific problem, we can determine whether this is a package bug or a bug in your application, and figure out the fix.

manstie commented 2 weeks ago

I have created an app replicating my environment and settings here: https://github.com/manstie/laravel-permissions-example

Something to note: I seed the users with roles, but when you query them via the user model they are not found, despite existing in the database. I suppose that's my issue or it's related. I wasn't setting the team id in my tinker session