From 88cd61521459a18599407f83347f1d6a0e7700cc Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Mon, 24 Aug 2015 13:21:09 +0100 Subject: [PATCH 1/9] Introduce IDBConnection::setValues() setValues() attempts to insert a new row, or failing that, update an existing row. The ability to set preconditions is also available. --- lib/private/allconfig.php | 55 +++++------------ lib/private/appframework/db/db.php | 15 +++++ lib/private/db/connection.php | 47 +++++++++++++++ lib/public/idbconnection.php | 14 +++++ tests/lib/allconfig.php | 22 +------ tests/lib/db/connection.php | 94 ++++++++++++++++++++++++++++-- 6 files changed, 179 insertions(+), 68 deletions(-) diff --git a/lib/private/allconfig.php b/lib/private/allconfig.php index af7ffa4168e..3c85bfacbb8 100644 --- a/lib/private/allconfig.php +++ b/lib/private/allconfig.php @@ -205,57 +205,28 @@ class AllConfig implements \OCP\IConfig { // TODO - FIXME $this->fixDIInit(); - // Check if the key does exist - $sql = 'SELECT `configvalue` FROM `*PREFIX*preferences` '. - 'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'; - $result = $this->connection->executeQuery($sql, array($userId, $appName, $key)); - $oldValue = $result->fetchColumn(); - $result->closeCursor(); - $exists = $oldValue !== false; - - if($oldValue === strval($value)) { - // no changes - return; + $preconditionArray = []; + if (isset($preCondition)) { + $preconditionArray = [ + 'configvalue' => $preCondition, + ]; } - $affectedRows = 0; - if (!$exists && $preCondition === null) { - $this->connection->insertIfNotExist('*PREFIX*preferences', [ - 'configvalue' => $value, - 'userid' => $userId, - 'appid' => $appName, - 'configkey' => $key, - ], ['configkey', 'userid', 'appid']); - $affectedRows = 1; - } elseif ($exists) { - $data = array($value, $userId, $appName, $key); - - $sql = 'UPDATE `*PREFIX*preferences` SET `configvalue` = ? '. - 'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ? '; - - if($preCondition !== null) { - if($this->getSystemValue('dbtype', 'sqlite') === 'oci') { - //oracle hack: need to explicitly cast CLOB to CHAR for comparison - $sql .= 'AND to_char(`configvalue`) = ?'; - } else { - $sql .= 'AND `configvalue` = ?'; - } - $data[] = $preCondition; - } - $affectedRows = $this->connection->executeUpdate($sql, $data); - } + $this->connection->setValues('preferences', [ + 'userid' => $userId, + 'appid' => $appName, + 'configkey' => $key, + ], [ + 'configvalue' => $value, + ], $preconditionArray); // only add to the cache if we already loaded data for the user - if ($affectedRows > 0 && isset($this->userCache[$userId])) { + if (isset($this->userCache[$userId])) { if (!isset($this->userCache[$userId][$appName])) { $this->userCache[$userId][$appName] = array(); } $this->userCache[$userId][$appName][$key] = $value; } - - if ($preCondition !== null && $affectedRows === 0) { - throw new PreConditionNotMetException; - } } /** diff --git a/lib/private/appframework/db/db.php b/lib/private/appframework/db/db.php index 812649daa78..5fdc5d1066c 100644 --- a/lib/private/appframework/db/db.php +++ b/lib/private/appframework/db/db.php @@ -146,6 +146,21 @@ class Db implements IDb { return $this->connection->insertIfNotExist($table, $input, $compare); } + /** + * Insert or update a row value + * + * @param string $table + * @param array $keys (column name => value) + * @param array $values (column name => value) + * @param array $updatePreconditionValues ensure values match preconditions (column name => value) + * @return int number of new rows + * @throws \Doctrine\DBAL\DBALException + * @throws PreconditionNotMetException + */ + public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []) { + return $this->connection->setValues($table, $keys, $values, $updatePreconditionValues); + } + /** * Start a transaction */ diff --git a/lib/private/db/connection.php b/lib/private/db/connection.php index 28bf3b6e05b..b1c1c12cb49 100644 --- a/lib/private/db/connection.php +++ b/lib/private/db/connection.php @@ -32,6 +32,7 @@ use Doctrine\Common\EventManager; use OC\DB\QueryBuilder\ExpressionBuilder; use OC\DB\QueryBuilder\QueryBuilder; use OCP\IDBConnection; +use OCP\PreconditionNotMetException; class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { /** @@ -241,6 +242,52 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { return $this->adapter->insertIfNotExist($table, $input, $compare); } + /** + * Insert or update a row value + * + * @param string $table + * @param array $keys (column name => value) + * @param array $values (column name => value) + * @param array $updatePreconditionValues ensure values match preconditions (column name => value) + * @return int number of new rows + * @throws \Doctrine\DBAL\DBALException + * @throws PreconditionNotMetException + */ + public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []) { + try { + $insertQb = $this->getQueryBuilder(); + $insertQb->insert($table) + ->values( + array_map(function($value) use ($insertQb) { + return $insertQb->createNamedParameter($value); + }, array_merge($keys, $values)) + ); + return $insertQb->execute(); + } catch (\Doctrine\DBAL\Exception\UniqueConstraintViolationException $e) { + // value already exists, try update + $updateQb = $this->getQueryBuilder(); + $updateQb->update($table); + foreach ($values as $name => $value) { + $updateQb->set($name, $updateQb->createNamedParameter($value)); + } + $where = $updateQb->expr()->andx(); + $whereValues = array_merge($keys, $updatePreconditionValues); + foreach ($whereValues as $name => $value) { + $where->add($updateQb->expr()->eq( + $name, $updateQb->createNamedParameter($value) + )); + } + $updateQb->where($where); + $affected = $updateQb->execute(); + + if ($affected === 0) { + throw new PreconditionNotMetException(); + } + + return 0; + } + } + /** * returns the error code and message as a string for logging * works with DoctrineException diff --git a/lib/public/idbconnection.php b/lib/public/idbconnection.php index 49856fb78a5..c5767e65a82 100644 --- a/lib/public/idbconnection.php +++ b/lib/public/idbconnection.php @@ -108,6 +108,20 @@ interface IDBConnection { */ public function insertIfNotExist($table, $input, array $compare = null); + /** + * Insert or update a row value + * + * @param string $table + * @param array $keys (column name => value) + * @param array $values (column name => value) + * @param array $updatePreconditionValues ensure values match preconditions (column name => value) + * @return int number of new rows + * @throws \Doctrine\DBAL\DBALException + * @throws PreconditionNotMetException + * @since 9.0.0 + */ + public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []); + /** * Start a transaction * @since 6.0.0 diff --git a/tests/lib/allconfig.php b/tests/lib/allconfig.php index 0caf8163cfc..869bf9b8d66 100644 --- a/tests/lib/allconfig.php +++ b/tests/lib/allconfig.php @@ -90,16 +90,7 @@ class TestAllConfig extends \Test\TestCase { } public function testSetUserValueWithPreCondition() { - // mock the check for the database to run the correct SQL statements for each database type - $systemConfig = $this->getMockBuilder('\OC\SystemConfig') - ->disableOriginalConstructor() - ->getMock(); - $systemConfig->expects($this->once()) - ->method('getValue') - ->with($this->equalTo('dbtype'), - $this->equalTo('sqlite')) - ->will($this->returnValue(\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite'))); - $config = $this->getConfig($systemConfig); + $config = $this->getConfig(); $selectAllSQL = 'SELECT `userid`, `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?'; @@ -136,16 +127,7 @@ class TestAllConfig extends \Test\TestCase { * @expectedException \OCP\PreConditionNotMetException */ public function testSetUserValueWithPreConditionFailure() { - // mock the check for the database to run the correct SQL statements for each database type - $systemConfig = $this->getMockBuilder('\OC\SystemConfig') - ->disableOriginalConstructor() - ->getMock(); - $systemConfig->expects($this->once()) - ->method('getValue') - ->with($this->equalTo('dbtype'), - $this->equalTo('sqlite')) - ->will($this->returnValue(\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite'))); - $config = $this->getConfig($systemConfig); + $config = $this->getConfig(); $selectAllSQL = 'SELECT `userid`, `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?'; diff --git a/tests/lib/db/connection.php b/tests/lib/db/connection.php index 348a5e31e09..51b4145b7a0 100644 --- a/tests/lib/db/connection.php +++ b/tests/lib/db/connection.php @@ -25,20 +25,17 @@ class Connection extends \Test\TestCase { */ private $connection; - public static function setUpBeforeClass() - { + public static function setUpBeforeClass() { self::dropTestTable(); parent::setUpBeforeClass(); } - public static function tearDownAfterClass() - { + public static function tearDownAfterClass() { self::dropTestTable(); parent::tearDownAfterClass(); } - protected static function dropTestTable() - { + protected static function dropTestTable() { if (\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite') !== 'oci') { \OC::$server->getDatabaseConnection()->dropTable('table'); } @@ -92,4 +89,89 @@ class Connection extends \Test\TestCase { $this->connection->dropTable('table'); $this->assertTableNotExist('table'); } + + private function getTextValueByIntergerField($integerField) { + $builder = $this->connection->getQueryBuilder(); + $query = $builder->select('textfield') + ->from('table') + ->where($builder->expr()->eq('integerfield', $builder->createNamedParameter($integerField, \PDO::PARAM_INT))); + + $result = $query->execute(); + return $result->fetchColumn(); + } + + public function testSetValues() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo' + ]); + + $this->assertEquals('foo', $this->getTextValueByIntergerField(1)); + + $this->connection->dropTable('table'); + } + + public function testSetValuesOverWrite() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo' + ]); + + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'bar' + ]); + + $this->assertEquals('bar', $this->getTextValueByIntergerField(1)); + + $this->connection->dropTable('table'); + } + + public function testSetValuesOverWritePrecondition() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo', + 'booleanfield' => true + ]); + + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'bar' + ], [ + 'booleanfield' => true + ]); + + $this->assertEquals('bar', $this->getTextValueByIntergerField(1)); + + $this->connection->dropTable('table'); + } + + /** + * @expectedException \OCP\PreConditionNotMetException + */ + public function testSetValuesOverWritePreconditionFailed() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo', + 'booleanfield' => true + ]); + + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'bar' + ], [ + 'booleanfield' => false + ]); + } } From da4127d23b168dbb34c066d9590b3fe1b965af46 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Mon, 24 Aug 2015 16:13:16 +0100 Subject: [PATCH 2/9] Introduce CredentialsManager for storage of credentials in DB CredentialsManager performs a simple role, of storing and retrieving encrypted credentials from the database. Credentials are stored by user ID (which may be null) and credentials identifier. Credentials themselves may be of any type that can be JSON encoded. The rationale behind this is to avoid further (mis)use of oc_preferences, which was being used for all manner of data not related to user preferences. --- db_structure.xml | 55 ++++++++ .../dependencyinjection/dicontainer.php | 4 + lib/private/security/credentialsmanager.php | 125 ++++++++++++++++++ lib/private/server.php | 13 ++ lib/public/iservercontainer.php | 8 ++ lib/public/security/icredentialsmanager.php | 71 ++++++++++ tests/lib/security/credentialsmanager.php | 90 +++++++++++++ 7 files changed, 366 insertions(+) create mode 100644 lib/private/security/credentialsmanager.php create mode 100644 lib/public/security/icredentialsmanager.php create mode 100644 tests/lib/security/credentialsmanager.php diff --git a/db_structure.xml b/db_structure.xml index e4bd8d998ee..cfda315e3c2 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -1493,5 +1493,60 @@ + + + *dbprefix*credentials + + + + + user + text + + false + 64 + + + + identifier + text + true + 64 + + + + credentials + clob + false + + + + credentials_user_id + true + true + + user + ascending + + + identifier + ascending + + + + + credentials_user + false + + user + ascending + + + + + +
diff --git a/lib/private/appframework/dependencyinjection/dicontainer.php b/lib/private/appframework/dependencyinjection/dicontainer.php index 8fc52141d5b..a9614262603 100644 --- a/lib/private/appframework/dependencyinjection/dicontainer.php +++ b/lib/private/appframework/dependencyinjection/dicontainer.php @@ -215,6 +215,10 @@ class DIContainer extends SimpleContainer implements IAppContainer { return $this->getServer()->getHasher(); }); + $this->registerService('OCP\\Security\\ICredentialsManager', function($c) { + return $this->getServer()->getCredentialsManager(); + }); + $this->registerService('OCP\\Security\\ISecureRandom', function($c) { return $this->getServer()->getSecureRandom(); }); diff --git a/lib/private/security/credentialsmanager.php b/lib/private/security/credentialsmanager.php new file mode 100644 index 00000000000..405922847be --- /dev/null +++ b/lib/private/security/credentialsmanager.php @@ -0,0 +1,125 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Security; + +use OCP\Security\ICrypto; +use OCP\IDBConnection; +use OCP\Security\ICredentialsManager; +use OCP\IConfig; + +/** + * Store and retrieve credentials for external services + * + * @package OC\Security + */ +class CredentialsManager implements ICredentialsManager { + + const DB_TABLE = 'credentials'; + + /** @var ICrypto */ + protected $crypto; + + /** @var IDBConnection */ + protected $dbConnection; + + /** + * @param ICrypto $crypto + * @param IDBConnection $dbConnection + */ + public function __construct(ICrypto $crypto, IDBConnection $dbConnection) { + $this->crypto = $crypto; + $this->dbConnection = $dbConnection; + } + + /** + * Store a set of credentials + * + * @param string|null $userId Null for system-wide credentials + * @param string $identifier + * @param mixed $credentials + */ + public function store($userId, $identifier, $credentials) { + $value = $this->crypto->encrypt(json_encode($credentials)); + + $this->dbConnection->setValues(self::DB_TABLE, [ + 'user' => $userId, + 'identifier' => $identifier, + ], [ + 'credentials' => $value, + ]); + } + + /** + * Retrieve a set of credentials + * + * @param string|null $userId Null for system-wide credentials + * @param string $identifier + * @return mixed + */ + public function retrieve($userId, $identifier) { + $qb = $this->dbConnection->getQueryBuilder(); + $qb->select('credentials') + ->from(self::DB_TABLE) + ->where($qb->expr()->eq('user', $qb->createNamedParameter($userId))) + ->andWhere($qb->expr()->eq('identifier', $qb->createNamedParameter($identifier))) + ; + $result = $qb->execute()->fetch(); + + if (!$result) { + return null; + } + $value = $result['credentials']; + + return json_decode($this->crypto->decrypt($value), true); + } + + /** + * Delete a set of credentials + * + * @param string|null $userId Null for system-wide credentials + * @param string $identifier + * @return int rows removed + */ + public function delete($userId, $identifier) { + $qb = $this->dbConnection->getQueryBuilder(); + $qb->delete(self::DB_TABLE) + ->where($qb->expr()->eq('user', $qb->createNamedParameter($userId))) + ->andWhere($qb->expr()->eq('identifier', $qb->createNamedParameter($identifier))) + ; + return $qb->execute(); + } + + /** + * Erase all credentials stored for a user + * + * @param string $userId + * @return int rows removed + */ + public function erase($userId) { + $qb = $this->dbConnection->getQueryBuilder(); + $qb->delete(self::DB_TABLE) + ->where($qb->expr()->eq('user', $qb->createNamedParameter($userId))) + ; + return $qb->execute(); + } + +} diff --git a/lib/private/server.php b/lib/private/server.php index 79bd96652ed..7c4b7dad764 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -63,6 +63,7 @@ use OC\Notification\Manager; use OC\Security\CertificateManager; use OC\Security\Crypto; use OC\Security\Hasher; +use OC\Security\CredentialsManager; use OC\Security\SecureRandom; use OC\Security\TrustedDomainHelper; use OC\Session\CryptoWrapper; @@ -332,6 +333,9 @@ class Server extends ServerContainer implements IServerContainer { $this->registerService('Hasher', function (Server $c) { return new Hasher($c->getConfig()); }); + $this->registerService('CredentialsManager', function (Server $c) { + return new CredentialsManager($c->getCrypto(), $c->getDatabaseConnection()); + }); $this->registerService('DatabaseConnection', function (Server $c) { $factory = new \OC\DB\ConnectionFactory(); $systemConfig = $c->getSystemConfig(); @@ -919,6 +923,15 @@ class Server extends ServerContainer implements IServerContainer { return $this->query('Hasher'); } + /** + * Returns a CredentialsManager instance + * + * @return \OCP\Security\ICredentialsManager + */ + public function getCredentialsManager() { + return $this->query('CredentialsManager'); + } + /** * Returns an instance of the db facade * @deprecated use getDatabaseConnection, will be removed in ownCloud 10 diff --git a/lib/public/iservercontainer.php b/lib/public/iservercontainer.php index 1976f59f613..82084f021e8 100644 --- a/lib/public/iservercontainer.php +++ b/lib/public/iservercontainer.php @@ -180,6 +180,14 @@ interface IServerContainer { */ public function getSecureRandom(); + /** + * Returns a CredentialsManager instance + * + * @return \OCP\Security\ICredentialsManager + * @since 9.0.0 + */ + public function getCredentialsManager(); + /** * Returns an instance of the db facade * @deprecated 8.1.0 use getDatabaseConnection, will be removed in ownCloud 10 diff --git a/lib/public/security/icredentialsmanager.php b/lib/public/security/icredentialsmanager.php new file mode 100644 index 00000000000..d3d076f043e --- /dev/null +++ b/lib/public/security/icredentialsmanager.php @@ -0,0 +1,71 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCP\Security; + +/** + * Store and retrieve credentials for external services + * + * @package OCP\Security + * @since 8.2.0 + */ +interface ICredentialsManager { + + /** + * Store a set of credentials + * + * @param string|null $userId Null for system-wide credentials + * @param string $identifier + * @param mixed $credentials + * @since 8.2.0 + */ + public function store($userId, $identifier, $credentials); + + /** + * Retrieve a set of credentials + * + * @param string|null $userId Null for system-wide credentials + * @param string $identifier + * @return mixed + * @since 8.2.0 + */ + public function retrieve($userId, $identifier); + + /** + * Delete a set of credentials + * + * @param string|null $userId Null for system-wide credentials + * @param string $identifier + * @return int rows removed + * @since 8.2.0 + */ + public function delete($userId, $identifier); + + /** + * Erase all credentials stored for a user + * + * @param string $userId + * @return int rows removed + * @since 8.2.0 + */ + public function erase($userId); + +} diff --git a/tests/lib/security/credentialsmanager.php b/tests/lib/security/credentialsmanager.php new file mode 100644 index 00000000000..ed8f437e0ac --- /dev/null +++ b/tests/lib/security/credentialsmanager.php @@ -0,0 +1,90 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +use \OCP\Security\ICrypto; +use \OCP\IDBConnection; +use \OC\Security\CredentialsManager; + +class CredentialsManagerTest extends \Test\TestCase { + + /** @var ICrypto */ + protected $crypto; + + /** @var IDBConnection */ + protected $dbConnection; + + /** @var CredentialsManager */ + protected $manager; + + protected function setUp() { + parent::setUp(); + $this->crypto = $this->getMock('\OCP\Security\ICrypto'); + $this->dbConnection = $this->getMockBuilder('\OC\DB\Connection') + ->disableOriginalConstructor() + ->getMock(); + $this->manager = new CredentialsManager($this->crypto, $this->dbConnection); + } + + public function testStore() { + $userId = 'abc'; + $identifier = 'foo'; + $credentials = 'bar'; + + $this->crypto->expects($this->once()) + ->method('encrypt') + ->with(json_encode($credentials)) + ->willReturn('baz'); + + $this->dbConnection->expects($this->once()) + ->method('setValues') + ->with(CredentialsManager::DB_TABLE, + ['user' => $userId, 'identifier' => $identifier], + ['credentials' => 'baz'] + ); + + $this->manager->store($userId, $identifier, $credentials); + } + + public function testRetrieve() { + $userId = 'abc'; + $identifier = 'foo'; + + $this->crypto->expects($this->once()) + ->method('decrypt') + ->with('baz') + ->willReturn(json_encode('bar')); + + $qb = $this->getMockBuilder('\OC\DB\QueryBuilder\QueryBuilder') + ->setConstructorArgs([$this->dbConnection]) + ->setMethods(['execute']) + ->getMock(); + $qb->expects($this->once()) + ->method('execute') + ->willReturn(['credentials' => 'baz']); + + $this->dbConnection->expects($this->once()) + ->method('getQueryBuilder') + ->willReturn($qb); + + $this->manager->retrieve($userId, $identifier); + } + +} From 3fe802d93112d7d3642807c3a83ec474b4b5c404 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Mon, 24 Aug 2015 16:32:44 +0100 Subject: [PATCH 3/9] Introduce 'login credentials' auth mechanism Stores user credentials in the database after user login, uses the new CredentialsManager class --- apps/files_external/appinfo/application.php | 1 + .../lib/auth/password/logincredentials.php | 92 +++++++++++++++++++ .../lib/auth/password/sessioncredentials.php | 3 +- .../files_external/lib/auth/publickey/rsa.php | 3 +- apps/files_external/lib/backend/smb.php | 4 +- apps/files_external/lib/backend/smb_oc.php | 3 +- .../lib/config/configadapter.php | 4 +- .../lib/storagemodifiertrait.php | 4 +- 8 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 apps/files_external/lib/auth/password/logincredentials.php diff --git a/apps/files_external/appinfo/application.php b/apps/files_external/appinfo/application.php index 0c8b90935d3..86c58a45d5c 100644 --- a/apps/files_external/appinfo/application.php +++ b/apps/files_external/appinfo/application.php @@ -103,6 +103,7 @@ class Application extends App { // AuthMechanism::SCHEME_PASSWORD mechanisms $container->query('OCA\Files_External\Lib\Auth\Password\Password'), $container->query('OCA\Files_External\Lib\Auth\Password\SessionCredentials'), + $container->query('OCA\Files_External\Lib\Auth\Password\LoginCredentials'), // AuthMechanism::SCHEME_OAUTH1 mechanisms $container->query('OCA\Files_External\Lib\Auth\OAuth1\OAuth1'), diff --git a/apps/files_external/lib/auth/password/logincredentials.php b/apps/files_external/lib/auth/password/logincredentials.php new file mode 100644 index 00000000000..99cac3f4202 --- /dev/null +++ b/apps/files_external/lib/auth/password/logincredentials.php @@ -0,0 +1,92 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\Files_External\Lib\Auth\Password; + +use \OCP\IL10N; +use \OCP\IUser; +use \OCA\Files_External\Lib\DefinitionParameter; +use \OCA\Files_External\Lib\Auth\AuthMechanism; +use \OCA\Files_External\Lib\StorageConfig; +use \OCP\ISession; +use \OCP\Security\ICredentialsManager; +use \OCP\Files\Storage; +use \OCA\Files_External\Lib\SessionStorageWrapper; +use \OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; + +/** + * Username and password from login credentials, saved in DB + */ +class LoginCredentials extends AuthMechanism { + + const CREDENTIALS_IDENTIFIER = 'password::logincredentials/credentials'; + + /** @var ISession */ + protected $session; + + /** @var ICredentialsManager */ + protected $credentialsManager; + + public function __construct(IL10N $l, ISession $session, ICredentialsManager $credentialsManager) { + $this->session = $session; + $this->credentialsManager = $credentialsManager; + + $this + ->setIdentifier('password::logincredentials') + ->setScheme(self::SCHEME_PASSWORD) + ->setText($l->t('Login credentials')) + ->addParameters([ + ]) + ; + + \OCP\Util::connectHook('OC_User', 'post_login', $this, 'authenticate'); + } + + /** + * Hook listener on post login + * + * @param array $params + */ + public function authenticate(array $params) { + $userId = $params['uid']; + $credentials = [ + 'user' => $this->session->get('loginname'), + 'password' => $params['password'] + ]; + $this->credentialsManager->store($userId, self::CREDENTIALS_IDENTIFIER, $credentials); + } + + public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) { + if (!isset($user)) { + throw new InsufficientDataForMeaningfulAnswerException('No login credentials saved'); + } + $uid = $user->getUID(); + $credentials = $this->credentialsManager->retrieve($uid, self::CREDENTIALS_IDENTIFIER); + + if (!isset($credentials)) { + throw new InsufficientDataForMeaningfulAnswerException('No login credentials saved'); + } + + $storage->setBackendOption('user', $credentials['user']); + $storage->setBackendOption('password', $credentials['password']); + } + +} diff --git a/apps/files_external/lib/auth/password/sessioncredentials.php b/apps/files_external/lib/auth/password/sessioncredentials.php index 4f7d24c2f60..3fb8b8526cc 100644 --- a/apps/files_external/lib/auth/password/sessioncredentials.php +++ b/apps/files_external/lib/auth/password/sessioncredentials.php @@ -21,6 +21,7 @@ namespace OCA\Files_External\Lib\Auth\Password; +use \OCP\IUser; use \OCP\IL10N; use \OCA\Files_External\Lib\DefinitionParameter; use \OCA\Files_External\Lib\Auth\AuthMechanism; @@ -66,7 +67,7 @@ class SessionCredentials extends AuthMechanism { $this->session->set('password::sessioncredentials/credentials', $this->crypto->encrypt(json_encode($params))); } - public function manipulateStorageConfig(StorageConfig &$storage) { + public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) { $encrypted = $this->session->get('password::sessioncredentials/credentials'); if (!isset($encrypted)) { throw new InsufficientDataForMeaningfulAnswerException('No session credentials saved'); diff --git a/apps/files_external/lib/auth/publickey/rsa.php b/apps/files_external/lib/auth/publickey/rsa.php index 131b3f36526..9045f6818f9 100644 --- a/apps/files_external/lib/auth/publickey/rsa.php +++ b/apps/files_external/lib/auth/publickey/rsa.php @@ -26,6 +26,7 @@ use \OCA\Files_External\Lib\DefinitionParameter; use \OCA\Files_External\Lib\Auth\AuthMechanism; use \OCA\Files_External\Lib\StorageConfig; use \OCP\IConfig; +use OCP\IUser; use \phpseclib\Crypt\RSA as RSACrypt; /** @@ -55,7 +56,7 @@ class RSA extends AuthMechanism { ; } - public function manipulateStorageConfig(StorageConfig &$storage) { + public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) { $auth = new RSACrypt(); $auth->setPassword($this->config->getSystemValue('secret', '')); if (!$auth->loadKey($storage->getBackendOption('private_key'))) { diff --git a/apps/files_external/lib/backend/smb.php b/apps/files_external/lib/backend/smb.php index aaf7658751f..9b71636936a 100644 --- a/apps/files_external/lib/backend/smb.php +++ b/apps/files_external/lib/backend/smb.php @@ -30,6 +30,7 @@ use \OCA\Files_External\Lib\StorageConfig; use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill; use \OCA\Files_External\Lib\Auth\Password\Password; +use OCP\IUser; class SMB extends Backend { @@ -56,8 +57,9 @@ class SMB extends Backend { /** * @param StorageConfig $storage + * @param IUser $user */ - public function manipulateStorageConfig(StorageConfig &$storage) { + public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) { $user = $storage->getBackendOption('user'); if ($domain = $storage->getBackendOption('domain')) { $storage->setBackendOption('user', $domain.'\\'.$user); diff --git a/apps/files_external/lib/backend/smb_oc.php b/apps/files_external/lib/backend/smb_oc.php index 57fdfc30ff3..ba38754ce5a 100644 --- a/apps/files_external/lib/backend/smb_oc.php +++ b/apps/files_external/lib/backend/smb_oc.php @@ -30,6 +30,7 @@ use \OCA\Files_External\Lib\Auth\Password\SessionCredentials; use \OCA\Files_External\Lib\StorageConfig; use \OCA\Files_External\Lib\LegacyDependencyCheckPolyfill; use \OCA\Files_External\Lib\Backend\SMB; +use OCP\IUser; /** * Deprecated SMB_OC class - use SMB with the password::sessioncredentials auth mechanism @@ -59,7 +60,7 @@ class SMB_OC extends Backend { ; } - public function manipulateStorageConfig(StorageConfig &$storage) { + public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) { $username_as_share = ($storage->getBackendOption('username_as_share') === true); if ($username_as_share) { diff --git a/apps/files_external/lib/config/configadapter.php b/apps/files_external/lib/config/configadapter.php index 0cd1381c815..2bf39bcaa4f 100644 --- a/apps/files_external/lib/config/configadapter.php +++ b/apps/files_external/lib/config/configadapter.php @@ -85,8 +85,8 @@ class ConfigAdapter implements IMountProvider { $storage->setBackendOption('objectstore', new $objectClass($objectStore)); } - $storage->getAuthMechanism()->manipulateStorageConfig($storage); - $storage->getBackend()->manipulateStorageConfig($storage); + $storage->getAuthMechanism()->manipulateStorageConfig($storage, $user); + $storage->getBackend()->manipulateStorageConfig($storage, $user); } /** diff --git a/apps/files_external/lib/storagemodifiertrait.php b/apps/files_external/lib/storagemodifiertrait.php index ec2b0a14ab1..30c2108feec 100644 --- a/apps/files_external/lib/storagemodifiertrait.php +++ b/apps/files_external/lib/storagemodifiertrait.php @@ -21,6 +21,7 @@ namespace OCA\Files_External\Lib; +use \OCP\IUser; use \OCP\Files\Storage; use \OCA\Files_External\Lib\StorageConfig; use \OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; @@ -45,10 +46,11 @@ trait StorageModifierTrait { * Modify a StorageConfig parameters * * @param StorageConfig $storage + * @param IUser $user User the storage is being used as * @throws InsufficientDataForMeaningfulAnswerException * @throws StorageNotAvailableException */ - public function manipulateStorageConfig(StorageConfig &$storage) { + public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) { } /** From 7e01f32c27726a50d0f7761dda70b4e34a5e421d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 5 Jan 2016 16:56:09 +0100 Subject: [PATCH 4/9] Pass the user when updating external storage status --- .../controller/storagescontroller.php | 16 +++++++++----- .../userglobalstoragescontroller.php | 22 ++++++++++++++++++- .../controller/userstoragescontroller.php | 19 +++++++++++++++- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/apps/files_external/controller/storagescontroller.php b/apps/files_external/controller/storagescontroller.php index 07e2e69f601..64b989f0c77 100644 --- a/apps/files_external/controller/storagescontroller.php +++ b/apps/files_external/controller/storagescontroller.php @@ -212,6 +212,15 @@ abstract class StoragesController extends Controller { return null; } + protected function manipulateStorageConfig(StorageConfig $storage) { + /** @var AuthMechanism */ + $authMechanism = $storage->getAuthMechanism(); + $authMechanism->manipulateStorageConfig($storage); + /** @var Backend */ + $backend = $storage->getBackend(); + $backend->manipulateStorageConfig($storage); + } + /** * Check whether the given storage is available / valid. * @@ -222,13 +231,10 @@ abstract class StoragesController extends Controller { */ protected function updateStorageStatus(StorageConfig &$storage) { try { - /** @var AuthMechanism */ - $authMechanism = $storage->getAuthMechanism(); - $authMechanism->manipulateStorageConfig($storage); + $this->manipulateStorageConfig($storage); + /** @var Backend */ $backend = $storage->getBackend(); - $backend->manipulateStorageConfig($storage); - // update status (can be time-consuming) $storage->setStatus( \OC_Mount_Config::getBackendStatus( diff --git a/apps/files_external/controller/userglobalstoragescontroller.php b/apps/files_external/controller/userglobalstoragescontroller.php index 5aea7875ed4..85e843a4235 100644 --- a/apps/files_external/controller/userglobalstoragescontroller.php +++ b/apps/files_external/controller/userglobalstoragescontroller.php @@ -21,6 +21,7 @@ namespace OCA\Files_External\Controller; +use OCA\Files_External\Lib\Auth\AuthMechanism; use \OCP\IRequest; use \OCP\IL10N; use \OCP\AppFramework\Http\DataResponse; @@ -30,11 +31,17 @@ use \OCA\Files_external\Service\UserGlobalStoragesService; use \OCA\Files_external\NotFoundException; use \OCA\Files_external\Lib\StorageConfig; use \OCA\Files_External\Lib\Backend\Backend; +use OCP\IUserSession; /** * User global storages controller */ class UserGlobalStoragesController extends StoragesController { + /** + * @var IUserSession + */ + private $userSession; + /** * Creates a new user global storages controller. * @@ -42,12 +49,14 @@ class UserGlobalStoragesController extends StoragesController { * @param IRequest $request request object * @param IL10N $l10n l10n service * @param UserGlobalStoragesService $userGlobalStoragesService storage service + * @param IUserSession $userSession */ public function __construct( $AppName, IRequest $request, IL10N $l10n, - UserGlobalStoragesService $userGlobalStoragesService + UserGlobalStoragesService $userGlobalStoragesService, + IUserSession $userSession ) { parent::__construct( $AppName, @@ -55,6 +64,7 @@ class UserGlobalStoragesController extends StoragesController { $l10n, $userGlobalStoragesService ); + $this->userSession = $userSession; } /** @@ -78,12 +88,22 @@ class UserGlobalStoragesController extends StoragesController { ); } + protected function manipulateStorageConfig(StorageConfig $storage) { + /** @var AuthMechanism */ + $authMechanism = $storage->getAuthMechanism(); + $authMechanism->manipulateStorageConfig($storage, $this->userSession->getUser()); + /** @var Backend */ + $backend = $storage->getBackend(); + $backend->manipulateStorageConfig($storage, $this->userSession->getUser()); + } + /** * Get an external storage entry. * * @param int $id storage id * @return DataResponse * + * @NoCSRFRequired * @NoAdminRequired */ public function show($id) { diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index 345e4bf652b..afc30de456b 100644 --- a/apps/files_external/controller/userstoragescontroller.php +++ b/apps/files_external/controller/userstoragescontroller.php @@ -23,6 +23,7 @@ namespace OCA\Files_External\Controller; +use OCA\Files_External\Lib\Auth\AuthMechanism; use \OCP\IConfig; use \OCP\IUserSession; use \OCP\IRequest; @@ -40,6 +41,11 @@ use \OCA\Files_External\Lib\Backend\Backend; * User storages controller */ class UserStoragesController extends StoragesController { + /** + * @var IUserSession + */ + private $userSession; + /** * Creates a new user storages controller. * @@ -52,7 +58,8 @@ class UserStoragesController extends StoragesController { $AppName, IRequest $request, IL10N $l10n, - UserStoragesService $userStoragesService + UserStoragesService $userStoragesService, + IUserSession $userSession ) { parent::__construct( $AppName, @@ -60,6 +67,16 @@ class UserStoragesController extends StoragesController { $l10n, $userStoragesService ); + $this->userSession = $userSession; + } + + protected function manipulateStorageConfig(StorageConfig $storage) { + /** @var AuthMechanism */ + $authMechanism = $storage->getAuthMechanism(); + $authMechanism->manipulateStorageConfig($storage, $this->userSession->getUser()); + /** @var Backend */ + $backend = $storage->getBackend(); + $backend->manipulateStorageConfig($storage, $this->userSession->getUser()); } /** From 7ba715d14418f8cb8703528f7373d76492410e38 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 Jan 2016 15:18:55 +0100 Subject: [PATCH 5/9] fix test --- .../controller/userglobalstoragescontroller.php | 1 - .../controller/userstoragescontroller.php | 1 + .../controller/userstoragescontrollertest.php | 3 ++- tests/lib/db/connection.php | 12 ++++++++---- tests/lib/security/credentialsmanager.php | 14 +++++++++++++- 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/apps/files_external/controller/userglobalstoragescontroller.php b/apps/files_external/controller/userglobalstoragescontroller.php index 85e843a4235..6d4548754df 100644 --- a/apps/files_external/controller/userglobalstoragescontroller.php +++ b/apps/files_external/controller/userglobalstoragescontroller.php @@ -103,7 +103,6 @@ class UserGlobalStoragesController extends StoragesController { * @param int $id storage id * @return DataResponse * - * @NoCSRFRequired * @NoAdminRequired */ public function show($id) { diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index afc30de456b..741e906dec1 100644 --- a/apps/files_external/controller/userstoragescontroller.php +++ b/apps/files_external/controller/userstoragescontroller.php @@ -53,6 +53,7 @@ class UserStoragesController extends StoragesController { * @param IRequest $request request object * @param IL10N $l10n l10n service * @param UserStoragesService $userStoragesService storage service + * @param IUserSession $userSession */ public function __construct( $AppName, diff --git a/apps/files_external/tests/controller/userstoragescontrollertest.php b/apps/files_external/tests/controller/userstoragescontrollertest.php index dd761fa9767..671e019fea0 100644 --- a/apps/files_external/tests/controller/userstoragescontrollertest.php +++ b/apps/files_external/tests/controller/userstoragescontrollertest.php @@ -48,7 +48,8 @@ class UserStoragesControllerTest extends StoragesControllerTest { 'files_external', $this->getMock('\OCP\IRequest'), $this->getMock('\OCP\IL10N'), - $this->service + $this->service, + $this->getMock('\OCP\IUserSession') ); } diff --git a/tests/lib/db/connection.php b/tests/lib/db/connection.php index 51b4145b7a0..dd9b31f3ed7 100644 --- a/tests/lib/db/connection.php +++ b/tests/lib/db/connection.php @@ -105,7 +105,8 @@ class Connection extends \Test\TestCase { $this->connection->setValues('table', [ 'integerfield' => 1 ], [ - 'textfield' => 'foo' + 'textfield' => 'foo', + 'clobfield' => 'not_null' ]); $this->assertEquals('foo', $this->getTextValueByIntergerField(1)); @@ -118,7 +119,8 @@ class Connection extends \Test\TestCase { $this->connection->setValues('table', [ 'integerfield' => 1 ], [ - 'textfield' => 'foo' + 'textfield' => 'foo', + 'clobfield' => 'not_null' ]); $this->connection->setValues('table', [ @@ -138,7 +140,8 @@ class Connection extends \Test\TestCase { 'integerfield' => 1 ], [ 'textfield' => 'foo', - 'booleanfield' => true + 'booleanfield' => true, + 'clobfield' => 'not_null' ]); $this->connection->setValues('table', [ @@ -163,7 +166,8 @@ class Connection extends \Test\TestCase { 'integerfield' => 1 ], [ 'textfield' => 'foo', - 'booleanfield' => true + 'booleanfield' => true, + 'clobfield' => 'not_null' ]); $this->connection->setValues('table', [ diff --git a/tests/lib/security/credentialsmanager.php b/tests/lib/security/credentialsmanager.php index ed8f437e0ac..72f061e05bb 100644 --- a/tests/lib/security/credentialsmanager.php +++ b/tests/lib/security/credentialsmanager.php @@ -43,6 +43,18 @@ class CredentialsManagerTest extends \Test\TestCase { $this->manager = new CredentialsManager($this->crypto, $this->dbConnection); } + private function getQeuryResult($row) { + $result = $this->getMockBuilder('\Doctrine\DBAL\Driver\Statement') + ->disableOriginalConstructor() + ->getMock(); + + $result->expects($this->any()) + ->method('fetch') + ->will($this->returnValue($row)); + + return $result; + } + public function testStore() { $userId = 'abc'; $identifier = 'foo'; @@ -78,7 +90,7 @@ class CredentialsManagerTest extends \Test\TestCase { ->getMock(); $qb->expects($this->once()) ->method('execute') - ->willReturn(['credentials' => 'baz']); + ->willReturn($this->getQeuryResult(['credentials' => 'baz'])); $this->dbConnection->expects($this->once()) ->method('getQueryBuilder') From 895fd49fb26da4c00ab8c64a45d58dae41fd7962 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 Jan 2016 16:13:03 +0100 Subject: [PATCH 6/9] also handle not null violations --- lib/private/db/connection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/db/connection.php b/lib/private/db/connection.php index b1c1c12cb49..2d7545df36e 100644 --- a/lib/private/db/connection.php +++ b/lib/private/db/connection.php @@ -263,7 +263,7 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { }, array_merge($keys, $values)) ); return $insertQb->execute(); - } catch (\Doctrine\DBAL\Exception\UniqueConstraintViolationException $e) { + } catch (\Doctrine\DBAL\Exception\ConstraintViolationException $e) { // value already exists, try update $updateQb = $this->getQueryBuilder(); $updateQb->update($table); From ebd15fd5edbce1c173af885e61f9a1fbd1b4e33e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 Jan 2016 16:47:17 +0100 Subject: [PATCH 7/9] handle bool in setValue --- lib/private/db/connection.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/private/db/connection.php b/lib/private/db/connection.php index 2d7545df36e..a961c84f372 100644 --- a/lib/private/db/connection.php +++ b/lib/private/db/connection.php @@ -242,6 +242,16 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { return $this->adapter->insertIfNotExist($table, $input, $compare); } + private function getType($value) { + if (is_bool($value)) { + return \PDO::PARAM_BOOL; + } else if (is_int($value)) { + return \PDO::PARAM_INT; + } else { + return \PDO::PARAM_STR; + } + } + /** * Insert or update a row value * @@ -259,7 +269,7 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { $insertQb->insert($table) ->values( array_map(function($value) use ($insertQb) { - return $insertQb->createNamedParameter($value); + return $insertQb->createNamedParameter($value, $this->getType($value)); }, array_merge($keys, $values)) ); return $insertQb->execute(); @@ -268,13 +278,13 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { $updateQb = $this->getQueryBuilder(); $updateQb->update($table); foreach ($values as $name => $value) { - $updateQb->set($name, $updateQb->createNamedParameter($value)); + $updateQb->set($name, $updateQb->createNamedParameter($value), $this->getType($value)); } $where = $updateQb->expr()->andx(); $whereValues = array_merge($keys, $updatePreconditionValues); foreach ($whereValues as $name => $value) { $where->add($updateQb->expr()->eq( - $name, $updateQb->createNamedParameter($value) + $name, $updateQb->createNamedParameter($value, $this->getType($value)) )); } $updateQb->where($where); From 15451b29dabe2cb05f8df5aaa3ac6dde9ec6c140 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 18 Jan 2016 12:09:44 +0100 Subject: [PATCH 8/9] bumb app version --- apps/files_external/appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/appinfo/info.xml b/apps/files_external/appinfo/info.xml index 1a9fa73de3f..1cd4f602075 100644 --- a/apps/files_external/appinfo/info.xml +++ b/apps/files_external/appinfo/info.xml @@ -13,7 +13,7 @@ admin-external-storage false - 0.5.1 + 0.5.2 From 58afddfaa585fdb9efb34c01d1a5fa6282ed2bd1 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 18 Jan 2016 16:03:41 +0100 Subject: [PATCH 9/9] allow comparing clob using expressionbuilder->eq if you explicitly say you're comparing strings --- lib/private/db/connection.php | 4 ++- .../db/querybuilder/expressionbuilder.php | 8 +++-- .../db/querybuilder/ociexpressionbuilder.php | 33 +++++++++++++++++++ lib/private/db/querybuilder/querybuilder.php | 7 +++- .../db/querybuilder/iexpressionbuilder.php | 4 ++- 5 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 lib/private/db/querybuilder/ociexpressionbuilder.php diff --git a/lib/private/db/connection.php b/lib/private/db/connection.php index a961c84f372..6c4f518dfb5 100644 --- a/lib/private/db/connection.php +++ b/lib/private/db/connection.php @@ -284,7 +284,9 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { $whereValues = array_merge($keys, $updatePreconditionValues); foreach ($whereValues as $name => $value) { $where->add($updateQb->expr()->eq( - $name, $updateQb->createNamedParameter($value, $this->getType($value)) + $name, + $updateQb->createNamedParameter($value, $this->getType($value)), + $this->getType($value) )); } $updateQb->where($where); diff --git a/lib/private/db/querybuilder/expressionbuilder.php b/lib/private/db/querybuilder/expressionbuilder.php index de10f69b361..1e86db5a081 100644 --- a/lib/private/db/querybuilder/expressionbuilder.php +++ b/lib/private/db/querybuilder/expressionbuilder.php @@ -27,10 +27,10 @@ use OCP\IDBConnection; class ExpressionBuilder implements IExpressionBuilder { /** @var \Doctrine\DBAL\Query\Expression\ExpressionBuilder */ - private $expressionBuilder; + protected $expressionBuilder; /** @var QuoteHelper */ - private $helper; + protected $helper; /** * Initializes a new ExpressionBuilder. @@ -109,10 +109,12 @@ class ExpressionBuilder implements IExpressionBuilder { * * @param mixed $x The left expression. * @param mixed $y The right expression. + * @param int|null $type one of the \PDO::PARAM_* constants + * required when comparing text fields for oci compatibility * * @return string */ - public function eq($x, $y) { + public function eq($x, $y, $type = null) { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); return $this->expressionBuilder->eq($x, $y); diff --git a/lib/private/db/querybuilder/ociexpressionbuilder.php b/lib/private/db/querybuilder/ociexpressionbuilder.php new file mode 100644 index 00000000000..742611bf719 --- /dev/null +++ b/lib/private/db/querybuilder/ociexpressionbuilder.php @@ -0,0 +1,33 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\DB\QueryBuilder; + +class OCIExpressionBuilder extends ExpressionBuilder { + public function eq($x, $y, $type = null) { + $x = $this->helper->quoteColumnName($x); + $y = $this->helper->quoteColumnName($y); + if ($type === \PDO::PARAM_STR) { + $x = new QueryFunction('to_char(' . $x . ')'); + } + return $this->expressionBuilder->eq($x, $y); + } +} diff --git a/lib/private/db/querybuilder/querybuilder.php b/lib/private/db/querybuilder/querybuilder.php index 492e9bc9abf..42b290b90e7 100644 --- a/lib/private/db/querybuilder/querybuilder.php +++ b/lib/private/db/querybuilder/querybuilder.php @@ -21,6 +21,7 @@ namespace OC\DB\QueryBuilder; +use OC\DB\OracleConnection; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\QueryBuilder\IQueryFunction; use OCP\DB\QueryBuilder\IParameter; @@ -82,7 +83,11 @@ class QueryBuilder implements IQueryBuilder { * @return \OCP\DB\QueryBuilder\IExpressionBuilder */ public function expr() { - return new ExpressionBuilder($this->connection); + if ($this->connection instanceof OracleConnection) { + return new OCIExpressionBuilder($this->connection); + } else { + return new ExpressionBuilder($this->connection); + } } /** diff --git a/lib/public/db/querybuilder/iexpressionbuilder.php b/lib/public/db/querybuilder/iexpressionbuilder.php index ae62694fcaf..0549d3f0125 100644 --- a/lib/public/db/querybuilder/iexpressionbuilder.php +++ b/lib/public/db/querybuilder/iexpressionbuilder.php @@ -84,11 +84,13 @@ interface IExpressionBuilder { * * @param mixed $x The left expression. * @param mixed $y The right expression. + * @param int|null $type @since 9.0.0 one of the \PDO::PARAM_* constants + * required when comparing text fields for oci compatibility. * * @return string * @since 8.2.0 */ - public function eq($x, $y); + public function eq($x, $y, $type = null); /** * Creates a non equality comparison expression with the given arguments.