From c4960b76cb9978aa3601d61ab5ec491d2766b1ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Wed, 18 Mar 2026 19:17:28 +0100 Subject: [PATCH 01/15] [4.x] Laravel 13 support (#1443) - Update ci.yml and composer.json - Wrap single database tenancy trait scopes in whenBooted() - Update SessionSeparationTest to use laravel-cache- prefix in L13 and laravel_cache_ in <=L12. Our own prefix remains tenant_%tenant%_ (as configured in tenancy.cache.prefix). We could update this to be tenant-%tenant%- from now on for consistency with Laravel's prefixes (changed in https://github.com/laravel/framework/pull/56172) but I'm not sure yet. _ seems to read a bit better but perhaps consistency is more important. We may change this later and it can be adjusted in userland easily (since it's just a config option). --- .github/workflows/ci.yml | 1 + composer.json | 10 ++--- .../Concerns/BelongsToPrimaryModel.php | 11 ++++++ src/Database/Concerns/BelongsToTenant.php | 11 ++++++ tests/SessionSeparationTest.php | 37 ++++++++++++------- 5 files changed, 52 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca7c20f6..48b1ffd2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,7 @@ jobs: matrix: include: - laravel: "^12.0" + - laravel: "^13.0" steps: - name: Checkout diff --git a/composer.json b/composer.json index b03e1b2f..4f843504 100644 --- a/composer.json +++ b/composer.json @@ -18,17 +18,17 @@ "require": { "php": "^8.4", "ext-json": "*", - "illuminate/support": "^12.0", - "laravel/tinker": "^2.0", + "illuminate/support": "^12.0|^13.0", + "laravel/tinker": "^2.0|^3.0", "ramsey/uuid": "^4.7.3", - "stancl/jobpipeline": "2.0.0-rc6", + "stancl/jobpipeline": "2.0.0-rc7", "stancl/virtualcolumn": "^1.5.0", "spatie/invade": "*", "laravel/prompts": "0.*" }, "require-dev": { - "laravel/framework": "^12.0", - "orchestra/testbench": "^10.0", + "laravel/framework": "^13.0", + "orchestra/testbench": "^10.0|^11.0", "league/flysystem-aws-s3-v3": "^3.12.2", "doctrine/dbal": "^3.6.0", "spatie/valuestore": "^1.2.5", diff --git a/src/Database/Concerns/BelongsToPrimaryModel.php b/src/Database/Concerns/BelongsToPrimaryModel.php index 2c8c435f..ca3ba66f 100644 --- a/src/Database/Concerns/BelongsToPrimaryModel.php +++ b/src/Database/Concerns/BelongsToPrimaryModel.php @@ -12,6 +12,17 @@ trait BelongsToPrimaryModel abstract public function getRelationshipToPrimaryModel(): string; public static function bootBelongsToPrimaryModel(): void + { + if (method_exists(static::class, 'whenBooted')) { + // Laravel 13 + // For context see https://github.com/calebporzio/sushi/commit/62ff7f432cac736cb1da9f46d8f471cb78914b92 + static::whenBooted(fn () => static::configureBelongsToPrimaryModelScope()); + } else { + static::configureBelongsToPrimaryModelScope(); + } + } + + protected static function configureBelongsToPrimaryModelScope() { $implicitRLS = config('tenancy.rls.manager') === TraitRLSManager::class && TraitRLSManager::$implicitRLS; diff --git a/src/Database/Concerns/BelongsToTenant.php b/src/Database/Concerns/BelongsToTenant.php index da5dc84a..5c0f50fb 100644 --- a/src/Database/Concerns/BelongsToTenant.php +++ b/src/Database/Concerns/BelongsToTenant.php @@ -26,6 +26,17 @@ trait BelongsToTenant } public static function bootBelongsToTenant(): void + { + if (method_exists(static::class, 'whenBooted')) { + // Laravel 13 + // For context see https://github.com/calebporzio/sushi/commit/62ff7f432cac736cb1da9f46d8f471cb78914b92 + static::whenBooted(fn () => static::configureBelongsToTenantScope()); + } else { + static::configureBelongsToTenantScope(); + } + } + + protected static function configureBelongsToTenantScope(): void { // If TraitRLSManager::$implicitRLS is true or this model implements RLSModel // Postgres RLS is used for scoping, so we don't enable the scope used with single-database tenancy. diff --git a/tests/SessionSeparationTest.php b/tests/SessionSeparationTest.php index d699bc61..6c7a8aa1 100644 --- a/tests/SessionSeparationTest.php +++ b/tests/SessionSeparationTest.php @@ -23,6 +23,7 @@ use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Middleware\InitializeTenancyByPath; use Stancl\Tenancy\Middleware\PreventAccessFromUnwantedDomains; use Stancl\Tenancy\Tests\Etc\Tenant; + use function Stancl\Tenancy\Tests\pest; // todo@tests write similar low-level tests for the cache bootstrapper? including the database driver in a single-db setup @@ -100,7 +101,7 @@ test('redis sessions are separated using the redis bootstrapper', function (bool expect($redisClient->getOption($redisClient::OPT_PREFIX) === "tenant_{$tenant->id}_")->toBe($bootstrappedEnabled); expect(array_filter(Redis::keys('*'), function (string $key) use ($tenant) { - return str($key)->startsWith("tenant_{$tenant->id}_laravel_cache_"); + return str($key)->startsWith(formatLaravelCacheKey(prefix: "tenant_{$tenant->id}_")); }))->toHaveCount($bootstrappedEnabled ? 1 : 0); })->with([true, false]); @@ -118,13 +119,13 @@ test('redis sessions are separated using the cache bootstrapper', function (bool Route::middleware(StartSession::class, InitializeTenancyByPath::class)->get('/{tenant}/foo', fn () => 'bar'); pest()->get("/{$tenant->id}/foo"); - expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix() === "laravel_cache_tenant_{$tenant->id}_")->toBe($scopeSessions); + expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix() === formatLaravelCacheKey("tenant_{$tenant->id}_"))->toBe($scopeSessions); tenancy()->end(); - expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix())->toBe('laravel_cache_'); + expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix())->toBe(formatLaravelCacheKey()); expect(array_filter(Redis::keys('*'), function (string $key) use ($tenant) { - return str($key)->startsWith("foolaravel_cache_tenant_{$tenant->id}"); + return str($key)->startsWith(formatLaravelCacheKey(prefix: 'foo', suffix: "tenant_{$tenant->id}")); }))->toHaveCount($scopeSessions ? 1 : 0); })->with([true, false]); @@ -148,14 +149,14 @@ test('memcached sessions are separated using the cache bootstrapper', function ( Route::middleware(StartSession::class, InitializeTenancyByPath::class)->get('/{tenant}/foo', fn () => 'bar'); pest()->get("/{$tenant->id}/foo"); - expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix() === "laravel_cache_tenant_{$tenant->id}_")->toBe($scopeSessions); + expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix() === formatLaravelCacheKey("tenant_{$tenant->id}_"))->toBe($scopeSessions); tenancy()->end(); - expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix())->toBe('laravel_cache_'); + expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix())->toBe(formatLaravelCacheKey()); sleep(1.1); // 1s+ sleep is necessary for getAllKeys() to work. if this causes race conditions or we want to avoid the delay, we can refactor this to some type of a mock expect(array_filter($allMemcachedKeys(), function (string $key) use ($tenant) { - return str($key)->startsWith("laravel_cache_tenant_{$tenant->id}"); + return str($key)->startsWith(formatLaravelCacheKey("tenant_{$tenant->id}")); }))->toHaveCount($scopeSessions ? 1 : 0); Artisan::call('cache:clear memcached'); @@ -177,13 +178,13 @@ test('dynamodb sessions are separated using the cache bootstrapper', function (b Route::middleware(StartSession::class, InitializeTenancyByPath::class)->get('/{tenant}/foo', fn () => 'bar'); pest()->get("/{$tenant->id}/foo"); - expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix() === "laravel_cache_tenant_{$tenant->id}_")->toBe($scopeSessions); + expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix() === formatLaravelCacheKey("tenant_{$tenant->id}_"))->toBe($scopeSessions); tenancy()->end(); - expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix())->toBe('laravel_cache_'); + expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix())->toBe(formatLaravelCacheKey()); expect(array_filter($allDynamodbKeys(), function (string $key) use ($tenant) { - return str($key)->startsWith("laravel_cache_tenant_{$tenant->id}"); + return str($key)->startsWith(formatLaravelCacheKey("tenant_{$tenant->id}")); }))->toHaveCount($scopeSessions ? 1 : 0); })->with([true, false]); @@ -202,13 +203,13 @@ test('apc sessions are separated using the cache bootstrapper', function (bool $ Route::middleware(StartSession::class, InitializeTenancyByPath::class)->get('/{tenant}/foo', fn () => 'bar'); pest()->get("/{$tenant->id}/foo"); - expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix() === "laravel_cache_tenant_{$tenant->id}_")->toBe($scopeSessions); + expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix() === formatLaravelCacheKey("tenant_{$tenant->id}_"))->toBe($scopeSessions); tenancy()->end(); - expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix())->toBe('laravel_cache_'); + expect(app('session')->driver()->getHandler()->getCache()->getStore()->getPrefix())->toBe(formatLaravelCacheKey()); expect(array_filter($allApcuKeys(), function (string $key) use ($tenant) { - return str($key)->startsWith("laravel_cache_tenant_{$tenant->id}"); + return str($key)->startsWith(formatLaravelCacheKey("tenant_{$tenant->id}")); }))->toHaveCount($scopeSessions ? 1 : 0); })->with([true, false]); @@ -250,3 +251,13 @@ test('database sessions are separated regardless of whether the session bootstra // [false, true], // when the connection IS set, the session bootstrapper becomes necessary [false, false], ]); + +function formatLaravelCacheKey(string $suffix = '', string $prefix = ''): string +{ + // todo@release if we drop Laravel 12 support we can just switch to - syntax everywhere + if (version_compare(app()->version(), '13.0.0') >= 0) { + return $prefix . 'laravel-cache-' . $suffix; + } else { + return $prefix . 'laravel_cache_' . $suffix; + } +} From fb654e7a6bb3a6163ffd2362d7f9f1b770e559b7 Mon Sep 17 00:00:00 2001 From: Samuel Mwangi Date: Mon, 30 Mar 2026 10:44:53 +0300 Subject: [PATCH 02/15] [4.x] Update Pest to v4 (#1430) --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 4f843504..180cbaab 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,7 @@ "league/flysystem-aws-s3-v3": "^3.12.2", "doctrine/dbal": "^3.6.0", "spatie/valuestore": "^1.2.5", - "pestphp/pest": "^3.0", + "pestphp/pest": "^4.0", "larastan/larastan": "^3.0", "league/flysystem-path-prefixing": "^3.0" }, From 60dd5226c44adeb3e005f137f41bd67adf2a7d8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Wed, 8 Apr 2026 19:21:43 +0200 Subject: [PATCH 03/15] [4.x] Add Tenancy::reinitialize() method (#1449) Some bootstrappers read attributes of the tenant during bootstrap() but don't respond to changes made to the tenant afterwards. Therefore, when making changes to the tenant that'd affect the behavior of a bootstrapper, it's necessary to reinitialize tenancy (if it matters that changes are reflected immediately). This adds a convenience helper for that purpose. --- src/Tenancy.php | 20 +++++++++++++++++++ tests/AutomaticModeTest.php | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/Tenancy.php b/src/Tenancy.php index f9c9c9ae..a2271bed 100644 --- a/src/Tenancy.php +++ b/src/Tenancy.php @@ -152,6 +152,26 @@ class Tenancy $this->initialized = false; } + /** + * End tenancy and initialize it again for the current tenant. + * + * This can be helpful when changing "dependencies" of bootstrappers such as + * attributes of the current tenant that are only read once, during bootstrap(). + * + * If tenancy is not initialized, this method is a no-op. + */ + public function reinitialize(): void + { + if ($this->tenant === null) { + return; + } + + $tenant = $this->tenant; + $this->end(); + + $this->initialize($tenant); + } + /** @return TenancyBootstrapper[] */ public function getBootstrappers(): array { diff --git a/tests/AutomaticModeTest.php b/tests/AutomaticModeTest.php index fbeb06fc..599d14d9 100644 --- a/tests/AutomaticModeTest.php +++ b/tests/AutomaticModeTest.php @@ -103,6 +103,33 @@ test('central helper doesnt change tenancy state when called in central context' expect(tenant())->toBeNull(); }); +test('reinitialize method does nothing in the central context', function () { + expect(tenancy()->initialized)->toBe(false); + expect(fn () => tenancy()->reinitialize())->not()->toThrow(\Throwable::class); + expect(tenancy()->initialized)->toBe(false); +}); + +test('reinitialize method runs bootstrappers again for the current tenant', function () { + config(['tenancy.bootstrappers' => [ + ReinitBootstrapper::class, + ]]); + + tenancy()->initialize($tenant = Tenant::create(['reinit_bootstrapper_key' => 'foo'])); + + expect(tenant()->getKey())->toBe($tenant->getKey()); + expect(app('tenancy_reinit_bootstrapper_key'))->toBe('foo'); + + $tenant->update(['reinit_bootstrapper_key' => 'bar']); + + // Unchanged until we reinitialize... + expect(app('tenancy_reinit_bootstrapper_key'))->toBe('foo'); + + tenancy()->reinitialize(); + + expect(tenant()->getKey())->toBe($tenant->getKey()); + expect(app('tenancy_reinit_bootstrapper_key'))->toBe('bar'); +}); + class MyBootstrapper implements TenancyBootstrapper { public function bootstrap(\Stancl\Tenancy\Contracts\Tenant $tenant): void @@ -115,3 +142,16 @@ class MyBootstrapper implements TenancyBootstrapper app()->instance('tenancy_ended', true); } } + +class ReinitBootstrapper implements TenancyBootstrapper +{ + public function bootstrap(\Stancl\Tenancy\Contracts\Tenant $tenant): void + { + app()->instance('tenancy_reinit_bootstrapper_key', $tenant->getAttribute('reinit_bootstrapper_key')); + } + + public function revert(): void + { + app()->instance('tenancy_reinit_bootstrapper_key', null); + } +} From e31249dd097b8b3b526808c378fbc5db1bc941fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Mon, 13 Apr 2026 23:57:59 +0200 Subject: [PATCH 04/15] Prevent mkdir() race conditions in FilesystemTenancyBootstrapper (#1453) This prevents race conditions that may occur if there are two concurrent processes trying to create the storage path for the tenant. The storagePath() method runs during bootstrap() which can easily happen in two places at once. The race condition specifically occurs in between the is_dir() check and the mkdir() call, the latter producing an exception if the dir already exist. We simply ignore any error coming out of mkdir() and then check for success separately. We could omit that success check since failure is unlikely and would only occur due to a server misconfiguration that would manifest itself in other ways as well, but this way the simple TOC/TOU race condition is prevented while other errors are still reported. We apply the same change to the mkdir() in scopeSessions() as the logic is similar. Resolves #1452 --- .../FilesystemTenancyBootstrapper.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index af2b809f..56091600 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Stancl\Tenancy\Bootstrappers; +use Exception; use Illuminate\Foundation\Application; use Illuminate\Session\FileSessionHandler; use Illuminate\Support\Facades\Storage; @@ -75,8 +76,13 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper : $this->originalStoragePath . '/framework/cache'; if (! is_dir($path)) { - // Create tenant framework/cache directory if it does not exist - mkdir($path, 0750, true); + // Create tenant framework/cache directory if it does not exist. + // We ignore errors due to TOCTOU race conditions, instead we check for success below. + @mkdir($path, 0750, true); + + if (! is_dir($path)) { + throw new Exception("Unable to create tenant storage directory [{$path}]."); + } } if ($suffix === false) { @@ -222,8 +228,13 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper : $this->originalStoragePath . '/framework/sessions'; if (! is_dir($path)) { - // Create tenant framework/sessions directory if it does not exist - mkdir($path, 0750, true); + // Create tenant framework/sessions directory if it does not exist. + // We ignore errors due to TOCTOU race conditions, instead we check for success below. + @mkdir($path, 0750, true); + + if (! is_dir($path)) { + throw new Exception("Unable to create tenant session directory [{$path}]."); + } } $this->app['config']['session.files'] = $path; From c32f52ce7cb9e705cbf1f5a5e884e466c8dde319 Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Wed, 15 Apr 2026 11:09:22 +0200 Subject: [PATCH 05/15] phpstan fix: Scope generics phpstan started failing with '... implements generic interface Illuminate\Database\Eloquent\Scope but does not specify its types: TModel'. We solve this by adding an implements docblock to the scopes implementing that interface. They're fairly generic - we just use the Model type itself in the code - so we use Model for the type parameter. --- src/Database/Concerns/PendingScope.php | 1 + src/Database/ParentModelScope.php | 1 + src/Database/TenantScope.php | 1 + 3 files changed, 3 insertions(+) diff --git a/src/Database/Concerns/PendingScope.php b/src/Database/Concerns/PendingScope.php index 712de6c7..52b8eb19 100644 --- a/src/Database/Concerns/PendingScope.php +++ b/src/Database/Concerns/PendingScope.php @@ -8,6 +8,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Scope; +/** @implements Scope */ class PendingScope implements Scope { /** diff --git a/src/Database/ParentModelScope.php b/src/Database/ParentModelScope.php index ee1a200e..44f4ac12 100644 --- a/src/Database/ParentModelScope.php +++ b/src/Database/ParentModelScope.php @@ -8,6 +8,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Scope; +/** @implements Scope */ class ParentModelScope implements Scope { /** diff --git a/src/Database/TenantScope.php b/src/Database/TenantScope.php index 050da365..94ff4572 100644 --- a/src/Database/TenantScope.php +++ b/src/Database/TenantScope.php @@ -9,6 +9,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Scope; use Stancl\Tenancy\Tenancy; +/** @implements Scope */ class TenantScope implements Scope { /** From ab2a4d84385b2fd857cf9f8fb9e950f208898d22 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 22 Apr 2026 14:32:53 +0200 Subject: [PATCH 06/15] Fix chaining `withoutPending()` with `where()` (#1457) At the moment, `where()` cannot be used correctly while using `withoutPending()`. For example, if we have a single non-pending tenant in our DB (with ID 'foo'), queries like `Tenant::withoutPending()->where('id', 'nonexistent')->first()`will incorrectly return the non-pending tenant ('foo'). This is because `withoutPending()` does `$builder->whereNull('data->pending_since')->orWhereNull('data')`. These two aren't grouped, so `withoutPending()->where('id', 'nonexistent')` basically translates to "WHERE data->pending_since IS NULL **OR (data IS NULL AND id = 'nonexistent')**". So the query will include all tenants whose `pending_since` is null (= all non-pending tenants). Grouping `->whereNull('data->pending_since')->orWhereNull('data')` in a closure passed to a separate `where()` fixes this issue. --- src/Database/Concerns/PendingScope.php | 6 ++++-- tests/PendingTenantsTest.php | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Database/Concerns/PendingScope.php b/src/Database/Concerns/PendingScope.php index 52b8eb19..99a5ef59 100644 --- a/src/Database/Concerns/PendingScope.php +++ b/src/Database/Concerns/PendingScope.php @@ -58,8 +58,10 @@ class PendingScope implements Scope { $builder->macro('withoutPending', function (Builder $builder) { $builder->withoutGlobalScope(static::class) - ->whereNull($builder->getModel()->getColumnForQuery('pending_since')) - ->orWhereNull($builder->getModel()->getDataColumn()); + ->where(function (Builder $query) { + $query->whereNull($query->getModel()->getColumnForQuery('pending_since')) + ->orWhereNull($query->getModel()->getDataColumn()); + }); return $builder; }); diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index a90aceed..433b85fb 100644 --- a/tests/PendingTenantsTest.php +++ b/tests/PendingTenantsTest.php @@ -111,6 +111,18 @@ test('a new tenant gets created while pulling a pending tenant if the pending po expect(Tenant::withPending()->get()->count())->toBe(1); // All tenants }); +test('withoutPending chained with where clauses returns correct results', function () { + $tenant = Tenant::create(); + $pendingTenant = Tenant::createPending(); + + // The query returned the correct tenant + expect(Tenant::withoutPending()->where('id', $tenant->id)->first()->id)->toBe($tenant->id); + // No tenant with this ID exists, the query returns null + expect(Tenant::withoutPending()->where('id', Str::random(8) . 'nonexistent-id')->first())->toBeNull(); + // withoutPending() correctly excludes the pending tenant from the query + expect(Tenant::withoutPending()->where('id', $pendingTenant->id)->first())->toBeNull(); +}); + test('pending tenants are included in all queries based on the include_in_queries config', function () { Tenant::createPending(); From 984911946a31351e1cb63bcb13d53b2390b5baf2 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 22 Apr 2026 16:45:54 +0200 Subject: [PATCH 07/15] Change tenant storage listeners into jobs (#1446) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `CreateTenantStorage` and `DeleteTenantStorage` listeners were used alongside JobPipelines. When the `TenantCreated` JobPipeline had `shouldBeQueued(true)` and the `Listeners\CreateTenantStorage` was uncommented, the listener would throw an exception (`Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException Database tenantX.sqlite does not exist.`) because at the time of executing the listener, the tenant DB wasn't created yet. The same issue could likely also occur in the `DeleteTenantStorage` listener as it uses `tenancy()->run()` to resolve the tenant's storage path which wouldn't work if the tenant's database (or other resources) was already deleted, making initialization impossible. This PR changes `DeleteTenantStorage` into a job and puts it (commented) into the job pipeline, so that it can be queued with the rest of the jobs. It also removes `CreateTenantStorage` because it should be redundant with the FilesystemTenancyBootstrapper creating the same paths automatically when storage path is suffixed. The old classes are kept but deprecated for backwards compatibility. We've also added some edge case hardening to `DeleteTenantStorage` to make sure it never deletes the central storage path directory, which previously could in theory occur due to a misconfiguration if a user enabled this job/listener but disabled storage path suffixing. Co-authored-by: Samuel Štancl Co-authored-by: github-actions[bot] Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- assets/TenancyServiceProvider.stub.php | 5 +- src/Jobs/DeleteTenantStorage.php | 43 ++++++++++++++ src/Listeners/CreateTenantStorage.php | 6 +- src/Listeners/DeleteTenantStorage.php | 21 ++++++- .../FilesystemTenancyBootstrapperTest.php | 57 ++++++++++++++++--- 5 files changed, 112 insertions(+), 20 deletions(-) create mode 100644 src/Jobs/DeleteTenantStorage.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 1cb358de..915e80c2 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -53,8 +53,6 @@ class TenancyServiceProvider extends ServiceProvider ])->send(function (Events\TenantCreated $event) { return $event->tenant; })->shouldBeQueued(false), - - // Listeners\CreateTenantStorage::class, ], Events\SavingTenant::class => [], Events\TenantSaved::class => [], @@ -63,12 +61,11 @@ class TenancyServiceProvider extends ServiceProvider Events\DeletingTenant::class => [ JobPipeline::make([ Jobs\DeleteDomains::class, + // Jobs\DeleteTenantStorage::class, // Jobs\RemoveStorageSymlinks::class, ])->send(function (Events\DeletingTenant $event) { return $event->tenant; })->shouldBeQueued(false), - - // Listeners\DeleteTenantStorage::class, ], Events\TenantDeleted::class => [ JobPipeline::make([ diff --git a/src/Jobs/DeleteTenantStorage.php b/src/Jobs/DeleteTenantStorage.php new file mode 100644 index 00000000..36a0d326 --- /dev/null +++ b/src/Jobs/DeleteTenantStorage.php @@ -0,0 +1,43 @@ +central(fn () => storage_path()); + $tenantStoragePath = tenancy()->run($this->tenant, fn () => storage_path()); + + if ($tenantStoragePath === $centralStoragePath) { + // Check again to ensure the tenant storage path is distinct from the central storage path + // to avoid any accidental central storage path deletion + return; + } + + if (is_dir($tenantStoragePath)) { + File::deleteDirectory($tenantStoragePath); + } + } +} diff --git a/src/Listeners/CreateTenantStorage.php b/src/Listeners/CreateTenantStorage.php index 3bebb731..0ffdef60 100644 --- a/src/Listeners/CreateTenantStorage.php +++ b/src/Listeners/CreateTenantStorage.php @@ -7,11 +7,7 @@ namespace Stancl\Tenancy\Listeners; use Stancl\Tenancy\Events\Contracts\TenantEvent; /** - * Can be used to manually create framework directories in the tenant storage when storage_path() is scoped. - * - * Useful when using real-time facades which use the framework/cache directory. - * - * Generally not needed anymore as the directory is also created by the FilesystemTenancyBootstrapper. + * @deprecated FilesystemTenancyBootstrapper creates the path automatically when suffix_storage_path is enabled. */ class CreateTenantStorage { diff --git a/src/Listeners/DeleteTenantStorage.php b/src/Listeners/DeleteTenantStorage.php index ec360073..06f20454 100644 --- a/src/Listeners/DeleteTenantStorage.php +++ b/src/Listeners/DeleteTenantStorage.php @@ -7,14 +7,29 @@ namespace Stancl\Tenancy\Listeners; use Illuminate\Support\Facades\File; use Stancl\Tenancy\Events\Contracts\TenantEvent; +/** + * @deprecated Use Stancl\Tenancy\Jobs\DeleteTenantStorage in a job pipeline instead. + */ class DeleteTenantStorage { public function handle(TenantEvent $event): void { - $path = tenancy()->run($event->tenant, fn () => storage_path()); + if (config('tenancy.filesystem.suffix_storage_path') === false) { + // Skip storage deletion if path suffixing is disabled + return; + } - if (is_dir($path)) { - File::deleteDirectory($path); + $centralStoragePath = tenancy()->central(fn () => storage_path()); + $tenantStoragePath = tenancy()->run($event->tenant, fn () => storage_path()); + + if ($tenantStoragePath === $centralStoragePath) { + // Check again to ensure the tenant storage path is distinct from the central storage path + // to avoid any accidental central storage path deletion + return; + } + + if (is_dir($tenantStoragePath)) { + File::deleteDirectory($tenantStoragePath); } } } diff --git a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php index 628b974e..4e834917 100644 --- a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php +++ b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php @@ -13,7 +13,7 @@ use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Jobs\CreateStorageSymlinks; use Stancl\Tenancy\Jobs\RemoveStorageSymlinks; use Stancl\Tenancy\Listeners\BootstrapTenancy; -use Stancl\Tenancy\Listeners\DeleteTenantStorage; +use Stancl\Tenancy\Jobs\DeleteTenantStorage; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use function Stancl\Tenancy\Tests\pest; @@ -184,21 +184,63 @@ test('create and delete storage symlinks jobs work', function() { $this->assertDirectoryDoesNotExist(public_path("public-$tenantKey")); }); -test('tenant storage can get deleted after the tenant when DeletingTenant listens to DeleteTenantStorage', function() { - Event::listen(DeletingTenant::class, DeleteTenantStorage::class); +test('tenant storage gets deleted during tenant deletion when the DeletingTenant pipeline contains DeleteTenantStorage', function() { + Event::listen(DeletingTenant::class, + JobPipeline::make([DeleteTenantStorage::class])->send(function (DeletingTenant $event) { + return $event->tenant; + })->shouldBeQueued(false)->toListener() + ); + + $centralStoragePath = storage_path(); + tenancy()->initialize(Tenant::create()); + + // FilesystemTenancyBootstrapper not enabled, + // tenant and central storage path is the same, + // the storage deletion will be skipped. + $tenantStoragePath = storage_path(); + expect($tenantStoragePath)->toBe($centralStoragePath); + expect(File::isDirectory($centralStoragePath))->toBeTrue(); + tenant()->delete(); + + expect(File::isDirectory($centralStoragePath))->toBeTrue(); + + config([ + 'tenancy.bootstrappers' => [FilesystemTenancyBootstrapper::class], + 'tenancy.filesystem.suffix_storage_path' => false, + ]); + + tenancy()->initialize(Tenant::create()); + + $tenantStoragePath = storage_path(); + + // FilesystemTenancyBootstrapper enabled, + // but tenant and central storage path is still the same + // because suffix_storage_path is false. + // The storage deletion will be skipped. + expect($tenantStoragePath)->toBe($centralStoragePath); + expect(File::isDirectory($centralStoragePath))->toBeTrue(); + tenant()->delete(); + + expect(File::isDirectory($centralStoragePath))->toBeTrue(); + + config([ + 'tenancy.bootstrappers' => [FilesystemTenancyBootstrapper::class], + 'tenancy.filesystem.suffix_storage_path' => true, + ]); tenancy()->initialize(Tenant::create()); $tenantStoragePath = storage_path(); - Storage::fake('test'); - + // FilesystemTenancyBootstrapper enabled, + // suffix_storage_path enabled, so the two paths are distinct. + // Tenant storage will be deleted. + expect($tenantStoragePath)->not()->toBe($centralStoragePath); expect(File::isDirectory($tenantStoragePath))->toBeTrue(); - Storage::put('test.txt', 'testing file'); - tenant()->delete(); expect(File::isDirectory($tenantStoragePath))->toBeFalse(); + expect(File::isDirectory($centralStoragePath))->toBeTrue(); }); test('the framework/cache directory is created when storage_path is scoped', function (bool $suffixStoragePath) { @@ -256,4 +298,3 @@ test('scoped disks are scoped per tenant', function () { expect(file_get_contents(storage_path() . "/app/public/scoped_disk_prefix/foo.txt"))->toBe('central2'); expect(file_get_contents(storage_path() . "/tenant{$tenant->id}/app/public/scoped_disk_prefix/foo.txt"))->toBe('tenant'); }); - From 53f44762cab15f5171e1196245cf644a64e1e8b5 Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Fri, 1 May 2026 15:48:11 +0200 Subject: [PATCH 08/15] docker: change mssql env yaml syntax --- docker-compose.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 70a68019..6cc03e12 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -80,8 +80,8 @@ services: mssql: image: mcr.microsoft.com/mssql/server:2022-latest environment: - - ACCEPT_EULA=Y - - SA_PASSWORD=P@ssword # must be the same as TENANCY_TEST_SQLSRV_PASSWORD + ACCEPT_EULA: "Y" + SA_PASSWORD: "P@ssword" # must be the same as TENANCY_TEST_SQLSRV_PASSWORD healthcheck: # https://github.com/Microsoft/mssql-docker/issues/133#issuecomment-1995615432 test: timeout 2 bash -c 'cat < /dev/null > /dev/tcp/127.0.0.1/1433' interval: 10s From 41701aff5f1105c025d99f091b5d7fe850330d27 Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Fri, 1 May 2026 16:07:18 +0200 Subject: [PATCH 09/15] phpstan fix: Model covariants in Scope generics Builds on changes in recent commit: Commit ID: c32f52ce7cb9e705cbf1f5a5e884e466c8dde319 Change ID: qsnosyvyulxzrnzorpxqwqqztmqorsmk --- src/Database/Concerns/PendingScope.php | 2 +- src/Database/ParentModelScope.php | 2 +- src/Database/TenantScope.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Database/Concerns/PendingScope.php b/src/Database/Concerns/PendingScope.php index 99a5ef59..e8805d8a 100644 --- a/src/Database/Concerns/PendingScope.php +++ b/src/Database/Concerns/PendingScope.php @@ -14,7 +14,7 @@ class PendingScope implements Scope /** * Apply the scope to a given Eloquent query builder. * - * @param Builder $builder + * @param Builder $builder * * @return void */ diff --git a/src/Database/ParentModelScope.php b/src/Database/ParentModelScope.php index 44f4ac12..9268fea9 100644 --- a/src/Database/ParentModelScope.php +++ b/src/Database/ParentModelScope.php @@ -12,7 +12,7 @@ use Illuminate\Database\Eloquent\Scope; class ParentModelScope implements Scope { /** - * @param Builder $builder + * @param Builder $builder */ public function apply(Builder $builder, Model $model): void { diff --git a/src/Database/TenantScope.php b/src/Database/TenantScope.php index 94ff4572..c6ce5f09 100644 --- a/src/Database/TenantScope.php +++ b/src/Database/TenantScope.php @@ -13,7 +13,7 @@ use Stancl\Tenancy\Tenancy; class TenantScope implements Scope { /** - * @param Builder $builder + * @param Builder $builder */ public function apply(Builder $builder, Model $model) { From 23b18c93a0cd75a855856bce5df911b35f930674 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 1 May 2026 21:57:19 +0200 Subject: [PATCH 10/15] Skip DB deletion when create_database=false, add ignoreFailures (#1394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Database deletion is now skipped by default if the tenant has the `create_database` internal attribute set to false, meaning it was likely created without a database. This skip can be opted out of by changing a static property. It also adds an opt-in static property for ignoring any other failures during database deletion, to allow continuing execution of the delete pipeline. --------- Co-authored-by: Samuel Štancl --- src/Jobs/DeleteDatabase.php | 26 ++++++++++- tests/DatabasePreparationTest.php | 77 +++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/src/Jobs/DeleteDatabase.php b/src/Jobs/DeleteDatabase.php index b59a1c05..ad022fda 100644 --- a/src/Jobs/DeleteDatabase.php +++ b/src/Jobs/DeleteDatabase.php @@ -22,12 +22,34 @@ class DeleteDatabase implements ShouldQueue protected TenantWithDatabase&Model $tenant, ) {} + /** Skip database deletion if the create_database internal attribute is false. */ + public static bool $skipWhenCreateDatabaseIsFalse = true; + + /** Ignore exceptions thrown during database deletion and continue execution. */ + public static bool $ignoreFailures = false; + public function handle(): void { + if (static::$skipWhenCreateDatabaseIsFalse && $this->tenant->getInternal('create_database') === false) { + // If database creation was skipped, we presume deletion should also be skipped. + // To avoid this skip, either unset the `create_database` attribute (or make it true), or + // set the $skipWhenCreateDatabaseIsFalse static property to false. + return; + } + event(new DeletingDatabase($this->tenant)); - $this->tenant->database()->manager()->deleteDatabase($this->tenant); + $deleted = false; - event(new DatabaseDeleted($this->tenant)); + try { + $this->tenant->database()->manager()->deleteDatabase($this->tenant); + $deleted = true; + } catch (\Throwable $e) { + if (! static::$ignoreFailures) { + throw $e; + } + } + + if ($deleted) event(new DatabaseDeleted($this->tenant)); } } diff --git a/tests/DatabasePreparationTest.php b/tests/DatabasePreparationTest.php index 1f3a4f09..ccf7eb2e 100644 --- a/tests/DatabasePreparationTest.php +++ b/tests/DatabasePreparationTest.php @@ -2,17 +2,27 @@ declare(strict_types=1); +use Illuminate\Database\QueryException; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Schema; use Stancl\JobPipeline\JobPipeline; use Stancl\Tenancy\Events\TenantCreated; +use Stancl\Tenancy\Events\TenantDeleted; use Stancl\Tenancy\Jobs\CreateDatabase; +use Stancl\Tenancy\Jobs\DeleteDatabase; use Stancl\Tenancy\Jobs\MigrateDatabase; use Stancl\Tenancy\Jobs\SeedDatabase; use Stancl\Tenancy\Tests\Etc\Tenant; use Illuminate\Foundation\Auth\User as Authenticable; use Stancl\Tenancy\Tests\Etc\TestSeeder; +beforeEach($cleanup = function () { + DeleteDatabase::$ignoreFailures = false; + DeleteDatabase::$skipWhenCreateDatabaseIsFalse = true; +}); + +afterEach($cleanup); + test('database can be created after tenant creation', function () { config(['tenancy.database.template_tenant_connection' => 'mysql']); @@ -82,6 +92,73 @@ test('custom job can be added to the pipeline', function () { }); }); +test('database can be deleted after tenant deletion', function () { + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + Event::listen(TenantDeleted::class, JobPipeline::make([DeleteDatabase::class])->send(function (TenantDeleted $event) { + return $event->tenant; + })->toListener()); + + $tenant = Tenant::create(); + $manager = $tenant->database()->manager(); + + expect($manager->databaseExists($tenant->database()->getName()))->toBeTrue(); + + $tenant->delete(); + + expect($manager->databaseExists($tenant->database()->getName()))->toBeFalse(); +}); + +test('database deletion is skipped when create_database is false', function (bool $skipWhenCreateDatabaseIsFalse) { + Event::listen(TenantDeleted::class, JobPipeline::make([DeleteDatabase::class])->send(function (TenantDeleted $event) { + return $event->tenant; + })->toListener()); + + // create_database=false means no DB is created (e.g. tenant uses a pre-existing DB) + // On deletion, DeleteDatabase should skip rather than attempting DROP DATABASE on a non-existent DB + $tenant = Tenant::create(['tenancy_create_database' => false, 'tenancy_db_name' => 'non_existing_db']); + + $manager = $tenant->database()->manager(); + expect($manager->databaseExists($tenant->database()->getName()))->toBeFalse(); + + DeleteDatabase::$skipWhenCreateDatabaseIsFalse = $skipWhenCreateDatabaseIsFalse; + + if ($skipWhenCreateDatabaseIsFalse) { + $tenant->delete(); // no exception + } else { + expect(fn () => $tenant->delete())->toThrow(QueryException::class, "database doesn't exist"); + } + + expect($manager->databaseExists($tenant->database()->getName()))->toBeFalse(); +})->with([true, false]); + +test('database deletion failure is ignored when ignoreFailures is true', function (bool $ignoreFailures) { + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + Event::listen(TenantDeleted::class, JobPipeline::make([DeleteDatabase::class])->send(function (TenantDeleted $event) { + return $event->tenant; + })->toListener()); + + DeleteDatabase::$ignoreFailures = $ignoreFailures; + + $tenant = Tenant::create(); + $manager = $tenant->database()->manager(); + expect($manager->databaseExists($tenant->database()->getName()))->toBeTrue(); + + $manager->deleteDatabase($tenant); // manually delete so the job fails + expect($manager->databaseExists($tenant->database()->getName()))->toBeFalse(); + + if ($ignoreFailures) { + $tenant->delete(); // no exception + } else { + expect(fn () => $tenant->delete())->toThrow(QueryException::class, "database doesn't exist"); + } +})->with([true, false]); + class User extends Authenticable { protected $guarded = []; From ec06dcc52e24859e3beb3486e60368fae087bacc Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 11 May 2026 14:26:06 +0200 Subject: [PATCH 11/15] Correct `DomainTenantResolver::isSubdomain()` check (#1425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added a failing test for determining if a host is a subdomain, then fixed `DomainTenantResolver::isSubdomain()` (similar fix as in #1423) and a related assertion. Previously, while having `tenancy.identification.central_domains` set to e.g. `['site.com']`, the `isSubdomain()` check consider `tenantsite.com` a subdomain because it ends with `site.com`. Now, instead of the `endsWith()` check, the method checks if the passed domain is in the configured central domains. If it is, it returns `false`. Otherwise, loop through all the central domains and check if the passed domain matches any of the central domains prefixed with a dot (e.g. `tenant.site.com` would be considered a subdomain, `tenant.site.com` wouldn't). Because in InitializeTenancyByDomainOrSubdomain, if tenancy fails to initialize using a subdomain (before this PR's changes, e.g. `tenantsite.com` would be considered a subdomain, and `tenantsite` would be used for initializing tenancy), it'll catch the exception and use the whole domain for identification instead, this error will likely never be noticed in real-world usage. So this PR corrects the subdomain detection logic, but the real-world impact of that is negligible. > Note: The subdomain error catching logic in domainOrSubdomain ID MW was added in v4. If we applied this change in v3, it'd fix a real issue where domainOrSubdomain ID MW would just fail at the subdomain initialization, without attempting domain initialization after the failure. --------- Co-authored-by: github-actions[bot] Co-authored-by: Samuel Štancl --- src/Resolvers/DomainTenantResolver.php | 15 ++++++++++++++- tests/EarlyIdentificationTest.php | 2 +- tests/SubdomainTest.php | 9 +++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/Resolvers/DomainTenantResolver.php b/src/Resolvers/DomainTenantResolver.php index 9535cdf2..59ebb81f 100644 --- a/src/Resolvers/DomainTenantResolver.php +++ b/src/Resolvers/DomainTenantResolver.php @@ -7,6 +7,7 @@ namespace Stancl\Tenancy\Resolvers; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\Relation; +use Illuminate\Support\Arr; use Illuminate\Support\Str; use Stancl\Tenancy\Contracts\Domain; use Stancl\Tenancy\Contracts\SingleDomainTenant; @@ -58,7 +59,19 @@ class DomainTenantResolver extends Contracts\CachedTenantResolver public static function isSubdomain(string $domain): bool { - return Str::endsWith($domain, config('tenancy.identification.central_domains')); + $centralDomains = Arr::wrap(config('tenancy.identification.central_domains')); + + if (in_array($domain, $centralDomains, true)) { + return false; + } + + foreach ($centralDomains as $centralDomain) { + if (Str::endsWith($domain, '.' . $centralDomain)) { + return true; + } + } + + return false; } public function resolved(Tenant $tenant, mixed ...$args): void diff --git a/tests/EarlyIdentificationTest.php b/tests/EarlyIdentificationTest.php index e6c08d26..4521b613 100644 --- a/tests/EarlyIdentificationTest.php +++ b/tests/EarlyIdentificationTest.php @@ -300,7 +300,7 @@ test('using different default route modes works with global domain identificatio $exception = match ($middleware) { InitializeTenancyByDomain::class => TenantCouldNotBeIdentifiedOnDomainException::class, InitializeTenancyBySubdomain::class => NotASubdomainException::class, - InitializeTenancyByDomainOrSubdomain::class => NotASubdomainException::class, + InitializeTenancyByDomainOrSubdomain::class => TenantCouldNotBeIdentifiedOnDomainException::class, }; expect(fn () => $this->withoutExceptionHandling()->get('http://localhost/central-route'))->toThrow($exception); diff --git a/tests/SubdomainTest.php b/tests/SubdomainTest.php index a7cc58ae..62e002f2 100644 --- a/tests/SubdomainTest.php +++ b/tests/SubdomainTest.php @@ -7,6 +7,7 @@ use Stancl\Tenancy\Database\Concerns\HasDomains; use Stancl\Tenancy\Exceptions\NotASubdomainException; use Stancl\Tenancy\Middleware\InitializeTenancyBySubdomain; use Stancl\Tenancy\Database\Models; +use Stancl\Tenancy\Resolvers\DomainTenantResolver; use function Stancl\Tenancy\Tests\pest; beforeEach(function () { @@ -108,6 +109,14 @@ test('we cant use a subdomain that doesnt belong to our central domains', functi ->get('http://foo.localhost/foo/abc/xyz'); }); +test('domain resolver correctly determines if string is a subdomain', function() { + config(['tenancy.identification.central_domains' => ['site.com', 'blog.site.com']]); + + expect(DomainTenantResolver::isSubdomain('blog.site.com'))->toBeFalse(); + expect(DomainTenantResolver::isSubdomain('tenant.site.com'))->toBeTrue(); + expect(DomainTenantResolver::isSubdomain('tenantsite.com'))->toBeFalse(); +}); + class SubdomainTenant extends Models\Tenant { use HasDomains; From da7eb94c07791e6c86397dac506d0997709d4965 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 12 May 2026 23:59:21 +0200 Subject: [PATCH 12/15] Remove redundant universal route check from PreventAccess MW (#1427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PreventAcessFromUnwantedDomains MW had the `tenancy()->routeIsUniversal($route)` check either for returning early, or it was a leftover from some older implementation, so I removed it. The middleware aborts if the `$this->accessingTenantRouteFromCentralDomain($request, $route) || $this->accessingCentralRouteFromTenantDomain($request, $route)` check passes. Meaning, **for the middleware to abort, the route has to be either in central or tenant mode**. When the route is in universal mode, the middleware will never reach `return $abortRequest()`. `return $next($request)` will always get reached, even when the `|| tenancy()->routeIsUniversal($route)` check is deleted from the previous condition, so that check was basically useless. Since the docblock for the class does mention the behavior for universal routes explicitly, we've instead added a comment documenting that things work this way. That's probably the most reasonable way to have this explicit behavior for universal routes easily understandable in this fairly complex logic without redundant code. Resolves #1418 --------- Co-authored-by: Samuel Štancl --- src/Middleware/PreventAccessFromUnwantedDomains.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Middleware/PreventAccessFromUnwantedDomains.php b/src/Middleware/PreventAccessFromUnwantedDomains.php index cdfa3b2c..7f628583 100644 --- a/src/Middleware/PreventAccessFromUnwantedDomains.php +++ b/src/Middleware/PreventAccessFromUnwantedDomains.php @@ -31,10 +31,12 @@ class PreventAccessFromUnwantedDomains { $route = tenancy()->getRoute($request); - if ($this->shouldBeSkipped($route) || tenancy()->routeIsUniversal($route)) { + if ($this->shouldBeSkipped($route)) { return $next($request); } + // If the route is universal, neither of these checks will pass and the logic will + // fall through to the $next($request) call at the end. if ($this->accessingTenantRouteFromCentralDomain($request, $route) || $this->accessingCentralRouteFromTenantDomain($request, $route)) { $abortRequest = static::$abortRequest ?? function () { abort(404); From c0fbf6dcbdfd4f6e553ab469af2628ecbf94406d Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 5 Jun 2026 23:15:19 +0200 Subject: [PATCH 13/15] [MINOR BC] UserImpersonation: store auth guard in session, add `$logout` param to `stopImpersonating()` (#1437) > Minor breaking change: `session('tenancy_impersonating')` doesn't work anymore. Use `session('tenancy_impersonation_guard')` instead. The 'tenancy_impersonating' session variable got replaced by 'tenancy_impersonation_guard'. `UserImpersonation::stopImpersonating()` now calls `logout()` on the guard retrieved by `session()->get('tenancy_impersonation_guard')` instead of calling `logout()` on the _current_ auth guard. Now. if you create the impersonation token with guard 'web', and call `UserImpersonation::stopImpersonating()`, for example in a route that has the `auth:sanctum` middleware (= the current guard in that route would be `RequestGuard` which doesn't even have the `logout()` method -- not the guard for which the impersonation token was created), the method will correctly log the user out of the 'web' guard using which he was actually authenticated instead of the current guard of the visited route (which doesn't have to be the same guard for which impersonation started). `UserImpersonation::stopImpersonating()` now also accepts the `$logout` parameter, which is `true` by default. If `false` is passed, the method just forgets `tenancy_impersonation_guard` from session without logging out. `UserImpersonation::stopImpersonating()` now throws an exception if impersonation wasn't active at the point of calling the method. --------- Co-authored-by: Samuel Stancl Co-authored-by: github-actions[bot] --- src/Features/UserImpersonation.php | 29 ++++++-- tests/TenantUserImpersonationTest.php | 103 +++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 11 deletions(-) diff --git a/src/Features/UserImpersonation.php b/src/Features/UserImpersonation.php index d286b8ba..be2b01fd 100644 --- a/src/Features/UserImpersonation.php +++ b/src/Features/UserImpersonation.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Stancl\Tenancy\Features; +use Exception; use Illuminate\Database\Eloquent\Model; use Illuminate\Http\RedirectResponse; use Illuminate\Support\Facades\Auth; @@ -61,9 +62,9 @@ class UserImpersonation implements Feature Auth::guard($token->auth_guard)->loginUsingId($token->user_id, $token->remember); - $token->delete(); + session()->put('tenancy_impersonation_guard', $token->auth_guard); - session()->put('tenancy_impersonating', true); + $token->delete(); return redirect($token->redirect_url); } @@ -76,16 +77,30 @@ class UserImpersonation implements Feature public static function isImpersonating(): bool { - return session()->has('tenancy_impersonating'); + return session()->has('tenancy_impersonation_guard'); } /** - * Logout from the current domain and forget impersonation session. + * Stop user impersonation by forgetting the impersonation session. + * + * When $logout is true, the user will also be logged out + * from the impersonation guard stored in the session. + * + * Throws an exception if impersonation is not active + * (= the impersonation guard is not in the session). */ - public static function stopImpersonating(): void + public static function stopImpersonating(bool $logout = true): void { - auth()->logout(); + if (! static::isImpersonating()) { + throw new Exception('Not currently impersonating any user.'); + } - session()->forget('tenancy_impersonating'); + if ($logout) { + $guard = session()->get('tenancy_impersonation_guard'); + + auth($guard)->logout(); + } + + session()->forget('tenancy_impersonation_guard'); } } diff --git a/tests/TenantUserImpersonationTest.php b/tests/TenantUserImpersonationTest.php index ea679357..120ce826 100644 --- a/tests/TenantUserImpersonationTest.php +++ b/tests/TenantUserImpersonationTest.php @@ -89,13 +89,14 @@ test('tenant user can be impersonated on a tenant domain', function () { ->assertSee('You are logged in as Joe'); expect(UserImpersonation::isImpersonating())->toBeTrue(); - expect(session('tenancy_impersonating'))->toBeTrue(); + expect(session('tenancy_impersonation_guard'))->toBe('web'); + expect($token->auth_guard)->toBe('web'); // Leave impersonation UserImpersonation::stopImpersonating(); expect(UserImpersonation::isImpersonating())->toBeFalse(); - expect(session('tenancy_impersonating'))->toBeNull(); + expect(session('tenancy_impersonation_guard'))->toBeNull(); // Assert can't access the tenant dashboard pest()->get('http://foo.localhost/dashboard') @@ -135,19 +136,113 @@ test('tenant user can be impersonated on a tenant path', function () { ->assertSee('You are logged in as Joe'); expect(UserImpersonation::isImpersonating())->toBeTrue(); - expect(session('tenancy_impersonating'))->toBeTrue(); + expect(session('tenancy_impersonation_guard'))->toBe('web'); + expect($token->auth_guard)->toBe('web'); // Leave impersonation UserImpersonation::stopImpersonating(); expect(UserImpersonation::isImpersonating())->toBeFalse(); - expect(session('tenancy_impersonating'))->toBeNull(); + expect(session('tenancy_impersonation_guard'))->toBeNull(); // Assert can't access the tenant dashboard pest()->get('/acme/dashboard') ->assertRedirect('/login'); }); +test('stopImpersonating can keep the user authenticated', function () { + makeLoginRoute(); + + Route::middleware(InitializeTenancyByPath::class)->prefix('/{tenant}')->group(getRoutes(false)); + + $tenant = Tenant::create([ + 'id' => 'acme', + 'tenancy_db_name' => 'db' . Str::random(16), + ]); + + migrateTenants(); + + $user = $tenant->run(function () { + return ImpersonationUser::create([ + 'name' => 'Joe', + 'email' => 'joe@local', + 'password' => bcrypt('secret'), + ]); + }); + + // Impersonate the user + $token = tenancy()->impersonate($tenant, $user->id, '/acme/dashboard'); + + pest()->get('/acme/impersonate/' . $token->token) + ->assertRedirect('/acme/dashboard'); + + expect(UserImpersonation::isImpersonating())->toBeTrue(); + + // Stop impersonating without logging out + UserImpersonation::stopImpersonating(false); + + // The impersonation session key should be cleared + expect(UserImpersonation::isImpersonating())->toBeFalse(); + expect(session('tenancy_impersonation_guard'))->toBeNull(); + + // The user should still be authenticated + pest()->get('/acme/dashboard') + ->assertSuccessful() + ->assertSee('You are logged in as Joe'); +}); + +test('stopImpersonating logs out the user from the impersonation guard stored in session', function () { + Route::middleware(InitializeTenancyByPath::class)->prefix('/{tenant}')->group(getRoutes(false)); + + $tenant = Tenant::create([ + 'id' => 'acme', + 'tenancy_db_name' => 'db' . Str::random(16), + ]); + + migrateTenants(); + + $user = $tenant->run(function () { + return ImpersonationUser::create([ + 'name' => 'Joe', + 'email' => 'joe@local', + 'password' => bcrypt('secret'), + ]); + }); + + // Impersonate the user + $token = tenancy()->impersonate($tenant, $user->id, '/acme/dashboard'); + + pest()->get('/acme/impersonate/' . $token->token) + ->assertRedirect('/acme/dashboard'); + + expect(session('tenancy_impersonation_guard'))->toBe('web'); + + // Impersonation logged in the user using the current guard ('web') + expect(auth('web')->check())->toBeTrue(); + + config(['auth.guards.test' => [ + 'driver' => 'session', + 'provider' => 'users', + ]]); + + // Manually log the user in through the 'test' guard + auth('test')->loginUsingId($user->id); + + // Should log the user out from the guard used for impersonation ('web') + UserImpersonation::stopImpersonating(); + + expect(auth('web')->check())->toBeFalse(); + expect(auth('test')->check())->toBeTrue(); + + expect(UserImpersonation::isImpersonating())->toBeFalse(); + + // tenancy_impersonation_guard isn't in the session anymore, + // stopImpersonating should throw an exception instead of logging out + expect(fn() => UserImpersonation::stopImpersonating())->toThrow(Exception::class); + + expect(auth('test')->check())->toBeTrue(); +}); + test('tokens have a limited ttl', function () { Route::middleware(InitializeTenancyByDomain::class)->group(getRoutes()); From ad4c924d5c0fbcc9cdc94ec0186f8ff571add166 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Sat, 6 Jun 2026 00:36:57 +0200 Subject: [PATCH 14/15] [MINOR BC] Create pending tenants with pending_since, improve --with-pending (#1458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > Minor breaking change: Pending tenants would previously go through the creation pipeline as *not* pending and would only be marked as pending after full creation. Now, pending tenants go through the creation process with pending_since set from the start. Pending tenants aren't getting their `pending_since` set until they're created completely (e.g. their DB was created, migrated and seeded -- first, the tenant is created fully, and only after that, the tenant is updated to have `pending_since`). This is a problem if someone wants to e.g. add a job to the `DatabaseCreated` job pipeline that would check `$this->tenant->pending()`. Since at the point of `DatabaseCreated`, the tenant's `pending_since` isn't set yet, `$this->tenant->pending()` returns `false`, even for tenants created using `createPending()`. So instead of letting the pending tenant get fully created, and only after that, setting its `pending_since` (using `update()`), we now set `pending_since` in `create()`. `CreatingPendingTenant` is now dispatched from the `static::creating` hook, and `PendingTenantCreated` is dispatched from `static::created` for consistency. Setting `pending_since` right in `create()` made the `MigrateDatabase` and `SeedDatabase` jobs exclude the pending tenants during their creation if the `tenancy.pending.include_in_queries` config was set to `false` -- in that case, these jobs would never migrate or seed the databases of pending tenants. So these jobs now pass `--with-pending` to their underlying commands, with the value set in their `$includePending` static property (`true` by default). This overrides the `tenancy.pending.include_in_queries` config -- unless the `$includePending` properties are set to `false`, these jobs will always include pending tenants. The `--with-pending` tenant command option originally worked just to opt-in for including pending tenants in the command. Now, `--with-pending` can accept values (`true`/`1` or `false`/`0`), so e.g. - `tenants:run foo` with `--with-pending`/`--with-pending=true`/`--with-pending=1` includes pending tenants - `tenants:run foo` with `--with-pending=false`/`--with-pending=0` **excludes** pending tenants (also `--with-pending=foobar` -- invalid input, considered `false`) Passing `--with-pending` makes the command bypass the `tenancy.pending.include_in_queries` config (so e.g. if `tenancy.pending.include_in_queries` is set to `true`, and `--with-pending=false` is passed to a command, the command will exclude pending tenants). When `--with-pending` is not passed, the command will include or exclude pending tenants based on the `tenancy.pending.include_in_queries` config. --------- Co-authored-by: Copilot Co-authored-by: Samuel Štancl --- src/Concerns/HasTenantOptions.php | 8 +- src/Database/Concerns/HasPending.php | 33 ++--- src/Jobs/MigrateDatabase.php | 11 ++ src/Jobs/SeedDatabase.php | 11 ++ tests/PendingTenantsTest.php | 189 +++++++++++++++++++++++---- 5 files changed, 206 insertions(+), 46 deletions(-) diff --git a/src/Concerns/HasTenantOptions.php b/src/Concerns/HasTenantOptions.php index c1ea221f..3933c469 100644 --- a/src/Concerns/HasTenantOptions.php +++ b/src/Concerns/HasTenantOptions.php @@ -18,7 +18,7 @@ trait HasTenantOptions { return array_merge([ new InputOption('tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, 'The tenants to run this command for. Leave empty for all tenants', null), - new InputOption('with-pending', null, InputOption::VALUE_NONE, 'Include pending tenants in query'), // todo@pending should we also offer without-pending? if we add this, mention in docs + new InputOption('with-pending', null, InputOption::VALUE_OPTIONAL, 'Include pending tenants in query if true/1, exclude if false/0. Defaults to the tenancy.pending.include_in_queries config value.'), ], parent::getOptions()); } @@ -43,7 +43,11 @@ trait HasTenantOptions $query->whereIn(tenancy()->model()->getTenantKeyName(), $this->option('tenants')); }) ->when(tenancy()->model()::hasGlobalScope(PendingScope::class), function ($query) { - $query->withPending(config('tenancy.pending.include_in_queries') ?: $this->option('with-pending')); + $includePending = $this->input->hasParameterOption('--with-pending') + ? filter_var($this->option('with-pending') ?? true, FILTER_VALIDATE_BOOLEAN) + : config('tenancy.pending.include_in_queries'); + + $query->withPending($includePending); }); } diff --git a/src/Database/Concerns/HasPending.php b/src/Database/Concerns/HasPending.php index 0a572680..04fcccc1 100644 --- a/src/Database/Concerns/HasPending.php +++ b/src/Database/Concerns/HasPending.php @@ -28,6 +28,18 @@ trait HasPending public static function bootHasPending(): void { static::addGlobalScope(new PendingScope()); + + static::creating(function (self $tenant): void { + if ($tenant->pending()) { + event(new CreatingPendingTenant($tenant)); + } + }); + + static::created(function (self $tenant): void { + if ($tenant->pending()) { + event(new PendingTenantCreated($tenant)); + } + }); } /** Initialize the trait. */ @@ -49,22 +61,11 @@ trait HasPending */ public static function createPending(array $attributes = []): Model&Tenant { - $tenant = null; - - try { - $tenant = static::create(array_merge(static::getPendingAttributes($attributes), $attributes)); - event(new CreatingPendingTenant($tenant)); - } finally { - // Update the pending_since value only after the tenant is created so it's - // not marked as pending until after migrations, seeders, etc are run. - $tenant?->update([ - 'pending_since' => now()->timestamp, - ]); - } - - event(new PendingTenantCreated($tenant)); - - return $tenant; + return static::create(array_merge( + static::getPendingAttributes($attributes), + $attributes, + ['pending_since' => now()->timestamp], + )); } /** diff --git a/src/Jobs/MigrateDatabase.php b/src/Jobs/MigrateDatabase.php index 424dacc9..b090b70a 100644 --- a/src/Jobs/MigrateDatabase.php +++ b/src/Jobs/MigrateDatabase.php @@ -17,6 +17,16 @@ class MigrateDatabase implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + /** + * Should pending tenants be included while migrating, + * regardless of the tenancy.pending.include_in_queries config value. + * + * If false, pending tenants will be specifically excluded. + * + * If null, default to tenancy.pending.include_in_queries config. + */ + public static ?bool $includePending = true; + public function __construct( protected TenantWithDatabase&Model $tenant, ) {} @@ -25,6 +35,7 @@ class MigrateDatabase implements ShouldQueue { Artisan::call('tenants:migrate', [ '--tenants' => [$this->tenant->getTenantKey()], + '--with-pending' => static::$includePending ?? config('tenancy.pending.include_in_queries'), ]); } } diff --git a/src/Jobs/SeedDatabase.php b/src/Jobs/SeedDatabase.php index 9958695e..85058b5d 100644 --- a/src/Jobs/SeedDatabase.php +++ b/src/Jobs/SeedDatabase.php @@ -17,6 +17,16 @@ class SeedDatabase implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + /** + * Should pending tenants be included while seeding, + * regardless of the tenancy.pending.include_in_queries config value. + * + * If false, pending tenants will be specifically excluded. + * + * If null, default to tenancy.pending.include_in_queries config. + */ + public static ?bool $includePending = true; + public function __construct( protected TenantWithDatabase&Model $tenant, ) {} @@ -25,6 +35,7 @@ class SeedDatabase implements ShouldQueue { Artisan::call('tenants:seed', [ '--tenants' => [$this->tenant->getTenantKey()], + '--with-pending' => static::$includePending ?? config('tenancy.pending.include_in_queries'), ]); } } diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index 433b85fb..b04f8bc4 100644 --- a/tests/PendingTenantsTest.php +++ b/tests/PendingTenantsTest.php @@ -16,10 +16,25 @@ use Stancl\Tenancy\Events\PendingTenantPulled; use Stancl\Tenancy\Events\PullingPendingTenant; use Stancl\Tenancy\Tests\Etc\Tenant; use function Stancl\Tenancy\Tests\pest; +use Stancl\Tenancy\Events\TenantCreated; +use Stancl\JobPipeline\JobPipeline; +use Stancl\Tenancy\Jobs\CreateDatabase; +use Stancl\Tenancy\Jobs\MigrateDatabase; +use Stancl\Tenancy\Jobs\SeedDatabase; +use Stancl\Tenancy\Tests\Etc\User; +use Stancl\Tenancy\Tests\Etc\TestSeeder; +use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; +use Stancl\Tenancy\Events\TenancyInitialized; +use Stancl\Tenancy\Listeners\BootstrapTenancy; +use Stancl\Tenancy\Events\TenancyEnded; +use Stancl\Tenancy\Listeners\RevertToCentralContext; beforeEach($cleanup = function () { Tenant::$extraCustomColumns = []; Tenant::$getPendingAttributesUsing = null; + + MigrateDatabase::$includePending = true; + SeedDatabase::$includePending = true; }); afterEach($cleanup); @@ -154,8 +169,8 @@ test('pending events are dispatched', function () { Event::assertDispatched(PendingTenantPulled::class); }); -test('commands do not run for pending tenants if tenancy.pending.include_in_queries is false and the with pending option does not get passed', function() { - config(['tenancy.pending.include_in_queries' => false]); +test('commands include tenants based on the include_in_queries config when --with-pending is not passed', function (bool $includeInQueries) { + config(['tenancy.pending.include_in_queries' => $includeInQueries]); $tenants = collect([ Tenant::create(), @@ -164,21 +179,21 @@ test('commands do not run for pending tenants if tenancy.pending.include_in_quer Tenant::createPending(), ]); - pest()->artisan('tenants:migrate --with-pending'); + $command = pest()->artisan("tenants:run 'bar testing testing@test.test password foo'"); - $artisan = pest()->artisan("tenants:run 'foo foo --b=bar --c=xyz'"); + $tenants->each(function ($tenant) use ($command, $includeInQueries) { + if ($tenant->pending() && ! $includeInQueries) { + $command->doesntExpectOutputToContain("Tenant: {$tenant->getTenantKey()}"); + } else { + $command->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}"); + } + }); - $pendingTenants = $tenants->filter->pending(); - $readyTenants = $tenants->reject->pending(); + $command->assertSuccessful(); +})->with([true, false]); - $pendingTenants->each(fn ($tenant) => $artisan->doesntExpectOutputToContain("Tenant: {$tenant->getTenantKey()}")); - $readyTenants->each(fn ($tenant) => $artisan->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}")); - - $artisan->assertExitCode(0); -}); - -test('commands run for pending tenants too if tenancy.pending.include_in_queries is true', function() { - config(['tenancy.pending.include_in_queries' => true]); +test('commands include pending tenants when truthy --with-pending is passed', function (bool $includeInQueries) { + config(['tenancy.pending.include_in_queries' => $includeInQueries]); $tenants = collect([ Tenant::create(), @@ -187,17 +202,22 @@ test('commands run for pending tenants too if tenancy.pending.include_in_queries Tenant::createPending(), ]); - pest()->artisan('tenants:migrate --with-pending'); + foreach ([ + '--with-pending', + '--with-pending=true', + '--with-pending=1' + ] as $option) { + $command = pest()->artisan("tenants:run 'bar testing testing@test.test password foo' {$option}"); - $artisan = pest()->artisan("tenants:run 'foo foo --b=bar --c=xyz'"); + // Pending tenants are included regardless of tenancy.pending.include_in_queries + $tenants->each(fn ($tenant) => $command->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}")); - $tenants->each(fn ($tenant) => $artisan->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}")); + $command->assertSuccessful(); + } +})->with([true, false]); - $artisan->assertExitCode(0); -}); - -test('commands run for pending tenants too if the with pending option is passed', function() { - config(['tenancy.pending.include_in_queries' => false]); +test('commands exclude pending tenants when falsy --with-pending is passed', function (bool $includeInQueries) { + config(['tenancy.pending.include_in_queries' => $includeInQueries]); $tenants = collect([ Tenant::create(), @@ -206,14 +226,25 @@ test('commands run for pending tenants too if the with pending option is passed' Tenant::createPending(), ]); - pest()->artisan('tenants:migrate --with-pending'); + foreach ([ + '--with-pending=false', + '--with-pending=0', + '--with-pending=foo' // Invalid values are treated as false + ] as $option) { + $command = pest()->artisan("tenants:run 'bar testing testing@test.test password foo' {$option}"); - $artisan = pest()->artisan("tenants:run 'foo foo --b=bar --c=xyz' --with-pending"); + $tenants->each(function ($tenant) use ($command) { + if ($tenant->pending()) { + // Pending tenants are excluded regardless of tenancy.pending.include_in_queries + $command->doesntExpectOutputToContain("Tenant: {$tenant->getTenantKey()}"); + } else { + $command->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}"); + } + }); - $tenants->each(fn ($tenant) => $artisan->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}")); - - $artisan->assertExitCode(0); -}); + $command->assertSuccessful(); + } +})->with([true, false]); test('pending tenants can have default attributes for non-nullable columns', function (bool $withPendingAttributes) { Schema::table('tenants', function (Blueprint $table) { @@ -236,3 +267,105 @@ test('pending tenants can have default attributes for non-nullable columns', fun else expect($fn)->toThrow(QueryException::class); })->with([true, false]); + +test('pending tenant databases can be migrated using a job unless configured otherwise', function (bool $includeInQueries, ?bool $migrateWithPending) { + config([ + 'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class], + 'tenancy.pending.include_in_queries' => $includeInQueries, + ]); + + MigrateDatabase::$includePending = $migrateWithPending; + + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); + Event::listen(TenantCreated::class, JobPipeline::make([ + CreateDatabase::class, + MigrateDatabase::class, + ])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + $pendingTenant = Tenant::createPending(); + + expect(Schema::hasTable('users'))->toBeFalse(); + + tenancy()->initialize($pendingTenant); + + // MigrateDatabase includes/excludes pending tenants based on its $includePending property, + // regardless of the tenancy.pending.include_in_queries config. + expect(Schema::hasTable('users'))->toBe($migrateWithPending ?? $includeInQueries); +})->with([ + 'include pending in queries' => [true], + 'exclude pending from queries' => [false], +])->with([ + 'migrate with pending' => [true], + 'migrate without pending' => [false], + 'default to config' => [null], +]); + +test('pending tenant databases can be seeded using a job unless configured otherwise', function (bool $includeInQueries, ?bool $seedWithPending) { + config([ + 'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class], + 'tenancy.pending.include_in_queries' => $includeInQueries, + 'tenancy.seeder_parameters.--class' => TestSeeder::class, + ]); + + MigrateDatabase::$includePending = true; + SeedDatabase::$includePending = $seedWithPending; + + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); + Event::listen(TenantCreated::class, JobPipeline::make([ + CreateDatabase::class, + MigrateDatabase::class, + SeedDatabase::class, + ])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + $pendingTenant = Tenant::createPending(); + + tenancy()->initialize($pendingTenant); + + // SeedDatabase includes/excludes pending tenants based on its $includePending property, + // regardless of the tenancy.pending.include_in_queries config. + expect(User::where('email', 'seeded@user')->exists())->toBe($seedWithPending ?? $includeInQueries); +})->with([ + 'include pending in queries' => [true], + 'exclude pending from queries' => [false], +])->with([ + 'seed with pending' => [true], + 'seed without pending' => [false], + 'default to config' => [null], +]); + +test('jobs that run before tenants get fully created recognize pending tenants', function () { + config([ + 'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class], + ]); + + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); + Event::listen(TenantCreated::class, JobPipeline::make([ + CreateDatabase::class, + PendingTenantJob::class, + ])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + Tenant::createPending(); + + expect(app('tenant_is_pending'))->toBeTrue(); +}); + +class PendingTenantJob +{ + public function __construct( + public Tenant $tenant, + ) {} + + public function handle() + { + app()->instance('tenant_is_pending', $this->tenant->pending()); + } +} From dfb0e1ad66c6610253032968bb793855747bb391 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Sat, 6 Jun 2026 23:52:37 +0200 Subject: [PATCH 15/15] TenancyUrlGenerator: override `toRoute()`, refactor (#1439) This PR adds the `toRoute()` method override to `TenancyUrlGenerator`. `toRoute()` now attempts to find a tenant equivalent of the passed route (= a route with the same name as the passed one, but with the tenant prefix) and generates URL for the tenant route. This behavior can be bypassed using the bypass parameter, like with the `route()` method override `TenancyUrlGenerator` had until now. The primary reason for adding this is that Livewire v4 no longer uses the `route()` helper (which automatically prefixes the passed route name because of the override in `TenancyUrlGenerator`) in `Livewire::getUpdateUri()`. Now, it uses `toRoute()` (https://github.com/livewire/livewire/commit/544aa3dfb8195f342ef0adbf179139841ad817b7#diff-e7609f8b0a60bde5a85067803d4e2f08f235c7cee9225a51ea67a85ff9a1d694R52), which didn't automatically swap the route for its 'tenant.'-prefixed equivalent in tenant context (until now). So for the Livewire integration to work with path identification, we need to override `toRoute()` as described. The `temporarySignedRoute()` override got removed because `temporarySignedRoute()` calls `route()` under the hood, there's no need to specifically override `temporarySignedRoute()`. > Note: Browsing old convos, it seems like the `temporarySignedRoute()` override was needed to make Livewire file uploads work with path identification, but it's not needed anymore. TenancyUrlGenerator had some changes since then, and now, I can't see the _exact_ reason why we needed the override (`temporarySignedRoute()` uses `route()` under the hood, so the only thing that should really matter is overriding `route()`/`toRoute()`). It was likely a leftover from some older implementation. The `route()` override got simplified. Since `route()` uses `toRoute()` under the hood, the `route()` override only has to have the prefixing logic. The rest is delegated to `toRoute()`. > Note: Even though we override `toRoute()` now which `route()` uses for generating the URLs, we still need to override `route()` for its `$this->routes->getByName($name)` call to receive the prefixed name. For example, if `route()` wasn't overridden, and we only had one route: `tenant.foo` (no central `foo` route), and we'd call `route('foo')`, we'd get an exception saying that route "foo" wasn't found, even if automatic route name prefixing was enabled and `toRoute()` was overridden. With the `route()` override, `route('foo')` acts as if we passed 'tenant.foo' instead of 'foo'. Comments in TenancyUrlGenerator and UrlGeneratorBootstrapper got updated to be more accurate. All _intentionally_ affected methods are listed in TenancyUrlGenerator's docblock. --------- Co-authored-by: Samuel Stancl --- .../UrlGeneratorBootstrapper.php | 2 +- src/Overrides/TenancyUrlGenerator.php | 71 +++++++++++-------- .../UrlGeneratorBootstrapperTest.php | 44 ++++++++++++ 3 files changed, 86 insertions(+), 31 deletions(-) diff --git a/src/Bootstrappers/UrlGeneratorBootstrapper.php b/src/Bootstrappers/UrlGeneratorBootstrapper.php index 3708d636..ba1a6d05 100644 --- a/src/Bootstrappers/UrlGeneratorBootstrapper.php +++ b/src/Bootstrappers/UrlGeneratorBootstrapper.php @@ -16,7 +16,7 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; /** * Makes the app use TenancyUrlGenerator (instead of Illuminate\Routing\UrlGenerator) which: * - prefixes route names with the tenant route name prefix (PathTenantResolver::tenantRouteNamePrefix() by default) - * - passes the tenant parameter to the link generated by route() and temporarySignedRoute() (PathTenantResolver::tenantParameterName() by default). + * - passes the tenant parameter (PathTenantResolver::tenantParameterName() by default) to the link generated by the affected methods like route() and temporarySignedRoute(). * * Used with path and query string identification. * diff --git a/src/Overrides/TenancyUrlGenerator.php b/src/Overrides/TenancyUrlGenerator.php index f7ed9a84..bcf1bd3f 100644 --- a/src/Overrides/TenancyUrlGenerator.php +++ b/src/Overrides/TenancyUrlGenerator.php @@ -22,16 +22,18 @@ use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; * - Automatically passing ['tenant' => ...] to each route() call -- if TenancyUrlGenerator::$passTenantParameterToRoutes is enabled * This is a more universal solution since it supports both path identification and query parameter identification. * - * - Prepends route names passed to route() and URL::temporarySignedRoute() - * with `tenant.` (or the configured prefix) if $prefixRouteNames is enabled. + * - Prepends route names with the tenant route name prefix ('tenant.' by default, + * configurable at tenant_route_name_prefix under PathTenantResolver) if $prefixRouteNames is enabled. * This is primarily useful when using route cloning with path identification. * - * To bypass this behavior on any single route() call, pass the $bypassParameter as true (['central' => true] by default). + * Affected methods: route(), toRoute(), temporarySignedRoute(), signedRoute() (the last two via the route() override). + * + * To bypass this behavior on any single affected method call, pass the $bypassParameter as true (['central' => true] by default). */ class TenancyUrlGenerator extends UrlGenerator { /** - * Parameter which works as a flag for bypassing the behavior modification of route() and temporarySignedRoute(). + * Parameter which works as a flag for bypassing the behavior modification of the affected methods. * * For example, in tenant context: * Route::get('/', ...)->name('home'); @@ -44,12 +46,12 @@ class TenancyUrlGenerator extends UrlGenerator * Note: UrlGeneratorBootstrapper::$addTenantParameterToDefaults is not affected by this, though * it doesn't matter since it doesn't pass any extra parameters when not needed. * - * @see UrlGeneratorBootstrapper + * @see Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper */ public static string $bypassParameter = 'central'; /** - * Should route names passed to route() or temporarySignedRoute() + * Should route names passed to the affected methods * get prefixed with the tenant route name prefix. * * This is useful when using e.g. path identification with third-party packages @@ -59,12 +61,12 @@ class TenancyUrlGenerator extends UrlGenerator public static bool $prefixRouteNames = false; /** - * Should the tenant parameter be passed to route() or temporarySignedRoute() calls. + * Should the tenant parameter be passed to the affected methods. * * This is useful with path or query parameter identification. The former can be handled * more elegantly using UrlGeneratorBootstrapper::$addTenantParameterToDefaults. * - * @see UrlGeneratorBootstrapper + * @see Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper */ public static bool $passTenantParameterToRoutes = false; @@ -105,8 +107,18 @@ class TenancyUrlGenerator extends UrlGenerator public static bool $passQueryParameter = true; /** - * Override the route() method so that the route name gets prefixed - * and the tenant parameter gets added when in tenant context. + * Override the route() method to prefix the route name before $this->routes->getByName($name) is called + * in the parent route() call. + * + * This is necessary because $this->routes->getByName($name) is called to retrieve the route + * before passing it to toRoute(). If only the prefixed route (e.g. 'tenant.foo') is registered + * and the original ('foo') isn't, route() would throw a RouteNotFoundException. + * So route() has to be overridden to prefix the passed route name, even though toRoute() is overridden already. + * + * Only the name is taken from prepareRouteInputs() here — parameter handling + * (adding tenant parameter, removing bypass parameter) is delegated to toRoute(). + * + * Affects temporarySignedRoute() and signedRoute() as well since they call route() under the hood. */ public function route($name, $parameters = [], $absolute = true) { @@ -114,32 +126,28 @@ class TenancyUrlGenerator extends UrlGenerator throw new InvalidArgumentException('Attribute [name] expects a string backed enum.'); } - [$name, $parameters] = $this->prepareRouteInputs($name, Arr::wrap($parameters)); // @phpstan-ignore argument.type + [$name] = $this->prepareRouteInputs(Arr::wrap($parameters), $name); // @phpstan-ignore argument.type return parent::route($name, $parameters, $absolute); } /** - * Override the temporarySignedRoute() method so that the route name gets prefixed - * and the tenant parameter gets added when in tenant context. + * Override the toRoute() to prefix the route name + * and add the tenant parameter when in tenant context. + * + * Also affects route(). Even though route() is overridden separately, it delegates parameter handling to toRoute(). */ - public function temporarySignedRoute($name, $expiration, $parameters = [], $absolute = true) + public function toRoute($route, $parameters, $absolute) { - if ($name instanceof BackedEnum && ! is_string($name = $name->value)) { - throw new InvalidArgumentException('Attribute [name] expects a string backed enum.'); + $name = $route->getName(); + + [$prefixedName, $parameters] = $this->prepareRouteInputs(Arr::wrap($parameters), $name); + + if ($name && $prefixedName !== $name && $tenantRoute = $this->routes->getByName($prefixedName)) { + $route = $tenantRoute; } - $wrappedParameters = Arr::wrap($parameters); - - [$name, $parameters] = $this->prepareRouteInputs($name, $wrappedParameters); // @phpstan-ignore argument.type - - if (isset($wrappedParameters[static::$bypassParameter])) { - // If the bypass parameter was passed, we need to add it back to the parameters after prepareRouteInputs() removes it, - // so that the underlying route() call in parent::temporarySignedRoute() can bypass the behavior modification as well. - $parameters[static::$bypassParameter] = $wrappedParameters[static::$bypassParameter]; - } - - return parent::temporarySignedRoute($name, $expiration, $parameters, $absolute); + return parent::toRoute($route, $parameters, $absolute); } /** @@ -155,16 +163,19 @@ class TenancyUrlGenerator extends UrlGenerator } /** - * Takes a route name and an array of parameters to return the prefixed route name + * Takes an array of parameters and a route name to return the prefixed route name * and the route parameters with the tenant parameter added. * * To skip these modifications, pass the bypass parameter in route parameters. * Before returning the modified route inputs, the bypass parameter is removed from the parameters. */ - protected function prepareRouteInputs(string $name, array $parameters): array + protected function prepareRouteInputs(array $parameters, string|null $name): array { if (! $this->routeBehaviorModificationBypassed($parameters)) { - $name = $this->routeNameOverride($name) ?? $this->prefixRouteName($name); + if (! is_null($name)) { + $name = $this->routeNameOverride($name) ?? $this->prefixRouteName($name); + } + $parameters = $this->addTenantParameter($parameters); } diff --git a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php index f089207a..18664b06 100644 --- a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php +++ b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php @@ -401,3 +401,47 @@ test('the bypass parameter works correctly with temporarySignedRoute', function( ->toContain('localhost/foo') ->not()->toContain('central='); // Bypass parameter gets removed from the generated URL }); + +test('toRoute can automatically prefix the passed route name', function () { + config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); + + Route::get('/central/home', fn () => 'central')->name('home'); + Route::get('/tenant/home', fn () => 'tenant')->name('tenant.home'); + + TenancyUrlGenerator::$prefixRouteNames = true; + + $tenant = Tenant::create(); + + tenancy()->initialize($tenant); + + $centralRoute = Route::getRoutes()->getByName('home'); + + // url()->toRoute() prefixes the name of the passed route ('home') with the tenant prefix + // and generates the URL for the tenant route (as if the 'tenant.home' route was passed to the method) + expect(url()->toRoute($centralRoute, [], true))->toBe('http://localhost/tenant/home'); + + // Passing the bypass parameter skips the name prefixing, so the method returns the central route URL + expect(url()->toRoute($centralRoute, ['central' => true], true))->toBe('http://localhost/central/home'); +}); + +test('toRoute modifies parameters even when the route has no name', function () { + config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); + + TenancyUrlGenerator::$passTenantParameterToRoutes = true; + + $unnamedRoute = Route::get('/unnamed', fn () => 'unnamed'); + + $tenant = Tenant::create(); + + tenancy()->initialize($tenant); + + // The tenant parameter is added to the URL even for unnamed routes + expect(url()->toRoute($unnamedRoute, [], true)) + ->toBe("http://localhost/unnamed?tenant={$tenant->getTenantKey()}"); + + // The bypass parameter prevents passing the tenant parameter and is stripped from the URL + expect(url()->toRoute($unnamedRoute, ['central' => true], true)) + ->toBe("http://localhost/unnamed") + ->not()->toContain('tenant=') + ->not()->toContain('central='); +});