Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use pdo for postgres setup #381

Merged
merged 4 commits into from
Jul 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/private/AllConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -428,4 +428,8 @@ public function getUsersForUserValue($appName, $key, $value) {

return $userIDs;
}

public function getSystemConfig() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😰 why? just use the methods from above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connectionfactory wants a systemconfig instance

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then mark @internal at least please

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not exposed in the public interface

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmmm Maybe we should change the connectionfactory at some point then.

return $this->systemConfig;
}
}
4 changes: 2 additions & 2 deletions lib/private/Setup.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ public function getSupportedDatabases($allowAllDatabases = false) {
'name' => 'MySQL/MariaDB'
),
'pgsql' => array(
'type' => 'function',
'call' => 'pg_connect',
'type' => 'pdo',
'call' => 'pgsql',
'name' => 'PostgreSQL'
),
'oci' => array(
Expand Down
23 changes: 22 additions & 1 deletion lib/private/Setup/AbstractDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
*/
namespace OC\Setup;

use OC\AllConfig;
use OC\DB\ConnectionFactory;
use OCP\IConfig;
use OCP\ILogger;
use OCP\Security\ISecureRandom;
Expand All @@ -45,7 +47,7 @@ abstract class AbstractDatabase {
protected $dbPort;
/** @var string */
protected $tablePrefix;
/** @var IConfig */
/** @var AllConfig */
protected $config;
/** @var ILogger */
protected $logger;
Expand Down Expand Up @@ -98,6 +100,25 @@ public function initialize($config) {
$this->tablePrefix = $dbTablePrefix;
}

/**
* @param array $configOverwrite
* @return \OC\DB\Connection
*/
protected function connect(array $configOverwrite = []) {
$systemConfig = $this->config->getSystemConfig();
$cf = new ConnectionFactory();
$connectionParams = $cf->createConnectionParams($systemConfig);
// we don't save username/password to the config immediately so this might not be set
if (!$connectionParams['user']) {
$connectionParams['user'] = $this->dbUser;
}
if (!$connectionParams['password']) {
$connectionParams['password'] = $this->dbPassword;
}
$connectionParams = array_merge($connectionParams, $configOverwrite);
return $cf->getConnection($systemConfig->getValue('dbtype', 'sqlite'), $connectionParams);
}

/**
* @param string $userName
*/
Expand Down
35 changes: 0 additions & 35 deletions lib/private/Setup/MySQL.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,41 +87,6 @@ private function createDBUser($connection) {
$connection->executeUpdate($query);
}

/**
* @return \OC\DB\Connection
* @throws \OC\DatabaseSetupException
*/
private function connect() {

$connectionParams = array(
'host' => $this->dbHost,
'user' => $this->dbUser,
'password' => $this->dbPassword,
'tablePrefix' => $this->tablePrefix,
);

// adding port support through installer
if(!empty($this->dbPort)) {
if (ctype_digit($this->dbPort)) {
$connectionParams['port'] = $this->dbPort;
} else {
$connectionParams['unix_socket'] = $this->dbPort;
}
} else if (strpos($this->dbHost, ':')) {
// Host variable may carry a port or socket.
list($host, $portOrSocket) = explode(':', $this->dbHost, 2);
if (ctype_digit($portOrSocket)) {
$connectionParams['port'] = $portOrSocket;
} else {
$connectionParams['unix_socket'] = $portOrSocket;
}
$connectionParams['host'] = $host;
}

$cf = new ConnectionFactory();
return $cf->getConnection('mysql', $connectionParams);
}

/**
* @param $username
* @param IDBConnection $connection
Expand Down
185 changes: 82 additions & 103 deletions lib/private/Setup/PostgreSQL.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,42 +26,34 @@
*/
namespace OC\Setup;

use OC\DatabaseException;
use OC\DB\QueryBuilder\Literal;
use OCP\IDBConnection;

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);

// adding port support through installer
if(!empty($this->dbPort)) {
// casting to int to avoid malicious input
$port = (int)$this->dbPort;
} else if(strpos($e_host, ':')) {
list($e_host, $port)=explode(':', $e_host, 2);
} else {
$port=false;
$connection = $this->connect([
'dbname' => 'postgres'
]);
//check for roles creation rights in postgresql
$builder = $connection->getQueryBuilder();
$builder->automaticTablePrefix(false);
$query = $builder
->select('rolname')
->from('pg_roles')
->where($builder->expr()->eq('rolcreaterole', new Literal('TRUE')))
->andWhere($builder->expr()->eq('rolname', $builder->createNamedParameter($this->dbUser)));

try {
$result = $query->execute();
$canCreateRoles = $result->rowCount() > 0;
} catch (DatabaseException $e) {
$canCreateRoles = false;
}

//check if the database user has admin rights
$connection_string = "host='$e_host' dbname=postgres user='$e_user' port='$port' password='$e_password'";
$connection = @pg_connect($connection_string);
if(!$connection) {
// Try if we can connect to the DB with the specified name
$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);

if(!$connection)
throw new \OC\DatabaseSetupException($this->trans->t('PostgreSQL connection failed'),
$this->trans->t('Please check your connection details.'));
}
$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);
if($result and pg_num_rows($result) > 0) {
if($canCreateRoles) {
//use the admin login data for the new database user

//add prefix to the postgresql user name to prevent collisions
Expand All @@ -72,106 +64,93 @@ public function setupDatabase($username) {
$this->createDBUser($connection);
}

$systemConfig = \OC::$server->getSystemConfig();
$systemConfig = $this->config->getSystemConfig();
$systemConfig->setValues([
'dbuser' => $this->dbUser,
'dbpassword' => $this->dbPassword,
]);

//create the database
$this->createDatabase($connection);
$query = $connection->prepare("select count(*) FROM pg_class WHERE relname=? limit 1");
$query->execute([$this->tablePrefix . "users"]);
$tablesSetup = $query->fetchColumn() > 0;

// the connection to dbname=postgres is not needed anymore
pg_close($connection);
$connection->close();

// connect to the ownCloud database (dbname=$this->dbname) and check if it needs to be filled
$this->dbUser = $systemConfig->getValue('dbuser');
$this->dbPassword = $systemConfig->getValue('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, ':')) {
list($e_host, $port)=explode(':', $e_host, 2);
} else {
$port=false;
}

$connection_string = "host='$e_host' dbname='$e_dbname' user='$e_user' port='$port' password='$e_password'";
$connection = @pg_connect($connection_string);
if(!$connection) {
$connection = $this->connect();
try {
$connection->connect();
} catch (\Exception $e) {
$this->logger->logException($e);
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.'));
$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";
$result = pg_query($connection, $query);
if($result) {
$row = pg_fetch_row($result);
}
if(!$result or $row[0]==0) {


if(!$tablesSetup) {
\OC_DB::createDbFromStructure($this->dbDefinitionFile);
}
}

private function createDatabase($connection) {
//we can't 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);
$query = "select datname from pg_database where datname = '$e_name'";
$result = pg_query($connection, $query);
if(!$result) {
$entry = $this->trans->t('DB Error: "%s"', array(pg_last_error($connection))) . '<br />';
$entry .= $this->trans->t('Offending command was: "%s"', array($query)) . '<br />';
\OCP\Util::writeLog('setup.pg', $entry, \OCP\Util::WARN);
}
if(! pg_fetch_row($result)) {
private function createDatabase(IDBConnection $connection) {
if(!$this->databaseExists($connection)) {
//The database does not exists... let's create it
$query = "CREATE DATABASE \"$e_name\" OWNER \"$e_user\"";
$result = pg_query($connection, $query);
if(!$result) {
$entry = $this->trans->t('DB Error: "%s"', array(pg_last_error($connection))) . '<br />';
$entry .= $this->trans->t('Offending command was: "%s"', array($query)) . '<br />';
\OCP\Util::writeLog('setup.pg', $entry, \OCP\Util::WARN);
$query = $connection->prepare("CREATE DATABASE " . addslashes($this->dbName) . " OWNER " . addslashes($this->dbUser));
try {
$query->execute();
} catch (DatabaseException $e) {
$this->logger->error('Error while trying to create database');
$this->logger->logException($e);
}
else {
$query = "REVOKE ALL PRIVILEGES ON DATABASE \"$e_name\" FROM PUBLIC";
pg_query($connection, $query);
} else {
$query = $connection->prepare("REVOKE ALL PRIVILEGES ON DATABASE " . addslashes($this->dbName) . " FROM PUBLIC");
try {
$query->execute();
} catch (DatabaseException $e) {
$this->logger->error('Error while trying to restrict database permissions');
$this->logger->logException($e);
}
}
}

private function createDBUser($connection) {
$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) {
$entry = $this->trans->t('DB Error: "%s"', array(pg_last_error($connection))) . '<br />';
$entry .= $this->trans->t('Offending command was: "%s"', array($query)) . '<br />';
\OCP\Util::writeLog('setup.pg', $entry, \OCP\Util::WARN);
}
private function userExists(IDBConnection $connection) {
$builder = $connection->getQueryBuilder();
$builder->automaticTablePrefix(false);
$query = $builder->select('*')
->from('pg_roles')
->where($builder->expr()->eq('rolname', $builder->createNamedParameter($this->dbUser)));
$result = $query->execute();
return $result->rowCount() > 0;
}

if(! pg_fetch_row($result)) {
//user does not exists let's create it :)
$query = "CREATE USER \"$e_name\" CREATEDB PASSWORD '$e_password';";
$result = pg_query($connection, $query);
if(!$result) {
$entry = $this->trans->t('DB Error: "%s"', array(pg_last_error($connection))) . '<br />';
$entry .= $this->trans->t('Offending command was: "%s"', array($query)) . '<br />';
\OCP\Util::writeLog('setup.pg', $entry, \OCP\Util::WARN);
}
}
else { // change password of the existing role
$query = "ALTER ROLE \"$e_name\" WITH PASSWORD '$e_password';";
$result = pg_query($connection, $query);
if(!$result) {
$entry = $this->trans->t('DB Error: "%s"', array(pg_last_error($connection))) . '<br />';
$entry .= $this->trans->t('Offending command was: "%s"', array($query)) . '<br />';
\OCP\Util::writeLog('setup.pg', $entry, \OCP\Util::WARN);
private function databaseExists(IDBConnection $connection) {
$builder = $connection->getQueryBuilder();
$builder->automaticTablePrefix(false);
$query = $builder->select('datname')
->from('pg_database')
->where($builder->expr()->eq('datname', $builder->createNamedParameter($this->dbName)));
$result = $query->execute();
return $result->rowCount() > 0;
}

private function createDBUser(IDBConnection $connection) {
try {
if ($this->userExists($connection, $this->dbUser)) {
// change the password
$query = $connection->prepare("ALTER ROLE " . addslashes($this->dbUser) . " CREATEDB WITH PASSWORD " . addslashes($this->dbPassword));
} else {
// create the user
$query = $connection->prepare("CREATE USER " . addslashes($this->dbUser) . " CREATEDB PASSWORD " . addslashes($this->dbPassword));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broke the tests on my app:

Syntax error: 7 ERROR:  syntax error at or near "zxngvsgscub2i9frs64ac9r43ivk6j"

LINE 1: CREATE USER oc_admin CREATEDB PASSWORD zxngvsgscub2i9frs64ac

The ' are missing, I will add them.

https://travis-ci.org/nextcloud/announcementcenter/jobs/145487372

}
$query->execute();
} catch (DatabaseException $e) {
$this->logger->error('Error while trying to create database user');
$this->logger->logException($e);
}
}
}
16 changes: 8 additions & 8 deletions tests/lib/SetupTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function testGetSupportedDatabasesWithOneWorking() {
->method('is_callable')
->will($this->returnValue(false));
$this->setupClass
->expects($this->once())
->expects($this->any())
->method('getAvailableDbDriversForPdo')
->will($this->returnValue([]));
$result = $this->setupClass->getSupportedDatabases();
Expand All @@ -76,15 +76,15 @@ public function testGetSupportedDatabasesWithNoWorking() {
array('sqlite', 'mysql', 'oci', 'pgsql')
));
$this->setupClass
->expects($this->once())
->expects($this->any())
->method('class_exists')
->will($this->returnValue(false));
$this->setupClass
->expects($this->exactly(2))
->expects($this->any())
->method('is_callable')
->will($this->returnValue(false));
$this->setupClass
->expects($this->once())
->expects($this->any())
->method('getAvailableDbDriversForPdo')
->will($this->returnValue([]));
$result = $this->setupClass->getSupportedDatabases();
Expand All @@ -100,17 +100,17 @@ public function testGetSupportedDatabasesWithAllWorking() {
array('sqlite', 'mysql', 'pgsql', 'oci')
));
$this->setupClass
->expects($this->once())
->expects($this->any())
->method('class_exists')
->will($this->returnValue(true));
$this->setupClass
->expects($this->exactly(2))
->expects($this->any())
->method('is_callable')
->will($this->returnValue(true));
$this->setupClass
->expects($this->once())
->expects($this->any())
->method('getAvailableDbDriversForPdo')
->will($this->returnValue(['mysql']));
->will($this->returnValue(['mysql', 'pgsql']));
$result = $this->setupClass->getSupportedDatabases();
$expectedResult = array(
'sqlite' => 'SQLite',
Expand Down