From fc5651faad83649c641eee548d14914c75a0ba2d Mon Sep 17 00:00:00 2001 From: denkfabrik-li <274324701+denkfabrik-li@users.noreply.github.com> Date: Fri, 28 Aug 2026 04:38:18 +0200 Subject: [PATCH] Check the read half of the redirect rule at every door, not one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three middleware answer before HandleInertiaRequests and so have to repeat its 302→303 upgrade themselves: EnsureSetupIsComplete, EnsureUserIsActive and EnforceTwoFactor. This file has a write case for each, and the rule has a second half -- a read still gets a plain 302, because a 303 there would be an upgrade nobody asked for. That half was checked once, on the deactivation door, under a name that said otherwise: "leaves a read alone in every one of those cases". The setup door and the two-factor door were not covered at all, so a change that upgraded reads at either of them would have gone through with the suite green and this test's name still claiming it would not. Both are covered now, as a dataset with one case per door. The setup case reads a guest-reachable GET for the same reason the write case posts to /timezone: anything behind `auth` is answered by the guest redirect before EnsureSetupIsComplete sees it. No production code changes; today all three doors answer a read with 302, which is what the new cases assert. Demonstrated by mutation rather than reversion: making EnsureSetupIsComplete upgrade every redirect to 303 fails this file (1 failed / 8 passed) and passes the old one (7 passed). --- .../Feature/Auth/WriteRedirectStatusTest.php | 44 ++++++++++++++++--- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/tests/Feature/Auth/WriteRedirectStatusTest.php b/tests/Feature/Auth/WriteRedirectStatusTest.php index fabf0696..9021825f 100644 --- a/tests/Feature/Auth/WriteRedirectStatusTest.php +++ b/tests/Feature/Auth/WriteRedirectStatusTest.php @@ -77,11 +77,43 @@ it('answers a write with 303 when two-factor enrolment is being enforced', funct ->assertRedirect(route('two-factor.show')); }); -it('leaves a read alone in every one of those cases', function () { - $user = User::factory()->create(); - $user->update(['active' => false]); +// The other half of the rule, and the half that says the upgrade is +// targeted: a read still gets the plain 302. Once per door, because "every +// one of those cases" was one of them — the deactivation — and a change +// that upgraded reads at either of the other two would have been invisible +// here while this name said otherwise. +it('leaves a read alone at every one of those doors', function (Closure $arrange, string $read, Closure $target) { + $arrange(); - $this->actingAs($user)->get(route('dashboard')) + $this->get($read) ->assertStatus(302) - ->assertRedirect(route('login')); -}); + ->assertRedirect($target()); +})->with([ + // A guest-reachable GET, for the same reason the write case uses + // PUT /timezone: anything behind `auth` is answered by the guest + // redirect before EnsureSetupIsComplete ever sees it. + 'no administrator yet' => [ + fn () => User::query()->delete(), + '/login', + fn () => route('setup'), + ], + 'deactivated mid-session' => [ + function (): void { + $user = User::factory()->create(); + $user->update(['active' => false]); + test()->actingAs($user); + }, + '/dashboard', + fn () => route('login'), + ], + 'two-factor enrolment enforced' => [ + function (): void { + // Set explicitly rather than relying on the default: the + // settings cache outlives a database rollback in this suite. + app(Settings::class)->set(Setting::TwoFactorEnforcement, TwoFactorEnforcement::All->value); + test()->actingAs(User::factory()->create()); + }, + '/dashboard', + fn () => route('two-factor.show'), + ], +]);