From 9454b13576f59941d64049cb7fb276faa7e570d8 Mon Sep 17 00:00:00 2001 From: Ahmad Afandi Date: Wed, 28 Oct 2026 06:41:46 +0700 Subject: [PATCH 1/5] fix(user): cegah update user lain dan pemberian role oleh non super-admin UserController::update() tidak memeriksa kepemilikan dan memproses role mentah dari request, sehingga administrator-website bisa mengubah user lain dan memberikan role super-admin. - Non super-admin hanya boleh mengubah profilnya sendiri (403 jika tidak). - Hanya super-admin yang boleh menyinkronkan role. - Pengecekan ditempatkan di luar try agar abort(403) tidak tertangkap. Fixes #1782 Co-Authored-By: Claude Haiku 5.5 --- app/Http/Controllers/User/UserController.php | 11 ++-- tests/Feature/UserControllerTest.php | 54 ++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/User/UserController.php b/app/Http/Controllers/User/UserController.php index 054668b7c..a8a528d5f 100644 --- a/app/Http/Controllers/User/UserController.php +++ b/app/Http/Controllers/User/UserController.php @@ -151,17 +151,22 @@ public function edit($id) */ public function update(UserRequest $request, $id) { + $actor = Auth::user(); + $isSuperAdmin = $actor->roles->contains('name', 'super-admin'); + if (!$isSuperAdmin && $id != $actor->id) { + abort(403, 'Anda tidak berhak mengubah user tersebut'); + } + try { $input = $request->validated(); $user = User::findOrFail($id); $this->handleFileUpload($request, $input, 'image', 'user', false); $user->update($input); - if (!empty($request->role)) { + if ($isSuperAdmin && !empty($request->role)) { $roles = $request->input('role') ? $request->input('role') : []; $user->syncRoles($roles); } - $user = Auth::user(); - if (!$user->roles->contains('name', 'super-admin')) { + if (!$isSuperAdmin) { return redirect()->route('dashboard')->with('success', 'User berhasil diperbarui!'); } return redirect()->route('setting.user.index')->with('success', 'User berhasil diperbarui!'); diff --git a/tests/Feature/UserControllerTest.php b/tests/Feature/UserControllerTest.php index 38669f86f..7088eee21 100644 --- a/tests/Feature/UserControllerTest.php +++ b/tests/Feature/UserControllerTest.php @@ -296,6 +296,60 @@ public function test_update_updates_profile_and_redirects_to_dashboard_for_regul ]); } + /** + * Test update method forbids non super admin from updating another user. + * + * @return void + */ + public function test_update_forbids_non_super_admin_from_updating_another_user() + { + $actor = User::factory()->create(); + $actor->assignRole('administrator-website'); + $actor->givePermissionTo('access.setting'); + + $targetEmail = $this->faker->unique()->safeEmail; + $targetUser = User::factory()->create(['email' => $targetEmail]); + + $response = $this->actingAs($actor)->put(route('setting.user.update', $targetUser->id), [ + 'name' => 'Hacked Name', + 'email' => $this->faker->unique()->safeEmail, + 'address' => 'Hacked Address', + ]); + + $response->assertForbidden(); + + $this->assertDatabaseHas('users', [ + 'id' => $targetUser->id, + 'email' => $targetEmail, + ]); + } + + /** + * Test update method ignores role from non super admin editing own profile. + * + * @return void + */ + public function test_update_ignores_role_from_non_super_admin_editing_own_profile() + { + $actor = User::factory()->create(); + $actor->assignRole('administrator-website'); + $actor->givePermissionTo('access.setting'); + + $response = $this->actingAs($actor)->put(route('setting.user.update', $actor->id), [ + 'name' => 'Self Name', + 'email' => $actor->email, + 'address' => 'Self Address', + 'role' => ['super-admin'], + ]); + + $response->assertRedirect(route('dashboard')); + + $actor->refresh(); + $this->assertSame('Self Name', $actor->name); + $this->assertFalse($actor->hasRole('super-admin')); + $this->assertTrue($actor->hasRole('administrator-website')); + } + /** * Test destroy method deactivates user successfully. * From b455545773819716ebbcb6fba0994fb1232acca7 Mon Sep 17 00:00:00 2001 From: Ahmad Afandi Date: Wed, 28 Oct 2026 07:07:22 +0700 Subject: [PATCH 2/5] fix(setting): izinkan user membuka dan mengubah profil sendiri tanpa access.setting Route edit dan update profil user berada di bawah middleware access.setting, sehingga role admin-kecamatan (yang sengaja tidak punya akses setting) mendapat 403 saat membuka profilnya sendiri. - Pindahkan route setting.user.edit dan setting.user.update ke grup yang hanya memakai auth. Kepemilikan tetap dicek di UserController (non super-admin hanya boleh mengakses id miliknya). - Route pengaturan user lain dan foto profil tetap dilindungi access.setting. - Tambah test untuk profil sendiri tanpa access.setting, serta penjagaan bahwa edit user lain dan daftar user tetap ditolak. Co-Authored-By: Claude Haiku 5.5 --- routes/web.php | 6 ++- tests/Feature/UserControllerTest.php | 81 ++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/routes/web.php b/routes/web.php index fcde690df..3bfd3b94f 100644 --- a/routes/web.php +++ b/routes/web.php @@ -893,9 +893,13 @@ Route::delete('permanent-destroy/{id}', 'permanentDestroy')->name('setting.user.permanent-destroy'); }); - Route::group(['prefix' => 'user', 'controller' => UserController::class, 'middleware' => ['action_permission:access.setting']], function () { + // Profil sendiri bisa diakses semua user yang login; kepemilikan dicek di UserController + Route::group(['prefix' => 'user', 'controller' => UserController::class], function () { Route::get('edit/{id}', 'edit')->name('setting.user.edit'); Route::put('update/{id}', 'update')->name('setting.user.update'); + }); + + Route::group(['prefix' => 'user', 'controller' => UserController::class, 'middleware' => ['action_permission:access.setting']], function () { Route::get('photo-profil/{id}', 'photo')->name('setting.user.photo'); Route::put('update-photo/{id}', 'updatePhoto')->name('setting.user.uphoto'); }); diff --git a/tests/Feature/UserControllerTest.php b/tests/Feature/UserControllerTest.php index 7088eee21..441e64a6b 100644 --- a/tests/Feature/UserControllerTest.php +++ b/tests/Feature/UserControllerTest.php @@ -350,6 +350,87 @@ public function test_update_ignores_role_from_non_super_admin_editing_own_profil $this->assertTrue($actor->hasRole('administrator-website')); } + /** + * Test user without access.setting can open edit form for own profile. + * + * @return void + */ + public function test_edit_displays_own_profile_without_access_setting_permission() + { + $actor = $this->createUserWithoutSettingAccess(); + + $response = $this->actingAs($actor)->get(route('setting.user.edit', $actor->id)); + + $response->assertStatus(200); + $response->assertViewIs('user.edit'); + $response->assertViewHas('user', $actor); + } + + /** + * Test user without access.setting can update own profile. + * + * @return void + */ + public function test_update_own_profile_without_access_setting_permission() + { + $actor = $this->createUserWithoutSettingAccess(); + + $response = $this->actingAs($actor)->put(route('setting.user.update', $actor->id), [ + 'name' => 'Profil Sendiri', + 'email' => $actor->email, + 'address' => 'Alamat Sendiri', + ]); + + $response->assertRedirect(route('dashboard')); + $this->assertDatabaseHas('users', [ + 'id' => $actor->id, + 'name' => 'Profil Sendiri', + ]); + } + + /** + * Test user without access.setting cannot open edit form for another user. + * + * @return void + */ + public function test_edit_forbids_another_user_without_access_setting_permission() + { + $actor = $this->createUserWithoutSettingAccess(); + $targetUser = User::factory()->create(); + + $response = $this->actingAs($actor)->get(route('setting.user.edit', $targetUser->id)); + + $response->assertForbidden(); + } + + /** + * Test user without access.setting still cannot open user management list. + * + * @return void + */ + public function test_index_forbids_user_without_access_setting_permission() + { + $actor = $this->createUserWithoutSettingAccess(); + + $response = $this->actingAs($actor)->get(route('setting.user.index')); + + $response->assertForbidden(); + } + + /** + * Create a user whose role has no access.setting permission, like admin-kecamatan. + * + * @return User + */ + private function createUserWithoutSettingAccess() + { + $role = Role::firstOrCreate(['name' => 'admin-kecamatan', 'guard_name' => 'web']); + $actor = User::factory()->create(); + $actor->assignRole($role); + + return $actor; + } + /** * Test destroy method deactivates user successfully. * From 5ca085096b9e2548732357485eb72abe20b8416e Mon Sep 17 00:00:00 2001 From: Ahmad Afandi Date: Wed, 28 Oct 2026 07:31:46 +0700 Subject: [PATCH 3/5] fix(user): tegakkan pembatasan email di controller Pembatasan edit email (hanya untuk akun sendiri) sebelumnya hanya ada di view. Field readonly tetap ikut terkirim, sehingga nilai email user lain bisa diubah lewat request. - Di UserController::update(), email dibuang dari input jika user yang diubah bukan akun yang sedang login. Berlaku untuk semua role, termasuk super-admin, sesuai aturan di form.blade.php. - Test super-admin mengubah user lain diubah: nama tetap berubah, email tidak. - Tambah test: super-admin bisa mengubah email akunnya sendiri, dan non super-admin bisa mengubah email profilnya sendiri. Fixes #1783 Co-Authored-By: Claude Haiku 5.5 --- app/Http/Controllers/User/UserController.php | 3 + tests/Feature/UserControllerTest.php | 84 +++++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/User/UserController.php b/app/Http/Controllers/User/UserController.php index a8a528d5f..ff6b57fe3 100644 --- a/app/Http/Controllers/User/UserController.php +++ b/app/Http/Controllers/User/UserController.php @@ -159,6 +159,9 @@ public function update(UserRequest $request, $id) try { $input = $request->validated(); + if ($id != $actor->id) { + unset($input['email']); + } $user = User::findOrFail($id); $this->handleFileUpload($request, $input, 'image', 'user', false); $user->update($input); diff --git a/tests/Feature/UserControllerTest.php b/tests/Feature/UserControllerTest.php index 441e64a6b..8f86b2c7b 100644 --- a/tests/Feature/UserControllerTest.php +++ b/tests/Feature/UserControllerTest.php @@ -253,6 +253,8 @@ public function test_update_updates_user_successfully_for_super_admin() 'role' => [$role->name], ]; + $originalEmail = $targetUser->email; + $response = $this->actingAs($superAdmin)->put(route('setting.user.update', $targetUser->id), $updateData); $response->assertRedirect(route('setting.user.index')); @@ -261,7 +263,61 @@ public function test_update_updates_user_successfully_for_super_admin() $this->assertDatabaseHas('users', [ 'id' => $targetUser->id, 'name' => 'Updated Name', - 'email' => $updateData['email'], + 'email' => $originalEmail, + ]); + } + + /** + * Test update method lets super admin change email of own account. + * + * @return void + */ + public function test_update_allows_super_admin_to_change_own_email() + { + $superAdmin = User::whereHas('roles', function ($query) { + $query->where('name', 'super-admin'); + })->first(); + + $newEmail = $this->faker->unique()->safeEmail; + + $response = $this->actingAs($superAdmin)->put(route('setting.user.update', $superAdmin->id), [ + 'name' => $superAdmin->name, + 'email' => $newEmail, + 'address' => 'Alamat Super Admin', + ]); + + $response->assertSessionHas('success'); + $this->assertDatabaseHas('users', [ + 'id' => $superAdmin->id, + 'email' => $newEmail, + ]); + } + + /** + * Test update method ignores email change when super admin edits another user. + * + * @return void + */ + public function test_update_ignores_email_change_when_editing_another_user() + { + $superAdmin = User::whereHas('roles', function ($query) { + $query->where('name', 'super-admin'); + })->first(); + + $targetEmail = $this->faker->unique()->safeEmail; + $targetUser = User::factory()->create(['email' => $targetEmail]); + + $response = $this->actingAs($superAdmin)->put(route('setting.user.update', $targetUser->id), [ + 'name' => 'Nama Baru', + 'email' => $this->faker->unique()->safeEmail, + 'address' => 'Alamat Baru', + ]); + + $response->assertSessionHas('success'); + $this->assertDatabaseHas('users', [ + 'id' => $targetUser->id, + 'name' => 'Nama Baru', + 'email' => $targetEmail, ]); } @@ -324,6 +380,32 @@ public function test_update_forbids_non_super_admin_from_updating_another_user() ]); } + /** + * Test update method lets non super admin change email of own profile. + * + * @return void + */ + public function test_update_allows_non_super_admin_to_change_own_email() + { + $actor = User::factory()->create(); + $actor->assignRole('administrator-website'); + $actor->givePermissionTo('access.setting'); + + $newEmail = $this->faker->unique()->safeEmail; + + $response = $this->actingAs($actor)->put(route('setting.user.update', $actor->id), [ + 'name' => $actor->name, + 'email' => $newEmail, + 'address' => 'Alamat Sendiri', + ]); + + $response->assertRedirect(route('dashboard')); + $this->assertDatabaseHas('users', [ + 'id' => $actor->id, + 'email' => $newEmail, + ]); + } + /** * Test update method ignores role from non super admin editing own profile. * From 1d22b9715120bc4abb1eb85d42b1a9478c482134 Mon Sep 17 00:00:00 2001 From: Ahmad Afandi Date: Wed, 28 Oct 2026 07:35:38 +0700 Subject: [PATCH 4/5] test(auth): perbaiki setup role admin-kecamatan di AuthorizationTest Role dibuat dengan firstOrCreate tanpa guard_name, sehingga Spatie mengambil role guard api yang tidak punya permission. Akibatnya test dashboard admin-kecamatan mendapat 403. - Tentukan guard_name web, sama seperti seeder. - Berikan permission access.dashboard secara eksplisit, supaya test tidak bergantung pada isi database. Co-Authored-By: Claude Haiku 5.5 --- tests/Feature/Auth/AuthorizationTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Auth/AuthorizationTest.php b/tests/Feature/Auth/AuthorizationTest.php index 56880daa4..d2a6c8646 100644 --- a/tests/Feature/Auth/AuthorizationTest.php +++ b/tests/Feature/Auth/AuthorizationTest.php @@ -12,6 +12,7 @@ use App\Models\User; use Illuminate\Foundation\Testing\DatabaseTransactions; +use Spatie\Permission\Models\Permission; use Spatie\Permission\Models\Role; uses(DatabaseTransactions::class); @@ -21,7 +22,10 @@ // Ensure roles exist $this->adminRole = Role::firstOrCreate(['name' => 'super-admin']); - $this->kecamatanRole = Role::firstOrCreate(['name' => 'admin-kecamatan']); + $this->kecamatanRole = Role::firstOrCreate(['name' => 'admin-kecamatan', 'guard_name' => 'web']); + $this->kecamatanRole->syncPermissions([ + Permission::firstOrCreate(['name' => 'access.dashboard', 'guard_name' => 'web']), + ]); $this->admin = User::factory()->create(['status' => 1]); $this->admin->assignRole($this->adminRole); From e55c7974ace4fe4ec9c8a34a61378c6795a22abd Mon Sep 17 00:00:00 2001 From: Ahmad Afandi Date: Wed, 28 Oct 2026 07:47:54 +0700 Subject: [PATCH 5/5] test(user): sesuaikan UserManagementCrudTest dengan pembatasan email (#1783) Test update oleh super-admin masih mengharapkan email user lain berubah. Sesuai #1783, email user lain tidak boleh diubah lewat request, sehingga assertion diubah: nama berubah, email tetap. Co-Authored-By: Claude Haiku 5.5 --- tests/Feature/Settings/UserManagementCrudTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Settings/UserManagementCrudTest.php b/tests/Feature/Settings/UserManagementCrudTest.php index c30561d27..863629968 100644 --- a/tests/Feature/Settings/UserManagementCrudTest.php +++ b/tests/Feature/Settings/UserManagementCrudTest.php @@ -147,6 +147,8 @@ $targetUser = User::where('id', '!=', $superAdmin->id)->first(); $role = Role::where('name', 'administrator-website')->first(); + $originalEmail = $targetUser->email; + $updateData = [ 'name' => 'Updated Name', 'email' => 'updated@test.com', @@ -160,10 +162,11 @@ $response->assertRedirect(route('setting.user.index')); $response->assertSessionHas('success'); + // Email user lain tidak boleh diubah (lihat #1783) $this->assertDatabaseHas('users', [ 'id' => $targetUser->id, 'name' => 'Updated Name', - 'email' => 'updated@test.com', + 'email' => $originalEmail, ]); });