1
0
Fork 0
mirror of https://github.com/archtechx/tenancy.git synced 2026-08-06 07:54:03 +00:00

Parameter validation and other DB manager improvements (#1459)

### Parameter validation

In `statement()` calls of `TenantDatabaseManager`s, use parameter
binding when possible. When that's not possible, validate the parameters
using `validateParameter()` or `validatePassword()`.

Passwords use a less strict allowlist than other parameters (e.g. DB
names), since passwords tend to use more special characters but we can
afford to be more restrictive in the generic `validateParameter()`.

In `SQLiteDatabaseManager`, names of file-based databases are validated
(in `createDatabase`, `deleteDatabase` and `databaseExists`) using a
similar allowlist to the (non-password) parameters in other DB managers,
with an additional character: `.` (this addition is necessary since
file-based SQLite databases end with `.sqlite`).

### DatabaseTenancyBootstrapper - harden() and the lost test file

While checking for more places that could use validation, I realized
that it's possible to update tenant's db_name to the central DB or the
DB of another tenant. Added the `DatabaseTenancyBootstrapper::$harden`
property -- setting it to true prevents tenants from connecting to the
wrong databases (`RuntimeException` is thrown after connecting to the
wrong database).

Also, the DatabaseTenancyBootstrapper test file was ignored while
running tests because it lacked the `Test` suffix. Added the suffix and
fixed the broken `DATABASE_URL` test in the file.

### SQLiteDatabaseManager - respect static $path property in
makeConnectionConfig()

SQLiteDatabaseManager had a bug in `makeConnectionConfig`: the method
didn't respect the static `$path` property, it used `database_path()`
instead. Added a regression test for that. Also recognizing in-memory
SQLite databases (using `isInMemory()`) is more strict now so that
simply having a db_name with `_tenancy_inmemory_` somewhere in the name
doesn't make a file-based database considered in-memory.

### MySQLDatabaseManager - charset and collation defaulting

Creating databases with `null` charsets and collations resulted in a
`QueryException`, since null isn't a valid charset/collation. To solve
that, in the `CREATE DATABASE` statement in MySQLDatabaseManager, only
add charset/collation to the statement if they are not null.

MySQL defaults to the server's charset and collation, so it's safe to
not pass any charset/collation in the `CREATE DATABASE` statement and
let MySQL choose. Also, if e.g. collation is non-null and charset is
null, MySQL will use a charset compatible with the used collation, and
this works both ways.

---------

Co-authored-by: Samuel Stancl <samuel@archte.ch>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit is contained in:
lukinovec 2026-06-28 04:51:30 +02:00 committed by GitHub
parent ecf031237d
commit aa9d1d7fcf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 695 additions and 61 deletions

View file

@ -5,14 +5,42 @@ declare(strict_types=1);
namespace Stancl\Tenancy\Bootstrappers;
use Exception;
use Illuminate\Support\Facades\Schema;
use RuntimeException;
use Stancl\Tenancy\Contracts\TenancyBootstrapper;
use Stancl\Tenancy\Contracts\Tenant;
use Stancl\Tenancy\Database\Contracts\TenantWithDatabase;
use Stancl\Tenancy\Database\DatabaseManager;
use Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException;
use Throwable;
class DatabaseTenancyBootstrapper implements TenancyBootstrapper
{
/**
* When true, throw an exception if a tenant gets connected to
* another tenant's database or to the central database.
*
* This case should never come up in well-configured apps where
* users cannot set or edit tenant IDs or database names, so this
* option is disabled by default.
*
* However, applications dealing with extremely sensitive data may
* choose to enable this runtime check to prevent a bug or misconfiguration
* from creating an exploit that would let an attacker access another
* tenant's data or data from the central database.
*
* One way such a scenario might come up is if an application allows
* broad tenant attribute updates on a page for updating some fields
* on the tenant, without restricting that action to only a limited
* set of fields that are safe to edit. An attacker might be able to add
* something like ['tenancy_db_name' => '...'] to the request which could
* lead to this internal attribute being updated on an existing tenant.
*
* It's possible that enabling this setting will negate the performance
* benefits of cached tenant lookup.
*/
public static bool $harden = false;
/** @var DatabaseManager */
protected $database;
@ -41,10 +69,39 @@ class DatabaseTenancyBootstrapper implements TenancyBootstrapper
}
$this->database->connectToTenant($tenant);
if (static::$harden) {
try {
$this->verifyTenantCanUseDatabase($tenant);
} catch (Throwable $e) {
// Revert connection back to central
$this->revert();
throw $e;
}
}
}
public function revert(): void
{
$this->database->reconnectToCentral();
}
protected function verifyTenantCanUseDatabase(Tenant $tenant): void
{
/** @var \Stancl\Tenancy\Database\Models\Tenant&TenantWithDatabase $tenant */
$tenantDbName = $tenant->database()->getName();
// Check that no other tenant uses this tenant's database
if ($tenant::where($tenant->getTenantKeyName(), '!=', $tenant->getTenantKey())
->where($tenant::getDataColumn() . '->' . $tenant->internalPrefix() . 'db_name', $tenantDbName)
->exists()) {
throw new RuntimeException('Tenant cannot use a database of another tenant.');
}
if (Schema::hasTable($tenant->getTable())) {
// Throw if the current database/schema has the tenants table (i.e. it's not central)
throw new RuntimeException('Tenant cannot use the central database.');
}
}
}

