Skip to content

Commit e80d7b0

Browse files
committed
Fix initializeCache() not clearing in-memory perms collection on tenant/cache switch
Fixes #2964 **Problem:** `initializeCache()` re-resolves the cache store and config but leaves `$this->permissions` untouched. Since `PermissionRegistrar` is a singleton and `loadPermissions()` short-circuits when that collection is already populated: ```php if ($this->permissions) { return; } ``` any process that switches cache context mid-lifetime (multi-tenant apps switching tenants, queue workers, artisan commands looping over tenants, Octane workers) keeps serving the *previous* tenant's permissions after calling `initializeCache()` — **exactly the call our own docs recommend for this scenario**. Because `hasDirectPermission()` matches by primary key, and each tenant DB has independent auto-increment IDs, a leftover collection doesn't just serve stale data — it can resolve permission names to the wrong tenant's IDs and return incorrect authorization results. **Fix:** `initializeCache()` now also clears the in-memory permissions collection (via `clearPermissionsCollection()`) and the transient `$cachedRoles` buffer, so the next permission check rebuilds from the newly-configured cache/tenant instead of reusing stale data. **Changes:** - [`src/PermissionRegistrar.php`](src/PermissionRegistrar.php) — `initializeCache()` clears in-memory permissions/roles state. - [`tests/Integration/PermissionRegistrarTest.php`](tests/Integration/PermissionRegistrarTest.php) — regression test asserting the loaded collection is `null` after `initializeCache()`. - [`docs/advanced-usage/cache.md`](docs/advanced-usage/cache.md) — clarifies that `initializeCache()` now also discards the loaded collection. **Impact:** No behavior change for typical single-tenant apps. `initializeCache()` is otherwise only called once at registrar construction, where `$permissions` is already `null`, so this adds no overhead there. It only changes behavior for the explicit-reinitialize-mid-request pattern our docs already describe, making that pattern actually work as documented.
1 parent 9d4eb1e commit e80d7b0

3 files changed

Lines changed: 64 additions & 0 deletions

File tree

docs/advanced-usage/cache.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ Tip: Most parts of your multitenancy app will relate to a single tenant during a
8080
app()->make(\Spatie\Permission\PermissionRegistrar::class)->initializeCache();
8181
```
8282

83+
`initializeCache()` also discards the registrar's in-memory permissions/roles collection (equivalent to `clearPermissionsCollection()`), so the next permission check reloads from the newly-configured cache/tenant instead of reusing the previous tenant's already-loaded collection.
84+
8385

8486
### Custom Cache Store
8587

src/PermissionRegistrar.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ public function initializeCache(): void
7676
$this->pivotPermission = config('permission.column_names.permission_pivot_key') ?: 'permission_id';
7777

7878
$this->cache = $this->getCacheStoreFromConfig();
79+
80+
// Discard any in-memory permissions/roles loaded under the previous cache config,
81+
// so a subsequent call rebuilds them from the new cache/tenant.
82+
$this->clearPermissionsCollection();
83+
$this->cachedRoles = [];
7984
}
8085

8186
protected function getCacheStoreFromConfig(): Repository

tests/Integration/PermissionRegistrarTest.php

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?php
22

3+
use Illuminate\Support\Facades\DB;
34
use Spatie\Permission\Contracts\Permission as PermissionContract;
45
use Spatie\Permission\Contracts\Role as RoleContract;
56
use Spatie\Permission\Models\Permission as SpatiePermission;
@@ -22,6 +23,62 @@
2223
expect($reflectedProperty->getValue(app(PermissionRegistrar::class)))->toBeNull();
2324
});
2425

26+
it('clears the loaded permissions collection when reinitializing the cache', function () {
27+
$reflectedClass = new ReflectionClass(app(PermissionRegistrar::class));
28+
$reflectedProperty = $reflectedClass->getProperty('permissions');
29+
$reflectedProperty->setAccessible(true);
30+
31+
app(PermissionRegistrar::class)->getPermissions();
32+
33+
expect($reflectedProperty->getValue(app(PermissionRegistrar::class)))->not->toBeNull();
34+
35+
app(PermissionRegistrar::class)->initializeCache();
36+
37+
expect($reflectedProperty->getValue(app(PermissionRegistrar::class)))->toBeNull();
38+
});
39+
40+
it('does not leak a previous tenant\'s permissions after switching cache context via initializeCache', function () {
41+
// Two separate cache "stores" stand in for two tenants' cache namespaces
42+
// (e.g. distinct cache prefixes/connections in a real multi-tenant app).
43+
config([
44+
'cache.stores.tenant_a' => ['driver' => 'array'],
45+
'cache.stores.tenant_b' => ['driver' => 'array'],
46+
]);
47+
48+
// Insert both tenants' rows via the query builder, bypassing Eloquent,
49+
// so the RefreshesPermissionCache model events don't auto-bust the cache and
50+
// mask the very staleness this test is meant to catch.
51+
$tenantAId = DB::table('permissions')->insertGetId([
52+
'name' => 'tenant-permission',
53+
'guard_name' => 'web',
54+
'created_at' => now(),
55+
'updated_at' => now(),
56+
]);
57+
58+
config(['permission.cache.store' => 'tenant_a']);
59+
app(PermissionRegistrar::class)->initializeCache();
60+
61+
$loaded = app(PermissionRegistrar::class)->getPermissions()->firstWhere('name', 'tenant-permission');
62+
expect($loaded->getKey())->toBe($tenantAId);
63+
64+
// Simulate switching to tenant B: its own row for the "same" permission has
65+
// a different primary key, as it would in a separate tenant database.
66+
DB::table('permissions')->where('id', $tenantAId)->delete();
67+
$tenantBId = DB::table('permissions')->insertGetId([
68+
'name' => 'tenant-permission',
69+
'guard_name' => 'web',
70+
'created_at' => now(),
71+
'updated_at' => now(),
72+
]);
73+
expect($tenantBId)->not->toBe($tenantAId);
74+
75+
config(['permission.cache.store' => 'tenant_b']);
76+
app(PermissionRegistrar::class)->initializeCache();
77+
78+
$loaded = app(PermissionRegistrar::class)->getPermissions()->firstWhere('name', 'tenant-permission');
79+
expect($loaded->getKey())->toBe($tenantBId);
80+
});
81+
2582
it('can check uids', function () {
2683
$uids = [
2784
// UUIDs

0 commit comments

Comments
 (0)