From a5d004132cbe38f1d174c14de592597b4cfab021 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Thu, 27 Apr 2023 13:47:56 +0200 Subject: [PATCH] Delete temp action, improve tests --- src/Actions/CreateRLSPoliciesForTables.php | 71 ---------------------- tests/PostgresTest.php | 50 +++++++-------- 2 files changed, 25 insertions(+), 96 deletions(-) delete mode 100644 src/Actions/CreateRLSPoliciesForTables.php diff --git a/src/Actions/CreateRLSPoliciesForTables.php b/src/Actions/CreateRLSPoliciesForTables.php deleted file mode 100644 index 4c9479fb..00000000 --- a/src/Actions/CreateRLSPoliciesForTables.php +++ /dev/null @@ -1,71 +0,0 @@ -getTable(); - - DB::statement("DROP POLICY IF EXISTS {$table}_rls_policy ON {$table}"); - - if (! Schema::hasColumn($table, $tenantKey)) { - // Table is not directly related to tenant - if (in_array(BelongsToPrimaryModel::class, class_uses_recursive($model::class))) { - $parentName = $model->getRelationshipToPrimaryModel(); - $parentKey = $model->$parentName()->getForeignKeyName(); - $parentModel = $model->$parentName()->make(); - $parentTable = str($parentModel->getTable())->toString(); - - DB::statement("CREATE POLICY {$table}_rls_policy ON {$table} USING ( - {$parentKey} IN ( - SELECT id - FROM {$parentTable} - WHERE ({$tenantKey}::TEXT = ( - SELECT {$tenantKey} - FROM {$parentTable} - WHERE id = {$parentKey} - )) - ) - )"); - - DB::statement("ALTER TABLE {$table} FORCE ROW LEVEL SECURITY"); - - return Command::SUCCESS; - } else { - $modelName = $model::class; - // $this->components->info("Table '$table' is not related to tenant. Make sure $modelName uses the BelongsToPrimaryModel trait."); - - return Command::FAILURE; - } - } - - DB::statement("CREATE POLICY {$table}_rls_policy ON {$table} USING ({$tenantKey}::TEXT = current_user);"); - - DB::statement("ALTER TABLE {$table} FORCE ROW LEVEL SECURITY"); - - // $this->components->info("Created RLS policy for table '$table'"); - } - - return Command::SUCCESS; - } - - public static function getTenantModels(): array - { - return array_map(fn (string $modelName) => (new $modelName), config('tenancy.models.rls')); - } -} diff --git a/tests/PostgresTest.php b/tests/PostgresTest.php index 33083387..7fea7dd8 100644 --- a/tests/PostgresTest.php +++ b/tests/PostgresTest.php @@ -15,6 +15,7 @@ beforeEach(function () { config(['tenancy.models.tenant' => Tenant::class]); + // Drop all existing policies foreach (DB::select('select * from pg_policies') as $policy) { DB::statement("DROP POLICY IF EXISTS {$policy->policyname} ON {$policy->tablename};"); } @@ -48,7 +49,7 @@ test('postgres user can get deleted using the job', function() { expect($tenantHasPostgresUser())->toBeFalse(); }); -test('correct rls policies get created', function(bool $action) { +test('correct rls policies get created', function () { config([ 'tenancy.models.rls' => [ Post::class, // Primary model (directly belongs to tenant) @@ -63,8 +64,6 @@ test('correct rls policies get created', function(bool $action) { Schema::create('tenants', function (Blueprint $table) { $table->string('id')->primary(); - // your custom columns may go here - $table->timestamps(); $table->json('data')->nullable(); }); @@ -89,20 +88,18 @@ test('correct rls policies get created', function(bool $action) { }); $getRlsPolicies = fn () => DB::select('select * from pg_policies'); - $getModelTables = fn () => collect(config('tenancy.models.rls'))->map(fn (string $model) => (new $model)->getTable()); + $getRlsModels = fn () => config('tenancy.models.rls'); + $getModelTables = fn () => collect($getRlsModels())->map(fn (string $model) => (new $model)->getTable()); $getRlsTables = fn() => $getModelTables()->map(fn ($table) => DB::select('select relname, relrowsecurity, relforcerowsecurity from pg_class WHERE oid = ' . "'$table'::regclass"))->collapse(); expect($getRlsPolicies())->toHaveCount(0); - if ($action) { - CreateRLSPoliciesForTables::handle(); - } else { - pest()->artisan('tenants:create-rls-policies'); - } + pest()->artisan('tenants:create-rls-policies'); - expect($getRlsPolicies())->toHaveCount(count(config('tenancy.models.rls'))); // 1 - expect($getRlsTables())->toHaveCount(count(config('tenancy.models.rls'))); // 1 - // Check if tables with policies are RLS protected + expect($getRlsPolicies())->toHaveCount(count($getRlsModels())); // 1 + expect($getRlsTables())->toHaveCount(count($getRlsModels())); // 1 + + // Check if RLS is forced on tables with the policies foreach ($getRlsTables() as $table) { expect($getModelTables())->toContain($table->relname); expect($table->relforcerowsecurity)->toBeTrue(); @@ -111,25 +108,28 @@ test('correct rls policies get created', function(bool $action) { config([ 'tenancy.models.rls' => array_merge([ ScopedComment::class, // Add secondary model to RLS protected models (belongs to tenant through Post) - ], config('tenancy.models.rls')), + ], $getRlsModels()), ]); - if ($action) { - CreateRLSPoliciesForTables::handle(); - } else { - pest()->artisan('tenants:create-rls-policies'); + expect(count($getRlsModels()))->toBe(2); // Post::class + ScopedComment::class + + // Drop all existing policies to check if the command creates policies for multiple tables + foreach ($getRlsPolicies() as $policy) { + DB::statement("DROP POLICY IF EXISTS {$policy->policyname} ON {$policy->tablename}"); } - // Check if tables with policies are RLS protected (even the ones not directly related to the tenant) - // Models related to tenant through some model must use the BelongsToPrimaryModel trait to work properly - expect($getRlsPolicies())->toHaveCount(count(config('tenancy.models.rls'))); // 2 - expect($getRlsTables())->toHaveCount(count(config('tenancy.models.rls'))); // 2 + expect($getRlsPolicies())->toHaveCount(0); + + pest()->artisan('tenants:create-rls-policies'); + + // Check if all tables with policies are RLS protected (even the ones not directly related to the tenant) + // Models related to tenant through some model must use the BelongsToPrimaryModel trait + // For the command to create the policy correctly for the model's table + expect($getRlsPolicies())->toHaveCount(count($getRlsModels())); // 2 + expect($getRlsTables())->toHaveCount(count($getRlsModels())); // 2 foreach ($getRlsTables() as $table) { expect($getModelTables())->toContain($table->relname); expect($table->relforcerowsecurity)->toBeTrue(); } -})->with([ - 'action' => true, - 'command' => false -]); +});