fix(billing): bound approved/paid amounts on claim transition (M1)
approve/pay accepted any approved_rials/paid_rials with no bounds, so the claiming tenant could write arbitrary figures into the insurer-debt ledger (negative, or far above the claimed total). Validate: approved ∈ [0, claimed], paid ∈ [0, approved] → 422 otherwise. (The "force arbitrary status" half of the finding was already prevented by Claim::canTransitionTo.) Regression: tests/Billing/ClaimAmountBoundsTest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+3
-3
@@ -163,11 +163,11 @@
|
||||
| action | body اختیاری | اثر |
|
||||
|--------|--------------|-----|
|
||||
| submit | — | pending → submitted |
|
||||
| approve | `approved_rials` | submitted → approved (پیشفرض = کل ادعا) |
|
||||
| approve | `approved_rials` | submitted → approved (پیشفرض = کل ادعا) — باید `0 ≤ approved_rials ≤ total_claimed_rials` |
|
||||
| reject | `reason` (الزامی) | submitted → rejected |
|
||||
| pay | `paid_rials` | approved → paid (پیشفرض = approved) |
|
||||
| pay | `paid_rials` | approved → paid (پیشفرض = approved) — باید `0 ≤ paid_rials ≤ total_approved_rials` |
|
||||
|
||||
**Errors:** `422` انتقال نامعتبر یا دلیل رد خالی · `404` مطالبه یافت نشد.
|
||||
**Errors:** `422` انتقال نامعتبر، دلیل رد خالی، یا مبلغ `approved_rials`/`paid_rials` خارج از بازه (`field` در پاسخ) · `404` مطالبه یافت نشد.
|
||||
|
||||
## GET /api/v1/billing/reports/insurance-debt
|
||||
گزارش بدهی بیمهها برای tenant (group بر اساس بیمه).
|
||||
|
||||
@@ -54,7 +54,7 @@ _None outstanding._
|
||||
|
||||
| # | Task | File:line | Cat | How to test |
|
||||
|---|------|-----------|-----|-------------|
|
||||
| M1 | Mass-assign: claim approve/pay lets owning tenant set arbitrary `approved_rials`/`paid_rials` + force status PAID/APPROVED | src/Billing/Controller/BillingController.php:192 | security-massassign | POST claim approve w/ huge amounts → server computes, ignores caller figures |
|
||||
| ✅M1 | Mass-assign: claim approve/pay lets owning tenant set arbitrary `approved_rials`/`paid_rials` ~~+ force status~~ | src/Billing/Controller/BillingController.php | security-massassign | **DONE** — status-jump part was already guarded by `canTransitionTo` (false alarm). Added bounds: approved ∈ [0, claimed], paid ∈ [0, approved] → 422. `tests/Billing/ClaimAmountBoundsTest`. |
|
||||
| M2 | IDOR read: `GET /insurance/{id}` (showDoctorInsurance) leaks any doctor's insurance incl. negotiated price by id enumeration | src/Insurance/Controller/InsuranceController.php:499 | security-idor | GET insurance/{id} as non-owner → 403 |
|
||||
| M3 | IDOR read: `showAddress` loads any DoctorAddress by id, no owner check | src/Doctor/Controller/DoctorController.php:559 | security-idor | GET doctor-address/{id} as non-owner → 403 |
|
||||
| M4 | IDOR write: ClinicService `createItem`/`updateItem` bind staff via global `staffRepo->findByUuid()`, no entity-scope check (cross-tenant staff binding) | src/ClinicService/Controller/ClinicServiceController.php:157,195 | security-idor | Bind other clinic's staff_uuid → 403/validation |
|
||||
|
||||
@@ -224,6 +224,22 @@ class BillingController extends BaseController
|
||||
return $this->error(ErrorCodes::ERR_VALIDATION_001, 'دلیل رد الزامی است', 422);
|
||||
}
|
||||
|
||||
// Bound the financial figures: approved/paid cannot be negative, approved
|
||||
// cannot exceed the claimed total, and paid cannot exceed approved.
|
||||
if ($action === 'approve' && isset($data['approved_rials'])) {
|
||||
$approved = (int) $data['approved_rials'];
|
||||
if ($approved < 0 || $approved > $claim->getTotalClaimedRials()) {
|
||||
return $this->error(ErrorCodes::ERR_VALIDATION_001, 'مبلغ تأییدشده باید بین ۰ و مبلغ مطالبهشده باشد', 422, 'approved_rials');
|
||||
}
|
||||
}
|
||||
if ($action === 'pay' && isset($data['paid_rials'])) {
|
||||
$paid = (int) $data['paid_rials'];
|
||||
$ceiling = $claim->getTotalApprovedRials() ?? $claim->getTotalClaimedRials();
|
||||
if ($paid < 0 || $paid > $ceiling) {
|
||||
return $this->error(ErrorCodes::ERR_VALIDATION_001, 'مبلغ پرداختی باید بین ۰ و مبلغ تأییدشده باشد', 422, 'paid_rials');
|
||||
}
|
||||
}
|
||||
|
||||
$this->claimService->transition($claim, $target, [
|
||||
'approved_rials' => isset($data['approved_rials']) ? (int) $data['approved_rials'] : null,
|
||||
'paid_rials' => isset($data['paid_rials']) ? (int) $data['paid_rials'] : null,
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
<?php
|
||||
|
||||
namespace App\Tests\Billing;
|
||||
|
||||
use App\Auth\Entity\User;
|
||||
use App\Billing\Entity\Claim;
|
||||
use App\Billing\Entity\ClaimItem;
|
||||
use App\Doctor\Entity\Doctor;
|
||||
use App\Tests\ApiTestCase;
|
||||
|
||||
/**
|
||||
* The claim approve/pay transition must bound the financial figures: approved
|
||||
* cannot exceed the claimed total (nor be negative). Guards the insurer-debt
|
||||
* ledger from arbitrary amounts entered by the claiming tenant.
|
||||
*/
|
||||
class ClaimAmountBoundsTest extends ApiTestCase
|
||||
{
|
||||
private Doctor $doctor;
|
||||
private User $owner;
|
||||
|
||||
private function submittedClaim(int $claimedRials): Claim
|
||||
{
|
||||
$claim = new Claim('doctor', $this->doctor->getId(), 1, 'base');
|
||||
$item = new ClaimItem($claim, 1, $claimedRials);
|
||||
$claim->addItem($item);
|
||||
$claim->submit();
|
||||
$this->em->persist($claim);
|
||||
$this->em->persist($item);
|
||||
$this->em->flush();
|
||||
|
||||
return $claim;
|
||||
}
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
parent::setUp();
|
||||
$this->owner = $this->createUser(['ROLE_DOCTOR']);
|
||||
$this->doctor = new Doctor($this->owner, 'دکتر تست');
|
||||
$this->em->persist($this->doctor);
|
||||
$this->em->flush();
|
||||
}
|
||||
|
||||
public function testApproveAboveClaimedIsRejected(): void
|
||||
{
|
||||
$claim = $this->submittedClaim(100_000);
|
||||
|
||||
$this->authJson('POST', '/api/v1/billing/claims/' . $claim->getUuid() . '/approve', $this->owner, [
|
||||
'approved_rials' => 999_999,
|
||||
]);
|
||||
|
||||
$this->assertSame(422, $this->responseCode());
|
||||
}
|
||||
|
||||
public function testApproveWithinBoundsSucceeds(): void
|
||||
{
|
||||
$claim = $this->submittedClaim(100_000);
|
||||
|
||||
$this->authJson('POST', '/api/v1/billing/claims/' . $claim->getUuid() . '/approve', $this->owner, [
|
||||
'approved_rials' => 60_000,
|
||||
]);
|
||||
|
||||
$this->assertSame(200, $this->responseCode());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user