Commit Graph
16 Commits
Author SHA1 Message Date
hamedandClaude Opus 4.8 447482c2ea perf(db): composite index on wallet_transactions(user_id, type) (M14)
Helps the per-type SUM balance query. M13 (service_items.section_id) and M15
(clinic_doctor_invitations.doctor_id) were false positives — both columns carry
a FK and are therefore auto-indexed by InnoDB; verified against the live schema.

Structural regression: InfraSmokeTest::testWalletUserTypeIndexExists.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 20:28:24 +03:30
hamedandClaude Opus 4.8 670cef24f4 fix(security): per-mobile OTP cap + refresh-token rotation (M6, M7)
M6: send-code rate-limited only per IP, so a victim's number could be
SMS-flooded from rotating IPs. Add a per-mobile bucket (same 5/hour policy)
keyed by the validated mobile.

M7: /oauth/token/refresh reused the presented refresh token verbatim (no
rotation) and never re-checked the user. The rotation infra already existed
(issueTokens mints a fresh refresh token) — the controller just discarded it.
Now revoke the presented token (single-use), issue a fresh pair, and reject a
suspended user (status != 1).

Regressions: tests/Auth/SendCodeMobileRateLimitTest,
tests/Auth/RefreshTokenRotationTest (both fail without the fix).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 20:25:22 +03:30
hamedandClaude Opus 4.8 fe6383314e fix(security): enforce ownership on 4 IDOR read/bind endpoints (M2-M5)
- M2 GET /insurance/{id}: was unguarded; now owner-or-admin (403 otherwise) —
  stops reading another doctor's negotiated price by id enumeration.
- M3 GET /clinic-pro/doctor-address/{id}: add the same owner/admin check the
  sibling PATCH/DELETE already had.
- M4 POST/PATCH /service-item: staff_uuid must belong to the caller's tenant
  (entity_type/entity_id) → 422; stops binding another tenant's staff.
- M5 appointment-settings list endpoints (date-override/holidays/
  available-locations): add the per-doctor ownership check the sibling
  single-record endpoints already enforce.

Regressions (6 negative cases fail without the fixes):
DoctorInsuranceOwnershipTest, DoctorAddressOwnershipTest,
ServiceItemStaffOwnershipTest, AppointmentSettingsListOwnershipTest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 20:17:52 +03:30
hamedandClaude Opus 4.8 23ca56b293 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>
2026-06-28 20:07:19 +03:30
hamed c9ae3882fe docs(audit): mark Critical+High tier complete in backlog 2026-06-28 19:42:39 +03:30
hamedandClaude Opus 4.8 d8f7db0ada perf(billing,settlement): paginate claims and wallet transactions (H9, H10)
GET /billing/claims loaded every tenant claim with no limit. Add
findByTenant(page, limit) + countByTenant (shared query builder), default
limit 50 / max 100, and expose totals as data.meta — kept inside the existing
{ data: { data: [...] } } envelope so current clients are unaffected.

GET /wallet/transactions was already bounded (findByUser defaulted to limit 50)
but page-less; add page/offset + countByUser + the same additive meta.

Regression: tests/Billing/ClaimsListPaginationTest,
tests/Settlement/WalletTransactionsPaginationTest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 19:42:04 +03:30
hamedandClaude Opus 4.8 42d934abc2 perf(rating): fetch-join comment tree to kill N+1 in public list (H7)
findApprovedRootsByDoctor used a plain findBy, so Comment::toArray() lazy-loaded
likes, replies and the author per comment (and recursively per reply). Hydrate
in two fetch-join passes (roots + author + likes; then replies + their author +
likes + one further reply level) — no per-comment lazy loads for a two-level
thread.

Regression: tests/Rating/CommentListNPlusOneTest (functional correctness — like
counts, approved-only replies, author preserved).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 19:35:34 +03:30
hamedandClaude Opus 4.8 eb1997066e perf(insurance): batch-fetch service items in coverage list (H8)
listServiceCoverage called serviceItemRepo->find() once per coverage row (N+1).
Collect the ids and fetch them in one findBy(['id' => $ids]), then map by id.

Regression: tests/Insurance/ServiceCoverageNPlusOneTest (functional correctness —
every row resolves the right service_item_uuid).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 19:29:06 +03:30
hamedandClaude Opus 4.8 6b12f3ddb9 perf(doctor): fetch-join specialties in clinic doctor list (H6)
findByClinicWithFilters left-joined specialties only for filtering, so
toListArray() lazy-loaded them per doctor (N+1). addSelect them and switch the
result fetch to Paginator(fetchJoinCollection: true) so LIMIT still paginates by
doctor.

Test infra: ApiTestCase::countQueries() (via doctrine.debug_data_holder).
Regression: tests/Doctor/ClinicDoctorListNPlusOneTest asserts the query count
does not grow with doctor count (4→10 without the fix).

