Skip to content

fix: generate token sinkronisasi#1607

Merged
affandii06 merged 6 commits into
rilis-devfrom
dev-fix-sinkronisasi
Jun 26, 2026
Merged

fix: generate token sinkronisasi#1607
affandii06 merged 6 commits into
rilis-devfrom
dev-fix-sinkronisasi

Conversation

@habibie11

@habibie11 habibie11 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

issue terkait https://github.com/OpenSID/premium/issues/6518
image

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 tabel das_setting tetap 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:

  • Penghapusan Config::set() yang mengubah state JWT global secara berbahaya (CRITICAL).
  • Penerapan try-finally pada file stream handling untuk mencegah resource leak (HIGH).
  • Penambahan audit trail logging dan metadata response (MEDIUM).

🛠️ Perubahan yang Dilakukan

1. app/Http/Controllers/Api/TokenController.php

Bug Fix — Token tidak tersimpan ke database:

  • Menambahkan use App\Models\SettingAplikasi;.
  • Menambahkan SettingAplikasi::updateOrCreate(['key' => 'api_key_opendk'], ['value' => $token]) di dalam index() agar token selalu direkam sebelum dikembalikan ke response.

Security Fix — CRITICAL: Config::set() JWT global override:

  • Dihapus: 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.
  • Diganti dengan: JWTAuth::customClaims(['exp' => now()->addYear()->timestamp])->fromUser($user) — klaim exp di-set langsung pada payload token secara terisolasi, TTL efektif dikurangi menjadi 1 tahun.

Security Fix — MEDIUM: Tidak ada audit trail:

  • Menambahkan Log::info(...) yang merekam user_id, expires_at, dan ip setiap kali token digenerate.
  • Response JSON diperluas dengan field expires_at dan token_type untuk transparansi ke client.
- use Illuminate\Support\Facades\Config;
+ use Illuminate\Support\Facades\Log;

  public function index()
  {
-     // Set the token's expiration time, 10 tahun
-     Config::set('jwt.ttl', 10 * 365 * 24 * 60);
      $user = Auth::user();
-     $token = JWTAuth::fromUser($user);
+     $expiresAt = now()->addYear()->timestamp;
+     $token = JWTAuth::customClaims(['exp' => $expiresAt])->fromUser($user);

-     return response()->json(['token' => $token]);
+     Log::info('Token sinkronisasi digenerate', [
+         'user_id'    => $user->id,
+         'expires_at' => now()->addYear()->toDateTimeString(),
+         'ip'         => request()->ip(),
+     ]);
+
+     return response()->json([
+         'token'      => $token,
+         'expires_at' => now()->addYear()->toDateTimeString(),
+         'token_type' => 'Bearer',
+     ]);
  }

2. app/Services/FileUploadService.php

Security Fix — HIGH: Resource leak pada file stream handling:

  • Menambahkan validasi $stream === false setelah fopen() — jika file gagal dibuka, langsung throw RuntimeException dengan pesan deskriptif.
  • Membungkus Storage::disk('public')->put() dengan blok try-finally agar fclose() selalu dieksekusi meski terjadi exception, mencegah resource leak di environment manapun.
  $stream = fopen($file->getPathname(), 'r');
+ if ($stream === false) {
+     throw new \RuntimeException('Failed to open file: ' . $file->getPathname());
+ }
+
  $path = trim($directory . '/' . $safeFileName, '/');
- Storage::disk('public')->put($path, $stream);
- if (is_resource($stream)) {
-     fclose($stream);
- }
+
+ try {
+     Storage::disk('public')->put($path, $stream);
+ } finally {
+     if (is_resource($stream)) {
+         fclose($stream);
+     }
+ }

✅ Test Cases yang Diimplementasikan

  • Saat Admin menekan tombol "Buat Token Baru", UI tetap menampilkan token secara normal.
  • Token yang terbentuk berhasil tersimpan ke dalam tabel das_setting pada baris dengan kunci api_key_opendk.
  • Middleware token.registered dapat mengenali dan menerima Bearer token yang sesuai dengan yang ada di database.
  • Validasi berikutnya (seperti "Isian kode desa yang dipilih tidak valid") akan berjalan normal jika data desa dari OpenSID belum terdaftar.
  • TTL token tidak lagi memodifikasi state konfigurasi JWT global — tidak ada race condition pada concurrent request.
  • File stream selalu ditutup (tidak ada resource leak) bahkan jika Storage::put() throw exception.
  • Setiap pembuatan token terekam di log aplikasi (storage/logs/laravel.log).

