From ab2a4d84385b2fd857cf9f8fb9e950f208898d22 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 22 Apr 2026 14:32:53 +0200 Subject: [PATCH 01/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 02/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 03/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 04/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 05/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 06/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 07/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 08/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 09/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 10/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='); +}); From 652bc987cedb19d345bcb1a948ad02a6ce40e238 Mon Sep 17 00:00:00 2001 From: Jimish Gamit Date: Mon, 8 Jun 2026 03:48:38 +0530 Subject: [PATCH 11/15] Add --skip-tenants option to HasTenantOptions (#1436) Adds a --skip-tenants option to all tenant artisan commands (`tenants:run`, `tenants:migrate`, `tenants:rollback`, `tenants:seed`, `tenants:up`, `tenants:down`). The option is the complement of the existing `--tenants` option instead of specifying which tenants to include, you specify which to exclude. --------- Co-authored-by: Jimish Gamit Co-authored-by: Samuel Stancl Co-authored-by: lukinovec --- src/Commands/Run.php | 3 +- src/Concerns/HasTenantOptions.php | 10 +++++-- tests/CommandsTest.php | 48 +++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/Commands/Run.php b/src/Commands/Run.php index 7dd69e0f..d3435ca2 100644 --- a/src/Commands/Run.php +++ b/src/Commands/Run.php @@ -17,7 +17,8 @@ class Run extends Command protected $description = 'Run a command for tenant(s)'; protected $signature = 'tenants:run {commandname : The artisan command.} - {--tenants=* : The tenant(s) to run the command for. Default: all}'; + {--tenants=* : The tenant(s) to run the command for. Default: all} + {--skip-tenants=* : The tenant(s) to skip}'; public function handle(): int { diff --git a/src/Concerns/HasTenantOptions.php b/src/Concerns/HasTenantOptions.php index 3933c469..b10d7bd4 100644 --- a/src/Concerns/HasTenantOptions.php +++ b/src/Concerns/HasTenantOptions.php @@ -10,15 +10,16 @@ use Stancl\Tenancy\Database\Concerns\PendingScope; use Symfony\Component\Console\Input\InputOption; /** - * Adds 'tenants' and 'with-pending' options. + * Adds 'tenants', 'skip-tenants', and 'with-pending' options. */ trait HasTenantOptions { protected function getOptions() { 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_OPTIONAL, 'Include pending tenants in query if true/1, exclude if false/0. Defaults to the tenancy.pending.include_in_queries config value.'), + 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('skip-tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, 'The tenants to skip when running this command', null), + 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()); } @@ -42,6 +43,9 @@ trait HasTenantOptions ->when($this->option('tenants'), function ($query) { $query->whereIn(tenancy()->model()->getTenantKeyName(), $this->option('tenants')); }) + ->when($this->option('skip-tenants'), function ($query) { + $query->whereNotIn(tenancy()->model()->getTenantKeyName(), $this->option('skip-tenants')); + }) ->when(tenancy()->model()::hasGlobalScope(PendingScope::class), function ($query) { $includePending = $this->input->hasParameterOption('--with-pending') ? filter_var($this->option('with-pending') ?? true, FILTER_VALIDATE_BOOLEAN) diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index a5b3b856..bda3eea9 100644 --- a/tests/CommandsTest.php +++ b/tests/CommandsTest.php @@ -515,3 +515,51 @@ test('migrate fresh command only deletes tenant databases if drop_tenant_databas expect($tenantHasDatabase($tenant))->toBe($shouldHaveDBAfterMigrateFresh); } })->with([true, false]); + +test('migrate commands can skip specified tenants', function (string $command) { + $tenant1 = Tenant::create(); + $tenant2 = Tenant::create(); + $tenant3 = Tenant::create(); + + pest()->artisan("{$command} --skip-tenants={$tenant1->getTenantKey()} --skip-tenants={$tenant2->getTenantKey()}"); + + tenancy()->initialize($tenant1); + + expect(Schema::hasTable('users'))->toBeFalse(); + + tenancy()->initialize($tenant2); + + expect(Schema::hasTable('users'))->toBeFalse(); + + tenancy()->initialize($tenant3); + + expect(Schema::hasTable('users'))->toBeTrue(); +})->with([ + 'tenants:migrate', + 'tenants:migrate-fresh', +]); + +test('run command can skip specified tenants', function () { + $tenant1 = Tenant::create()->getTenantKey(); + $tenant2 = Tenant::create()->getTenantKey(); + $tenant3 = Tenant::create()->getTenantKey(); + + pest()->artisan("tenants:run --skip-tenants=$tenant1 --skip-tenants=$tenant2 'bar foo foo@bar foobar arg --option=option'") + ->doesntExpectOutputToContain("Tenant: $tenant1") + ->doesntExpectOutputToContain("Tenant: $tenant2") + ->expectsOutputToContain("Tenant: $tenant3") + ->assertExitCode(0); +}); + +test('tenants and skip-tenants options can be used together', function () { + $tenant1 = Tenant::create()->getTenantKey(); + $tenant2 = Tenant::create()->getTenantKey(); + $tenant3 = Tenant::create()->getTenantKey(); + + // Scope to tenant1+tenant2, then skip tenant2 — only tenant1 should run + pest()->artisan("tenants:run --tenants=$tenant1 --tenants=$tenant2 --skip-tenants=$tenant2 'bar foo foo@bar foobar arg --option=option'") + ->expectsOutputToContain("Tenant: $tenant1") + ->doesntExpectOutputToContain("Tenant: $tenant2") + ->doesntExpectOutputToContain("Tenant: $tenant3") + ->assertExitCode(0); +}); From 04da9c896b4e948097cb414781673b720dca47f8 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 26 Jun 2026 04:51:39 +0200 Subject: [PATCH 12/15] [MINOR BC] Fix pending tenant pull race conditions (#1463) > Minor breaking change: clearing pending_since no longer fires Eloquent events, PullingPendingTenant is now fired at a different point in the lifecycle and does not guarantee the tenant will actually be pulled. `pullPendingFromPool` had a race condition when user A attempted to pull a tenant at the same time as user B. Both could end up grabbing the same tenant, and the result was unexpected, e.g. one of them ending up with no pending tenant pulled at all even though there was a pending tenant in the pool. instead of selecting a pending tenant and updating the same model, we now run `update()` conditionally -- it clears `pending_since` _only_ if the tenant is still pending, and we check the affected row count. Only one process can get a row back, the other gets 0 and retries with the next pending candidate in the pool. The loop always terminates since every lost claim means the pool shrank by one. Eventually it's empty and we create a new tenant (or return null). The claim and the attribute update happen in a single transaction now, so if updating `$attributes` fails, the claim rolls back and the tenant stays in the pool. Added a regression test that simulates a concurrent "steal" synchronously via a PullingPendingTenant listener. Fails with the old code, passes with the HasPending changes. Very minor BC: - Clearing `pending_since` no longer fires model updating/updated events (since the update goes through query builder). `PendingTenantPulled` still fires the same as before and is the listener you'd want to use anyway. - `PullingPendingTenant` now fires before the claim (and outside the transaction), so it can fire more than once with concurrent pulls (e.g. when a tenant gets claimed by someone else). `PendingTenantPulled` is still the one that fires exactly once for the actually pulled tenant. --------- Co-authored-by: Samuel Stancl --- src/Database/Concerns/HasPending.php | 58 ++++++++++++++++++++-------- tests/PendingTenantsTest.php | 47 ++++++++++++++++++++++ 2 files changed, 88 insertions(+), 17 deletions(-) diff --git a/src/Database/Concerns/HasPending.php b/src/Database/Concerns/HasPending.php index 04fcccc1..e3d8a6fb 100644 --- a/src/Database/Concerns/HasPending.php +++ b/src/Database/Concerns/HasPending.php @@ -100,27 +100,51 @@ trait HasPending */ public static function pullPendingFromPool(bool $firstOrCreate = false, array $attributes = []): ?Tenant { - $tenant = DB::transaction(function () use ($attributes): ?Tenant { - /** @var (Model&Tenant)|null $tenant */ - $tenant = static::onlyPending()->first(); + // Attempt pulling a pending tenant. + // The loop handles the case where a single tenant is being pulled by multiple processes at the same time. + // If a tenant was pulled by a concurrent process, try pulling the next one in the pool. + while (true) { + /** @var (Model&Tenant)|null $pullCandidate */ + $pullCandidate = static::onlyPending()->first(); - if ($tenant !== null) { - event(new PullingPendingTenant($tenant)); - $tenant->update(array_merge($attributes, [ - 'pending_since' => null, - ])); + if ($pullCandidate === null) { + return $firstOrCreate ? static::create($attributes) : null; } + // Fired before the claim, so it can fire once per attempt, including for a candidate + // that ends up being claimed by a different process (in which case the loop retries). + // PendingTenantPulled (below) fires exactly once, for the actually pulled tenant. + event(new PullingPendingTenant($pullCandidate)); + + $tenant = DB::transaction(function () use ($pullCandidate, $attributes): ?Tenant { + $tenantWasPulled = static::onlyPending() + ->whereKey($pullCandidate->getKey()) + ->update([$pullCandidate->getColumnForQuery('pending_since') => null]) > 0; + + if (! $tenantWasPulled) { + return null; + } + + // The tenant's pending_since was just cleared, and a PullingPendingTenant listener + // may have made changes to the tenant, so re-fetch it to make sure it's up to date. + /** @var Model&Tenant $pulledTenant */ + $pulledTenant = static::findOrFail($pullCandidate->getKey()); + + if (! empty($attributes)) { + $pulledTenant->update($attributes); + } + + return $pulledTenant; + }); + + if ($tenant === null) { + // If another pull claimed this tenant first, try claiming the next one + continue; + } + + event(new PendingTenantPulled($tenant)); + return $tenant; - }); - - if ($tenant === null) { - return $firstOrCreate ? static::create($attributes) : null; } - - // Only triggered if a tenant that was pulled from the pool is returned - event(new PendingTenantPulled($tenant)); - - return $tenant; } } diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index b04f8bc4..c9960728 100644 --- a/tests/PendingTenantsTest.php +++ b/tests/PendingTenantsTest.php @@ -5,6 +5,7 @@ declare(strict_types=1); use Illuminate\Database\QueryException; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Artisan; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Schema; use Illuminate\Support\Str; @@ -126,6 +127,52 @@ 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('pulling a pending tenant retries when the tenant is claimed concurrently', function () { + Tenant::createPending(); + Tenant::createPending(); + + $stolenId = null; + + Event::listen(PullingPendingTenant::class, function (PullingPendingTenant $event) use (&$stolenId) { + if ($stolenId !== null) { + return; + } + + $stolenId = $event->tenant->id; + + // Steal the tenant like a concurrent process would + Tenant::onlyPending() + ->whereKey($event->tenant->id) + ->update([$event->tenant->getColumnForQuery('pending_since') => null]); + }); + + $pulled = Tenant::pullPendingFromPool(); + + expect($pulled)->not()->toBeNull(); + expect($pulled->id)->not()->toBe($stolenId); // Stolen tenant was skipped, the next one was claimed by the pull + expect(Tenant::onlyPending()->count())->toBe(0); // Both tenants claimed +}); + +test('the pull is rolled back and the tenant stays in the pool if setting attributes fails', function () { + // Pulling a tenant and setting its attributes happen in one transaction, + // so if setting the attributes fails, the whole pull rolls back and the tenant stays in the pool. + Schema::table('tenants', function (Blueprint $table) { + $table->string('slug')->nullable()->unique(); + }); + + Tenant::$extraCustomColumns = ['slug']; + + Tenant::create(['slug' => 'taken']); + Tenant::createPending(); + + // During the pull, set slug to 'taken', which is already used by another tenant to make the attribute update throw + expect(fn () => Tenant::pullPendingFromPool(false, ['slug' => 'taken'])) + ->toThrow(QueryException::class); + + // The pull rolled back, so the tenant is still pending + expect(Tenant::onlyPending()->count())->toBe(1); +}); + test('withoutPending chained with where clauses returns correct results', function () { $tenant = Tenant::create(); $pendingTenant = Tenant::createPending(); From ecf031237d5eca044a54325f96cb346ba7e51bcb Mon Sep 17 00:00:00 2001 From: lukinovec Date: Sun, 28 Jun 2026 03:30:01 +0200 Subject: [PATCH 13/15] Make globalCache always use central conn with DB cache stores (#1462) `globalCache` should always use the central connection, but when using a `database`-driver cache store with `DatabaseTenancyBootstrapper`, it does not (with the exception of `DatabaseCacheBootstrapper`, explained below). `globalCache` creates a fresh `CacheManager` each time it's resolved (it's a `bind`, not a `singleton`). A freshly-created manager builds its database stores using the current default DB connection. When `DatabaseTenancyBootstrapper` is active, that default is `tenant`. So `globalCache` in tenant context points at the tenant DB. Specifically, `CachedTenantResolver` stores cached tenant lookups via `globalCache`. When a domain is deleted in tenant context, the invalidation logic calls `globalCache->forget(...)`, but that hits the tenant DB, while the resolver cache entry is in the central DB. `globalCache->forget(...)` doesn't actually do anything in that case. With `DatabaseCacheBootstrapper`, this is already handled. `globalCache` is always central because it sets `TenancyServiceProvider::$adjustCacheManagerUsing` to a callback that explicitly restores the central connection on `globalCache`'s stores. To fix this, after constructing the fresh CacheManager in the globalCache binding, explicitly set the connection of every database-driver store to its configured connection value, **falling back to central_connection** when the config value is null (null value = inherits whatever the current default DB connection is). This is sufficient for `CacheTenancyBootstrapper` and any other bootstrapper that doesn't explicitly set the store's DB connection. For `DatabaseCacheBootstrapper` specifically, this alone is not enough since it explicitly sets the store's config connection to 'tenant'. That's why DatabaseCacheBootstrapper's `$adjustCacheManagerUsing` callback runs after and overrides those stores back to the original (central) connection. > In short: `makeDatabaseCacheStoresCentral()` handles stores with a `null` connection config (falls back to central). `$adjustCacheManagerUsing` handles the `DatabaseCacheBootstrapper` case where the config is explicitly set to 'tenant'. Added datasets that use the database cache store + CacheTenancyBootstrapper to the relevant tests (globalCache and invalidation) to test regression (https://github.com/archtechx/tenancy/commit/0cf7043b733848b6d139de967df676e30247323e), and the changes mentioned above (https://github.com/archtechx/tenancy/pull/1462/commits/5e65c67ea0daf98f57f2a6a7b0e1937bbc397a56) make these tests pass. --------- Co-authored-by: Samuel Stancl --- .../CacheTenancyBootstrapper.php | 9 +++++++ .../DatabaseCacheBootstrapper.php | 8 ++++-- src/TenancyServiceProvider.php | 26 +++++++++++++++---- tests/CachedTenantResolverTest.php | 1 + tests/GlobalCacheTest.php | 1 + 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/Bootstrappers/CacheTenancyBootstrapper.php b/src/Bootstrappers/CacheTenancyBootstrapper.php index 97bd7d24..74fc8490 100644 --- a/src/Bootstrappers/CacheTenancyBootstrapper.php +++ b/src/Bootstrappers/CacheTenancyBootstrapper.php @@ -16,6 +16,15 @@ use Stancl\Tenancy\Contracts\Tenant; /** * Makes cache tenant-aware by applying a prefix. + * + * Using this bootstrapper together with DatabaseTenancyBootstrapper + * with a database cache store will result in "double scoping". The store will be scoped + * by the DB connection (entries will go into the tenant's database) *and* by the prefix. + * This is harmless in most cases, but is important to be aware of. + * + * If you only use database cache stores, consider using DatabaseCacheBootstrapper instead. + * + * @see Stancl\Tenancy\Bootstrappers\DatabaseCacheBootstrapper */ class CacheTenancyBootstrapper implements TenancyBootstrapper { diff --git a/src/Bootstrappers/DatabaseCacheBootstrapper.php b/src/Bootstrappers/DatabaseCacheBootstrapper.php index 0e41849f..81611d0a 100644 --- a/src/Bootstrappers/DatabaseCacheBootstrapper.php +++ b/src/Bootstrappers/DatabaseCacheBootstrapper.php @@ -21,11 +21,15 @@ use Stancl\Tenancy\TenancyServiceProvider; * * By default, this bootstrapper scopes ALL cache stores that use the database driver. If you only * want to scope SOME stores, set the static $stores property to an array of names of the stores - * you want to scope. These stores must use 'database' as their driver. + * you want to scope. Those stores must use 'database' as their driver. * * Notably, this bootstrapper sets TenancyServiceProvider::$adjustCacheManagerUsing to a callback * that ensures all affected stores still use the central connection when accessed via global cache - * (typicaly the GlobalCache facade or global_cache() helper). + * (typically the GlobalCache facade or global_cache() helper). The code in TenancyServiceProvider + * that uses `extend()` callbacks to make database stores on the global cache manager use the central + * connection only corrects stores scoped by the Database*Tenancy*Bootstrapper. This bootstrapper + * also changes the stores' connection in the *config* to 'tenant' which doesn't let that callback + * change the connection back to central on the global cache manager. */ class DatabaseCacheBootstrapper implements TenancyBootstrapper { diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index afd20fb6..23d1ffab 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -86,14 +86,30 @@ class TenancyServiceProvider extends ServiceProvider // This works great for cache stores that are *directly* scoped, like Redis or // any other tagged or prefixed stores, but it doesn't work for the database driver. // - // When we use the DatabaseTenancyBootstrapper, it changes the default connection, - // and therefore the connection of the database store that will be created when - // this new CacheManager is instantiated again. + // When DatabaseTenancyBootstrapper is used, it changes the default DB connection + // to 'tenant'. A freshly created CacheManager would therefore instantiate database + // stores with the tenant connection. // - // For that reason, we also adjust the relevant stores on this new CacheManager - // using the callback below. It is set by DatabaseCacheBootstrapper. + // For that reason, we override the 'database' driver creator on this manager so that + // database stores are built with the central connection, and we run the + // $adjustCacheManagerUsing callback below (set by DatabaseCacheBootstrapper). $manager = new CacheManager($app); + // When DatabaseTenancyBootstrapper is used, database stores whose 'connection' + // config is null fall back to the default DB connection ('tenant'). Reset each + // such store to its explicitly configured connection, or fall back to central. + $centralConnection = $app['config']['tenancy.database.central_connection']; + $manager->extend('database', function ($_app, array $config) use ($centralConnection) { + $config['connection'] ??= $centralConnection; + + /** @var CacheManager $this */ + return $this->createDatabaseDriver($config); // @phpstan-ignore method.protected + }); + + // DatabaseCacheBootstrapper explicitly writes 'tenant' into each store's 'connection' + // config. The extend() closure above would then read 'tenant' as the configured value + // (not null) and use it directly, so the central connection fallback wouldn't be used. + // This callback is used to correct those connections back to central for globalCache. if (static::$adjustCacheManagerUsing !== null) { (static::$adjustCacheManagerUsing)($manager); } diff --git a/tests/CachedTenantResolverTest.php b/tests/CachedTenantResolverTest.php index 920c95a1..fc6cfb79 100644 --- a/tests/CachedTenantResolverTest.php +++ b/tests/CachedTenantResolverTest.php @@ -165,6 +165,7 @@ test('cache is invalidated when tenant is updated from within the tenant context ['redis', [CacheTenancyBootstrapper::class]], ['redis', [CacheTagsBootstrapper::class]], ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, CacheTenancyBootstrapper::class]], ]); test('cache is invalidated when the tenant is deleted', function (string $resolver, bool $configureTenantModelColumn) { diff --git a/tests/GlobalCacheTest.php b/tests/GlobalCacheTest.php index 016ad2a4..4cda8b74 100644 --- a/tests/GlobalCacheTest.php +++ b/tests/GlobalCacheTest.php @@ -165,6 +165,7 @@ test('global cache is always central', function (string $store, array $bootstrap ['redis', [CacheTagsBootstrapper::class]], ['redis', [CacheTenancyBootstrapper::class]], ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, CacheTenancyBootstrapper::class]], ])->with([ 'helper', 'facade', From aa9d1d7fcf1d74ffe509163cbdec89db4cfbb1eb Mon Sep 17 00:00:00 2001 From: lukinovec Date: Sun, 28 Jun 2026 04:51:30 +0200 Subject: [PATCH 14/15] Parameter validation and other DB manager improvements (#1459) ### Parameter validation In `statement()` calls of `TenantDatabaseManager`s, use parameter binding when possible. When that's not possible, validate the parameters using `validateParameter()` or `validatePassword()`. Passwords use a less strict allowlist than other parameters (e.g. DB names), since passwords tend to use more special characters but we can afford to be more restrictive in the generic `validateParameter()`. In `SQLiteDatabaseManager`, names of file-based databases are validated (in `createDatabase`, `deleteDatabase` and `databaseExists`) using a similar allowlist to the (non-password) parameters in other DB managers, with an additional character: `.` (this addition is necessary since file-based SQLite databases end with `.sqlite`). ### DatabaseTenancyBootstrapper - harden() and the lost test file While checking for more places that could use validation, I realized that it's possible to update tenant's db_name to the central DB or the DB of another tenant. Added the `DatabaseTenancyBootstrapper::$harden` property -- setting it to true prevents tenants from connecting to the wrong databases (`RuntimeException` is thrown after connecting to the wrong database). Also, the DatabaseTenancyBootstrapper test file was ignored while running tests because it lacked the `Test` suffix. Added the suffix and fixed the broken `DATABASE_URL` test in the file. ### SQLiteDatabaseManager - respect static $path property in makeConnectionConfig() SQLiteDatabaseManager had a bug in `makeConnectionConfig`: the method didn't respect the static `$path` property, it used `database_path()` instead. Added a regression test for that. Also recognizing in-memory SQLite databases (using `isInMemory()`) is more strict now so that simply having a db_name with `_tenancy_inmemory_` somewhere in the name doesn't make a file-based database considered in-memory. ### MySQLDatabaseManager - charset and collation defaulting Creating databases with `null` charsets and collations resulted in a `QueryException`, since null isn't a valid charset/collation. To solve that, in the `CREATE DATABASE` statement in MySQLDatabaseManager, only add charset/collation to the statement if they are not null. MySQL defaults to the server's charset and collation, so it's safe to not pass any charset/collation in the `CREATE DATABASE` statement and let MySQL choose. Also, if e.g. collation is non-null and charset is null, MySQL will use a charset compatible with the used collation, and this works both ways. --------- Co-authored-by: Samuel Stancl Co-authored-by: github-actions[bot] Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../DatabaseTenancyBootstrapper.php | 57 +++++ .../Concerns/ManagesPostgresUsers.php | 7 +- .../Concerns/ValidatesDatabaseParameters.php | 96 +++++++ .../MicrosoftSQLDatabaseManager.php | 10 +- .../MySQLDatabaseManager.php | 28 ++- ...olledMicrosoftSQLServerDatabaseManager.php | 18 +- ...rmissionControlledMySQLDatabaseManager.php | 12 +- ...ionControlledPostgreSQLDatabaseManager.php | 8 +- ...ssionControlledPostgreSQLSchemaManager.php | 14 +- .../PostgreSQLDatabaseManager.php | 14 +- .../PostgreSQLSchemaManager.php | 14 +- .../SQLiteDatabaseManager.php | 42 +++- .../TenantDatabaseManager.php | 3 + .../DatabaseTenancyBootstrapper.php | 35 --- .../DatabaseTenancyBootstrapperTest.php | 162 ++++++++++++ tests/TenantDatabaseManagerTest.php | 236 ++++++++++++++++++ 16 files changed, 695 insertions(+), 61 deletions(-) create mode 100644 src/Database/Concerns/ValidatesDatabaseParameters.php delete mode 100644 tests/Bootstrappers/DatabaseTenancyBootstrapper.php create mode 100644 tests/Bootstrappers/DatabaseTenancyBootstrapperTest.php diff --git a/src/Bootstrappers/DatabaseTenancyBootstrapper.php b/src/Bootstrappers/DatabaseTenancyBootstrapper.php index 7f0bce0a..08b36f1d 100644 --- a/src/Bootstrappers/DatabaseTenancyBootstrapper.php +++ b/src/Bootstrappers/DatabaseTenancyBootstrapper.php @@ -5,14 +5,42 @@ declare(strict_types=1); namespace Stancl\Tenancy\Bootstrappers; use Exception; +use Illuminate\Support\Facades\Schema; +use RuntimeException; use Stancl\Tenancy\Contracts\TenancyBootstrapper; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Database\Contracts\TenantWithDatabase; use Stancl\Tenancy\Database\DatabaseManager; use Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException; +use Throwable; class DatabaseTenancyBootstrapper implements TenancyBootstrapper { + /** + * When true, throw an exception if a tenant gets connected to + * another tenant's database or to the central database. + * + * This case should never come up in well-configured apps where + * users cannot set or edit tenant IDs or database names, so this + * option is disabled by default. + * + * However, applications dealing with extremely sensitive data may + * choose to enable this runtime check to prevent a bug or misconfiguration + * from creating an exploit that would let an attacker access another + * tenant's data or data from the central database. + * + * One way such a scenario might come up is if an application allows + * broad tenant attribute updates on a page for updating some fields + * on the tenant, without restricting that action to only a limited + * set of fields that are safe to edit. An attacker might be able to add + * something like ['tenancy_db_name' => '...'] to the request which could + * lead to this internal attribute being updated on an existing tenant. + * + * It's possible that enabling this setting will negate the performance + * benefits of cached tenant lookup. + */ + public static bool $harden = false; + /** @var DatabaseManager */ protected $database; @@ -41,10 +69,39 @@ class DatabaseTenancyBootstrapper implements TenancyBootstrapper } $this->database->connectToTenant($tenant); + + if (static::$harden) { + try { + $this->verifyTenantCanUseDatabase($tenant); + } catch (Throwable $e) { + // Revert connection back to central + $this->revert(); + + throw $e; + } + } } public function revert(): void { $this->database->reconnectToCentral(); } + + protected function verifyTenantCanUseDatabase(Tenant $tenant): void + { + /** @var \Stancl\Tenancy\Database\Models\Tenant&TenantWithDatabase $tenant */ + $tenantDbName = $tenant->database()->getName(); + + // Check that no other tenant uses this tenant's database + if ($tenant::where($tenant->getTenantKeyName(), '!=', $tenant->getTenantKey()) + ->where($tenant::getDataColumn() . '->' . $tenant->internalPrefix() . 'db_name', $tenantDbName) + ->exists()) { + throw new RuntimeException('Tenant cannot use a database of another tenant.'); + } + + if (Schema::hasTable($tenant->getTable())) { + // Throw if the current database/schema has the tenants table (i.e. it's not central) + throw new RuntimeException('Tenant cannot use the central database.'); + } + } } diff --git a/src/Database/Concerns/ManagesPostgresUsers.php b/src/Database/Concerns/ManagesPostgresUsers.php index bec94f49..62b920b9 100644 --- a/src/Database/Concerns/ManagesPostgresUsers.php +++ b/src/Database/Concerns/ManagesPostgresUsers.php @@ -28,6 +28,9 @@ trait ManagesPostgresUsers $username = $databaseConfig->getUsername(); $password = $databaseConfig->getPassword(); + $this->validateParameter($username); + $this->validatePassword($password); + $createUser = ! $this->userExists($username); if ($createUser) { @@ -44,6 +47,8 @@ trait ManagesPostgresUsers // Tenant DB username $username = $databaseConfig->getUsername(); + $this->validateParameter($username); + // Tenant host connection config $connectionName = $this->connection()->getConfig('name'); $centralDatabase = $this->connection()->getConfig('database'); @@ -77,6 +82,6 @@ trait ManagesPostgresUsers public function userExists(string $username): bool { - return (bool) $this->connection()->selectOne("SELECT usename FROM pg_user WHERE usename = '{$username}'"); + return (bool) $this->connection()->select('SELECT usename FROM pg_user WHERE usename = ?', [$username]); } } diff --git a/src/Database/Concerns/ValidatesDatabaseParameters.php b/src/Database/Concerns/ValidatesDatabaseParameters.php new file mode 100644 index 00000000..d6d01a03 --- /dev/null +++ b/src/Database/Concerns/ValidatesDatabaseParameters.php @@ -0,0 +1,96 @@ +?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_abcdefghijklmnopqrstuvwxyz{|}~'; + + /** + * Ensure that parameter (database name, username, etc.) + * only contains allowed characters before being used in SQL statements + * (or paths in the case of SQLiteDatabaseManager). + * + * By default, only the characters in $allowedParameterCharacters are allowed. + * + * @throws InvalidArgumentException + */ + protected function validateParameter(mixed $parameter, string|null $allowedCharacters = null): void + { + if (is_null($parameter)) { + throw new InvalidArgumentException('Parameter cannot be null.'); + } + + if (is_numeric($parameter)) { + $parameter = (string) $parameter; + } + + if (! is_string($parameter)) { + throw new InvalidArgumentException('Parameter has to be a string.'); + } + + if ($parameter === '') { + throw new InvalidArgumentException('Parameter cannot be an empty string.'); + } + + $allowedCharacters ??= static::$allowedParameterCharacters; + + foreach (str_split($parameter) as $character) { + if (! str_contains($allowedCharacters, $character)) { + throw new InvalidArgumentException("Forbidden character '{$character}' in parameter."); + } + } + } + + /** + * Ensure password only contains allowed characters ($allowedPasswordCharacters) + * before being used in SQL statements. + * + * Used in permission controlled managers as a shorthand for calling validateParameter() + * with the less strict allowlist to validate database user passwords. + * + * @throws InvalidArgumentException + */ + protected function validatePassword(string|null $password): void + { + if (is_null($password)) { + throw new InvalidArgumentException('Password cannot be null.'); + } + + if ($password === '') { + throw new InvalidArgumentException('Password cannot be an empty string.'); + } + + $this->validateParameter($password, allowedCharacters: static::$allowedPasswordCharacters); + } +} diff --git a/src/Database/TenantDatabaseManagers/MicrosoftSQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/MicrosoftSQLDatabaseManager.php index da993956..f28ffd1e 100644 --- a/src/Database/TenantDatabaseManagers/MicrosoftSQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/MicrosoftSQLDatabaseManager.php @@ -12,16 +12,22 @@ class MicrosoftSQLDatabaseManager extends TenantDatabaseManager { $database = $tenant->database()->getName(); + $this->validateParameter($database); + return $this->connection()->statement("CREATE DATABASE [{$database}]"); } public function deleteDatabase(TenantWithDatabase $tenant): bool { - return $this->connection()->statement("DROP DATABASE [{$tenant->database()->getName()}]"); + $database = $tenant->database()->getName(); + + $this->validateParameter($database); + + return $this->connection()->statement("DROP DATABASE [{$database}]"); } public function databaseExists(string $name): bool { - return (bool) $this->connection()->select("SELECT name FROM master.sys.databases WHERE name = '$name'"); + return (bool) $this->connection()->select('SELECT name FROM master.sys.databases WHERE name = ?', [$name]); } } diff --git a/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php index b86faef2..10385208 100644 --- a/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php @@ -14,16 +14,38 @@ class MySQLDatabaseManager extends TenantDatabaseManager $charset = $this->connection()->getConfig('charset'); $collation = $this->connection()->getConfig('collation'); - return $this->connection()->statement("CREATE DATABASE `{$database}` CHARACTER SET `$charset` COLLATE `$collation`"); + $this->validateParameter($database); + + // MySQL defaults to the server's charset and collation + // if charset and collation are not specified. + // If charset is specified but collation is null, MySQL + // will choose a default collation for the specified charset (and vice versa). + $statement = "CREATE DATABASE `{$database}`"; + + if ($charset !== null) { + $this->validateParameter($charset); + $statement .= " CHARACTER SET `{$charset}`"; + } + + if ($collation !== null) { + $this->validateParameter($collation); + $statement .= " COLLATE `{$collation}`"; + } + + return $this->connection()->statement($statement); } public function deleteDatabase(TenantWithDatabase $tenant): bool { - return $this->connection()->statement("DROP DATABASE `{$tenant->database()->getName()}`"); + $database = $tenant->database()->getName(); + + $this->validateParameter($database); + + return $this->connection()->statement("DROP DATABASE `{$database}`"); } public function databaseExists(string $name): bool { - return (bool) $this->connection()->select("SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = '$name'"); + return (bool) $this->connection()->select('SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = ?', [$name]); } } diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php index b373f41e..b10b82bc 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php @@ -24,6 +24,10 @@ class PermissionControlledMicrosoftSQLServerDatabaseManager extends MicrosoftSQL $username = $databaseConfig->getUsername(); $password = $databaseConfig->getPassword(); + $this->validateParameter($database); + $this->validateParameter($username); + $this->validatePassword($password); + // Create login $this->connection()->statement("CREATE LOGIN [$username] WITH PASSWORD = '$password'"); @@ -37,12 +41,16 @@ class PermissionControlledMicrosoftSQLServerDatabaseManager extends MicrosoftSQL public function deleteUser(DatabaseConfig $databaseConfig): bool { - return $this->connection()->statement("DROP LOGIN [{$databaseConfig->getUsername()}]"); + $username = $databaseConfig->getUsername(); + + $this->validateParameter($username); + + return $this->connection()->statement("DROP LOGIN [{$username}]"); } public function userExists(string $username): bool { - return (bool) $this->connection()->select("SELECT sp.name as username FROM sys.server_principals sp WHERE sp.name = '{$username}'"); + return (bool) $this->connection()->select('SELECT sp.name as username FROM sys.server_principals sp WHERE sp.name = ?', [$username]); } public function makeConnectionConfig(array $baseConfig, string $databaseName): array @@ -54,11 +62,15 @@ class PermissionControlledMicrosoftSQLServerDatabaseManager extends MicrosoftSQL public function deleteDatabase(TenantWithDatabase $tenant): bool { + $name = $tenant->database()->getName(); + + $this->validateParameter($name); + // Close all connections to the database before deleting it // Set the database to SINGLE_USER mode to ensure that // No other connections are using the database while we're trying to delete it // Rollback all active transactions - $this->connection()->statement("ALTER DATABASE [{$tenant->database()->getName()}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;"); + $this->connection()->statement("ALTER DATABASE [{$name}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;"); return parent::deleteDatabase($tenant); } diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php index 47ec11a2..421b3bc3 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php @@ -25,6 +25,10 @@ class PermissionControlledMySQLDatabaseManager extends MySQLDatabaseManager impl $username = $databaseConfig->getUsername(); $password = $databaseConfig->getPassword(); + $this->validateParameter($database); + $this->validateParameter($username); + $this->validatePassword($password); + $this->connection()->statement("CREATE USER `{$username}`@`%` IDENTIFIED BY '{$password}'"); $grants = implode(', ', static::$grants); @@ -48,11 +52,15 @@ class PermissionControlledMySQLDatabaseManager extends MySQLDatabaseManager impl public function deleteUser(DatabaseConfig $databaseConfig): bool { - return $this->connection()->statement("DROP USER IF EXISTS '{$databaseConfig->getUsername()}'"); + $username = $databaseConfig->getUsername(); + + $this->validateParameter($username); + + return $this->connection()->statement("DROP USER IF EXISTS '{$username}'"); } public function userExists(string $username): bool { - return (bool) $this->connection()->select("SELECT count(*) FROM mysql.user WHERE user = '$username'")[0]->{'count(*)'}; + return (bool) $this->connection()->select('SELECT count(*) FROM mysql.user WHERE user = ?', [$username])[0]->{'count(*)'}; } } diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php index 1522234e..c846ace9 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php @@ -20,6 +20,10 @@ class PermissionControlledPostgreSQLDatabaseManager extends PostgreSQLDatabaseMa $username = $databaseConfig->getUsername(); $schema = $databaseConfig->connection()['search_path']; + $this->validateParameter($database); + $this->validateParameter($username); + $this->validateParameter($schema); + // Host config $connectionName = $this->connection()->getConfig('name'); $centralDatabase = $this->connection()->getConfig('database'); @@ -32,10 +36,10 @@ class PermissionControlledPostgreSQLDatabaseManager extends PostgreSQLDatabaseMa $this->connection()->reconnect(); // Grant permissions to create and use tables in the configured schema ("public" by default) to the user - $this->connection()->statement("GRANT USAGE, CREATE ON SCHEMA {$schema} TO \"{$username}\""); + $this->connection()->statement("GRANT USAGE, CREATE ON SCHEMA \"{$schema}\" TO \"{$username}\""); // Grant permissions to use sequences in the current schema to the user - $this->connection()->statement("GRANT USAGE ON ALL SEQUENCES IN SCHEMA {$schema} TO \"{$username}\""); + $this->connection()->statement("GRANT USAGE ON ALL SEQUENCES IN SCHEMA \"{$schema}\" TO \"{$username}\""); // Reconnect to central database config(["database.connections.{$connectionName}.database" => $centralDatabase]); diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php index b528d4e3..b972ba0b 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php @@ -23,23 +23,27 @@ class PermissionControlledPostgreSQLSchemaManager extends PostgreSQLSchemaManage // Central database name $database = DB::connection(config('tenancy.database.central_connection'))->getDatabaseName(); - $this->connection()->statement("GRANT CONNECT ON DATABASE {$database} TO \"{$username}\""); + $this->validateParameter($username); + $this->validateParameter($schema); + $this->validateParameter($database); + + $this->connection()->statement("GRANT CONNECT ON DATABASE \"{$database}\" TO \"{$username}\""); $this->connection()->statement("GRANT USAGE, CREATE ON SCHEMA \"{$schema}\" TO \"{$username}\""); $this->connection()->statement("GRANT USAGE ON ALL SEQUENCES IN SCHEMA \"{$schema}\" TO \"{$username}\""); - $tables = $this->connection()->select("SELECT table_name FROM information_schema.tables WHERE table_schema = '{$schema}' AND table_type = 'BASE TABLE'"); + $tables = $this->connection()->select("SELECT table_name FROM information_schema.tables WHERE table_schema = ? AND table_type = 'BASE TABLE'", [$schema]); // Grant permissions to any existing tables. This is used with RLS foreach ($tables as $table) { $tableName = $table->table_name; /** @var string $primaryKey */ - $primaryKey = $this->connection()->selectOne(<<connection()->selectOne(<<<'SQL' SELECT column_name FROM information_schema.key_column_usage - WHERE table_name = '{$tableName}' + WHERE table_name = ? AND constraint_name LIKE '%_pkey' - SQL)->column_name; + SQL, [$tableName])->column_name; // Grant all permissions for all existing tables $this->connection()->statement("GRANT ALL ON \"{$schema}\".\"{$tableName}\" TO \"{$username}\""); diff --git a/src/Database/TenantDatabaseManagers/PostgreSQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PostgreSQLDatabaseManager.php index 4fff7202..fc293403 100644 --- a/src/Database/TenantDatabaseManagers/PostgreSQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PostgreSQLDatabaseManager.php @@ -10,16 +10,24 @@ class PostgreSQLDatabaseManager extends TenantDatabaseManager { public function createDatabase(TenantWithDatabase $tenant): bool { - return $this->connection()->statement("CREATE DATABASE \"{$tenant->database()->getName()}\" WITH TEMPLATE=template0"); + $name = $tenant->database()->getName(); + + $this->validateParameter($name); + + return $this->connection()->statement("CREATE DATABASE \"{$name}\" WITH TEMPLATE=template0"); } public function deleteDatabase(TenantWithDatabase $tenant): bool { - return $this->connection()->statement("DROP DATABASE \"{$tenant->database()->getName()}\""); + $name = $tenant->database()->getName(); + + $this->validateParameter($name); + + return $this->connection()->statement("DROP DATABASE \"{$name}\""); } public function databaseExists(string $name): bool { - return (bool) $this->connection()->selectOne("SELECT datname FROM pg_database WHERE datname = '$name'"); + return (bool) $this->connection()->select('SELECT datname FROM pg_database WHERE datname = ?', [$name]); } } diff --git a/src/Database/TenantDatabaseManagers/PostgreSQLSchemaManager.php b/src/Database/TenantDatabaseManagers/PostgreSQLSchemaManager.php index d0fb0337..354eb768 100644 --- a/src/Database/TenantDatabaseManagers/PostgreSQLSchemaManager.php +++ b/src/Database/TenantDatabaseManagers/PostgreSQLSchemaManager.php @@ -10,17 +10,25 @@ class PostgreSQLSchemaManager extends TenantDatabaseManager { public function createDatabase(TenantWithDatabase $tenant): bool { - return $this->connection()->statement("CREATE SCHEMA \"{$tenant->database()->getName()}\""); + $name = $tenant->database()->getName(); + + $this->validateParameter($name); + + return $this->connection()->statement("CREATE SCHEMA \"{$name}\""); } public function deleteDatabase(TenantWithDatabase $tenant): bool { - return $this->connection()->statement("DROP SCHEMA \"{$tenant->database()->getName()}\" CASCADE"); + $name = $tenant->database()->getName(); + + $this->validateParameter($name); + + return $this->connection()->statement("DROP SCHEMA \"{$name}\" CASCADE"); } public function databaseExists(string $name): bool { - return (bool) $this->connection()->select("SELECT schema_name FROM information_schema.schemata WHERE schema_name = '$name'"); + return (bool) $this->connection()->select('SELECT schema_name FROM information_schema.schemata WHERE schema_name = ?', [$name]); } public function makeConnectionConfig(array $baseConfig, string $databaseName): array diff --git a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php index 295cf304..ce3582c0 100644 --- a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php @@ -6,13 +6,17 @@ namespace Stancl\Tenancy\Database\TenantDatabaseManagers; use Closure; use Illuminate\Database\Eloquent\Model; +use InvalidArgumentException; use PDO; +use Stancl\Tenancy\Database\Concerns\ValidatesDatabaseParameters; use Stancl\Tenancy\Database\Contracts\TenantDatabaseManager; use Stancl\Tenancy\Database\Contracts\TenantWithDatabase; use Throwable; class SQLiteDatabaseManager implements TenantDatabaseManager { + use ValidatesDatabaseParameters; + /** * SQLite database directory path. * @@ -57,6 +61,13 @@ class SQLiteDatabaseManager implements TenantDatabaseManager */ public static Closure|null $closeInMemoryConnectionUsing = null; + /** + * Characters allowed in database names. + * + * Includes dots to support file extensions (e.g. '.sqlite'). + */ + public static string $allowedDatabaseNameCharacters = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-.'; + public function createDatabase(TenantWithDatabase $tenant): bool { /** @var TenantWithDatabase&Model $tenant */ @@ -122,6 +133,9 @@ class SQLiteDatabaseManager implements TenantDatabaseManager public function makeConnectionConfig(array $baseConfig, string $databaseName): array { if ($this->isInMemory($databaseName)) { + // Named in-memory DBs are formatted like 'file:_tenancy_inmemory_tenant123?mode=memory&cache=shared' + $this->validateDatabaseName($databaseName, extraAllowedCharacters: ':?=&'); + $baseConfig['database'] = $databaseName; if (static::$persistInMemoryConnectionUsing !== null) { @@ -129,7 +143,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager (static::$persistInMemoryConnectionUsing)(new PDO($dsn), $dsn); } } else { - $baseConfig['database'] = database_path($databaseName); + $baseConfig['database'] = $this->getPath($databaseName); } return $baseConfig; @@ -137,6 +151,8 @@ class SQLiteDatabaseManager implements TenantDatabaseManager public function getPath(string $name): string { + $this->validateDatabaseName($name); + if (static::$path) { return rtrim(static::$path, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . $name; } @@ -146,6 +162,28 @@ class SQLiteDatabaseManager implements TenantDatabaseManager public static function isInMemory(string $name): bool { - return $name === ':memory:' || str_contains($name, '_tenancy_inmemory_'); + $isNamed = str_starts_with($name, 'file:_tenancy_inmemory_') && + str_ends_with($name, '?mode=memory&cache=shared'); + + return $name === ':memory:' || $isNamed; + } + + /** + * Ensure database name only contains allowed characters + * (allowedDatabaseNameCharacters() + $extraAllowedCharacters) and is not a directory name. + * + * @throws InvalidArgumentException + */ + protected function validateDatabaseName(string $name, string $extraAllowedCharacters = ''): void + { + $this->validateParameter($name, static::$allowedDatabaseNameCharacters . $extraAllowedCharacters); + + if ($name === '') { + throw new InvalidArgumentException('Database name cannot be empty.'); + } + + if (is_dir($name)) { + throw new InvalidArgumentException('Database name cannot be a directory.'); + } } } diff --git a/src/Database/TenantDatabaseManagers/TenantDatabaseManager.php b/src/Database/TenantDatabaseManagers/TenantDatabaseManager.php index 3d8d7610..a0822615 100644 --- a/src/Database/TenantDatabaseManagers/TenantDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/TenantDatabaseManager.php @@ -6,11 +6,14 @@ namespace Stancl\Tenancy\Database\TenantDatabaseManagers; use Illuminate\Database\Connection; use Illuminate\Support\Facades\DB; +use Stancl\Tenancy\Database\Concerns\ValidatesDatabaseParameters; use Stancl\Tenancy\Database\Contracts\StatefulTenantDatabaseManager; use Stancl\Tenancy\Database\Exceptions\NoConnectionSetException; abstract class TenantDatabaseManager implements StatefulTenantDatabaseManager { + use ValidatesDatabaseParameters; + /** The database connection to the server. */ protected string $connection; diff --git a/tests/Bootstrappers/DatabaseTenancyBootstrapper.php b/tests/Bootstrappers/DatabaseTenancyBootstrapper.php deleted file mode 100644 index 14109500..00000000 --- a/tests/Bootstrappers/DatabaseTenancyBootstrapper.php +++ /dev/null @@ -1,35 +0,0 @@ - $databaseUrl]); - - pest()->expectException(Exception::class); - } - - config(['tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class]]); - - $tenant1 = Tenant::create(); - - pest()->artisan('tenants:migrate'); - - tenancy()->initialize($tenant1); - - expect(true)->toBe(true); -})->with(['abc.us-east-1.rds.amazonaws.com', null]); - diff --git a/tests/Bootstrappers/DatabaseTenancyBootstrapperTest.php b/tests/Bootstrappers/DatabaseTenancyBootstrapperTest.php new file mode 100644 index 00000000..63762672 --- /dev/null +++ b/tests/Bootstrappers/DatabaseTenancyBootstrapperTest.php @@ -0,0 +1,162 @@ + [DatabaseTenancyBootstrapper::class], + "tenancy.database.managers.{$connection}" => $manager, + ]); + + // Point the central connection at the tested connection's config and migrate it + // (so that the central database/schema contains the tenants table). + $centralConnection = config('tenancy.database.central_connection'); + $centralConfig = config("database.connections.{$connection}"); + + if ($connection === 'sqlite') { + $centralConfig['database'] = database_path($sqliteCentralDb = 'central.sqlite'); + } + + DB::purge($centralConnection); + config(["database.connections.{$centralConnection}" => $centralConfig]); + + pest()->artisan('migrate:fresh', [ + '--force' => true, + '--path' => __DIR__ . '/../../assets/migrations', + '--realpath' => true, + ]); + + DatabaseTenancyBootstrapper::$harden = $harden; + + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + // Create the tenant with its own database, then repoint it at the central database/schema + // (which contains the tenants table that the hardening check looks for). + $tenant = Tenant::create(['tenancy_db_connection' => $connection]); + + $central = DB::connection($centralConnection); + $centralName = match (true) { + $manager === PostgreSQLSchemaManager::class => $central->selectOne('SELECT current_schema() AS schema')->schema, // Central schema name + $connection === 'sqlite' => $sqliteCentralDb, // Central SQLite DB name + default => $central->getDatabaseName(), // Central DB name + }; + + $tenant->update(['tenancy_db_name' => $centralName]); + + if ($harden) { + // Harden blocks initialization for tenants that use the central database + expect(fn () => tenancy()->initialize($tenant))->toThrow(RuntimeException::class); + + // Connection should be reverted back to central + expect(DB::connection()->getName())->toBe($centralConnection); + } else { + expect(fn () => tenancy()->initialize($tenant))->not()->toThrow(Throwable::class); + + // Connection not reverted to central + expect(DB::connection()->getName())->toBe('tenant'); + } +})->with([ + 'hardening enabled' => true, + 'hardening disabled' => false, +])->with('db_managers'); + +test('harden prevents tenants from using the database of another tenant', function (bool $harden, string $connection, string $manager) { + config([ + 'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class], + "tenancy.database.managers.{$connection}" => $manager, + ]); + + DatabaseTenancyBootstrapper::$harden = $harden; + + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + $tenant = Tenant::create(['tenancy_db_connection' => $connection]); + + $dbName = Str::random(8) . ($connection === 'sqlite' ? '.sqlite' : ''); + + Tenant::create(['tenancy_db_name' => $dbName, 'tenancy_db_connection' => $connection]); + + $tenant->update(['tenancy_db_name' => $dbName]); + + if ($harden) { + // Harden blocks initialization for tenants that use the database of another tenant + expect(fn () => tenancy()->initialize($tenant))->toThrow(RuntimeException::class); + + // Connection should be reverted back to central + expect(DB::connection()->getName())->toBe('central'); + } else { + expect(fn() => tenancy()->initialize($tenant))->not()->toThrow(Throwable::class); + + // Connection not reverted to central + expect(DB::connection()->getName())->toBe('tenant'); + } +})->with([ + 'hardening enabled' => true, + 'hardening disabled' => false, +])->with('db_managers'); + +test('database tenancy bootstrapper throws an exception if DATABASE_URL is set', function (string|null $databaseUrl) { + config(['database.connections.central.url' => $databaseUrl]); + + config(['tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class]]); + + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + if ($databaseUrl) { + expect(fn() => Tenant::create())->toThrow(QueryException::class); + } else { + expect(function() { + $tenant1 = Tenant::create(); + + pest()->artisan('tenants:migrate'); + + tenancy()->initialize($tenant1); + })->not()->toThrow(Throwable::class); + } +})->with(['abc.us-east-1.rds.amazonaws.com', null]); + +// Database managers to test with hardening. +// Permission controlled managers omitted as they inherit the non-perm controlled managers (= they share the same code paths), +// each important code path is covered by testing the non-permission controlled manager, so adding permission controlled managers +// would add unnecessary complexity to the tests. +dataset('db_managers', [ + 'mysql' => ['mysql', MySQLDatabaseManager::class], + 'pgsql (database)' => ['pgsql', PostgreSQLDatabaseManager::class], + 'pgsql (schema)' => ['pgsql', PostgreSQLSchemaManager::class], + 'sqlite' => ['sqlite', SQLiteDatabaseManager::class], +]); diff --git a/tests/TenantDatabaseManagerTest.php b/tests/TenantDatabaseManagerTest.php index 0d83e70e..d95cb1f0 100644 --- a/tests/TenantDatabaseManagerTest.php +++ b/tests/TenantDatabaseManagerTest.php @@ -17,6 +17,7 @@ use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; +use Stancl\Tenancy\Database\Contracts\ManagesDatabaseUsers; use Stancl\Tenancy\Database\Contracts\StatefulTenantDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\SQLiteDatabaseManager; @@ -36,6 +37,10 @@ beforeEach(function () { SQLiteDatabaseManager::$path = null; }); +afterEach(function () { + SQLiteDatabaseManager::$path = null; +}); + test('databases can be created and deleted', function ($driver, $databaseManager) { Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { return $event->tenant; @@ -539,6 +544,237 @@ test('partial tenant connection templates get merged into the central connection expect($manager->connection()->getConfig('url'))->toBeNull(); }); +test('database managers validate parameters used in raw sql statements', function ($driver, $databaseManager) { + config()->set([ + "tenancy.database.template_tenant_connection" => $driver, + ]); + + $manager = app($databaseManager); + + if ($manager instanceof StatefulTenantDatabaseManager) { + $manager->setConnection($driver); + } + + $invalidDatabaseName = "\"database_with_quotes\""; + + if (! ($manager instanceof ManagesDatabaseUsers)) { + // Only test createDatabase() and deleteDatabase() with non-permission controlled managers here + // since permission controlled managers override these methods to e.g. delete users before + // calling parent::deleteDatabase(), and with invalid DB name, the user deletion will already + // fail before we even get to actual deleteDatabase() logic. + $tenant = Tenant::make([ + 'tenancy_db_name' => $invalidDatabaseName, + ]); + + expect(fn () => $manager->createDatabase($tenant)) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + expect(fn () => $manager->deleteDatabase($tenant)) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + } else { + // Invalid username, createUser() and deleteUser() should + // throw an invalid argument exception. + $tenantWithInvalidUsername = Tenant::make([ + 'tenancy_db_name' => 'valid_database_name890', + 'tenancy_db_username' => "username with spaces", + ]); + + expect(fn () => $manager->createUser($tenantWithInvalidUsername->database())) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + expect(fn () => $manager->deleteUser($tenantWithInvalidUsername->database())) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + // Invalid database name, createUser() should throw + // an invalid argument exception. deleteUser() doesn't + // validate the DB name (it only validates the username). + $tenantWithInvalidDatabase = Tenant::make([ + 'tenancy_db_name' => $invalidDatabaseName, + 'tenancy_db_username' => 'valid_USERNAME', + 'tenancy_db_password' => 'valid_password', + ]); + + expect(fn () => $manager->createUser($tenantWithInvalidDatabase->database())) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + $tenantWithInvalidPassword = Tenant::make([ + 'tenancy_db_name' => 'valid_database_name890', + 'tenancy_db_username' => 'valid_USERNAME', + 'tenancy_db_password' => "p'ssword", + ]); + + expect(fn () => $manager->createUser($tenantWithInvalidPassword->database())) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + // Special characters are allowed in passwords + $tenantWithValidPassword = Tenant::make([ + 'tenancy_db_name' => 'valid_database_name890' . Str::random(8), + 'tenancy_db_username' => 'valid_USERNAME' . Str::random(8), + 'tenancy_db_password' => "]pa$$ ;word", + ]); + + expect(fn () => $manager->createUser($tenantWithValidPassword->database())) + ->not()->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + $tenantWithNullCredentials = Tenant::make([ + 'tenancy_db_name' => 'valid_db_name', + 'tenancy_db_username' => null, + 'tenancy_db_password' => null, + ]); + + expect(fn () => $manager->createUser($tenantWithNullCredentials->database())) + ->toThrow(InvalidArgumentException::class, 'Parameter cannot be null.'); + } +})->with('database_managers'); + +test('mysql database manager validates charset and collation correctly', function (string $param) { + $manager = app(MySQLDatabaseManager::class); + $manager->setConnection('mysql'); + + // using a non-string value (empty array) which is invalid + config(["database.connections.mysql.$param" => []]); + DB::purge('mysql'); + + $tenant = Tenant::make([ + 'tenancy_db_name' => 'valid_db_name', + ]); + + expect(fn () => $manager->createDatabase($tenant)) + ->toThrow(InvalidArgumentException::class, 'Parameter has to be a string.'); +})->with(['charset', 'collation']); + +test('sqlite database manager validates database names correctly', function () { + $manager = app(SQLiteDatabaseManager::class); + + // Dots are allowed in database names + expect(fn () => $manager->databaseExists('valid-db_name.sqlite')) + ->not()->toThrow(InvalidArgumentException::class); + + // Directory names are considered invalid input for database names + expect(fn () => $manager->databaseExists('..')) + ->toThrow(InvalidArgumentException::class); + + // Empty strings are considered invalid input for database names + expect(fn () => $manager->databaseExists('')) + ->toThrow(InvalidArgumentException::class); +}); + +test('sqlite database manager recognizes inmemory databases correctly', function () { + $manager = app(SQLiteDatabaseManager::class); + + expect($manager->isInMemory('file:_tenancy_inmemory_123?mode=memory&cache=shared'))->toBeTrue(); + expect($manager->isInMemory(':memory:'))->toBeTrue(); + + // Missing the '?mode=memory&cache=shared' suffix + expect($manager->isInMemory('file:_tenancy_inmemory_456'))->toBeFalse(); + + // Doesn't start with 'file:_tenancy_inmemory_' + expect($manager->isInMemory('_tenancy_inmemory_123?mode=memory&cache=shared'))->toBeFalse(); + + // In-memory DB name is validated correctly in makeConnectionConfig() + expect(fn () => $manager->makeConnectionConfig([], 'file:_tenancy_inmemory_12"3?mode=memory&cache=shared')) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + expect(fn () => $manager->makeConnectionConfig([], 'file:_tenancy_inmemory_123?mode=memory&cache=shared')) + ->not()->toThrow(InvalidArgumentException::class); + + expect(fn () => $manager->makeConnectionConfig([], ':memory:')) + ->not()->toThrow(InvalidArgumentException::class); +}); + +test('sqlite database manager respects the configured path while making the database config', function () { + config()->set([ + 'tenancy.database.template_tenant_connection' => 'sqlite', + ]); + + $tenant = Tenant::make([ + 'tenancy_db_name' => 'tenant.sqlite', + ]); + + // SQLiteDatabaseManager::$path is null, the database path is built using database_path() + expect($tenant->database()->connection()['database'])->toBe(database_path('tenant.sqlite')); + + SQLiteDatabaseManager::$path = '/custom/path/'; + + expect($tenant->database()->connection()['database'])->toBe('/custom/path/tenant.sqlite'); +}); + +test('newly created tenant databases use the correct charset and collation with mysql', function () { + config([ + 'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class], + 'database.connections.mysql.charset' => 'utf8mb4', + 'database.connections.mysql.collation' => 'utf8mb4_unicode_ci', + ]); + + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + withBootstrapping(); + + $serverDefaultCharset = DB::selectOne('SELECT @@character_set_server AS charset')->charset; + $serverDefaultCollation = DB::selectOne('SELECT @@collation_server AS collation')->collation; + + $databaseCharset = fn () => DB::selectOne('SELECT DEFAULT_CHARACTER_SET_NAME FROM information_schema.SCHEMATA WHERE SCHEMA_NAME = DATABASE()')->DEFAULT_CHARACTER_SET_NAME; + $databaseCollation = fn () => DB::selectOne('SELECT DEFAULT_COLLATION_NAME FROM information_schema.SCHEMATA WHERE SCHEMA_NAME = DATABASE()')->DEFAULT_COLLATION_NAME; + + $defaultTenant = Tenant::create(); + + tenancy()->initialize($defaultTenant); + + // No charset or collation specified, + // defaults from the MySQL config used. + expect($databaseCharset())->toBe('utf8mb4'); + expect($databaseCollation())->toBe('utf8mb4_unicode_ci'); + + $tenantWithCharsetAndCollation = Tenant::create([ + 'tenancy_db_charset' => 'latin1', + 'tenancy_db_collation' => 'latin1_swedish_ci', + ]); + + tenancy()->initialize($tenantWithCharsetAndCollation); + + // Custom charset and collation from tenant config + expect($databaseCharset())->toBe('latin1'); + expect($databaseCollation())->toBe('latin1_swedish_ci'); + + $tenantWithNullCharsetAndCollation = Tenant::create([ + 'tenancy_db_charset' => null, + 'tenancy_db_collation' => null, + ]); + + tenancy()->initialize($tenantWithNullCharsetAndCollation); + + // Default MySQL server charset and collation + // (e.g. charset = utf8mb4, collation = utf8mb4_0900_ai_ci) + expect($databaseCharset())->toBe($serverDefaultCharset); + expect($databaseCollation())->toBe($serverDefaultCollation); + + $tenantWithCharsetAndNullCollation = Tenant::create([ + 'tenancy_db_charset' => 'binary', + 'tenancy_db_collation' => null, + ]); + + tenancy()->initialize($tenantWithCharsetAndNullCollation); + + // Charset specified, collation is null, + // MySQL will choose a default collation for the specified charset. + expect($databaseCharset())->toBe('binary'); + expect($databaseCollation())->toBe('binary'); + + // Collation specified, charset is null, + // MySQL will choose a default charset for the specified collation. + $tenantWithCollationAndNullCharset = Tenant::create([ + 'tenancy_db_charset' => null, + 'tenancy_db_collation' => 'latin1_swedish_ci', + ]); + + tenancy()->initialize($tenantWithCollationAndNullCharset); + + expect($databaseCharset())->toBe('latin1'); + expect($databaseCollation())->toBe('latin1_swedish_ci'); +}); + // Datasets dataset('database_managers', [ ['mysql', MySQLDatabaseManager::class], From df4be2e060de9cd9c6c05e2dfbd803a6c86b027a Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 29 Jun 2026 04:04:02 +0200 Subject: [PATCH 15/15] migrate-fresh: show migration output when verbose (#1464) Resubmission of #1369 (by @lordofthebrain), changes adapted to v4. Also added a test (passes with the MigrateFresh changes, fails without them). --------- Co-authored-by: lordofthebrain Co-authored-by: Samuel Stancl --- src/Commands/MigrateFresh.php | 7 +++++-- tests/CommandsTest.php | 30 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/Commands/MigrateFresh.php b/src/Commands/MigrateFresh.php index d4733552..e53d6a89 100644 --- a/src/Commands/MigrateFresh.php +++ b/src/Commands/MigrateFresh.php @@ -14,6 +14,7 @@ use Stancl\Tenancy\Concerns\ParallelCommand; use Stancl\Tenancy\Database\Contracts\TenantWithDatabase; use Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException; use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Console\Output\OutputInterface as OI; class MigrateFresh extends BaseCommand @@ -72,11 +73,13 @@ class MigrateFresh extends BaseCommand protected function migrateTenant(TenantWithDatabase $tenant): bool { - return $this->callSilently('tenants:migrate', [ + $output = $this->getOutput()->isVerbose() ? $this->output : new NullOutput; + + return $this->runCommand('tenants:migrate', [ '--tenants' => [$tenant->getTenantKey()], '--step' => $this->option('step'), '--force' => true, - ]) === 0; + ], $output) === 0; } protected function childHandle(mixed ...$args): bool diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index bda3eea9..faa897bf 100644 --- a/tests/CommandsTest.php +++ b/tests/CommandsTest.php @@ -366,6 +366,36 @@ test('migrate fresh command works', function () { expect(DB::table('users')->exists())->toBeFalse(); }); +test('migrate fresh command only shows migration output when run with the verbose option', function () { + $tenant = Tenant::create(); + $migratingOutput = 'Migrating tenant ' . $tenant->getTenantKey(); + + // CI runs pest with --verbose which makes Artisan::call() inherit the verbosity + // so we cannot easily test commands without -v. To work around that, we temporarily + // override $_ENV['SHELL_VERBOSITY'] immediately before executing the command. If this + // ever stops working, try also overriding the value in $_SERVER and putenv(). + $emptySentinel = new \stdClass(); + $originalVerbosity = $_ENV['SHELL_VERBOSITY'] ?? $emptySentinel; + try { + $_ENV['SHELL_VERBOSITY'] = 0; + Artisan::call('tenants:migrate-fresh'); + $defaultOutput = Artisan::output(); + } finally { + if ($originalVerbosity === $emptySentinel) { + unset($_ENV['SHELL_VERBOSITY']); + } else { + $_ENV['SHELL_VERBOSITY'] = $originalVerbosity; + } + } + + Artisan::call('tenants:migrate-fresh -v'); + $verboseOutput = Artisan::output(); + + // The output is silent by default and only shown with the verbose option + expect($defaultOutput)->not()->toContain($migratingOutput); + expect($verboseOutput)->toContain($migratingOutput); +}); + test('migrate fresh command respects force option in production', function () { // Set environment to production app()->detectEnvironment(fn() => 'production');