From e8bd8268fd9df64babc5f94749a5c30d0871b13f Mon Sep 17 00:00:00 2001 From: nexxo Date: Tue, 28 Jul 2026 14:42:29 +0200 Subject: [PATCH] Stop a disabled operator from bypassing the hidden-site gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PublicSiteGate let anyone through on a bare Auth::guard('operator')->check(), which stays true for a disabled operator's live session — disabling someone does not sign them out, so it never stopped this gate from showing them the real site indefinitely, even though EnsureAdmin already treats them as inactive. Now requires the same thing EnsureAdmin does: an active operator holding a console role. Chosen to merely ignore an invalid operator session here rather than log it out. This gate only decides what one request sees; logging out would ripple into whatever else that session is doing (a concurrent console tab, mid-task), a bigger side effect than a visibility check exists to have. EnsureAdmin sets the precedent — it also rejects a disabled operator per request without touching their session. --- app/Http/Middleware/PublicSiteGate.php | 22 ++++++++++++++- tests/Feature/PublicSiteGateTest.php | 39 +++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/app/Http/Middleware/PublicSiteGate.php b/app/Http/Middleware/PublicSiteGate.php index 411c838..e76e22a 100644 --- a/app/Http/Middleware/PublicSiteGate.php +++ b/app/Http/Middleware/PublicSiteGate.php @@ -57,7 +57,7 @@ class PublicSiteGate return $next($request); } - if ($this->fromManagementNetwork($request) || Auth::guard('operator')->check()) { + if ($this->fromManagementNetwork($request) || $this->isActiveOperator()) { return $next($request); } @@ -74,4 +74,24 @@ class PublicSiteGate return $ranges !== [] && IpUtils::checkIp((string) $request->ip(), $ranges); } + + /** + * Same standard as EnsureAdmin: an active operator holding a console role. + * A bare `check()` stays true for a disabled operator's live session — + * disabling someone did not sign them out, so it never stopped this gate + * from waving them through indefinitely. + * + * Deliberately checked, not enforced by logging out: this gate only + * decides what ONE request sees, and destroying the session would ripple + * into whatever else that session is doing (a concurrent console tab, + * mid-task) — a much bigger side effect than a visibility check exists to + * have. EnsureAdmin sets the precedent: it also 403s a disabled operator + * per request without touching their session. + */ + private function isActiveOperator(): bool + { + $operator = Auth::guard('operator')->user(); + + return $operator !== null && $operator->isActive() && $operator->can('console.view'); + } } diff --git a/tests/Feature/PublicSiteGateTest.php b/tests/Feature/PublicSiteGateTest.php index 72121b3..5c133ac 100644 --- a/tests/Feature/PublicSiteGateTest.php +++ b/tests/Feature/PublicSiteGateTest.php @@ -1,8 +1,10 @@ assertStatus(503); }); +it('does not let a disabled operator bypass the hidden site', function () { + // EnsureAdmin already refuses a disabled operator (isActive()); this gate + // used to check only Auth::guard('operator')->check(), which stays true + // for a disabled operator's live session — an indefinite bypass of + // maintenance/private-site mode for anyone disabled while signed in. + Settings::set('site.public', false); + + $op = operator('Support'); + $op->update(['disabled_at' => now()]); + + $this->actingAs($op, 'operator') + ->withServerVariables(['REMOTE_ADDR' => '203.0.113.50']) + ->get('/') + ->assertStatus(503); + + // Chosen deliberately: merely ignored, not logged out — see + // PublicSiteGate's docblock for why (this gate only decides what ONE + // request sees; logging out would ripple into whatever else that + // session is doing, e.g. a concurrent console tab). + expect(Auth::guard('operator')->check())->toBeTrue(); +}); + +it('does not let an operator holding no console role bypass the hidden site', function () { + // Mirrors EnsureAdmin's other half: isActive() alone is not enough, + // the operator must also hold a role that reaches the console. + Settings::set('site.public', false); + + $op = Operator::factory()->create(); // no role assigned + + $this->actingAs($op, 'operator') + ->withServerVariables(['REMOTE_ADDR' => '203.0.113.50']) + ->get('/') + ->assertStatus(503); +}); + it('keeps the console and the webhook reachable while hidden', function () { Settings::set('site.public', false); @@ -102,7 +139,7 @@ it('flips visibility from the console, and only with the capability', function ( it('keeps serving when the settings table is unavailable', function () { // A deploy that runs new code before migrations, or a database blip: the // gate must fall back to "public", not take the whole site down with a 500. - Illuminate\Support\Facades\Schema::drop('app_settings'); + Schema::drop('app_settings'); $this->get('/')->assertOk(); });