Merge branch 'may25' into resolve-test-todos

tests/PathIdentificationTest.php

@@ -3,15 +3,29 @@
 declare(strict_types=1);
 
 use Illuminate\Contracts\Http\Kernel;
+use Illuminate\Database\QueryException;
 use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Routing\Middleware\SubstituteBindings;
 use Illuminate\Support\Facades\Route;
 use Illuminate\Support\Facades\Schema;
+use Illuminate\Support\Facades\Event;
 use Stancl\Tenancy\Exceptions\RouteIsMissingTenantParameterException;
 use Stancl\Tenancy\Exceptions\TenantColumnNotWhitelistedException;
 use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedByPathException;
 use Stancl\Tenancy\Middleware\InitializeTenancyByPath;
 use Stancl\Tenancy\Resolvers\PathTenantResolver;
 use Stancl\Tenancy\Tests\Etc\Tenant;
+use Stancl\Tenancy\Events\TenantCreated;
+use Stancl\Tenancy\Jobs\CreateDatabase;
+use Stancl\Tenancy\Jobs\MigrateDatabase;
+use Stancl\JobPipeline\JobPipeline;
+use Stancl\Tenancy\Events\TenancyInitialized;
+use Stancl\Tenancy\Listeners\BootstrapTenancy;
+use Stancl\Tenancy\Listeners\RevertToCentralContext;
+use Stancl\Tenancy\Tests\Etc\User;
+use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper;
+use Stancl\Tenancy\Events\TenancyEnded;
+
 use function Stancl\Tenancy\Tests\pest;
 
 beforeEach(function () {
@@ -246,3 +260,38 @@ test('any extra model column needs to be whitelisted', function () {
     config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.allowed_extra_model_columns' => ['slug']]);
     pest()->get('/acme/foo')->assertSee($tenant->getTenantKey());
 });
+
+test('route model binding works with path identification', function() {
+    config(['tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class]]);
+
+    Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
+    Event::listen(TenantCreated::class, JobPipeline::make([
+        CreateDatabase::class, MigrateDatabase::class,
+    ])->send(fn (TenantCreated $event) => $event->tenant)->toListener());
+    Event::listen(TenancyEnded::class, RevertToCentralContext::class);
+
+    $tenant = Tenant::create();
+
+    $this->withoutExceptionHandling();
+
+    // Importantly, the route must have the 'web' middleware group, or SubstituteBindings directly
+    Route::get('/{tenant}/foo/{user}', fn (User $user) => $user->name)->middleware([InitializeTenancyByPath::class, 'web']);
+    Route::get('/{tenant}/bar/{user}', fn (User $user) => $user->name)->middleware([InitializeTenancyByPath::class, SubstituteBindings::class]);
+
+    $user = $tenant->run(fn () => User::create(['name' => 'John Doe', 'email' => 'john@doe.com', 'password' => 'foobar']));
+
+    pest()->get("/{$tenant->id}/foo/{$user->id}")->assertSee("John Doe");
+    tenancy()->end();
+    pest()->get("/{$tenant->id}/bar/{$user->id}")->assertSee("John Doe");
+    tenancy()->end();
+
+    // If SubstituteBindings comes BEFORE tenancy middleware and middleware priority is not set, route model binding is NOT expected to work correctly
+    // Since SubstituteBindings runs first, it tries to query the central database instead of the tenant database (which fails with a QueryException in this case)
+    Route::get('/{tenant}/baz/{user}', fn (User $user) => $user->name ?: 'No user')->middleware([SubstituteBindings::class, InitializeTenancyByPath::class]);
+    expect(fn () => pest()->get("/{$tenant->id}/baz/{$user->id}"))->toThrow(QueryException::class);
+    tenancy()->end();
+
+    // If SubstituteBindings is NOT USED AT ALL, we simply get an empty User instance
+    Route::get('/{tenant}/xyz/{user}', fn (User $user) => $user->name ?: 'No user')->middleware([InitializeTenancyByPath::class]);
+    pest()->get("/{$tenant->id}/xyz/{$user->id}")->assertSee('No user');
+});

tests/RLS/PolicyTest.php

@@ -20,6 +20,7 @@ use Stancl\Tenancy\Bootstrappers\PostgresRLSBootstrapper;
 use function Stancl\Tenancy\Tests\pest;
 
 beforeEach(function () {
+    CreateUserWithRLSPolicies::$forceRls = true;
     TraitRLSManager::$excludedModels = [Article::class];
     TraitRLSManager::$modelDirectories = [__DIR__ . '/Etc'];
 
@@ -79,6 +80,10 @@ beforeEach(function () {
     });
 });
 
+afterEach(function () {
+    CreateUserWithRLSPolicies::$forceRls = true;
+});
+
 // Regression test for https://github.com/archtechx/tenancy/pull/1280
 test('rls command doesnt fail when a view is in the database', function (string $manager) {
     DB::statement("
@@ -184,7 +189,9 @@ test('rls command recreates policies if the force option is passed', function (s
     TraitRLSManager::class,
 ]);
 
-test('queries will stop working when the tenant session variable is not set', function(string $manager) {
+test('queries will stop working when the tenant session variable is not set', function(string $manager, bool $forceRls) {
+    CreateUserWithRLSPolicies::$forceRls = $forceRls;
+
     config(['tenancy.rls.manager' => $manager]);
 
     $sessionVariableName = config('tenancy.rls.session_variable_name');
@@ -216,7 +223,4 @@ test('queries will stop working when the tenant session variable is not set', fu
         INSERT INTO posts (text, tenant_id, author_id)
         VALUES ('post2', ?, ?)
     SQL, [$tenant->id, $authorId]))->toThrow(QueryException::class);
-})->with([
-    TableRLSManager::class,
-    TraitRLSManager::class,
-]);
+})->with([TableRLSManager::class, TraitRLSManager::class])->with([true, false]);

tests/RLS/TableManagerTest.php

@@ -22,6 +22,7 @@ use Stancl\Tenancy\Database\Exceptions\RecursiveRelationshipException;
 use function Stancl\Tenancy\Tests\pest;
 
 beforeEach(function () {
+    CreateUserWithRLSPolicies::$forceRls = true;
     TableRLSManager::$scopeByDefault = true;
 
     Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
@@ -107,6 +108,10 @@ beforeEach(function () {
     });
 });
 
+afterEach(function () {
+    CreateUserWithRLSPolicies::$forceRls = true;
+});
+
 test('correct rls policies get created with the correct hash using table manager', function() {
     $manager = app(config('tenancy.rls.manager'));
 
@@ -159,7 +164,9 @@ test('correct rls policies get created with the correct hash using table manager
     }
 });
 
-test('queries are correctly scoped using RLS', function() {
+test('queries are correctly scoped using RLS', function (bool $forceRls) {
+    CreateUserWithRLSPolicies::$forceRls = $forceRls;
+
     // 3-levels deep relationship
     Schema::create('notes', function (Blueprint $table) {
         $table->id();
@@ -320,7 +327,7 @@ test('queries are correctly scoped using RLS', function() {
 
     expect(fn () => DB::statement("INSERT INTO notes (text, comment_id) VALUES ('baz', {$post1Comment->id})"))
         ->toThrow(QueryException::class);
-});
+})->with([true, false]);
 
 test('table rls manager generates relationship trees with tables related to the tenants table', function (bool $scopeByDefault) {
     TableRLSManager::$scopeByDefault = $scopeByDefault;
@@ -535,6 +542,109 @@ test('table rls manager generates relationship trees with tables related to the
     ]);
 })->with([true, false]);
 
+// https://github.com/archtechx/tenancy/pull/1293
+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;");
+
+    // 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,
+    ])]);
+
+    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();
+
+    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
+
+    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 {
+        // 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]);
+
 test('table rls manager generates queries correctly', function() {
     expect(array_values(app(TableRLSManager::class)->generateQueries()))->toEqualCanonicalizing([
         <<<SQL
@@ -663,6 +773,33 @@ test('table manager ignores recursive relationship if the foreign key responsibl
     expect(fn () => app(TableRLSManager::class)->generateTrees())->not()->toThrow(RecursiveRelationshipException::class);
 });
 
+function createPostgresUser(string $username, string $password = 'password', bool $bypassRls = false): array
+{
+    try {
+        DB::statement("DROP OWNED BY {$username};");
+    } catch (\Throwable) {}
+
+    DB::statement("DROP USER IF EXISTS {$username};");
+
+    DB::statement("CREATE USER {$username} WITH ENCRYPTED PASSWORD '{$password}'");
+    DB::statement("ALTER USER {$username} CREATEDB");
+    DB::statement("ALTER USER {$username} CREATEROLE");
+
+    // Grant BYPASSRLS privilege if requested
+    if ($bypassRls) {
+        DB::statement("ALTER USER {$username} BYPASSRLS");
+    }
+
+    // Grant privileges to the new central user
+    DB::statement("GRANT ALL PRIVILEGES ON DATABASE main to {$username}");
+    DB::statement("GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO {$username}");
+    DB::statement("GRANT ALL ON SCHEMA public TO {$username}");
+    DB::statement("ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT ALL PRIVILEGES ON TABLES TO {$username}");
+    DB::statement("GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO {$username}");
+
+    return [$username, $password];
+}
+
 class Post extends Model
 {
     protected $guarded = [];
@@ -715,3 +852,8 @@ class Author extends Model
 {
     protected $guarded = [];
 }
+
+class Order extends Model
+{
+    protected $guarded = [];
+}

tests/RLS/TraitManagerTest.php

@@ -28,6 +28,7 @@ use Stancl\Tenancy\Database\Concerns\BelongsToPrimaryModel;
 use function Stancl\Tenancy\Tests\pest;
 
 beforeEach(function () {
+    CreateUserWithRLSPolicies::$forceRls = true;
     TraitRLSManager::$implicitRLS = true;
     TraitRLSManager::$modelDirectories = [__DIR__ . '/Etc'];
     TraitRLSManager::$excludedModels = [Article::class];
@@ -78,6 +79,10 @@ beforeEach(function () {
     });
 });
 
+afterEach(function () {
+    CreateUserWithRLSPolicies::$forceRls = true;
+});
+
 test('correct rls policies get created with the correct hash using trait manager', function () {
     $manager = app(TraitRLSManager::class);
 
@@ -149,7 +154,8 @@ test('global scope is not applied when using rls with single db traits', functio
     expect(NonRLSComment::make()->hasGlobalScope(ParentModelScope::class))->toBeFalse();
 });
 
-test('queries are correctly scoped using RLS with trait rls manager', function (bool $implicitRLS) {
+test('queries are correctly scoped using RLS with trait rls manager', function (bool $implicitRLS, bool $forceRls) {
+    CreateUserWithRLSPolicies::$forceRls = $forceRls;
     TraitRLSManager::$implicitRLS = $implicitRLS;
 
     $postModel = $implicitRLS ? NonRLSPost::class : Post::class;
@@ -263,10 +269,7 @@ test('queries are correctly scoped using RLS with trait rls manager', function (
 
     expect(fn () => DB::statement("INSERT INTO comments (text, post_id) VALUES ('third comment', {$post1->id})"))
         ->toThrow(QueryException::class);
-})->with([
-    true,
-    false
-]);
+})->with([true, false])->with([true, false]);
 
 test('trait rls manager generates queries correctly', function() {
     /** @var TraitRLSManager $manager */