From dc34b5b8c6428426f84b0a61261c6c4371ecc66e Mon Sep 17 00:00:00 2001 From: Zyra Date: Thu, 3 Sep 2026 11:09:24 +0000 Subject: [PATCH] Fix #120: relaties beperken tot velden uit defineFields() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BuildoraController::store()/update() beperkten kolom-mass-assignment al correct tot de velden uit Resource::defineFields(). handleRelationships() gebruikte echter ongefilterd $request->all(), waardoor elke public, parameterloze modelmethode die een Eloquent-relatie teruggeeft (bv. een niet-geëxposeerde "roles" BelongsToMany) alsnog gesynchroniseerd of geassocieerd kon worden door simpelweg een gelijknamig veld mee te sturen. Dat is mass assignment op relatieniveau, los van defineFields(). Fix: geef alleen $request->only($fields) door aan handleRelationships(), zodat relaties enkel aangepast kunnen worden als ze expliciet als veld in defineFields() staan. Regressietest voegt een niet-geëxposeerde BelongsToMany-relatie toe en bevestigt dat deze niet meer via de request gesynchroniseerd wordt, terwijl een wél gedefinieerde relatie blijft werken. Co-Authored-By: Claude Sonnet 5 --- src/Http/Controllers/BuildoraController.php | 10 +- .../BuildoraControllerMassAssignmentTest.php | 204 ++++++++++++++++++ 2 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 tests/Feature/BuildoraControllerMassAssignmentTest.php diff --git a/src/Http/Controllers/BuildoraController.php b/src/Http/Controllers/BuildoraController.php index 29f7056..d7c5c12 100644 --- a/src/Http/Controllers/BuildoraController.php +++ b/src/Http/Controllers/BuildoraController.php @@ -106,7 +106,10 @@ public function store(Request $request, string $model): RedirectResponse } $createdItem = $modelInstance::create($filteredData); - $this->handleRelationships($createdItem, $request->all()); + // Alleen relaties syncen die expliciet als veld zijn gedefinieerd in + // defineFields(); nooit de ruwe request gebruiken, anders kan elke + // relatiemethode op het model misbruikt worden (mass assignment). + $this->handleRelationships($createdItem, $request->only($fields)); return redirect()->route('buildora.index', ['resource' => $model]) ->with('success', ucfirst($model) . ' ' . __buildora('created successfully.')); @@ -187,7 +190,10 @@ public function update(Request $request, string $model, int $id): RedirectRespon $filteredData = array_intersect_key($finalData, array_flip($fields)); $item->update($filteredData); - $this->handleRelationships($item, $request->all()); + // Alleen relaties syncen die expliciet als veld zijn gedefinieerd in + // defineFields(); nooit de ruwe request gebruiken, anders kan elke + // relatiemethode op het model misbruikt worden (mass assignment). + $this->handleRelationships($item, $request->only($fields)); return redirect()->route('buildora.index', ['resource' => $model]) ->with('success', ucfirst($model) . ' ' . __buildora('updated successfully.')); diff --git a/tests/Feature/BuildoraControllerMassAssignmentTest.php b/tests/Feature/BuildoraControllerMassAssignmentTest.php new file mode 100644 index 0000000..d049103 --- /dev/null +++ b/tests/Feature/BuildoraControllerMassAssignmentTest.php @@ -0,0 +1,204 @@ +update()) waren al veilig: $filteredData wordt + * altijd teruggebracht tot de veldnamen uit defineFields(). + * - handleRelationships() gebruikte echter $request->all() ongefilterd. Elke + * public, parameterloze methode op het model die een Eloquent-relatie + * teruggeeft (bv. een BelongsToMany "roles" relatie) kon zo gesynchroniseerd + * worden door simpelweg een gelijknamig veld in de request mee te sturen, + * ook al stond die relatie niet in defineFields(). Dat is een mass + * assignment-kwetsbaarheid op relatieniveau (bv. privilege-escalatie via + * een niet-geëxposeerde "roles"-relatie). + */ +class BuildoraControllerMassAssignmentTest extends TestCase +{ + protected function setUp(): void + { + parent::setUp(); + + Schema::create('mass_assignment_test_roles', function ($table) { + $table->increments('id'); + $table->string('name'); + }); + + Schema::create('mass_assignment_test_projects', function ($table) { + $table->increments('id'); + $table->string('name'); + $table->timestamps(); + }); + + Schema::create('mass_assignment_test_project_role', function ($table) { + $table->unsignedInteger('mass_assignment_test_project_id'); + $table->unsignedInteger('mass_assignment_test_role_id'); + }); + } + + protected function tearDown(): void + { + Schema::dropIfExists('mass_assignment_test_project_role'); + Schema::dropIfExists('mass_assignment_test_projects'); + Schema::dropIfExists('mass_assignment_test_roles'); + + parent::tearDown(); + } + + /** @test */ + public function store_does_not_sync_a_relation_that_is_not_defined_in_define_fields(): void + { + $role = MassAssignmentTestRole::create(['name' => 'admin']); + + $request = Request::create('/buildora/resource/massassignmenttestproject', 'POST', [ + 'name' => 'Legit project', + // "roles" bestaat niet in MassAssignmentTestProjectBuildora::defineFields(), + // maar wel als BelongsToMany-relatie op het model. + 'roles' => [$role->id], + ]); + + $controller = new BuildoraController(); + $controller->store($request, 'massassignmenttestproject'); + + $project = MassAssignmentTestProject::firstOrFail(); + + $this->assertSame('Legit project', $project->name); + $this->assertCount( + 0, + $project->roles, + 'Een relatie die niet in defineFields() staat, mag niet via de request gesynchroniseerd worden.' + ); + } + + /** @test */ + public function update_does_not_sync_a_relation_that_is_not_defined_in_define_fields(): void + { + $project = MassAssignmentTestProject::create(['name' => 'Legit project']); + $role = MassAssignmentTestRole::create(['name' => 'admin']); + + $request = Request::create('/buildora/resource/massassignmenttestproject/' . $project->id, 'PUT', [ + 'name' => 'Renamed project', + 'roles' => [$role->id], + ]); + + $controller = new BuildoraController(); + $controller->update($request, 'massassignmenttestproject', $project->id); + + $project->refresh(); + + $this->assertSame('Renamed project', $project->name); + $this->assertCount( + 0, + $project->roles, + 'Een relatie die niet in defineFields() staat, mag niet via de request gesynchroniseerd worden.' + ); + } + + /** @test */ + public function store_still_syncs_a_relation_that_is_explicitly_defined_in_define_fields(): void + { + $role = MassAssignmentTestRole::create(['name' => 'admin']); + + $request = Request::create('/buildora/resource/massassignmenttestprojectwithroles', 'POST', [ + 'name' => 'Legit project', + 'roles' => [$role->id], + ]); + + $controller = new BuildoraController(); + $controller->store($request, 'massassignmenttestprojectwithroles'); + + $project = MassAssignmentTestProject::firstOrFail(); + + $this->assertSame('Legit project', $project->name); + $this->assertCount( + 1, + $project->roles, + 'Een relatie die wel in defineFields() staat, moet nog steeds werken.' + ); + } +} + +/** + * Testfixtures voor deze testclass. Ze staan bewust in dit bestand zodat de + * test zelfstandig blijft draaien zonder aparte host-app. + */ +class MassAssignmentTestRole extends \Illuminate\Database\Eloquent\Model +{ + protected $table = 'mass_assignment_test_roles'; + protected $fillable = ['name']; + public $timestamps = false; +} + +class MassAssignmentTestProject extends \Illuminate\Database\Eloquent\Model +{ + use \Ginkelsoft\Buildora\Traits\HasBuildora; + + protected $table = 'mass_assignment_test_projects'; + protected $fillable = ['name']; + + public function roles(): \Illuminate\Database\Eloquent\Relations\BelongsToMany + { + return $this->belongsToMany( + MassAssignmentTestRole::class, + 'mass_assignment_test_project_role', + 'mass_assignment_test_project_id', + 'mass_assignment_test_role_id' + ); + } +} + +namespace App\Buildora\Resources; + +use Ginkelsoft\Buildora\Fields\Types\BelongsToManyField; +use Ginkelsoft\Buildora\Fields\Types\TextField; +use Ginkelsoft\Buildora\Resources\BuildoraResource; +use Ginkelsoft\Buildora\Tests\Feature\MassAssignmentTestProject; + +/** + * Resource waarvan defineFields() de "roles"-relatie bewust NIET blootstelt. + */ +class MassAssignmentTestProjectBuildora extends BuildoraResource +{ + public function defineFields(): array + { + return [ + TextField::make('name', 'Name')->validation(['required', 'string', 'max:255']), + ]; + } + + public static function modelClass(): string + { + return MassAssignmentTestProject::class; + } +} + +/** + * Resource waarvan defineFields() de "roles"-relatie wel expliciet definieert, + * om te bevestigen dat legitiem gebruik van relatievelden blijft werken. + */ +class MassAssignmentTestProjectWithRolesBuildora extends BuildoraResource +{ + public function defineFields(): array + { + return [ + TextField::make('name', 'Name')->validation(['required', 'string', 'max:255']), + BelongsToManyField::make('roles', 'Roles'), + ]; + } + + public static function modelClass(): string + { + return MassAssignmentTestProject::class; + } +}