From cf3d06c8ec2d1c8ce0e9e70cf5755cd2a1b447f4 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 8 Jan 2024 04:07:43 +0100 Subject: [PATCH] Add permission-controlled SqlSrv database manager (#17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add permission controleld MSSQL DB manager * Fix code style (php-cs-fixer) * Fix manager * Don't change databases when creating MSSQL user * Test permission controlled MSSQL DB manager * Fix code style (php-cs-fixer) * Delete redundant config resetting in tests * Grant user permissions insteead of making the user the database owner * Test that user gets created in the tenant DB * Test that the correct permissions are granted to the DB users * Fix code style (php-cs-fixer) * Update config comment * Fix typo * Add perm controlled sqlsr db manager to test dataset * Close all connections before deleting MSSQL DBs * Fix code style (php-cs-fixer) * Add explanation to deleteDatabase() * Update tests/DatabaseUsersTest.php Co-authored-by: Samuel Štancl * Fix code style (php-cs-fixer) --------- Co-authored-by: PHP CS Fixer Co-authored-by: Samuel Štancl --- assets/config.php | 3 +- ...olledMicrosoftSQLServerDatabaseManager.php | 65 +++++++++ tests/DatabaseUsersTest.php | 123 ++++++++++++++---- tests/TenantDatabaseManagerTest.php | 4 +- 4 files changed, 167 insertions(+), 28 deletions(-) create mode 100644 src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php diff --git a/assets/config.php b/assets/config.php index 37aa921f..391dadae 100644 --- a/assets/config.php +++ b/assets/config.php @@ -187,10 +187,11 @@ return [ 'sqlsrv' => Stancl\Tenancy\Database\TenantDatabaseManagers\MicrosoftSQLDatabaseManager::class, /** - * Use this database manager for MySQL to have a DB user created for each tenant database. + * Use these database managers to have a DB user created for each tenant database. * You can customize the grants given to these users by changing the $grants property. */ // 'mysql' => Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledMySQLDatabaseManager::class, + // 'sqlsrv' => Stancl\Tenancy\TenantDatabaseManagers\PermissionControlledMicrosoftSQLServerDatabaseManager::class, /** * Disable the pgsql manager above, and enable the one below if you diff --git a/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php new file mode 100644 index 00000000..aec43d46 --- /dev/null +++ b/src/Database/TenantDatabaseManagers/PermissionControlledMicrosoftSQLServerDatabaseManager.php @@ -0,0 +1,65 @@ +getName(); + $username = $databaseConfig->getUsername(); + $password = $databaseConfig->getPassword(); + + // Create login + $this->database()->statement("CREATE LOGIN [$username] WITH PASSWORD = '$password'"); + + // Create user in the database + // Grant the user permissions specified in the $grants array + // The 'CONNECT' permission is granted automatically + $grants = implode(', ', static::$grants); + + return $this->database()->statement("USE [$database]; CREATE USER [$username] FOR LOGIN [$username]; GRANT $grants TO [$username]"); + } + + public function deleteUser(DatabaseConfig $databaseConfig): bool + { + return $this->database()->statement("DROP LOGIN [{$databaseConfig->getUsername()}]"); + } + + public function userExists(string $username): bool + { + return (bool) $this->database()->select("SELECT sp.name as username FROM sys.server_principals sp WHERE sp.name = '{$username}'"); + } + + public function makeConnectionConfig(array $baseConfig, string $databaseName): array + { + $baseConfig['database'] = $databaseName; + + return $baseConfig; + } + + public function deleteDatabase(TenantWithDatabase $tenant): bool + { + // 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->database()->statement("ALTER DATABASE [{$tenant->database()->getName()}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;"); + + return parent::deleteDatabase($tenant); + } +} diff --git a/tests/DatabaseUsersTest.php b/tests/DatabaseUsersTest.php index 7b2af158..ce16f9f1 100644 --- a/tests/DatabaseUsersTest.php +++ b/tests/DatabaseUsersTest.php @@ -2,25 +2,29 @@ declare(strict_types=1); -use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Event; use Illuminate\Support\Str; +use Illuminate\Support\Facades\DB; use Stancl\JobPipeline\JobPipeline; -use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; -use Stancl\Tenancy\Contracts\ManagesDatabaseUsers; -use Stancl\Tenancy\Events\DatabaseCreated; -use Stancl\Tenancy\Events\TenancyInitialized; -use Stancl\Tenancy\Events\TenantCreated; -use Stancl\Tenancy\Database\Exceptions\TenantDatabaseUserAlreadyExistsException; -use Stancl\Tenancy\Jobs\CreateDatabase; -use Stancl\Tenancy\Listeners\BootstrapTenancy; -use Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager; -use Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledMySQLDatabaseManager; use Stancl\Tenancy\Tests\Etc\Tenant; +use Illuminate\Support\Facades\Event; +use Stancl\Tenancy\Jobs\CreateDatabase; +use Stancl\Tenancy\Events\TenantCreated; +use Stancl\Tenancy\Events\DatabaseCreated; +use Stancl\Tenancy\Database\DatabaseManager; +use Stancl\Tenancy\Events\TenancyInitialized; +use Stancl\Tenancy\Listeners\BootstrapTenancy; +use Stancl\Tenancy\Contracts\ManagesDatabaseUsers; +use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; +use Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager; +use Stancl\Tenancy\Database\TenantDatabaseManagers\MicrosoftSQLDatabaseManager; +use Stancl\Tenancy\Database\Exceptions\TenantDatabaseUserAlreadyExistsException; +use Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledMySQLDatabaseManager; +use Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledMicrosoftSQLServerDatabaseManager; beforeEach(function () { config([ 'tenancy.database.managers.mysql' => PermissionControlledMySQLDatabaseManager::class, + 'tenancy.database.managers.sqlsrv' => PermissionControlledMicrosoftSQLServerDatabaseManager::class, 'tenancy.database.suffix' => '', 'tenancy.database.template_tenant_connection' => 'mysql', ]); @@ -37,22 +41,44 @@ beforeEach(function () { })->toListener()); }); -test('users are created when permission controlled mysql manager is used', function () { +test('users are created when permission controlled manager is used', function (string $connection) { + config([ + 'database.default' => $connection, + 'tenancy.database.template_tenant_connection' => $connection, + ]); + $tenant = new Tenant([ 'id' => 'foo' . Str::random(10), ]); + $tenant->database()->makeCredentials(); /** @var ManagesDatabaseUsers $manager */ $manager = $tenant->database()->manager(); - expect($manager->userExists($tenant->database()->getUsername()))->toBeFalse(); + $username = $tenant->database()->getUsername(); + + expect($manager->userExists($username))->toBeFalse(); $tenant->save(); - expect($manager->userExists($tenant->database()->getUsername()))->toBeTrue(); -}); + expect($manager->userExists($username))->toBeTrue(); + + if ($connection === 'sqlsrv') { + app(DatabaseManager::class)->connectToTenant($tenant); + + expect((bool) DB::select("SELECT dp.name as username FROM sys.database_principals dp WHERE dp.name = '{$username}'"))->toBeTrue(); + } +})->with([ + 'mysql', + 'sqlsrv', +]); + +test('a tenants database cannot be created when the user already exists', function (string $connection) { + config([ + 'database.default' => $connection, + 'tenancy.database.template_tenant_connection' => $connection, + ]); -test('a tenants database cannot be created when the user already exists', function () { $username = 'foo' . Str::random(8); $tenant = Tenant::create([ 'tenancy_db_username' => $username, @@ -76,9 +102,12 @@ test('a tenants database cannot be created when the user already exists', functi // database was not created because of DB transaction expect($manager2->databaseExists($tenant2->database()->getName()))->toBeFalse(); Event::assertNotDispatched(DatabaseCreated::class); -}); +})->with([ + 'mysql', + 'sqlsrv', +]); -test('correct grants are given to users', function () { +test('correct grants are given to users using mysql', function () { PermissionControlledMySQLDatabaseManager::$grants = [ 'ALTER', 'ALTER ROUTINE', 'CREATE', ]; @@ -89,19 +118,32 @@ test('correct grants are given to users', function () { $query = DB::connection('mysql')->select("SHOW GRANTS FOR `{$tenant->database()->getUsername()}`@`%`")[1]; expect($query->{"Grants for {$user}@%"})->toStartWith('GRANT CREATE, ALTER, ALTER ROUTINE ON'); // @mysql because that's the hostname within the docker network +}); - // Reset static property - PermissionControlledMySQLDatabaseManager::$grants = [ - 'ALTER', 'ALTER ROUTINE', 'CREATE', 'CREATE ROUTINE', 'CREATE TEMPORARY TABLES', 'CREATE VIEW', - 'DELETE', 'DROP', 'EVENT', 'EXECUTE', 'INDEX', 'INSERT', 'LOCK TABLES', 'REFERENCES', 'SELECT', - 'SHOW VIEW', 'TRIGGER', 'UPDATE', - ]; +test('correct grants are given to users using sqlsrv', function () { + config([ + 'database.default' => 'sqlsrv', + 'tenancy.database.template_tenant_connection' => 'sqlsrv', + ]); + + PermissionControlledMicrosoftSQLServerDatabaseManager::$grants = ['SELECT']; + + $tenant = Tenant::create(['tenancy_db_username' => $user = 'tenant_user' . Str::random(8)]); + + // Connect to the tenant database to access tenant database user's permissions + app(DatabaseManager::class)->connectToTenant($tenant); + + $userGrants = DB::select("SELECT dp.permission_name as name FROM sys.database_permissions dp WHERE USER_NAME(dp.grantee_principal_id) = '$user'"); + + expect(array_map(fn ($grant) => $grant->name, $userGrants))->toEqual(array_merge( + ['CONNECT'], // Granted automatically + PermissionControlledMicrosoftSQLServerDatabaseManager::$grants + )); }); test('having existing databases without users and switching to permission controlled mysql manager doesnt break existing dbs', function () { config([ 'tenancy.database.managers.mysql' => MySQLDatabaseManager::class, - 'tenancy.database.suffix' => '', 'tenancy.database.template_tenant_connection' => 'mysql', 'tenancy.bootstrappers' => [ DatabaseTenancyBootstrapper::class, @@ -126,3 +168,32 @@ test('having existing databases without users and switching to permission contro expect($tenant->database()->manager() instanceof PermissionControlledMySQLDatabaseManager)->toBeTrue(); expect(config('database.connections.tenant.username'))->toBe('root'); }); + +test('having existing databases without users and switching to permission controlled sqlsrv manager doesnt break existing dbs', function () { + config([ + 'database.default' => 'sqlsrv', + 'tenancy.database.managers.sqlsrv' => MicrosoftSQLDatabaseManager::class, + 'tenancy.database.template_tenant_connection' => 'sqlsrv', + 'tenancy.bootstrappers' => [ + DatabaseTenancyBootstrapper::class, + ], + ]); + + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + + $tenant = Tenant::create([ + 'id' => 'foo' . Str::random(10), + ]); + + expect($tenant->database()->manager() instanceof MicrosoftSQLDatabaseManager)->toBeTrue(); + + tenancy()->initialize($tenant); // check if everything works + tenancy()->end(); + + config(['tenancy.database.managers.sqlsrv' => PermissionControlledMicrosoftSQLServerDatabaseManager::class]); + + tenancy()->initialize($tenant); // check if everything works + + expect($tenant->database()->manager() instanceof PermissionControlledMicrosoftSQLServerDatabaseManager)->toBeTrue(); + expect(config('database.connections.tenant.username'))->toBe('sa'); // default user for the sqlsrv connection +}); diff --git a/tests/TenantDatabaseManagerTest.php b/tests/TenantDatabaseManagerTest.php index 03718c66..99d9c4f8 100644 --- a/tests/TenantDatabaseManagerTest.php +++ b/tests/TenantDatabaseManagerTest.php @@ -21,6 +21,7 @@ use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Database\TenantDatabaseManagers\MicrosoftSQLDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledMySQLDatabaseManager; +use Stancl\Tenancy\Database\TenantDatabaseManagers\PermissionControlledMicrosoftSQLServerDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\PostgreSQLDatabaseManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\PostgreSQLSchemaManager; use Stancl\Tenancy\Database\TenantDatabaseManagers\SQLiteDatabaseManager; @@ -478,7 +479,8 @@ dataset('database_managers', [ ['sqlite', SQLiteDatabaseManager::class], ['pgsql', PostgreSQLDatabaseManager::class], ['pgsql', PostgreSQLSchemaManager::class], - ['sqlsrv', MicrosoftSQLDatabaseManager::class] + ['sqlsrv', MicrosoftSQLDatabaseManager::class], + ['sqlsrv', PermissionControlledMicrosoftSQLServerDatabaseManager::class] ]); function createUsersTable()