From 105446210a5469ff172c74911d3e2097ae15805c Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Thu, 27 Aug 2026 22:51:19 -0500 Subject: [PATCH 1/9] Pin the tenant password algorithm with tests In the separate-database separation mode a tenant's database password is never stored: it is recomputed from the website on every connection. If the algorithm, its inputs, or the way those inputs become a string ever change, every existing tenant loses access to its database at once, and reverting the code does not help unless the old algorithm comes back with it. Five tests pin that down. Two freeze the exact hash for both branches, the keyed one and the legacy fallback for installations that never set tenancy.key. The others assert determinism, that every input feeds the hash, and that created_at does. That last one documents a sharp edge rather than endorsing it. The timestamp reaches the hash through sprintf, so anything changing how it renders as a string invalidates the password of every affected tenant. Carbon 2 and Carbon 3 were compared directly on this point and both render "2023-05-14 09:31:07", so the Carbon 3 requirement arriving with Laravel 12 does not break existing tenants. All inputs are synthetic. No real key or tenant data belongs in a test. --- .../Generators/PasswordGeneratorTest.php | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 tests/unit-tests/Generators/PasswordGeneratorTest.php diff --git a/tests/unit-tests/Generators/PasswordGeneratorTest.php b/tests/unit-tests/Generators/PasswordGeneratorTest.php new file mode 100644 index 00000000..41b543f8 --- /dev/null +++ b/tests/unit-tests/Generators/PasswordGeneratorTest.php @@ -0,0 +1,154 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + * + * @see https://tenancy.dev + * @see https://github.com/hyn/multi-tenant + */ + +namespace Hyn\Tenancy\Tests\Generators; + +use Hyn\Tenancy\Contracts\Database\PasswordGenerator; +use Hyn\Tenancy\Models\Website; +use Hyn\Tenancy\Tests\Test; +use Illuminate\Support\Carbon; + +/** + * Frozen-value tests for the tenant database password generator. + * + * In the separate database division mode the password is never stored, but + * recomputed from the website on every connection. Any change to the algorithm + * or its inputs locks every existing tenant out of its database at once. + * + * The expected hashes are the contract, not whatever the code returns today. + * Should one of these fail, find out what changed rather than updating it. + * + * All inputs are synthetic. No production key or tenant data belongs here. + */ +class PasswordGeneratorTest extends Test +{ + /** + * A fixed key, unrelated to any real application key. + */ + private const KEY = 'base64:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA='; + + private const WEBSITE_ID = 7; + private const WEBSITE_UUID = '3c0f0a1e2b4d5e6f7a8b9c0d1e2f3a4b'; + private const WEBSITE_CREATED_AT = '2023-05-14 09:31:07'; + + /** + * @test + */ + public function keyed_algorithm_is_frozen() + { + config(['tenancy.key' => self::KEY]); + + $this->assertSame( + 'e2794f90e67aad228c91f7e3b10ba3ec', + app(PasswordGenerator::class)->generate($this->website()), + 'The keyed password algorithm changed. Every existing tenant just ' + . 'lost access to its database. Do not update this expectation.' + ); + } + + /** + * @test + */ + public function legacy_algorithm_is_frozen() + { + // With no tenancy.key configured the generator falls back to hashing + // the application key and the website id. Installations created before + // tenancy.key existed still depend on this branch. + config(['tenancy.key' => null, 'app.key' => self::KEY]); + + $this->assertSame( + 'fad6af9d31d525bb5de0df001a9c7bf9', + app(PasswordGenerator::class)->generate($this->website()), + 'The legacy password algorithm changed. Installations that never ' + . 'set tenancy.key just lost access to every tenant database.' + ); + } + + /** + * @test + */ + public function generation_is_deterministic() + { + config(['tenancy.key' => self::KEY]); + + $generator = app(PasswordGenerator::class); + + $this->assertSame( + $generator->generate($this->website()), + $generator->generate($this->website()), + 'The generator must return the same password for the same website ' + . 'every time; the value is never stored, only recomputed.' + ); + } + + /** + * @test + */ + public function created_at_is_part_of_the_hash() + { + // Documents a sharp edge rather than endorsing it: the timestamp is an + // input, so anything that changes how it is rendered as a string — + // a date cast, a serialisation format, a data migration touching + // created_at — invalidates the password of every affected tenant. + config(['tenancy.key' => self::KEY]); + + $generator = app(PasswordGenerator::class); + + $moved = $this->website(); + $moved->created_at = Carbon::parse('2023-05-14 09:31:08'); + + $this->assertNotSame( + $generator->generate($this->website()), + $generator->generate($moved), + 'created_at feeds the hash, so a one second difference must change ' + . 'the password. If this ever stops being true the algorithm changed.' + ); + } + + /** + * @test + */ + public function every_input_changes_the_hash() + { + config(['tenancy.key' => self::KEY]); + + $generator = app(PasswordGenerator::class); + $base = $generator->generate($this->website()); + + $otherId = $this->website(); + $otherId->id = self::WEBSITE_ID + 1; + + $otherUuid = $this->website(); + $otherUuid->uuid = str_repeat('f', 32); + + $this->assertNotSame($base, $generator->generate($otherId), 'id must feed the hash'); + $this->assertNotSame($base, $generator->generate($otherUuid), 'uuid must feed the hash'); + + config(['tenancy.key' => self::KEY . 'x']); + $this->assertNotSame($base, $generator->generate($this->website()), 'the key must feed the hash'); + } + + /** + * A website that is never persisted, so the values stay fixed. + */ + private function website(): Website + { + $website = new Website(); + $website->id = self::WEBSITE_ID; + $website->uuid = self::WEBSITE_UUID; + $website->created_at = Carbon::parse(self::WEBSITE_CREATED_AT); + + return $website; + } +} From 204f9b9d40997369e80cc195ce9187f763bbe63c Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Thu, 27 Aug 2026 23:01:33 -0500 Subject: [PATCH 2/9] Clear the connection configuration when a tenant is released Connection::purge() clears both the open connection and the stored configuration. Connection::set() does not: called with null it closes the connection but leaves the previous tenant's database, username and password in config('database.connections.tenant'). Nothing is open at that point, which is why it looks harmless. It is not. Any model using UsesTenantConnection asks for that connection unconditionally, so Laravel opens a fresh one from the leftover configuration and lands in the previous tenant's database. TenantAwareConnection is unaffected because it checks exists() first, but UsesTenantConnection, which is what applications use at scale, has no such guard. Listeners\Database\ConnectsTenants passes whatever a Websites event carries straight into set(), and that can be null. set() now delegates to purge() when there is no website. Two supporting additions. Environment::forgetTenant(), because there was no way to say "no tenant is active": tenant(null) reads as releasing one but only returns the current tenant. And Events\Websites\Forgotten as the counterpart to Switched, so listeners holding per-tenant state can tear it down. Ten isolation tests come with it. They assert on the exact rows visible and name the tenant whose data appeared, because a failure here means one customer can read another's records. The last of them aims a connection straight at another tenant's database and requires the server to refuse the rows. It asserts the outcome rather than the mechanism, because engines differ: MySQL and MariaDB reject the connection, since a tenant's user holds privileges on its own database only, while PostgreSQL admits it and refuses at the table instead, as every role may CONNECT to a new database by default. Both are acceptable; returning rows is not. --- src/Database/Connection.php | 13 + src/Environment.php | 18 ++ src/Events/Websites/Forgotten.php | 27 ++ src/Listeners/Database/ConnectsTenants.php | 16 ++ tests/traits/InteractsWithIsolation.php | 129 +++++++++ .../Isolation/ConnectionIsolationTest.php | 266 ++++++++++++++++++ 6 files changed, 469 insertions(+) create mode 100644 src/Events/Websites/Forgotten.php create mode 100644 tests/traits/InteractsWithIsolation.php create mode 100644 tests/unit-tests/Isolation/ConnectionIsolationTest.php diff --git a/src/Database/Connection.php b/src/Database/Connection.php index 04526c7e..3118872a 100644 --- a/src/Database/Connection.php +++ b/src/Database/Connection.php @@ -143,6 +143,19 @@ public function set($to, $connection = null): bool $website = $this->convertWebsiteOrHostnameToWebsite($to); + if (! $website) { + // Closing the connection is not enough: the previous tenant's + // credentials would stay in config, ready for the next model + // asking for the tenant connection to reopen them. + $this->purge($connection); + + $this->emitEvent( + new Events\Database\ConnectionSet(null, $connection) + ); + + return true; + } + $existing = $this->configuration($connection); if ($website) { diff --git a/src/Environment.php b/src/Environment.php index 6de81300..da4cc339 100644 --- a/src/Environment.php +++ b/src/Environment.php @@ -130,6 +130,24 @@ public function tenant(Website $website = null): ?Website return $this->app->make(Tenant::class); } + /** + * Release the active tenant, so that none is active. + * + * tenant(null) reads as releasing one but returns whatever is active + * instead, leaving no way to say that there is none. + */ + public function forgetTenant(): void + { + $empty = function () { + return null; + }; + + $this->app->forgetInstance(Tenant::class); + $this->app->singleton(Tenant::class, $empty); + + $this->emitEvent(new Events\Websites\Forgotten()); + } + protected function defaults() { $empty = function () { diff --git a/src/Events/Websites/Forgotten.php b/src/Events/Websites/Forgotten.php new file mode 100644 index 00000000..b77fd358 --- /dev/null +++ b/src/Events/Websites/Forgotten.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + * + * @see https://tenancy.dev + * @see https://github.com/hyn/multi-tenant + */ + +namespace Hyn\Tenancy\Events\Websites; + +use Hyn\Tenancy\Abstracts\AbstractEvent; + +/** + * The active tenant has been released and none is active any more. + * + * The counterpart to Switched, for listeners to tear down whatever they set up + * there. It carries no website, since the point is that there is none. + */ +class Forgotten extends AbstractEvent +{ +} diff --git a/src/Listeners/Database/ConnectsTenants.php b/src/Listeners/Database/ConnectsTenants.php index 1fc03b1a..5aec770b 100644 --- a/src/Listeners/Database/ConnectsTenants.php +++ b/src/Listeners/Database/ConnectsTenants.php @@ -38,6 +38,7 @@ public function subscribe(Dispatcher $events) { $events->listen(Events\Websites\Identified::class, [$this, 'switch']); $events->listen(Events\Websites\Switched::class, [$this, 'switch']); + $events->listen(Events\Websites\Forgotten::class, [$this, 'forget']); } /** @@ -50,4 +51,19 @@ public function switch(WebsiteEvent $event) : bool { return $this->connection->set($event->website); } + + /** + * Reacts when the active tenant is released. + * + * purge() clears the stored configuration as well as the open connection, + * which set(null) does not. + * + * @return bool + */ + public function forget(): bool + { + $this->connection->purge(); + + return true; + } } diff --git a/tests/traits/InteractsWithIsolation.php b/tests/traits/InteractsWithIsolation.php new file mode 100644 index 00000000..31f8130f --- /dev/null +++ b/tests/traits/InteractsWithIsolation.php @@ -0,0 +1,129 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + * + * @see https://tenancy.dev + * @see https://github.com/hyn/multi-tenant + */ + +namespace Hyn\Tenancy\Tests\Traits; + +use Hyn\Tenancy\Contracts\Website; +use Hyn\Tenancy\Environment; +use Hyn\Tenancy\Tests\Extend\TenantExtend; + +/** + * Fixtures for the isolation tests. + * + * Two tenants are provisioned, each with the sample table migrated and one row + * carrying a marker of its own. Any query returning another number of rows, or + * the wrong marker, is a leak. + */ +trait InteractsWithIsolation +{ + /** @var Website */ + protected $tenantA; + + /** @var Website */ + protected $tenantB; + + protected const MARKER_A = 'belongs-to-tenant-a'; + + protected const MARKER_B = 'belongs-to-tenant-b'; + + /** + * Provision two tenants, each holding one identifiable row. + */ + protected function setUpIsolation(): void + { + $this->tenantA = $this->createIsolatedTenant(self::MARKER_A); + $this->tenantB = $this->createIsolatedTenant(self::MARKER_B); + + // Leave nothing active, so a test that forgets to switch fails loudly + // instead of quietly inheriting whichever tenant was created last. + $this->releaseTenant(); + } + + protected function createIsolatedTenant(string $marker): Website + { + $website = new \Hyn\Tenancy\Models\Website(); + $this->websites->create($website); + + $this->connection->migrate($website, __DIR__.'/../migrations'); + + $this->connection->set($website); + $this->writeMarker($marker); + $this->connection->purge(); + + return $website; + } + + /** + * Write a marker row on whichever tenant connection is active. + * + * TenantExtend declares no fillable attributes, hence the direct + * assignment. + */ + protected function writeMarker(string $marker): void + { + $row = new TenantExtend(); + $row->name = $marker; + $row->save(); + } + + /** + * Run a callback with the given tenant active, then release it. + */ + protected function asTenant(Website $website, callable $callback) + { + $this->connection->set($website); + + try { + return $callback(); + } finally { + $this->connection->purge(); + } + } + + /** + * Drop any active tenant, both the connection and the resolved instance. + */ + protected function releaseTenant(): void + { + $this->connection->purge(); + app(Environment::class)->tenant(null); + } + + /** + * The markers visible on the tenant connection right now. + */ + protected function visibleMarkers(): array + { + return TenantExtend::query()->orderBy('name')->pluck('name')->all(); + } + + /** + * Assert that exactly the expected tenant's data is reachable. + * + * Reports the markers that were visible, so a leak names the tenant whose + * rows appeared. + */ + protected function assertOnlySees(string $marker, string $context = ''): void + { + $seen = $this->visibleMarkers(); + $where = $context === '' ? '' : " ({$context})"; + + $this->assertSame( + [$marker], + $seen, + "TENANT ISOLATION FAILURE{$where}: expected to see only [{$marker}], " + ."saw [".implode(', ', $seen).'].' + ); + } +} diff --git a/tests/unit-tests/Isolation/ConnectionIsolationTest.php b/tests/unit-tests/Isolation/ConnectionIsolationTest.php new file mode 100644 index 00000000..bd913896 --- /dev/null +++ b/tests/unit-tests/Isolation/ConnectionIsolationTest.php @@ -0,0 +1,266 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + * + * @see https://tenancy.dev + * @see https://github.com/hyn/multi-tenant + */ + +namespace Hyn\Tenancy\Tests\Isolation; + +use Hyn\Tenancy\Tests\Extend\AwareExtend; +use Hyn\Tenancy\Tests\Test; +use Hyn\Tenancy\Tests\Traits\InteractsWithIsolation; +use Illuminate\Contracts\Foundation\Application; + +/** + * Whether one tenant's data can ever be reached while another is active. + * + * The assertions name the tenant whose data leaked rather than report a + * mismatched count, since a failure here means one customer reading another's. + */ +class ConnectionIsolationTest extends Test +{ + use InteractsWithIsolation; + + protected function duringSetUp(Application $app) + { + $this->setUpHostnames(true); + $this->setUpIsolation(); + } + + /** + * @test + */ + public function switching_between_tenants_shows_only_that_tenant() + { + $this->asTenant($this->tenantA, fn () => $this->assertOnlySees(self::MARKER_A, 'first switch to A')); + $this->asTenant($this->tenantB, fn () => $this->assertOnlySees(self::MARKER_B, 'switch to B')); + $this->asTenant($this->tenantA, fn () => $this->assertOnlySees(self::MARKER_A, 'switch back to A')); + } + + /** + * @test + */ + public function switching_without_an_explicit_purge_still_isolates() + { + // Connection::set() is expected to purge and reconnect on its own. If + // it relies on callers remembering to purge first, this fails. + $this->connection->set($this->tenantA); + $this->assertOnlySees(self::MARKER_A, 'set A without purging'); + + $this->connection->set($this->tenantB); + $this->assertOnlySees(self::MARKER_B, 'set B directly after A, no purge in between'); + + $this->connection->purge(); + } + + /** + * @test + */ + public function releasing_the_tenant_does_not_leave_the_previous_one_reachable() + { + // The vector this exists for: after the active tenant is released, a + // model that always asks for the tenant connection must not quietly + // reconnect using the previous tenant's credentials. + $this->connection->set($this->tenantA); + $this->assertOnlySees(self::MARKER_A, 'A is active'); + + $this->releaseTenant(); + + $leaked = null; + + try { + $leaked = $this->visibleMarkers(); + } catch (\Throwable $e) { + // Refusing to connect is the correct outcome: no tenant is active, + // so there is nothing legitimate to read. + $this->assertTrue(true); + + return; + } + + $this->fail( + 'TENANT ISOLATION FAILURE: after releasing the tenant, a query on the ' + .'tenant connection still returned ['.implode(', ', $leaked).']. ' + .'The connection kept the previous tenant\'s configuration.' + ); + } + + /** + * @test + */ + public function setting_a_null_tenant_does_not_leave_the_previous_configuration_armed() + { + // ConnectsTenants calls Connection::set() with whatever a Websites + // event carries, and that can be null. Unlike purge(), set(null) closes + // the connection but leaves the tenant's credentials in config, from + // which the next model asking for the connection reopens them. + $this->connection->set($this->tenantA); + $this->assertOnlySees(self::MARKER_A, 'A is active'); + + $this->connection->set(null); + + $configured = $this->connection->configuration(); + + $this->assertSame( + [], + $configured, + 'TENANT ISOLATION FAILURE: after set(null) the tenant connection is ' + .'still configured for '.($configured['database'] ?? 'a previous tenant') + .'. Any model using UsesTenantConnection will reconnect straight into it.' + ); + } + + /** + * @test + */ + public function reading_after_a_null_tenant_never_returns_the_previous_tenants_rows() + { + // The same defect stated as consequence rather than as state: what a + // query actually returns once no tenant is meant to be active. + $this->connection->set($this->tenantA); + $this->assertOnlySees(self::MARKER_A, 'A is active'); + + $this->connection->set(null); + + try { + $seen = $this->visibleMarkers(); + } catch (\Throwable $e) { + // Refusing to connect is the correct outcome. + $this->assertTrue(true); + + return; + } + + $this->fail( + 'TENANT ISOLATION FAILURE: with no tenant active, a query on the tenant ' + .'connection returned ['.implode(', ', $seen).']. set(null) left the ' + ."previous tenant's credentials in place." + ); + } + + /** + * @test + */ + public function a_tenant_aware_model_falls_back_to_system_rather_than_the_last_tenant() + { + $this->connection->set($this->tenantA); + $this->assertSame($this->connection->tenantName(), (new AwareExtend())->getConnectionName()); + + $this->releaseTenant(); + + $this->assertSame( + $this->connection->systemName(), + (new AwareExtend())->getConnectionName(), + 'With no tenant active a tenant-aware model must use the system connection, ' + .'never the connection left behind by the previous tenant.' + ); + } + + /** + * @test + */ + public function a_system_model_ignores_whichever_tenant_is_active() + { + $websites = $this->connection->systemName(); + + $this->asTenant($this->tenantA, function () use ($websites) { + $this->assertSame($websites, (new \Hyn\Tenancy\Models\Website())->getConnectionName()); + }); + + $this->asTenant($this->tenantB, function () use ($websites) { + $this->assertSame($websites, (new \Hyn\Tenancy\Models\Website())->getConnectionName()); + }); + } + + /** + * @test + */ + public function writes_land_in_the_active_tenant_only() + { + $this->asTenant($this->tenantA, function () { + $this->writeMarker('written-while-a-was-active'); + }); + + $this->asTenant($this->tenantB, function () { + $this->assertOnlySees(self::MARKER_B, 'B must not see a row written while A was active'); + }); + + $this->asTenant($this->tenantA, function () { + $this->assertSame( + ['belongs-to-tenant-a', 'written-while-a-was-active'], + $this->visibleMarkers(), + 'The row written while A was active should be in A.' + ); + }); + } + + /** + * @test + */ + public function each_tenant_gets_its_own_database_and_credentials() + { + $a = $this->connection->generateConfigurationArray($this->tenantA); + $b = $this->connection->generateConfigurationArray($this->tenantB); + + $this->assertNotSame($a['database'], $b['database'], 'Tenants must not share a database name.'); + $this->assertNotSame($a['username'], $b['username'], 'Tenants must not share a database user.'); + $this->assertNotSame($a['password'], $b['password'], 'Tenants must not share a database password.'); + $this->assertSame($this->tenantA->uuid, $a['uuid']); + $this->assertSame($this->tenantB->uuid, $b['uuid']); + } + + /** + * @test + */ + public function one_tenant_cannot_read_another_tenants_data_even_with_its_own_credentials() + { + // The last line of defence: with a connection aimed straight at + // another tenant's database, the server itself must refuse the rows. + // + // How it refuses differs by engine, and only the outcome is asserted + // here. MySQL and MariaDB reject the connection outright, because a + // tenant's user is granted privileges on its own database only. + // PostgreSQL lets the connection through, since every role may CONNECT + // to a new database by default, and refuses at the table instead. Both + // are acceptable; returning rows is not. + $a = $this->connection->generateConfigurationArray($this->tenantA); + $b = $this->connection->generateConfigurationArray($this->tenantB); + + $crossed = $a; + $crossed['database'] = $b['database']; + $crossed['search_path'] = $b['search_path'] ?? null; + + config(['database.connections.crossed-tenant' => $crossed]); + + $rows = null; + + try { + $rows = app('db')->connection('crossed-tenant') + ->table('samples') + ->pluck('name') + ->all(); + } catch (\Throwable $e) { + // Refused, at whichever layer. That is the point. + $this->assertTrue(true); + + return; + } finally { + app('db')->purge('crossed-tenant'); + } + + $this->fail( + "TENANT ISOLATION FAILURE: tenant A's credentials read [" + .implode(', ', $rows)."] out of tenant B's database ({$b['database']}). " + .'The database user has wider grants than the separate-database ' + .'division mode assumes.' + ); + } +} From 7bc6d43d838a96f1b09264bfb85b1b95d92de49a Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Thu, 27 Aug 2026 22:51:43 -0500 Subject: [PATCH 3/9] Scope the active tenant to the job being processed A queue worker is a long-lived process handling one job after another in the same memory, so anything a job leaves active is inherited by whatever runs next. QueueProvider activated a tenant when a payload carried website_id and did nothing otherwise, which produced four distinct leaks: - a job with no website_id inherited the previous job's tenant - a finished job left the worker holding that customer's connection - a job that threw left its tenant active for the next one - a job naming a deleted website inherited whichever tenant ran before it The last is the sharpest: findById returns null, nothing is activated, and the previous tenant simply stays. This one fails silently, unlike the rest. Each tenant has its own database user, so a misrouted query normally hits an access denied error. Here the inherited connection is a valid, authenticated connection to a real tenant's database: every query succeeds, nothing is logged, and the only evidence is data ending up in the wrong customer's records. The tenant is now released when a job declares none, and restored to whatever was active beforehand when a job ends or fails. Restoring rather than clearing matters: under dispatch_sync the job runs inside a request that already has its own tenant, and wiping it would break the caller rather than protect it. The previous tenants are kept on a stack, since a synchronous job can dispatch another. Inline execution is left alone. It dies with the request that asked for it, so there is no next job to inherit anything, and DispatcherMiddleware deliberately switches the ambient tenant there. Behind tenancy.queue.reset-tenant-between-jobs, defaulting to the safe value, because this changes behaviour for every installation: jobs that relied on an ambient tenant now fail with a connection error rather than touch the wrong database. UPGRADE.md covers what to do and the escape hatch. --- UPGRADE.md | 63 ++++++ assets/configs/tenancy.php | 19 ++ src/Providers/Tenants/QueueProvider.php | 130 +++++++++-- .../Isolation/QueueIsolationTest.php | 205 ++++++++++++++++++ 4 files changed, 404 insertions(+), 13 deletions(-) create mode 100644 UPGRADE.md create mode 100644 tests/unit-tests/Isolation/QueueIsolationTest.php diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 00000000..96d89a45 --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,63 @@ +# Upgrading + +## Unreleased + +### The queue no longer carries a tenant between jobs ⚠️ behaviour change + +**What changed.** A queue worker used to keep whatever tenant the previous job +activated. A job with no `website_id` inherited it, a finished job left it +active, a job that threw left it active, and a job naming a deleted website +inherited whichever tenant ran before it. + +Now the active tenant is scoped to the job: it is released when a job declares +none, and restored to whatever was active beforehand when a job ends or fails. + +**How this can affect you.** Jobs that quietly relied on inheriting an ambient +tenant will start failing with a connection error instead of reading and +writing another tenant's database. That is the point of the change, but it does +mean previously silent behaviour becomes a visible failure. + +**What to do.** Make sure every job that touches tenant data is dispatched from +within that tenant's context, so its payload carries `website_id`. Jobs that +belong to the system should not touch models using `UsesTenantConnection`. + +**Escape hatch.** `TENANCY_QUEUE_RESET_TENANT=false` restores the old +behaviour while you adapt. It is not recommended: the old behaviour leaks +tenant context between jobs, on a connection that is valid and authenticated, +so nothing errors and nothing is logged. + +**Not affected.** Synchronous dispatch. `dispatch_sync` runs inside the request +that asked for it, and `DispatcherMiddleware` deliberately switches the ambient +tenant there. That behaviour is unchanged. + +### Connection::set(null) now clears the connection configuration + +**What changed.** Releasing the tenant with `Connection::set(null)` used to +close the connection but leave the previous tenant's database, user and +password in `config('database.connections.tenant')`. The next model using +`UsesTenantConnection` reopened it straight into that tenant's database. It now +purges the configuration as well. + +**What to do.** Nothing, unless you relied on the connection configuration +surviving a release, which was never safe. + +### New: Environment::forgetTenant() + +There was no supported way to say "no tenant is active". +`Environment::tenant(null)` reads as releasing one but does nothing: the null +branch simply returns whatever is currently active. `forgetTenant()` releases +it and emits `Events\Websites\Forgotten`. + +### New: Events\Websites\Forgotten + +The counterpart to `Websites\Switched`. Listeners that set up per-tenant state +when a tenant becomes active should tear it down here. It carries no website on +purpose: the point is that there is not one. + +### New configuration key + +```php +'queue' => [ + 'reset-tenant-between-jobs' => env('TENANCY_QUEUE_RESET_TENANT', true), +], +``` diff --git a/assets/configs/tenancy.php b/assets/configs/tenancy.php index 828418ce..c82ff433 100644 --- a/assets/configs/tenancy.php +++ b/assets/configs/tenancy.php @@ -166,6 +166,25 @@ */ 'update-app-url' => false, ], + 'queue' => [ + /** + * Release the active tenant when a job declares none, and after every + * job ends or fails. + * + * A queue worker is a long-lived process handling one job after + * another in the same memory. Without this, a job with no website_id + * runs against whichever tenant went before it, on a connection that + * is valid and authenticated: nothing errors, nothing is logged, and + * the only evidence is data landing in the wrong tenant's database. + * + * @warn Turning this off restores the behaviour of hyn/multi-tenant + * 5.9 and earlier, which leaks tenant context between jobs. + * Only do so as a temporary measure while adapting jobs that + * relied on inheriting an ambient tenant. + */ + 'reset-tenant-between-jobs' => env('TENANCY_QUEUE_RESET_TENANT', true), + ], + 'db' => [ /** * The default connection to use; this overrules the Laravel database.default diff --git a/src/Providers/Tenants/QueueProvider.php b/src/Providers/Tenants/QueueProvider.php index 1b6f01b8..2b31b4bd 100644 --- a/src/Providers/Tenants/QueueProvider.php +++ b/src/Providers/Tenants/QueueProvider.php @@ -16,12 +16,15 @@ use Hyn\Tenancy\Contracts\Repositories\WebsiteRepository; use Hyn\Tenancy\Environment; +use Hyn\Tenancy\Queue\DispatcherMiddleware; +use Illuminate\Contracts\Bus\Dispatcher; +use Illuminate\Queue\Events\JobExceptionOccurred; +use Illuminate\Queue\Events\JobFailed; +use Illuminate\Queue\Events\JobProcessed; use Illuminate\Queue\Events\JobProcessing; use Illuminate\Queue\QueueManager; -use Illuminate\Support\ServiceProvider; use Illuminate\Support\Arr; -use Hyn\Tenancy\Queue\DispatcherMiddleware; -use Illuminate\Contracts\Bus\Dispatcher; +use Illuminate\Support\ServiceProvider; class QueueProvider extends ServiceProvider { @@ -29,7 +32,7 @@ public function boot() { $this->app->booted(function () { $this->app->extend('queue', function (QueueManager $queue) { - $queue->createPayloadUsing(function (string $connection, string $queue = null, array $payload = []) { + $queue->createPayloadUsing(function (string $connection, ?string $queue = null, array $payload = []) { /** @var Environment $environment */ $environment = resolve(Environment::class); @@ -43,19 +46,120 @@ public function boot() }); }); - $this->app['events']->listen(JobProcessing::class, function ($event) { - if ($key = Arr::get($event->job->payload(), 'website_id')) { - /** @var Environment $environment */ - $environment = resolve(Environment::class); - /** @var WebsiteRepository $repository */ - $repository = resolve(WebsiteRepository::class); + $this->bindTenantToJobLifecycle(); + + $this->app->make(Dispatcher::class)->pipeThrough([DispatcherMiddleware::class]); + } + + /** + * Tenants active before each job, restored once it ends. + * + * A worker hands one job after another to the same process, so a tenant + * left active is inherited by whatever runs next. A stack rather than a + * single value, since a synchronous job can dispatch another. + * + * @var array + */ + protected $tenantStack = []; + + protected function bindTenantToJobLifecycle(): void + { + $events = $this->app['events']; + + $events->listen(JobProcessing::class, function (JobProcessing $event) { + if ($this->runsInline($event->connectionName)) { + return; + } + + $environment = resolve(Environment::class); + + $this->tenantStack[] = $environment->tenant(); + + $key = Arr::get($event->job->payload(), 'website_id'); + + if (! $key) { + // A job that declares no tenant is a system job. It must not + // inherit the previous one. + $this->releaseTenant($environment); + + return; + } + + $tenant = resolve(WebsiteRepository::class)->findById($key); - $tenant = $repository->findById($key); + if (! $tenant) { + // Deleted while the job sat in the queue. Resolving nothing + // must not mean keeping whatever was active. + $this->releaseTenant($environment); - $environment->tenant($tenant); + return; } + + $environment->tenant($tenant); }); - $this->app->make(Dispatcher::class)->pipeThrough([DispatcherMiddleware::class]); + // An idle worker should hold no customer's connection. + foreach ([JobProcessed::class, JobFailed::class, JobExceptionOccurred::class] as $event) { + $events->listen($event, function ($event) { + if ($this->runsInline($event->connectionName)) { + return; + } + + $this->restoreTenant(); + }); + } + } + + /** + * Whether the job runs inline rather than on a worker. + * + * A synchronous dispatch dies with the request that asked for it, so no + * next job inherits anything. It also has deliberate semantics of its own: + * DispatcherMiddleware switches the ambient tenant and the suite asserts it + * stays switched, which restoring here would undo. + */ + protected function runsInline(?string $connection): bool + { + return $connection === 'sync'; + } + + /** + * Put back whatever tenant was active before the job ran. + */ + protected function restoreTenant(): void + { + if (! config('tenancy.queue.reset-tenant-between-jobs', true)) { + array_pop($this->tenantStack); + + return; + } + + $previous = array_pop($this->tenantStack); + + $environment = resolve(Environment::class); + + if ($previous) { + $environment->tenant($previous); + + return; + } + + $environment->forgetTenant(); + } + + /** + * Release the active tenant, unless an application has opted out. + * + * Jobs that relied on an ambient tenant start failing with a connection + * error rather than reaching the wrong database, so the old behaviour stays + * available while an installation adapts. + */ + protected function releaseTenant(Environment $environment): void + { + if (! config('tenancy.queue.reset-tenant-between-jobs', true)) { + return; + } + + $environment->forgetTenant(); } } diff --git a/tests/unit-tests/Isolation/QueueIsolationTest.php b/tests/unit-tests/Isolation/QueueIsolationTest.php new file mode 100644 index 00000000..06f033bc --- /dev/null +++ b/tests/unit-tests/Isolation/QueueIsolationTest.php @@ -0,0 +1,205 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + * + * @see https://tenancy.dev + * @see https://github.com/hyn/multi-tenant + */ + +namespace Hyn\Tenancy\Tests\Isolation; + +use Hyn\Tenancy\Environment; +use Hyn\Tenancy\Tests\Test; +use Hyn\Tenancy\Tests\Traits\InteractsWithIsolation; +use Illuminate\Contracts\Foundation\Application; +use Illuminate\Contracts\Queue\Job; +use Illuminate\Queue\Events\JobExceptionOccurred; +use Illuminate\Queue\Events\JobProcessed; +use Illuminate\Queue\Events\JobProcessing; +use Mockery; + +/** + * Tenant isolation across jobs sharing a queue worker. + * + * A worker handles one job after another in the same memory, with nothing + * tearing the state down in between as a request would. + * + * These drive the JobProcessing event, the path a worker takes. The + * dispatch_sync tests exercise the bus middleware, a different one. + */ +class QueueIsolationTest extends Test +{ + use InteractsWithIsolation; + + protected function duringSetUp(Application $app) + { + $this->setUpHostnames(true); + $this->setUpIsolation(); + } + + protected function tearDown(): void + { + Mockery::close(); + + parent::tearDown(); + } + + /** + * Simulate a worker picking up a job, exactly as QueueProvider sees it. + */ + private function workerPicksUp(?int $websiteId): Job + { + $payload = ['job' => 'stub', 'data' => []]; + + if ($websiteId !== null) { + $payload['website_id'] = $websiteId; + } + + $job = Mockery::mock(Job::class); + $job->shouldReceive('payload')->andReturn($payload); + $job->shouldReceive('getConnectionName')->andReturn('database'); + $job->shouldReceive('resolveName')->andReturn('StubJob'); + + event(new JobProcessing('database', $job)); + + return $job; + } + + private function workerFinishes(Job $job): void + { + event(new JobProcessed('database', $job)); + } + + /** + * @test + */ + public function a_job_carrying_a_website_id_activates_that_tenant() + { + $this->workerPicksUp($this->tenantA->id); + + $this->assertOnlySees(self::MARKER_A, 'job declared website_id of tenant A'); + } + + /** + * @test + */ + public function consecutive_tenant_jobs_do_not_bleed_into_each_other() + { + $this->workerPicksUp($this->tenantA->id); + $this->assertOnlySees(self::MARKER_A, 'first job, tenant A'); + + $this->workerPicksUp($this->tenantB->id); + $this->assertOnlySees(self::MARKER_B, 'second job, tenant B, same worker'); + + $this->workerPicksUp($this->tenantA->id); + $this->assertOnlySees(self::MARKER_A, 'third job, back to tenant A'); + } + + /** + * @test + */ + public function a_system_job_after_a_tenant_job_does_not_inherit_the_tenant() + { + // A job with no website_id is a system job: a scheduled command, a + // cleanup task. With the tenant still active it reads and writes the + // previous customer's database. + $this->workerPicksUp($this->tenantA->id); + $this->assertOnlySees(self::MARKER_A, 'tenant job ran first'); + + $this->workerPicksUp(null); + + $this->assertNull( + app(Environment::class)->tenant(), + 'TENANT ISOLATION FAILURE: a job with no website_id was picked up while ' + .'tenant '.$this->tenantA->uuid.' was still active. Whatever that job ' + .'touches lands in the previous tenant\'s database.' + ); + } + + /** + * @test + */ + public function a_system_job_after_a_tenant_job_cannot_read_the_tenants_rows() + { + // The same defect stated as consequence rather than as state. + $this->workerPicksUp($this->tenantA->id); + $this->workerPicksUp(null); + + try { + $seen = $this->visibleMarkers(); + } catch (\Throwable $e) { + // Refusing to connect is correct: no tenant should be active. + $this->assertTrue(true); + + return; + } + + $this->fail( + 'TENANT ISOLATION FAILURE: a job with no website_id read [' + .implode(', ', $seen).'] from the tenant connection left behind by ' + .'the previous job.' + ); + } + + /** + * @test + */ + public function a_finished_job_releases_its_tenant() + { + // A worker that has finished a job holds no customer's connection while + // it waits for the next one. + $job = $this->workerPicksUp($this->tenantA->id); + $this->assertOnlySees(self::MARKER_A, 'job running'); + + $this->workerFinishes($job); + + $this->assertNull( + app(Environment::class)->tenant(), + 'TENANT ISOLATION FAILURE: the worker still holds tenant ' + .$this->tenantA->uuid.' after the job finished. An idle worker ' + .'should hold no tenant at all.' + ); + } + + /** + * @test + */ + public function a_job_that_throws_releases_its_tenant() + { + // Failure paths are where cleanup gets forgotten, and a worker keeps + // going after a job throws. + $job = $this->workerPicksUp($this->tenantA->id); + + event(new JobExceptionOccurred('database', $job, new \RuntimeException('boom'))); + + $this->assertNull( + app(Environment::class)->tenant(), + 'TENANT ISOLATION FAILURE: tenant '.$this->tenantA->uuid.' is still ' + .'active after the job threw. The next job inherits it.' + ); + } + + /** + * @test + */ + public function a_job_naming_a_website_that_no_longer_exists_does_not_inherit_the_previous_tenant() + { + // Websites get deleted while their jobs are still queued. Resolving + // nothing must not silently mean "keep whatever was active". + $this->workerPicksUp($this->tenantA->id); + + $this->workerPicksUp(999999); + + $this->assertNull( + app(Environment::class)->tenant(), + 'TENANT ISOLATION FAILURE: a job referencing a website that does not ' + .'exist left tenant '.$this->tenantA->uuid.' active.' + ); + } +} From 5d714ef40dba903df7623cb5979dbef121a7d1d1 Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Fri, 28 Aug 2026 05:51:18 -0500 Subject: [PATCH 4/9] Provision PostgreSQL 15 and newer correctly PostgreSQL 15 revoked the CREATE privilege the public schema granted to every role. Privileges on the database no longer cover it, so a tenant database was created and then failed every migration with "permission denied for schema public". Upstream only ever tested against 14. A schema grant only applies inside its own database, so provisioning now opens a connection to the new one to issue it. It goes to PUBLIC rather than to the tenant role: a role level grant is recorded as a dependency in pg_shdepend, and DROP USER then fails when the tenant is deleted. CONNECT is revoked from PUBLIC on the new database so that stays a narrowing. Until now any role could connect to any tenant database; rows were refused, the shape of the schema was not. --- UPGRADE.md | 39 +++++++++++++++++++ .../Webserver/Database/Drivers/PostgreSQL.php | 37 +++++++++++++++++- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/UPGRADE.md b/UPGRADE.md index 96d89a45..0a442af8 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -30,6 +30,45 @@ so nothing errors and nothing is logged. that asked for it, and `DispatcherMiddleware` deliberately switches the ambient tenant there. That behaviour is unchanged. +### PostgreSQL 15 and newer can provision tenants again ⚠️ behaviour change + +**What changed.** PostgreSQL 15 revoked the `CREATE` privilege that the `public` +schema used to hand to every role. The driver only ever granted privileges on +the *database*, which on 15 and newer is no longer enough to create a table, so +a tenant database was created successfully and then failed every migration with +`permission denied for schema public`. Provisioning now also grants on the +schema, from a connection to the new database, because a schema grant has no +effect from anywhere else. + +Two things follow from that grant. It is issued to `PUBLIC` rather than to the +tenant role, since a role level grant is recorded as a dependency and would make +`DROP USER` fail when the tenant is deleted. And to keep that from widening +anything, `CONNECT` is now revoked from `PUBLIC` on each new tenant database. + +**How this can affect you.** Any role that used to reach a tenant database +purely through the default `PUBLIC` connect privilege will be refused on +databases created from now on. Reading rows was already refused, but listing +tables was not. Roles that were granted access explicitly, the tenant itself, +and the owner of the database are unaffected. + +Backup, monitoring or reporting roles are the ones to check. Grant them what +they need explicitly: + +```sql +GRANT CONNECT ON DATABASE "" TO ""; +``` + +**Databases created before this change do not gain the revoke retroactively.** +They keep working exactly as before. To apply the same boundary to them, run +this once per existing tenant database: + +```sql +REVOKE CONNECT ON DATABASE "" FROM PUBLIC; +``` + +**Not affected.** The `schema` division mode, which grants on its own schema and +never relied on `public`. MySQL and MariaDB. + ### Connection::set(null) now clears the connection configuration **What changed.** Releasing the tenant with `Connection::set(null)` used to diff --git a/src/Generators/Webserver/Database/Drivers/PostgreSQL.php b/src/Generators/Webserver/Database/Drivers/PostgreSQL.php index ac4d911f..9170d0b4 100644 --- a/src/Generators/Webserver/Database/Drivers/PostgreSQL.php +++ b/src/Generators/Webserver/Database/Drivers/PostgreSQL.php @@ -64,7 +64,42 @@ protected function createDatabase(IlluminateConnection $connection, array $confi protected function grantPrivileges(IlluminateConnection $connection, array $config) { - return $connection->statement("GRANT ALL PRIVILEGES ON DATABASE \"{$config['database']}\" TO \"{$config['username']}\""); + $granted = $connection->statement("GRANT ALL PRIVILEGES ON DATABASE \"{$config['database']}\" TO \"{$config['username']}\""); + + // Every role may connect to a new database by default, leaving one + // tenant able to reach another's. + $connection->statement("REVOKE CONNECT ON DATABASE \"{$config['database']}\" FROM PUBLIC"); + + return $this->grantSchemaPrivileges($connection, $config) && $granted; + } + + /** + * Grants the tenant the right to create objects in its own database. + * + * PostgreSQL 15 revoked the CREATE privilege the public schema used to + * grant to everyone, which privileges on the database no longer cover. A + * schema grant only applies inside its own database, hence the connection. + * + * It goes to PUBLIC rather than to the tenant role because a role level + * grant is recorded as a dependency and makes DROP USER fail on deletion. + * CONNECT was revoked from PUBLIC above, so nothing else reaches it. + */ + protected function grantSchemaPrivileges(IlluminateConnection $connection, array $config) + { + $name = 'tenancy-provisioning'; + + config([ + "database.connections.{$name}" => array_merge( + $connection->getConfig(), + ['database' => $config['database']] + ), + ]); + + try { + return app('db')->connection($name)->statement('GRANT ALL ON SCHEMA public TO PUBLIC'); + } finally { + app('db')->purge($name); + } } protected function userExists($connection, string $username): bool From f7fa1bce6da2f6d299c649a7818155cc06f172f5 Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Fri, 28 Aug 2026 05:51:27 -0500 Subject: [PATCH 5/9] Stop tenancy:migrate:fresh from wiping a shared database The command called db:wipe on the tenant connection, which empties every table in the database behind it. That database belongs to the tenant alone in the database and schema division modes, but in prefix mode all tenants and the system tables share one, and running it dropped them all. The command took itself down with them: its next page of websites came from the table it had just dropped. It now drops only the tables carrying the tenant's prefix, and refuses in a shared database that hands out no prefix, where a wipe cannot be aimed at anything narrower than everything. Views and user defined types are left alone there, having no prefix to tell whose they are. --- UPGRADE.md | 29 ++++++ .../Console/Migrations/FreshCommand.php | 89 +++++++++++++++++-- .../unit-tests/Commands/FreshCommandTest.php | 35 ++++++++ 3 files changed, 147 insertions(+), 6 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 0a442af8..cb72f095 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -30,6 +30,35 @@ so nothing errors and nothing is logged. that asked for it, and `DispatcherMiddleware` deliberately switches the ambient tenant there. That behaviour is unchanged. +### tenancy:migrate:fresh no longer wipes the whole database in prefix mode ⚠️ behaviour change + +**What changed.** The command called `db:wipe` on the tenant connection, which +empties every table in the database behind it. In the `database` and `schema` +division modes that database belongs to the tenant alone, so that was right. In +`prefix` mode all tenants and the system tables share one database, so running +it dropped **every tenant's tables along with `websites` and `hostnames`**. The +damage was visible from inside the command itself, whose next page of websites +was read from the table it had just dropped. + +It now drops only the tables carrying that tenant's prefix. + +**How this can affect you.** If you use `prefix` mode, this command was +destroying your installation and you very likely never ran it twice. Nothing you +relied on changes; it simply stops taking everything else with it. + +**Views and user defined types are not dropped** in `prefix` mode. `--drop-views` +and `--drop-types` are honoured in the modes that give the tenant a database of +its own, where dropping everything is safe. In a shared database there is no +prefix on those objects to tell whose they are. + +**In the `bypass` division mode the command now refuses to run.** There, the +tenant connection *is* the system connection and nothing marks a tenant's tables +apart, so any wipe would take the system with it. It fails with an explanation +instead. + +**Not affected.** The `database` and `schema` division modes, which keep calling +`db:wipe` exactly as before. + ### PostgreSQL 15 and newer can provision tenants again ⚠️ behaviour change **What changed.** PostgreSQL 15 revoked the `CREATE` privilege that the `public` diff --git a/src/Database/Console/Migrations/FreshCommand.php b/src/Database/Console/Migrations/FreshCommand.php index 8b7f0ec8..ac0b0dd7 100644 --- a/src/Database/Console/Migrations/FreshCommand.php +++ b/src/Database/Console/Migrations/FreshCommand.php @@ -15,8 +15,11 @@ namespace Hyn\Tenancy\Database\Console\Migrations; use Hyn\Tenancy\Contracts\Website; +use Hyn\Tenancy\Database\Connection; +use Hyn\Tenancy\Exceptions\ConnectionException; use Hyn\Tenancy\Traits\MutatesMigrationCommands; use Illuminate\Database\Console\Migrations\FreshCommand as BaseCommand; +use Illuminate\Database\Schema\Builder as SchemaBuilder; class FreshCommand extends BaseCommand { @@ -36,12 +39,8 @@ public function handle() $this->processHandle(function (Website $website) { $database = $this->connection->tenantName(); - $this->call('db:wipe', array_filter([ - '--database' => $database, - '--drop-views' => $this->option('drop-views'), - '--drop-types' => $this->option('drop-types'), - '--force' => true, - ])); + + $this->wipe($database); $this->call('tenancy:migrate', [ '--database' => $database, @@ -62,6 +61,84 @@ public function handle() }); } + /** + * Drops the tenant's tables. + * + * db:wipe empties the whole database behind the connection, which belongs + * to the tenant alone only in the database and schema division modes. In + * prefix mode it would take the other tenants and the system tables too. + */ + protected function wipe(string $database) + { + $mode = config('tenancy.db.tenant-division-mode'); + + if (in_array($mode, [ + Connection::DIVISION_MODE_SEPARATE_DATABASE, + Connection::DIVISION_MODE_SEPARATE_SCHEMA, + ], true)) { + $this->call('db:wipe', array_filter([ + '--database' => $database, + '--drop-views' => $this->option('drop-views'), + '--drop-types' => $this->option('drop-types'), + '--force' => true, + ])); + + return; + } + + // Keyed on the mode rather than on the presence of a prefix, since the + // application may configure one of its own. + if ($mode !== Connection::DIVISION_MODE_SEPARATE_PREFIX) { + throw new ConnectionException( + "Division mode '$mode' marks no tables as the tenant's own, wiping would drop the system tables." + ); + } + + $connection = $this->connection->get(); + $prefix = $connection->getTablePrefix(); + + if ($prefix === '') { + throw new ConnectionException("Tenant connection carries no table prefix, unable to tell its tables apart."); + } + + $schema = $connection->getSchemaBuilder(); + + $schema->disableForeignKeyConstraints(); + + foreach ($this->tenantTables($schema, $prefix) as $table) { + $schema->drop($table); + } + + $schema->enableForeignKeyConstraints(); + } + + /** + * Tenant owned table names, with the prefix stripped off again because the + * schema builder applies it when dropping. + */ + protected function tenantTables(SchemaBuilder $schema, string $prefix): array + { + // getTableListing() arrived in Laravel 10.37, before which the rows of + // getAllTables() are shaped by the driver. + $tables = method_exists($schema, 'getTableListing') + ? $schema->getTableListing() + : array_map(function ($table) { + $table = (array) $table; + + return $table['tablename'] ?? $table['name'] ?? reset($table); + }, $schema->getAllTables()); + + $owned = []; + + foreach ($tables as $table) { + if (strpos($table, $prefix) === 0) { + $owned[] = substr($table, strlen($prefix)); + } + } + + return $owned; + } + /** * Get the console command options. * diff --git a/tests/unit-tests/Commands/FreshCommandTest.php b/tests/unit-tests/Commands/FreshCommandTest.php index f5bce9a4..ae8c0c7e 100644 --- a/tests/unit-tests/Commands/FreshCommandTest.php +++ b/tests/unit-tests/Commands/FreshCommandTest.php @@ -17,6 +17,7 @@ use Hyn\Tenancy\Database\Console\Migrations\FreshCommand; use Hyn\Tenancy\Models\Website; use Illuminate\Contracts\Foundation\Application; +use Illuminate\Database\Schema\Blueprint; use Hyn\Tenancy\Tests\Seeds\SampleSeeder; class FreshCommandTest extends DatabaseCommandTest @@ -94,6 +95,40 @@ public function does_not_purge_connection_after_running_fresh_on_one_tenant() $connection->shouldNotHaveReceived('purge'); } + /** + * In the prefix division mode every tenant and the system tables share one + * database, so a wipe aimed at the connection empties all of them. + * + * @test + */ + public function running_fresh_leaves_the_system_and_the_other_tenants_alone() + { + $other = new Website(); + $this->websites->create($other); + + // A table of the other tenant's, to notice the loss of. + $this->connection->set($other); + $this->connection->get()->getSchemaBuilder()->create('canary', function (Blueprint $table) { + $table->increments('id'); + }); + $this->connection->purge(); + + $this->migrateAndTest('migrate:fresh', null, null, [ + '--website_id' => [$this->website->id], + ]); + + $this->assertTrue( + $this->connection->system()->getSchemaBuilder()->hasTable('websites'), + 'A tenant migrate:fresh dropped the system websites table.' + ); + + $this->connection->set($other); + $this->assertTrue( + $this->connection->get()->getSchemaBuilder()->hasTable('canary'), + "Tenant {$other->uuid} lost its tables to another tenant's migrate:fresh." + ); + } + protected function duringSetUp(Application $app) { $this->setUpWebsites(true); From 3f6066b396e00534e432cd77fb94b8f6c4218f85 Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Fri, 28 Aug 2026 05:51:44 -0500 Subject: [PATCH 6/9] Ask the connection about the schema instead of Doctrine Doctrine's schema manager works below the connection, so it applies neither the table prefix nor the search path. The seed tests looked for "samples" where the table is "1_samples" in prefix mode, and outside the tenant's schema in schema mode, and failed in both. ConnectionTest caught Doctrine\DBAL\Driver\PDOException around a reconnect. PDO throws the native PDOException, which is not an instance of it, so the assertion inside that block could never run. Listing databases through the connection needs a query per engine, since PostgreSQL keeps them in a catalogue rather than in information_schema. Nothing in the package uses Doctrine DBAL any more, which Laravel 11 removes. --- tests/unit-tests/Commands/SeedCommandTest.php | 12 ++++++------ tests/unit-tests/Database/ConnectionTest.php | 5 +++-- tests/unit-tests/Database/MultiDatabaseTest.php | 10 +++++++++- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/unit-tests/Commands/SeedCommandTest.php b/tests/unit-tests/Commands/SeedCommandTest.php index 866ea742..3c3d7a8a 100644 --- a/tests/unit-tests/Commands/SeedCommandTest.php +++ b/tests/unit-tests/Commands/SeedCommandTest.php @@ -49,8 +49,8 @@ public function runs_seed_on_one_tenant() $this->connection->set($this->website); - $this->assertFalse($this->connection->get()->getDoctrineSchemaManager()->tablesExist('users')); - $this->assertTrue($this->connection->get()->getDoctrineSchemaManager()->tablesExist('samples')); + $this->assertFalse($this->connection->get()->getSchemaBuilder()->hasTable('users')); + $this->assertTrue($this->connection->get()->getSchemaBuilder()->hasTable('samples')); $this->assertGreaterThan( 0, @@ -87,8 +87,8 @@ public function runs_configured_seed() $this->connection->set($this->website); - $this->assertFalse($this->connection->get()->getDoctrineSchemaManager()->tablesExist('users')); - $this->assertTrue($this->connection->get()->getDoctrineSchemaManager()->tablesExist('samples')); + $this->assertFalse($this->connection->get()->getSchemaBuilder()->hasTable('users')); + $this->assertTrue($this->connection->get()->getSchemaBuilder()->hasTable('samples')); } /** @@ -98,14 +98,14 @@ public function runs_seed_on_tenants() { $this->connection->set($this->website); - $this->assertFalse($this->connection->get()->getDoctrineSchemaManager()->tablesExist('samples')); + $this->assertFalse($this->connection->get()->getSchemaBuilder()->hasTable('samples')); $this->migrateAndTest('migrate'); $this->seedAndTest(function (Website $website) { $this->connection->set($website); - $this->assertTrue($this->connection->get()->getDoctrineSchemaManager()->tablesExist('samples')); + $this->assertTrue($this->connection->get()->getSchemaBuilder()->hasTable('samples')); $this->assertEquals( 2, diff --git a/tests/unit-tests/Database/ConnectionTest.php b/tests/unit-tests/Database/ConnectionTest.php index cf8fe2ec..7aa65117 100644 --- a/tests/unit-tests/Database/ConnectionTest.php +++ b/tests/unit-tests/Database/ConnectionTest.php @@ -14,7 +14,8 @@ namespace Hyn\Tenancy\Tests\Database; -use Doctrine\DBAL\Driver\PDOException; +use Illuminate\Database\QueryException; +use PDOException; use Hyn\Tenancy\Commands\UpdateKeyCommand; use Hyn\Tenancy\Contracts\CurrentHostname; use Hyn\Tenancy\Environment; @@ -137,7 +138,7 @@ public function can_rotate_tenant_key() app(Environment::class)->tenant($this->website); try { $this->connection->get()->reconnect(); - } catch (PDOException $e) { + } catch (PDOException | QueryException $e) { $this->assertTrue($e->getCode() === 1045 || $e->getCode() === 7, 'Access should be denied for tenant database user: [code: '.$e->getCode().'] '. $e->getMessage()); } diff --git a/tests/unit-tests/Database/MultiDatabaseTest.php b/tests/unit-tests/Database/MultiDatabaseTest.php index a3e9ec0e..c3ac3708 100644 --- a/tests/unit-tests/Database/MultiDatabaseTest.php +++ b/tests/unit-tests/Database/MultiDatabaseTest.php @@ -50,9 +50,17 @@ public function allow_writing_to_secondary_database() // make sure the Website model still uses the regular system name. $this->assertEquals(app(Connection::class)->systemName(), $this->website->getConnectionName()); + $secondary = $this->getConnection('secondary'); + + // PostgreSQL keeps its databases in a catalogue of their own. + $databases = $secondary->getDriverName() === 'pgsql' + ? $secondary->select('select datname as name from pg_database') + : $secondary->select('select schema_name as name from information_schema.schemata'); + $this->assertTrue(in_array( $this->website->uuid, - $this->getConnection('secondary')->getDoctrineSchemaManager()->listDatabases() + array_column(array_map('get_object_vars', $databases), 'name'), + true )); } } From b4f5896576020a5fce390945d76a22a88862bc80 Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Fri, 28 Aug 2026 05:51:52 -0500 Subject: [PATCH 7/9] Scope the tests to the division modes they apply to Three tests asserted something only the database division mode provides and failed everywhere else: a database of the tenant's own on a second server, credentials of its own to cross, and a database name and user that differ between two tenants. The first two skip outside that mode. The third asks instead for whichever separator the configured mode hands out, a schema or a table prefix, which is the guarantee actually being made. --- .../unit-tests/Database/MultiDatabaseTest.php | 6 +++ .../Isolation/ConnectionIsolationTest.php | 44 ++++++++++++++----- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/tests/unit-tests/Database/MultiDatabaseTest.php b/tests/unit-tests/Database/MultiDatabaseTest.php index c3ac3708..e78b248a 100644 --- a/tests/unit-tests/Database/MultiDatabaseTest.php +++ b/tests/unit-tests/Database/MultiDatabaseTest.php @@ -40,6 +40,12 @@ public function allow_writing_to_secondary_database() return $this->markTestSkipped("Can't access secondary database for testing"); } + if (config('tenancy.db.tenant-division-mode') !== Connection::DIVISION_MODE_SEPARATE_DATABASE) { + return $this->markTestSkipped( + 'Only the database division mode gives the tenant a database of its own on the secondary server.' + ); + } + $this->website->managed_by_database_connection = 'secondary'; $this->websites->create($this->website); diff --git a/tests/unit-tests/Isolation/ConnectionIsolationTest.php b/tests/unit-tests/Isolation/ConnectionIsolationTest.php index bd913896..1059d9d1 100644 --- a/tests/unit-tests/Isolation/ConnectionIsolationTest.php +++ b/tests/unit-tests/Isolation/ConnectionIsolationTest.php @@ -14,6 +14,7 @@ namespace Hyn\Tenancy\Tests\Isolation; +use Hyn\Tenancy\Database\Connection; use Hyn\Tenancy\Tests\Extend\AwareExtend; use Hyn\Tenancy\Tests\Test; use Hyn\Tenancy\Tests\Traits\InteractsWithIsolation; @@ -205,16 +206,34 @@ public function writes_land_in_the_active_tenant_only() /** * @test */ - public function each_tenant_gets_its_own_database_and_credentials() + public function each_tenant_gets_its_own_slice_of_the_server() { $a = $this->connection->generateConfigurationArray($this->tenantA); $b = $this->connection->generateConfigurationArray($this->tenantB); - $this->assertNotSame($a['database'], $b['database'], 'Tenants must not share a database name.'); - $this->assertNotSame($a['username'], $b['username'], 'Tenants must not share a database user.'); - $this->assertNotSame($a['password'], $b['password'], 'Tenants must not share a database password.'); $this->assertSame($this->tenantA->uuid, $a['uuid']); $this->assertSame($this->tenantB->uuid, $b['uuid']); + + // Assert on whichever separator the configured mode hands out. + switch (config('tenancy.db.tenant-division-mode')) { + case Connection::DIVISION_MODE_SEPARATE_DATABASE: + $this->assertNotSame($a['database'], $b['database'], 'Tenants must not share a database name.'); + $this->assertNotSame($a['username'], $b['username'], 'Tenants must not share a database user.'); + $this->assertNotSame($a['password'], $b['password'], 'Tenants must not share a database password.'); + break; + + case Connection::DIVISION_MODE_SEPARATE_SCHEMA: + $this->assertNotSame($a['schema'], $b['schema'], 'Tenants must not share a schema.'); + $this->assertNotSame($a['username'], $b['username'], 'Tenants must not share a database user.'); + break; + + case Connection::DIVISION_MODE_SEPARATE_PREFIX: + $this->assertNotSame($a['prefix'], $b['prefix'], 'Tenants must not share a table prefix.'); + break; + + default: + $this->markTestSkipped('This division mode does not separate tenants at the connection level.'); + } } /** @@ -225,12 +244,17 @@ public function one_tenant_cannot_read_another_tenants_data_even_with_its_own_cr // The last line of defence: with a connection aimed straight at // another tenant's database, the server itself must refuse the rows. // - // How it refuses differs by engine, and only the outcome is asserted - // here. MySQL and MariaDB reject the connection outright, because a - // tenant's user is granted privileges on its own database only. - // PostgreSQL lets the connection through, since every role may CONNECT - // to a new database by default, and refuses at the table instead. Both - // are acceptable; returning rows is not. + // Only the outcome is asserted. MySQL and MariaDB reject the connection, + // PostgreSQL refuses at CONNECT or, on databases provisioned before the + // revoke, at the table. Any refusal will do; returning rows will not. + if (config('tenancy.db.tenant-division-mode') !== Connection::DIVISION_MODE_SEPARATE_DATABASE) { + $this->markTestSkipped( + 'Only the database division mode hands each tenant its own credentials, so there are ' + .'none to cross here. The other modes separate tenants inside one database, which the ' + .'switching tests in this class cover.' + ); + } + $a = $this->connection->generateConfigurationArray($this->tenantA); $b = $this->connection->generateConfigurationArray($this->tenantB); From 1ed3a56a094148bfb7ae2de378e030454c7b5f2e Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Fri, 28 Aug 2026 12:52:56 -0500 Subject: [PATCH 8/9] Release the tenant's disk along with the tenant ActivatesDisk rooted the tenant disk in the active website's directory on Identified and Switched, and did nothing on release. The disk stayed pointed at the tenant that had just been let go, so anything writing to Storage::disk('tenant') afterwards landed in that customer's files. It is the filesystem counterpart of the connection keeping the previous tenant's credentials, and it matters most on a queue worker, which now releases the tenant between jobs. The disk is cleared instead of repointed at the shared root, so the next access fails rather than writing somewhere plausible but wrong. --- src/Listeners/Filesystem/ActivatesDisk.php | 16 +++++ .../Filesystem/ActivatesDiskTest.php | 61 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/src/Listeners/Filesystem/ActivatesDisk.php b/src/Listeners/Filesystem/ActivatesDisk.php index 262f9bfc..0c0d2411 100644 --- a/src/Listeners/Filesystem/ActivatesDisk.php +++ b/src/Listeners/Filesystem/ActivatesDisk.php @@ -16,6 +16,7 @@ use Hyn\Tenancy\Abstracts\WebsiteEvent; use Hyn\Tenancy\Events\Websites\Identified; +use Hyn\Tenancy\Events\Websites\Forgotten; use Hyn\Tenancy\Events\Websites\Switched; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Contracts\Filesystem\Factory; @@ -40,6 +41,7 @@ public function __construct(Factory $filesystem) public function subscribe(Dispatcher $events) { $events->listen([Identified::class, Switched::class], [$this, 'activate']); + $events->listen(Forgotten::class, [$this, 'deactivate']); } /** @@ -59,4 +61,18 @@ public function activate(WebsiteEvent $event) $this->filesystem->set('tenant', null); } } + + /** + * Reacts when the active tenant is released. + * + * Leaving the disk configured would keep it rooted in the released + * tenant's directory, so whatever wrote to it next would land in that + * customer's files. + */ + public function deactivate() + { + config(['filesystems.disks.tenant' => null]); + + $this->filesystem->set('tenant', null); + } } diff --git a/tests/unit-tests/Filesystem/ActivatesDiskTest.php b/tests/unit-tests/Filesystem/ActivatesDiskTest.php index 9277d4a5..01848c82 100644 --- a/tests/unit-tests/Filesystem/ActivatesDiskTest.php +++ b/tests/unit-tests/Filesystem/ActivatesDiskTest.php @@ -15,6 +15,7 @@ namespace Hyn\Tenancy\Tests\Filesystem; use Hyn\Tenancy\Tests\Test; +use Hyn\Tenancy\Environment; use Illuminate\Contracts\Foundation\Application; use Illuminate\Filesystem\FilesystemManager; use InvalidArgumentException; @@ -49,4 +50,64 @@ public function sets_the_disk_during_switch() $this->assertTrue($disk->put('foo', 'bar')); $this->assertTrue($disk->exists('foo')); } + + /** + * The shared root is not the isolation boundary: the tenant's uuid is. + * + * @test + */ + public function the_disk_is_rooted_in_the_active_tenants_own_directory() + { + $this->activateTenant(); + + $this->assertStringEndsWith( + '/'.$this->website->uuid, + config('filesystems.disks.tenant.root'), + 'The tenant disk is not rooted in the active tenant.' + ); + } + + /** + * @test + */ + public function switching_tenant_repoints_the_disk() + { + $this->activateTenant(); + $other = $this->getReplicatedWebsite(); + + $this->files->disk('tenant')->put('only-the-first.txt', 'x'); + + app(Environment::class)->tenant($other); + + $this->assertStringEndsWith( + '/'.$other->uuid, + config('filesystems.disks.tenant.root'), + 'The disk kept pointing at the previous tenant after switching.' + ); + + $this->assertFalse( + $this->files->disk('tenant')->exists('only-the-first.txt'), + "Tenant {$other->uuid} can see a file written by {$this->website->uuid}." + ); + } + + /** + * The filesystem counterpart of releasing the connection: an idle worker + * must not still be pointed at the last customer's directory. + * + * @test + */ + public function releasing_the_tenant_leaves_no_disk_pointed_at_it() + { + $this->activateTenant(); + $uuid = $this->website->uuid; + + app(Environment::class)->forgetTenant(); + + $this->assertNotEquals( + $uuid, + basename((string) config('filesystems.disks.tenant.root')), + "The tenant disk is still rooted in {$uuid} after it was released." + ); + } } From 27c5f8d2e05875d95d2ddd152b777de2718dce25 Mon Sep 17 00:00:00 2001 From: Andres Daza Date: Fri, 28 Aug 2026 12:52:56 -0500 Subject: [PATCH 9/9] Put back the tenant that tenancy:run found The command switches tenant on each website in turn and left the last one active when it finished, and on the way out of an exception. From a terminal that hardly matters, since the process ends. Called through Artisan::call() from a request or a job, the caller carried on as a customer it never asked for. The commands built on processHandle() are deliberately left alone: with a single tenant in the chunk they keep the connection active, and the suite asserts that. --- UPGRADE.md | 37 ++++++++++++++++++ src/Commands/RunCommand.php | 30 ++++++++++----- tests/unit-tests/Commands/RunCommandTest.php | 40 ++++++++++++++++++++ 3 files changed, 97 insertions(+), 10 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index cb72f095..8d2b42de 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -30,6 +30,43 @@ so nothing errors and nothing is logged. that asked for it, and `DispatcherMiddleware` deliberately switches the ambient tenant there. That behaviour is unchanged. +### Releasing the tenant now releases its disk too ⚠️ behaviour change + +**What changed.** `ActivatesDisk` rooted the `tenant` disk in the active +tenant's directory when one was identified or switched to, and did nothing when +one was released. The disk stayed pointed at the tenant that had just been let +go, so anything writing to `Storage::disk('tenant')` afterwards landed in that +customer's files. It is the filesystem counterpart of the connection keeping the +previous tenant's credentials. + +Releasing the tenant now clears the disk as well, which makes the next access +fail loudly rather than write to the wrong place. + +**How this can affect you.** Code that wrote to `Storage::disk('tenant')` +outside any tenant's context used to succeed silently against whichever tenant +came last. It now throws. That is the point of the change, but it does turn +previously silent behaviour into a visible failure — most likely on a queue +worker, which releases the tenant between jobs. + +**What to do.** Make sure anything touching `disk('tenant')` runs inside the +context of the tenant it belongs to. + +### tenancy:run puts back the tenant it found ⚠️ behaviour change + +**What changed.** The command switched tenant on each website in turn and left +the last one active when it finished, and on the way out of an exception. Run +from a terminal that hardly matters, since the process ends. Called from a +request or a job through `Artisan::call('tenancy:run')`, the caller silently +carried on as a customer it never asked for. + +It now restores whatever tenant was active beforehand, or releases it when +there was none. + +**Not affected.** The commands built on `processHandle()` — +`tenancy:migrate` and friends — keep their current behaviour: with a single +tenant in the chunk the connection is deliberately left active, and the suite +asserts it. + ### tenancy:migrate:fresh no longer wipes the whole database in prefix mode ⚠️ behaviour change **What changed.** The command called `db:wipe` on the tenant connection, which diff --git a/src/Commands/RunCommand.php b/src/Commands/RunCommand.php index 93808c92..b0079fa6 100644 --- a/src/Commands/RunCommand.php +++ b/src/Commands/RunCommand.php @@ -73,16 +73,26 @@ public function handle(Environment $environment, WebsiteRepository $repository) $exitCodes = []; - $query->chunk(50, function ($websites) use ($environment, $options, &$exitCodes) { - foreach ($websites as $website) { - $environment->tenant($website); - - $exitCodes[] = $this->call( - $this->argument('run'), - $options->toArray() - ); - } - }); + // Whatever was active before, the command puts back afterwards. The + // loop leaves the last tenant of the last chunk active otherwise, and + // that outlives the command whenever it is called from a request or a + // job rather than from a terminal. + $previous = $environment->tenant(); + + try { + $query->chunk(50, function ($websites) use ($environment, $options, &$exitCodes) { + foreach ($websites as $website) { + $environment->tenant($website); + + $exitCodes[] = $this->call( + $this->argument('run'), + $options->toArray() + ); + } + }); + } finally { + $previous ? $environment->tenant($previous) : $environment->forgetTenant(); + } if (count($exitCodes) === 0) { $this->warn("Command was executed on zero tenants."); diff --git a/tests/unit-tests/Commands/RunCommandTest.php b/tests/unit-tests/Commands/RunCommandTest.php index 068a3e47..df385534 100644 --- a/tests/unit-tests/Commands/RunCommandTest.php +++ b/tests/unit-tests/Commands/RunCommandTest.php @@ -15,6 +15,7 @@ namespace Hyn\Tenancy\Tests\Commands; use App\Console\Kernel; +use Hyn\Tenancy\Environment; use Hyn\Tenancy\Tests\Test; use Illuminate\Contracts\Foundation\Application; @@ -80,4 +81,43 @@ public function takes_options_and_arguments() ]); $this->assertEquals(0, $code); } + + /** + * The loop switches tenant on every website and the last one outlives the + * command, which matters whenever it is called from a request or a job. + * + * @test + */ + public function running_across_tenants_leaves_no_tenant_active() + { + $this->setUpWebsites(true); + $this->getReplicatedWebsite(); + + $this->artisan('tenancy:run', ['run' => 'env']); + + $this->assertNull( + app(Environment::class)->tenant(), + 'tenancy:run left a tenant active after it finished.' + ); + } + + /** + * @test + */ + public function a_failing_command_leaves_no_tenant_active_either() + { + $this->setUpWebsites(true); + $this->getReplicatedWebsite(); + + try { + $this->artisan('tenancy:run', ['run' => 'commandThatDoesNotExist']); + } catch (\Throwable $e) { + // The point is what is left behind, not the exception itself. + } + + $this->assertNull( + app(Environment::class)->tenant(), + 'tenancy:run left a tenant active after failing.' + ); + } }