fix: generate token sinkronisasi#1607
Conversation
|
🔄 AI PR Review sedang antri di server...
|
apidong
left a comment
There was a problem hiding this comment.
🔒 Security Audit — PR #1607
PR: fix: generate token sinkronisasi
Audit Date: 24 Juni 2026
Files Reviewed: TokenController.php, FileUploadService.php
Executive Summary
| Severity | Count |
|---|---|
| 🔴 CRITICAL | 1 |
| 🟠HIGH | 1 |
| 🟡 MEDIUM | 1 |
| Total | 3 |
Verdict: ✅ Bisa merge dengan catatan — temuan CRITICAL adalah pre-existing issue yang perlu dijadikan issue terpisah.
✅ Temuan Positif
PR ini memiliki beberapa aspek positif:
- Bug fix yang valid — Memperbaiki masalah nyata di mana token tidak tersimpan ke database, sehingga middleware
token.registeredselalu menolak request. - Penggunaan
updateOrCreateyang tepat — Memastikan hanya ada SATU record token di database, sehingga token lama otomatis tergantikan saat generate baru. - FileUploadService improvement — Mengatasi masalah compatibility di Windows/Laragon environment di mana
$file->getRealPath()return false. - Keamanan tetap terjaga — Path traversal protection, MIME validation, dan extension blacklist tetap berfungsi di
FileUploadService.
🔴 CRITICAL-01: Token JWT Tersimpan sebagai Plaintext di Database
Lokasi: app/Http/Controllers/Api/TokenController.php:54-57
SettingAplikasi::updateOrCreate(
['key' => 'api_key_opendk'],
['value' => $token] // � Token JWT disimpan mentah
);Dampak:
- Token JWT dapat dibaca langsung oleh siapa pun yang memiliki akses database (admin DB, backup leak, SQL injection).
- Token JWT berisi informasi user (
sub,iat,exp) yang bisa di-decode tanpa secret key. - Pelanggaran prinsip defense-in-depth — jika database bocor, seluruh API langsung bisa diakses.
Catatan: Ini adalah pre-existing issue — middleware TokenRegistered yang sudah ada juga membandingkan token secara plaintext. PR ini hanya mengikuti pola yang sudah ada. Namun, sebaiknya diperbaiki di issue terpisah.
Rekomendasi: Simpan hash token, bukan token mentah
// TokenController.php
SettingAplikasi::updateOrCreate(
['key' => 'api_key_opendk'],
['value' => hash('sha256', $token)]
);// TokenRegistered.php
$checkToken = SettingAplikasi::where([
'key' => 'api_key_opendk',
'value' => hash('sha256', $token)
])->exists();Mengapa hash (bukan encrypt):
- Middleware hanya perlu verifikasi (match/tidak), tidak perlu ambil token asli
- Lebih sederhana, tidak perlu key management
- Lebih aman — tidak bisa di-reverse meskipun database bocor
- Tidak bergantung pada
APP_KEY(jikaAPP_KEYdi-rotate, encrypt bisa broken)
🟠HIGH-01: Resource Leak pada File Stream Handling
Lokasi: app/Services/FileUploadService.php:38-44
$stream = fopen($file->getPathname(), 'r'); // � Bisa return false
$path = trim($directory.'/'.$safeFileName, '/');
Storage::disk('public')->put($path, $stream); // � Bisa throw exception
if (is_resource($stream)) {
fclose($stream); // � Tidak dieksekusi jika exception
}Masalah:
- Jika
fopen()gagal mengembalikanfalse,Storage::put()akan menerimafalsesebagai data. - Jika
Storage::put()throw exception,fclose()tidak pernah dieksekusi = resource leak. - Tidak ada validasi bahwa
fopen()berhasil.
Rekomendasi: Gunakan try-finally pattern
$stream = fopen($file->getPathname(), 'r');
if ($stream === false) {
throw new \RuntimeException('Failed to open file: ' . $file->getPathname());
}
try {
$path = trim($directory . '/' . $safeFileName, '/');
Storage::disk('public')->put($path, $stream);
} finally {
if (is_resource($stream)) {
fclose($stream);
}
}🟡 MEDIUM-01: Masa Berlaku Token 10 Tahun Terlalu Lama
Lokasi: app/Http/Controllers/Api/TokenController.php:50
Config::set('jwt.ttl', 10 * 365 * 24 * 60); // 10 tahun dalam menitDampak:
- Jika token bocor, attacker memiliki 10 tahun untuk mengeksploitasi.
- Melanggar OWASP recommendation: token harus memiliki masa berlaku terbatas dengan mekanisme refresh.
Catatan: Ini juga pre-existing issue, bukan perubahan dari PR ini.
Rekomendasi: Kurangi TTL menjadi 90 hari (3 bulan) untuk API sync, dan tambahkan mekanisme refresh token atau auto-renewal.
📋 Ringkasan Rekomendasi
| # | Priority | Action | Effort |
|---|---|---|---|
| 1 | 🔴 WAJIB | Hash token sebelum simpan ke DB (issue terpisah) | 15 menit |
| 2 | 🟠DISARANKAN | Tambah try-finally untuk stream handling | 10 menit |
| 3 | 🟡 OPSIONAL | Kurangi JWT TTL dari 10 tahun ke 90 hari | 5 menit |
� Kesimpulan
PR ini layak merge karena memperbaiki bug fungsional yang valid. Mekanisme updateOrCreate sudah benar dalam menangani token replacement (token lama otomatis tergantikan). Temuan CRITICAL (plaintext storage) adalah pre-existing issue yang sudah ada di middleware TokenRegistered sebelum PR ini — sebaiknya dijadikan issue terpisah untuk perbaikan ke depan.
💡 Saran: Buat issue terpisah untuk implementasi hash token agar seluruh flow token generation dan validasi bisa diperbaiki secara konsisten.
apidong
left a comment
There was a problem hiding this comment.
Security Audit - PR #1607
PR: fix: generate token sinkronisasi
Audit Date: 24 Juni 2026
Files Reviewed: TokenController.php, FileUploadService.php
Executive Summary
| Severity | Count |
|---|---|
| CRITICAL | 1 |
| HIGH | 1 |
| MEDIUM | 1 |
| Total | 3 |
Verdict: Bisa merge dengan catatan - temuan CRITICAL adalah pre-existing issue yang perlu dijadikan issue terpisah.
Temuan Positif
PR ini memiliki beberapa aspek positif:
- Bug fix yang valid - Memperbaiki masalah nyata di mana token tidak tersimpan ke database, sehingga middleware
token.registeredselalu menolak request. - Penggunaan
updateOrCreateyang tepat - Memastikan hanya ada SATU record token di database, sehingga token lama otomatis tergantikan saat generate baru. - FileUploadService improvement - Mengatasi masalah compatibility di Windows/Laragon environment di mana
$file->getRealPath()return false. - Keamanan tetap terjaga - Path traversal protection, MIME validation, dan extension blacklist tetap berfungsi di
FileUploadService.
[CRITICAL] Token JWT Tersimpan sebagai Plaintext di Database
Lokasi: app/Http/Controllers/Api/TokenController.php:54-57
SettingAplikasi::updateOrCreate(
['key' => 'api_key_opendk'],
['value' => $token] // Token JWT disimpan mentah
);Dampak:
- Token JWT dapat dibaca langsung oleh siapa pun yang memiliki akses database (admin DB, backup leak, SQL injection).
- Token JWT berisi informasi user (
sub,iat,exp) yang bisa di-decode tanpa secret key. - Pelanggaran prinsip defense-in-depth - jika database bocor, seluruh API langsung bisa diakses.
Catatan: Ini adalah pre-existing issue - middleware TokenRegistered yang sudah ada juga membandingkan token secara plaintext. PR ini hanya mengikuti pola yang sudah ada. Namun, sebaiknya diperbaiki di issue terpisah.
Rekomendasi: Simpan hash token, bukan token mentah
// TokenController.php
SettingAplikasi::updateOrCreate(
['key' => 'api_key_opendk'],
['value' => hash('sha256', $token)]
);// TokenRegistered.php
$checkToken = SettingAplikasi::where([
'key' => 'api_key_opendk',
'value' => hash('sha256', $token)
])->exists();Mengapa hash (bukan encrypt):
- Middleware hanya perlu verifikasi (match/tidak), tidak perlu ambil token asli
- Lebih sederhana, tidak perlu key management
- Lebih aman - tidak bisa di-reverse meskipun database bocor
- Tidak bergantung pada
APP_KEY(jikaAPP_KEYdi-rotate, encrypt bisa broken)
[HIGH] Resource Leak pada File Stream Handling
Lokasi: app/Services/FileUploadService.php:38-44
$stream = fopen($file->getPathname(), 'r'); // Bisa return false
$path = trim($directory.'/'.$safeFileName, '/');
Storage::disk('public')->put($path, $stream); // Bisa throw exception
if (is_resource($stream)) {
fclose($stream); // Tidak dieksekusi jika exception
}Masalah:
- Jika
fopen()gagal mengembalikanfalse,Storage::put()akan menerimafalsesebagai data. - Jika
Storage::put()throw exception,fclose()tidak pernah dieksekusi = resource leak. - Tidak ada validasi bahwa
fopen()berhasil.
Rekomendasi: Gunakan try-finally pattern
$stream = fopen($file->getPathname(), 'r');
if ($stream === false) {
throw new \RuntimeException('Failed to open file: ' . $file->getPathname());
}
try {
$path = trim($directory . '/' . $safeFileName, '/');
Storage::disk('public')->put($path, $stream);
} finally {
if (is_resource($stream)) {
fclose($stream);
}
}[MEDIUM] Masa Berlaku Token 10 Tahun Terlalu Lama
Lokasi: app/Http/Controllers/Api/TokenController.php:50
Config::set('jwt.ttl', 10 * 365 * 24 * 60); // 10 tahun dalam menitDampak:
- Jika token bocor, attacker memiliki 10 tahun untuk mengeksploitasi.
- Melanggar OWASP recommendation: token harus memiliki masa berlaku terbatas dengan mekanisme refresh.
Catatan: Ini juga pre-existing issue, bukan perubahan dari PR ini.
Rekomendasi: Kurangi TTL menjadi 90 hari (3 bulan) untuk API sync, dan tambahkan mekanisme refresh token atau auto-renewal.
Ringkasan Rekomendasi
| # | Priority | Action | Effort |
|---|---|---|---|
| 1 | CRITICAL | Hash token sebelum simpan ke DB (issue terpisah) | 15 menit |
| 2 | HIGH | Tambah try-finally untuk stream handling | 10 menit |
| 3 | MEDIUM | Kurangi JWT TTL dari 10 tahun ke 90 hari | 5 menit |
Kesimpulan
PR ini layak merge karena memperbaiki bug fungsional yang valid. Mekanisme updateOrCreate sudah benar dalam menangani token replacement (token lama otomatis tergantikan). Temuan CRITICAL (plaintext storage) adalah pre-existing issue yang sudah ada di middleware TokenRegistered sebelum PR ini - sebaiknya dijadikan issue terpisah untuk perbaikan ke depan.
Saran: Buat issue terpisah untuk implementasi hash token agar seluruh flow token generation dan validasi bisa diperbaiki secara konsisten.
pandigresik
left a comment
There was a problem hiding this comment.
ikuti rekomendasi
[HIGH] Resource Leak pada File Stream Handling
lihat komentar review AI
|
Ini menggunakan rilis-dev simplescreenrecorder-2026-06-25_10.48.18.mp4setelah buat token, klik tombol simpan. Alurnya seperti itu agar sama dengan pengaturan lainnya |
issue terkait https://github.com/OpenSID/premium/issues/6518

