From 17d779e130813f3988316deb317979ff97cc5f27 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 9 May 2023 16:07:06 +0200 Subject: [PATCH] Scope queries using Postgres RLS (WIP) --- .../CreateRLSPoliciesForTenantTables.php | 12 +++- src/Database/Concerns/BelongsToTenant.php | 6 +- src/Jobs/CreatePostgresUserForTenant.php | 15 +++++ src/TenancyServiceProvider.php | 6 +- tests/PostgresTest.php | 59 +++++++++++-------- tests/TestCase.php | 6 +- 6 files changed, 71 insertions(+), 33 deletions(-) diff --git a/src/Commands/CreateRLSPoliciesForTenantTables.php b/src/Commands/CreateRLSPoliciesForTenantTables.php index 46dccadf..04a1ec92 100644 --- a/src/Commands/CreateRLSPoliciesForTenantTables.php +++ b/src/Commands/CreateRLSPoliciesForTenantTables.php @@ -35,7 +35,7 @@ class CreateRLSPoliciesForTenantTables extends Command {$parentKey} IN ( SELECT id FROM {$parentTable} - WHERE ({$tenantKey}::TEXT = ( + WHERE ({$tenantKey}::UUID = ( SELECT {$tenantKey} FROM {$parentTable} WHERE id = {$parentKey} @@ -43,7 +43,7 @@ class CreateRLSPoliciesForTenantTables extends Command ) )"); - DB::statement("ALTER TABLE {$table} FORCE ROW LEVEL SECURITY"); + $this->makeTableUseRls($table); } else { $modelName = $model::class; $this->components->info("Table '$table' is not related to tenant. Make sure $modelName uses the BelongsToPrimaryModel trait."); @@ -51,7 +51,7 @@ class CreateRLSPoliciesForTenantTables extends Command } else { DB::statement("CREATE POLICY {$table}_rls_policy ON {$table} USING ({$tenantKey}::TEXT = current_user);"); - DB::statement("ALTER TABLE {$table} FORCE ROW LEVEL SECURITY"); + $this->makeTableUseRls($table); $this->components->info("Created RLS policy for table '$table'"); } @@ -64,4 +64,10 @@ class CreateRLSPoliciesForTenantTables extends Command { return array_map(fn (string $modelName) => (new $modelName), config('tenancy.models.rls')); } + + protected function makeTableUseRls(string $table): void + { + DB::statement("ALTER TABLE {$table} ENABLE ROW LEVEL SECURITY"); + DB::statement("ALTER TABLE {$table} FORCE ROW LEVEL SECURITY"); + } } diff --git a/src/Database/Concerns/BelongsToTenant.php b/src/Database/Concerns/BelongsToTenant.php index 3ca9703c..6c0def7e 100644 --- a/src/Database/Concerns/BelongsToTenant.php +++ b/src/Database/Concerns/BelongsToTenant.php @@ -21,7 +21,11 @@ trait BelongsToTenant public static function bootBelongsToTenant(): void { - static::addGlobalScope(new TenantScope); + // The queries performed for models present in the tenancy.models.rls are scoped using Postgres RLS instead of the global scope + if (! in_array(static::class, config('tenancy.models.rls'))) { + static::addGlobalScope(new TenantScope); + } + static::creating(function ($model) { if (! $model->getAttribute(Tenancy::tenantKeyColumn()) && ! $model->relationLoaded('tenant')) { diff --git a/src/Jobs/CreatePostgresUserForTenant.php b/src/Jobs/CreatePostgresUserForTenant.php index 13402477..979b3704 100644 --- a/src/Jobs/CreatePostgresUserForTenant.php +++ b/src/Jobs/CreatePostgresUserForTenant.php @@ -43,5 +43,20 @@ class CreatePostgresUserForTenant implements ShouldQueue if (! count(DB::select("SELECT usename FROM pg_user WHERE usename = '$name';")) > 0) { DB::statement("CREATE USER \"$name\" LOGIN PASSWORD '$password';"); } + + $this->grantPermissions($name); + } + + protected function grantPermissions(string $userName): void + { + /** + * @var \Stancl\Tenancy\Database\Contracts\StatefulTenantDatabaseManager $databaseManager + */ + $databaseManager = $this->tenant->database()->manager(); + foreach (array_map(fn (string $modelName) => (new $modelName), config('tenancy.models.rls')) as $model) { + $table = $model->getTable(); + + $databaseManager->database()->statement("GRANT ALL ON {$table} TO \"{$userName}\""); + } } } diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index 24f8fa1b..bc11a474 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -5,12 +5,12 @@ declare(strict_types=1); namespace Stancl\Tenancy; use Illuminate\Cache\CacheManager; -use Illuminate\Database\Console\Migrations\FreshCommand; -use Illuminate\Support\ServiceProvider; -use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use Stancl\Tenancy\Contracts\Domain; use Stancl\Tenancy\Contracts\Tenant; +use Illuminate\Support\ServiceProvider; use Stancl\Tenancy\Resolvers\DomainTenantResolver; +use Illuminate\Database\Console\Migrations\FreshCommand; +use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; class TenancyServiceProvider extends ServiceProvider { diff --git a/tests/PostgresTest.php b/tests/PostgresTest.php index 65c6f87b..dbe01afe 100644 --- a/tests/PostgresTest.php +++ b/tests/PostgresTest.php @@ -2,18 +2,30 @@ declare(strict_types=1); +use Illuminate\Support\Str; use Illuminate\Support\Facades\DB; use Stancl\Tenancy\Tests\Etc\Post; use Stancl\Tenancy\Tests\Etc\Tenant; +use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Schema; +use Stancl\Tenancy\Events\TenancyEnded; use Illuminate\Database\Schema\Blueprint; +use Stancl\Tenancy\Bootstrappers\Integrations\PostgresTenancyBootstrapper; use Stancl\Tenancy\Tests\Etc\ScopedComment; +use Stancl\Tenancy\Events\TenancyInitialized; +use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Jobs\DeleteTenantsPostgresUser; use Stancl\Tenancy\Jobs\CreatePostgresUserForTenant; +use Stancl\Tenancy\Listeners\RevertToCentralContext; beforeEach(function () { DB::purge('central'); + config(['tenancy.bootstrappers' => [PostgresTenancyBootstrapper::class]]); + + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); + config(['database.connections.central' => config('database.connections.pgsql')]); config(['tenancy.models.tenant_key_column' => 'tenant_id']); config(['tenancy.models.tenant' => $tenantClass = Tenant::class]); @@ -35,33 +47,34 @@ beforeEach(function () { Schema::dropIfExists($secondaryModel->getTable()); Schema::dropIfExists($primaryModel->getTable()); + Schema::dropIfExists('domains'); + Schema::dropIfExists($tenantTable); - if(! Schema::hasTable($tenantTable)) { - Schema::create($tenantTable, function (Blueprint $table) { - $table->string('id')->primary(); + // todo1 The Post/Comment models have non-UUID primary keys + Schema::create($tenantTable, function (Blueprint $table) { + $table->uuid('id')->default(Str::uuid()->toString())->nullable(false)->primary(); + $table->timestamps(); + $table->json('data')->nullable(); + }); - $table->timestamps(); - $table->json('data')->nullable(); - }); - } - - Schema::create($primaryModel->getTable(), function (Blueprint $table) use ($tenantTable) { - $table->increments('id'); + Schema::create($primaryModel->getTable(), function (Blueprint $table) { + $table->uuid('id')->default(Str::uuid()->toString())->nullable(false)->primary(); $table->string('text'); - $table->string($tenantKey = tenancy()->tenantKeyColumn()); - - $table->foreign($tenantKey)->references('id')->on($tenantTable)->onUpdate('cascade')->onDelete('cascade'); + $table->foreignUuid('tenant_id')->constrained('tenants')->onUpdate('cascade')->onDelete('cascade');; }); Schema::create($secondaryModel->getTable(), function (Blueprint $table) use ($primaryModel) { - $table->increments('id'); + $table->uuid('id')->default(Str::uuid()->toString())->nullable(false)->primary(); $table->string('text'); - $table->unsignedInteger($primaryModel->getForeignKey()); - - $table->foreign($primaryModel->getForeignKey())->references('id')->on($primaryModel->getTable())->onUpdate('cascade')->onDelete('cascade'); + $table->foreignUuid($primaryModel->getForeignKey())->constrained($primaryModel->getTable())->onUpdate('cascade')->onDelete('cascade'); }); }); +afterEach(function () { + Schema::dropIfExists('comments'); + Schema::dropIfExists('posts'); +}); + test('postgres user can get created using the job', function() { $tenant = Tenant::create(); $name = $tenant->getTenantKey(); @@ -94,7 +107,7 @@ test('correct rls policies get created', function () { $rlsModels = config('tenancy.models.rls'); $modelTables = collect($rlsModels)->map(fn (string $model) => (new $model)->getTable()); $getRlsPolicies = fn () => DB::select('select * from pg_policies'); - $getRlsTables = fn() => $modelTables->map(fn ($table) => DB::select('select relname, relrowsecurity, relforcerowsecurity from pg_class WHERE oid = ' . "'$table'::regclass"))->collapse(); + $getRlsTables = fn () => $modelTables->map(fn ($table) => DB::select('select relname, relrowsecurity, relforcerowsecurity from pg_class WHERE oid = ' . "'$table'::regclass"))->collapse(); // Drop all existing policies to check if the command creates policies for multiple tables foreach ($getRlsPolicies() as $policy) { @@ -118,17 +131,17 @@ test('correct rls policies get created', function () { }); test('queries are correctly scoped using RLS', function() { - // 1) create rls policies for tables + // Create rls policies for tables pest()->artisan('tenants:create-rls-policies'); - // 2) create two tenants with postgres users + // Create two tenants with postgres users $tenant = Tenant::create(); $secondTenant = Tenant::create(); CreatePostgresUserForTenant::dispatchSync($tenant); CreatePostgresUserForTenant::dispatchSync($secondTenant); - // 3) create posts and comments for both tenants + // Create posts and comments for both tenants tenancy()->initialize($tenant); $post1 = Post::create(['text' => 'first post']); @@ -144,7 +157,7 @@ test('queries are correctly scoped using RLS', function() { tenancy()->end(); // todo1 Add option to disable the global scopes that the BelongsToTenant trait adds to the models, make RLS scope the queries - // 4) Ensure RLS scopes the queries – expect that tenants cannot access the records (posts and comments) of other tenants + // Ensure RLS scopes the queries – expect that tenants cannot access the records (posts and comments) of other tenants tenancy()->initialize($tenant); expect(Post::all()->pluck('text')) @@ -163,4 +176,4 @@ test('queries are correctly scoped using RLS', function() { expect(ScopedComment::all()->pluck('text'))->toContain($post2Comment->text)->not()->toContain($post1Comment->text); tenancy()->end(); -})->skip('queries are scoped using the global scope'); +})->group('test'); diff --git a/tests/TestCase.php b/tests/TestCase.php index 804835eb..092e17f1 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -9,14 +9,14 @@ use Dotenv\Dotenv; use Stancl\Tenancy\Facades\Tenancy; use Stancl\Tenancy\Tests\Etc\Tenant; use Illuminate\Support\Facades\Redis; -use Stancl\Tenancy\Bootstrappers\PrefixCacheTenancyBootstrapper; use Illuminate\Foundation\Application; use Stancl\Tenancy\Facades\GlobalCache; use Stancl\Tenancy\TenancyServiceProvider; -use Stancl\Tenancy\Bootstrappers\RedisTenancyBootstrapper; -use Stancl\Tenancy\Bootstrappers\BroadcastTenancyBootstrapper; use Stancl\Tenancy\Bootstrappers\UrlTenancyBootstrapper; use Stancl\Tenancy\Bootstrappers\MailTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\RedisTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\BroadcastTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\PrefixCacheTenancyBootstrapper; abstract class TestCase extends \Orchestra\Testbench\TestCase {