diff --git a/src/Bootstrappers/CacheTenancyBootstrapper.php b/src/Bootstrappers/CacheTenancyBootstrapper.php index 97bd7d24..74fc8490 100644 --- a/src/Bootstrappers/CacheTenancyBootstrapper.php +++ b/src/Bootstrappers/CacheTenancyBootstrapper.php @@ -16,6 +16,15 @@ use Stancl\Tenancy\Contracts\Tenant; /** * Makes cache tenant-aware by applying a prefix. + * + * Using this bootstrapper together with DatabaseTenancyBootstrapper + * with a database cache store will result in "double scoping". The store will be scoped + * by the DB connection (entries will go into the tenant's database) *and* by the prefix. + * This is harmless in most cases, but is important to be aware of. + * + * If you only use database cache stores, consider using DatabaseCacheBootstrapper instead. + * + * @see Stancl\Tenancy\Bootstrappers\DatabaseCacheBootstrapper */ class CacheTenancyBootstrapper implements TenancyBootstrapper { diff --git a/src/Bootstrappers/DatabaseCacheBootstrapper.php b/src/Bootstrappers/DatabaseCacheBootstrapper.php index 0e41849f..81611d0a 100644 --- a/src/Bootstrappers/DatabaseCacheBootstrapper.php +++ b/src/Bootstrappers/DatabaseCacheBootstrapper.php @@ -21,11 +21,15 @@ use Stancl\Tenancy\TenancyServiceProvider; * * By default, this bootstrapper scopes ALL cache stores that use the database driver. If you only * want to scope SOME stores, set the static $stores property to an array of names of the stores - * you want to scope. These stores must use 'database' as their driver. + * you want to scope. Those stores must use 'database' as their driver. * * Notably, this bootstrapper sets TenancyServiceProvider::$adjustCacheManagerUsing to a callback * that ensures all affected stores still use the central connection when accessed via global cache - * (typicaly the GlobalCache facade or global_cache() helper). + * (typically the GlobalCache facade or global_cache() helper). The code in TenancyServiceProvider + * that uses `extend()` callbacks to make database stores on the global cache manager use the central + * connection only corrects stores scoped by the Database*Tenancy*Bootstrapper. This bootstrapper + * also changes the stores' connection in the *config* to 'tenant' which doesn't let that callback + * change the connection back to central on the global cache manager. */ class DatabaseCacheBootstrapper implements TenancyBootstrapper { diff --git a/src/Bootstrappers/DatabaseTenancyBootstrapper.php b/src/Bootstrappers/DatabaseTenancyBootstrapper.php index 7f0bce0a..175d8b56 100644 --- a/src/Bootstrappers/DatabaseTenancyBootstrapper.php +++ b/src/Bootstrappers/DatabaseTenancyBootstrapper.php @@ -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; @@ -25,8 +53,8 @@ class DatabaseTenancyBootstrapper implements TenancyBootstrapper { /** @var TenantWithDatabase $tenant */ if (data_get($tenant->database()->getTemplateConnection(), 'url')) { - // The package works with individual parts of the database connection config, so DATABASE_URL is not supported. - // When DATABASE_URL is set, this bootstrapper can silently fail i.e. keep using the template connection's database URL + // The package works with individual parts of the database connection config, so DB_URL is not supported. + // When DB_URL is set, this bootstrapper can silently fail i.e. keep using the template connection's database URL // which takes precedence over individual segments of the connection config. This issue can be hard to debug as it can be // production-specific. Therefore, we throw an exception (that effectively blocks all tenant pages) to prevent incorrect DB use. throw new Exception('The template connection must NOT have URL defined. Specify the connection using individual parts instead of a database URL.'); @@ -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.'); + } + } } diff --git a/src/Commands/MigrateFresh.php b/src/Commands/MigrateFresh.php index d4733552..e53d6a89 100644 --- a/src/Commands/MigrateFresh.php +++ b/src/Commands/MigrateFresh.php @@ -14,6 +14,7 @@ use Stancl\Tenancy\Concerns\ParallelCommand; use Stancl\Tenancy\Database\Contracts\TenantWithDatabase; use Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException; use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Console\Output\OutputInterface as OI; class MigrateFresh extends BaseCommand @@ -72,11 +73,13 @@ class MigrateFresh extends BaseCommand protected function migrateTenant(TenantWithDatabase $tenant): bool { - return $this->callSilently('tenants:migrate', [ + $output = $this->getOutput()->isVerbose() ? $this->output : new NullOutput; + + return $this->runCommand('tenants:migrate', [ '--tenants' => [$tenant->getTenantKey()], '--step' => $this->option('step'), '--force' => true, - ]) === 0; + ], $output) === 0; } protected function childHandle(mixed ...$args): bool diff --git a/src/Database/Concerns/HasPending.php b/src/Database/Concerns/HasPending.php index 04fcccc1..e3d8a6fb 100644 --- a/src/Database/Concerns/HasPending.php +++ b/src/Database/Concerns/HasPending.php @@ -100,27 +100,51 @@ trait HasPending */ public static function pullPendingFromPool(bool $firstOrCreate = false, array $attributes = []): ?Tenant { - $tenant = DB::transaction(function () use ($attributes): ?Tenant { - /** @var (Model&Tenant)|null $tenant */ - $tenant = static::onlyPending()->first(); + // Attempt pulling a pending tenant. + // The loop handles the case where a single tenant is being pulled by multiple processes at the same time. + // If a tenant was pulled by a concurrent process, try pulling the next one in the pool. + while (true) { + /** @var (Model&Tenant)|null $pullCandidate */ + $pullCandidate = static::onlyPending()->first(); - if ($tenant !== null) { - event(new PullingPendingTenant($tenant)); - $tenant->update(array_merge($attributes, [ - 'pending_since' => null, - ])); + if ($pullCandidate === null) { + return $firstOrCreate ? static::create($attributes) : null; } + // Fired before the claim, so it can fire once per attempt, including for a candidate + // that ends up being claimed by a different process (in which case the loop retries). + // PendingTenantPulled (below) fires exactly once, for the actually pulled tenant. + event(new PullingPendingTenant($pullCandidate)); + + $tenant = DB::transaction(function () use ($pullCandidate, $attributes): ?Tenant { + $tenantWasPulled = static::onlyPending() + ->whereKey($pullCandidate->getKey()) + ->update([$pullCandidate->getColumnForQuery('pending_since') => null]) > 0; + + if (! $tenantWasPulled) { + return null; + } + + // The tenant's pending_since was just cleared, and a PullingPendingTenant listener + // may have made changes to the tenant, so re-fetch it to make sure it's up to date. + /** @var Model&Tenant $pulledTenant */ + $pulledTenant = static::findOrFail($pullCandidate->getKey()); + + if (! empty($attributes)) { + $pulledTenant->update($attributes); + } + + return $pulledTenant; + }); + + if ($tenant === null) { + // If another pull claimed this tenant first, try claiming the next one + continue; + } + + event(new PendingTenantPulled($tenant)); + return $tenant; - }); - - if ($tenant === null) { - return $firstOrCreate ? static::create($attributes) : null; } - - // Only triggered if a tenant that was pulled from the pool is returned - event(new PendingTenantPulled($tenant)); - - return $tenant; } } diff --git a/src/Database/Concerns/ManagesPostgresUsers.php b/src/Database/Concerns/ManagesPostgresUsers.php index bec94f49..62b920b9 100644 --- a/src/Database/Concerns/ManagesPostgresUsers.php +++ b/src/Database/Concerns/ManagesPostgresUsers.php @@ -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]); } } diff --git a/src/Database/Concerns/ValidatesDatabaseParameters.php b/src/Database/Concerns/ValidatesDatabaseParameters.php new file mode 100644 index 00000000..d6d01a03 --- /dev/null +++ b/src/Database/Concerns/ValidatesDatabaseParameters.php @@ -0,0 +1,96 @@ +?@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); + } +} diff --git a/src/Database/TenantDatabaseManagers/MicrosoftSQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/MicrosoftSQLDatabaseManager.php index da993956..f28ffd1e 100644 --- a/src/Database/TenantDatabaseManagers/MicrosoftSQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/MicrosoftSQLDatabaseManager.php @@ -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]); } } diff --git a/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php index b86faef2..10385208 100644 --- a/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/MySQLDatabaseManager.php @@ -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]); } } diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php index b373f41e..b10b82bc 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php @@ -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); } diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php index 47ec11a2..421b3bc3 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php @@ -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(*)'}; } } diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php index 1522234e..c846ace9 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLDatabaseManager.php @@ -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]); diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php index b528d4e3..b972ba0b 100644 --- a/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php +++ b/src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php @@ -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(<<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}\""); diff --git a/src/Database/TenantDatabaseManagers/PostgreSQLDatabaseManager.php b/src/Database/TenantDatabaseManagers/PostgreSQLDatabaseManager.php index 4fff7202..fc293403 100644 --- a/src/Database/TenantDatabaseManagers/PostgreSQLDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/PostgreSQLDatabaseManager.php @@ -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]); } } diff --git a/src/Database/TenantDatabaseManagers/PostgreSQLSchemaManager.php b/src/Database/TenantDatabaseManagers/PostgreSQLSchemaManager.php index d0fb0337..354eb768 100644 --- a/src/Database/TenantDatabaseManagers/PostgreSQLSchemaManager.php +++ b/src/Database/TenantDatabaseManagers/PostgreSQLSchemaManager.php @@ -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 diff --git a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php index 295cf304..ce3582c0 100644 --- a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php @@ -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.'); + } } } diff --git a/src/Database/TenantDatabaseManagers/TenantDatabaseManager.php b/src/Database/TenantDatabaseManagers/TenantDatabaseManager.php index 3d8d7610..a0822615 100644 --- a/src/Database/TenantDatabaseManagers/TenantDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/TenantDatabaseManager.php @@ -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; diff --git a/src/Features/DisallowSqliteAttach.php b/src/Features/DisallowSqliteAttach.php index 5cbfbf50..e5ae5c8f 100644 --- a/src/Features/DisallowSqliteAttach.php +++ b/src/Features/DisallowSqliteAttach.php @@ -74,8 +74,8 @@ class DisallowSqliteAttach implements Feature // @phpstan-ignore method.notFound $pdo->setAuthorizer(static function (int $action): int { return $action === 24 // SQLITE_ATTACH - ? PDO\Sqlite::DENY - : PDO\Sqlite::OK; + ? PDO\Sqlite::DENY // @phpstan-ignore classConstant.notFound + : PDO\Sqlite::OK; // @phpstan-ignore classConstant.notFound }); } } diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index afd20fb6..23d1ffab 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -86,14 +86,30 @@ class TenancyServiceProvider extends ServiceProvider // This works great for cache stores that are *directly* scoped, like Redis or // any other tagged or prefixed stores, but it doesn't work for the database driver. // - // When we use the DatabaseTenancyBootstrapper, it changes the default connection, - // and therefore the connection of the database store that will be created when - // this new CacheManager is instantiated again. + // When DatabaseTenancyBootstrapper is used, it changes the default DB connection + // to 'tenant'. A freshly created CacheManager would therefore instantiate database + // stores with the tenant connection. // - // For that reason, we also adjust the relevant stores on this new CacheManager - // using the callback below. It is set by DatabaseCacheBootstrapper. + // For that reason, we override the 'database' driver creator on this manager so that + // database stores are built with the central connection, and we run the + // $adjustCacheManagerUsing callback below (set by DatabaseCacheBootstrapper). $manager = new CacheManager($app); + // When DatabaseTenancyBootstrapper is used, database stores whose 'connection' + // config is null fall back to the default DB connection ('tenant'). Reset each + // such store to its explicitly configured connection, or fall back to central. + $centralConnection = $app['config']['tenancy.database.central_connection']; + $manager->extend('database', function ($_app, array $config) use ($centralConnection) { + $config['connection'] ??= $centralConnection; + + /** @var CacheManager $this */ + return $this->createDatabaseDriver($config); // @phpstan-ignore method.protected + }); + + // DatabaseCacheBootstrapper explicitly writes 'tenant' into each store's 'connection' + // config. The extend() closure above would then read 'tenant' as the configured value + // (not null) and use it directly, so the central connection fallback wouldn't be used. + // This callback is used to correct those connections back to central for globalCache. if (static::$adjustCacheManagerUsing !== null) { (static::$adjustCacheManagerUsing)($manager); } diff --git a/src/helpers.php b/src/helpers.php index 0b812e65..75514714 100644 --- a/src/helpers.php +++ b/src/helpers.php @@ -30,7 +30,6 @@ if (! function_exists('tenant')) { return app(Tenant::class); } - // @phpstan-ignore-next-line nullsafe.neverNull return app(Tenant::class)?->getAttribute($key); } } diff --git a/tests/Bootstrappers/DatabaseTenancyBootstrapper.php b/tests/Bootstrappers/DatabaseTenancyBootstrapper.php deleted file mode 100644 index 14109500..00000000 --- a/tests/Bootstrappers/DatabaseTenancyBootstrapper.php +++ /dev/null @@ -1,35 +0,0 @@ - $databaseUrl]); - - pest()->expectException(Exception::class); - } - - config(['tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class]]); - - $tenant1 = Tenant::create(); - - pest()->artisan('tenants:migrate'); - - tenancy()->initialize($tenant1); - - expect(true)->toBe(true); -})->with(['abc.us-east-1.rds.amazonaws.com', null]); - diff --git a/tests/Bootstrappers/DatabaseTenancyBootstrapperTest.php b/tests/Bootstrappers/DatabaseTenancyBootstrapperTest.php new file mode 100644 index 00000000..4debea9e --- /dev/null +++ b/tests/Bootstrappers/DatabaseTenancyBootstrapperTest.php @@ -0,0 +1,160 @@ + [DatabaseTenancyBootstrapper::class], + "tenancy.database.managers.{$connection}" => $manager, + ]); + + // Point the central connection at the tested connection's config and migrate it + // (so that the central database/schema contains the tenants table). + $centralConnection = config('tenancy.database.central_connection'); + $centralConfig = config("database.connections.{$connection}"); + + if ($connection === 'sqlite') { + $centralConfig['database'] = database_path($sqliteCentralDb = 'central.sqlite'); + } + + DB::purge($centralConnection); + config(["database.connections.{$centralConnection}" => $centralConfig]); + + pest()->artisan('migrate:fresh', [ + '--force' => true, + '--path' => __DIR__ . '/../../assets/migrations', + '--realpath' => true, + ]); + + DatabaseTenancyBootstrapper::$harden = $harden; + + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + // Create the tenant with its own database, then repoint it at the central database/schema + // (which contains the tenants table that the hardening check looks for). + $tenant = Tenant::create(['tenancy_db_connection' => $connection]); + + $central = DB::connection($centralConnection); + $centralName = match (true) { + $manager === PostgreSQLSchemaManager::class => $central->selectOne('SELECT current_schema() AS schema')->schema, // Central schema name + $connection === 'sqlite' => $sqliteCentralDb, // Central SQLite DB name + default => $central->getDatabaseName(), // Central DB name + }; + + $tenant->update(['tenancy_db_name' => $centralName]); + + if ($harden) { + // Harden blocks initialization for tenants that use the central database + expect(fn () => tenancy()->initialize($tenant))->toThrow(RuntimeException::class); + + // Connection should be reverted back to central + expect(DB::connection()->getName())->toBe($centralConnection); + } else { + expect(fn () => tenancy()->initialize($tenant))->not()->toThrow(Throwable::class); + + // Connection not reverted to central + expect(DB::connection()->getName())->toBe('tenant'); + } +})->with([ + 'hardening enabled' => true, + 'hardening disabled' => false, +])->with('db_managers'); + +test('harden prevents tenants from using the database of another tenant', function (bool $harden, string $connection, string $manager) { + config([ + 'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class], + "tenancy.database.managers.{$connection}" => $manager, + ]); + + DatabaseTenancyBootstrapper::$harden = $harden; + + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + $tenant = Tenant::create(['tenancy_db_connection' => $connection]); + + $dbName = Str::random(8) . ($connection === 'sqlite' ? '.sqlite' : ''); + + Tenant::create(['tenancy_db_name' => $dbName, 'tenancy_db_connection' => $connection]); + + $tenant->update(['tenancy_db_name' => $dbName]); + + if ($harden) { + // Harden blocks initialization for tenants that use the database of another tenant + expect(fn () => tenancy()->initialize($tenant))->toThrow(RuntimeException::class); + + // Connection should be reverted back to central + $centralConnection = config('tenancy.database.central_connection'); + + expect(DB::connection()->getName())->toBe($centralConnection); + } else { + expect(fn() => tenancy()->initialize($tenant))->not()->toThrow(Throwable::class); + + // Connection not reverted to central + expect(DB::connection()->getName())->toBe('tenant'); + } +})->with([ + 'hardening enabled' => true, + 'hardening disabled' => false, +])->with('db_managers'); + +test('database tenancy bootstrapper throws an exception if DB_URL is set', function (string|null $databaseUrl) { + config(['tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class]]); + + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + $tenant = Tenant::create(); + + config(['database.connections.central.url' => $databaseUrl]); + + if ($databaseUrl) { + expect(fn() => tenancy()->initialize($tenant)) + ->toThrow(Exception::class, 'The template connection must NOT have URL defined.'); + } else { + expect(fn() => tenancy()->initialize($tenant))->not()->toThrow(Throwable::class); + } +})->with(['abc.us-east-1.rds.amazonaws.com', null]); + +// Database managers to test with hardening. +// Permission controlled managers omitted as they inherit the non-perm controlled managers (= they share the same code paths), +// each important code path is covered by testing the non-permission controlled manager, so adding permission controlled managers +// would add unnecessary complexity to the tests. +dataset('db_managers', [ + 'mysql' => ['mysql', MySQLDatabaseManager::class], + 'pgsql (database)' => ['pgsql', PostgreSQLDatabaseManager::class], + 'pgsql (schema)' => ['pgsql', PostgreSQLSchemaManager::class], + 'sqlite' => ['sqlite', SQLiteDatabaseManager::class], +]); diff --git a/tests/CachedTenantResolverTest.php b/tests/CachedTenantResolverTest.php index 920c95a1..fc6cfb79 100644 --- a/tests/CachedTenantResolverTest.php +++ b/tests/CachedTenantResolverTest.php @@ -165,6 +165,7 @@ test('cache is invalidated when tenant is updated from within the tenant context ['redis', [CacheTenancyBootstrapper::class]], ['redis', [CacheTagsBootstrapper::class]], ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, CacheTenancyBootstrapper::class]], ]); test('cache is invalidated when the tenant is deleted', function (string $resolver, bool $configureTenantModelColumn) { diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index bda3eea9..faa897bf 100644 --- a/tests/CommandsTest.php +++ b/tests/CommandsTest.php @@ -366,6 +366,36 @@ test('migrate fresh command works', function () { expect(DB::table('users')->exists())->toBeFalse(); }); +test('migrate fresh command only shows migration output when run with the verbose option', function () { + $tenant = Tenant::create(); + $migratingOutput = 'Migrating tenant ' . $tenant->getTenantKey(); + + // CI runs pest with --verbose which makes Artisan::call() inherit the verbosity + // so we cannot easily test commands without -v. To work around that, we temporarily + // override $_ENV['SHELL_VERBOSITY'] immediately before executing the command. If this + // ever stops working, try also overriding the value in $_SERVER and putenv(). + $emptySentinel = new \stdClass(); + $originalVerbosity = $_ENV['SHELL_VERBOSITY'] ?? $emptySentinel; + try { + $_ENV['SHELL_VERBOSITY'] = 0; + Artisan::call('tenants:migrate-fresh'); + $defaultOutput = Artisan::output(); + } finally { + if ($originalVerbosity === $emptySentinel) { + unset($_ENV['SHELL_VERBOSITY']); + } else { + $_ENV['SHELL_VERBOSITY'] = $originalVerbosity; + } + } + + Artisan::call('tenants:migrate-fresh -v'); + $verboseOutput = Artisan::output(); + + // The output is silent by default and only shown with the verbose option + expect($defaultOutput)->not()->toContain($migratingOutput); + expect($verboseOutput)->toContain($migratingOutput); +}); + test('migrate fresh command respects force option in production', function () { // Set environment to production app()->detectEnvironment(fn() => 'production'); diff --git a/tests/GlobalCacheTest.php b/tests/GlobalCacheTest.php index 016ad2a4..4cda8b74 100644 --- a/tests/GlobalCacheTest.php +++ b/tests/GlobalCacheTest.php @@ -165,6 +165,7 @@ test('global cache is always central', function (string $store, array $bootstrap ['redis', [CacheTagsBootstrapper::class]], ['redis', [CacheTenancyBootstrapper::class]], ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, CacheTenancyBootstrapper::class]], ])->with([ 'helper', 'facade', diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index b04f8bc4..c9960728 100644 --- a/tests/PendingTenantsTest.php +++ b/tests/PendingTenantsTest.php @@ -5,6 +5,7 @@ declare(strict_types=1); use Illuminate\Database\QueryException; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Artisan; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Schema; use Illuminate\Support\Str; @@ -126,6 +127,52 @@ test('a new tenant gets created while pulling a pending tenant if the pending po expect(Tenant::withPending()->get()->count())->toBe(1); // All tenants }); +test('pulling a pending tenant retries when the tenant is claimed concurrently', function () { + Tenant::createPending(); + Tenant::createPending(); + + $stolenId = null; + + Event::listen(PullingPendingTenant::class, function (PullingPendingTenant $event) use (&$stolenId) { + if ($stolenId !== null) { + return; + } + + $stolenId = $event->tenant->id; + + // Steal the tenant like a concurrent process would + Tenant::onlyPending() + ->whereKey($event->tenant->id) + ->update([$event->tenant->getColumnForQuery('pending_since') => null]); + }); + + $pulled = Tenant::pullPendingFromPool(); + + expect($pulled)->not()->toBeNull(); + expect($pulled->id)->not()->toBe($stolenId); // Stolen tenant was skipped, the next one was claimed by the pull + expect(Tenant::onlyPending()->count())->toBe(0); // Both tenants claimed +}); + +test('the pull is rolled back and the tenant stays in the pool if setting attributes fails', function () { + // Pulling a tenant and setting its attributes happen in one transaction, + // so if setting the attributes fails, the whole pull rolls back and the tenant stays in the pool. + Schema::table('tenants', function (Blueprint $table) { + $table->string('slug')->nullable()->unique(); + }); + + Tenant::$extraCustomColumns = ['slug']; + + Tenant::create(['slug' => 'taken']); + Tenant::createPending(); + + // During the pull, set slug to 'taken', which is already used by another tenant to make the attribute update throw + expect(fn () => Tenant::pullPendingFromPool(false, ['slug' => 'taken'])) + ->toThrow(QueryException::class); + + // The pull rolled back, so the tenant is still pending + expect(Tenant::onlyPending()->count())->toBe(1); +}); + test('withoutPending chained with where clauses returns correct results', function () { $tenant = Tenant::create(); $pendingTenant = Tenant::createPending(); diff --git a/tests/TenantDatabaseManagerTest.php b/tests/TenantDatabaseManagerTest.php index 0d83e70e..d95cb1f0 100644 --- a/tests/TenantDatabaseManagerTest.php +++ b/tests/TenantDatabaseManagerTest.php @@ -17,6 +17,7 @@ use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; +use Stancl\Tenancy\Database\Contracts\ManagesDatabaseUsers; use Stancl\Tenancy\Database\Contracts\StatefulTenantDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\SQLiteDatabaseManager; @@ -36,6 +37,10 @@ beforeEach(function () { SQLiteDatabaseManager::$path = null; }); +afterEach(function () { + SQLiteDatabaseManager::$path = null; +}); + test('databases can be created and deleted', function ($driver, $databaseManager) { Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { return $event->tenant; @@ -539,6 +544,237 @@ test('partial tenant connection templates get merged into the central connection expect($manager->connection()->getConfig('url'))->toBeNull(); }); +test('database managers validate parameters used in raw sql statements', function ($driver, $databaseManager) { + config()->set([ + "tenancy.database.template_tenant_connection" => $driver, + ]); + + $manager = app($databaseManager); + + if ($manager instanceof StatefulTenantDatabaseManager) { + $manager->setConnection($driver); + } + + $invalidDatabaseName = "\"database_with_quotes\""; + + if (! ($manager instanceof ManagesDatabaseUsers)) { + // Only test createDatabase() and deleteDatabase() with non-permission controlled managers here + // since permission controlled managers override these methods to e.g. delete users before + // calling parent::deleteDatabase(), and with invalid DB name, the user deletion will already + // fail before we even get to actual deleteDatabase() logic. + $tenant = Tenant::make([ + 'tenancy_db_name' => $invalidDatabaseName, + ]); + + expect(fn () => $manager->createDatabase($tenant)) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + expect(fn () => $manager->deleteDatabase($tenant)) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + } else { + // Invalid username, createUser() and deleteUser() should + // throw an invalid argument exception. + $tenantWithInvalidUsername = Tenant::make([ + 'tenancy_db_name' => 'valid_database_name890', + 'tenancy_db_username' => "username with spaces", + ]); + + expect(fn () => $manager->createUser($tenantWithInvalidUsername->database())) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + expect(fn () => $manager->deleteUser($tenantWithInvalidUsername->database())) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + // Invalid database name, createUser() should throw + // an invalid argument exception. deleteUser() doesn't + // validate the DB name (it only validates the username). + $tenantWithInvalidDatabase = Tenant::make([ + 'tenancy_db_name' => $invalidDatabaseName, + 'tenancy_db_username' => 'valid_USERNAME', + 'tenancy_db_password' => 'valid_password', + ]); + + expect(fn () => $manager->createUser($tenantWithInvalidDatabase->database())) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + $tenantWithInvalidPassword = Tenant::make([ + 'tenancy_db_name' => 'valid_database_name890', + 'tenancy_db_username' => 'valid_USERNAME', + 'tenancy_db_password' => "p'ssword", + ]); + + expect(fn () => $manager->createUser($tenantWithInvalidPassword->database())) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + // Special characters are allowed in passwords + $tenantWithValidPassword = Tenant::make([ + 'tenancy_db_name' => 'valid_database_name890' . Str::random(8), + 'tenancy_db_username' => 'valid_USERNAME' . Str::random(8), + 'tenancy_db_password' => "]pa$$ ;word", + ]); + + expect(fn () => $manager->createUser($tenantWithValidPassword->database())) + ->not()->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + $tenantWithNullCredentials = Tenant::make([ + 'tenancy_db_name' => 'valid_db_name', + 'tenancy_db_username' => null, + 'tenancy_db_password' => null, + ]); + + expect(fn () => $manager->createUser($tenantWithNullCredentials->database())) + ->toThrow(InvalidArgumentException::class, 'Parameter cannot be null.'); + } +})->with('database_managers'); + +test('mysql database manager validates charset and collation correctly', function (string $param) { + $manager = app(MySQLDatabaseManager::class); + $manager->setConnection('mysql'); + + // using a non-string value (empty array) which is invalid + config(["database.connections.mysql.$param" => []]); + DB::purge('mysql'); + + $tenant = Tenant::make([ + 'tenancy_db_name' => 'valid_db_name', + ]); + + expect(fn () => $manager->createDatabase($tenant)) + ->toThrow(InvalidArgumentException::class, 'Parameter has to be a string.'); +})->with(['charset', 'collation']); + +test('sqlite database manager validates database names correctly', function () { + $manager = app(SQLiteDatabaseManager::class); + + // Dots are allowed in database names + expect(fn () => $manager->databaseExists('valid-db_name.sqlite')) + ->not()->toThrow(InvalidArgumentException::class); + + // Directory names are considered invalid input for database names + expect(fn () => $manager->databaseExists('..')) + ->toThrow(InvalidArgumentException::class); + + // Empty strings are considered invalid input for database names + expect(fn () => $manager->databaseExists('')) + ->toThrow(InvalidArgumentException::class); +}); + +test('sqlite database manager recognizes inmemory databases correctly', function () { + $manager = app(SQLiteDatabaseManager::class); + + expect($manager->isInMemory('file:_tenancy_inmemory_123?mode=memory&cache=shared'))->toBeTrue(); + expect($manager->isInMemory(':memory:'))->toBeTrue(); + + // Missing the '?mode=memory&cache=shared' suffix + expect($manager->isInMemory('file:_tenancy_inmemory_456'))->toBeFalse(); + + // Doesn't start with 'file:_tenancy_inmemory_' + expect($manager->isInMemory('_tenancy_inmemory_123?mode=memory&cache=shared'))->toBeFalse(); + + // In-memory DB name is validated correctly in makeConnectionConfig() + expect(fn () => $manager->makeConnectionConfig([], 'file:_tenancy_inmemory_12"3?mode=memory&cache=shared')) + ->toThrow(InvalidArgumentException::class, 'Forbidden character'); + + expect(fn () => $manager->makeConnectionConfig([], 'file:_tenancy_inmemory_123?mode=memory&cache=shared')) + ->not()->toThrow(InvalidArgumentException::class); + + expect(fn () => $manager->makeConnectionConfig([], ':memory:')) + ->not()->toThrow(InvalidArgumentException::class); +}); + +test('sqlite database manager respects the configured path while making the database config', function () { + config()->set([ + 'tenancy.database.template_tenant_connection' => 'sqlite', + ]); + + $tenant = Tenant::make([ + 'tenancy_db_name' => 'tenant.sqlite', + ]); + + // SQLiteDatabaseManager::$path is null, the database path is built using database_path() + expect($tenant->database()->connection()['database'])->toBe(database_path('tenant.sqlite')); + + SQLiteDatabaseManager::$path = '/custom/path/'; + + expect($tenant->database()->connection()['database'])->toBe('/custom/path/tenant.sqlite'); +}); + +test('newly created tenant databases use the correct charset and collation with mysql', function () { + config([ + 'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class], + 'database.connections.mysql.charset' => 'utf8mb4', + 'database.connections.mysql.collation' => 'utf8mb4_unicode_ci', + ]); + + Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + return $event->tenant; + })->toListener()); + + withBootstrapping(); + + $serverDefaultCharset = DB::selectOne('SELECT @@character_set_server AS charset')->charset; + $serverDefaultCollation = DB::selectOne('SELECT @@collation_server AS collation')->collation; + + $databaseCharset = fn () => DB::selectOne('SELECT DEFAULT_CHARACTER_SET_NAME FROM information_schema.SCHEMATA WHERE SCHEMA_NAME = DATABASE()')->DEFAULT_CHARACTER_SET_NAME; + $databaseCollation = fn () => DB::selectOne('SELECT DEFAULT_COLLATION_NAME FROM information_schema.SCHEMATA WHERE SCHEMA_NAME = DATABASE()')->DEFAULT_COLLATION_NAME; + + $defaultTenant = Tenant::create(); + + tenancy()->initialize($defaultTenant); + + // No charset or collation specified, + // defaults from the MySQL config used. + expect($databaseCharset())->toBe('utf8mb4'); + expect($databaseCollation())->toBe('utf8mb4_unicode_ci'); + + $tenantWithCharsetAndCollation = Tenant::create([ + 'tenancy_db_charset' => 'latin1', + 'tenancy_db_collation' => 'latin1_swedish_ci', + ]); + + tenancy()->initialize($tenantWithCharsetAndCollation); + + // Custom charset and collation from tenant config + expect($databaseCharset())->toBe('latin1'); + expect($databaseCollation())->toBe('latin1_swedish_ci'); + + $tenantWithNullCharsetAndCollation = Tenant::create([ + 'tenancy_db_charset' => null, + 'tenancy_db_collation' => null, + ]); + + tenancy()->initialize($tenantWithNullCharsetAndCollation); + + // Default MySQL server charset and collation + // (e.g. charset = utf8mb4, collation = utf8mb4_0900_ai_ci) + expect($databaseCharset())->toBe($serverDefaultCharset); + expect($databaseCollation())->toBe($serverDefaultCollation); + + $tenantWithCharsetAndNullCollation = Tenant::create([ + 'tenancy_db_charset' => 'binary', + 'tenancy_db_collation' => null, + ]); + + tenancy()->initialize($tenantWithCharsetAndNullCollation); + + // Charset specified, collation is null, + // MySQL will choose a default collation for the specified charset. + expect($databaseCharset())->toBe('binary'); + expect($databaseCollation())->toBe('binary'); + + // Collation specified, charset is null, + // MySQL will choose a default charset for the specified collation. + $tenantWithCollationAndNullCharset = Tenant::create([ + 'tenancy_db_charset' => null, + 'tenancy_db_collation' => 'latin1_swedish_ci', + ]); + + tenancy()->initialize($tenantWithCollationAndNullCharset); + + expect($databaseCharset())->toBe('latin1'); + expect($databaseCollation())->toBe('latin1_swedish_ci'); +}); + // Datasets dataset('database_managers', [ ['mysql', MySQLDatabaseManager::class], diff --git a/tests/TestCase.php b/tests/TestCase.php index ea480735..b9f36334 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -133,7 +133,7 @@ abstract class TestCase extends \Orchestra\Testbench\TestCase 'cache.stores.apc' => ['driver' => 'apc'], 'database.connections.central' => [ 'driver' => 'mysql', - 'url' => env('DATABASE_URL'), + 'url' => env('DB_URL'), 'host' => 'mysql', 'port' => env('DB_PORT', '3306'), 'database' => 'main',