From 6d2fd564e27e0b0ddaa4423da2fdfe320f38a199 Mon Sep 17 00:00:00 2001 From: hamed <15238-genius.ha@users.noreply.drupalcode.org> Date: Tue, 28 Jul 2026 12:46:30 +0330 Subject: [PATCH] chore(tenant): audit the paths the filter cannot reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- docs/architecture/tenancy.md | 32 ++++ src/Billing/Repository/ClaimRepository.php | 6 + src/Shared/Command/TenantDumpCommand.php | 174 +++++++++++++++++++++ tests/Billing/ClaimsByPatientTest.php | 20 +++ tests/Shared/TenantDumpCommandTest.php | 103 ++++++++++++ 5 files changed, 335 insertions(+) create mode 100644 src/Shared/Command/TenantDumpCommand.php create mode 100644 tests/Shared/TenantDumpCommandTest.php diff --git a/docs/architecture/tenancy.md b/docs/architecture/tenancy.md index 809b20b4..b3c89688 100644 --- a/docs/architecture/tenancy.md +++ b/docs/architecture/tenancy.md @@ -108,6 +108,38 @@ clinic_uuid صریحِ درخواست > UserActiveContext ذخیره‌شده --- +## SQL خام — آدیت‌شده + +فیلتر روی `Connection::executeQuery/executeStatement` اعمال نمی‌شود. هر نقطه‌ای که SQL خام می‌زند بررسی و طبقه‌بندی شده: + +| فایل | دسته | چرا امن است | +|---|---|---| +| `Billing/Repository/ClaimRepository` | **tenant-دار** | هر سه کوئری `WHERE c.entity_type = :type AND c.entity_id = :id` دارند؛ `ClaimsByPatientTest::testAnotherTenantsClaimsNeverAppearInTheDashboard` تثبیتش می‌کند | +| `Admin/Controller/AdminApiController` | ادمین | `#[IsGranted('ROLE_ADMIN')]` سطح کلاس؛ عمداً cross-tenant | +| `Representation/Controller/RepresentationActionController` | نماینده | فقط `doctors`، اسکوپ `representation_id` | +| `Category/Service/CategoryImporter` | سراسری | فقط جدول‌های مرجع؛ نام جدول از ثابت `TABLES` می‌آید (نه ورودی کاربر) و `isValidBundle()` + `ROLE_ADMIN` گیتش می‌کنند | +| `Doctor/Command/Purge*Command` · `Shared/Command/SeedDemoDataCommand` | کنسول | dry-run پیش‌فرض، `--force` لازم، prod از سطح kernel مسدود | +| `Shared/Controller/HealthController` | سراسری | `SELECT 1` | +| `Shared/Logging/DbLogger` | سراسری | `app_log` در `GlobalTables` | + +`getReference()` در کل `src/` یک مورد است و روی `User` (سراسری) — بدون اثر tenant. + +--- + +## بکاپ per-tenant + +```bash +php bin/console app:tenant:dump --tenant=clinic:12 --output=/tmp/clinic12.sql +``` + +جدول‌ها از metadata خوانده می‌شوند (همان معیار `TenantFilter`)، پس جدولی که فردا جفت tenant بگیرد خودکار وارد خروجی می‌شود. + +⚠️ فرزندان aggregate ستون محیط ندارند و در خروجی **نمی‌آیند**. برای بکاپ کامل یک محیط، آن‌ها باید از ریشه دنبال شوند. + +ورودی `--tenant` با regex بسته اعتبارسنجی می‌شود چون مستقیم داخل `--where` و خط فرمان می‌رود؛ `TenantDumpCommandTest` هفت ورودی بدشکل (تزریق SQL و شل، نوع ناشناخته، id صفر/منفی) را می‌سنجد. + +--- + ## entity جدید می‌سازی؟ ۱. اگر به یک محیط تعلق دارد → `use TenantOwnedTrait;` و در نقطهٔ ساخت `assignTenant()` را صدا بزن diff --git a/src/Billing/Repository/ClaimRepository.php b/src/Billing/Repository/ClaimRepository.php index 7186f53b..a46799d0 100644 --- a/src/Billing/Repository/ClaimRepository.php +++ b/src/Billing/Repository/ClaimRepository.php @@ -185,6 +185,12 @@ class ClaimRepository extends ServiceEntityRepository return (int) $this->getEntityManager()->getConnection()->executeQuery($sql, $params)->fetchOne(); } + /** + * ⚠️ SQL خام است، پس TenantFilter آن را نمی‌بیند. شرط محیط دستی و همیشگی است: + * هر سه کوئریِ این فایل با `c.entity_type = :type AND c.entity_id = :id` بسته + * می‌شوند و فراخوان جفت را اجباراً پاس می‌دهد. اگر روزی این WHERE از این متد + * برداشته شود، داشبورد مطالبات cross-tenant می‌شود بی‌آنکه هیچ تستی بشکند. + */ private function patientAggregateSql(string $where): string { return <<addOption('tenant', null, InputOption::VALUE_REQUIRED, 'محیط به شکل doctor: یا clinic:') + ->addOption('output', null, InputOption::VALUE_REQUIRED, 'مسیر فایل خروجی'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + + $tenant = (string) $input->getOption('tenant'); + $target = (string) $input->getOption('output'); + + if ($target === '') { + $io->error('گزینهٔ --output الزامی است.'); + + return Command::FAILURE; + } + + $parsed = $this->parseTenant($tenant); + if ($parsed === null) { + $io->error('قالب --tenant باید doctor: یا clinic: باشد؛ id عدد صحیح مثبت.'); + + return Command::FAILURE; + } + + [$type, $id] = $parsed; + + if (!$this->tenantExists($type, $id)) { + $io->error(sprintf('محیطی با شناسهٔ %s:%d وجود ندارد.', $type, $id)); + + return Command::FAILURE; + } + + $tables = $this->tenantTables(); + $io->text(sprintf('%d جدول محیط‌دار برای %s:%d', count($tables), $type, $id)); + + $written = $this->dump($tables, $type, $id, $target); + if ($written === null) { + $io->error('mysqldump شکست خورد؛ خروجی ناقص نوشته نشد.'); + + return Command::FAILURE; + } + + $io->success(sprintf('نوشته شد: %s (%d جدول)', $target, count($tables))); + + return Command::SUCCESS; + } + + /** + * مقدارها مستقیم داخل شرط --where و نام جدول می‌روند، پس فقط از allowlist و + * تبدیل به int عبور می‌کنند — escape کافی نیست. + * + * @return array{0: string, 1: int}|null + */ + private function parseTenant(string $raw): ?array + { + if (!preg_match('/^(doctor|clinic):(\d+)$/', trim($raw), $m)) { + return null; + } + + $id = (int) $m[2]; + + return $id > 0 ? [$m[1], $id] : null; + } + + private function tenantExists(string $type, int $id): bool + { + $table = $type === EntityContext::TYPE_CLINIC ? 'clinics' : 'doctors'; + + return (bool) $this->conn->fetchOne("SELECT 1 FROM {$table} WHERE id = ?", [$id]); + } + + /** + * جدول‌هایی که جفت (entity_type, entity_id) دارند — همان معیاری که TenantFilter + * با آن تصمیم می‌گیرد، تا این دو هرگز از هم واگرا نشوند. + * + * @return string[] + */ + private function tenantTables(): array + { + $tables = []; + foreach ($this->em->getMetadataFactory()->getAllMetadata() as $meta) { + if ($meta->hasField('entityType') && $meta->hasField('entityId')) { + $tables[] = $meta->getTableName(); + } + } + sort($tables); + + return $tables; + } + + /** @param string[] $tables @return int|null تعداد جدول‌های نوشته‌شده، یا null در شکست */ + private function dump(array $tables, string $type, int $id, string $target): ?int + { + $params = $this->conn->getParams(); + $where = sprintf("entity_type='%s' AND entity_id=%d", $type, $id); + + $handle = @fopen($target, 'w'); + if ($handle === false) { + return null; + } + + try { + foreach ($tables as $table) { + $process = new Process([ + 'mysqldump', + '--host=' . ($params['host'] ?? 'db'), + '--user=' . ($params['user'] ?? 'db'), + '--password=' . ($params['password'] ?? 'db'), + '--no-create-info', + '--skip-add-locks', + '--complete-insert', + '--where=' . $where, + (string) ($params['dbname'] ?? 'db'), + $table, + ]); + $process->setTimeout(300); + $process->run(); + + if (!$process->isSuccessful()) { + return null; + } + + fwrite($handle, $process->getOutput()); + } + } finally { + fclose($handle); + } + + return count($tables); + } +} diff --git a/tests/Billing/ClaimsByPatientTest.php b/tests/Billing/ClaimsByPatientTest.php index 6c83d316..2f3ed127 100644 --- a/tests/Billing/ClaimsByPatientTest.php +++ b/tests/Billing/ClaimsByPatientTest.php @@ -183,4 +183,24 @@ class ClaimsByPatientTest extends ApiTestCase self::assertSame(422, $this->responseCode()); } + + /** + * این داشبورد با SQL خام ساخته می‌شود، پس TenantFilter آن را نمی‌بیند و تنها + * محافظش شرط دستیِ `c.entity_type/:entity_id` در ClaimRepository است. اگر آن + * WHERE روزی برداشته شود، هیچ چیز جز این تست خبر نمی‌دهد. + */ + public function testAnotherTenantsClaimsNeverAppearInTheDashboard(): void + { + $this->invoiceWithClaims(10_000_000, 7_000_000, 0); + + $strangerUser = $this->createUser(['ROLE_DOCTOR']); + $strangerDoctor = new Doctor($strangerUser, 'دکتر بیگانه'); + $this->em->persist($strangerDoctor); + $this->em->flush(); + + $res = $this->authJson('GET', '/api/v1/billing/claims/by-patient', $strangerUser); + + self::assertSame(200, $this->responseCode()); + self::assertSame([], $res['data'], 'مطالبات پزشک دیگر نباید دیده شود'); + } } diff --git a/tests/Shared/TenantDumpCommandTest.php b/tests/Shared/TenantDumpCommandTest.php new file mode 100644 index 00000000..49238fd2 --- /dev/null +++ b/tests/Shared/TenantDumpCommandTest.php @@ -0,0 +1,103 @@ +find('app:tenant:dump')); + } + + private function outputPath(): string + { + return sys_get_temp_dir() . '/tenant-dump-test-' . bin2hex(random_bytes(8)) . '.sql'; + } + + public function testDumpsOnlyTheRequestedClinicRows(): void + { + $clinic = new Clinic($this->createUser(['ROLE_CLINIC'])); + $clinic->setName('کلینیک بکاپ'); + $this->em->persist($clinic); + $this->em->flush(); + + $path = $this->outputPath(); + $tester = $this->tester(); + $exit = $tester->execute(['--tenant' => 'clinic:' . $clinic->getId(), '--output' => $path]); + + self::assertSame(0, $exit, $tester->getDisplay()); + self::assertFileExists($path); + + $sql = (string) file_get_contents($path); + self::assertStringNotContainsString("'doctor',", $sql, 'ردیف محیط دیگری نباید در خروجی باشد'); + + unlink($path); + } + + /** ⚠️ محیط بدون داده: فایل معتبر، بدون خطا. */ + public function testTenantWithNoRowsStillProducesAValidFile(): void + { + $doctor = new Doctor($this->createUser(['ROLE_DOCTOR']), 'دکتر بی‌داده'); + $this->em->persist($doctor); + $this->em->flush(); + + $path = $this->outputPath(); + $tester = $this->tester(); + + self::assertSame(0, $tester->execute(['--tenant' => 'doctor:' . $doctor->getId(), '--output' => $path])); + self::assertFileExists($path); + + unlink($path); + } + + public function testUnknownTenantFails(): void + { + $path = $this->outputPath(); + $tester = $this->tester(); + + self::assertSame(1, $tester->execute(['--tenant' => 'clinic:99999999', '--output' => $path])); + self::assertStringContainsString('وجود ندارد', $tester->getDisplay()); + } + + #[DataProvider('malformedTenants')] + public function testMalformedTenantIsRejectedBeforeTouchingTheDatabase(string $tenant): void + { + $tester = $this->tester(); + + self::assertSame(1, $tester->execute(['--tenant' => $tenant, '--output' => $this->outputPath()])); + self::assertStringContainsString('doctor:', $tester->getDisplay()); + } + + /** @return iterable */ + public static function malformedTenants(): iterable + { + yield 'sql injection' => ['clinic:1 OR 1=1']; + yield 'shell injection' => ['clinic:1; rm -rf /']; + yield 'unknown type' => ['admin:1']; + yield 'zero id' => ['clinic:0']; + yield 'negative id' => ['clinic:-1']; + yield 'missing id' => ['clinic']; + yield 'empty' => ['']; + } + + public function testOutputOptionIsRequired(): void + { + $tester = $this->tester(); + + self::assertSame(1, $tester->execute(['--tenant' => 'clinic:1'])); + self::assertStringContainsString('--output', $tester->getDisplay()); + } +}