diff --git a/src/Commands/CreateUserWithRLSPolicies.php b/src/Commands/CreateUserWithRLSPolicies.php index e48e52a4..420df935 100644 --- a/src/Commands/CreateUserWithRLSPolicies.php +++ b/src/Commands/CreateUserWithRLSPolicies.php @@ -23,12 +23,19 @@ class CreateUserWithRLSPolicies extends Command protected $description = "Creates RLS policies for all tables related to the tenant table. Also creates the RLS user if it doesn't exist yet"; /** - * Force RLS scoping on the tables, so that the table owner users - * don't bypass the scoping (table owners bypass RLS by default). + * Force, rather than just enable, the created RLS policies. * - * E.g. when using a custom implementation where you create tables as the RLS user, - * the queries won't be scoped for the RLS user unless we force the RLS scoping using - * the `ALTER TABLE {$table} FORCE ROW LEVEL SECURITY` query in the `enableRLS` method. + * By default, table owners bypass RLS policies. When this is enabled, + * they also need the BYPASSRLS permission. If your setup lets you create + * a user with BYPASSRLS, you may prefer leaving this on for additional + * safety. Otherwise, if you can't use BYPASSRLS, you can set this to false + * and depend on the behavior of table owners bypassing RLS automatically. + * + * This setting generally doesn't affect behavior at all with "default" + * setups, however if you have a more custom setup, with additional users + * involved (e.g. central connection user not being the same user that + * creates tables, or the created "RLS user" creating some tables) you + * should take care with how you configure this. */ public static bool $forceRls = true; diff --git a/tests/RLS/TableManagerTest.php b/tests/RLS/TableManagerTest.php index b5cc43a3..d48e5d66 100644 --- a/tests/RLS/TableManagerTest.php +++ b/tests/RLS/TableManagerTest.php @@ -164,7 +164,7 @@ test('correct rls policies get created with the correct hash using table manager } }); -test('queries are correctly scoped using RLS', function(bool $forceRls) { +test('queries are correctly scoped using RLS', function (bool $forceRls) { CreateUserWithRLSPolicies::$forceRls = $forceRls; // 3-levels deep relationship @@ -543,18 +543,68 @@ test('table rls manager generates relationship trees with tables related to the })->with([true, false]); // https://github.com/archtechx/tenancy/pull/1293 -test('user without BYPASSRLS can only query owned tables if forceRls is true', function(bool $forceRls) { +test('forceRls prevents even the table owner from querying his own tables if he doesnt have a BYPASSRLS permission', function (bool $forceRls) { CreateUserWithRLSPolicies::$forceRls = $forceRls; // Drop all tables created in beforeEach DB::statement("DROP TABLE authors, categories, posts, comments, reactions, articles;"); - [$username, $password] = createPostgresUser('administrator'); + // Create a new user so we have full control over the permissions. + // We explicitly set bypassRls to false. + [$username, $password] = createPostgresUser('administrator', bypassRls: false); - config(['database.connections.central' => array_merge( - config('database.connections.pgsql'), - ['username' => $username, 'password' => $password] - )]); + config(['database.connections.central' => array_merge(config('database.connections.pgsql'), [ + 'username' => $username, + 'password' => $password, + ])]); + + DB::reconnect(); + + // This table is owned by the newly created 'administrator' user + Schema::create('orders', function (Blueprint $table) { + $table->id(); + $table->string('name'); + + $table->string('tenant_id')->comment('rls'); + $table->foreign('tenant_id')->references('id')->on('tenants')->onUpdate('cascade')->onDelete('cascade'); + + $table->timestamps(); + }); + + $tenant1 = Tenant::create(); + + // Create RLS policy for the orders table + pest()->artisan('tenants:rls'); + + $tenant1->run(fn () => Order::create(['name' => 'order1', 'tenant_id' => $tenant1->getTenantKey()])); + + // We are still using the 'administrator' user - owner of the orders table + + if ($forceRls) { + // RLS is forced, so by default, not even the table owner should be able to query the table protected by the RLS policy. + // The RLS policy is not being bypassed, 'unrecognized configuration parameter' means + // that the my.current_tenant session variable isn't set -- the RLS policy is *still* being enforced. + expect(fn () => Order::first())->toThrow(QueryException::class, 'unrecognized configuration parameter "my.current_tenant"'); + } else { + // RLS is not forced, so the table owner should be able to query the table, bypassing the RLS policy + expect(Order::first())->not()->toBeNull(); + } +})->with([true, false]); + +test('users with BYPASSRLS privilege can bypass RLS regardless of forceRls setting', function (bool $forceRls, bool $bypassRls) { + CreateUserWithRLSPolicies::$forceRls = $forceRls; + + // Drop all tables created in beforeEach + DB::statement("DROP TABLE authors, categories, posts, comments, reactions, articles;"); + + // Create a new user so we have control over his BYPASSRLS permission + // and use that as the new central connection user + [$username, $password] = createPostgresUser('administrator', 'password', $bypassRls); + + config(['database.connections.central' => array_merge(config('database.connections.pgsql'), [ + 'username' => $username, + 'password' => $password, + ])]); DB::reconnect(); @@ -575,16 +625,25 @@ test('user without BYPASSRLS can only query owned tables if forceRls is true', f $tenant1->run(fn () => Order::create(['name' => 'order1', 'tenant_id' => $tenant1->getTenantKey()])); - if ($forceRls) { - // RLS is forced, so by default, not even the table owner should be able to query the table protected by the RLS policy. - // The RLS policy is not being bypassed, 'unrecognized configuration parameter' means - // that the my.current_tenant session variable isn't set. - expect(fn () => Order::first())->toThrow(QueryException::class, 'unrecognized configuration parameter'); + // We are still usng the 'administrator' user + + if ($bypassRls) { + // Users with BYPASSRLS can always query tables regardless of forceRls setting + expect(Order::count())->toBe(1); + expect(Order::first()->name)->toBe('order1'); } else { - // RLS is not forced, so the table owner should be able to query the table, bypassing the RLS policy - expect(Order::first())->not()->toBeNull(); + // Users without BYPASSRLS are subject to RLS policies even if they're table owners when forceRls is true + // OR they can bypass as table owners (when forceRls=false) + if ($forceRls) { + // Even table owners need session variable -- this means RLS was NOT bypassed + expect(fn () => Order::first())->toThrow(QueryException::class, 'unrecognized configuration parameter "my.current_tenant"'); + } else { + // Table owners can bypass RLS automatically when forceRls is false + expect(Order::count())->toBe(1); + expect(Order::first()->name)->toBe('order1'); + } } -})->with([true, false]); +})->with([true, false])->with([true, false]); test('table rls manager generates queries correctly', function() { expect(array_values(app(TableRLSManager::class)->generateQueries()))->toEqualCanonicalizing([ @@ -714,58 +773,6 @@ test('table manager ignores recursive relationship if the foreign key responsibl expect(fn () => app(TableRLSManager::class)->generateTrees())->not()->toThrow(RecursiveRelationshipException::class); }); -test('users with BYPASSRLS privilege can bypass RLS regardless of forceRls setting', function(bool $forceRls, bool $bypassRls) { - CreateUserWithRLSPolicies::$forceRls = $forceRls; - - // Drop all tables created in beforeEach - DB::statement("DROP TABLE authors, categories, posts, comments, reactions, articles;"); - - [$username, $password] = createPostgresUser('administrator', 'password', $bypassRls); - - config(['database.connections.central' => array_merge( - config('database.connections.pgsql'), - ['username' => $username, 'password' => $password] - )]); - - DB::reconnect(); - - Schema::create('orders', function (Blueprint $table) { - $table->id(); - $table->string('name'); - - $table->string('tenant_id')->comment('rls'); - $table->foreign('tenant_id')->references('id')->on('tenants')->onUpdate('cascade')->onDelete('cascade'); - - $table->timestamps(); - }); - - $tenant1 = Tenant::create(); - - // Create RLS policy for the orders table - pest()->artisan('tenants:rls'); - - $tenant1->run(fn () => Order::create(['name' => 'order1', 'tenant_id' => $tenant1->getTenantKey()])); - - if ($bypassRls) { - // Users with BYPASSRLS can always query tables regardless of forceRls setting - // This is the normal production setup behavior - expect(Order::first())->not()->toBeNull(); - expect(Order::first()->name)->toBe('order1'); - } else { - // Users without BYPASSRLS are subject to RLS policies even if they're table owners when forceRls is true - // OR they can bypass as table owners (when forceRls=false) - if ($forceRls) { - // Even table owners need session variable - expect(fn () => Order::first())->toThrow(QueryException::class, 'unrecognized configuration parameter'); - } else { - // Table owners can bypass RLS automatically - expect(Order::first())->not()->toBeNull(); - expect(Order::first()->name)->toBe('order1'); - } - } -})->with([true, false]) - ->with([true, false]); - function createPostgresUser(string $username, string $password = 'password', bool $bypassRls = false): array { try {