[pandigresik]

Pull Request: Fix: Keamanan manajemen user (update tanpa kepemilikan, role mentah, dan email di sisi klien)

Description

Tiga celah di UserController::update() dan route profil diperbaiki dalam satu PR. Update user tidak mengecek kepemilikan dan memproses role mentah dari request, sehingga administrator-website bisa mengubah user lain dan memberi role super-admin. Pembatasan email hanya ada di view, sehingga nilai email user lain tetap bisa diubah lewat request. Di sisi lain, route profil sendiri ikut terkunci oleh access.setting, sehingga user seperti admin-kecamatan tidak bisa membuka profilnya sendiri.

Changes made:

1. Authorization: app/Http/Controllers/User/UserController.php — non super-admin yang mengakses update() untuk user lain mendapat 403. Pengecekan ditempatkan sebelum blok try, karena catch (\Exception) akan menangkap abort(403).
2. Role guard: app/Http/Controllers/User/UserController.php — syncRoles() hanya dijalankan untuk super-admin. Input role dari role lain diabaikan.
3. Pembatasan email: app/Http/Controllers/User/UserController.php — email dibuang dari input jika user yang diubah bukan akun yang sedang login. Berlaku untuk semua role, termasuk super-admin, sesuai aturan di resources/views/user/form.blade.php.
4. Route profil: routes/web.php — setting.user.edit dan setting.user.update dipindah ke grup yang hanya memakai auth. Kepemilikan tetap dicek di controller. Route pengaturan user lain dan foto profil tetap dilindungi access.setting.
5. Test: tests/Feature/UserControllerTest.php — test baru dan perubahan test super-admin, mencakup 403 untuk user lain, role diabaikan, email user lain tidak berubah, perubahan email akun sendiri, dan profil sendiri tanpa access.setting.

Reason for change:

• Point 1: update() tidak mengecek kepemilikan, berbeda dengan edit() yang sudah mengecek. Akibatnya user non super-admin bisa mengubah akun siapa pun berdasarkan id.
• Point 2: role dibaca langsung dari request dan tidak divalidasi, sehingga role super-admin bisa diberikan.
• Point 3: Pembatasan email hanya di view. Field readonly tetap ikut terkirim, sehingga server harus menegakkan aturan yang sama.
• Point 4: Route profil sendiri tidak seharusnya membutuhkan access.setting. Role admin-kecamatan sengaja tidak punya akses pengaturan, tetapi tetap perlu mengelola profilnya sendiri.

Impact of change:

✅ Akun user lain terlindungi: non super-admin tidak bisa lagi mengubah user lain, dan super-admin tidak bisa mengubah email user lain melalui request.
✅ Eskalasi privilege ditutup: role super-admin hanya bisa diberikan oleh super-admin.
✅ Profil sendiri tetap bisa dikelola: nama, alamat, dan email profil sendiri bisa diubah oleh semua user yang login, tanpa membuka menu pengaturan lain.

Related Issue

• Solution for fix related to issue [Security] Update user tanpa cek kepemilikan dan role mentah dapat memberi super-admin #1782
• Solution for fix related to issue [Security] Pembatasan edit email user hanya di sisi klien #1783
• Perbaikan route profil sendiri tidak punya issue terpisah. Terkait dengan kasus admin-kecamatan yang tidak bisa membuka profilnya.

#1782
#1783

Steps to Reproduce

Before fix (problem):

1. Login sebagai user dengan role administrator-website
2. Kirim PUT /setting/user/update/{id} dengan id user lain, mengisi name dan email baru
3. ❌ Request diterima dan data user lain berubah
4. Kirim PUT ke profil sendiri dengan role[]=super-admin
5. ❌ Role super-admin berhasil diberikan ke akun non super-admin
6. Login sebagai admin-kecamatan, buka GET /setting/user/edit/{id} untuk profil sendiri
7. ❌ 403, karena route memakai access.setting

After fix (solution):

1. Login sebagai user dengan role administrator-website
2. Kirim PUT /setting/user/update/{id} dengan id user lain
3. ✅ Response 403 Anda tidak berhak mengubah user tersebut, data user lain tidak berubah
4. Kirim PUT ke profil sendiri dengan role[]=super-admin
5. ✅ Field role diabaikan, role akun tetap administrator-website
6. Login sebagai admin-kecamatan, buka GET /setting/user/edit/{id} untuk profil sendiri
7. ✅ Form profil tampil dan bisa disimpan

Testing on related features:

