From 473e8a60ec3cef7b45ae3df74169eeee84c648c7 Mon Sep 17 00:00:00 2001 From: Zyra Date: Thu, 3 Sep 2026 12:49:18 +0000 Subject: [PATCH] Fix #124: web-middleware (CSRF) hard-coded om Buildora-routes, niet enkel via config Audit: alle datatable- en bulk-action-routes in routes/buildora.php (inclusief store/update/destroy en de switch-locale/settings/2FA-routes) erven de 'web'-middlewaregroep uitsluitend via config('buildora.middleware'). Met de standaardconfig zit 'web' daar wel in, dus CSRF-verificatie is vandaag actief (geverifieerd met een regressietest en een live POST tegen een draaiende testbench-server: beide geven 419 zonder token). Dit is echter fragiel: een consumerende applicatie die de gepubliceerde config/buildora.php aanpast kan per ongeluk 'web' uit de middleware-lijst verwijderen, waarmee CSRF-bescherming voor de gevoeligste routes (create, update, delete, bulk-acties) stilzwijgend verdwijnt. routes/auth.php in hetzelfde package past dit patroon niet toe en wrapt zijn routes altijd hard in Route::middleware('web'). Fix: routes/buildora.php extra wrappen in een hard-coded Route::middleware('web'), naast de bestaande config-gestuurde middleware. Zo kan CSRF nooit via configuratie worden uitgeschakeld. Regressietests tonen 419 zonder CSRF-token aan, zowel met de standaardconfig als met een config waaruit 'web' is verwijderd. Co-Authored-By: Claude Sonnet 5 --- routes/buildora.php | 10 ++- tests/Feature/CsrfProtectionTest.php | 77 +++++++++++++++++++ .../CsrfProtectionWithoutWebInConfigTest.php | 48 ++++++++++++ tests/TestCase.php | 3 + 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/CsrfProtectionTest.php create mode 100644 tests/Feature/CsrfProtectionWithoutWebInConfigTest.php diff --git a/routes/buildora.php b/routes/buildora.php index db1cfff..890dddb 100644 --- a/routes/buildora.php +++ b/routes/buildora.php @@ -14,7 +14,15 @@ use Ginkelsoft\Buildora\Http\Controllers\TwoFactorController; use Ginkelsoft\Buildora\Http\Controllers\BuildoraSettingsController; -Route::prefix(config('buildora.route_prefix', 'buildora')) +// De 'web'-middlewaregroep (sessies, CSRF-verificatie, cookie-encryptie) +// wordt hier hard-coded toegepast, los van config('buildora.middleware'). +// Zo blijft CSRF-bescherming voor alle datatable- en bulk-action-routes +// (o.a. store/update/destroy en de acties die daarnaar posten) gegarandeerd +// actief, ook als een consumerende applicatie de gepubliceerde config +// aanpast en daarbij per ongeluk 'web' uit de middleware-lijst verwijdert. +// Zie issue #124. +Route::middleware('web') + ->prefix(config('buildora.route_prefix', 'buildora')) ->middleware(config('buildora.middleware', ['web', 'buildora.auth', 'buildora.ensure-user-resource'])) ->group(function () { diff --git a/tests/Feature/CsrfProtectionTest.php b/tests/Feature/CsrfProtectionTest.php new file mode 100644 index 0000000..fe7861a --- /dev/null +++ b/tests/Feature/CsrfProtectionTest.php @@ -0,0 +1,77 @@ +app['env'] = 'local'; + } + + public function test_post_to_switch_locale_without_csrf_token_is_rejected_with_419(): void + { + $response = $this->post('/buildora/switch-locale', [ + 'locale' => 'nl', + ]); + + $response->assertStatus(419); + } + + public function test_post_to_resource_store_without_csrf_token_is_rejected_with_419(): void + { + // De dynamische resource-routes (store/update/destroy), waar ook + // bulk-actions naartoe posten, zitten genest in dezelfde groep en + // moeten dezelfde CSRF-bescherming erven. + $response = $this->post('/buildora/resource/users', [ + 'name' => 'Attacker', + ]); + + $response->assertStatus(419); + } + + public function test_delete_to_resource_destroy_without_csrf_token_is_rejected_with_419(): void + { + // DELETE wordt door bulk-delete-acties gebruikt. + $response = $this->delete('/buildora/resource/users/1'); + + $response->assertStatus(419); + } + + public function test_switch_locale_route_carries_the_web_middleware_group(): void + { + // Directe assertie op de route-registratie: bewijst dat de fix + // (of de bestaande, correcte registratie) niet per ongeluk wordt + // teruggedraaid, onafhankelijk van omgevingsdetectie-trucs hierboven. + $route = \Illuminate\Support\Facades\Route::getRoutes()->getByName('buildora.locale.switch'); + + $this->assertContains('web', $route->gatherMiddleware()); + } + + public function test_resource_store_route_carries_the_web_middleware_group(): void + { + $route = \Illuminate\Support\Facades\Route::getRoutes()->getByName('buildora.store'); + + $this->assertContains('web', $route->gatherMiddleware()); + } +} diff --git a/tests/Feature/CsrfProtectionWithoutWebInConfigTest.php b/tests/Feature/CsrfProtectionWithoutWebInConfigTest.php new file mode 100644 index 0000000..8fbc114 --- /dev/null +++ b/tests/Feature/CsrfProtectionWithoutWebInConfigTest.php @@ -0,0 +1,48 @@ +set('buildora.middleware', [ + 'buildora.auth', + 'buildora.ensure-user-resource', + ]); + } + + protected function setUp(): void + { + parent::setUp(); + + // Forceer echte CSRF-afdwinging; Laravel schakelt dit anders + // automatisch uit tijdens unit tests. + $this->app['env'] = 'local'; + } + + public function test_post_without_csrf_token_is_still_rejected_with_419_even_without_web_in_config(): void + { + $response = $this->post('/buildora/switch-locale', [ + 'locale' => 'nl', + ]); + + $response->assertStatus(419); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 39191a9..203418b 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -23,6 +23,9 @@ protected function getPackageProviders($app): array protected function getEnvironmentSetUp($app): void { + // App key is nodig voor sessie-encryptie en CSRF-tokens. + $app['config']->set('app.key', 'base64:' . base64_encode(random_bytes(32))); + // Setup default database to use sqlite :memory: $app['config']->set('database.default', 'testbench'); $app['config']->set('database.connections.testbench', [