Two tenants, one bug: the negative tests that prove isolation actually holds

Every article about multi-tenant SaaS ends the same way. Write tests. None of them say what to assert, so the test that gets written is the one that proves the happy path: Acme logs in, Acme sees Acme's three invoices, green tick, next ticket.

That test passes on an application with no tenant isolation whatsoever. With one tenant's data in the database, and there usually is only one, it cannot tell a scoped query from an unscoped one.

The tests that do the work are negative. Two tenants exist, the second one's data is real, and the assertion is that the thing you are afraid of does not work. The boundary overview says where to point them and ends with a single example. This is the whole file, in the order I write it on a codebase I have never seen before.

The failure mode that makes the whole suite worthless

First, the thing nobody mentions. A negative test that cannot fail is not a test, and in a multi-tenant codebase there is a specific and extremely common way for all of them to be vacuous at once.

The BelongsToTenant trait stamps the current tenant onto a model as it is created. That is correct in production, and it quietly destroys your fixtures:

// Current tenant is Acme, set by a trait, a helper or the previous line.
$invoice = Invoice::factory()->for($globex)->create();

// The creating hook overwrote tenant_id with Acme's. There is no Globex
// invoice. Every assertion below it now proves that Acme cannot read a
// row which does not exist, and passes forever.

So the first test in the file is the one that tests the fixtures:

it('can actually create data belonging to another tenant', function () {
    [$acme, $globex] = Tenant::factory()->count(2)->create();

    Tenancy::set($acme->id);
    $invoice = asTenant($globex, fn () => Invoice::factory()->create());

    // Read it back with no scope at all, or the scope hides it from you too.
    expect(Invoice::withoutGlobalScopes()->find($invoice->id)->tenant_id)
        ->toBe($globex->id);
});

One fixture, two tenants, four roles

Everything below rests on two helpers. The first runs a closure as a given tenant and puts the previous context back, which is what makes a cross-tenant fixture possible at all. The second builds the world.

// tests/Pest.php
function asTenant(Tenant $tenant, Closure $callback): mixed
{
    $previous = Tenancy::currentId();

    try {
        Tenancy::set($tenant->id);
        return $callback();
    } finally {
        $previous ? Tenancy::set($previous) : Tenancy::forget();
    }
}

/** Two tenants, both fully populated, both with a full set of roles. */
function twoTenants(): array
{
    [$acme, $globex] = Tenant::factory()->count(2)->create();

    foreach ([$acme, $globex] as $tenant) {
        asTenant($tenant, function () use ($tenant) {
            Invoice::factory()->count(3)->create();
            Document::factory()->count(2)->create();

            foreach (['owner', 'admin', 'member', 'viewer'] as $role) {
                User::factory()->for($tenant)->create(['role' => $role]);
            }
        });
    }

    return [$acme, $globex];
}

Four roles rather than one, because the interesting question is not whether a viewer can read another tenant's invoice. It is whether an owner can. Owner holds every permission inside its own tenant, which makes it the role most likely to have been handed a shortcut somewhere in your authorisation code, and the one a gate is most likely to wave straight through.

What each attempt should return

The 404 rule is the row people argue about, so to be exact about it: inside a tenant, a record the user may not touch is a 403. Across a tenant boundary it is a 404, because the honest answer to does this id exist is none of their business.

Reads: the test everybody writes, and the one they do not

it('refuses a cross-tenant read for every role', function (string $role) {
    [$acme, $globex] = twoTenants();

    $invoice = asTenant($globex, fn () => Invoice::factory()->create());
    $user = $acme->users()->where('role', $role)->sole();

    actingAs($user)->get("/invoices/{$invoice->id}")->assertNotFound();
})->with(['owner', 'admin', 'member', 'viewer']);

Four tests out of one body, and owner is the one that fails. Then the version that is missing from every codebase I have looked at:

it('does not leak through a nested route', function () {
    [$acme, $globex] = twoTenants();

    // Acme's own project. Globex's document. Both ids are real.
    $project = asTenant($acme, fn () => Project::factory()->create());
    $document = asTenant($globex, fn () => Document::factory()->create());

    actingAs($acme->users()->first())
        ->get("/projects/{$project->id}/documents/{$document->id}")
        ->assertNotFound();
});

