From 2657f032dc1e403a274503f48835bd14828b18de Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 14 Sep 2023 17:17:16 +0200 Subject: [PATCH] Allow to automatically fix missing grants & elevalte database users Co-authored-by: Johannes Meyer --- .../controllers/MigrationsController.php | 22 ++- application/forms/MigrationForm.php | 102 +++++++++++ .../Application/Hook/Common/DbMigration.php | 17 +- .../Icinga/Application/Hook/MigrationHook.php | 26 +-- .../Icinga/Application/MigrationManager.php | 169 +++++++++++++++++- .../Application/ProvidedHook/DbMigration.php | 10 +- public/css/icinga/pending-migration.less | 23 ++- 7 files changed, 327 insertions(+), 42 deletions(-) diff --git a/application/controllers/MigrationsController.php b/application/controllers/MigrationsController.php index 1a6fc6a27..e1bcd5c4d 100644 --- a/application/controllers/MigrationsController.php +++ b/application/controllers/MigrationsController.php @@ -4,8 +4,11 @@ namespace Icinga\Controllers; +use Exception; use Icinga\Application\Hook\MigrationHook; +use Icinga\Application\Icinga; use Icinga\Application\MigrationManager; +use Icinga\Common\Database; use Icinga\Exception\MissingParameterException; use Icinga\Forms\MigrationForm; use Icinga\Web\Notification; @@ -19,6 +22,13 @@ use ipl\Web\Widget\ActionLink; class MigrationsController extends CompatController { + use Database; + + public function init() + { + Icinga::app()->getModuleManager()->loadModule('setup'); + } + public function indexAction(): void { $mm = MigrationManager::instance(); @@ -44,6 +54,8 @@ class MigrationsController extends CompatController } $migrateListForm = new MigrationForm(); + $migrateListForm->setRenderDatabaseUserChange(! $mm->validateDatabasePrivileges()); + $migrateGlobalForm = new MigrationForm(); $migrateGlobalForm->getAttributes()->set('name', sprintf('migrate-%s', MigrationHook::ALL_MIGRATIONS)); @@ -193,12 +205,18 @@ class MigrationsController extends CompatController $form->on(MigrationForm::ON_SUCCESS, function (MigrationForm $form) { $mm = MigrationManager::instance(); + /** @var array $elevatedPrivileges */ + $elevatedPrivileges = $form->getValue('database_setup'); + if ($elevatedPrivileges !== null && $elevatedPrivileges['grant_privileges'] === 'y') { + $mm->fixIcingaWebMysqlGrants($this->getDb(), $elevatedPrivileges); + } + $pressedButton = $form->getPressedSubmitElement(); if ($pressedButton) { $name = substr($pressedButton->getName(), 8); switch ($name) { case MigrationHook::ALL_MIGRATIONS: - if ($mm->applyAll()) { + if ($mm->applyAll($elevatedPrivileges)) { Notification::success($this->translate('Applied all migrations successfully')); } else { Notification::error( @@ -211,7 +229,7 @@ class MigrationsController extends CompatController break; default: $migration = $mm->getMigration($name); - if ($mm->apply($migration)) { + if ($mm->apply($migration, $elevatedPrivileges)) { Notification::success($this->translate('Applied pending migrations successfully')); } else { Notification::error( diff --git a/application/forms/MigrationForm.php b/application/forms/MigrationForm.php index 940479153..120c84684 100644 --- a/application/forms/MigrationForm.php +++ b/application/forms/MigrationForm.php @@ -4,20 +4,34 @@ namespace Icinga\Forms; +use Icinga\Application\MigrationManager; +use ipl\Html\Attributes; use ipl\Html\Form; +use ipl\Html\FormElement\CheckboxElement; +use ipl\Html\FormElement\FieldsetElement; +use ipl\Html\HtmlElement; +use ipl\Html\Text; +use ipl\I18n\Translation; +use ipl\Validator\CallbackValidator; use ipl\Web\Common\CsrfCounterMeasure; use ipl\Web\Common\FormUid; +use ipl\Web\FormDecorator\IcingaFormDecorator; +use PDOException; class MigrationForm extends Form { use CsrfCounterMeasure; use FormUid; + use Translation; protected $defaultAttributes = [ 'class' => ['icinga-form', 'migration-form', 'icinga-controls'], 'name' => 'migration-form' ]; + /** @var bool Whether to allow changing the current database user and password */ + protected $renderDatabaseUserChange = false; + public function hasBeenSubmitted(): bool { if (! $this->hasBeenSent()) { @@ -25,11 +39,99 @@ class MigrationForm extends Form } $pressedButton = $this->getPressedSubmitElement(); + return $pressedButton && strpos($pressedButton->getName(), 'migrate-') !== false; } + public function setRenderDatabaseUserChange(bool $value = true): self + { + $this->renderDatabaseUserChange = $value; + + return $this; + } + protected function assemble(): void { $this->addHtml($this->createUidElement()); + + if ($this->renderDatabaseUserChange) { + $mm = MigrationManager::instance(); + $newDbSetup = new FieldsetElement('database_setup', ['required' => true]); + $newDbSetup + ->setDefaultElementDecorator(new IcingaFormDecorator()) + ->addElement('text', 'username', [ + 'required' => true, + 'label' => $this->translate('Username'), + 'description' => $this->translate( + 'A user which is able to create and/or alter the database schema.' + ) + ]) + ->addElement('password', 'password', [ + 'required' => true, + 'autocomplete' => 'new-password', + 'label' => $this->translate('Password'), + 'description' => $this->translate('The password for the database user defined above.'), + 'validators' => [ + new CallbackValidator(function ($_, CallbackValidator $validator) use ($mm, $newDbSetup): bool { + /** @var array $values */ + $values = $this->getValue('database_setup'); + /** @var CheckboxElement $checkBox */ + $checkBox = $newDbSetup->getElement('grant_privileges'); + $canIssueGrants = $checkBox->isChecked(); + $elevationConfig = [ + 'username' => $values['username'], + 'password' => $values['password'] + ]; + + try { + if (! $mm->validateDatabasePrivileges($elevationConfig, $canIssueGrants)) { + $validator->addMessage(sprintf( + $this->translate( + 'The provided credentials cannot be used to execute "%s" SQL commands' + . ' and/or grant the missing privileges to other users.' + ), + implode(' ,', $mm->getRequiredDatabasePrivileges()) + )); + + return false; + } + } catch (PDOException $e) { + $validator->addMessage($e->getMessage()); + + return false; + } + + return true; + }) + ] + ]) + ->addElement('checkbox', 'grant_privileges', [ + 'required' => false, + 'label' => $this->translate('Grant Missing Privileges'), + 'description' => $this->translate( + 'Allows to automatically grant the required privileges to the database user specified' + . ' in the respective resource config. If you do not want to provide additional credentials' + . ' each time, you can enable this and Icinga Web will grant the active database user the' + . ' missing privileges.' + ) + ]); + + $this->addHtml( + new HtmlElement( + 'div', + Attributes::create(['class' => 'change-database-user-description']), + new HtmlElement('span', null, Text::create(sprintf( + $this->translate( + 'It seems that the currently used database user does not have the required privileges to' + . ' execute the %s SQL commands. Please provide an alternative user' + . ' that has the appropriate credentials to resolve this issue.' + ), + implode(', ', $mm->getRequiredDatabasePrivileges()) + ))) + ) + ); + + $this->addElement($newDbSetup); + } } } diff --git a/library/Icinga/Application/Hook/Common/DbMigration.php b/library/Icinga/Application/Hook/Common/DbMigration.php index a115e10d1..ae79da785 100644 --- a/library/Icinga/Application/Hook/Common/DbMigration.php +++ b/library/Icinga/Application/Hook/Common/DbMigration.php @@ -27,8 +27,7 @@ class DbMigration public function __construct(string $version, string $scriptPath) { $this->scriptPath = $scriptPath; - - $this->setVersion($version); + $this->version = $version; } /** @@ -41,20 +40,6 @@ class DbMigration return $this->version; } - /** - * Set the sql script version the queries are loaded from - * - * @param string $version - * - * @return $this - */ - public function setVersion(string $version): self - { - $this->version = $version; - - return $this; - } - /** * Get upgrade script relative path name * diff --git a/library/Icinga/Application/Hook/MigrationHook.php b/library/Icinga/Application/Hook/MigrationHook.php index 1811b9839..4b8c632d0 100644 --- a/library/Icinga/Application/Hook/MigrationHook.php +++ b/library/Icinga/Application/Hook/MigrationHook.php @@ -14,13 +14,16 @@ use Icinga\Application\Icinga; use Icinga\Application\Logger; use Icinga\Application\Modules\Module; use Icinga\Model\Schema; +use Icinga\Module\Setup\Utils\DbTool; use Icinga\Web\Session; use ipl\I18n\Translation; use ipl\Orm\Query; use ipl\Sql\Adapter\Pgsql; use ipl\Sql\Connection; use ipl\Stdlib\Filter; +use ipl\Stdlib\Str; use PDO; +use PDOException; use SplFileInfo; use stdClass; @@ -128,6 +131,13 @@ abstract class MigrationHook implements Countable */ abstract public function getVersion(): string; + /** + * Get a database connection + * + * @return Connection + */ + abstract public function getDb(): Connection; + /** * Get all the pending migrations of this hook * @@ -166,9 +176,12 @@ abstract class MigrationHook implements Countable * * @return bool Whether the migration(s) have been successfully applied */ - public function run(): bool + final public function run(Connection $conn = null): bool { - $conn = $this->getDb(); + if (! $conn) { + $conn = $this->getDb(); + } + foreach ($this->getMigrations() as $migration) { try { $migration->apply($conn); @@ -176,7 +189,7 @@ abstract class MigrationHook implements Countable $this->version = $migration->getVersion(); unset($this->migrations[$migration->getVersion()]); - Logger::error( + Logger::info( "Applied %s pending migration version %s successfully", $this->getName(), $migration->getVersion() @@ -243,13 +256,6 @@ abstract class MigrationHook implements Countable return count($this->getMigrations()); } - /** - * Get a database connection - * - * @return Connection - */ - abstract protected function getDb(): Connection; - /** * Get a schema version query * diff --git a/library/Icinga/Application/MigrationManager.php b/library/Icinga/Application/MigrationManager.php index b1065261e..6151a84c2 100644 --- a/library/Icinga/Application/MigrationManager.php +++ b/library/Icinga/Application/MigrationManager.php @@ -8,11 +8,14 @@ use Countable; use Generator; use Icinga\Application\Hook\MigrationHook; use Icinga\Exception\NotFoundError; +use Icinga\Module\Setup\Utils\DbTool; +use Icinga\Module\Setup\WebWizard; use ipl\I18n\Translation; +use ipl\Sql; +use ReflectionClass; /** - * Migration manager encapsulates PHP code and DB migrations and manages all pending migrations in a - * structured way. + * Migration manager allows you to manage all pending migrations in a structured way. */ final class MigrationManager implements Countable { @@ -82,7 +85,7 @@ final class MigrationManager implements Countable * * @return MigrationHook * - * @throws NotFoundError When there are no pending PHP code migrations matching the given module name + * @throws NotFoundError When there are no pending migrations matching the given module name */ public function getMigration(string $module): MigrationHook { @@ -124,10 +127,11 @@ final class MigrationManager implements Countable * Apply the given migration hook * * @param MigrationHook $hook + * @param ?array $elevateConfig * * @return bool */ - public function apply(MigrationHook $hook): bool + public function apply(MigrationHook $hook, array $elevateConfig = null): bool { if ($hook->isModule() && $this->hasMigrations(MigrationHook::DEFAULT_MODULE)) { Logger::error( @@ -137,7 +141,12 @@ final class MigrationManager implements Countable return false; } - if ($hook->run()) { + $conn = $hook->getDb(); + if ($elevateConfig && ! $this->checkRequiredPrivileges($conn)) { + $conn = $this->elevateDatabaseConnection($conn, $elevateConfig); + } + + if ($hook->run($conn)) { unset($this->pendingMigrations[$hook->getModuleName()]); Logger::info('Applied pending %s migrations successfully', $hook->getName()); @@ -151,21 +160,23 @@ final class MigrationManager implements Countable /** * Apply all pending modules/framework migrations * + * @param ?array $elevateConfig + * * @return bool */ - public function applyAll(): bool + public function applyAll(array $elevateConfig = null): bool { $default = MigrationHook::DEFAULT_MODULE; if ($this->hasMigrations($default)) { $migration = $this->getMigration($default); - if (! $this->apply($migration)) { + if (! $this->apply($migration, $elevateConfig)) { return false; } } $succeeded = true; foreach ($this->getPendingMigrations() as $migration) { - if (! $this->apply($migration) && $succeeded) { + if (! $this->apply($migration, $elevateConfig) && $succeeded) { $succeeded = false; } } @@ -189,6 +200,99 @@ final class MigrationManager implements Countable } } + /** + * Get the required database privileges for database migrations + * + * @return string[] + */ + public function getRequiredDatabasePrivileges(): array + { + return ['CREATE','SELECT','INSERT','UPDATE','DELETE','DROP','ALTER','CREATE VIEW','INDEX','EXECUTE']; + } + + /** + * Verify whether all database users of all pending migrations do have the required SQL privileges + * + * @param ?array $elevateConfig + * @param bool $canIssueGrant + * + * @return bool + */ + public function validateDatabasePrivileges(array $elevateConfig = null, bool $canIssueGrant = false): bool + { + if (! $this->hasPendingMigrations()) { + return true; + } + + foreach ($this->getPendingMigrations() as $migration) { + if (! $this->checkRequiredPrivileges($migration->getDb(), $elevateConfig, $canIssueGrant)) { + return false; + } + } + + return true; + } + + /** + * Check if there are missing grants for the Icinga Web database and fix them + * + * This fixes the following problems on existing installations: + * - Setups made by the wizard have no access to `icingaweb_schema` + * - Setups made by the wizard have no DDL grants + * - Setups done manually using the advanced documentation chapter have no DDL grants + * + * @param Sql\Connection $db + * @param array $elevateConfig + */ + public function fixIcingaWebMysqlGrants(Sql\Connection $db, array $elevateConfig): void + { + $wizardProperties = (new ReflectionClass(WebWizard::class)) + ->getDefaultProperties(); + /** @var array $privileges */ + $privileges = $wizardProperties['databaseUsagePrivileges']; + /** @var array $tables */ + $tables = $wizardProperties['databaseTables']; + + $actualUsername = $db->getConfig()->username; + $db = $this->elevateDatabaseConnection($db, $elevateConfig); + $tool = $this->createDbTool($db); + $tool->connectToDb(); + + if ($tool->checkPrivileges(['SELECT'], [], $actualUsername)) { + // Checks only database level grants. If this succeeds, the grants were issued manually. + if (! $tool->checkPrivileges($privileges, [], $actualUsername) && $tool->isGrantable($privileges)) { + // Any missing grant is now granted on database level as well, not to mix things up + $tool->grantPrivileges($privileges, [], $actualUsername); + } + } elseif (! $tool->checkPrivileges($privileges, $tables, $actualUsername) && $tool->isGrantable($privileges)) { + // The above ensures that if this fails, we can safely apply table level grants, as it's + // very likely that the existing grants were issued by the setup wizard + $tool->grantPrivileges($privileges, $tables, $actualUsername); + } + } + + /** + * Create and return a DbTool instance + * + * @param Sql\Connection $db + * + * @return DbTool + */ + private function createDbTool(Sql\Connection $db): DbTool + { + $config = $db->getConfig(); + + return new DbTool(array_merge([ + 'db' => $config->db, + 'host' => $config->host, + 'port' => $config->port, + 'dbname' => $config->dbname, + 'username' => $config->username, + 'password' => $config->password, + 'charset' => $config->charset + ], $db->getAdapter()->getOptions($config))); + } + protected function load(): void { $this->pendingMigrations = []; @@ -205,6 +309,55 @@ final class MigrationManager implements Countable ksort($this->pendingMigrations); } + /** + * Check the required SQL privileges of the given connection + * + * @param Sql\Connection $conn + * @param ?array $elevateConfig + * @param bool $canIssueGrants + * + * @return bool + */ + protected function checkRequiredPrivileges( + Sql\Connection $conn, + array $elevateConfig = null, + bool $canIssueGrants = false + ): bool { + if ($elevateConfig) { + $conn = $this->elevateDatabaseConnection($conn, $elevateConfig); + } + + $dbTool = $this->createDbTool($conn); + $dbTool->connectToDb(); + if (! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges())) { + return false; + } + + if ($canIssueGrants && ! $dbTool->isGrantable($this->getRequiredDatabasePrivileges())) { + return false; + } + + return true; + } + + /** + * Override the database config of the given connection by the specified new config + * + * Overrides only the username and password of existing database connection. + * + * @param Sql\Connection $conn + * @param array $elevateConfig + * @return Sql\Connection + */ + protected function elevateDatabaseConnection(Sql\Connection $conn, array $elevateConfig): Sql\Connection + { + $config = clone $conn->getConfig(); + $config->username = $elevateConfig['username']; + $config->password = $elevateConfig['password']; + + return new Sql\Connection($config); + } + /** * Get all pending migrations as an array * diff --git a/library/Icinga/Application/ProvidedHook/DbMigration.php b/library/Icinga/Application/ProvidedHook/DbMigration.php index 5411ae157..37d299535 100644 --- a/library/Icinga/Application/ProvidedHook/DbMigration.php +++ b/library/Icinga/Application/ProvidedHook/DbMigration.php @@ -8,10 +8,18 @@ use Icinga\Application\Hook\MigrationHook; use Icinga\Common\Database; use Icinga\Model\Schema; use ipl\Orm\Query; +use ipl\Sql\Connection; class DbMigration extends MigrationHook { - use Database; + use Database { + getDb as public getPublicDb; + } + + public function getDb(): Connection + { + return $this->getPublicDb(); + } public function getName(): string { diff --git a/public/css/icinga/pending-migration.less b/public/css/icinga/pending-migration.less index 96f693bd4..512183167 100644 --- a/public/css/icinga/pending-migration.less +++ b/public/css/icinga/pending-migration.less @@ -2,13 +2,11 @@ @visual-width: 1.5em; -.migration-state-banner { +.migration-state-banner, .change-database-user-description { .rounded-corners(); - border: 1px solid @gray-lighter; - color: @text-color-light; - padding: 1em; - text-align: center; + border: 1px solid @gray-light; + color: @text-color; } .migrations { @@ -45,6 +43,17 @@ } // Layout + +.migration-state-banner, .change-database-user-description { + padding: 1em; + text-align: center; + + &.change-database-user-description { + max-width: 50em; + padding: .5em; + } +} + .pending-migrations-hint { text-align: center; @@ -63,6 +72,10 @@ .icinga-form { // Reset Icinga Form layout styles width: unset; max-width: unset; + + fieldset { + max-width: 50em; + } } .migration-list-control {