d2f4b5c4284cd8b92b56f60e95d65d71b03e7a63
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 hamedandClaude Opus 5
|
d2f4b5c428 |
fix(tenant): check the environment wherever a uuid comes from the request
Phase 7 was scoped to guard aggregate children, which the Doctrine filter cannot reach. Measuring first — as the plan required — moved the target: all 22 children and their 20 repositories were already sound. Every list query anchors on its root, and ServiceItemRepository even joins service_sections and filters on the pair by hand. A repository-level guard would have found nothing. The real exposure was one layer up. Where a uuid arrives from a request body or query string, the entity it names is loaded by uuid alone, and the filter is no help: aggregate children have no tenant column, and a panel user who never chose an environment is not filtered at all. Three leaks, each proven by removing the fix and watching the new tests go red: - GET /api/v1/appointment-service-slots accepted service_item_uuids from any environment. Existence, bookable state and duration leaked through the error messages and the returned slots. The booking path in the same controller had guarded this since it was written; the slot path never did. - POST /api/v1/my/appointment attached service_section_uuid, service_item_uuid, staff_uuid and the service list without any check, and persisted them onto the appointment. A write, not just a read. - PatientService did the same in all three of its loops — pricing, session create, session update — so another environment's service price entered the invoice and its SessionService row was stored, staff included. TenantOwnershipChecker is the single place that answers "does this belong to the current environment?". It reads getEntityType()/getEntityId(), so ServiceItem now delegates that pair to its section: an aggregate child exposing the tenant it inherits. An entity that exposes no pair throws rather than returning false — silence here builds an always-closed guard, which is its own bug. TenantLookupInventoryTest keeps a per-file count of these lookups. It earned its place immediately: the first run found more sites than the manual grep had, and reviewing them turned up the third PatientService loop. StaffController looked unguarded until read properly — ownsStaff sits two lines below the null check. One assertion was wrong before it was right: the create-path test read `$session['services'] ?? []`, which passes vacuously. It now counts the stored rows through the repository, and fails without the fix. Tests: 879 passing. PHPStan unchanged at its 17 pre-existing errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
6d2fd564e2 |
chore(tenant): audit the paths the filter cannot reach
Phase 5, the last of the tenant-marking series. The Doctrine filter added in phase 4 does not see raw DBAL, so every place that writes SQL by hand was read and classified rather than assumed safe. The audit found no code to fix. ClaimRepository was the only tenant-owning table reached by raw SQL, and all three of its queries already close on c.entity_type/:entity_id. That protection had no test, so it now has one: the claims dashboard is the only tenant surface whose isolation depends entirely on a hand-written WHERE, and nothing would have reported its removal. Everything else falls outside the question. AdminApiController is cross-tenant on purpose behind a class-level ROLE_ADMIN. RepresentationActionController only counts doctors, scoped by representation_id. CategoryImporter interpolates a table name, but it comes from a hardcoded const map behind isValidBundle() and ROLE_ADMIN, so it cannot be steered by input. The purge and seed commands are console-only, dry-run by default, and blocked from prod at the kernel. The health check is SELECT 1 and the logger writes to a global table. getReference() appears once in src, on User, which is global. app:tenant:dump gives one environment's rows as SQL — the practical benefit of database-per-tenant without its cost. It reads the table list from metadata using the same test the filter applies, so a table that gains a tenant pair later is included automatically instead of being silently missed. The --tenant value ends up inside a --where clause and an argv entry, so it is validated by a closed regex rather than escaped; seven malformed inputs are covered, including SQL and shell injection attempts. Verified by running it against the dev database: a real clinic produced 20 tables with only that clinic's rows and no doctor-owned row, an unknown id exited non-zero with a Persian message, "clinic:1 OR 1=1" was refused, and a tenant with no data still produced a valid file. Not verified: browser-level checks of the admin panel and the public site. The OTP login is behind an Altcha proof-of-work, so no interactive token was obtained. What was checked instead: the admin SPA type-checks clean, the public doctor and specialty endpoints answer 200 with cross-tenant results, and neither nobat724_front nor clinic-pro-tauri references owner_type, owner_id, clinic_key or db_type anywhere. The functional suite already exercises the same HTTP path with real JWTs and the subscriber active. Tests: 856 passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
75d5052f72 |
feat(tenant): enforce environment isolation in the ORM layer
Phase 4 of the tenant-marking series. Until now isolation depended on every query remembering its own WHERE clause. With 82 entities and 844 tests, that is not a guarantee — it is a hope. MariaDB has no row-level security, so the backstop has to live in Doctrine. TenantFilter appends (entity_type, entity_id) to every DQL query on a tenant-owning entity. It ships disabled and TenantFilterSubscriber turns it on per request. The filter engages only for a **chosen** environment — an explicit clinic_uuid on the request, or a stored UserActiveContext. EntityContext now records which of the two produced it. Locking a user to the role fallback instead would hide data they are entitled to: a clinic-member doctor who never switched context lost every appointment belonging to that clinic. Five tests caught exactly that before the gate was added. Admins and unauthenticated marketplace traffic stay outside the filter by design. Two findings from running it rather than reasoning about it: - Dereferencing a lazy proxy whose target the filter excluded raises EntityNotFoundException, which surfaced as 500 on four patient endpoints. ExceptionSubscriber now maps it to 404: outside your environment means it does not exist for you. It is logged at info level so a genuinely broken FK is still visible. - EntityManager::find() by primary key IS filtered in Doctrine ORM 3, contrary to the limitation carried over from older versions. The stronger guarantee is pinned by a test so a future regression is noticed, and the documented table was corrected. The filter also caught a real leak: a clinic secretary's appointment list filtered by doctor id alone, so a doctor's personal-practice booking appeared in the clinic list. The test had been asserting that behaviour. GlobalTables classifies all 82 entities into four states — carries a tenant, deliberately global, aggregate child, or recorded debt — and TenantSchemaCoverageTest fails on anything unclassified. Aggregate children declare their root explicitly, because several attach through a scalar FK rather than a Doctrine association and cannot be inferred from metadata; the test walks each chain to a tenant-owning root. Financial tables stay in DEFERRED with a ceiling assertion so the list cannot grow quietly. Deliberately not built: the prePersist assignment listener from the plan. The tenant columns are NOT NULL without a default, so a missing assignTenant() already fails loudly at flush — phase 2 surfaced 123 such failures. A listener would add silent auto-assignment where the current behaviour is an explicit crash. EXPLAIN with the filter's conditions still picks idx_appointments_tenant_slot and uniq_patient_record. Tests: 844 passing. PHPStan unchanged at its 17 pre-existing errors, none in files touched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 4.8
|
89191eee57 |
feat: insurance & medical billing system (6 phases)
Multi-tenant insurance contracts, service coverage, versioned tariffs, invoice calculation, and insurance claims with debt reporting. - TenantInsurance: per-tenant insurance contracts (coverage/franchise/ceiling, versioning, soft-deactivate) + active guard - ServiceItem.insuranceCovered + TenantServiceCoverage per-service overrides - Tariff: versioned yearly tariffs with fallback to ServiceItem price - Billing domain: Money/ShareBreakdown VOs, BillingCalculator (unit-tested), Invoice/InvoiceItem aggregate, InvoiceService.createFromSession - Claim/ClaimItem with state machine (pending->submitted->approved/rejected->paid), ClaimService, insurance-debt report - ClaimSubmitterInterface + ManualClaimSubmitter (future insurance API ready) - Admin UI: insurance-pricing page, claims page, service tariff modal, service insurance toggle; routes + sidebar entries - Architecture doc + billing/insurance/clinic-services API docs Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |