From ba861ad5799ef5b0513a0f10ba42479b30ccbbab Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 20:56:26 +0200 Subject: [PATCH] fix(security): match ADMIN_HOSTS case-insensitively; share test helpers DNS names are case-insensitive and Symfony's getHost() always returns lowercase, so ADMIN_HOSTS=Admin.Example.com rejected every request and locked operators out. Normalised in the config AND at the comparison, so the value is safe whichever route it took in (env, cached config, runtime set). operator()/admin() moved from RbacTest/HostManagementTest to tests/Pest.php: they were only loaded when those files happened to be in the run, so a targeted single-file run died on 'undefined function operator()'. Co-Authored-By: Claude Opus 4.8 --- app/Http/Middleware/RestrictAdminHost.php | 9 ++++++++- config/admin_access.php | 4 +++- .../Feature/Admin/AdminHostRestrictionTest.php | 17 ++++++++++++++++- tests/Feature/Admin/HostManagementTest.php | 5 ----- tests/Feature/Admin/RbacTest.php | 5 ----- tests/Pest.php | 14 ++++++++++++++ 6 files changed, 41 insertions(+), 13 deletions(-) diff --git a/app/Http/Middleware/RestrictAdminHost.php b/app/Http/Middleware/RestrictAdminHost.php index 59be3d8..33546f0 100644 --- a/app/Http/Middleware/RestrictAdminHost.php +++ b/app/Http/Middleware/RestrictAdminHost.php @@ -30,7 +30,14 @@ class RestrictAdminHost return $next($request); } - $allowed = (array) config('admin_access.hosts', []); + // Lowercased here as well as in the config: DNS names are + // case-insensitive and getHost() always returns lowercase, so a + // capitalised ADMIN_HOSTS entry must not lock operators out — whatever + // route the value took into the config (env, cached config, runtime set). + $allowed = array_map( + fn ($host) => strtolower((string) $host), + (array) config('admin_access.hosts', []), + ); if ($allowed !== [] && ! in_array($request->getHost(), $allowed, true)) { abort(404); diff --git a/config/admin_access.php b/config/admin_access.php index 8183074..fb11675 100644 --- a/config/admin_access.php +++ b/config/admin_access.php @@ -16,8 +16,10 @@ return [ | Example (production): | ADMIN_HOSTS=admin.clupilot.com */ + // Lowercased: DNS names are case-insensitive and Symfony's getHost() always + // returns lowercase, so ADMIN_HOSTS=Admin.Example.com must still match. 'hosts' => array_values(array_filter(array_map( - 'trim', + fn ($host) => strtolower(trim($host)), explode(',', (string) env('ADMIN_HOSTS', '')) ))), diff --git a/tests/Feature/Admin/AdminHostRestrictionTest.php b/tests/Feature/Admin/AdminHostRestrictionTest.php index 5beeb54..d7a3b77 100644 --- a/tests/Feature/Admin/AdminHostRestrictionTest.php +++ b/tests/Feature/Admin/AdminHostRestrictionTest.php @@ -2,7 +2,7 @@ use App\Models\User; -// operator() helper is defined in RbacTest. +// operator()/admin() helpers live in tests/Pest.php. it('serves the console on any host when ADMIN_HOSTS is empty (default)', function () { config()->set('admin_access.hosts', []); @@ -37,6 +37,21 @@ it('serves the console on an allowed hostname', function () { ->assertOk(); }); +it('matches configured hostnames case-insensitively', function () { + // DNS is case-insensitive and getHost() lowercases, so ADMIN_HOSTS with any + // capitals must still match — otherwise a typo silently locks operators out. + config()->set('admin_access.hosts', ['Admin.Dev.CluPilot.COM']); + + $this->actingAs(operator('Owner')) + ->get('http://admin.dev.clupilot.com/admin') + ->assertOk(); + + // …and the restriction still bites on everything else. + $this->actingAs(operator('Owner')) + ->get('http://www.dev.clupilot.com/admin') + ->assertNotFound(); +}); + it('404s before revealing a login redirect to guests on a public host', function () { config()->set('admin_access.hosts', ['admin.dev.clupilot.com']); diff --git a/tests/Feature/Admin/HostManagementTest.php b/tests/Feature/Admin/HostManagementTest.php index d48e06e..e7eb543 100644 --- a/tests/Feature/Admin/HostManagementTest.php +++ b/tests/Feature/Admin/HostManagementTest.php @@ -14,11 +14,6 @@ use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Queue; use Livewire\Livewire; -function admin(): User -{ - return User::factory()->create(['is_admin' => true]); -} - it('gates the add-host page', function () { $this->get(route('admin.hosts.create'))->assertRedirect('/login'); $this->actingAs(User::factory()->create(['is_admin' => false]))->get(route('admin.hosts.create'))->assertForbidden(); diff --git a/tests/Feature/Admin/RbacTest.php b/tests/Feature/Admin/RbacTest.php index 7643fb6..23d25f8 100644 --- a/tests/Feature/Admin/RbacTest.php +++ b/tests/Feature/Admin/RbacTest.php @@ -7,11 +7,6 @@ use App\Models\User; use Illuminate\Auth\Access\AuthorizationException; use Livewire\Livewire; -function operator(string $role): User -{ - return User::factory()->operator($role)->create(); -} - it('grants console access to every operator role but not to customers', function () { foreach (['Owner', 'Admin', 'Support', 'Billing', 'Read-only'] as $role) { $this->actingAs(operator($role))->get(route('admin.overview'))->assertOk(); diff --git a/tests/Pest.php b/tests/Pest.php index 5544609..4bf4c0b 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -73,3 +73,17 @@ function fakeServices(): array return ['shell' => $shell, 'hub' => $hub, 'pve' => $pve, 'dns' => $dns, 'traefik' => $traefik, 'monitoring' => $monitoring]; } + +/* +| Shared account helpers. Defined here — NOT in an individual test file — so a +| targeted run (pest tests/Feature/Admin/OneFile.php) still resolves them. +*/ +function operator(string $role): App\Models\User +{ + return App\Models\User::factory()->operator($role)->create(); +} + +function admin(): App\Models\User +{ + return App\Models\User::factory()->create(['is_admin' => true]); +}