From 71d9a8f760df8bbdc2fc8f2ff39cb38d0cd32a50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Fri, 23 Oct 2020 22:58:49 -0300 Subject: [PATCH 1/3] Remove is_grantuser and is_createuser PHP globals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/Common.php | 5 +--- .../Database/PrivilegesController.php | 6 ++--- .../Server/PrivilegesController.php | 11 ++++---- .../Table/PrivilegesController.php | 6 ++--- libraries/classes/Server/Privileges.php | 27 +++++++------------ .../Database/PrivilegesControllerTest.php | 7 ++--- .../Table/PrivilegesControllerTest.php | 7 ++--- test/classes/Server/PrivilegesTest.php | 8 ++++-- 8 files changed, 37 insertions(+), 40 deletions(-) diff --git a/libraries/classes/Common.php b/libraries/classes/Common.php index c0fc78d86e..5df2ed9ffb 100644 --- a/libraries/classes/Common.php +++ b/libraries/classes/Common.php @@ -16,7 +16,7 @@ final class Common { public static function server(): void { - global $db, $table, $viewing_mode, $err_url, $is_grantuser, $is_createuser, $dbi; + global $db, $table, $viewing_mode, $err_url, $dbi; /** * Handles some variables that may have been sent by the calling script @@ -34,9 +34,6 @@ final class Common */ $err_url = Url::getFromRoute('/'); - $is_grantuser = $dbi->isUserType('grant'); - $is_createuser = $dbi->isUserType('create'); - // now, select the mysql db if (! $dbi->isSuperuser()) { return; diff --git a/libraries/classes/Controllers/Database/PrivilegesController.php b/libraries/classes/Controllers/Database/PrivilegesController.php index 444cf1d3f5..b58bc2648d 100644 --- a/libraries/classes/Controllers/Database/PrivilegesController.php +++ b/libraries/classes/Controllers/Database/PrivilegesController.php @@ -41,7 +41,7 @@ class PrivilegesController extends AbstractController */ public function index(array $params): string { - global $cfg, $text_dir, $is_createuser, $is_grantuser, $PMA_Theme; + global $cfg, $text_dir, $PMA_Theme; $scriptName = Util::getScriptNameForOption( $cfg['DefaultTabDatabase'], @@ -59,8 +59,8 @@ class PrivilegesController extends AbstractController 'database_url' => $scriptName, 'theme_image_path' => $PMA_Theme->getImgPath(), 'text_dir' => $text_dir, - 'is_createuser' => $is_createuser, - 'is_grantuser' => $is_grantuser, + 'is_createuser' => $this->dbi->isUserType('create'), + 'is_grantuser' => $this->dbi->isUserType('grant'), 'privileges' => $privileges, ]); } diff --git a/libraries/classes/Controllers/Server/PrivilegesController.php b/libraries/classes/Controllers/Server/PrivilegesController.php index 65a26e1a07..d4099465fd 100644 --- a/libraries/classes/Controllers/Server/PrivilegesController.php +++ b/libraries/classes/Controllers/Server/PrivilegesController.php @@ -125,9 +125,10 @@ class PrivilegesController extends AbstractController /** * Checks if the user is allowed to do what they try to... */ - if (! $this->dbi->isSuperuser() && ! $GLOBALS['is_grantuser'] - && ! $GLOBALS['is_createuser'] - ) { + $isGrantUser = $this->dbi->isUserType('grant'); + $isCreateUser = $this->dbi->isUserType('create'); + + if (! $this->dbi->isSuperuser() && ! $isGrantUser && ! $isCreateUser) { $this->render('server/sub_page_header', [ 'type' => 'privileges', 'is_image' => false, @@ -139,7 +140,7 @@ class PrivilegesController extends AbstractController return; } - if (! $GLOBALS['is_grantuser'] && ! $GLOBALS['is_createuser']) { + if (! $isGrantUser && ! $isCreateUser) { $this->response->addHTML(Message::notice( __('You do not have the privileges to administrate the users!') )->getDisplay()); @@ -245,7 +246,7 @@ class PrivilegesController extends AbstractController * Assign users to user groups */ if (! empty($_POST['changeUserGroup']) && $cfgRelation['menuswork'] - && $this->dbi->isSuperuser() && $GLOBALS['is_createuser'] + && $this->dbi->isSuperuser() && $this->dbi->isUserType('create') ) { $serverPrivileges->setUserGroup($username, $_POST['userGroup']); $message = Message::success(); diff --git a/libraries/classes/Controllers/Table/PrivilegesController.php b/libraries/classes/Controllers/Table/PrivilegesController.php index 5d4b1b50a9..44f0f041fd 100644 --- a/libraries/classes/Controllers/Table/PrivilegesController.php +++ b/libraries/classes/Controllers/Table/PrivilegesController.php @@ -42,7 +42,7 @@ class PrivilegesController extends AbstractController */ public function index(array $params): string { - global $cfg, $text_dir, $is_createuser, $is_grantuser, $PMA_Theme; + global $cfg, $text_dir, $PMA_Theme; $scriptName = Util::getScriptNameForOption( $cfg['DefaultTabTable'], @@ -64,8 +64,8 @@ class PrivilegesController extends AbstractController 'table_url' => $scriptName, 'theme_image_path' => $PMA_Theme->getImgPath(), 'text_dir' => $text_dir, - 'is_createuser' => $is_createuser, - 'is_grantuser' => $is_grantuser, + 'is_createuser' => $this->dbi->isUserType('create'), + 'is_grantuser' => $this->dbi->isUserType('grant'), 'privileges' => $privileges, ]); } diff --git a/libraries/classes/Server/Privileges.php b/libraries/classes/Server/Privileges.php index 83cd9949c6..5c017f7832 100644 --- a/libraries/classes/Server/Privileges.php +++ b/libraries/classes/Server/Privileges.php @@ -1308,18 +1308,17 @@ class Privileges */ public function getHtmlForAddUser($dbname) { - global $is_grantuser; - + $isGrantUser = $this->dbi->isUserType('grant'); $loginInformationFieldsNew = $this->getHtmlForLoginInformationFields('new'); $privilegesTable = ''; - if ($is_grantuser) { + if ($isGrantUser) { $privilegesTable = $this->getHtmlToDisplayPrivilegesTable('*', '*', false); } return $this->template->render('server/privileges/add_user', [ 'database' => $dbname, 'login_information_fields_new' => $loginInformationFieldsNew, - 'is_grant_user' => $is_grantuser, + 'is_grant_user' => $isGrantUser, 'privileges_table' => $privilegesTable, ]); } @@ -1695,8 +1694,6 @@ class Privileges $hostname, $username ) { - global $is_grantuser; - if (isset($GLOBALS['dbname'])) { //if (preg_match('/\\\\(?:_|%)/i', $dbname)) { if (preg_match('/(?template->render('server/privileges/new_user_ajax', [ 'user' => $user, - 'is_grantuser' => $is_grantuser, + 'is_grantuser' => $this->dbi->isUserType('grant'), 'initial' => $_GET['initial'] ?? '', ]); @@ -2014,7 +2011,7 @@ class Privileges $onePrivilege['name'] = $name; $onePrivilege['edit_link'] = ''; - if ($GLOBALS['is_grantuser']) { + if ($this->dbi->isUserType('grant')) { $onePrivilege['edit_link'] = $this->getUserLink( 'edit', $username, @@ -2124,8 +2121,6 @@ class Privileges */ public function getUsersOverview($result, array $db_rights, $themeImagePath, $text_dir) { - global $is_grantuser, $is_createuser; - $cfgRelation = $this->relation->getRelationsParam(); while ($row = $this->dbi->fetchAssoc($result)) { @@ -2187,8 +2182,8 @@ class Privileges 'text_dir' => $text_dir, 'initial' => $_GET['initial'] ?? '', 'hosts' => $hosts, - 'is_grantuser' => $is_grantuser, - 'is_createuser' => $is_createuser, + 'is_grantuser' => $this->dbi->isUserType('grant'), + 'is_createuser' => $this->dbi->isUserType('create'), ]); } @@ -3016,7 +3011,7 @@ class Privileges */ public function getAddUserHtmlFieldset($db = '', $table = '') { - if (! $GLOBALS['is_createuser']) { + if (! $this->dbi->isUserType('create')) { return ''; } $rel_params = []; @@ -3048,8 +3043,6 @@ class Privileges */ public function getHtmlForUserOverview($themeImagePath, $text_dir) { - global $is_createuser; - $password_column = 'Password'; $server_type = Util::getServerType(); $serverVersion = $this->dbi->getVersion(); @@ -3208,7 +3201,7 @@ class Privileges 'empty_user_notice' => $emptyUserNotice ?? '', 'initials' => $initials ?? '', 'users_overview' => $usersOverview ?? '', - 'is_createuser' => $is_createuser, + 'is_createuser' => $this->dbi->isUserType('create'), 'flush_notice' => $flushNotice ?? '', ]); } @@ -3858,7 +3851,7 @@ class Privileges $real_sql_query .= ';'; $sql_query .= ';'; // No Global GRANT_OPTION privilege - if (! $GLOBALS['is_grantuser']) { + if (! $this->dbi->isUserType('grant')) { $real_sql_query = ''; $sql_query = ''; } diff --git a/test/classes/Controllers/Database/PrivilegesControllerTest.php b/test/classes/Controllers/Database/PrivilegesControllerTest.php index 1b9cf351bf..c09945bced 100644 --- a/test/classes/Controllers/Database/PrivilegesControllerTest.php +++ b/test/classes/Controllers/Database/PrivilegesControllerTest.php @@ -10,6 +10,7 @@ use PhpMyAdmin\Server\Privileges; use PhpMyAdmin\Template; use PhpMyAdmin\Tests\AbstractTestCase; use PhpMyAdmin\Url; +use PhpMyAdmin\Util; class PrivilegesControllerTest extends AbstractTestCase { @@ -27,14 +28,14 @@ class PrivilegesControllerTest extends AbstractTestCase public function testIndex(): void { - global $dbi, $db, $server, $cfg, $PMA_PHP_SELF, $is_grantuser, $is_createuser; + global $dbi, $db, $server, $cfg, $PMA_PHP_SELF; $db = 'db'; $server = 0; $cfg['Server']['DisableIS'] = false; $PMA_PHP_SELF = 'index.php'; - $is_grantuser = true; - $is_createuser = true; + Util::cacheSet('is_grantuser', true); + Util::cacheSet('is_createuser', true); $privileges = []; diff --git a/test/classes/Controllers/Table/PrivilegesControllerTest.php b/test/classes/Controllers/Table/PrivilegesControllerTest.php index 14d7ab9c25..30a34a97f5 100644 --- a/test/classes/Controllers/Table/PrivilegesControllerTest.php +++ b/test/classes/Controllers/Table/PrivilegesControllerTest.php @@ -10,6 +10,7 @@ use PhpMyAdmin\Server\Privileges; use PhpMyAdmin\Template; use PhpMyAdmin\Tests\AbstractTestCase; use PhpMyAdmin\Url; +use PhpMyAdmin\Util; class PrivilegesControllerTest extends AbstractTestCase { @@ -27,15 +28,15 @@ class PrivilegesControllerTest extends AbstractTestCase public function testIndex(): void { - global $dbi, $db, $table, $server, $cfg, $PMA_PHP_SELF, $is_grantuser, $is_createuser; + global $dbi, $db, $table, $server, $cfg, $PMA_PHP_SELF; $db = 'db'; $table = 'table'; $server = 0; $cfg['Server']['DisableIS'] = false; $PMA_PHP_SELF = 'index.php'; - $is_grantuser = true; - $is_createuser = true; + Util::cacheSet('is_grantuser', true); + Util::cacheSet('is_createuser', true); $privileges = []; diff --git a/test/classes/Server/PrivilegesTest.php b/test/classes/Server/PrivilegesTest.php index 79c58b47f3..0dbcf92e55 100644 --- a/test/classes/Server/PrivilegesTest.php +++ b/test/classes/Server/PrivilegesTest.php @@ -36,6 +36,7 @@ class PrivilegesTest extends AbstractTestCase protected function setUp(): void { parent::setUp(); + parent::defineVersionConstants(); parent::setLanguage(); parent::setGlobalConfig(); parent::setTheme(); @@ -111,11 +112,12 @@ class PrivilegesTest extends AbstractTestCase $dbi->expects($this->any())->method('escapeString') ->will($this->returnArgument(0)); + $dbi->expects($this->any())->method('isUserType') + ->will($this->returnValue(true)); + $GLOBALS['dbi'] = $dbi; $this->serverPrivileges->dbi = $dbi; $this->serverPrivileges->relation->dbi = $dbi; - $GLOBALS['is_grantuser'] = true; - $GLOBALS['is_createuser'] = true; $GLOBALS['is_reload_priv'] = true; } @@ -1290,6 +1292,8 @@ class PrivilegesTest extends AbstractTestCase $dbi->expects($this->any()) ->method('escapeString') ->will($this->returnArgument(0)); + $dbi->expects($this->any())->method('isUserType') + ->will($this->returnValue(true)); $GLOBALS['dbi'] = $dbi; $this->serverPrivileges->dbi = $dbi; From a83ec78ebf7f73abf60bb03a73a4e49b22654db0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 24 Oct 2020 00:40:03 -0300 Subject: [PATCH 2/3] Make DatabaseInterface::isUserType method private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/Common.php | 2 +- .../Database/OperationsController.php | 2 +- .../Database/PrivilegesController.php | 8 +++---- .../classes/Controllers/ImportController.php | 2 +- .../Server/DatabasesController.php | 4 ++-- .../Server/PrivilegesController.php | 10 ++++---- .../Server/ReplicationController.php | 4 ++-- .../Server/Status/MonitorController.php | 2 +- .../Server/UserGroupsController.php | 4 ++-- .../Server/VariablesController.php | 2 +- .../classes/Controllers/SqlController.php | 2 +- .../Table/PrivilegesController.php | 8 +++---- libraries/classes/Database/Routines.php | 4 ++-- libraries/classes/DatabaseInterface.php | 24 +++++++++++++------ libraries/classes/Dbal/DbalInterface.php | 18 +++++--------- libraries/classes/Footer.php | 2 +- libraries/classes/Header.php | 2 +- libraries/classes/Menu.php | 15 +++++------- libraries/classes/Server/Privileges.php | 22 ++++++++--------- 19 files changed, 69 insertions(+), 68 deletions(-) diff --git a/libraries/classes/Common.php b/libraries/classes/Common.php index 5df2ed9ffb..f34081f001 100644 --- a/libraries/classes/Common.php +++ b/libraries/classes/Common.php @@ -35,7 +35,7 @@ final class Common $err_url = Url::getFromRoute('/'); // now, select the mysql db - if (! $dbi->isSuperuser()) { + if (! $dbi->isSuperUser()) { return; } diff --git a/libraries/classes/Controllers/Database/OperationsController.php b/libraries/classes/Controllers/Database/OperationsController.php index 4304409635..672f8fee0d 100644 --- a/libraries/classes/Controllers/Database/OperationsController.php +++ b/libraries/classes/Controllers/Database/OperationsController.php @@ -303,7 +303,7 @@ class OperationsController extends AbstractController $hasAdjustPrivileges = $GLOBALS['db_priv'] && $GLOBALS['table_priv'] && $GLOBALS['col_priv'] && $GLOBALS['proc_priv'] && $GLOBALS['is_reload_priv']; - $isDropDatabaseAllowed = ($this->dbi->isSuperuser() || $cfg['AllowUserDropDatabase']) + $isDropDatabaseAllowed = ($this->dbi->isSuperUser() || $cfg['AllowUserDropDatabase']) && ! $db_is_system_schema && $db !== 'mysql'; $switchToNew = isset($_SESSION['pma_switch_to_new']) && $_SESSION['pma_switch_to_new']; diff --git a/libraries/classes/Controllers/Database/PrivilegesController.php b/libraries/classes/Controllers/Database/PrivilegesController.php index b58bc2648d..04b15a5157 100644 --- a/libraries/classes/Controllers/Database/PrivilegesController.php +++ b/libraries/classes/Controllers/Database/PrivilegesController.php @@ -49,18 +49,18 @@ class PrivilegesController extends AbstractController ); $privileges = []; - if ($this->dbi->isSuperuser()) { + if ($this->dbi->isSuperUser()) { $privileges = $this->privileges->getAllPrivileges($params['checkprivsdb']); } return $this->template->render('database/privileges/index', [ - 'is_superuser' => $this->dbi->isSuperuser(), + 'is_superuser' => $this->dbi->isSuperUser(), 'db' => $params['checkprivsdb'], 'database_url' => $scriptName, 'theme_image_path' => $PMA_Theme->getImgPath(), 'text_dir' => $text_dir, - 'is_createuser' => $this->dbi->isUserType('create'), - 'is_grantuser' => $this->dbi->isUserType('grant'), + 'is_createuser' => $this->dbi->isCreateUser(), + 'is_grantuser' => $this->dbi->isGrantUser(), 'privileges' => $privileges, ]); } diff --git a/libraries/classes/Controllers/ImportController.php b/libraries/classes/Controllers/ImportController.php index eb62cb09ae..3f0e401ba4 100644 --- a/libraries/classes/Controllers/ImportController.php +++ b/libraries/classes/Controllers/ImportController.php @@ -782,7 +782,7 @@ final class ImportController extends AbstractController if ($this->sql->hasNoRightsToDropDatabase( $analyzed_sql_results, $cfg['AllowUserDropDatabase'], - $this->dbi->isSuperuser() + $this->dbi->isSuperUser() )) { Generator::mysqlDie( __('"DROP DATABASE" statements are disabled.'), diff --git a/libraries/classes/Controllers/Server/DatabasesController.php b/libraries/classes/Controllers/Server/DatabasesController.php index 395e3cd068..694271f63c 100644 --- a/libraries/classes/Controllers/Server/DatabasesController.php +++ b/libraries/classes/Controllers/Server/DatabasesController.php @@ -176,7 +176,7 @@ class DatabasesController extends AbstractController 'max_db_list' => $cfg['MaxDbList'], 'has_master_replication' => $primaryInfo['status'], 'has_slave_replication' => $replicaInfo['status'], - 'is_drop_allowed' => $this->dbi->isSuperuser() || $cfg['AllowUserDropDatabase'], + 'is_drop_allowed' => $this->dbi->isSuperUser() || $cfg['AllowUserDropDatabase'], 'theme_image_path' => $PMA_Theme->getImgPath(), 'text_dir' => $text_dir, ]); @@ -277,7 +277,7 @@ class DatabasesController extends AbstractController if (! isset($params['drop_selected_dbs']) || ! $this->response->isAjax() - || (! $this->dbi->isSuperuser() && ! $cfg['AllowUserDropDatabase']) + || (! $this->dbi->isSuperUser() && ! $cfg['AllowUserDropDatabase']) ) { $message = Message::error(); $json = ['message' => $message]; diff --git a/libraries/classes/Controllers/Server/PrivilegesController.php b/libraries/classes/Controllers/Server/PrivilegesController.php index d4099465fd..25ab3d0052 100644 --- a/libraries/classes/Controllers/Server/PrivilegesController.php +++ b/libraries/classes/Controllers/Server/PrivilegesController.php @@ -91,7 +91,7 @@ class PrivilegesController extends AbstractController $this->response->addHTML('
'); $this->render('server/privileges/subnav', [ 'active' => 'privileges', - 'is_super_user' => $this->dbi->isSuperuser(), + 'is_super_user' => $this->dbi->isSuperUser(), ]); } @@ -125,10 +125,10 @@ class PrivilegesController extends AbstractController /** * Checks if the user is allowed to do what they try to... */ - $isGrantUser = $this->dbi->isUserType('grant'); - $isCreateUser = $this->dbi->isUserType('create'); + $isGrantUser = $this->dbi->isGrantUser(); + $isCreateUser = $this->dbi->isCreateUser(); - if (! $this->dbi->isSuperuser() && ! $isGrantUser && ! $isCreateUser) { + if (! $this->dbi->isSuperUser() && ! $isGrantUser && ! $isCreateUser) { $this->render('server/sub_page_header', [ 'type' => 'privileges', 'is_image' => false, @@ -246,7 +246,7 @@ class PrivilegesController extends AbstractController * Assign users to user groups */ if (! empty($_POST['changeUserGroup']) && $cfgRelation['menuswork'] - && $this->dbi->isSuperuser() && $this->dbi->isUserType('create') + && $this->dbi->isSuperUser() && $this->dbi->isCreateUser() ) { $serverPrivileges->setUserGroup($username, $_POST['userGroup']); $message = Message::success(); diff --git a/libraries/classes/Controllers/Server/ReplicationController.php b/libraries/classes/Controllers/Server/ReplicationController.php index 55ed1ed001..ed66cc99a5 100644 --- a/libraries/classes/Controllers/Server/ReplicationController.php +++ b/libraries/classes/Controllers/Server/ReplicationController.php @@ -63,7 +63,7 @@ class ReplicationController extends AbstractController $url_params = $params['url_params']; } - if ($this->dbi->isSuperuser()) { + if ($this->dbi->isSuperUser()) { $this->replicationGui->handleControlRequest(); } @@ -89,7 +89,7 @@ class ReplicationController extends AbstractController $this->render('server/replication/index', [ 'url_params' => $url_params, - 'is_super_user' => $this->dbi->isSuperuser(), + 'is_super_user' => $this->dbi->isSuperUser(), 'error_messages' => $errorMessages, 'is_master' => $primaryInfo['status'], 'master_configure' => $params['mr_configure'], diff --git a/libraries/classes/Controllers/Server/Status/MonitorController.php b/libraries/classes/Controllers/Server/Status/MonitorController.php index 332458155f..5f05eda098 100644 --- a/libraries/classes/Controllers/Server/Status/MonitorController.php +++ b/libraries/classes/Controllers/Server/Status/MonitorController.php @@ -60,7 +60,7 @@ class MonitorController extends AbstractController $form = [ 'server_time' => (int) (microtime(true) * 1000), 'server_os' => SysInfo::getOs(), - 'is_superuser' => $this->dbi->isSuperuser(), + 'is_superuser' => $this->dbi->isSuperUser(), 'server_db_isLocal' => $this->data->dbIsLocal, ]; diff --git a/libraries/classes/Controllers/Server/UserGroupsController.php b/libraries/classes/Controllers/Server/UserGroupsController.php index 8ed6da2b3a..7db704ec69 100644 --- a/libraries/classes/Controllers/Server/UserGroupsController.php +++ b/libraries/classes/Controllers/Server/UserGroupsController.php @@ -46,7 +46,7 @@ class UserGroupsController extends AbstractController /** * Only allowed to superuser */ - if (! $this->dbi->isSuperuser()) { + if (! $this->dbi->isSuperUser()) { $this->response->addHTML( Message::error(__('No Privileges'))->getDisplay() ); @@ -57,7 +57,7 @@ class UserGroupsController extends AbstractController $this->response->addHTML('
'); $this->render('server/privileges/subnav', [ 'active' => 'user-groups', - 'is_super_user' => $this->dbi->isSuperuser(), + 'is_super_user' => $this->dbi->isSuperUser(), ]); /** diff --git a/libraries/classes/Controllers/Server/VariablesController.php b/libraries/classes/Controllers/Server/VariablesController.php index 28ee06c675..98e1e1f7f2 100644 --- a/libraries/classes/Controllers/Server/VariablesController.php +++ b/libraries/classes/Controllers/Server/VariablesController.php @@ -99,7 +99,7 @@ class VariablesController extends AbstractController $this->render('server/variables/index', [ 'variables' => $variables, 'filter_value' => $filterValue, - 'is_superuser' => $this->dbi->isSuperuser(), + 'is_superuser' => $this->dbi->isSuperUser(), 'is_mariadb' => $this->dbi->isMariaDB(), ]); } diff --git a/libraries/classes/Controllers/SqlController.php b/libraries/classes/Controllers/SqlController.php index 34049b70b0..f6e906a57d 100644 --- a/libraries/classes/Controllers/SqlController.php +++ b/libraries/classes/Controllers/SqlController.php @@ -163,7 +163,7 @@ class SqlController extends AbstractController if ($this->sql->hasNoRightsToDropDatabase( $analyzed_sql_results, $cfg['AllowUserDropDatabase'], - $this->dbi->isSuperuser() + $this->dbi->isSuperUser() )) { Generator::mysqlDie( __('"DROP DATABASE" statements are disabled.'), diff --git a/libraries/classes/Controllers/Table/PrivilegesController.php b/libraries/classes/Controllers/Table/PrivilegesController.php index 44f0f041fd..9ec506fffc 100644 --- a/libraries/classes/Controllers/Table/PrivilegesController.php +++ b/libraries/classes/Controllers/Table/PrivilegesController.php @@ -50,7 +50,7 @@ class PrivilegesController extends AbstractController ); $privileges = []; - if ($this->dbi->isSuperuser()) { + if ($this->dbi->isSuperUser()) { $privileges = $this->privileges->getAllPrivileges( $params['checkprivsdb'], $params['checkprivstable'] @@ -60,12 +60,12 @@ class PrivilegesController extends AbstractController return $this->template->render('table/privileges/index', [ 'db' => $params['checkprivsdb'], 'table' => $params['checkprivstable'], - 'is_superuser' => $this->dbi->isSuperuser(), + 'is_superuser' => $this->dbi->isSuperUser(), 'table_url' => $scriptName, 'theme_image_path' => $PMA_Theme->getImgPath(), 'text_dir' => $text_dir, - 'is_createuser' => $this->dbi->isUserType('create'), - 'is_grantuser' => $this->dbi->isUserType('grant'), + 'is_createuser' => $this->dbi->isCreateUser(), + 'is_grantuser' => $this->dbi->isGrantUser(), 'privileges' => $privileges, ]); } diff --git a/libraries/classes/Database/Routines.php b/libraries/classes/Database/Routines.php index 96581fff6a..2dfeb5e7f0 100644 --- a/libraries/classes/Database/Routines.php +++ b/libraries/classes/Database/Routines.php @@ -1783,7 +1783,7 @@ class Routines // Since editing a procedure involved dropping and recreating, check also for // CREATE ROUTINE privilege to avoid lost procedures. $hasEditPrivilege = (Util::currentUserHasPrivilege('CREATE ROUTINE', $db) - && $currentUser == $routineDefiner) || $this->dbi->isSuperuser(); + && $currentUser == $routineDefiner) || $this->dbi->isSuperUser(); // There is a problem with Util::currentUserHasPrivilege(): // it does not detect all kinds of privileges, for example @@ -1829,7 +1829,7 @@ class Routines } $hasExportPrivilege = (Util::currentUserHasPrivilege('CREATE ROUTINE', $db) - && $currentUser == $routineDefiner) || $this->dbi->isSuperuser(); + && $currentUser == $routineDefiner) || $this->dbi->isSuperUser(); return $this->template->render('database/routines/row', [ 'db' => $db, diff --git a/libraries/classes/DatabaseInterface.php b/libraries/classes/DatabaseInterface.php index 83c483a6b6..e24e5548e7 100644 --- a/libraries/classes/DatabaseInterface.php +++ b/libraries/classes/DatabaseInterface.php @@ -1748,16 +1748,26 @@ class DatabaseInterface implements DbalInterface return '@'; } - /** - * Checks if current user is superuser - * - * @return bool Whether user is a superuser - */ - public function isSuperuser(): bool + public function isSuperUser(): bool { return $this->isUserType('super'); } + public function isGrantUser(): bool + { + return $this->isUserType('grant'); + } + + public function isCreateUser(): bool + { + return $this->isUserType('create'); + } + + public function isLoggedUser(): bool + { + return $this->isUserType('logged'); + } + /** * Checks if current user has global create user/grant privilege * or is a superuser (i.e. SELECT on mysql.users) @@ -1768,7 +1778,7 @@ class DatabaseInterface implements DbalInterface * * @return bool Whether user is a given type of user */ - public function isUserType(string $type): bool + private function isUserType(string $type): bool { if (Util::cacheExists('is_' . $type . 'user')) { return Util::cacheGet('is_' . $type . 'user'); diff --git a/libraries/classes/Dbal/DbalInterface.php b/libraries/classes/Dbal/DbalInterface.php index 793b1e7503..4ec650009c 100644 --- a/libraries/classes/Dbal/DbalInterface.php +++ b/libraries/classes/Dbal/DbalInterface.php @@ -493,19 +493,13 @@ interface DbalInterface * * @return bool Whether user is a superuser */ - public function isSuperuser(): bool; + public function isSuperUser(): bool; - /** - * Checks if current user has global create user/grant privilege - * or is a superuser (i.e. SELECT on mysql.users) - * while caching the result in session. - * - * @param string $type type of user to check for - * i.e. 'create', 'grant', 'super' - * - * @return bool Whether user is a given type of user - */ - public function isUserType(string $type): bool; + public function isGrantUser(): bool; + + public function isCreateUser(): bool; + + public function isLoggedUser(): bool; /** * Get the current user and host diff --git a/libraries/classes/Footer.php b/libraries/classes/Footer.php index acd5275860..72f3fece51 100644 --- a/libraries/classes/Footer.php +++ b/libraries/classes/Footer.php @@ -259,7 +259,7 @@ class Footer || ! empty($GLOBALS['error_message']) || empty($GLOBALS['sql_query']) || ! isset($dbi) - || ! $dbi->isUserType('logged') + || ! $dbi->isLoggedUser() ) { return; } diff --git a/libraries/classes/Header.php b/libraries/classes/Header.php index 0241e3e344..90c08ce9d4 100644 --- a/libraries/classes/Header.php +++ b/libraries/classes/Header.php @@ -248,7 +248,7 @@ class Header 'confirm' => $GLOBALS['cfg']['Confirm'], 'LoginCookieValidity' => $GLOBALS['cfg']['LoginCookieValidity'], 'session_gc_maxlifetime' => (int) ini_get('session.gc_maxlifetime'), - 'logged_in' => isset($dbi) ? $dbi->isUserType('logged') : false, + 'logged_in' => isset($dbi) ? $dbi->isLoggedUser() : false, 'is_https' => $GLOBALS['PMA_Config']->isHttps(), 'rootPath' => $GLOBALS['PMA_Config']->getRootPath(), 'arg_separator' => Url::getArgSeparator(), diff --git a/libraries/classes/Menu.php b/libraries/classes/Menu.php index 9daaefdea5..fc054dd02d 100644 --- a/libraries/classes/Menu.php +++ b/libraries/classes/Menu.php @@ -262,9 +262,8 @@ class Menu $updatable_view = $dbi->getTable($this->db, $this->table) ->isUpdatableView(); } - $is_superuser = $dbi->isSuperuser(); - $isCreateOrGrantUser = $dbi->isUserType('grant') - || $dbi->isUserType('create'); + $is_superuser = $dbi->isSuperUser(); + $isCreateOrGrantUser = $dbi->isGrantUser() || $dbi->isCreateUser(); $tabs = []; @@ -384,9 +383,8 @@ class Menu $db_is_system_schema = Utilities::isSystemSchema($this->db); $num_tables = count($dbi->getTables($this->db)); - $is_superuser = $dbi->isSuperuser(); - $isCreateOrGrantUser = $dbi->isUserType('grant') - || $dbi->isUserType('create'); + $is_superuser = $dbi->isSuperUser(); + $isCreateOrGrantUser = $dbi->isGrantUser() || $dbi->isCreateUser(); /** * Gets the relation settings @@ -506,9 +504,8 @@ class Menu /** @var DatabaseInterface $dbi */ global $route, $dbi; - $is_superuser = $dbi->isSuperuser(); - $isCreateOrGrantUser = $dbi->isUserType('grant') - || $dbi->isUserType('create'); + $is_superuser = $dbi->isSuperUser(); + $isCreateOrGrantUser = $dbi->isGrantUser() || $dbi->isCreateUser(); if (Util::cacheExists('binary_logs')) { $binary_logs = Util::cacheGet('binary_logs'); } else { diff --git a/libraries/classes/Server/Privileges.php b/libraries/classes/Server/Privileges.php index 5c017f7832..24fd4d716f 100644 --- a/libraries/classes/Server/Privileges.php +++ b/libraries/classes/Server/Privileges.php @@ -653,7 +653,7 @@ class Privileges $row = $this->dbi->fetchSingleRow($sql_query); } if (empty($row)) { - if ($table === '*' && $this->dbi->isSuperuser()) { + if ($table === '*' && $this->dbi->isSuperUser()) { $row = []; if ($db === '*') { $sql_query = 'SHOW COLUMNS FROM `mysql`.`user`;'; @@ -1068,7 +1068,7 @@ class Privileges . $this->dbi->escapeString($_POST['pma_pw']) . "')"; } elseif ($serverType === 'MariaDB' && $serverVersion >= 50200 - && $this->dbi->isSuperuser() + && $this->dbi->isSuperUser() ) { // Use 'UPDATE `mysql`.`user` ...' Syntax for MariaDB 5.2+ if ($authentication_plugin === 'mysql_native_password') { @@ -1308,7 +1308,7 @@ class Privileges */ public function getHtmlForAddUser($dbname) { - $isGrantUser = $this->dbi->isUserType('grant'); + $isGrantUser = $this->dbi->isGrantUser(); $loginInformationFieldsNew = $this->getHtmlForLoginInformationFields('new'); $privilegesTable = ''; if ($isGrantUser) { @@ -1726,7 +1726,7 @@ class Privileges ]; $extra_data['new_user_string'] = $this->template->render('server/privileges/new_user_ajax', [ 'user' => $user, - 'is_grantuser' => $this->dbi->isUserType('grant'), + 'is_grantuser' => $this->dbi->isGrantUser(), 'initial' => $_GET['initial'] ?? '', ]); @@ -2011,7 +2011,7 @@ class Privileges $onePrivilege['name'] = $name; $onePrivilege['edit_link'] = ''; - if ($this->dbi->isUserType('grant')) { + if ($this->dbi->isGrantUser()) { $onePrivilege['edit_link'] = $this->getUserLink( 'edit', $username, @@ -2182,8 +2182,8 @@ class Privileges 'text_dir' => $text_dir, 'initial' => $_GET['initial'] ?? '', 'hosts' => $hosts, - 'is_grantuser' => $this->dbi->isUserType('grant'), - 'is_createuser' => $this->dbi->isUserType('create'), + 'is_grantuser' => $this->dbi->isGrantUser(), + 'is_createuser' => $this->dbi->isCreateUser(), ]); } @@ -3011,7 +3011,7 @@ class Privileges */ public function getAddUserHtmlFieldset($db = '', $table = '') { - if (! $this->dbi->isUserType('create')) { + if (! $this->dbi->isCreateUser()) { return ''; } $rel_params = []; @@ -3201,7 +3201,7 @@ class Privileges 'empty_user_notice' => $emptyUserNotice ?? '', 'initials' => $initials ?? '', 'users_overview' => $usersOverview ?? '', - 'is_createuser' => $this->dbi->isUserType('create'), + 'is_createuser' => $this->dbi->isCreateUser(), 'flush_notice' => $flushNotice ?? '', ]); } @@ -3851,7 +3851,7 @@ class Privileges $real_sql_query .= ';'; $sql_query .= ';'; // No Global GRANT_OPTION privilege - if (! $this->dbi->isUserType('grant')) { + if (! $this->dbi->isGrantUser()) { $real_sql_query = ''; $sql_query = ''; } @@ -3951,7 +3951,7 @@ class Privileges $isNew = ($serverType === 'MySQL' && $serverVersion >= 50507) || ($serverType === 'MariaDB' && $serverVersion >= 50200); $hasMoreAuthPlugins = ($serverType === 'MySQL' && $serverVersion >= 50706) - || ($this->dbi->isSuperuser() && $editOthers); + || ($this->dbi->isSuperUser() && $editOthers); $activeAuthPlugins = ['mysql_native_password' => __('Native MySQL authentication')]; From 4873c322731ccf998796d2ab1c9567ad9a1912ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sun, 25 Oct 2020 20:33:17 -0300 Subject: [PATCH 3/3] Remove DatabaseInterface::isUserType method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/DatabaseInterface.php | 183 ++++++++++-------- libraries/classes/Dbal/DbalInterface.php | 2 +- libraries/classes/Footer.php | 2 +- libraries/classes/Header.php | 2 +- .../Database/PrivilegesControllerTest.php | 3 - .../Table/PrivilegesControllerTest.php | 3 - test/classes/Server/PrivilegesTest.php | 6 +- 7 files changed, 113 insertions(+), 88 deletions(-) diff --git a/libraries/classes/DatabaseInterface.php b/libraries/classes/DatabaseInterface.php index e24e5548e7..3d70bef65a 100644 --- a/libraries/classes/DatabaseInterface.php +++ b/libraries/classes/DatabaseInterface.php @@ -1750,104 +1750,133 @@ class DatabaseInterface implements DbalInterface public function isSuperUser(): bool { - return $this->isUserType('super'); + if (Util::cacheExists('is_superuser')) { + return Util::cacheGet('is_superuser'); + } + + if (! $this->isConnected()) { + return false; + } + + $result = $this->tryQuery( + 'SELECT 1 FROM mysql.user LIMIT 1', + self::CONNECT_USER, + self::QUERY_STORE + ); + $isSuperUser = false; + + if ($result) { + $isSuperUser = (bool) $this->numRows($result); + } + + $this->freeResult($result); + Util::cacheSet('is_superuser', $isSuperUser); + + return $isSuperUser; } public function isGrantUser(): bool { - return $this->isUserType('grant'); + global $cfg; + + if (Util::cacheExists('is_grantuser')) { + return Util::cacheGet('is_grantuser'); + } + + if (! $this->isConnected()) { + return false; + } + + $hasGrantPrivilege = false; + + if ($cfg['Server']['DisableIS']) { + $grants = $this->getCurrentUserGrants(); + + foreach ($grants as $grant) { + if (strpos($grant, 'WITH GRANT OPTION') !== false) { + $hasGrantPrivilege = true; + break; + } + } + + Util::cacheSet('is_grantuser', $hasGrantPrivilege); + + return $hasGrantPrivilege; + } + + [$user, $host] = $this->getCurrentUserAndHost(); + $query = QueryGenerator::getInformationSchemaDataForGranteeRequest($user, $host); + $result = $this->tryQuery($query, self::CONNECT_USER, self::QUERY_STORE); + + if ($result) { + $hasGrantPrivilege = (bool) $this->numRows($result); + } + + $this->freeResult($result); + Util::cacheSet('is_grantuser', $hasGrantPrivilege); + + return $hasGrantPrivilege; } public function isCreateUser(): bool { - return $this->isUserType('create'); - } + global $cfg; - public function isLoggedUser(): bool - { - return $this->isUserType('logged'); - } - - /** - * Checks if current user has global create user/grant privilege - * or is a superuser (i.e. SELECT on mysql.users) - * while caching the result in session. - * - * @param string $type type of user to check for - * i.e. 'create', 'grant', 'super' - * - * @return bool Whether user is a given type of user - */ - private function isUserType(string $type): bool - { - if (Util::cacheExists('is_' . $type . 'user')) { - return Util::cacheGet('is_' . $type . 'user'); + if (Util::cacheExists('is_createuser')) { + return Util::cacheGet('is_createuser'); } - // when connection failed we don't have a $userlink - if (! isset($this->links[self::CONNECT_USER])) { + if (! $this->isConnected()) { return false; } - // checking if user is logged in - if ($type === 'logged') { - return true; - } + $hasCreatePrivilege = false; - if (! $GLOBALS['cfg']['Server']['DisableIS'] || $type === 'super') { - // Prepare query for each user type check - $query = ''; - if ($type === 'super') { - $query = 'SELECT 1 FROM mysql.user LIMIT 1'; - } elseif ($type === 'create') { - [$user, $host] = $this->getCurrentUserAndHost(); - $query = QueryGenerator::getInformationSchemaDataForCreateRequest($user, $host); - } elseif ($type === 'grant') { - [$user, $host] = $this->getCurrentUserAndHost(); - $query = QueryGenerator::getInformationSchemaDataForGranteeRequest($user, $host); - } + if ($cfg['Server']['DisableIS']) { + $grants = $this->getCurrentUserGrants(); - $is = false; - $result = $this->tryQuery( - $query, - self::CONNECT_USER, - self::QUERY_STORE - ); - if ($result) { - $is = (bool) $this->numRows($result); - } - $this->freeResult($result); - } else { - $is = false; - $grants = $this->fetchResult( - 'SHOW GRANTS FOR CURRENT_USER();', - null, - null, - self::CONNECT_USER, - self::QUERY_STORE - ); - if ($grants) { - foreach ($grants as $grant) { - if ($type === 'create') { - if (strpos($grant, 'ALL PRIVILEGES ON *.*') !== false - || strpos($grant, 'CREATE USER') !== false - ) { - $is = true; - break; - } - } elseif ($type === 'grant') { - if (strpos($grant, 'WITH GRANT OPTION') !== false) { - $is = true; - break; - } - } + foreach ($grants as $grant) { + if (strpos($grant, 'ALL PRIVILEGES ON *.*') !== false + || strpos($grant, 'CREATE USER') !== false + ) { + $hasCreatePrivilege = true; + break; } } + + Util::cacheSet('is_createuser', $hasCreatePrivilege); + + return $hasCreatePrivilege; } - Util::cacheSet('is_' . $type . 'user', $is); + [$user, $host] = $this->getCurrentUserAndHost(); + $query = QueryGenerator::getInformationSchemaDataForCreateRequest($user, $host); + $result = $this->tryQuery($query, self::CONNECT_USER, self::QUERY_STORE); - return $is; + if ($result) { + $hasCreatePrivilege = (bool) $this->numRows($result); + } + + $this->freeResult($result); + Util::cacheSet('is_createuser', $hasCreatePrivilege); + + return $hasCreatePrivilege; + } + + public function isConnected(): bool + { + return isset($this->links[self::CONNECT_USER]); + } + + private function getCurrentUserGrants(): array + { + return $this->fetchResult( + 'SHOW GRANTS FOR CURRENT_USER();', + null, + null, + self::CONNECT_USER, + self::QUERY_STORE + ); } /** diff --git a/libraries/classes/Dbal/DbalInterface.php b/libraries/classes/Dbal/DbalInterface.php index 4ec650009c..c0da4c1f8e 100644 --- a/libraries/classes/Dbal/DbalInterface.php +++ b/libraries/classes/Dbal/DbalInterface.php @@ -499,7 +499,7 @@ interface DbalInterface public function isCreateUser(): bool; - public function isLoggedUser(): bool; + public function isConnected(): bool; /** * Get the current user and host diff --git a/libraries/classes/Footer.php b/libraries/classes/Footer.php index 72f3fece51..23e758a379 100644 --- a/libraries/classes/Footer.php +++ b/libraries/classes/Footer.php @@ -259,7 +259,7 @@ class Footer || ! empty($GLOBALS['error_message']) || empty($GLOBALS['sql_query']) || ! isset($dbi) - || ! $dbi->isLoggedUser() + || ! $dbi->isConnected() ) { return; } diff --git a/libraries/classes/Header.php b/libraries/classes/Header.php index 90c08ce9d4..08804a2cf3 100644 --- a/libraries/classes/Header.php +++ b/libraries/classes/Header.php @@ -248,7 +248,7 @@ class Header 'confirm' => $GLOBALS['cfg']['Confirm'], 'LoginCookieValidity' => $GLOBALS['cfg']['LoginCookieValidity'], 'session_gc_maxlifetime' => (int) ini_get('session.gc_maxlifetime'), - 'logged_in' => isset($dbi) ? $dbi->isLoggedUser() : false, + 'logged_in' => isset($dbi) ? $dbi->isConnected() : false, 'is_https' => $GLOBALS['PMA_Config']->isHttps(), 'rootPath' => $GLOBALS['PMA_Config']->getRootPath(), 'arg_separator' => Url::getArgSeparator(), diff --git a/test/classes/Controllers/Database/PrivilegesControllerTest.php b/test/classes/Controllers/Database/PrivilegesControllerTest.php index c09945bced..c0ee99e9ed 100644 --- a/test/classes/Controllers/Database/PrivilegesControllerTest.php +++ b/test/classes/Controllers/Database/PrivilegesControllerTest.php @@ -10,7 +10,6 @@ use PhpMyAdmin\Server\Privileges; use PhpMyAdmin\Template; use PhpMyAdmin\Tests\AbstractTestCase; use PhpMyAdmin\Url; -use PhpMyAdmin\Util; class PrivilegesControllerTest extends AbstractTestCase { @@ -34,8 +33,6 @@ class PrivilegesControllerTest extends AbstractTestCase $server = 0; $cfg['Server']['DisableIS'] = false; $PMA_PHP_SELF = 'index.php'; - Util::cacheSet('is_grantuser', true); - Util::cacheSet('is_createuser', true); $privileges = []; diff --git a/test/classes/Controllers/Table/PrivilegesControllerTest.php b/test/classes/Controllers/Table/PrivilegesControllerTest.php index 30a34a97f5..326487019b 100644 --- a/test/classes/Controllers/Table/PrivilegesControllerTest.php +++ b/test/classes/Controllers/Table/PrivilegesControllerTest.php @@ -10,7 +10,6 @@ use PhpMyAdmin\Server\Privileges; use PhpMyAdmin\Template; use PhpMyAdmin\Tests\AbstractTestCase; use PhpMyAdmin\Url; -use PhpMyAdmin\Util; class PrivilegesControllerTest extends AbstractTestCase { @@ -35,8 +34,6 @@ class PrivilegesControllerTest extends AbstractTestCase $server = 0; $cfg['Server']['DisableIS'] = false; $PMA_PHP_SELF = 'index.php'; - Util::cacheSet('is_grantuser', true); - Util::cacheSet('is_createuser', true); $privileges = []; diff --git a/test/classes/Server/PrivilegesTest.php b/test/classes/Server/PrivilegesTest.php index 0dbcf92e55..48aa11c04f 100644 --- a/test/classes/Server/PrivilegesTest.php +++ b/test/classes/Server/PrivilegesTest.php @@ -112,7 +112,9 @@ class PrivilegesTest extends AbstractTestCase $dbi->expects($this->any())->method('escapeString') ->will($this->returnArgument(0)); - $dbi->expects($this->any())->method('isUserType') + $dbi->expects($this->any())->method('isCreateUser') + ->will($this->returnValue(true)); + $dbi->expects($this->any())->method('isGrantUser') ->will($this->returnValue(true)); $GLOBALS['dbi'] = $dbi; @@ -1292,7 +1294,7 @@ class PrivilegesTest extends AbstractTestCase $dbi->expects($this->any()) ->method('escapeString') ->will($this->returnArgument(0)); - $dbi->expects($this->any())->method('isUserType') + $dbi->expects($this->any())->method('isGrantUser') ->will($this->returnValue(true)); $GLOBALS['dbi'] = $dbi;