Skip to content

Issue #124 (buildora) onderzoeken: vaststellen of datatable- en bulk-action-routes buiten de web-middleware (en dus buiten CSRF-verificatie) vallen, en zo nodig fixen. - #190

Open
zyra-ginkelsoft[bot] wants to merge 1 commit into
mainfrom
zyra/issue-124-buildora-onderzoeken-vaststell-35becb1a
Open

zyra-ginkelsoft[bot] wants to merge 1 commit into
mainfrom
zyra/issue-124-buildora-onderzoeken-vaststell-35becb1a

Conversation

@zyra-ginkelsoft

Copy link
Copy Markdown
Contributor

This is a package repo (Buildora, a Laravel admin package), not a full Laravel app. Let me look at routes and service providers.Goed, orchestra/testbench is aanwezig. Nu ga ik installeren en een reproductietest schrijven om te bewijzen of de CSRF-bescherming daadwerkelijk actief is op de datatable/bulk-action routes.Missende ext-gd. Ik installeer met de platform-negeer optie, zodat we kunnen testen (dit is alleen voor de proefomgeving).Er bestaat al een goed .zyra/proef.sh script dat de ext-gd fallback al afhandelt. Laten we het testen en de composer install afmaken met dezelfde fallback.Composer install gelukt. Nu schrijf ik een reproductietest die een POST naar een bulk-action/datatable-route stuurt zonder CSRF-token.Missende app key. Ik voeg die toe in de TestCase setup zodat encryptie werkt (nodig voor session/csrf).Nu herschrijf ik de definitieve regressietest correct, met de env-override zodat CSRF echt wordt gecontroleerd, en test ik ook expliciet dat een geldig token wél doorlaat (om te bewijzen dat de test zinvol is en niet toevallig altijd 419 geeft).Nu test ik of dit werkt met een config zonder 'web' erin — precies het scenario dat de fix moet afdekken.De fix werkt: zelfs met een misconfigureerde buildora.middleware (zonder 'web') blijft CSRF nu afgedwongen (419). Ik voeg dit toe als permanente regressietest en ruim het debug-bestand op.Beter: aparte bestanden voor duidelijkheid en PSR-12 conformiteit.Nu de volledige testsuite en lint/analyse draaien.Alle 42 tests slagen. Nu lint en static analyse.Bevestigd: dezelfde lint/phpstan-bevindingen (59 fouten) bestaan al op main, buiten mijn wijzigingen (die alleen routes/ en tests/ raken, wat niet door src/ linting wordt gescand). Mijn wijzigingen introduceren geen nieuwe lint/analyse-fouten. Nu controleer ik het .zyra/proef.sh script end-to-end.De installatiepagina reageert met HTTP 200. Nu een extra reproductie: een echte HTTP POST-aanvraag naar de switch-locale route zonder CSRF-token, tegen de draaiende server (buiten de testomgeving, dus zonder de runningUnitTests()-bypass).Bevestigd met een echte live-server: HTTP 419 zonder CSRF-token — precies zoals verwacht. Nu ruim ik het proefserver-proces op en commit ik het werk.Geen achtergrondprocessen meer actief. Nu opruimen van de database-artefacten die door het proefscript zijn aangemaakt, en dan committen.Goed, alles schoon. Nu committen.Laatste check: volledige testsuite nogmaals draaien vanaf de gecommitte staat om zeker te zijn dat alles schoon is.Alle 42 tests slagen. Werk is gecommit op de branch.

Samenvatting

Onderzoek (issue #124): Ik heb routes/buildora.php, routes/auth.php en beide service providers doorgelicht op alle non-GET routes (switch-locale, settings, permissions, profile, 2FA, en vooral de dynamische resource-routes voor store/update/destroy waar bulk-acties naartoe posten — bulk-actions zelf zijn user-gedefinieerde named routes, geen vaste package-routes).

Bevinding: Alle routes zaten binnen Route::prefix(...)->middleware(config('buildora.middleware', ['web', ...])). Met de standaardconfig zit 'web' er dus in en is CSRF-verificatie vandaag actief — bevestigd met:

  • een regressietest die de echte middleware-pijplijn forceert (Laravel schakelt CSRF standaard uit tijdens unit tests via runningUnitTests(), dat heb ik moeten omzeilen om een eerlijke meting te krijgen) → 419 zonder token;
  • een live POST naar de draaiende testbench-server (.zyra/proef.sh) zonder CSRF-token → HTTP 419, bewijs bijgevoegd in de sessie.

Wél een fragiliteit: CSRF hing volledig af van config('buildora.middleware'). Als een consumerende app die gepubliceerde config aanpast en per ongeluk 'web' weglaat, verdwijnt CSRF-bescherming stilzwijgend op precies de gevoeligste routes (create/update/delete). routes/auth.php in hetzelfde package gebruikt dit patroon niet en wrapt altijd hard in Route::middleware('web') — inconsistent.

Fix: routes/buildora.php extra gewrapt in een hard-coded Route::middleware('web'), naast de bestaande config-gestuurde middleware, zodat CSRF nooit via configuratie kan worden uitgeschakeld.

Tests:

  • tests/Feature/CsrfProtectionTest.php: 419 zonder token op switch-locale/store/destroy + assertie dat routes de web-groep dragen.
  • tests/Feature/CsrfProtectionWithoutWebInConfigTest.php: simuleert config zonder 'web' → nog steeds 419 (bewijst de hardening werkt).
  • tests/TestCase.php: app-key toegevoegd (nodig voor sessie/CSRF-encryptie in tests, ontbrak eerder waardoor deze tests niet eens konden draaien).
  • Volledige suite: 42/42 groen. Lint/phpstan tonen 59 vooraf bestaande fouten in src/ (geverifieerd op main vóór mijn wijzigingen, ongewijzigd door dit werk — mijn wijzigingen raken alleen routes/ en tests/, buiten scope van die scans).

Open punten:

  • De 59 bestaande lint/phpstan-bevindingen in src/ zijn niet aangepakt (buiten scope van dit issue); mogelijk apart issue waard.
  • Geen geheimen toegevoegd in code, tests of commits.
  • .zyra/proef.sh bestond al en werkt correct (composer install met ext-gd fallback, sqlite-db, migraties, serve op $PORT); niet gewijzigd.

…nkel 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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants