cleanup

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
+     * Force, rather than just enable, the created RLS policies.
-     * don't bypass the scoping (table owners bypass RLS by default).
      *
-     * E.g. when using a custom implementation where you create tables as the RLS user,
+     * By default, table owners bypass RLS policies. When this is enabled,
-     * the queries won't be scoped for the RLS user unless we force the RLS scoping using
+     * they also need the BYPASSRLS permission. If your setup lets you create
-     * the `ALTER TABLE {$table} FORCE ROW LEVEL SECURITY` query in the `enableRLS` method.
+     * 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;
 

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.central' => array_merge(config('database.connections.pgsql'), [
-        config('database.connections.pgsql'),
+        'username' => $username,
-        ['username' => $username, 'password' => $password]
+        '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) {
+    // We are still usng the 'administrator' user
-        // 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
+    if ($bypassRls) {
-        // that the my.current_tenant session variable isn't set.
+        // Users with BYPASSRLS can always query tables regardless of forceRls setting
-        expect(fn () => Order::first())->toThrow(QueryException::class, 'unrecognized configuration parameter');
+        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
+        // Users without BYPASSRLS are subject to RLS policies even if they're table owners when forceRls is true
-        expect(Order::first())->not()->toBeNull();
+        // 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 {