View file

@ -28,6 +28,9 @@ trait ManagesPostgresUsers
$username = $databaseConfig->getUsername();
$password = $databaseConfig->getPassword();
$this->validateParameter($username);
$this->validatePassword($password);
$createUser = ! $this->userExists($username);
if ($createUser) {
@ -44,6 +47,8 @@ trait ManagesPostgresUsers
// Tenant DB username
$username = $databaseConfig->getUsername();
$this->validateParameter($username);
// Tenant host connection config
$connectionName = $this->connection()->getConfig('name');
$centralDatabase = $this->connection()->getConfig('database');
@ -77,6 +82,6 @@ trait ManagesPostgresUsers
public function userExists(string $username): bool
{
return (bool) $this->connection()->selectOne("SELECT usename FROM pg_user WHERE usename = '{$username}'");
return (bool) $this->connection()->select('SELECT usename FROM pg_user WHERE usename = ?', [$username]);
}
}

View file

@ -0,0 +1,96 @@
<?php
declare(strict_types=1);
namespace Stancl\Tenancy\Database\Concerns;
use InvalidArgumentException;
/**
* Provides methods to validate database parameters (e.g. database names, usernames, passwords)
* before using them in SQL statements (or in file paths in the case of SQLiteDatabaseManager).
*
* Used where parameters can be provided by users, and where parameter binding cannot be used.
*
* @see \Stancl\Tenancy\Database\TenantDatabaseManagers\TenantDatabaseManager
* @see \Stancl\Tenancy\Database\TenantDatabaseManagers\SQLiteDatabaseManager
*/
trait ValidatesDatabaseParameters
{
/**
* Characters allowed in parameters.
*
* Used as the default allowlist in validateParameter(), which validates non-password
* parameters such as database names or usernames.
*
* Since non-password parameters don't need to use as many special characters, we use
* a stricter allowlist here.
*/
public static string $allowedParameterCharacters = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-';
/**
* Characters allowed in database user passwords.
*
* The allowlist for passwords is less strict than for other parameters
* because it's more common to use more special characters in passwords.
*/
public static string $allowedPasswordCharacters = ' !#$%&()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_abcdefghijklmnopqrstuvwxyz{|}~';
/**
* Ensure that parameter (database name, username, etc.)
* only contains allowed characters before being used in SQL statements
* (or paths in the case of SQLiteDatabaseManager).
*
* By default, only the characters in $allowedParameterCharacters are allowed.
*
* @throws InvalidArgumentException
*/
protected function validateParameter(mixed $parameter, string|null $allowedCharacters = null): void
{
if (is_null($parameter)) {
throw new InvalidArgumentException('Parameter cannot be null.');
}
if (is_numeric($parameter)) {
$parameter = (string) $parameter;
}
if (! is_string($parameter)) {
throw new InvalidArgumentException('Parameter has to be a string.');
}
if ($parameter === '') {
throw new InvalidArgumentException('Parameter cannot be an empty string.');
}
$allowedCharacters ??= static::$allowedParameterCharacters;
foreach (str_split($parameter) as $character) {
if (! str_contains($allowedCharacters, $character)) {
throw new InvalidArgumentException("Forbidden character '{$character}' in parameter.");
}
}
}
/**
* Ensure password only contains allowed characters ($allowedPasswordCharacters)
* before being used in SQL statements.
*
* Used in permission controlled managers as a shorthand for calling validateParameter()
* with the less strict allowlist to validate database user passwords.
*
* @throws InvalidArgumentException
*/
protected function validatePassword(string|null $password): void
{
if (is_null($password)) {
throw new InvalidArgumentException('Password cannot be null.');
}
if ($password === '') {
throw new InvalidArgumentException('Password cannot be an empty string.');
}
$this->validateParameter($password, allowedCharacters: static::$allowedPasswordCharacters);
}
}

View file

@ -12,16 +12,22 @@ class MicrosoftSQLDatabaseManager extends TenantDatabaseManager
{
$database = $tenant->database()->getName();
$this->validateParameter($database);
return $this->connection()->statement("CREATE DATABASE [{$database}]");
}
public function deleteDatabase(TenantWithDatabase $tenant): bool
{
return $this->connection()->statement("DROP DATABASE [{$tenant->database()->getName()}]");
$database = $tenant->database()->getName();
$this->validateParameter($database);
return $this->connection()->statement("DROP DATABASE [{$database}]");
}
public function databaseExists(string $name): bool
{
return (bool) $this->connection()->select("SELECT name FROM master.sys.databases WHERE name = '$name'");
return (bool) $this->connection()->select('SELECT name FROM master.sys.databases WHERE name = ?', [$name]);
}
}

View file

@ -14,16 +14,38 @@ class MySQLDatabaseManager extends TenantDatabaseManager
$charset = $this->connection()->getConfig('charset');
$collation = $this->connection()->getConfig('collation');
return $this->connection()->statement("CREATE DATABASE `{$database}` CHARACTER SET `$charset` COLLATE `$collation`");
$this->validateParameter($database);
// MySQL defaults to the server's charset and collation
// if charset and collation are not specified.
// If charset is specified but collation is null, MySQL
// will choose a default collation for the specified charset (and vice versa).
$statement = "CREATE DATABASE `{$database}`";
if ($charset !== null) {
$this->validateParameter($charset);
$statement .= " CHARACTER SET `{$charset}`";
}
if ($collation !== null) {
$this->validateParameter($collation);
$statement .= " COLLATE `{$collation}`";
}
return $this->connection()->statement($statement);
}
public function deleteDatabase(TenantWithDatabase $tenant): bool
{
return $this->connection()->statement("DROP DATABASE `{$tenant->database()->getName()}`");
$database = $tenant->database()->getName();
$this->validateParameter($database);
return $this->connection()->statement("DROP DATABASE `{$database}`");
}
public function databaseExists(string $name): bool
{
return (bool) $this->connection()->select("SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = '$name'");
return (bool) $this->connection()->select('SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = ?', [$name]);
}
}

View file