Route model binding resolves each parameter on its own. The project is scoped, the document is scoped, neither is ever checked against the other, and the only thing standing between a document and the wrong parent is that nobody tried it. Laravel's scoped bindings close this in one call per route, which is worth knowing before you go hunting for it in a policy.

Writes: the half of the suite that is usually missing

Read tests get written because a leak reads like a breach. Write tests get skipped, and they catch the worse class of bug, because a cross-tenant write corrupts data instead of exposing it and there is nothing for anyone to notice.

it('refuses a cross-tenant update', function () {
    [$acme, $globex] = twoTenants();
    $invoice = asTenant($globex, fn () => Invoice::factory()->create(['total' => 100]));

    actingAs($acme->users()->first())
        ->patch("/invoices/{$invoice->id}", ['total' => 1])
        ->assertNotFound();

    expect($invoice->fresh()->total)->toBe(100);   // assert the row, not the status
});

it('ignores a tenant_id supplied by the client', function () {
    [$acme, $globex] = twoTenants();

    actingAs($acme->users()->first())
        ->post('/invoices', ['total' => 50, 'tenant_id' => $globex->id]);

    expect(Invoice::withoutGlobalScopes()->latest('id')->first()->tenant_id)
        ->toBe($acme->id);
});

it('never writes a row with no tenant at all', function () {
    [$acme] = twoTenants();

    actingAs($acme->users()->first())->post('/invoices', ['total' => 50]);

    expect(Invoice::withoutGlobalScopes()->whereNull('tenant_id')->count())->toBe(0);
});

The second is a four-line test for the bug that lets a customer move records into somebody else's account by editing a form field. The third catches the quieter one: a write that lands with a null tenant_id and becomes invisible to everyone, including the person who just created it. Both assert against the database rather than the response, which is the habit that separates a real suite from a reassuring one. A 200 tells you the request was accepted. It tells you nothing about what was written.

Exports, because that is where the raw SQL lives

A global scope only rewrites queries that go through the ORM. Exports are where a codebase keeps the queries that do not, because somebody needed a join and a sum that Eloquent made awkward. So assert on the artefact, not on the query:

it('exports no foreign rows, whatever the query underneath looks like', function () {
    [$acme, $globex] = twoTenants();

    asTenant($globex, fn () => Invoice::factory()->create(['reference' => 'GLOBEX-SECRET']));

    $csv = actingAs($acme->users()->where('role', 'owner')->sole())
        ->get('/invoices/export')
        ->streamedContent();

    expect($csv)->not->toContain('GLOBEX-SECRET');

    // The row count matters as much as the missing string.
    expect(explode(PHP_EOL, trim($csv)))->toHaveCount(4);   // header + Acme's three
});

An export that returns nothing, because the scope broke in the other direction and is now filtering on a tenant the reporting connection cannot see, is also a bug, and only the count finds it. The same shape covers a PDF, a nightly email report, and anything that leaves over a webhook.

Retries, schedules and impersonation

Three paths that need a different setup rather than a different assertion, because what breaks them is state left behind by earlier work. The queue case is covered at length in the queues, caches and logs article, and the one-line version is that you call handle() directly from a context that is deliberately wrong, because dispatching inside a test establishes the right tenant for you and hides the bug. Scheduled commands need the same thinking one level up:

it('keeps going, correctly, when one tenant in the loop fails', function () {
    $tenants = Tenant::factory()->count(3)->create();

    // Whatever makes the middle one fail: a throwing fake, a dead card, a 500.
    Billing::fake()->failFor($tenants[1]);

    artisan('invoices:issue-monthly')->assertSuccessful();

    // Tenants one and three are billed. Nobody is billed twice, and nobody
    // is billed against the tenant that threw.
    expect(Invoice::withoutGlobalScopes()->pluck('tenant_id')->sort()->values()->all())
        ->toBe([$tenants[0]->id, $tenants[2]->id]);

    expect(Tenancy::currentId())->toBeNull();   // the loop cleaned up after itself
});

