Fix initializeCache() not clearing in-memory perms collection on tenant/cache switch - #2965
Merged
Merged
Conversation
drbyte
force-pushed
the
tenant-initialize-cache-issue-2964
branch
from
July 3, 2026 14:43
98d347b to
735bc43
Compare
…nt/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.
drbyte
force-pushed
the
tenant-initialize-cache-issue-2964
branch
from
July 3, 2026 14:43
735bc43 to
e80d7b0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2964
Problem:
initializeCache()re-resolves the cache store and config but leaves$this->permissionsuntouched. SincePermissionRegistraris a singleton andloadPermissions()short-circuits when that collection is already populated: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. BecausehasDirectPermission()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 (viaclearPermissionsCollection()) and the transient$cachedRolesbuffer, so the next permission check rebuilds from the newly-configured cache/tenant instead of reusing stale data.Changes:
src/PermissionRegistrar.php—initializeCache()clears in-memory permissions/roles state.tests/Integration/PermissionRegistrarTest.php— regression test asserting the loaded collection isnullafterinitializeCache().docs/advanced-usage/cache.md— clarifies thatinitializeCache()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$permissionsis alreadynull, 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.