@ -24,6 +24,10 @@ class PermissionControlledMicrosoftSQLServerDatabaseManager extends MicrosoftSQL
$username = $databaseConfig->getUsername();
$password = $databaseConfig->getPassword();
$this->validateParameter($database);
$this->validateParameter($username);
$this->validatePassword($password);
// Create login
$this->connection()->statement("CREATE LOGIN [$username] WITH PASSWORD = '$password'");
@ -37,12 +41,16 @@ class PermissionControlledMicrosoftSQLServerDatabaseManager extends MicrosoftSQL
public function deleteUser(DatabaseConfig $databaseConfig): bool
{
return $this->connection()->statement("DROP LOGIN [{$databaseConfig->getUsername()}]");
$username = $databaseConfig->getUsername();
$this->validateParameter($username);
return $this->connection()->statement("DROP LOGIN [{$username}]");
}
public function userExists(string $username): bool
{
return (bool) $this->connection()->select("SELECT sp.name as username FROM sys.server_principals sp WHERE sp.name = '{$username}'");
return (bool) $this->connection()->select('SELECT sp.name as username FROM sys.server_principals sp WHERE sp.name = ?', [$username]);
}
public function makeConnectionConfig(array $baseConfig, string $databaseName): array
@ -54,11 +62,15 @@ class PermissionControlledMicrosoftSQLServerDatabaseManager extends MicrosoftSQL
public function deleteDatabase(TenantWithDatabase $tenant): bool
{
$name = $tenant->database()->getName();
$this->validateParameter($name);
// Close all connections to the database before deleting it
// Set the database to SINGLE_USER mode to ensure that
// No other connections are using the database while we're trying to delete it
// Rollback all active transactions
$this->connection()->statement("ALTER DATABASE [{$tenant->database()->getName()}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;");
$this->connection()->statement("ALTER DATABASE [{$name}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;");
return parent::deleteDatabase($tenant);
}

View file

@ -25,6 +25,10 @@ class PermissionControlledMySQLDatabaseManager extends MySQLDatabaseManager impl
$username = $databaseConfig->getUsername();
$password = $databaseConfig->getPassword();
$this->validateParameter($database);
$this->validateParameter($username);
$this->validatePassword($password);
$this->connection()->statement("CREATE USER `{$username}`@`%` IDENTIFIED BY '{$password}'");
$grants = implode(', ', static::$grants);
@ -48,11 +52,15 @@ class PermissionControlledMySQLDatabaseManager extends MySQLDatabaseManager impl
public function deleteUser(DatabaseConfig $databaseConfig): bool
{
return $this->connection()->statement("DROP USER IF EXISTS '{$databaseConfig->getUsername()}'");
$username = $databaseConfig->getUsername();
$this->validateParameter($username);
return $this->connection()->statement("DROP USER IF EXISTS '{$username}'");
}
public function userExists(string $username): bool
{
return (bool) $this->connection()->select("SELECT count(*) FROM mysql.user WHERE user = '$username'")[0]->{'count(*)'};
return (bool) $this->connection()->select('SELECT count(*) FROM mysql.user WHERE user = ?', [$username])[0]->{'count(*)'};
}
}

View file

@ -20,6 +20,10 @@ class PermissionControlledPostgreSQLDatabaseManager extends PostgreSQLDatabaseMa
$username = $databaseConfig->getUsername();
$schema = $databaseConfig->connection()['search_path'];
$this->validateParameter($database);
$this->validateParameter($username);
$this->validateParameter($schema);
// Host config
$connectionName = $this->connection()->getConfig('name');
$centralDatabase = $this->connection()->getConfig('database');
@ -32,10 +36,10 @@ class PermissionControlledPostgreSQLDatabaseManager extends PostgreSQLDatabaseMa
$this->connection()->reconnect();
// Grant permissions to create and use tables in the configured schema ("public" by default) to the user
$this->connection()->statement("GRANT USAGE, CREATE ON SCHEMA {$schema} TO \"{$username}\"");
$this->connection()->statement("GRANT USAGE, CREATE ON SCHEMA \"{$schema}\" TO \"{$username}\"");
// Grant permissions to use sequences in the current schema to the user
$this->connection()->statement("GRANT USAGE ON ALL SEQUENCES IN SCHEMA {$schema} TO \"{$username}\"");
$this->connection()->statement("GRANT USAGE ON ALL SEQUENCES IN SCHEMA \"{$schema}\" TO \"{$username}\"");
// Reconnect to central database
config(["database.connections.{$connectionName}.database" => $centralDatabase]);

