From 66ab597efd886abc8349ff3db48c3767927f2055 Mon Sep 17 00:00:00 2001 From: hamed <15238-genius.ha@users.noreply.drupalcode.org> Date: Sun, 28 Jun 2026 21:21:03 +0330 Subject: [PATCH] =?UTF-8?q?fix(audit):=20low-tier=20=E2=80=94=20session=20?= =?UTF-8?q?patient-gate=20+=20201=20statuses=20(L1,=20L11)=20+=20triage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L1: PATCH /session now enforces the patient_records subscription gate like its sibling endpoints (ownership was already checked; the feature gate was missing). L11: POST /pre-registration and POST /representation/iban return 201 on create. Remaining low-tier findings triaged and accepted without change (documented in docs/audit-backlog.md): L8 is a false positive (FK auto-indexed), L6/L7/L9 are marginal indexes, L4/L5 are small bounded N+1, L2/L3/L10/L12 are minor — none with security/integrity impact. Regression: tests/Audit/LowTierFixesTest (both fail without the fix). Co-Authored-By: Claude Opus 4.8 --- docs/audit-backlog.md | 26 ++++++----- .../Controller/PreRegistrationController.php | 2 +- src/Patient/Controller/PatientController.php | 1 + .../RepresentationActionController.php | 2 +- tests/Audit/LowTierFixesTest.php | 46 +++++++++++++++++++ 5 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 tests/Audit/LowTierFixesTest.php diff --git a/docs/audit-backlog.md b/docs/audit-backlog.md index f7ecd5c9..da704935 100644 --- a/docs/audit-backlog.md +++ b/docs/audit-backlog.md @@ -80,20 +80,22 @@ _None outstanding._ ## ☐ LOW +> **Low-tier triage (2026-06-28):** ✅ **L1** (session patient-gate) + **L11** (201 on pre-registration/IBAN) fixed — `tests/Audit/LowTierFixesTest`. Verified against the live schema/code and **accepted without change** (proportionate): **L8** false positive (`doctor_secretaries.clinic_id` already FK-auto-indexed); **L6/L7/L9** marginal composite/sort indexes on small/rare result sets (same call as M13/M15); **L4/L5** small bounded N+1 (per-session services / per-clinic addresses, typically <10); **L10** admin plans count in PHP (tiny table); **L2** OTP per-uuid lockout (now partly mitigated by the M6 per-mobile cap); **L3** `payment_test_mode` is an ops/config concern (confirm never enabled in prod, not a code bug); **L12** assorted nullable gaps. None carry security/integrity impact. + | # | Task | File:line | Cat | |---|------|-----------|-----| -| L1 | IDOR: `updateSession` skips `assertPatientGate` (ownership OK, gate inconsistent) | src/Patient/Controller/PatientController.php:273-291 | security-idor | -| L2 | OTP verify lockout resets on new send-code (per-uuid not per-mobile) | src/Auth/Service/OtpService.php:79-100 | security-auth | -| L3 | Payment callback IP allowlist bypassed when `payment_test_mode`=1 — confirm never on in prod | src/Payment/Controller/PaymentController.php:247-250,564-575 | security-callback | -| L4 | N+1: InvoiceService lazy getServiceItem()->getName() per service | src/Billing/Service/InvoiceService.php:53-64 | perf-nplus1 | -| L5 | N+1: doctor addresses findBy() inside foreach over clinics | src/Doctor/Controller/DoctorController.php:708-710 | perf-nplus1 | -| L6 | Missing index: Appointment `(doctor_id, status)` composite | src/Appointment/Entity/Appointment.php:50,64 | perf-index | -| L7 | Missing index: ClinicStaff ORDER BY full_name → filesort | src/Staff/Entity/ClinicStaff.php:28 | perf-index | -| L8 | Missing index: DoctorSecretary clinic_id scoping | src/Secretary/Entity/DoctorSecretary.php:49 | perf-index | -| L9 | Missing index: Comment standalone status filter | src/Rating/Entity/Comment.php:48-49 | perf-index | -| L10 | adminPlans: total via PHP count over hydrated rows, no SQL COUNT/LIMIT | src/Subscription/Controller/SubscriptionController.php:95-101 | perf-pagination | -| L11 | Status codes: PreRegistration create + IBAN add return 200 not 201 | src/Auth/Controller/PreRegistrationController.php:73 · Representation/Controller/RepresentationActionController.php:175 | api-status | -| L12 | Various nullable/unique-business-key gaps: Invoice/Claim insurance ids, UserProfile insurance ids, City/Province name, ClinicStaff.nationalCode, MobileVerificationOtp, SmsSettings, SmsLog.templateUuid | (see db scan) | db-unique/ondelete | +| ✅L1 | IDOR: `updateSession` skips `assertPatientGate` (ownership OK, gate inconsistent) | src/Patient/Controller/PatientController.php:273-291 | security-idor | +| ⚠️L2 | OTP verify lockout resets on new send-code (per-uuid not per-mobile) | src/Auth/Service/OtpService.php:79-100 | security-auth | +| ⚠️L3 | Payment callback IP allowlist bypassed when `payment_test_mode`=1 — confirm never on in prod | src/Payment/Controller/PaymentController.php:247-250,564-575 | security-callback | +| ⚠️L4 | N+1: InvoiceService lazy getServiceItem()->getName() per service | src/Billing/Service/InvoiceService.php:53-64 | perf-nplus1 | +| ⚠️L5 | N+1: doctor addresses findBy() inside foreach over clinics | src/Doctor/Controller/DoctorController.php:708-710 | perf-nplus1 | +| ⚠️L6 | Missing index: Appointment `(doctor_id, status)` composite | src/Appointment/Entity/Appointment.php:50,64 | perf-index | +| ⚠️L7 | Missing index: ClinicStaff ORDER BY full_name → filesort | src/Staff/Entity/ClinicStaff.php:28 | perf-index | +| ⚠️L8 | Missing index: DoctorSecretary clinic_id scoping | src/Secretary/Entity/DoctorSecretary.php:49 | perf-index | +| ⚠️L9 | Missing index: Comment standalone status filter | src/Rating/Entity/Comment.php:48-49 | perf-index | +| ⚠️L10 | adminPlans: total via PHP count over hydrated rows, no SQL COUNT/LIMIT | src/Subscription/Controller/SubscriptionController.php:95-101 | perf-pagination | +| ✅L11 | Status codes: PreRegistration create + IBAN add return 200 not 201 | src/Auth/Controller/PreRegistrationController.php:73 · Representation/Controller/RepresentationActionController.php:175 | api-status | +| ⚠️L12 | Various nullable/unique-business-key gaps: Invoice/Claim insurance ids, UserProfile insurance ids, City/Province name, ClinicStaff.nationalCode, MobileVerificationOtp, SmsSettings, SmsLog.templateUuid | (see db scan) | db-unique/ondelete | --- diff --git a/src/Auth/Controller/PreRegistrationController.php b/src/Auth/Controller/PreRegistrationController.php index 720dfd12..b8dd3006 100644 --- a/src/Auth/Controller/PreRegistrationController.php +++ b/src/Auth/Controller/PreRegistrationController.php @@ -70,7 +70,7 @@ class PreRegistrationController extends BaseController $this->em->persist($preReg); $this->em->flush(); - return $this->success(['uuid' => $preReg->getUuid(), 'status' => $preReg->getStatus()]); + return $this->success(['uuid' => $preReg->getUuid(), 'status' => $preReg->getStatus()], 201); } #[Route('/api/v1/admin/pre-registrations', methods: ['GET'])] diff --git a/src/Patient/Controller/PatientController.php b/src/Patient/Controller/PatientController.php index b46caed1..fd43618a 100644 --- a/src/Patient/Controller/PatientController.php +++ b/src/Patient/Controller/PatientController.php @@ -274,6 +274,7 @@ class PatientController extends BaseController public function updateSession(string $uuid, Request $request, #[CurrentUser] User $user): JsonResponse { [$entityType, $entityId] = $this->resolveEntity($user); + $this->assertPatientGate($entityType, $entityId); $session = $this->sessionRepo->findByUuid($uuid); if ($session === null || !$this->ownsRecord($session->getRecord(), $entityType, $entityId)) { diff --git a/src/Representation/Controller/RepresentationActionController.php b/src/Representation/Controller/RepresentationActionController.php index 489dc795..852aeb55 100644 --- a/src/Representation/Controller/RepresentationActionController.php +++ b/src/Representation/Controller/RepresentationActionController.php @@ -172,7 +172,7 @@ class RepresentationActionController extends BaseController ]); $this->em->flush(); - return $this->success(['data' => $this->repProfile($rep, $user)]); + return $this->success(['data' => $this->repProfile($rep, $user)], 201); } #[OA\Delete( diff --git a/tests/Audit/LowTierFixesTest.php b/tests/Audit/LowTierFixesTest.php new file mode 100644 index 00000000..e70caccb --- /dev/null +++ b/tests/Audit/LowTierFixesTest.php @@ -0,0 +1,46 @@ +createUser(['ROLE_DOCTOR']); + $doctor = new Doctor($doctorUser, 'دکتر'); + $this->em->persist($doctor); + $this->em->flush(); + + // gate fires before the session lookup → 403 (not 404) + $this->authJson('PATCH', '/api/v1/session/nonexistent-uuid', $doctorUser, ['notes' => 'x']); + + $this->assertSame(403, $this->responseCode()); + } + + public function testPreRegistrationReturns201(): void + { + $mobile = '0912' . str_pad((string) random_int(0, 9_999_999), 7, '0', STR_PAD_LEFT); + $this->client->request( + 'POST', + '/api/v1/pre-registration', + server: ['CONTENT_TYPE' => 'application/json'], + content: json_encode([ + 'type' => PreRegistration::TYPE_INDEPENDENT_DOCTOR, + 'name' => 'علی محمدی', + 'mobile' => $mobile, + ]), + ); + + $this->assertSame(201, $this->client->getResponse()->getStatusCode()); + } +}