Also relaxed AppointmentExpiryServiceTest's exact-count assertion (it counts all
stale pendings in the shared db_test, which accumulates) — logged test-isolation
debt as E6.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 19:24:49 +03:30
hamedandClaude Opus 4.8 4cf6873900 fix(db): purge insurance config when a doctor/clinic is deleted (H5)
tenant_insurances, entity_insurance_pricing and tenant_service_coverages
reference their owner through a polymorphic (entity_type, entity_id) pair, so no
database FK can cascade their cleanup. Hard-deleting a doctor (DoctorController)
or clinic (AdminApiController) left these rows orphaned.

Add TenantInsuranceCleanupService::purgeForEntity() and call it from both delete
paths — removes coverage (via owning tenant_insurance ids), then tenant
insurances, then pricing.

Residual (separate, lower-freq paths): deleting an insurance category or a
service_item still orphans rows that reference them by id — tracked under the
medium-tier soft-ref findings.

Regression: tests/Insurance/TenantInsuranceCleanupTest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 19:16:10 +03:30
hamedandClaude Opus 4.8 d14ac38da5 fix(db): RESTRICT delete of a Payment that has a FinancialBreakdown (H4)
The ledger FK used ON DELETE CASCADE on a non-nullable column, so deleting a
Payment silently destroyed its immutable financial breakdown rows. Switch to
RESTRICT — a settled payment can no longer be deleted out from under its ledger.

Regression: tests/Settlement/FinancialBreakdownIntegrityTest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 19:10:53 +03:30
hamedandClaude Opus 4.8 7fa4b55d3f fix(security): unique payment reference_id, reject replayed callbacks (H3)
reference_id (the gateway's settled-transaction ref) was not unique, so the
same successful callback — or a RefNum replayed onto another order — could
credit twice. Add a unique index (NULL until success, so pending/failed rows
don't collide) and an application-level pre-check in the callback that fails the
payment if the reference already belongs to another order. The unique index is
the hard backstop behind the check.

Regression: PaymentCallbackAmountTest::testReplayedGatewayReferenceIsRejected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 19:08:26 +03:30
hamedandClaude Opus 4.8 aa87b4a9cb fix(db): prevent double-booking a slot via unique active_slot_key (H2)
A non-unique index on (doctor_id, slot_start) plus a count-then-insert check
left a TOCTOU race: two concurrent requests could both pass isSlotTaken and
both insert. wrapInTransaction alone doesn't stop the phantom under InnoDB
REPEATABLE-READ.

Add a nullable, unique active_slot_key on Appointment = "doctorId:slotStart"
while the booking occupies the slot (pending/confirmed — in lockstep with
isSlotTaken); NULL once expired/completed/no_show/cancelled (NULLs don't collide
in a MySQL unique index, so released slots rebook freely). bookAtomically now:
catches the unique violation -> SlotTakenException, and expires lapsed pendings
in-transaction so the ~1-min window before the expiry cron doesn't wrongly block
rebooking. All three booking paths (online / my / admin) routed through it.

Migration backfills one row per (doctor, slot) — the latest id — so the index
builds even on dirty historical data without destructively cancelling bookings.
(Backfill surfaced a real pre-existing double-booked slot in dev data.)

Regression: tests/Appointment/SlotUniquenessTest. Adjusted the expiry-service
test fixture to use distinct slots (one live booking per slot is now enforced).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 19:04:54 +03:30
hamedandClaude Opus 4.8 c084571bf0 fix(security): enforce doctor scope on POST my/appointment (H1)
createAppointment only checked the caller held an allowed role, then booked
onto whatever doctor_uuid the request named — a doctor could book onto any
other doctor's calendar, a clinic onto doctors outside it, a secretary outside
their scope. Add canBookForDoctor(): doctor→own only, clinic→member doctors,
secretary→active scope + appointments.create permission, admin→any.

Regression: tests/Appointment/BookingScopeTest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 18:52:23 +03:30
hamedandClaude Opus 4.8 ae06498a96 fix(security): verify gateway-confirmed amount in payment callback (C1)
The callback marked an order success on any verify-ok result without comparing
the gateway-settled amount to the amount charged. SEP returns AffectiveAmount;
an underpayment or a replayed RefNum from a cheaper order would confirm the
expensive order. Now reject (status=failed, no activation) when the gateway
reports an amount that mismatches the stored amount_rials. Gateways that don't
report a settled amount (Mellat binds it server-side) skip the check.

MockGateway now echoes mock_amount so the guard is exercisable in tests.
Regression: tests/Payment/PaymentCallbackAmountTest (underpayment rejected,
matching amount succeeds).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 18:49:33 +03:30
hamedandClaude Opus 4.8 8e5cd51873 docs(audit): durable backlog — 5-dimension scan, 70 findings prioritized
Persists the audit plan so a session restart no longer loses it (TodoWrite
is volatile). 10 done, 1 critical, 10 high, 21 medium, 12 low, 4 epics.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 18:44:25 +03:30