diff --git a/docs/audit-backlog.md b/docs/audit-backlog.md index bb7670db..739505ff 100644 --- a/docs/audit-backlog.md +++ b/docs/audit-backlog.md @@ -40,7 +40,7 @@ _None outstanding._ | ✅H1 | IDOR write: `createAppointment` trusts request `doctor_uuid`, no scope check — any staff books onto any doctor's calendar | src/Appointment/Controller/MyAppointmentsController.php:59 | security-idor | **DONE** — `canBookForDoctor()` scope gate + `tests/Appointment/BookingScopeTest` | | ✅H2 | No UNIQUE `(doctor_id, slot_start)` on Appointment → double-booking race (index is non-unique) | src/Appointment/Entity/Appointment.php | db-unique | **DONE** — nullable unique `active_slot_key` (occupying = pending/confirmed, mirrors `isSlotTaken`); `bookAtomically` catches the unique violation + expires lapsed pendings in-txn; all 3 booking paths (online/my/admin) routed through it. Migration backfills one row per slot (non-destructive). `tests/Appointment/SlotUniquenessTest`. **NB:** backfill surfaced a real pre-existing double-booked slot in dev data (doctor 1764, two `expired` rows) — harmless (both expired = key NULL). | | ✅H3 | `Payment.referenceId` not unique → same gateway callback credited twice | src/Payment/Entity/Payment.php:61-62 | db-unique | **DONE** — unique index on `reference_id` (NULL until success → no collision) + callback pre-check rejects replays. `tests/Payment/PaymentCallbackAmountTest::testReplayedGatewayReferenceIsRejected` | -| H4 | `FinancialBreakdown.payment` onDelete CASCADE on non-nullable FK → deleting a Payment destroys ledger rows; should be RESTRICT | src/Settlement/Entity/FinancialBreakdown.php:28-30 | db-ondelete | Delete a Payment w/ breakdown → expect FK restrict error, ledger preserved | +| ✅H4 | `FinancialBreakdown.payment` onDelete CASCADE on non-nullable FK → deleting a Payment destroys ledger rows; should be RESTRICT | src/Settlement/Entity/FinancialBreakdown.php:28-30 | db-ondelete | **DONE** — onDelete RESTRICT + migration. `tests/Settlement/FinancialBreakdownIntegrityTest` | | H5 | Insurance pricing/coverage modeled as raw int FKs (no FK/onDelete) → orphan rows on delete: `EntityInsurancePricing.entity_id/insurance_id`, `TenantInsurance.entity_id/insurance_id`, `TenantServiceCoverage.tenant_insurance_id` | src/Insurance/Entity/EntityInsurancePricing.php:25-29 · TenantInsurance.php:29,32 · TenantServiceCoverage.php:23-24 | db-ondelete | Delete insurance/tenant → children removed or restricted, no dangling rows | | H6 | N+1: clinic doctors list lazy-loads `specialties` per doctor | src/Clinic/Controller/ClinicController.php:326 · src/Doctor/Repository/DoctorRepository.php:49 | perf-nplus1 | SQL profiler on clinic doctors list → 1 query/doctor for specialties | | H7 | N+1: comments list lazy-loads `likes`→`user`, `replies` (recursive), author realName | src/Rating/Controller/RatingController.php:278,352 | perf-nplus1 | Profiler GET comments → query count scales w/ comments | diff --git a/migrations/Version20260628153855.php b/migrations/Version20260628153855.php new file mode 100644 index 00000000..65b99a45 --- /dev/null +++ b/migrations/Version20260628153855.php @@ -0,0 +1,33 @@ +addSql('ALTER TABLE financial_breakdowns DROP FOREIGN KEY `FK_D66D9FF44C3A3BB`'); + $this->addSql('ALTER TABLE financial_breakdowns ADD CONSTRAINT FK_D66D9FF44C3A3BB FOREIGN KEY (payment_id) REFERENCES payments (id) ON DELETE RESTRICT'); + } + + public function down(Schema $schema): void + { + // this down() migration is auto-generated, please modify it to your needs + $this->addSql('ALTER TABLE financial_breakdowns DROP FOREIGN KEY FK_D66D9FF44C3A3BB'); + $this->addSql('ALTER TABLE financial_breakdowns ADD CONSTRAINT `FK_D66D9FF44C3A3BB` FOREIGN KEY (payment_id) REFERENCES payments (id) ON DELETE CASCADE'); + } +} diff --git a/src/Settlement/Entity/FinancialBreakdown.php b/src/Settlement/Entity/FinancialBreakdown.php index 4f997560..1f7f7356 100644 --- a/src/Settlement/Entity/FinancialBreakdown.php +++ b/src/Settlement/Entity/FinancialBreakdown.php @@ -26,7 +26,7 @@ class FinancialBreakdown private string $uuid; #[ORM\ManyToOne(targetEntity: Payment::class)] - #[ORM\JoinColumn(name: 'payment_id', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')] + #[ORM\JoinColumn(name: 'payment_id', referencedColumnName: 'id', nullable: false, onDelete: 'RESTRICT')] private Payment $payment; #[ORM\Column(type: 'string', length: 20)] diff --git a/tests/Settlement/FinancialBreakdownIntegrityTest.php b/tests/Settlement/FinancialBreakdownIntegrityTest.php new file mode 100644 index 00000000..aecbaa38 --- /dev/null +++ b/tests/Settlement/FinancialBreakdownIntegrityTest.php @@ -0,0 +1,37 @@ +createUser(); + $payment = new Payment($user, 100_000, 'mellat', Payment::TYPE_APPOINTMENT); + $this->em->persist($payment); + + $breakdown = new FinancialBreakdown( + $payment, + FinancialBreakdown::SOURCE_APPOINTMENT, + $user, + 100_000, 0, '0.00', 0, 100_000, '10.00', 10_000, 90_000, + null, null, null, + ); + $this->em->persist($breakdown); + $this->em->flush(); + + $this->em->remove($payment); + $this->expectException(ForeignKeyConstraintViolationException::class); + $this->em->flush(); + } +}