Fix PostgreSQL grants (#5130)

fixes #5129
This commit is contained in:
Johannes Meyer 2023-10-27 08:01:36 +02:00 committed by GitHub
commit 2fb36c0c8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 107 additions and 94 deletions

View File

@ -207,7 +207,7 @@ final class MigrationManager implements Countable
*/ */
public function getRequiredDatabasePrivileges(): array public function getRequiredDatabasePrivileges(): array
{ {
return ['CREATE','SELECT','INSERT','UPDATE','DELETE','DROP','ALTER','CREATE VIEW','INDEX','EXECUTE']; return ['CREATE','SELECT','INSERT','UPDATE','DELETE','DROP','ALTER','CREATE VIEW','INDEX','EXECUTE','USAGE'];
} }
/** /**
@ -258,7 +258,11 @@ final class MigrationManager implements Countable
$tool = $this->createDbTool($db); $tool = $this->createDbTool($db);
$tool->connectToDb(); $tool->connectToDb();
if ($tool->checkPrivileges(['SELECT'], [], $actualUsername)) { $isPgsql = $db->getAdapter() instanceof Sql\Adapter\Pgsql;
// PgSQL doesn't have SELECT privilege on a database level and granting the CREATE,CONNECT, and TEMPORARY
// privileges on a database doesn't permit a user to read data from a table. Hence, we have to grant the
// required database,schema and table privileges simultaneously.
if (! $isPgsql && $tool->checkPrivileges(['SELECT'], [], $actualUsername)) {
// Checks only database level grants. If this succeeds, the grants were issued manually. // Checks only database level grants. If this succeeds, the grants were issued manually.
if (! $tool->checkPrivileges($privileges, [], $actualUsername) && $tool->isGrantable($privileges)) { if (! $tool->checkPrivileges($privileges, [], $actualUsername) && $tool->isGrantable($privileges)) {
// Any missing grant is now granted on database level as well, not to mix things up // Any missing grant is now granted on database level as well, not to mix things up
@ -334,13 +338,20 @@ final class MigrationManager implements Countable
$dbTool = $this->createDbTool($conn); $dbTool = $this->createDbTool($conn);
$dbTool->connectToDb(); $dbTool->connectToDb();
if (! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges())
&& ! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges(), $tables) $isPgsql = $conn->getAdapter() instanceof Sql\Adapter\Pgsql;
) { $privileges = $this->getRequiredDatabasePrivileges();
$dbPrivilegesGranted = $dbTool->checkPrivileges($privileges);
$tablePrivilegesGranted = $dbTool->checkPrivileges($privileges, $tables);
if (! $dbPrivilegesGranted && ($isPgsql || ! $tablePrivilegesGranted)) {
return false; return false;
} }
if ($canIssueGrants && ! $dbTool->isGrantable($this->getRequiredDatabasePrivileges())) { if ($isPgsql && ! $tablePrivilegesGranted) {
return false;
}
if ($canIssueGrants && ! $dbTool->isGrantable($privileges)) {
return false; return false;
} }

View File

@ -141,14 +141,14 @@ class DbResourcePage extends Form
if ($this->getValue('db') === 'pgsql') { if ($this->getValue('db') === 'pgsql') {
if ($connectionError !== null) { if ($connectionError !== null) {
$this->warning(sprintf( // $this->warning(sprintf(
$this->translate('Unable to check the server\'s version. This is usually not a critical error' // $this->translate('Unable to check the server\'s version. This is usually not a critical error'
. ' as there is probably only access to the database permitted which does not exist yet. If you are' // . ' as there is probably only access to the database permitted which does not exist yet. If you are'
. ' absolutely sure you are running PostgreSQL in a version equal to or newer than 9.1,' // . ' absolutely sure you are running PostgreSQL in a version equal to or newer than 9.1,'
. ' you can skip the validation and safely proceed to the next step. The error was: %s'), // . ' you can skip the validation and safely proceed to the next step. The error was: %s'),
$connectionError->getMessage() // $connectionError->getMessage()
)); // ));
$state = false; // $state = false;
} else { } else {
$version = $db->getServerVersion(); $version = $db->getServerVersion();
if (version_compare($version, '9.1', '<')) { if (version_compare($version, '9.1', '<')) {

View File

@ -99,14 +99,7 @@ class DbTool
protected $pgsqlGrantContexts = array( protected $pgsqlGrantContexts = array(
'ALL' => 63, 'ALL' => 63,
'ALL PRIVILEGES' => 63, 'ALL PRIVILEGES' => 63,
'SELECT' => 24, 'CREATE' => 13,
'INSERT' => 24,
'UPDATE' => 24,
'DELETE' => 8,
'TRUNCATE' => 8,
'REFERENCES' => 24,
'TRIGGER' => 8,
'CREATE' => 12,
'CONNECT' => 4, 'CONNECT' => 4,
'TEMPORARY' => 4, 'TEMPORARY' => 4,
'TEMP' => 4, 'TEMP' => 4,
@ -633,13 +626,21 @@ class DbTool
} }
} elseif ($this->config['db'] === 'pgsql') { } elseif ($this->config['db'] === 'pgsql') {
$dbPrivileges = array(); $dbPrivileges = array();
$tablePrivileges = array(); $schemaPrivileges = [];
foreach (array_intersect($privileges, array_keys($this->pgsqlGrantContexts)) as $privilege) { foreach (array_intersect($privileges, array_keys($this->pgsqlGrantContexts)) as $privilege) {
if (! empty($context) && $this->pgsqlGrantContexts[$privilege] & static::TABLE_LEVEL) { if ($this->pgsqlGrantContexts[$privilege] & static::DATABASE_LEVEL) {
$tablePrivileges[] = $privilege;
} elseif ($this->pgsqlGrantContexts[$privilege] & static::DATABASE_LEVEL) {
$dbPrivileges[] = $privilege; $dbPrivileges[] = $privilege;
} }
if ($this->pgsqlGrantContexts[$privilege] & static::GLOBAL_LEVEL) {
$schemaPrivileges[] = $privilege;
}
}
if (! empty($schemaPrivileges)) {
// Allow the user to create,alter and use all attribute types in schema public
// such as creating and dropping custom data types (boolenum)
$this->exec(sprintf('GRANT %s ON SCHEMA public TO %s', implode(',', $schemaPrivileges), $username));
} }
if (! empty($dbPrivileges)) { if (! empty($dbPrivileges)) {
@ -651,15 +652,10 @@ class DbTool
)); ));
} }
if (! empty($tablePrivileges)) { foreach ($context as $table) {
foreach ($context as $table) { // PostgreSQL documentation says "You must own the table to use ALTER TABLE.", hence it isn't
$this->exec(sprintf( // sufficient to just issue grants, as the user is still not allowed to alter that table.
'GRANT %s ON TABLE %s TO %s', $this->exec(sprintf('ALTER TABLE %s OWNER TO %s', $table, $username));
join(',', $tablePrivileges),
$table,
$username
));
}
} }
} }
} }
@ -854,57 +850,71 @@ EOD;
$username = null $username = null
) { ) {
$privilegesGranted = true; $privilegesGranted = true;
if ($this->dbFromConfig) { $owner = $username ?: $this->config['username'];
$dbPrivileges = array(); $isSuperUser = $this->query('select rolsuper from pg_roles where rolname = :user', [':user' => $owner])
$tablePrivileges = array(); ->fetchColumn();
foreach (array_intersect($privileges, array_keys($this->pgsqlGrantContexts)) as $privilege) {
if (! empty($context) && $this->pgsqlGrantContexts[$privilege] & static::TABLE_LEVEL) {
$tablePrivileges[] = $privilege;
}
if ($this->pgsqlGrantContexts[$privilege] & static::DATABASE_LEVEL) {
$dbPrivileges[] = $privilege;
}
}
if (! empty($dbPrivileges)) { if ($this->dbFromConfig) {
$dbExclusivesGranted = true; $schemaPrivileges = [];
foreach ($dbPrivileges as $dbPrivilege) { $dbPrivileges = array();
$query = $this->query( if (! $isSuperUser) {
'SELECT has_database_privilege(:user, :dbname, :privilege) AS db_privilege_granted', foreach (array_intersect($privileges, array_keys($this->pgsqlGrantContexts)) as $privilege) {
array( if ($this->pgsqlGrantContexts[$privilege] & static::DATABASE_LEVEL) {
':user' => $username !== null ? $username : $this->config['username'], $dbPrivileges[] = $privilege;
':dbname' => $this->config['dbname'], }
':privilege' => $dbPrivilege . ($requireGrants ? ' WITH GRANT OPTION' : '') if ($this->pgsqlGrantContexts[$privilege] & static::GLOBAL_LEVEL) {
) $schemaPrivileges[] = $privilege;
); }
if (! $query->fetchObject()->db_privilege_granted) { }
$privilegesGranted = false;
if (! in_array($dbPrivilege, $tablePrivileges)) { if (! empty($schemaPrivileges)) {
$dbExclusivesGranted = false; foreach ($schemaPrivileges as $schemaPrivilege) {
$query = $this->query(
'SELECT has_schema_privilege(:user, :schema, :privilege) AS db_privilege_granted',
[
':user' => $owner,
':schema' => 'public',
':privilege' => $schemaPrivilege . ($requireGrants ? ' WITH GRANT OPTION' : '')
]
);
if (! $query->fetchObject()->db_privilege_granted) {
// The user doesn't fully have the provided privileges.
$privilegesGranted = false;
break;
} }
} }
} }
if ($privilegesGranted) { if ($privilegesGranted && ! empty($dbPrivileges)) {
// Do not check privileges twice if they are already granted at database level foreach ($dbPrivileges as $dbPrivilege) {
$tablePrivileges = array_diff($tablePrivileges, $dbPrivileges);
} elseif ($dbExclusivesGranted) {
$privilegesGranted = true;
}
}
if ($privilegesGranted && !empty($tablePrivileges)) {
foreach (array_intersect($context, $this->listTables()) as $table) {
foreach ($tablePrivileges as $tablePrivilege) {
$query = $this->query( $query = $this->query(
'SELECT has_table_privilege(:user, :table, :privilege) AS table_privilege_granted', 'SELECT has_database_privilege(:user, :dbname, :privilege) AS db_privilege_granted',
array( array(
':user' => $username !== null ? $username : $this->config['username'], ':user' => $owner,
':table' => $table, ':dbname' => $this->config['dbname'],
':privilege' => $tablePrivilege . ($requireGrants ? ' WITH GRANT OPTION' : '') ':privilege' => $dbPrivilege . ($requireGrants ? ' WITH GRANT OPTION' : '')
) )
); );
$privilegesGranted &= $query->fetchObject()->table_privilege_granted; if (! $query->fetchObject()->db_privilege_granted) {
// The user doesn't fully have the provided privileges.
$privilegesGranted = false;
break;
}
}
}
if ($privilegesGranted && ! empty($context)) {
foreach (array_intersect($context, $this->listTables()) as $table) {
$query = $this->query(
'SELECT tableowner FROM pg_catalog.pg_tables WHERE tablename = :tablename',
[':tablename' => $table]
);
if ($query->fetchColumn() !== $owner) {
$privilegesGranted = false;
break;
}
} }
} }
} }
@ -914,31 +924,27 @@ EOD;
// as the chances are very high that the database is created later causing the current user being // as the chances are very high that the database is created later causing the current user being
// the owner with ALL privileges. (Which in turn can be granted to others.) // the owner with ALL privileges. (Which in turn can be granted to others.)
if (array_search('CREATE', $privileges, true) !== false) { if (in_array('CREATE', $privileges, true)) {
$query = $this->query( $query = $this->query(
'select rolcreatedb from pg_roles where rolname = :user', 'select rolcreatedb from pg_roles where rolname = :user',
array(':user' => $username !== null ? $username : $this->config['username']) array(':user' => $username !== null ? $username : $this->config['username'])
); );
$privilegesGranted &= $query->fetchColumn() !== false; $privilegesGranted = $query->fetchColumn() !== false;
} }
} }
if (array_search('CREATEROLE', $privileges, true) !== false) { if ($privilegesGranted && in_array('CREATEROLE', $privileges, true)) {
$query = $this->query( $query = $this->query(
'select rolcreaterole from pg_roles where rolname = :user', 'select rolcreaterole from pg_roles where rolname = :user',
array(':user' => $username !== null ? $username : $this->config['username']) array(':user' => $username !== null ? $username : $this->config['username'])
); );
$privilegesGranted &= $query->fetchColumn() !== false; $privilegesGranted = $query->fetchColumn() !== false;
} }
if (array_search('SUPER', $privileges, true) !== false) { if ($privilegesGranted && in_array('SUPER', $privileges, true)) {
$query = $this->query( $privilegesGranted = $isSuperUser === true;
'select rolsuper from pg_roles where rolname = :user',
array(':user' => $username !== null ? $username : $this->config['username'])
);
$privilegesGranted &= $query->fetchColumn() !== false;
} }
return (bool) $privilegesGranted; return $privilegesGranted;
} }
} }

View File

@ -83,6 +83,7 @@ class WebWizard extends Wizard implements SetupWizard
'ALTER', 'ALTER',
'DROP', 'DROP',
'INDEX', 'INDEX',
'USAGE', // PostgreSQL
'TEMPORARY', // PostgreSql 'TEMPORARY', // PostgreSql
'CREATE TEMPORARY TABLES' // MySQL 'CREATE TEMPORARY TABLES' // MySQL
); );

View File

@ -25527,7 +25527,7 @@ parameters:
- -
message: "#^Cannot call method fetchColumn\\(\\) on mixed\\.$#" message: "#^Cannot call method fetchColumn\\(\\) on mixed\\.$#"
count: 8 count: 9
path: modules/setup/library/Setup/Utils/DbTool.php path: modules/setup/library/Setup/Utils/DbTool.php
- -
@ -25660,11 +25660,6 @@ parameters:
count: 1 count: 1
path: modules/setup/library/Setup/Utils/DbTool.php path: modules/setup/library/Setup/Utils/DbTool.php
-
message: "#^Parameter \\#1 \\$array of function array_intersect expects array, array\\|null given\\.$#"
count: 1
path: modules/setup/library/Setup/Utils/DbTool.php
- -
message: "#^Parameter \\#1 \\$dbname of method Icinga\\\\Module\\\\Setup\\\\Utils\\\\DbTool\\:\\:pdoConnect\\(\\) expects string, string\\|null given\\.$#" message: "#^Parameter \\#1 \\$dbname of method Icinga\\\\Module\\\\Setup\\\\Utils\\\\DbTool\\:\\:pdoConnect\\(\\) expects string, string\\|null given\\.$#"
count: 1 count: 1