📸 Cara Menjalankan Uji Coba Manual

  1. Pastikan Anda sudah login ke aplikasi web OpenDK sebagai Super Admin.
  2. Buka menu PengaturanAplikasi, lalu buka sub-menu tempat API Key dikelola.
  3. Klik tombol Buat Token Baru dan salin token yang ditampilkan.
  4. Verifikasi token tersimpan di database:
    php artisan tinker --execute="echo App\Models\SettingAplikasi::where('key', 'api_key_opendk')->first()->value;"
  5. Verifikasi log audit terekam:
    tail -n 20 storage/logs/laravel.log
    # Harus muncul: "Token sinkronisasi digenerate" dengan user_id, expires_at, ip
  6. Tempelkan token tersebut pada kolom api_key_opendk di pengaturan aplikasi OpenSID.
  7. Lakukan uji coba sinkronisasi penduduk dari aplikasi OpenSID.
    • Catatan: Pastikan bahwa Kode Desa yang ada di OpenSID sudah terdaftar terlebih dahulu di halaman Data Desa OpenDK agar tidak terkena validasi "Kode Desa Tidak Dikenal".

🔒 Security Checklist

  • CRITICAL — Dihapus Config::set() JWT global override; menggunakan customClaims(['exp']) yang terisolasi per-token.
  • HIGH — File stream di-wrap dengan try-finally; fopen() divalidasi sebelum digunakan.
  • MEDIUM — Audit trail logging ditambahkan; response menyertakan expires_at dan token_type.

@github-actions

Copy link
Copy Markdown

🔄 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 #1607

@apidong apidong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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:

  1. Bug fix yang valid — Memperbaiki masalah nyata di mana token tidak tersimpan ke database, sehingga middleware token.registered selalu menolak request.
  2. Penggunaan updateOrCreate yang tepat — Memastikan hanya ada SATU record token di database, sehingga token lama otomatis tergantikan saat generate baru.
  3. FileUploadService improvement — Mengatasi masalah compatibility di Windows/Laragon environment di mana $file->getRealPath() return false.
  4. 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 (jika APP_KEY di-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:

  1. Jika fopen() gagal mengembalikan false, Storage::put() akan menerima false sebagai data.
  2. Jika Storage::put() throw exception, fclose() tidak pernah dieksekusi = resource leak.
  3. 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 menit

Dampak:

  • 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 apidong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Bug fix yang valid - Memperbaiki masalah nyata di mana token tidak tersimpan ke database, sehingga middleware token.registered selalu menolak request.
  2. Penggunaan updateOrCreate yang tepat - Memastikan hanya ada SATU record token di database, sehingga token lama otomatis tergantikan saat generate baru.
  3. FileUploadService improvement - Mengatasi masalah compatibility di Windows/Laragon environment di mana $file->getRealPath() return false.
  4. 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 (jika APP_KEY di-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:

  1. Jika fopen() gagal mengembalikan false, Storage::put() akan menerima false sebagai data.
  2. Jika Storage::put() throw exception, fclose() tidak pernah dieksekusi = resource leak.
  3. 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 menit

Dampak:

  • 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 pandigresik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ikuti rekomendasi
[HIGH] Resource Leak pada File Stream Handling
lihat komentar review AI

@pandigresik

Copy link
Copy Markdown
Contributor

Ini menggunakan rilis-dev

simplescreenrecorder-2026-06-25_10.48.18.mp4

setelah buat token, klik tombol simpan. Alurnya seperti itu agar sama dengan pengaturan lainnya

@affandii06 affandii06 merged commit e8e2ddc into rilis-dev Jun 26, 2026
@affandii06 affandii06 deleted the dev-fix-sinkronisasi branch June 26, 2026 02:54
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.

4 participants