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>
This commit is contained in:
hamed
@@ -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()` را صدا بزن
|
||||
|
||||
@@ -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 <<<SQL
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
<?php
|
||||
|
||||
namespace App\Shared\Command;
|
||||
|
||||
use App\Shared\Context\EntityContext;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Component\Console\Attribute\AsCommand;
|
||||
use Symfony\Component\Console\Command\Command;
|
||||
use Symfony\Component\Console\Input\InputInterface;
|
||||
use Symfony\Component\Console\Input\InputOption;
|
||||
use Symfony\Component\Console\Output\OutputInterface;
|
||||
use Symfony\Component\Console\Style\SymfonyStyle;
|
||||
use Symfony\Component\Process\Process;
|
||||
|
||||
/**
|
||||
* خروجی SQL از دادهٔ یک محیط — بکاپ یا تحویلِ per-tenant بدون اینکه لازم باشد هر
|
||||
* کلینیک دیتابیس جدا داشته باشد.
|
||||
*
|
||||
* جدولهای محیطدار از metadata خوانده میشوند، پس جدولی که فردا جفت tenant بگیرد
|
||||
* خودکار وارد خروجی میشود و این کامند از قلم نمیافتد.
|
||||
*
|
||||
* php bin/console app:tenant:dump --tenant=clinic:12 --output=/tmp/clinic12.sql
|
||||
*
|
||||
* ⚠️ فرزندان aggregate (نوتهای بیمار، آیتمهای صورتحساب، …) ستون محیط ندارند و در
|
||||
* خروجی نمیآیند — رجوع به docs/architecture/tenancy.md.
|
||||
*/
|
||||
#[AsCommand(
|
||||
name: 'app:tenant:dump',
|
||||
description: 'Dump one tenant\'s rows (doctor or clinic) to a SQL file',
|
||||
)]
|
||||
class TenantDumpCommand extends Command
|
||||
{
|
||||
public function __construct(
|
||||
private readonly Connection $conn,
|
||||
private readonly EntityManagerInterface $em,
|
||||
) {
|
||||
parent::__construct();
|
||||
}
|
||||
|
||||
protected function configure(): void
|
||||
{
|
||||
$this
|
||||
->addOption('tenant', null, InputOption::VALUE_REQUIRED, 'محیط به شکل doctor:<id> یا clinic:<id>')
|
||||
->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:<id> یا clinic:<id> باشد؛ 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);
|
||||
}
|
||||
}
|
||||
@@ -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'], 'مطالبات پزشک دیگر نباید دیده شود');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
<?php
|
||||
|
||||
namespace App\Tests\Shared;
|
||||
|
||||
use App\Clinic\Entity\Clinic;
|
||||
use App\Doctor\Entity\Doctor;
|
||||
use App\Tests\ApiTestCase;
|
||||
use PHPUnit\Framework\Attributes\DataProvider;
|
||||
use Symfony\Bundle\FrameworkBundle\Console\Application;
|
||||
use Symfony\Component\Console\Tester\CommandTester;
|
||||
|
||||
/**
|
||||
* `app:tenant:dump` مقدارهایش را مستقیم داخل شرط --where و خط فرمان میگذارد، پس
|
||||
* اعتبارسنجی ورودی اینجا امنیتی است نه سلیقهای.
|
||||
*/
|
||||
class TenantDumpCommandTest extends ApiTestCase
|
||||
{
|
||||
private function tester(): CommandTester
|
||||
{
|
||||
$application = new Application(static::$kernel);
|
||||
|
||||
return new CommandTester($application->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:<id>', $tester->getDisplay());
|
||||
}
|
||||
|
||||
/** @return iterable<string, array{string}> */
|
||||
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());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user