From 114f128fc302cb65a85937e197d2ff2215e8164c Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Wed, 29 Jul 2015 18:09:23 +0200 Subject: Moving mysql setup code over to Doctrine --- lib/private/setup.php | 2 +- lib/private/setup/abstractdatabase.php | 37 +++++++++------ lib/private/setup/mysql.php | 87 ++++++++++++++++++---------------- lib/private/setup/oci.php | 38 +++++++-------- lib/private/setup/postgresql.php | 40 ++++++++-------- 5 files changed, 109 insertions(+), 95 deletions(-) diff --git a/lib/private/setup.php b/lib/private/setup.php index 870480feaa0..c862429fd2a 100644 --- a/lib/private/setup.php +++ b/lib/private/setup.php @@ -249,7 +249,7 @@ class Setup { $class = self::$dbSetupClasses[$dbType]; /** @var \OC\Setup\AbstractDatabase $dbSetup */ - $dbSetup = new $class($l, 'db_structure.xml'); + $dbSetup = new $class($l, 'db_structure.xml', $this->config); $error = array_merge($error, $dbSetup->validate($options)); // validate the data directory diff --git a/lib/private/setup/abstractdatabase.php b/lib/private/setup/abstractdatabase.php index 13daf1782fc..928af0568b5 100644 --- a/lib/private/setup/abstractdatabase.php +++ b/lib/private/setup/abstractdatabase.php @@ -22,22 +22,31 @@ */ namespace OC\Setup; +use OCP\IConfig; + abstract class AbstractDatabase { - /** - * @var \OC_L10N - */ + /** @var \OC_L10N */ protected $trans; + /** @var string */ protected $dbDefinitionFile; - protected $dbuser; - protected $dbpassword; - protected $dbname; - protected $dbhost; - protected $tableprefix; + /** @var string */ + protected $dbUser; + /** @var string */ + protected $dbPassword; + /** @var string */ + protected $dbName; + /** @var string */ + protected $dbHost; + /** @var string */ + protected $tablePrefix; + /** @var IConfig */ + protected $config; - public function __construct($trans, $dbDefinitionFile) { + public function __construct($trans, $dbDefinitionFile, IConfig $config) { $this->trans = $trans; $this->dbDefinitionFile = $dbDefinitionFile; + $this->config = $config; } public function validate($config) { @@ -67,11 +76,11 @@ abstract class AbstractDatabase { 'dbtableprefix' => $dbTablePrefix, ]); - $this->dbuser = $dbUser; - $this->dbpassword = $dbPass; - $this->dbname = $dbName; - $this->dbhost = $dbHost; - $this->tableprefix = $dbTablePrefix; + $this->dbUser = $dbUser; + $this->dbPassword = $dbPass; + $this->dbName = $dbName; + $this->dbHost = $dbHost; + $this->tablePrefix = $dbTablePrefix; } abstract public function setupDatabase($userName); diff --git a/lib/private/setup/mysql.php b/lib/private/setup/mysql.php index 9cf102393b8..906b9f1b6c5 100644 --- a/lib/private/setup/mysql.php +++ b/lib/private/setup/mysql.php @@ -23,21 +23,19 @@ */ namespace OC\Setup; +use OC\DB\ConnectionFactory; + class MySQL extends AbstractDatabase { public $dbprettyname = 'MySQL/MariaDB'; public function setupDatabase($username) { //check if the database user has admin right - $connection = @mysql_connect($this->dbhost, $this->dbuser, $this->dbpassword); - if(!$connection) { - throw new \OC\DatabaseSetupException($this->trans->t('MySQL/MariaDB username and/or password not valid'), - $this->trans->t('You need to enter either an existing account or the administrator.')); - } + $connection = $this->connect(); //user already specified in config $oldUser=\OC_Config::getValue('dbuser', false); //we don't have a dbuser specified in config - if($this->dbuser!=$oldUser) { + if($this->dbUser!=$oldUser) { //add prefix to the admin username to prevent collisions $adminUser=substr('oc_'.$username, 0, 16); @@ -45,18 +43,18 @@ class MySQL extends AbstractDatabase { while(true) { //this should be enough to check for admin rights in mysql $query="SELECT user FROM mysql.user WHERE user='$adminUser'"; - - $result = mysql_query($query, $connection); + $result = $connection->executeQuery($query); //current dbuser has admin rights if($result) { + $data = $result->fetchAll(); //new dbuser does not exist - if(mysql_num_rows($result) === 0) { + if(count($data) === 0) { //use the admin login data for the new database user - $this->dbuser=$adminUser; + $this->dbUser=$adminUser; //create a random password so we don't need to store the admin password in the config file - $this->dbpassword=\OC_Util::generateRandomBytes(30); + $this->dbPassword=\OC_Util::generateRandomBytes(30); $this->createDBUser($connection); @@ -73,8 +71,8 @@ class MySQL extends AbstractDatabase { }; \OC_Config::setValues([ - 'dbuser' => $this->dbuser, - 'dbpassword' => $this->dbpassword, + 'dbuser' => $this->dbUser, + 'dbpassword' => $this->dbPassword, ]); } @@ -83,50 +81,57 @@ class MySQL extends AbstractDatabase { //fill the database if needed $query='select count(*) from information_schema.tables' - ." where table_schema='".$this->dbname."' AND table_name = '".$this->tableprefix."users';"; - $result = mysql_query($query, $connection); - if($result) { - $row=mysql_fetch_row($result); - } + ." where table_schema='".$this->dbName."' AND table_name = '".$this->tablePrefix."users';"; + $result = $connection->executeQuery($query); + $row = $result->fetch(); if(!$result or $row[0]==0) { \OC_DB::createDbFromStructure($this->dbDefinitionFile); } - mysql_close($connection); } + /** + * @param \OC\DB\Connection $connection + */ private function createDatabase($connection) { - $name = $this->dbname; - $user = $this->dbuser; + $name = $this->dbName; + $user = $this->dbUser; //we cant use OC_BD functions here because we need to connect as the administrative user. $query = "CREATE DATABASE IF NOT EXISTS `$name` CHARACTER SET utf8 COLLATE utf8_bin;"; - $result = mysql_query($query, $connection); - if(!$result) { - $entry = $this->trans->t('DB Error: "%s"', array(mysql_error($connection))) . '
'; - $entry .= $this->trans->t('Offending command was: "%s"', array($query)) . '
'; - \OCP\Util::writeLog('setup.mysql', $entry, \OCP\Util::WARN); - } - $query="GRANT ALL PRIVILEGES ON `$name` . * TO '$user'"; + $connection->executeUpdate($query); //this query will fail if there aren't the right permissions, ignore the error - mysql_query($query, $connection); + $query="GRANT ALL PRIVILEGES ON `$name` . * TO '$user'"; + $connection->executeUpdate($query); } + /** + * @param \OC\DB\Connection $connection + * @throws \OC\DatabaseSetupException + */ private function createDBUser($connection) { - $name = $this->dbuser; - $password = $this->dbpassword; + $name = $this->dbUser; + $password = $this->dbPassword; // we need to create 2 accounts, one for global use and one for local user. if we don't specify the local one, // the anonymous user would take precedence when there is one. $query = "CREATE USER '$name'@'localhost' IDENTIFIED BY '$password'"; - $result = mysql_query($query, $connection); - if (!$result) { - throw new \OC\DatabaseSetupException($this->trans->t("MySQL/MariaDB user '%s'@'localhost' exists already.", array($name)), - $this->trans->t("Drop this user from MySQL/MariaDB", array($name))); - } + $connection->executeUpdate($query); $query = "CREATE USER '$name'@'%' IDENTIFIED BY '$password'"; - $result = mysql_query($query, $connection); - if (!$result) { - throw new \OC\DatabaseSetupException($this->trans->t("MySQL/MariaDB user '%s'@'%%' already exists", array($name)), - $this->trans->t("Drop this user from MySQL/MariaDB.")); - } + $connection->executeUpdate($query); + } + + /** + * @return \OC\DB\Connection + * @throws \OC\DatabaseSetupException + */ + private function connect() { + $type = 'mysql'; + $connectionParams = array( + 'host' => $this->dbHost, + 'user' => $this->dbUser, + 'password' => $this->dbPassword, + 'tablePrefix' => $this->tablePrefix, + ); + $cf = new ConnectionFactory(); + return $cf->getConnection($type, $connectionParams); } } diff --git a/lib/private/setup/oci.php b/lib/private/setup/oci.php index d46d5529da0..1e1eb1ff54e 100644 --- a/lib/private/setup/oci.php +++ b/lib/private/setup/oci.php @@ -38,10 +38,10 @@ class OCI extends AbstractDatabase { $this->dbtablespace = 'USERS'; } // allow empty hostname for oracle - $this->dbhost = $config['dbhost']; + $this->dbHost = $config['dbhost']; \OC_Config::setValues([ - 'dbhost' => $this->dbhost, + 'dbhost' => $this->dbHost, 'dbtablespace' => $this->dbtablespace, ]); } @@ -58,8 +58,8 @@ class OCI extends AbstractDatabase { } public function setupDatabase($username) { - $e_host = addslashes($this->dbhost); - $e_dbname = addslashes($this->dbname); + $e_host = addslashes($this->dbHost); + $e_dbname = addslashes($this->dbName); //check if the database user has admin right if ($e_host == '') { $easy_connect_string = $e_dbname; // use dbname as easy connect name @@ -67,7 +67,7 @@ class OCI extends AbstractDatabase { $easy_connect_string = '//'.$e_host.'/'.$e_dbname; } \OCP\Util::writeLog('setup oracle', 'connect string: ' . $easy_connect_string, \OCP\Util::DEBUG); - $connection = @oci_connect($this->dbuser, $this->dbpassword, $easy_connect_string); + $connection = @oci_connect($this->dbUser, $this->dbPassword, $easy_connect_string); if(!$connection) { $errorMessage = $this->getLastError(); if ($errorMessage) { @@ -103,23 +103,23 @@ class OCI extends AbstractDatabase { //use the admin login data for the new database user //add prefix to the oracle user name to prevent collisions - $this->dbuser='oc_'.$username; + $this->dbUser='oc_'.$username; //create a new password so we don't need to store the admin config in the config file - $this->dbpassword=\OC_Util::generateRandomBytes(30); + $this->dbPassword=\OC_Util::generateRandomBytes(30); //oracle passwords are treated as identifiers: // must start with alphanumeric char // needs to be shortened to 30 bytes, as the two " needed to escape the identifier count towards the identifier length. - $this->dbpassword=substr($this->dbpassword, 0, 30); + $this->dbPassword=substr($this->dbPassword, 0, 30); $this->createDBUser($connection); } } \OC_Config::setValues([ - 'dbuser' => $this->dbuser, - 'dbname' => $this->dbname, - 'dbpassword' => $this->dbpassword, + 'dbuser' => $this->dbUser, + 'dbname' => $this->dbName, + 'dbpassword' => $this->dbPassword, ]); //create the database not necessary, oracle implies user = schema @@ -131,26 +131,26 @@ class OCI extends AbstractDatabase { oci_close($connection); // connect to the oracle database (schema=$this->dbuser) an check if the schema needs to be filled - $this->dbuser = \OC_Config::getValue('dbuser'); + $this->dbUser = \OC_Config::getValue('dbuser'); //$this->dbname = \OC_Config::getValue('dbname'); - $this->dbpassword = \OC_Config::getValue('dbpassword'); + $this->dbPassword = \OC_Config::getValue('dbpassword'); - $e_host = addslashes($this->dbhost); - $e_dbname = addslashes($this->dbname); + $e_host = addslashes($this->dbHost); + $e_dbname = addslashes($this->dbName); if ($e_host == '') { $easy_connect_string = $e_dbname; // use dbname as easy connect name } else { $easy_connect_string = '//'.$e_host.'/'.$e_dbname; } - $connection = @oci_connect($this->dbuser, $this->dbpassword, $easy_connect_string); + $connection = @oci_connect($this->dbUser, $this->dbPassword, $easy_connect_string); if(!$connection) { throw new \OC\DatabaseSetupException($this->trans->t('Oracle username and/or password not valid'), $this->trans->t('You need to enter either an existing account or the administrator.')); } $query = "SELECT count(*) FROM user_tables WHERE table_name = :un"; $stmt = oci_parse($connection, $query); - $un = $this->tableprefix.'users'; + $un = $this->tablePrefix.'users'; oci_bind_by_name($stmt, ':un', $un); if (!$stmt) { $entry = $this->trans->t('DB Error: "%s"', array($this->getLastError($connection))) . '
'; @@ -171,8 +171,8 @@ class OCI extends AbstractDatabase { * @param resource $connection */ private function createDBUser($connection) { - $name = $this->dbuser; - $password = $this->dbpassword; + $name = $this->dbUser; + $password = $this->dbPassword; $query = "SELECT * FROM all_users WHERE USERNAME = :un"; $stmt = oci_parse($connection, $query); if (!$stmt) { diff --git a/lib/private/setup/postgresql.php b/lib/private/setup/postgresql.php index c8fd3b98fe4..319b6676ef8 100644 --- a/lib/private/setup/postgresql.php +++ b/lib/private/setup/postgresql.php @@ -27,9 +27,9 @@ class PostgreSQL extends AbstractDatabase { public $dbprettyname = 'PostgreSQL'; public function setupDatabase($username) { - $e_host = addslashes($this->dbhost); - $e_user = addslashes($this->dbuser); - $e_password = addslashes($this->dbpassword); + $e_host = addslashes($this->dbHost); + $e_user = addslashes($this->dbUser); + $e_password = addslashes($this->dbPassword); // Fix database with port connection if(strpos($e_host, ':')) { @@ -43,7 +43,7 @@ class PostgreSQL extends AbstractDatabase { $connection = @pg_connect($connection_string); if(!$connection) { // Try if we can connect to the DB with the specified name - $e_dbname = addslashes($this->dbname); + $e_dbname = addslashes($this->dbName); $connection_string = "host='$e_host' dbname='$e_dbname' user='$e_user' port='$port' password='$e_password'"; $connection = @pg_connect($connection_string); @@ -51,7 +51,7 @@ class PostgreSQL extends AbstractDatabase { throw new \OC\DatabaseSetupException($this->trans->t('PostgreSQL username and/or password not valid'), $this->trans->t('You need to enter either an existing account or the administrator.')); } - $e_user = pg_escape_string($this->dbuser); + $e_user = pg_escape_string($this->dbUser); //check for roles creation rights in postgresql $query="SELECT 1 FROM pg_roles WHERE rolcreaterole=TRUE AND rolname='$e_user'"; $result = pg_query($connection, $query); @@ -59,16 +59,16 @@ class PostgreSQL extends AbstractDatabase { //use the admin login data for the new database user //add prefix to the postgresql user name to prevent collisions - $this->dbuser='oc_'.$username; + $this->dbUser='oc_'.$username; //create a new password so we don't need to store the admin config in the config file - $this->dbpassword=\OC_Util::generateRandomBytes(30); + $this->dbPassword=\OC_Util::generateRandomBytes(30); $this->createDBUser($connection); } \OC_Config::setValues([ - 'dbuser' => $this->dbuser, - 'dbpassword' => $this->dbpassword, + 'dbuser' => $this->dbUser, + 'dbpassword' => $this->dbPassword, ]); //create the database @@ -78,13 +78,13 @@ class PostgreSQL extends AbstractDatabase { pg_close($connection); // connect to the ownCloud database (dbname=$this->dbname) and check if it needs to be filled - $this->dbuser = \OC_Config::getValue('dbuser'); - $this->dbpassword = \OC_Config::getValue('dbpassword'); + $this->dbUser = \OC_Config::getValue('dbuser'); + $this->dbPassword = \OC_Config::getValue('dbpassword'); - $e_host = addslashes($this->dbhost); - $e_dbname = addslashes($this->dbname); - $e_user = addslashes($this->dbuser); - $e_password = addslashes($this->dbpassword); + $e_host = addslashes($this->dbHost); + $e_dbname = addslashes($this->dbName); + $e_user = addslashes($this->dbUser); + $e_password = addslashes($this->dbPassword); // Fix database with port connection if(strpos($e_host, ':')) { @@ -99,7 +99,7 @@ class PostgreSQL extends AbstractDatabase { throw new \OC\DatabaseSetupException($this->trans->t('PostgreSQL username and/or password not valid'), $this->trans->t('You need to enter either an existing account or the administrator.')); } - $query = "select count(*) FROM pg_class WHERE relname='".$this->tableprefix."users' limit 1"; + $query = "select count(*) FROM pg_class WHERE relname='".$this->tablePrefix."users' limit 1"; $result = pg_query($connection, $query); if($result) { $row = pg_fetch_row($result); @@ -111,8 +111,8 @@ class PostgreSQL extends AbstractDatabase { private function createDatabase($connection) { //we cant use OC_BD functions here because we need to connect as the administrative user. - $e_name = pg_escape_string($this->dbname); - $e_user = pg_escape_string($this->dbuser); + $e_name = pg_escape_string($this->dbName); + $e_user = pg_escape_string($this->dbUser); $query = "select datname from pg_database where datname = '$e_name'"; $result = pg_query($connection, $query); if(!$result) { @@ -137,8 +137,8 @@ class PostgreSQL extends AbstractDatabase { } private function createDBUser($connection) { - $e_name = pg_escape_string($this->dbuser); - $e_password = pg_escape_string($this->dbpassword); + $e_name = pg_escape_string($this->dbUser); + $e_password = pg_escape_string($this->dbPassword); $query = "select * from pg_roles where rolname='$e_name';"; $result = pg_query($connection, $query); if(!$result) { -- cgit v1.2.3 From c3cac887f57278a21052391c99b37a6dfb8cef9f Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Thu, 30 Jul 2015 00:04:30 +0200 Subject: - more injection - less static calls - use params on sql queries - handle sql exception on database and user creation gracefully --- core/command/maintenance/install.php | 5 +- lib/base.php | 4 +- lib/private/setup.php | 26 +++++-- lib/private/setup/abstractdatabase.php | 12 ++- lib/private/setup/mysql.php | 136 +++++++++++++++++++-------------- lib/private/util.php | 3 +- 6 files changed, 119 insertions(+), 67 deletions(-) diff --git a/core/command/maintenance/install.php b/core/command/maintenance/install.php index 2fea5add438..7f5d9cae647 100644 --- a/core/command/maintenance/install.php +++ b/core/command/maintenance/install.php @@ -61,7 +61,10 @@ class Install extends Command { protected function execute(InputInterface $input, OutputInterface $output) { // validate the environment - $setupHelper = new Setup($this->config, \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), new \OC_Defaults()); + $server = \OC::$server; + $setupHelper = new Setup($this->config, $server->getIniWrapper(), + $server->getL10N('lib'), new \OC_Defaults(), $server->getLogger(), + $server->getSecureRandom()); $sysInfo = $setupHelper->getSystemInfo(true); $errors = $sysInfo['errors']; if (count($errors) > 0) { diff --git a/lib/base.php b/lib/base.php index fde67839560..299970e269c 100644 --- a/lib/base.php +++ b/lib/base.php @@ -835,7 +835,9 @@ class OC { // Check if ownCloud is installed or in maintenance (update) mode if (!$systemConfig->getValue('installed', false)) { \OC::$server->getSession()->clear(); - $setupHelper = new OC\Setup(\OC::$server->getConfig(), \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), new \OC_Defaults()); + $setupHelper = new OC\Setup(\OC::$server->getConfig(), \OC::$server->getIniWrapper(), + \OC::$server->getL10N('lib'), new \OC_Defaults(), \OC::$server->getLogger(), + \OC::$server->getSecureRandom()); $controller = new OC\Core\Setup\Controller($setupHelper); $controller->run($_POST); exit(); diff --git a/lib/private/setup.php b/lib/private/setup.php index c862429fd2a..afc88256da4 100644 --- a/lib/private/setup.php +++ b/lib/private/setup.php @@ -39,6 +39,8 @@ use bantu\IniGetWrapper\IniGetWrapper; use Exception; use OCP\IConfig; use OCP\IL10N; +use OCP\ILogger; +use OCP\Security\ISecureRandom; class Setup { /** @var \OCP\IConfig */ @@ -49,6 +51,10 @@ class Setup { protected $l10n; /** @var \OC_Defaults */ protected $defaults; + /** @var ILogger */ + protected $logger; + /** @var ISecureRandom */ + protected $random; /** * @param IConfig $config @@ -58,11 +64,16 @@ class Setup { function __construct(IConfig $config, IniGetWrapper $iniWrapper, IL10N $l10n, - \OC_Defaults $defaults) { + \OC_Defaults $defaults, + ILogger $logger, + ISecureRandom $random + ) { $this->config = $config; $this->iniWrapper = $iniWrapper; $this->l10n = $l10n; $this->defaults = $defaults; + $this->logger = $logger; + $this->random = $random; } static $dbSetupClasses = array( @@ -249,7 +260,8 @@ class Setup { $class = self::$dbSetupClasses[$dbType]; /** @var \OC\Setup\AbstractDatabase $dbSetup */ - $dbSetup = new $class($l, 'db_structure.xml', $this->config); + $dbSetup = new $class($l, 'db_structure.xml', $this->config, + $this->logger, $this->random); $error = array_merge($error, $dbSetup->validate($options)); // validate the data directory @@ -284,9 +296,9 @@ class Setup { } //generate a random salt that is used to salt the local user passwords - $salt = \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(30); + $salt = $this->random->getLowStrengthGenerator()->generate(30); // generate a secret - $secret = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(48); + $secret = $this->random->getMediumStrengthGenerator()->generate(48); //write the config file $this->config->setSystemValues([ @@ -351,7 +363,7 @@ class Setup { //try to write logtimezone if (date_default_timezone_get()) { - \OC_Config::setValue('logtimezone', date_default_timezone_get()); + $config->setSystemValue('logtimezone', date_default_timezone_get()); } //and we are done @@ -389,7 +401,9 @@ class Setup { * @throws \OC\HintException If .htaccess does not include the current version */ public static function updateHtaccess() { - $setupHelper = new \OC\Setup(\OC::$server->getConfig(), \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), new \OC_Defaults()); + $setupHelper = new \OC\Setup(\OC::$server->getConfig(), \OC::$server->getIniWrapper(), + \OC::$server->getL10N('lib'), new \OC_Defaults(), \OC::$server->getLogger(), + \OC::$server->getSecureRandom()); if(!$setupHelper->isCurrentHtaccess()) { throw new \OC\HintException('.htaccess file has the wrong version. Please upload the correct version. Maybe you forgot to replace it after updating?'); } diff --git a/lib/private/setup/abstractdatabase.php b/lib/private/setup/abstractdatabase.php index 928af0568b5..1ec853c3b02 100644 --- a/lib/private/setup/abstractdatabase.php +++ b/lib/private/setup/abstractdatabase.php @@ -23,6 +23,8 @@ namespace OC\Setup; use OCP\IConfig; +use OCP\ILogger; +use OCP\Security\ISecureRandom; abstract class AbstractDatabase { @@ -42,11 +44,17 @@ abstract class AbstractDatabase { protected $tablePrefix; /** @var IConfig */ protected $config; + /** @var ILogger */ + protected $logger; + /** @var ISecureRandom */ + protected $random; - public function __construct($trans, $dbDefinitionFile, IConfig $config) { + public function __construct($trans, $dbDefinitionFile, IConfig $config, ILogger $logger, ISecureRandom $random) { $this->trans = $trans; $this->dbDefinitionFile = $dbDefinitionFile; $this->config = $config; + $this->logger = $logger; + $this->random = $random; } public function validate($config) { @@ -70,7 +78,7 @@ abstract class AbstractDatabase { $dbHost = !empty($config['dbhost']) ? $config['dbhost'] : 'localhost'; $dbTablePrefix = isset($config['dbtableprefix']) ? $config['dbtableprefix'] : 'oc_'; - \OC_Config::setValues([ + $this->config->setSystemValues([ 'dbname' => $dbName, 'dbhost' => $dbHost, 'dbtableprefix' => $dbTablePrefix, diff --git a/lib/private/setup/mysql.php b/lib/private/setup/mysql.php index 906b9f1b6c5..5597592f21e 100644 --- a/lib/private/setup/mysql.php +++ b/lib/private/setup/mysql.php @@ -24,6 +24,7 @@ namespace OC\Setup; use OC\DB\ConnectionFactory; +use OCP\IDBConnection; class MySQL extends AbstractDatabase { public $dbprettyname = 'MySQL/MariaDB'; @@ -31,58 +32,15 @@ class MySQL extends AbstractDatabase { public function setupDatabase($username) { //check if the database user has admin right $connection = $this->connect(); - //user already specified in config - $oldUser=\OC_Config::getValue('dbuser', false); - //we don't have a dbuser specified in config - if($this->dbUser!=$oldUser) { - //add prefix to the admin username to prevent collisions - $adminUser=substr('oc_'.$username, 0, 16); - - $i = 1; - while(true) { - //this should be enough to check for admin rights in mysql - $query="SELECT user FROM mysql.user WHERE user='$adminUser'"; - $result = $connection->executeQuery($query); - - //current dbuser has admin rights - if($result) { - $data = $result->fetchAll(); - //new dbuser does not exist - if(count($data) === 0) { - //use the admin login data for the new database user - $this->dbUser=$adminUser; - - //create a random password so we don't need to store the admin password in the config file - $this->dbPassword=\OC_Util::generateRandomBytes(30); - - $this->createDBUser($connection); - - break; - } else { - //repeat with different username - $length=strlen((string)$i); - $adminUser=substr('oc_'.$username, 0, 16 - $length).$i; - $i++; - } - } else { - break; - } - }; - - \OC_Config::setValues([ - 'dbuser' => $this->dbUser, - 'dbpassword' => $this->dbPassword, - ]); - } + $this->createSpecificUser($username, $connection); //create the database $this->createDatabase($connection); //fill the database if needed - $query='select count(*) from information_schema.tables' - ." where table_schema='".$this->dbName."' AND table_name = '".$this->tablePrefix."users';"; - $result = $connection->executeQuery($query); + $query='select count(*) from information_schema.tables where table_schema=? AND table_name = ?'; + $result = $connection->executeQuery($query, [$this->dbName, $this->tablePrefix.'users']); $row = $result->fetch(); if(!$result or $row[0]==0) { \OC_DB::createDbFromStructure($this->dbDefinitionFile); @@ -93,19 +51,26 @@ class MySQL extends AbstractDatabase { * @param \OC\DB\Connection $connection */ private function createDatabase($connection) { - $name = $this->dbName; - $user = $this->dbUser; - //we cant use OC_BD functions here because we need to connect as the administrative user. - $query = "CREATE DATABASE IF NOT EXISTS `$name` CHARACTER SET utf8 COLLATE utf8_bin;"; - $connection->executeUpdate($query); - - //this query will fail if there aren't the right permissions, ignore the error - $query="GRANT ALL PRIVILEGES ON `$name` . * TO '$user'"; - $connection->executeUpdate($query); + try{ + $name = $this->dbName; + $user = $this->dbUser; + //we cant use OC_BD functions here because we need to connect as the administrative user. + $query = "CREATE DATABASE IF NOT EXISTS `$name` CHARACTER SET utf8 COLLATE utf8_bin;"; + $connection->executeUpdate($query); + + //this query will fail if there aren't the right permissions, ignore the error + $query="GRANT ALL PRIVILEGES ON `$name` . * TO '$user'"; + $connection->executeUpdate($query); + } catch (\Exception $ex) { + $this->logger->error('Database creation failed: {error}', [ + 'app' => 'mysql.setup', + 'error' => $ex->getMessage() + ]); + } } /** - * @param \OC\DB\Connection $connection + * @param IDbConnection $connection * @throws \OC\DatabaseSetupException */ private function createDBUser($connection) { @@ -134,4 +99,63 @@ class MySQL extends AbstractDatabase { $cf = new ConnectionFactory(); return $cf->getConnection($type, $connectionParams); } + + /** + * @param $username + * @param IDBConnection $connection + * @return array + */ + private function createSpecificUser($username, $connection) { + try { + //user already specified in config + $oldUser = $this->config->getSystemValue('dbuser', false); + + //we don't have a dbuser specified in config + if ($this->dbUser !== $oldUser) { + //add prefix to the admin username to prevent collisions + $adminUser = substr('oc_' . $username, 0, 16); + + $i = 1; + while (true) { + //this should be enough to check for admin rights in mysql + $query = 'SELECT user FROM mysql.user WHERE user=?'; + $result = $connection->executeQuery($query, [$adminUser]); + + //current dbuser has admin rights + if ($result) { + $data = $result->fetchAll(); + //new dbuser does not exist + if (count($data) === 0) { + //use the admin login data for the new database user + $this->dbUser = $adminUser; + + //create a random password so we don't need to store the admin password in the config file + $this->dbPassword = $this->random->getMediumStrengthGenerator()->generate(30); + + $this->createDBUser($connection); + + break; + } else { + //repeat with different username + $length = strlen((string)$i); + $adminUser = substr('oc_' . $username, 0, 16 - $length) . $i; + $i++; + } + } else { + break; + } + }; + } + } catch (\Exception $ex) { + $this->logger->error('Specific user creation failed: {error}', [ + 'app' => 'mysql.setup', + 'error' => $ex->getMessage() + ]); + } + + $this->config->setSystemValues([ + 'dbuser' => $this->dbUser, + 'dbpassword' => $this->dbPassword, + ]); + } } diff --git a/lib/private/util.php b/lib/private/util.php index 39d64952dc6..1b22e03ca6f 100644 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -567,7 +567,8 @@ class OC_Util { } $webServerRestart = false; - $setup = new \OC\Setup($config, \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), new \OC_Defaults()); + $setup = new \OC\Setup($config, \OC::$server->getIniWrapper(), \OC::$server->getL10N('lib'), + new \OC_Defaults(), \OC::$server->getLogger(), \OC::$server->getSecureRandom()); $availableDatabases = $setup->getSupportedDatabases(); if (empty($availableDatabases)) { $errors[] = array( -- cgit v1.2.3 From 4d672ded244e5e8a242d649588879979bdde1bb6 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 30 Jul 2015 09:02:35 +0200 Subject: properly mock dependencies for setup tests --- tests/lib/setup.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/lib/setup.php b/tests/lib/setup.php index d07eaa40ee0..fa9fe08ed95 100644 --- a/tests/lib/setup.php +++ b/tests/lib/setup.php @@ -20,6 +20,10 @@ class Test_OC_Setup extends \Test\TestCase { private $defaults; /** @var \OC\Setup | PHPUnit_Framework_MockObject_MockObject */ protected $setupClass; + /** @var \OCP\ILogger | PHPUnit_Framework_MockObject_MockObject */ + protected $logger; + /** @var \OCP\Security\ISecureRandom | PHPUnit_Framework_MockObject_MockObject */ + protected $random; protected function setUp() { parent::setUp(); @@ -28,9 +32,11 @@ class Test_OC_Setup extends \Test\TestCase { $this->iniWrapper = $this->getMock('\bantu\IniGetWrapper\IniGetWrapper'); $this->l10n = $this->getMock('\OCP\IL10N'); $this->defaults = $this->getMock('\OC_Defaults'); + $this->logger = $this->getMock('\OCP\ILogger'); + $this->random = $this->getMock('\OCP\Security\ISecureRandom'); $this->setupClass = $this->getMock('\OC\Setup', ['class_exists', 'is_callable'], - [$this->config, $this->iniWrapper, $this->l10n, $this->defaults]); + [$this->config, $this->iniWrapper, $this->l10n, $this->defaults, $this->logger, $this->random]); } public function testGetSupportedDatabasesWithOneWorking() { -- cgit v1.2.3 From e95bc68ac75222bdb839d2fb062abc9d4a8e1911 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 30 Jul 2015 12:32:22 +0200 Subject: Check for PDO instead of removed function for PHP 7 compatibility --- lib/private/setup.php | 30 ++++++++++++++++++++++-------- tests/lib/setup.php | 20 ++++++++++++++++---- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/lib/private/setup.php b/lib/private/setup.php index afc88256da4..8f1ae389e45 100644 --- a/lib/private/setup.php +++ b/lib/private/setup.php @@ -89,7 +89,7 @@ class Setup { * @param string $name * @return bool */ - public function class_exists($name) { + protected function class_exists($name) { return class_exists($name); } @@ -98,10 +98,19 @@ class Setup { * @param string $name * @return bool */ - public function is_callable($name) { + protected function is_callable($name) { return is_callable($name); } + /** + * Wrapper around \PDO::getAvailableDrivers + * + * @return array + */ + protected function getAvailableDbDriversForPdo() { + return \PDO::getAvailableDrivers(); + } + /** * Get the available and supported databases of this instance * @@ -117,8 +126,8 @@ class Setup { 'name' => 'SQLite' ), 'mysql' => array( - 'type' => 'function', - 'call' => 'mysql_connect', + 'type' => 'pdo', + 'call' => 'mysql', 'name' => 'MySQL/MariaDB' ), 'pgsql' => array( @@ -147,10 +156,15 @@ class Setup { foreach($configuredDatabases as $database) { if(array_key_exists($database, $availableDatabases)) { $working = false; - if($availableDatabases[$database]['type'] === 'class') { - $working = $this->class_exists($availableDatabases[$database]['call']); - } elseif ($availableDatabases[$database]['type'] === 'function') { - $working = $this->is_callable($availableDatabases[$database]['call']); + $type = $availableDatabases[$database]['type']; + $call = $availableDatabases[$database]['call']; + + if($type === 'class') { + $working = $this->class_exists($call); + } elseif ($type === 'function') { + $working = $this->is_callable($call); + } elseif($type === 'pdo') { + $working = in_array($call, $this->getAvailableDbDriversForPdo(), TRUE); } if($working) { $supportedDatabases[$database] = $availableDatabases[$database]['name']; diff --git a/tests/lib/setup.php b/tests/lib/setup.php index fa9fe08ed95..72c84520056 100644 --- a/tests/lib/setup.php +++ b/tests/lib/setup.php @@ -35,7 +35,7 @@ class Test_OC_Setup extends \Test\TestCase { $this->logger = $this->getMock('\OCP\ILogger'); $this->random = $this->getMock('\OCP\Security\ISecureRandom'); $this->setupClass = $this->getMock('\OC\Setup', - ['class_exists', 'is_callable'], + ['class_exists', 'is_callable', 'getAvailableDbDriversForPdo'], [$this->config, $this->iniWrapper, $this->l10n, $this->defaults, $this->logger, $this->random]); } @@ -51,9 +51,13 @@ class Test_OC_Setup extends \Test\TestCase { ->method('class_exists') ->will($this->returnValue(true)); $this->setupClass - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('is_callable') ->will($this->returnValue(false)); + $this->setupClass + ->expects($this->once()) + ->method('getAvailableDbDriversForPdo') + ->will($this->returnValue([])); $result = $this->setupClass->getSupportedDatabases(); $expectedResult = array( 'sqlite' => 'SQLite' @@ -74,9 +78,13 @@ class Test_OC_Setup extends \Test\TestCase { ->method('class_exists') ->will($this->returnValue(false)); $this->setupClass - ->expects($this->exactly(3)) + ->expects($this->exactly(2)) ->method('is_callable') ->will($this->returnValue(false)); + $this->setupClass + ->expects($this->once()) + ->method('getAvailableDbDriversForPdo') + ->will($this->returnValue([])); $result = $this->setupClass->getSupportedDatabases(); $this->assertSame(array(), $result); @@ -94,9 +102,13 @@ class Test_OC_Setup extends \Test\TestCase { ->method('class_exists') ->will($this->returnValue(true)); $this->setupClass - ->expects($this->exactly(3)) + ->expects($this->exactly(2)) ->method('is_callable') ->will($this->returnValue(true)); + $this->setupClass + ->expects($this->once()) + ->method('getAvailableDbDriversForPdo') + ->will($this->returnValue(['mysql'])); $result = $this->setupClass->getSupportedDatabases(); $expectedResult = array( 'sqlite' => 'SQLite', -- cgit v1.2.3