That last assertion is the single line most worth copying out of this article. Any test that drives a worker, a command or a batch should end by asserting there is no current tenant, because a leftover context is the mechanism behind most of the bugs in this cluster and it costs one line to detect.

it('records and expires support impersonation', function () {
    [$acme] = twoTenants();
    $support = User::factory()->support()->create();

    actingAs($support)
        ->post("/admin/impersonate/{$acme->id}", ['reason' => 'ticket 4821'])
        ->assertOk();

    expect(AuditLog::latest('id')->first())
        ->actor_id->toBe($support->id)
        ->tenant_id->toBe($acme->id)
        ->reason->not->toBeEmpty();

    travel(31)->minutes();
    actingAs($support)->get('/invoices')->assertForbidden();
});

it('refuses impersonation to a user who is merely an account owner', function () {
    [$acme, $globex] = twoTenants();

    actingAs($globex->users()->where('role', 'owner')->sole())
        ->post("/admin/impersonate/{$acme->id}")
        ->assertForbidden();
});

Impersonation is the one route in the application built to cross a tenant boundary, which makes it the only place where the correct assertion is that it works. Everything of value is in the surrounding conditions: an actor recorded, a reason recorded, a clock running, and a permission that an account owner does not hold.

The two tests that stop the suite going stale

Everything so far covers the paths that existed the day it was written. Isolation breaks on the route somebody adds in March. These two cover that, and per line of code they are the most valuable things in the file:

// Both helpers are a directory scan over app/ returning class names.
it('takes a tenant id in the constructor of every queueable job', function () {
    $missing = collect(classesImplementing(ShouldQueue::class))
        ->reject(function (string $job) {
            $parameters = (new ReflectionClass($job))->getConstructor()?->getParameters() ?? [];

            return collect($parameters)->contains(
                fn (ReflectionParameter $p) => $p->getName() === 'tenantId',
            );
        });

    expect($missing)->toBeEmpty("Jobs with no tenant in the payload: {$missing->implode(', ')}");
});

it('scopes every model that has a tenant_id column', function () {
    $unscoped = collect(modelClasses())
        ->filter(fn (string $m) => Schema::hasColumn((new $m)->getTable(), 'tenant_id'))
        ->reject(fn (string $m) => in_array(BelongsToTenant::class, class_uses_recursive($m)));

    expect($unscoped)->toBeEmpty("Unscoped tenant-owned models: {$unscoped->implode(', ')}");
});

Both fail in a pull request rather than in a support ticket, and both fail for the right reason: somebody added something new and did not know the rule yet. The message naming the offending class is what turns the failure into a fix instead of a question in Slack. That is the same instinct as putting the rule where it cannot be skipped rather than in a convention people are expected to remember.

Running them

• Use the real drivers. Postgres if you rely on row-level security, Redis if you rely on cache tags. The array store has no tags and an in-memory SQLite database has no RLS, so a green run on CI's convenient defaults has told you nothing about either boundary.
• Never set a default tenant in setUp. If the base TestCase establishes one, every test inherits a context, and the bugs that only appear when the context is wrong become unreachable. Set tenancy per test, explicitly, every time.
• Tag the group and require it. --group=isolation as its own required check means nobody merges past it while chasing an unrelated flake.
• Keep the file short enough to read. Twelve tests somebody maintains beats forty nobody runs. These are the twelve.

A positive test tells you the feature works for one customer. Only a negative test tells you it does not work for the wrong one, and only a negative test you have watched fail tells you anything at all.

If you are retrofitting this onto something live, write them in the order above and stop at the first failure rather than finishing the file. One red test tells you which layer to fix, and a fix with a failing test in front of it is a morning. Twelve red tests at once is a planning problem, and you will triage it worse than you would have scheduled it.

If you would rather have the suite written against your codebase than mine, that is most of what an architecture review produces: the boundaries that actually leak on your system, in priority order, each one with the test that proves the fix held. Book a SaaS architecture review.