mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-21 02:53:24 +00:00
Check the read half of the redirect rule at every door, not one
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).
This commit is contained in:
@@ -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'),
|
||||
],
|
||||
]);
|
||||
|
||||
Reference in New Issue
Block a user