diff options
author | Thomas Müller <thomas.mueller@tmit.eu> | 2016-01-22 13:14:14 +0100 |
---|---|---|
committer | Thomas Müller <thomas.mueller@tmit.eu> | 2016-01-22 13:14:14 +0100 |
commit | 9b4c9a0357ba9a10f4e0c7c1cafb3923ba5929db (patch) | |
tree | eb469af5c63d8df131d6e7fc00bcf42f6d4b75eb | |
parent | 0bccdbc959b0b7bbce2ebdd62b6b44121e1e0b61 (diff) | |
parent | 58afddfaa585fdb9efb34c01d1a5fa6282ed2bd1 (diff) | |
download | nextcloud-server-9b4c9a0357ba9a10f4e0c7c1cafb3923ba5929db.tar.gz nextcloud-server-9b4c9a0357ba9a10f4e0c7c1cafb3923ba5929db.zip |
Merge pull request #18531 from owncloud/ext-user-credentials
External storage 'Login credentials' auth mechanism
30 files changed, 780 insertions, 89 deletions
diff --git a/apps/files_external/appinfo/application.php b/apps/files_external/appinfo/application.php index c755b6a29b0..1571178596b 100644 --- a/apps/files_external/appinfo/application.php +++ b/apps/files_external/appinfo/application.php @@ -108,6 +108,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/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>admin-external-storage</admin> </documentation> <rememberlogin>false</rememberlogin> - <version>0.5.1</version> + <version>0.5.2</version> <types> <filesystem/> </types> 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..6d4548754df 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,24 +31,32 @@ 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. * * @param string $AppName application name * @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,6 +88,15 @@ 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. * diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index 345e4bf652b..741e906dec1 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; @@ -41,18 +42,25 @@ use \OCA\Files_External\Lib\Backend\Backend; */ class UserStoragesController extends StoragesController { /** + * @var IUserSession + */ + private $userSession; + + /** * Creates a new user storages controller. * * @param string $AppName application name * @param IRequest $request request object * @param IL10N $l10n l10n service * @param UserStoragesService $userStoragesService storage service + * @param IUserSession $userSession */ public function __construct( $AppName, IRequest $request, IL10N $l10n, - UserStoragesService $userStoragesService + UserStoragesService $userStoragesService, + IUserSession $userSession ) { parent::__construct( $AppName, @@ -60,6 +68,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()); } /** 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 @@ +<?php +/** + * @author Robin McCorkell <rmccorkell@owncloud.com> + * + * @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 <http://www.gnu.org/licenses/> + * + */ + +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) { } /** 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/db_structure.xml b/db_structure.xml index 058322f78a3..99bfa519b40 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -1580,5 +1580,60 @@ </table> + <table> + <!-- + Encrypted credentials storage + --> + <name>*dbprefix*credentials</name> + + <declaration> + + <field> + <name>user</name> + <type>text</type> + <default></default> + <notnull>false</notnull> + <length>64</length> + </field> + + <field> + <name>identifier</name> + <type>text</type> + <notnull>true</notnull> + <length>64</length> + </field> + + <field> + <name>credentials</name> + <type>clob</type> + <notnull>false</notnull> + </field> + + <index> + <name>credentials_user_id</name> + <primary>true</primary> + <unique>true</unique> + <field> + <name>user</name> + <sorting>ascending</sorting> + </field> + <field> + <name>identifier</name> + <sorting>ascending</sorting> + </field> + </index> + + <index> + <name>credentials_user</name> + <unique>false</unique> + <field> + <name>user</name> + <sorting>ascending</sorting> + </field> + </index> + + </declaration> + + </table> </database> 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 @@ -147,6 +147,21 @@ class Db implements IDb { } /** + * 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 */ public function beginTransaction() { 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/db/connection.php b/lib/private/db/connection.php index 28bf3b6e05b..6c4f518dfb5 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,64 @@ 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 + * + * @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, $this->getType($value)); + }, array_merge($keys, $values)) + ); + return $insertQb->execute(); + } catch (\Doctrine\DBAL\Exception\ConstraintViolationException $e) { + // value already exists, try update + $updateQb = $this->getQueryBuilder(); + $updateQb->update($table); + foreach ($values as $name => $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, $this->getType($value)), + $this->getType($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/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 <tt>ExpressionBuilder</tt>. @@ -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 @@ +<?php +/** + * @author Joas Schilling <nickvergessen@owncloud.com> + * + * @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 <http://www.gnu.org/licenses/> + * + */ + +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/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 @@ +<?php +/** + * @author Robin McCorkell <rmccorkell@owncloud.com> + * + * @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 <http://www.gnu.org/licenses/> + * + */ + +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 69f403308b3..2497c056600 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -65,6 +65,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; @@ -345,6 +346,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(); @@ -940,6 +944,15 @@ class Server extends ServerContainer implements IServerContainer { } /** + * 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/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. 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 @@ -109,6 +109,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/lib/public/iservercontainer.php b/lib/public/iservercontainer.php index 1976f59f613..82084f021e8 100644 --- a/lib/public/iservercontainer.php +++ b/lib/public/iservercontainer.php @@ -181,6 +181,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 * @return \OCP\IDb 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 @@ +<?php +/** + * @author Robin McCorkell <rmccorkell@owncloud.com> + * + * @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 <http://www.gnu.org/licenses/> + * + */ + +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/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..dd9b31f3ed7 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,93 @@ 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', + 'clobfield' => 'not_null' + ]); + + $this->assertEquals('foo', $this->getTextValueByIntergerField(1)); + + $this->connection->dropTable('table'); + } + + public function testSetValuesOverWrite() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo', + 'clobfield' => 'not_null' + ]); + + $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, + 'clobfield' => 'not_null' + ]); + + $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, + 'clobfield' => 'not_null' + ]); + + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'bar' + ], [ + 'booleanfield' => false + ]); + } } diff --git a/tests/lib/security/credentialsmanager.php b/tests/lib/security/credentialsmanager.php new file mode 100644 index 00000000000..72f061e05bb --- /dev/null +++ b/tests/lib/security/credentialsmanager.php @@ -0,0 +1,102 @@ +<?php +/** + * @author Robin McCorkell <rmccorkell@owncloud.com> + * + * @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 <http://www.gnu.org/licenses/> + * + */ + +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); + } + + 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'; + $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($this->getQeuryResult(['credentials' => 'baz'])); + + $this->dbConnection->expects($this->once()) + ->method('getQueryBuilder') + ->willReturn($qb); + + $this->manager->retrieve($userId, $identifier); + } + +} |