issue # [Tambahkan link issue di sini jika ada]
🎯 Deskripsi
Pull request ini memperbaiki bug kritis pada fitur sinkronisasi OpenSID di mana permintaan API dari OpenSID (misalnya
/api/v1/penduduk/storedata) selalu ditolak dengan error "Token not registered" atau "Unauthenticated".Akar permasalahannya adalah saat Admin OpenDK menekan tombol "Buat Token Baru", token JWT yang di-generate tersebut hanya dikembalikan untuk ditampilkan ke layar dan tidak pernah disimpan ke dalam database. Hal ini mengakibatkan middleware validasi (
token.registered) selalu menolak karena nilai token di tabeldas_settingtetap kosong atau kadaluwarsa. PR ini memastikan token selalu direkam ke database setiap kali dibuat.PR ini juga mencakup perbaikan keamanan lanjutan berdasarkan hasil security audit terhadap file yang sama, meliputi:
Config::set()yang mengubah state JWT global secara berbahaya (CRITICAL).try-finallypada file stream handling untuk mencegah resource leak (HIGH).🛠️ Perubahan yang Dilakukan
1.
app/Http/Controllers/Api/TokenController.phpBug Fix — Token tidak tersimpan ke database:
use App\Models\SettingAplikasi;.SettingAplikasi::updateOrCreate(['key' => 'api_key_opendk'], ['value' => $token])di dalamindex()agar token selalu direkam sebelum dikembalikan ke response.Security Fix — CRITICAL:
Config::set()JWT global override:Config::set('jwt.ttl', 10 * 365 * 24 * 60)yang mengubah TTL JWT secara global untuk seluruh lifecycle request (berpotensi race condition di concurrent request) dengan masa berlaku ekstrem 10 tahun.JWTAuth::customClaims(['exp' => now()->addYear()->timestamp])->fromUser($user)— klaimexpdi-set langsung pada payload token secara terisolasi, TTL efektif dikurangi menjadi 1 tahun.Security Fix — MEDIUM: Tidak ada audit trail:
Log::info(...)yang merekamuser_id,expires_at, danipsetiap kali token digenerate.expires_atdantoken_typeuntuk transparansi ke client.2.
app/Services/FileUploadService.phpSecurity Fix — HIGH: Resource leak pada file stream handling:
$stream === falsesetelahfopen()— jika file gagal dibuka, langsung throwRuntimeExceptiondengan pesan deskriptif.Storage::disk('public')->put()dengan bloktry-finallyagarfclose()selalu dieksekusi meski terjadi exception, mencegah resource leak di environment manapun.✅ Test Cases yang Diimplementasikan
das_settingpada baris dengan kunciapi_key_opendk.token.registereddapat mengenali dan menerima Bearer token yang sesuai dengan yang ada di database.Storage::put()throw exception.storage/logs/laravel.log).📸 Cara Menjalankan Uji Coba Manual
php artisan tinker --execute="echo App\Models\SettingAplikasi::where('key', 'api_key_opendk')->first()->value;"tail -n 20 storage/logs/laravel.log # Harus muncul: "Token sinkronisasi digenerate" dengan user_id, expires_at, ipapi_key_opendkdi pengaturan aplikasi OpenSID.🔒 Security Checklist
Config::set()JWT global override; menggunakancustomClaims(['exp'])yang terisolasi per-token.try-finally;fopen()divalidasi sebelum digunakan.expires_atdantoken_type.