Wildcard Permissions are not updated once assigned a role

[manstie]

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]

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]

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():

[drbyte]

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]

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

[spatie-bot]

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.

[manstie]

I do not have permission to reopen.

[drbyte]

Factors:

• wildcard permission mode enabled
• role does not have the actual permission assigned to it (it's presumed via a Gate rule), so hasPermissionTo() would be false if wildcard wasn't enabled
• checking permissions before assigning roles, and then rechecking permissions

Suspected cause

Because it's a wildcard implementation, I suspect that maybe the wildcard cache (which is used for performance reasons) might be preloaded with stale data.

Probable Workaround

Try this in your UserTest (taken from your example project):

    #[Test]
    public function it_can_be_assigned_a_role(): void
    {
        /** @var Company */
        $company = Company::factory()->create();
        setPermissionsTeamId($company->id);
        /** @var User */
        $user = User::factory()->create();
        $this->actingAs($user, 'api');

        $role = Role::getDefaultCompanyRole(Role::COMPANY_SUPER_ADMIN);

        // https://github.com/spatie/laravel-permission/issues/2725
-        // $this->assertFalse($user->can('company.delete'));
+        $this->assertFalse($user->can('company.delete'), 'company.delete should be false here');
        
+        // reset wildcard index because we just loaded $user permissions before assigning roles
+        app(\Spatie\Permission\PermissionRegistrar::class)->forgetWildcardPermissionIndex($user);

        $this->assertFalse($user->hasRole($role));
        $user->assignRole($role);
        $this->assertTrue($user->hasRole($role));

        $this->assertTrue($role->hasPermissionTo('company.delete'), 'role::hasPermissionTo company.delete');
        $this->assertTrue($user->can('company.delete'), 'company.delete should be true here');
    }

Fix

If indeed that workaround proves to be thorough, that begs a review of the wildcard indexing cache handling.
We may need to improve how we clear that index when assigning/removing roles, akin to how we do it with permissions.

[manstie]

The wildcard permission index needs to be cleared regardless of whether the permission uses the wildcard feature:

    #[Test]
    public function it_can_be_assigned_a_company_role(): void
    {
        /** @var Company */
        $company = Company::factory()->create();
        setPermissionsTeamId($company->id);
        /** @var User */
        $user = User::factory()->create();
        $this->actingAs($user, 'api');

        /** @var Role */
        $role = Role::create([
            'company_id' => $company->id,
            'name' => 'Test',
            'guard_name' => 'api',
        ]);

        $permission = 'test'; // no wildcard
        Permission::create([
            'name' => $permission,
            'guard_name' => 'api'
        ]);
        $role->givePermissionTo($permission);

        $this->assertTrue($role->hasPermissionTo($permission));

        $this->assertFalse($user->can($permission)); // causes below to fail
        $this->assertFalse($user->hasRole($role));
        $user->assignRole($role);
        // https://github.com/spatie/laravel-permission/issues/2725
        // app(\Spatie\Permission\PermissionRegistrar::class)->forgetWildcardPermissionIndex($user);

        $this->assertTrue($user->hasRole($role));
        $this->assertTrue($user->can($permission)); // fails
    }

[drbyte]

By wildcard I meant you're using the non-default mode that causes wildcard logic to be checked, even if no * is used:

laravel-permission/config/permission.php

Lines 164 to 169 in 0773eb0

	/*
	* By default wildcard permission lookups are disabled.
	* See documentation to understand supported syntax.
	*/
	'enable_wildcard_permission' => false,

[drbyte]

I'm pretty sure this is isolated to assignRole() and removeRole().

[drbyte]

Current theory / hopeful solution

In HasRoles.php, in assignRole() and removeRole() make this addition in both functions:

        if ($this instanceof Permission) {
            $this->forgetCachedPermissions();
        }

+        if (config('permission.enable_wildcard_permission')) {
+            app(PermissionRegistrar::class)->forgetWildcardPermissionIndex($this);
+        }

Would like to also add a test to the test suite that proves this isn't missed in any future changes.