spatie/laravel-permission

Cached permissions on multiple instances running on Octane / Kubernetes

Cluster2a opened this issue · 12 comments

Describe the bug
Running on Octane and multiple Kubernetes pods, the permissions seem to be cached on each pod. So basically a simple can check for viewAny will result in a 403 or a result (depending on which pod the request hits).

We are using wildcard permissions.

Versions

  • spatie/laravel-permission package version: 6.2.0
  • laravel/framework: v10.35.0

PHP version: 8.2

Database version: mysql:8.2.0

To Reproduce

  • run multiple instances on octane
  • assign a role to the user with a wildcard permission (e.g.: suites.viewAny)
  • call an API endpoint that checks for this endpoint
  • remove the permission from the role
  • call the endpoint multiple times: some requests result in a 403 / some in showing the resource list

Here is my example code and/or tests showing the problem in my app:

Controller:

$user = auth()->user();
Gate::forUser($user)->authorize('viewAny', Booking::class);

Policy:

public function viewAny(User $user): bool
{
    return $user->can(Resources::BOOKINGS . '.' . PermissionsActions::VIEW_ANY);
}

Even though register_octane_reset_listener set to true the permissions seem to be cached somehow. After checking the code, I saw that you call $event->sandbox->make(PermissionRegistrar::class)->clearPermissionsCollection(); if the flag is set to true.

I made my own Event Listener for the OperationTerminated-Event which looks like this:

class OctaneReloadPermissions
{
    /**
     * @param $event
     *
     * @return void
     */
    public function handle($event): void
    {
        $event->sandbox->make(PermissionRegistrar::class)->setPermissionsTeamId(null);
        $event->sandbox->make(PermissionRegistrar::class)->forgetCachedPermissions();
    }
}

I noticed that $event->sandbox->make(PermissionRegistrar::class)->forgetCachedPermissions(); also seems to clear the wildcard permissions, which we are using. Calling forgetCachedPermissions() seems to do the trick, as I am not able to reproduce the behaviour anymore.

If enable_wildcard_permission is set to true shouldn't the forgetWildcardPermissionIndex() also be called within the registerOctaneListener()?

I think the comment in the permissions.php NOTE: This should not be needed in most cases, but an Octane/Vapor combination benefited from it. might be missleading. I think every multi instance setup running on octane might be effected.

Expected behavior
The permission cache should be cleared after each request including wildcard permissions.

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

  • Image: php:8.2.13-cli-alpine3.18

I've not yet worked with Octane for a production app, so my "testing" of things like this has been limited. But would like to dig deeper.

Do you mind providing a demo app, replicating the features and symptoms you describe, which we can use for exploring and debugging further?

@drbyte, as the current code base relies on many services (realtime events, after role switch), it would be easier to create a simple example with a single controller and a few endpoints to change a role.

I think the best way to test & debug would be an example which runs on a local micrk8s instance. But I would need some time for the example.

$event->sandbox->make(PermissionRegistrar::class)->forgetCachedPermissions();

that removes the cache in each request and create a new one again, if you are going to do that it is better to use array in the cache store configuration, this way you will use less resources

'store' => 'default',

What do you use for the cache? redis?

What do you use for the cache? redis?

I am using redis. I tried a different approach, since I don't want to clear the entire cache:
image

This also works.

@drbyte, fyi.

Make a PR

$dispatcher->listen(function (\Laravel\Octane\Events\OperationTerminated $event) {
// @phpstan-ignore-next-line
$event->sandbox->make(PermissionRegistrar::class)->clearPermissionsCollection();
});

I'm not sure it's safe to hard-code any resets to instance of User because we can't guarantee that.
Perhaps instance of Authenticatable contract.

But is the call to $user->resetPermissionDate() critical?

I agree with the call to setPermissionsTeamId(null) and forgetWildcardPermissionIndex().

@drbyte, we are using some custom permission cache on the user - this part can be ignored. Most important seems to be, that the wildcard cache is beeing deleted.

Okay. I think what we'll do is, in this package add the reset for team ID and wildcard permissions.

And then for the parts you need in your own app, you can register a listener on Octane's OperationTerminated for those things unique to your own requirements.

Hmmm ... I forgot: we already have the reset for team ID since 6.1.0

So ... @erikn69 @parallels999 I'm curious your opinion on whether we should add an Octane-specific reset for forgetWildcardPermissionIndex(), or simply combine forgetWildcardPermissionIndex() into clearPermissionsCollection()

/**
* Clear already-loaded permissions collection.
* This is only intended to be called by the PermissionServiceProvider on boot,
* so that long-running instances like Octane or Swoole don't keep old data in memory.
*/
public function clearPermissionsCollection(): void
{
$this->permissions = null;
}

Noting:

/**
* Flush the cache.
*/
public function forgetCachedPermissions()
{
$this->permissions = null;
$this->forgetWildcardPermissionIndex();
return $this->cache->forget($this->cacheKey);
}
public function forgetWildcardPermissionIndex(?Model $record = null): void
{
if ($record) {
unset($this->wildcardPermissionsIndex[get_class($record)][$record->getKey()]);
return;
}
$this->wildcardPermissionsIndex = [];
}

public function clearPermissionsCollection(): void 
{ 
   $this->permissions = null; 
   $this->wildcardPermissionsIndex = []; 
}