• Edit profil sendiri (non super-admin dengan access.setting) ✅ Test test_update_updates_profile_and_redirects_to_dashboard_for_regular_user lulus
• Edit profil sendiri tanpa access.setting ✅ Test test_edit_displays_own_profile_without_access_setting_permission dan test_update_own_profile_without_access_setting_permission lulus
• Edit user lain tanpa access.setting tetap ditolak ✅ Test test_edit_forbids_another_user_without_access_setting_permission lulus
• Daftar user tanpa access.setting tetap ditolak ✅ Test test_index_forbids_user_without_access_setting_permission lulus
• Update user oleh super-admin ✅ Test test_update_updates_user_successfully_for_super_admin lulus (email user lain tidak berubah)
• Super-admin mengubah email akunnya sendiri ✅ Test test_update_allows_super_admin_to_change_own_email lulus
• Form edit email (readonly untuk user lain, editable untuk diri sendiri) ✅ Test edit form lulus
• Dashboard untuk admin-kecamatan ✅ AuthorizationTest lulus. Sebelumnya gagal karena setup test memakai role guard api, sudah diperbaiki di commit 1d22b971.

Checklist

• I have complied with script writing rules
• I have followed pull request review process
• I have created unit test/integration test to verify the fix
• Manual testing has been done in development environment
• No console errors or warnings
• Code has been reviewed by at least 1 person

Technical Details

Technical Explanation

Alur update() setelah perbaikan:

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();
        if ($id != $actor->id) {
            unset($input['email']);
        }
        ...
        if ($isSuperAdmin && !empty($request->role)) {
            $user->syncRoles($roles);
        }
        ...

Route profil setelah perbaikan:

// 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 photo-profil dan update-photo masih memakai action_permission:access.setting, sehingga admin-kecamatan belum bisa mengganti foto profilnya sendiri. Ini di luar cakupan PR ini.

Route PUT /setting/user/update/{id} untuk user lain sekarang juga memberi 403 bagi non super-admin. Sebelumnya UserController::update() menerima request tersebut.

Perbaikan ini ada di level controller dan tidak bergantung pada nama permission access.setting.edit, yang tidak ada di seeder.

Configuration changes

Tidak ada.

Dependencies added

No new dependencies

Testing

Manual Testing

• Login sebagai administrator-website, coba edit user lain → harus 403
• Login sebagai administrator-website, edit profil sendiri → berhasil
• Login sebagai admin-kecamatan, buka dan simpan profil sendiri → berhasil
• Login sebagai admin-kecamatan, buka /setting/user → 403
• Login sebagai super-admin, edit user lain → nama dan role berubah, email tidak
• Regression: halaman daftar user dan form edit tetap tampil

Automated Testing

• Feature Test - Tests\Feature\UserControllerTest (22 test lulus, termasuk test baru dan test yang diubah)
• Feature Test - Tests\Feature\Auth, Permission, UserControllerTest: 82 lulus (termasuk AuthorizationTest setelah perbaikan setup test)
• Feature Test - Settings, ExportRouteMiddlewareTest, TokenRegisteredMiddlewareTest dijalankan bersama kelompok di atas sebelum perbaikan AuthorizationTest: lulus semua kecuali AuthorizationTest, yang kini sudah diperbaiki
• Integration Test - belum dijalankan
• Playwright Test - belum dijalankan

Catatan: suite Feature penuh belum bisa dijalankan dengan pest --parallel, karena tests/Feature/JumlahDashboardTest.php memakai use Mockery; dan gagal saat dimuat. File itu tidak diubah dalam PR ini.

Browser Compatibility (if applicable)

Tidak ada perubahan UI.

Screenshots / Video

simplescreenrecorder-2026-10-28_07.41.50.mp4

Breaking Changes

Ya, perilaku ini berubah:

• Non super-admin yang mengirim PUT ke user lain sebelumnya berhasil, sekarang mendapat 403.
• Super-admin yang mengirim PUT ke user lain dengan email baru: nama dan role tetap diproses, tetapi email diabaikan.
• Field role dari non super-admin sebelumnya diproses, sekarang diabaikan tanpa error.

Migration Guide

Not required

References

• CWE-639 / CWE-269: https://cwe.mitre.org/data/definitions/639.html
• CWE-602: https://cwe.mitre.org/data/definitions/602.html
• OWASP A01:2025 Broken Access Control
• OWASP ASVS 8.2.1, 8.3.1

---

Additional notes:

• Pembatasan email diterapkan untuk semua role termasuk super-admin, mengikuti aturan di form. Kalau reviewer ingin super-admin tetap bisa mengganti email user lain, perlu mengubah controller dan view bersama-sama, serta test test_update_ignores_email_change_when_editing_another_user.
• Issue [Security] Update user tanpa cek kepemilikan dan role mentah dapat memberi super-admin #1782 menyebut perlu konfirmasi bahwa access.setting.edit tidak ada di tabel permissions produksi. Perbaikan ini tidak bergantung pada hal itu, tetapi belum diverifikasi terhadap data produksi.

🤖 Generated with Claude Code

[github-actions]

🔄 AI PR Review sedang antri di server...

Proses review akan segera dimulai di background — hasil akan muncul sebagai komentar setelah selesai.
Powered by CrewAI · PR #1785