From 6e8da3cc2cc6943e16eeaab79fc0bb55c86b4ec1 Mon Sep 17 00:00:00 2001 From: root Date: Mon, 8 Jun 2026 23:20:06 -0400 Subject: [PATCH] fix batch issue --- .../View/ReimbursementController.php | 258 +++++++++++------- security-reports/composer-audit.json | 48 ++++ 2 files changed, 210 insertions(+), 96 deletions(-) create mode 100644 security-reports/composer-audit.json diff --git a/app/Controllers/View/ReimbursementController.php b/app/Controllers/View/ReimbursementController.php index 37fbaa4..fa73c52 100644 --- a/app/Controllers/View/ReimbursementController.php +++ b/app/Controllers/View/ReimbursementController.php @@ -601,153 +601,219 @@ class ReimbursementController extends BaseController ]); } - public function updateBatchAssignment() - { - if (strtolower($this->request->getMethod()) !== 'post') { - return $this->response->setStatusCode(405)->setJSON([ - 'success' => false, - 'error' => 'Method not allowed', - ]); - } +public function updateBatchAssignment() +{ + if (strtolower($this->request->getMethod()) !== 'post') { + return $this->response->setStatusCode(405)->setJSON([ + 'success' => false, + 'error' => 'Method not allowed', + ]); + } - $expenseId = (int) $this->request->getPost('expense_id'); - $batchIdRaw = $this->request->getPost('batch_id'); - $batchId = (int) $batchIdRaw; - $fallbackBatchNumber = $this->request->getPost('batch_number'); - if ($batchId <= 0 && $fallbackBatchNumber !== null && trim((string) $fallbackBatchNumber) !== '') { - $batchId = (int) $fallbackBatchNumber; - } - $adminIdRaw = $this->request->getPost('admin_id'); - $adminId = ($adminIdRaw === '' || $adminIdRaw === null) ? null : (int) $adminIdRaw; - $reimbursementId = (int) $this->request->getPost('reimbursement_id') ?: null; + $expenseId = (int) $this->request->getPost('expense_id'); - if ($expenseId <= 0) { - return $this->response->setStatusCode(422)->setJSON([ - 'success' => false, - 'error' => 'Invalid expense id.', - ]); - } + $batchIdRaw = $this->request->getPost('batch_id'); + $batchId = (int) $batchIdRaw; - $now = date('Y-m-d H:i:s'); + $fallbackBatchNumber = $this->request->getPost('batch_number'); + if ($batchId <= 0 && $fallbackBatchNumber !== null && trim((string) $fallbackBatchNumber) !== '') { + $batchId = (int) $fallbackBatchNumber; + } + $adminIdRaw = $this->request->getPost('admin_id'); + $adminId = ($adminIdRaw === '' || $adminIdRaw === null) ? null : (int) $adminIdRaw; + + $reimbursementId = (int) $this->request->getPost('reimbursement_id') ?: null; + + if ($expenseId <= 0) { + return $this->response->setStatusCode(422)->setJSON([ + 'success' => false, + 'error' => 'Invalid expense id.', + ]); + } + + $now = date('Y-m-d H:i:s'); + + $newHashResponse = function (array $payload = []) { + $newHash = function_exists('csrf_hash') ? csrf_hash() : null; + + return $this->response + ->setHeader('X-CSRF-HASH', (string) $newHash) + ->setJSON(array_merge($payload, [ + 'csrf_hash' => $newHash, + ])); + }; + + $this->db->transBegin(); + + try { $activeItem = $this->batchItemModel ->where('expense_id', $expenseId) ->where('unassigned_at IS NULL', null, false) ->first(); + /** + * If no batch is provided, remove the item from active batch processing. + * This means "unassigned from batch", not "unassigned admin". + */ if ($batchId <= 0) { if ($activeItem) { - $this->batchItemModel->update((int) $activeItem['id'], ['unassigned_at' => $now]); + $this->batchItemModel->update((int) $activeItem['id'], [ + 'unassigned_at' => $now, + ]); + if (!$reimbursementId && !empty($activeItem['reimbursement_id'])) { $reimbursementId = (int) $activeItem['reimbursement_id']; } } - $newHash = function_exists('csrf_hash') ? csrf_hash() : null; - return $this->response - ->setHeader('X-CSRF-HASH', (string) $newHash) - ->setJSON([ - 'success' => true, - 'batch_id' => 0, - 'admin_id' => null, - 'reimbursement_id' => $reimbursementId, - 'csrf_hash' => $newHash, - ]); + if (!$this->db->transStatus()) { + throw new \RuntimeException('Transaction failed while unassigning batch item.'); + } + + $this->db->transCommit(); + + return $newHashResponse([ + 'success' => true, + 'batch_id' => 0, + 'admin_id' => null, + 'reimbursement_id' => $reimbursementId, + ]); } $batch = $this->batchModel->find($batchId); if (!$batch || strtolower((string) ($batch['status'] ?? 'open')) !== 'open') { + $this->db->transRollback(); + return $this->response->setStatusCode(404)->setJSON([ 'success' => false, 'error' => 'Batch not found or already closed.', ]); } - $currentAdmin = $activeItem ? (int) ($activeItem['admin_id'] ?? 0) : null; - $activeSameBatch = $activeItem && (int) ($activeItem['batch_id'] ?? 0) === $batchId; - if ($activeSameBatch && ($currentAdmin === ($adminId ?? 0))) { - $newHash = function_exists('csrf_hash') ? csrf_hash() : null; - return $this->response - ->setHeader('X-CSRF-HASH', (string) $newHash) - ->setJSON([ - 'success' => true, - 'batch_id' => $batchId, - 'admin_id' => $adminId, - 'reimbursement_id' => $activeItem['reimbursement_id'] ?? null, - 'csrf_hash' => $newHash, - ]); + if (!$reimbursementId) { + if ($activeItem && !empty($activeItem['reimbursement_id'])) { + $reimbursementId = (int) $activeItem['reimbursement_id']; + } else { + $reimbursementId = $this->lookupReimbursementId($expenseId); + } } - if ($activeSameBatch) { - if (!$reimbursementId) { - $reimbursementId = $activeItem['reimbursement_id'] ? (int) $activeItem['reimbursement_id'] : $this->lookupReimbursementId($expenseId); + $activeSameBatch = $activeItem && (int) ($activeItem['batch_id'] ?? 0) === $batchId; + $currentAdmin = $activeItem ? (int) ($activeItem['admin_id'] ?? 0) : null; + $requestedAdmin = $adminId ?? 0; + + /** + * If the item is already active in the same batch/admin slot, just return success. + */ + if ($activeSameBatch && $currentAdmin === $requestedAdmin) { + if (!$this->db->transStatus()) { + throw new \RuntimeException('Transaction failed while checking existing assignment.'); } - $updatePayload = [ + + $this->db->transCommit(); + + return $newHashResponse([ + 'success' => true, + 'batch_id' => $batchId, + 'admin_id' => $adminId, + 'reimbursement_id' => $activeItem['reimbursement_id'] ?? $reimbursementId, + ]); + } + + /** + * If same batch but different admin, update the existing active row. + * + * Important: + * admin_id = null means active in the batch but not assigned to an admin. + * unassigned_at != null means removed from active batch processing. + */ + if ($activeSameBatch) { + $this->batchItemModel->update((int) $activeItem['id'], [ 'admin_id' => $adminId, 'assigned_at' => $now, 'reimbursement_id' => $reimbursementId, - 'unassigned_at' => $adminId === null ? $now : null, - ]; - $this->batchItemModel->update((int) $activeItem['id'], $updatePayload); - $newHash = function_exists('csrf_hash') ? csrf_hash() : null; - return $this->response - ->setHeader('X-CSRF-HASH', (string) $newHash) - ->setJSON([ - 'success' => true, - 'batch_id' => $batchId, - 'admin_id' => $adminId, - 'reimbursement_id' => $reimbursementId, - 'csrf_hash' => $newHash, - ]); - } + 'unassigned_at' => null, + ]); - if ($activeItem) { - $this->batchItemModel->update((int) $activeItem['id'], ['unassigned_at' => $now]); - if (!$reimbursementId && !empty($activeItem['reimbursement_id'])) { - $reimbursementId = (int) $activeItem['reimbursement_id']; + if (!$this->db->transStatus()) { + throw new \RuntimeException('Transaction failed while updating existing assignment.'); } + + $this->db->transCommit(); + + return $newHashResponse([ + 'success' => true, + 'batch_id' => $batchId, + 'admin_id' => $adminId, + 'reimbursement_id' => $reimbursementId, + ]); } - if (!$reimbursementId) { - $reimbursementId = $this->lookupReimbursementId($expenseId); + /** + * If the item is active in another batch, soft-unassign that row first. + */ + if ($activeItem) { + $this->batchItemModel->update((int) $activeItem['id'], [ + 'unassigned_at' => $now, + ]); } - $insertData = [ + /** + * Critical fix: + * Check whether this expense was previously assigned to this batch. + * Because uq_batch_expense(batch_id, expense_id) blocks duplicate rows, + * we must reactivate/update the old row instead of inserting again. + */ + $existingBatchItem = $this->batchItemModel + ->where('batch_id', $batchId) + ->where('expense_id', $expenseId) + ->first(); + + $assignmentData = [ 'batch_id' => $batchId, 'expense_id' => $expenseId, 'reimbursement_id' => $reimbursementId, 'admin_id' => $adminId, 'assigned_at' => $now, + 'unassigned_at' => null, 'school_year' => $this->schoolYear, 'semester' => $this->semester, ]; - try { - $this->batchItemModel->insert($insertData); - } catch (\Throwable $e) { - log_message('error', 'Failed to assign expense #{expense} to batch #{batch}: {msg}', [ - 'expense' => $expenseId, - 'batch' => $batchId, - 'msg' => $e->getMessage(), - ]); - return $this->response->setStatusCode(500)->setJSON([ - 'success' => false, - 'error' => 'Unable to update batch assignment right now.', - ]); + if ($existingBatchItem) { + $this->batchItemModel->update((int) $existingBatchItem['id'], $assignmentData); + } else { + $this->batchItemModel->insert($assignmentData); } - $newHash = function_exists('csrf_hash') ? csrf_hash() : null; + if (!$this->db->transStatus()) { + throw new \RuntimeException('Transaction failed while saving assignment.'); + } - return $this->response - ->setHeader('X-CSRF-HASH', (string) $newHash) - ->setJSON([ - 'success' => true, - 'batch_id' => $batchId, - 'admin_id' => $adminId, - 'reimbursement_id' => $reimbursementId, - 'csrf_hash' => $newHash, - ]); + $this->db->transCommit(); + + return $newHashResponse([ + 'success' => true, + 'batch_id' => $batchId, + 'admin_id' => $adminId, + 'reimbursement_id' => $reimbursementId, + ]); + } catch (\Throwable $e) { + $this->db->transRollback(); + + log_message('error', 'Failed to update batch assignment for expense #{expense}, batch #{batch}: {msg}', [ + 'expense' => $expenseId, + 'batch' => $batchId, + 'msg' => $e->getMessage(), + ]); + + return $this->response->setStatusCode(500)->setJSON([ + 'success' => false, + 'error' => 'Unable to update batch assignment right now.', + ]); } +} public function lockBatch() { diff --git a/security-reports/composer-audit.json b/security-reports/composer-audit.json new file mode 100644 index 0000000..9512d86 --- /dev/null +++ b/security-reports/composer-audit.json @@ -0,0 +1,48 @@ +{ + "advisories": { + "phpoffice/math": [ + { + "advisoryId": "PKSA-jw72-bn8m-h7rc", + "packageName": "phpoffice/math", + "affectedVersions": "<=0.2.0", + "title": "PHPOffice Math allows XXE when processing an XML file in the MathML format ", + "cve": "CVE-2025-48882", + "link": "https://github.com/advisories/GHSA-42hm-pq2f-3r7m", + "reportedAt": "2025-05-29T17:27:39+00:00", + "sources": [ + { + "name": "GitHub", + "remoteId": "GHSA-42hm-pq2f-3r7m" + } + ], + "severity": "high" + } + ], + "phpunit/phpunit": [ + { + "advisoryId": "PKSA-z3gr-8qht-p93v", + "packageName": "phpunit/phpunit", + "affectedVersions": ">=0,<8.5.52|>=9.0.0,<9.6.33|>=10.0.0,<10.5.62|>=11.0.0,<11.5.50|>=12.0.0,<12.5.8", + "title": "Unsafe Deserialization in PHPT Code Coverage Handling", + "cve": "CVE-2026-24765", + "link": "https://github.com/sebastianbergmann/phpunit/security/advisories/GHSA-vvj3-c3rp-c85p", + "reportedAt": "2026-01-27T05:21:14+00:00", + "sources": [ + { + "name": "GitHub", + "remoteId": "GHSA-vvj3-c3rp-c85p" + }, + { + "name": "FriendsOfPHP/security-advisories", + "remoteId": "phpunit/phpunit/CVE-2026-24765.yaml" + } + ], + "severity": "high" + } + ] + }, + "abandoned": { + "paypal/rest-api-sdk-php": "paypal/paypal-server-sdk" + }, + "filter": [] +}