From 2021962438ae9603c32ece60b5df87788e9d35ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ersin=20KO=C3=87?= Date: Fri, 11 Sep 2026 20:29:40 +0300 Subject: [PATCH] fix(admin): son aktif admin korumasi es zamanli isteklerle asilabiliyordu Kok neden: "son aktif admin" korumasi check-then-act'ti - other_active_admin_exists() kilitsiz bir SELECT COUNT(...) yapiyor, her uc nokta (toggle_status.php ve user_collect_input() uzerinden update.php) sonra AYRI bir UPDATE calistiryordu. Tam iki aktif admin varken birbirini es zamanli pasiflestiren/dusuren iki istek de "baska admin var" gorup gecebilir ve ikisi de commit eder: sistem SIFIR aktif adminle kalir (tam kilitleme; kurtarma dogrudan DB mudahalesi ister). Kanit (round-6 proof'u, yerel MariaDB): gercek iki isci sureci - her biri kendi PDO baglantisiyla - marker dosyalariyla kesisme zorlandi; her iki guard da UPDATE'lerden once calisti, sonucta 0 aktif admin (FAIL). DuzeItSonrasi: bir isci guncelledi, digeri guard tarafindan reddedildi, 1 aktif admin kaldi (PASS). Cozum: admin/users/_validate.php'ye last_admin_atomic_guard() eklendi - cagiranin transaction'i icinde TUM aktif admin satirlari FOR UPDATE ile kilitlenir; ayni satir kumesi ayni sirada kilitlendigi icin es zamanli islemler serilesir, ikincisi bloklanir ve commit edilmis guncel durumu okuyarak reddedilir. toggle_status.php ve update.php kontrol+UPDATE'i tek transaction'a tasiyor; eski yalniz-SELECT kontrolu UX on-kontrolu olarak kaldi. Dayanikli regresyon testi tools/last_admin_race_test.php (yerel MariaDB ister; users tablosunu yedekleyip geri yukler): tek baglanti karar semasi + iki surecli zorlanmis kesisme yarisi - 7 kontrol, 0 FAIL. php tools/smoke_test.php: 58 OK / 1 FAIL (yalnizca /var/www/riskops yerlesim yolu onaylamasi); php tools/fresh_bootstrap_test.php: 13 OK / 0 FAIL. Not: dal dogrudan main uzerindendir; acik diger PR'lere bagimliligi yoktur. --- admin/users/_validate.php | 41 +++++ admin/users/toggle_status.php | 38 +++-- admin/users/update.php | 26 +++- tools/last_admin_race_test.php | 267 +++++++++++++++++++++++++++++++++ 4 files changed, 362 insertions(+), 10 deletions(-) create mode 100644 tools/last_admin_race_test.php diff --git a/admin/users/_validate.php b/admin/users/_validate.php index 3a760e1..a1345eb 100644 --- a/admin/users/_validate.php +++ b/admin/users/_validate.php @@ -39,6 +39,47 @@ function other_active_admin_exists(int $excludeUserId): bool return (int)$stmt->fetchColumn() > 0; } +/** + * Son aktif admin korumasini ISLEM (transaction) ICINDE atomik uygular. + * + * NEDEN GEREKLI: other_active_admin_exists() + ayri bir UPDATE klasik + * check-then-act yarisir. Iki aktif admin ayni anda birbirini + * pasiflestirirse/dusurse her iki istek de "baska admin var" gorup + * gecebilir ve sistem sifir aktif adminle kalir (olculdu: round-6 + * proof'u, zorlanmis kesisme ile 0 admin). + * + * COZUM: cagiranin actigi transaction icinde TUM aktif admin satirlari + * FOR UPDATE ile kilitlenir. Es zamanli iki islem ayni satir kumesini + * ayni sirada kilitledigi icin serilesirler: ilki commit edene kadar + * ikincisi bloklanir; sonra guncel (commit edilmis) durumu okur ve + * artik tek admin kaldiysa REDDEDILIR. Kilitli okuma (FOR UPDATE) + * REPEATABLE READ altinda bile guncel surumu gorur. + * + * KULLANIM: cagiran beginTransaction() yapmis olmali; bu fonksiyon + * kilitleri tutar ve KARAR verir; UPDATE ve commit cagirana aittir. + * + * @param PDO $pdo Cagiranin transaction baglantisi. + * @param int $targetUserId Degistirilmek istenen kullanici. + * @param bool $targetWasActiveAdmin Hedef su an aktif bir admin mi? + * @return bool true = isleme devam edilebilir; false = hedef son aktif + * admin, islem geri cevrilmeli. + */ +function last_admin_atomic_guard(PDO $pdo, int $targetUserId, bool $targetWasActiveAdmin): bool +{ + if (!$targetWasActiveAdmin) { + return true; + } + $ids = $pdo->query( + "SELECT id FROM users WHERE role = 'admin' AND status = 1 FOR UPDATE" + )->fetchAll(PDO::FETCH_COLUMN); + foreach ($ids as $id) { + if ((int)$id !== $targetUserId) { + return true; // kilide alinmis kumede baska bir aktif admin var + } + } + return false; +} + /** * POST verisini okur ve doğrular. * diff --git a/admin/users/toggle_status.php b/admin/users/toggle_status.php index 4ba4e4c..7e64035 100644 --- a/admin/users/toggle_status.php +++ b/admin/users/toggle_status.php @@ -33,19 +33,39 @@ $newStatus = (int)$user['status'] === 1 ? 0 : 1; -if ($newStatus === 0) { - if ($id === auth_id()) { - flash('error', 'Kendi hesabınızı devre dışı bırakamazsınız.'); - redirect('/admin/users/'); - } - if ($user['role'] === ROLE_ADMIN && !other_active_admin_exists($id)) { +if ($newStatus === 0 && $id === auth_id()) { + flash('error', 'Kendi hesabınızı devre dışı bırakamazsınız.'); + redirect('/admin/users/'); +} + +/* --- Son aktif admin koruması: ATOMİK ------------------------------- + Kontrol ve UPDATE aynı transaction içinde; tüm aktif admin satırları + FOR UPDATE ile kilitlenir (bkz. last_admin_atomic_guard). Ayrı bir + SELECT COUNT(...) + ayrı UPDATE klasik check-then-act yarışıydı: iki + admin birbirini aynı anda pasifleştirdiğinde her iki istek de + "başka admin var" görüyor ve sistem yönetimsiz kalıyordu. */ +$pdo = db(); +$pdo->beginTransaction(); +try { + $wasActiveAdmin = $user['role'] === ROLE_ADMIN && (int)$user['status'] === 1; + + if ($newStatus === 0 && !last_admin_atomic_guard($pdo, $id, $wasActiveAdmin)) { + $pdo->rollBack(); flash('error', 'Bu, sistemdeki tek aktif admin hesabı. Önce başka bir admin tanımlayın.'); redirect('/admin/users/'); } -} -db()->prepare('UPDATE users SET status = :s WHERE id = :id') - ->execute([':s' => $newStatus, ':id' => $id]); + $pdo->prepare('UPDATE users SET status = :s WHERE id = :id') + ->execute([':s' => $newStatus, ':id' => $id]); + $pdo->commit(); +} catch (Throwable $ex) { + if ($pdo->inTransaction()) { + $pdo->rollBack(); + } + app_log('error', 'User status toggle failed: ' . $ex->getMessage(), ['user' => $id]); + flash('error', 'Durum değiştirilemedi.'); + redirect('/admin/users/'); +} audit('user_status_changed', 'user', $id, ['status' => (int)$user['status']], diff --git a/admin/users/update.php b/admin/users/update.php index d722a0c..52d87f9 100644 --- a/admin/users/update.php +++ b/admin/users/update.php @@ -34,7 +34,26 @@ } try { - db()->prepare( + $pdo = db(); + $pdo->beginTransaction(); + + /* --- Son aktif admin koruması: ATOMİK --------------------------- + Doğrulamadaki kontrol (user_collect_input) yalnızca UX; yarış + penceresini kapatamaz. Yetkiyi düşüren veya hesabı kapatan + UPDATE'ten önce aynı transaction içinde kilitli kontrol. */ + $wasActiveAdmin = $before['role'] === ROLE_ADMIN && (int)$before['status'] === 1; + $staysActiveAdmin = $data['role'] === ROLE_ADMIN && (int)$data['status'] === 1; + + if ($wasActiveAdmin && !$staysActiveAdmin + && !last_admin_atomic_guard($pdo, $id, true)) { + $pdo->rollBack(); + user_fail_back( + ['role' => 'Bu, sistemdeki tek aktif admin hesabı. Önce başka bir admin tanımlayın.'], + '/admin/users/edit.php?id=' . $id + ); + } + + $pdo->prepare( 'UPDATE users SET name = :n, email = :e, role = :r, department_id = :d, title = :t, phone = :ph, status = :s @@ -49,7 +68,12 @@ ':s' => $data['status'], ':id' => $id, ]); + + $pdo->commit(); } catch (Throwable $ex) { + if (isset($pdo) && $pdo->inTransaction()) { + $pdo->rollBack(); + } app_log('error', 'User update failed: ' . $ex->getMessage(), ['user' => $id]); old_set($_POST); flash('error', 'Değişiklikler kaydedilemedi.'); diff --git a/tools/last_admin_race_test.php b/tools/last_admin_race_test.php new file mode 100644 index 0000000..1650e1b --- /dev/null +++ b/tools/last_admin_race_test.php @@ -0,0 +1,267 @@ +prepare('SELECT role, status FROM users WHERE id = :id LIMIT 1'); +$stmt->execute([':id' => $targetId]); +$row = $stmt->fetch(); +$wasActiveAdmin = is_array($row) + && $row['role'] === ROLE_ADMIN && (int)$row['status'] === 1; + +$pdo->beginTransaction(); +try { + touch($doneMarker); // kilit alma denemesini once bildir (sonra bloklansa da) + + if (!last_admin_atomic_guard($pdo, $targetId, $wasActiveAdmin)) { + $pdo->rollBack(); + finish($resultFile, ['completed' => true, 'guard_rejected' => true, 'updated' => false]); + } + if (!wait_marker($waitMarker)) { + $pdo->rollBack(); + finish($resultFile, ['completed' => false, 'error' => 'marker timeout']); + } + $pdo->prepare('UPDATE users SET status = 0 WHERE id = :id')->execute([':id' => $targetId]); + $pdo->commit(); + finish($resultFile, ['completed' => true, 'guard_rejected' => false, 'updated' => true]); +} catch (Throwable $e) { + if ($pdo->inTransaction()) { $pdo->rollBack(); } + finish($resultFile, ['completed' => false, 'error' => $e->getMessage()]); +} +PHP; + +$tmp = sys_get_temp_dir() . '/riskops-race-' . getmypid(); +if (!is_dir($tmp)) { + mkdir($tmp, 0777, true); +} +$workerFile = $tmp . '/worker.php'; +file_put_contents($workerFile, $workerSrc); + +/* ---- Ortam ----------------------------------------------------------- */ + +try { + $pdo = new PDO( + 'mysql:host=127.0.0.1;port=3306;dbname=riskops;charset=utf8mb4', + 'riskops_user', + 'riskops_local_dev', + [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, + PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, + PDO::ATTR_TIMEOUT => 5] + ); +} catch (Throwable $e) { + echo "\nYerel MariaDB'ye baglanilamadi (konteyner: docker start riskops-mariadb).\n"; + echo 'Hata: ' . $e->getMessage() . "\n"; + echo "LAST ADMIN RACE TEST: FAIL\n"; + exit(1); +} + +if (!function_exists('last_admin_atomic_guard')) { + define('RISKOPS_BOOTSTRAPPED', true); // _validate.php direct-access guard'i icin + require_once $repoRoot . '/config/config.php'; + require_once $repoRoot . '/admin/users/_validate.php'; +} + +/* Kalinti scratch kayitlari temizle, users durumunu yedekle */ +$pdo->exec("DELETE FROM users WHERE email LIKE '" . RACE_SCRATCH_EMAIL . "'"); +$snapshot = $pdo->query('SELECT id, role, status, must_change_password FROM users ORDER BY id')->fetchAll(); + +/* Basit geri yukleme: snapshot degerlerini tekrar uygula */ +$restoreSnapshot = static function () use ($pdo, $snapshot): void { + $pdo->exec("DELETE FROM users WHERE email LIKE '" . RACE_SCRATCH_EMAIL . "'"); + $upd = $pdo->prepare('UPDATE users SET role = :r, status = :s, must_change_password = :m WHERE id = :id'); + foreach ($snapshot as $row) { + $upd->execute([':r' => $row['role'], ':s' => $row['status'], + ':m' => $row['must_change_password'], ':id' => $row['id']]); + } +}; + +echo "\n================= Last Admin Race Regression Test ================="; + +/* ------------------------------------------------------------------ */ +section('1) Tek baglanti: guard karar semasi'); +/* ------------------------------------------------------------------ */ + +$pdo->exec('UPDATE users SET status = 0'); +$ins = $pdo->prepare('INSERT INTO users (name, email, password, role, status) VALUES (:n, :e, :p, \'admin\', 1)'); +$ins->execute([':n' => 'Race A', ':e' => 'race-test.a@riskops.local', ':p' => password_hash('x', PASSWORD_DEFAULT)]); +$aId = (int)$pdo->lastInsertId(); +$ins->execute([':n' => 'Race B', ':e' => 'race-test.b@riskops.local', ':p' => password_hash('x', PASSWORD_DEFAULT)]); +$bId = (int)$pdo->lastInsertId(); + +$pdo->beginTransaction(); +check('iki aktif admin varken hedef admin izinli', + last_admin_atomic_guard($pdo, $aId, true) === true); +$pdo->rollBack(); + +$pdo->prepare('UPDATE users SET status = 0 WHERE id = :id')->execute([':id' => $bId]); +$pdo->beginTransaction(); +check('tek kalan aktif admin REDDEDILIR', + last_admin_atomic_guard($pdo, $aId, true) === false); +$pdo->rollBack(); + +$pdo->prepare("UPDATE users SET role = 'viewer' WHERE id = :id")->execute([':id' => $bId]); +$pdo->beginTransaction(); +check('admin olmayan hedeften etkilenmez', + last_admin_atomic_guard($pdo, $bId, false) === true); +$pdo->rollBack(); + +/* ------------------------------------------------------------------ */ +section('2) Zorlanmis kesisme: iki islem, iki baglanti'); +/* ------------------------------------------------------------------ */ + +$pdo->exec('UPDATE users SET status = 0'); +$pdo->exec("UPDATE users SET role = 'admin', status = 1 WHERE id IN ({$aId}, {$bId})"); + +$mk = static fn(string $n): string => $tmp . '/' . $n . '.mk'; +array_map('unlink', glob($tmp . '/*.mk') ?: []); + +$specs = [ + [$bId, 'a_guard_done', 'b_guard_done', 'result_a.json'], + [$aId, 'b_guard_done', 'a_guard_done', 'result_b.json'], +]; +$procs = []; +foreach ($specs as [$target, $waitM, $doneM, $resultF]) { + $cmd = 'php ' . escapeshellarg($workerFile) + . ' ' . escapeshellarg((string)$target) + . ' ' . escapeshellarg($mk($waitM)) + . ' ' . escapeshellarg($mk($doneM)) + . ' ' . escapeshellarg($tmp . '/' . $resultF) + . ' ' . escapeshellarg($repoRoot); + $descriptors = [1 => ['file', $tmp . '/' . $resultF . '.log', 'w'], + 2 => ['file', $tmp . '/' . $resultF . '.err', 'w']]; + $pipes = []; + $procs[] = [proc_open($cmd, $descriptors, $pipes), $resultF]; +} + +$results = []; +foreach ($procs as [$proc, $resultF]) { + proc_close($proc); + $path = $tmp . '/' . $resultF; + $results[$resultF] = is_file($path) + ? (json_decode((string)file_get_contents($path), true) ?: []) + : ['missing' => true]; +} + +$activeAdmins = (int)$pdo->query( + "SELECT COUNT(*) FROM users WHERE role = 'admin' AND status = 1" +)->fetchColumn(); + +/* --- Geri yukle ------------------------------------------------------ */ + +$restoreSnapshot(); +$leftover = (int)$pdo->query( + "SELECT COUNT(*) FROM users WHERE email LIKE '" . RACE_SCRATCH_EMAIL . "' OR id IN ({$aId}, {$bId})" +)->fetchColumn(); +check('scratch kayitlar temizlendi, users geri yuklendi', $leftover === 0, $leftover . ' kayit'); + +/* --- Kararlar -------------------------------------------------------- */ + +check('her iki isci tamamlandi', + ($results['result_a.json']['completed'] ?? false) && ($results['result_b.json']['completed'] ?? false), + json_encode($results['result_b.json'])); +check('tam olarak bir isci guard tarafindan reddedildi', + (($results['result_a.json']['guard_rejected'] ?? false) ? 1 : 0) + + (($results['result_b.json']['guard_rejected'] ?? false) ? 1 : 0) === 1); +check('yaris sonrasi tam olarak 1 aktif admin kaldi', $activeAdmins === 1, (string)$activeAdmins); + +/* --- Gecici isciyi sil ----------------------------------------------- */ + +@unlink($workerFile); +array_map('unlink', glob($tmp . '/*') ?: []); +@rmdir($tmp); + +/* ------------------------------------------------------------------ */ + +echo "\n" . str_repeat('-', 72) . "\n"; +printf("Sonuc: %d OK, %d FAIL\n", $PASS, $FAIL); +if ($FAIL > 0) { + echo "LAST ADMIN RACE TEST: FAIL\n"; + exit(1); +} +echo "LAST ADMIN RACE TEST: PASS\n"; +exit(0);