View file

@ -23,23 +23,27 @@ class PermissionControlledPostgreSQLSchemaManager extends PostgreSQLSchemaManage
// Central database name
$database = DB::connection(config('tenancy.database.central_connection'))->getDatabaseName();
$this->connection()->statement("GRANT CONNECT ON DATABASE {$database} TO \"{$username}\"");
$this->validateParameter($username);
$this->validateParameter($schema);
$this->validateParameter($database);
$this->connection()->statement("GRANT CONNECT ON DATABASE \"{$database}\" TO \"{$username}\"");
$this->connection()->statement("GRANT USAGE, CREATE ON SCHEMA \"{$schema}\" TO \"{$username}\"");
$this->connection()->statement("GRANT USAGE ON ALL SEQUENCES IN SCHEMA \"{$schema}\" TO \"{$username}\"");
$tables = $this->connection()->select("SELECT table_name FROM information_schema.tables WHERE table_schema = '{$schema}' AND table_type = 'BASE TABLE'");
$tables = $this->connection()->select("SELECT table_name FROM information_schema.tables WHERE table_schema = ? AND table_type = 'BASE TABLE'", [$schema]);
// Grant permissions to any existing tables. This is used with RLS
foreach ($tables as $table) {
$tableName = $table->table_name;
/** @var string $primaryKey */
$primaryKey = $this->connection()->selectOne(<<<SQL
$primaryKey = $this->connection()->selectOne(<<<'SQL'
SELECT column_name
FROM information_schema.key_column_usage
WHERE table_name = '{$tableName}'
WHERE table_name = ?
AND constraint_name LIKE '%_pkey'
SQL)->column_name;
SQL, [$tableName])->column_name;
// Grant all permissions for all existing tables
$this->connection()->statement("GRANT ALL ON \"{$schema}\".\"{$tableName}\" TO \"{$username}\"");

View file

@ -10,16 +10,24 @@ class PostgreSQLDatabaseManager extends TenantDatabaseManager
{
public function createDatabase(TenantWithDatabase $tenant): bool
{
return $this->connection()->statement("CREATE DATABASE \"{$tenant->database()->getName()}\" WITH TEMPLATE=template0");
$name = $tenant->database()->getName();
$this->validateParameter($name);
return $this->connection()->statement("CREATE DATABASE \"{$name}\" WITH TEMPLATE=template0");
}
public function deleteDatabase(TenantWithDatabase $tenant): bool
{
return $this->connection()->statement("DROP DATABASE \"{$tenant->database()->getName()}\"");
$name = $tenant->database()->getName();
$this->validateParameter($name);
return $this->connection()->statement("DROP DATABASE \"{$name}\"");
}
public function databaseExists(string $name): bool
{
return (bool) $this->connection()->selectOne("SELECT datname FROM pg_database WHERE datname = '$name'");
return (bool) $this->connection()->select('SELECT datname FROM pg_database WHERE datname = ?', [$name]);
}
}

View file

@ -10,17 +10,25 @@ class PostgreSQLSchemaManager extends TenantDatabaseManager
{
public function createDatabase(TenantWithDatabase $tenant): bool
{
return $this->connection()->statement("CREATE SCHEMA \"{$tenant->database()->getName()}\"");
$name = $tenant->database()->getName();
$this->validateParameter($name);
return $this->connection()->statement("CREATE SCHEMA \"{$name}\"");
}
public function deleteDatabase(TenantWithDatabase $tenant): bool
{
return $this->connection()->statement("DROP SCHEMA \"{$tenant->database()->getName()}\" CASCADE");
$name = $tenant->database()->getName();
$this->validateParameter($name);
return $this->connection()->statement("DROP SCHEMA \"{$name}\" CASCADE");
}
public function databaseExists(string $name): bool
{
return (bool) $this->connection()->select("SELECT schema_name FROM information_schema.schemata WHERE schema_name = '$name'");
return (bool) $this->connection()->select('SELECT schema_name FROM information_schema.schemata WHERE schema_name = ?', [$name]);
}
public function makeConnectionConfig(array $baseConfig, string $databaseName): array

View file

@ -6,13 +6,17 @@ namespace Stancl\Tenancy\Database\TenantDatabaseManagers;
use Closure;
use Illuminate\Database\Eloquent\Model;
use InvalidArgumentException;
use PDO;
use Stancl\Tenancy\Database\Concerns\ValidatesDatabaseParameters;
use Stancl\Tenancy\Database\Contracts\TenantDatabaseManager;
use Stancl\Tenancy\Database\Contracts\TenantWithDatabase;
use Throwable;
class SQLiteDatabaseManager implements TenantDatabaseManager
{
use ValidatesDatabaseParameters;
/**
* SQLite database directory path.
*
@ -57,6 +61,13 @@ class SQLiteDatabaseManager implements TenantDatabaseManager
*/
public static Closure|null $closeInMemoryConnectionUsing = null;
/**
* Characters allowed in database names.
*
* Includes dots to support file extensions (e.g. '.sqlite').
*/
public static string $allowedDatabaseNameCharacters = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-.';
public function createDatabase(TenantWithDatabase $tenant): bool
{
/** @var TenantWithDatabase&Model $tenant */
@ -122,6 +133,9 @@ class SQLiteDatabaseManager implements TenantDatabaseManager
public function makeConnectionConfig(array $baseConfig, string $databaseName): array
{
if ($this->isInMemory($databaseName)) {
// Named in-memory DBs are formatted like 'file:_tenancy_inmemory_tenant123?mode=memory&cache=shared'
$this->validateDatabaseName($databaseName, extraAllowedCharacters: ':?=&');
$baseConfig['database'] = $databaseName;
if (static::$persistInMemoryConnectionUsing !== null) {
@ -129,7 +143,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager
(static::$persistInMemoryConnectionUsing)(new PDO($dsn), $dsn);
}
} else {
$baseConfig['database'] = database_path($databaseName);
$baseConfig['database'] = $this->getPath($databaseName);
}
return $baseConfig;
@ -137,6 +151,8 @@ class SQLiteDatabaseManager implements TenantDatabaseManager
public function getPath(string $name): string
{
$this->validateDatabaseName($name);
if (static::$path) {
return rtrim(static::$path, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . $name;
}
@ -146,6 +162,28 @@ class SQLiteDatabaseManager implements TenantDatabaseManager
public static function isInMemory(string $name): bool
{
return $name === ':memory:' || str_contains($name, '_tenancy_inmemory_');
$isNamed = str_starts_with($name, 'file:_tenancy_inmemory_') &&
str_ends_with($name, '?mode=memory&cache=shared');
return $name === ':memory:' || $isNamed;
}
/**
* Ensure database name only contains allowed characters
* (allowedDatabaseNameCharacters() + $extraAllowedCharacters) and is not a directory name.
*
* @throws InvalidArgumentException
*/
protected function validateDatabaseName(string $name, string $extraAllowedCharacters = ''): void
{
$this->validateParameter($name, static::$allowedDatabaseNameCharacters . $extraAllowedCharacters);
if ($name === '') {
throw new InvalidArgumentException('Database name cannot be empty.');
}
if (is_dir($name)) {
throw new InvalidArgumentException('Database name cannot be a directory.');
}
}
}

View file

@ -6,11 +6,14 @@ namespace Stancl\Tenancy\Database\TenantDatabaseManagers;
use Illuminate\Database\Connection;
use Illuminate\Support\Facades\DB;
use Stancl\Tenancy\Database\Concerns\ValidatesDatabaseParameters;
use Stancl\Tenancy\Database\Contracts\StatefulTenantDatabaseManager;
use Stancl\Tenancy\Database\Exceptions\NoConnectionSetException;
abstract class TenantDatabaseManager implements StatefulTenantDatabaseManager
{
use ValidatesDatabaseParameters;
/** The database connection to the server. */
protected string $connection;