Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.x] Make forcing RLS configurable #1293

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
20 changes: 12 additions & 8 deletions src/Commands/CreateUserWithRLSPolicies.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ 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";

public static bool $forceRls = true;

public function handle(PermissionControlledPostgreSQLSchemaManager $manager): int
{
$username = config('tenancy.rls.user.username');
Expand Down Expand Up @@ -49,14 +51,16 @@ protected function enableRLS(string $table): void
// Enable RLS scoping on the table (without this, queries won't be scoped using RLS)
DB::statement("ALTER TABLE {$table} ENABLE ROW LEVEL SECURITY");

/**
* Force RLS scoping on the table, so that the table owner users
* 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,
* the queries won't be scoped for the RLS user unless we force the RLS scoping using this query.
*/
DB::statement("ALTER TABLE {$table} FORCE ROW LEVEL SECURITY");
if (static::$forceRls) {
/**
* Force RLS scoping on the table, so that the table owner users
* 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,
* the queries won't be scoped for the RLS user unless we force the RLS scoping using this query.
*/
DB::statement("ALTER TABLE {$table} FORCE ROW LEVEL SECURITY");
}
}

/**
Expand Down
13 changes: 8 additions & 5 deletions tests/RLS/PolicyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Stancl\Tenancy\Bootstrappers\PostgresRLSBootstrapper;

beforeEach(function () {
CreateUserWithRLSPolicies::$forceRls = true;
TraitRLSManager::$excludedModels = [Article::class];
TraitRLSManager::$modelDirectories = [__DIR__ . '/Etc'];

Expand Down Expand Up @@ -183,7 +184,9 @@
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');
Expand Down Expand Up @@ -215,7 +218,7 @@
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],
[true, false]
);
80 changes: 78 additions & 2 deletions tests/RLS/TableManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Stancl\Tenancy\Database\Exceptions\RecursiveRelationshipException;

beforeEach(function () {
CreateUserWithRLSPolicies::$forceRls = true;
stancl marked this conversation as resolved.
Show resolved Hide resolved
TableRLSManager::$scopeByDefault = true;

Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
Expand Down Expand Up @@ -158,7 +159,9 @@
}
});

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();
Expand Down Expand Up @@ -319,7 +322,7 @@

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;
Expand Down Expand Up @@ -534,6 +537,74 @@
]);
})->with([true, false]);

test('user without BYPASSRLS can only query owned tables if forceRls is true', function(bool $forceRls) {
CreateUserWithRLSPolicies::$forceRls = $forceRls;

// Drop all tables created in beforeEach
DB::statement("DROP TABLE authors, categories, posts, comments, reactions, articles;");

try {
DB::statement("DROP OWNED BY administrator;");
} catch (\Throwable $th) {}

DB::statement("DROP USER IF EXISTS administrator;");

// Create new central user (without superuser and bypassrls privileges)
DB::statement("CREATE USER administrator WITH ENCRYPTED PASSWORD 'password'");
DB::statement("ALTER USER administrator CREATEDB");
DB::statement("ALTER USER administrator CREATEROLE");

// Grant privileges to the new central user
DB::statement("GRANT ALL PRIVILEGES ON DATABASE main to administrator");
DB::statement("GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO administrator");
DB::statement("GRANT ALL ON SCHEMA public TO administrator");
DB::statement("ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT ALL PRIVILEGES ON TABLES TO administrator");
DB::statement("GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO administrator");

config(['database.connections.central' => array_merge(
config('database.connections.pgsql'),
['username' => 'administrator', '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();
$tenant2 = Tenant::create();

// Create RLS policy for the orders table
pest()->artisan('tenants:rls');

tenancy()->initialize($tenant1);

Order::create(['name' => 'order1', 'tenant_id' => $tenant1->getTenantKey()]);
expect(Order::first())->not()->toBeNull();

tenancy()->initialize($tenant2);

expect(Order::first())->toBeNull(); // RLS works

tenancy()->end();

if ($forceRls) {
// RLS is forced, so by default, not even the table owner should not be able to query the table protected by the RLS policy
// "unrecognized configuration parameter" = the my.current_tenant session variable isn't set -- the RLS policy is working
expect(fn () => Order::first())->toThrow(QueryException::class, 'unrecognized configuration parameter');
} 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('table rls manager generates queries correctly', function() {
expect(array_values(app(TableRLSManager::class)->generateQueries()))->toEqualCanonicalizing([
<<<SQL
Expand Down Expand Up @@ -701,3 +772,8 @@ class Author extends Model
{
protected $guarded = [];
}

class Order extends Model
{
protected $guarded = [];
}
12 changes: 7 additions & 5 deletions tests/RLS/TraitManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Stancl\Tenancy\Database\Concerns\BelongsToPrimaryModel;

beforeEach(function () {
CreateUserWithRLSPolicies::$forceRls = true;
TraitRLSManager::$implicitRLS = true;
TraitRLSManager::$modelDirectories = [__DIR__ . '/Etc'];
TraitRLSManager::$excludedModels = [Article::class];
Expand Down Expand Up @@ -148,7 +149,8 @@
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;
Expand Down Expand Up @@ -262,10 +264,10 @@

expect(fn () => DB::statement("INSERT INTO comments (text, post_id) VALUES ('third comment', {$post1->id})"))
->toThrow(QueryException::class);
})->with([
true,
false
]);
})->with(
[true, false],
[true, false],
);
stancl marked this conversation as resolved.
Show resolved Hide resolved

test('trait rls manager generates queries correctly', function() {
/** @var TraitRLSManager $manager */
Expand Down
Loading