From adbc5a0b41be84f741f8689fda98b9f89cfdf5f6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Jan 2016 12:04:34 +0100 Subject: [PATCH 01/10] Fix invalid storages not showing in directory listing --- apps/files_external/lib/failedcache.php | 126 ++++++++++++++++++++++ apps/files_external/lib/failedstorage.php | 5 +- 2 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 apps/files_external/lib/failedcache.php diff --git a/apps/files_external/lib/failedcache.php b/apps/files_external/lib/failedcache.php new file mode 100644 index 00000000000..e646e4fe677 --- /dev/null +++ b/apps/files_external/lib/failedcache.php @@ -0,0 +1,126 @@ + + * + * @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 OCA\Files_External\Lib; + +use OC\Files\Cache\CacheEntry; +use OCP\Files\Cache\ICache; + +/** + * Storage placeholder to represent a missing precondition, storage unavailable + */ +class FailedCache implements ICache { + + public function getNumericStorageId() { + return -1; + } + + public function get($file) { + if ($file === '') { + return new CacheEntry([ + 'fileid' => -1, + 'size' => 0, + 'mimetype' => 'httpd/unix-directory', + 'mimepart' => 'httpd', + 'permissions' => 0, + 'mtime' => time() + ]); + } else { + return false; + } + } + + public function getFolderContents($folder) { + return []; + } + + public function getFolderContentsById($fileId) { + return []; + } + + public function put($file, array $data) { + return; + } + + public function update($id, array $data) { + return; + } + + public function getId($file) { + return -1; + } + + public function getParentId($file) { + return -1; + } + + public function inCache($file) { + return false; + } + + public function remove($file) { + return; + } + + public function move($source, $target) { + return; + } + + public function moveFromCache(ICache $sourceCache, $sourcePath, $targetPath) { + return; + } + + public function clear() { + return; + } + + public function getStatus($file) { + return ICache::NOT_FOUND; + } + + public function search($pattern) { + return []; + } + + public function searchByMime($mimetype) { + return []; + } + + public function searchByTag($tag, $userId) { + return []; + } + + public function getAll() { + return []; + } + + public function getIncomplete() { + return []; + } + + public function getPathById($id) { + return null; + } + + public function normalize($path) { + return $path; + } +} diff --git a/apps/files_external/lib/failedstorage.php b/apps/files_external/lib/failedstorage.php index 95fe179f570..7bcbfc31902 100644 --- a/apps/files_external/lib/failedstorage.php +++ b/apps/files_external/lib/failedstorage.php @@ -174,7 +174,7 @@ class FailedStorage extends Common { } public function verifyPath($path, $fileName) { - throw new StorageNotAvailableException($this->e->getMessage(), $this->e->getCode(), $this->e); + return true; } public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { @@ -205,4 +205,7 @@ class FailedStorage extends Common { throw new StorageNotAvailableException($this->e->getMessage(), $this->e->getCode(), $this->e); } + public function getCache($path = '', $storage = null) { + return new FailedCache(); + } } From 419507c1181e8b9685ebaa4fee8973c62967ac6e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Jan 2016 12:15:20 +0100 Subject: [PATCH 02/10] Add user provided credentials mechanism --- apps/files_external/appinfo/application.php | 1 + apps/files_external/appinfo/routes.php | 7 +- .../controller/usercredentialscontroller.php | 57 ++++++++++++++ .../lib/auth/password/userprovided.php | 78 +++++++++++++++++++ 4 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 apps/files_external/controller/usercredentialscontroller.php create mode 100644 apps/files_external/lib/auth/password/userprovided.php diff --git a/apps/files_external/appinfo/application.php b/apps/files_external/appinfo/application.php index 1571178596b..1bf258c48b4 100644 --- a/apps/files_external/appinfo/application.php +++ b/apps/files_external/appinfo/application.php @@ -109,6 +109,7 @@ class Application extends App { $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'), + $container->query('OCA\Files_External\Lib\Auth\Password\UserProvided'), // AuthMechanism::SCHEME_OAUTH1 mechanisms $container->query('OCA\Files_External\Lib\Auth\OAuth1\OAuth1'), diff --git a/apps/files_external/appinfo/routes.php b/apps/files_external/appinfo/routes.php index c3149a300cf..b30ad0a8542 100644 --- a/apps/files_external/appinfo/routes.php +++ b/apps/files_external/appinfo/routes.php @@ -44,7 +44,12 @@ namespace OCA\Files_External\AppInfo; 'url' => '/ajax/public_key.php', 'verb' => 'POST', 'requirements' => array() - ) + ), + [ + 'name' => 'UserCredentials#store', + 'url' => '/usercredentials/{storageId}', + 'verb' => 'PUT' + ] ) ) ); diff --git a/apps/files_external/controller/usercredentialscontroller.php b/apps/files_external/controller/usercredentialscontroller.php new file mode 100644 index 00000000000..2944611d8a9 --- /dev/null +++ b/apps/files_external/controller/usercredentialscontroller.php @@ -0,0 +1,57 @@ + + * + * @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\Controller; + +use OCA\Files_External\Lib\Auth\Password\UserProvided; +use OCP\AppFramework\Controller; +use OCP\IRequest; +use OCP\IUserSession; + +class UserCredentialsController extends Controller { + /** + * @var UserProvided + */ + private $authMechanism; + + /** + * @var IUserSession + */ + private $userSession; + + public function __construct($appName, IRequest $request, UserProvided $authMechanism, IUserSession $userSession) { + parent::__construct($appName, $request); + $this->authMechanism = $authMechanism; + $this->userSession = $userSession; + } + + /** + * @param int $storageId + * @param string $username + * @param string $password + * + * @NoAdminRequired + * @NoCSRFRequired + */ + public function store($storageId, $username, $password) { + $this->authMechanism->saveCredentials($this->userSession->getUser(), $storageId, $username, $password); + } +} diff --git a/apps/files_external/lib/auth/password/userprovided.php b/apps/files_external/lib/auth/password/userprovided.php new file mode 100644 index 00000000000..8854513e161 --- /dev/null +++ b/apps/files_external/lib/auth/password/userprovided.php @@ -0,0 +1,78 @@ + + * + * @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\Auth\AuthMechanism; +use OCA\Files_External\Lib\StorageConfig; +use OCP\Security\ICredentialsManager; +use OCP\Files\Storage; +use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; + +/** + * User provided Username and Password + */ +class UserProvided extends AuthMechanism { + + const CREDENTIALS_IDENTIFIER_PREFIX = 'password::userprovided/'; + + /** @var ICredentialsManager */ + protected $credentialsManager; + + public function __construct(IL10N $l, ICredentialsManager $credentialsManager) { + $this->credentialsManager = $credentialsManager; + + $this + ->setIdentifier('password::userprovided') + ->setScheme(self::SCHEME_PASSWORD) + ->setText($l->t('User provided')) + ->addParameters([]); + } + + private function getCredentialsIdentifier($storageId) { + return self::CREDENTIALS_IDENTIFIER_PREFIX . $storageId; + } + + public function saveCredentials(IUser $user, $id, $username, $password) { + $this->credentialsManager->store($user->getUID(), $this->getCredentialsIdentifier($id), [ + 'user' => $username, + 'password' => $password + ]); + } + + public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) { + if (!isset($user)) { + throw new InsufficientDataForMeaningfulAnswerException('No credentials saved'); + } + $uid = $user->getUID(); + $credentials = $this->credentialsManager->retrieve($uid, $this->getCredentialsIdentifier($storage->getId())); + + if (!isset($credentials)) { + throw new InsufficientDataForMeaningfulAnswerException('No credentials saved'); + } + + $storage->setBackendOption('user', $credentials['user']); + $storage->setBackendOption('password', $credentials['password']); + } + +} From f3e9729a5f68fa36ee6633955b7913f37e1c890e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Jan 2016 12:18:09 +0100 Subject: [PATCH 03/10] expose user provided credentials for admin mounts --- .../controller/usercredentialscontroller.php | 1 - .../controller/userglobalstoragescontroller.php | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/files_external/controller/usercredentialscontroller.php b/apps/files_external/controller/usercredentialscontroller.php index 2944611d8a9..bc514cba088 100644 --- a/apps/files_external/controller/usercredentialscontroller.php +++ b/apps/files_external/controller/usercredentialscontroller.php @@ -49,7 +49,6 @@ class UserCredentialsController extends Controller { * @param string $password * * @NoAdminRequired - * @NoCSRFRequired */ public function store($storageId, $username, $password) { $this->authMechanism->saveCredentials($this->userSession->getUser(), $storageId, $username, $password); diff --git a/apps/files_external/controller/userglobalstoragescontroller.php b/apps/files_external/controller/userglobalstoragescontroller.php index 6d4548754df..97b5c90e20c 100644 --- a/apps/files_external/controller/userglobalstoragescontroller.php +++ b/apps/files_external/controller/userglobalstoragescontroller.php @@ -22,6 +22,8 @@ namespace OCA\Files_External\Controller; use OCA\Files_External\Lib\Auth\AuthMechanism; +use OCA\Files_External\Lib\Auth\Password\UserProvided; +use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; use \OCP\IRequest; use \OCP\IL10N; use \OCP\AppFramework\Http\DataResponse; @@ -135,6 +137,14 @@ class UserGlobalStoragesController extends StoragesController { protected function sanitizeStorage(StorageConfig $storage) { $storage->setBackendOptions([]); $storage->setMountOptions([]); + + if ($storage->getAuthMechanism() instanceof UserProvided) { + try { + $storage->getAuthMechanism()->manipulateStorageConfig($storage, $this->userSession->getUser()); + } catch (InsufficientDataForMeaningfulAnswerException $e) { + + } + } } } From 860d51487b103f4c050d2427c35ee70dd4b2b05e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Jan 2016 14:16:11 +0100 Subject: [PATCH 04/10] Allow setting user provided credentials from the personal settings page --- .../controller/storagescontroller.php | 2 + .../controller/usercredentialscontroller.php | 49 +++++++- .../controller/userstoragescontroller.php | 1 + apps/files_external/js/settings.js | 115 +++++++++++++++--- .../lib/auth/password/userprovided.php | 9 +- .../lib/definitionparameter.php | 48 ++++---- 6 files changed, 180 insertions(+), 44 deletions(-) diff --git a/apps/files_external/controller/storagescontroller.php b/apps/files_external/controller/storagescontroller.php index 64b989f0c77..b09774ef515 100644 --- a/apps/files_external/controller/storagescontroller.php +++ b/apps/files_external/controller/storagescontroller.php @@ -25,6 +25,7 @@ namespace OCA\Files_External\Controller; use \OCP\IConfig; +use OCP\IUser; use \OCP\IUserSession; use \OCP\IRequest; use \OCP\IL10N; @@ -114,6 +115,7 @@ abstract class StoragesController extends Controller { $priority ); } catch (\InvalidArgumentException $e) { + \OC::$server->getLogger()->logException($e); return new DataResponse( [ 'message' => (string)$this->l10n->t('Invalid backend or authentication mechanism class') diff --git a/apps/files_external/controller/usercredentialscontroller.php b/apps/files_external/controller/usercredentialscontroller.php index bc514cba088..5153189d9c3 100644 --- a/apps/files_external/controller/usercredentialscontroller.php +++ b/apps/files_external/controller/usercredentialscontroller.php @@ -21,12 +21,19 @@ namespace OCA\Files_External\Controller; +use OCA\Calendar\Sabre\Backend; +use OCA\Files_External\Lib\Auth\AuthMechanism; use OCA\Files_External\Lib\Auth\Password\UserProvided; +use OCA\Files_external\Lib\StorageConfig; +use OCA\Files_External\Service\UserGlobalStoragesService; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; +use OCP\IL10N; use OCP\IRequest; use OCP\IUserSession; -class UserCredentialsController extends Controller { +class UserCredentialsController extends StoragesController { /** * @var UserProvided */ @@ -37,10 +44,22 @@ class UserCredentialsController extends Controller { */ private $userSession; - public function __construct($appName, IRequest $request, UserProvided $authMechanism, IUserSession $userSession) { - parent::__construct($appName, $request); + /** + * @var UserGlobalStoragesService + */ + private $globalStoragesService; + + public function __construct( + $appName, IRequest $request, + UserProvided $authMechanism, + IUserSession $userSession, + IL10N $l10n, + UserGlobalStoragesService $globalStoragesService + ) { + parent::__construct($appName, $request, $l10n, $globalStoragesService); $this->authMechanism = $authMechanism; $this->userSession = $userSession; + $this->globalStoragesService = $globalStoragesService; } /** @@ -49,8 +68,32 @@ class UserCredentialsController extends Controller { * @param string $password * * @NoAdminRequired + * @return DataResponse */ public function store($storageId, $username, $password) { $this->authMechanism->saveCredentials($this->userSession->getUser(), $storageId, $username, $password); + + $storage = $this->globalStoragesService->getStorage($storageId); + + $this->updateStorageStatus($storage); + + $storage->setBackendOptions([]); + $storage->setMountOptions([]); + $this->manipulateStorageConfig($storage); + + + return new DataResponse( + $storage, + Http::STATUS_OK + ); + } + + 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/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index 741e906dec1..ccf8bc24e09 100644 --- a/apps/files_external/controller/userstoragescontroller.php +++ b/apps/files_external/controller/userstoragescontroller.php @@ -25,6 +25,7 @@ namespace OCA\Files_External\Controller; use OCA\Files_External\Lib\Auth\AuthMechanism; use \OCP\IConfig; +use OCP\IUser; use \OCP\IUserSession; use \OCP\IRequest; use \OCP\IL10N; diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 756804238d0..16576ce7087 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -357,6 +357,9 @@ StorageConfig.prototype = { if (this.mountPoint === '') { return false; } + if (!this.backend) { + return false; + } if (this.errors) { return false; } @@ -432,6 +435,48 @@ UserStorageConfig.prototype = _.extend({}, StorageConfig.prototype, _url: 'apps/files_external/userstorages' }); +/** + * @class OCA.External.Settings.UserGlobalStorageConfig + * @augments OCA.External.Settings.StorageConfig + * + * @classdesc User external storage config + */ +var UserGlobalStorageConfig = function (id) { + this.id = id; +}; +UserGlobalStorageConfig.prototype = _.extend({}, StorageConfig.prototype, + /** @lends OCA.External.Settings.UserStorageConfig.prototype */ { + + _url: 'apps/files_external/userglobalstorages', + + /** + * Creates or saves the storage. + * + * @param {Function} [options.success] success callback, receives result as argument + * @param {Function} [options.error] error callback + */ + save: function (options) { + var self = this; + var url = OC.generateUrl('apps/files_external/usercredentials/{id}', {id: this.id}); + + $.ajax({ + type: 'PUT', + url: url, + contentType: 'application/json', + data: JSON.stringify({ + username: this.backendOptions.user, + password: this.backendOptions.password + }), + success: function (result) { + if (_.isFunction(options.success)) { + options.success(result); + } + }, + error: options.error + }); + } +}); + /** * @class OCA.External.Settings.MountOptionsDropdown * @@ -748,7 +793,7 @@ MountConfigListView.prototype = _.extend({ $.each(authMechanismConfiguration['configuration'], _.partial( this.writeParameterInput, $td, _, _, ['auth-param'] - )); + ).bind(this)); this.trigger('selectAuthMechanism', $tr, authMechanism, authMechanismConfiguration['scheme'], onCompletion @@ -770,6 +815,7 @@ MountConfigListView.prototype = _.extend({ var $tr = this.$el.find('tr#addMountPoint'); this.$el.find('tbody').append($tr.clone()); + $tr.data('storageConfig', storageConfig); $tr.find('td').last().attr('class', 'remove'); $tr.find('td.mountOptionsToggle').removeClass('hidden'); $tr.find('td').last().removeAttr('style'); @@ -805,7 +851,7 @@ MountConfigListView.prototype = _.extend({ $tr.find('td.authentication').append(selectAuthMechanism); var $td = $tr.find('td.configuration'); - $.each(backend.configuration, _.partial(this.writeParameterInput, $td)); + $.each(backend.configuration, _.partial(this.writeParameterInput, $td).bind(this)); this.trigger('selectBackend', $tr, backend.identifier, onCompletion); this.configureAuthMechanism($tr, storageConfig.authMechanism, onCompletion); @@ -866,8 +912,14 @@ MountConfigListView.prototype = _.extend({ success: function(result) { var onCompletion = jQuery.Deferred(); $.each(result, function(i, storageParams) { + var storageConfig; + var isUserProvidedAuth = storageParams.authMechanism === 'password::userprovided'; storageParams.mountPoint = storageParams.mountPoint.substr(1); // trim leading slash - var storageConfig = new self._storageConfigClass(); + if (isUserProvidedAuth) { + storageConfig = new UserGlobalStorageConfig(); + } else { + storageConfig = new self._storageConfigClass(); + } _.extend(storageConfig, storageParams); var $tr = self.newStorage(storageConfig, onCompletion); @@ -878,12 +930,16 @@ MountConfigListView.prototype = _.extend({ var $authentication = $tr.find('.authentication'); $authentication.text($authentication.find('select option:selected').text()); - // userglobal storages do not expose configuration data - $tr.find('.configuration').text(t('files_external', 'Admin defined')); - // disable any other inputs $tr.find('.mountOptionsToggle, .remove').empty(); - $tr.find('input, select, button').attr('disabled', 'disabled'); + $tr.find('input:not(.user_provided), select:not(.user_provided)').attr('disabled', 'disabled'); + + if (isUserProvidedAuth) { + $tr.find('.configuration').find(':not(.user_provided)').remove(); + } else { + // userglobal storages do not expose configuration data + $tr.find('.configuration').text(t('files_external', 'Admin defined')); + } }); onCompletion.resolve(); } @@ -918,22 +974,40 @@ MountConfigListView.prototype = _.extend({ * @return {jQuery} newly created input */ writeParameterInput: function($td, parameter, placeholder, classes) { + var hasFlag = function(flag) { + return placeholder.indexOf(flag) !== -1; + }; classes = $.isArray(classes) ? classes : []; classes.push('added'); if (placeholder.indexOf('&') === 0) { classes.push('optional'); placeholder = placeholder.substring(1); } + + if (hasFlag('@')) { + if (this._isPersonal) { + classes.push('user_provided'); + } else { + return; + } + } + var newElement; - if (placeholder.indexOf('*') === 0) { - newElement = $(''); - } else if (placeholder.indexOf('!') === 0) { + + var trimmedPlaceholder = placeholder; + var flags = ['@', '*', '!', '#', '&']; + while(flags.indexOf(trimmedPlaceholder[0]) !== -1) { + trimmedPlaceholder = trimmedPlaceholder.substr(1); + } + if (hasFlag('*')) { + newElement = $(''); + } else if (hasFlag('!')) { var checkboxId = _.uniqueId('checkbox_'); - newElement = $(''); - } else if (placeholder.indexOf('#') === 0) { + newElement = $(''); + } else if (hasFlag('#')) { newElement = $(''); } else { - newElement = $(''); + newElement = $(''); } highlightInput(newElement); $td.append(newElement); @@ -952,7 +1026,12 @@ MountConfigListView.prototype = _.extend({ // new entry storageId = null; } - var storage = new this._storageConfigClass(storageId); + + var storage = $tr.data('storageConfig'); + if (!storage) { + storage = new this._storageConfigClass(storageId); + } + storage.errors = null; storage.mountPoint = $tr.find('.mountPoint input').val(); storage.backend = $tr.find('.backend').data('identifier'); storage.authMechanism = $tr.find('.selectAuthMechanism').val(); @@ -966,7 +1045,7 @@ MountConfigListView.prototype = _.extend({ if ($input.attr('type') === 'button') { return; } - if (!isInputValid($input)) { + if (!isInputValid($input) && !$input.hasClass('optional')) { missingOptions.push(parameter); return; } @@ -994,7 +1073,7 @@ MountConfigListView.prototype = _.extend({ var users = []; var multiselect = getSelection($tr); $.each(multiselect, function(index, value) { - var pos = value.indexOf('(group)'); + var pos = (value.indexOf)?value.indexOf('(group)'): -1; if (pos !== -1) { groups.push(value.substr(0, pos)); } else { @@ -1057,7 +1136,9 @@ MountConfigListView.prototype = _.extend({ saveStorageConfig:function($tr, callback, concurrentTimer) { var self = this; var storage = this.getStorageConfig($tr); - if (!storage.validate()) { + console.log(storage); + if (!storage || !storage.validate()) { + console.log('invalid'); return false; } diff --git a/apps/files_external/lib/auth/password/userprovided.php b/apps/files_external/lib/auth/password/userprovided.php index 8854513e161..b0ff50a279a 100644 --- a/apps/files_external/lib/auth/password/userprovided.php +++ b/apps/files_external/lib/auth/password/userprovided.php @@ -21,6 +21,7 @@ namespace OCA\Files_External\Lib\Auth\Password; +use OCA\Files_External\Lib\DefinitionParameter; use OCP\IL10N; use OCP\IUser; use OCA\Files_External\Lib\Auth\AuthMechanism; @@ -46,7 +47,13 @@ class UserProvided extends AuthMechanism { ->setIdentifier('password::userprovided') ->setScheme(self::SCHEME_PASSWORD) ->setText($l->t('User provided')) - ->addParameters([]); + ->addParameters([ + (new DefinitionParameter('user', $l->t('Username'))) + ->setFlag(DefinitionParameter::FLAG_USER_PROVIDED), + (new DefinitionParameter('password', $l->t('Password'))) + ->setType(DefinitionParameter::VALUE_PASSWORD) + ->setFlag(DefinitionParameter::FLAG_USER_PROVIDED), + ]); } private function getCredentialsIdentifier($storageId) { diff --git a/apps/files_external/lib/definitionparameter.php b/apps/files_external/lib/definitionparameter.php index 59a098e6320..85c3dd08ad4 100644 --- a/apps/files_external/lib/definitionparameter.php +++ b/apps/files_external/lib/definitionparameter.php @@ -35,6 +35,7 @@ class DefinitionParameter implements \JsonSerializable { /** Flag constants */ const FLAG_NONE = 0; const FLAG_OPTIONAL = 1; + const FLAG_USER_PROVIDED = 2; /** @var string name of parameter */ private $name; @@ -121,7 +122,7 @@ class DefinitionParameter implements \JsonSerializable { * @return bool */ public function isFlagSet($flag) { - return (bool) $this->flags & $flag; + return (bool)($this->flags & $flag); } /** @@ -143,10 +144,11 @@ class DefinitionParameter implements \JsonSerializable { break; } - switch ($this->getFlags()) { - case self::FLAG_OPTIONAL: - $prefix = '&' . $prefix; - break; + if ($this->isFlagSet(self::FLAG_OPTIONAL)) { + $prefix = '&' . $prefix; + } + if ($this->isFlagSet(self::FLAG_USER_PROVIDED)) { + $prefix = '@' . $prefix; } return $prefix . $this->getText(); @@ -160,28 +162,28 @@ class DefinitionParameter implements \JsonSerializable { * @return bool success */ public function validateValue(&$value) { - $optional = $this->getFlags() & self::FLAG_OPTIONAL; + $optional = $this->isFlagSet(self::FLAG_OPTIONAL) || $this->isFlagSet(self::FLAG_USER_PROVIDED); switch ($this->getType()) { - case self::VALUE_BOOLEAN: - if (!is_bool($value)) { - switch ($value) { - case 'true': - $value = true; - break; - case 'false': - $value = false; - break; - default: + case self::VALUE_BOOLEAN: + if (!is_bool($value)) { + switch ($value) { + case 'true': + $value = true; + break; + case 'false': + $value = false; + break; + default: + return false; + } + } + break; + default: + if (!$value && !$optional) { return false; } - } - break; - default: - if (!$value && !$optional) { - return false; - } - break; + break; } return true; } From 5bdcd534b18ce842c6775f588c9a7330a82ed89d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Jan 2016 14:21:59 +0100 Subject: [PATCH 05/10] dont show user provided auth as option for personal mounts --- apps/files_external/js/settings.js | 5 ++--- apps/files_external/lib/auth/authmechanism.php | 1 + apps/files_external/lib/auth/password/userprovided.php | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 16576ce7087..52b46db6cc0 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -836,8 +836,9 @@ MountConfigListView.prototype = _.extend({ $tr.find('.backend').data('identifier', backend.identifier); var selectAuthMechanism = $(''); + var neededVisibility = (this._isPersonal) ? 1 : 2; $.each(this._allAuthMechanisms, function(authIdentifier, authMechanism) { - if (backend.authSchemes[authMechanism.scheme]) { + if (backend.authSchemes[authMechanism.scheme] && (authMechanism.visibility & neededVisibility)) { selectAuthMechanism.append( $('') ); @@ -1136,9 +1137,7 @@ MountConfigListView.prototype = _.extend({ saveStorageConfig:function($tr, callback, concurrentTimer) { var self = this; var storage = this.getStorageConfig($tr); - console.log(storage); if (!storage || !storage.validate()) { - console.log('invalid'); return false; } diff --git a/apps/files_external/lib/auth/authmechanism.php b/apps/files_external/lib/auth/authmechanism.php index 72b56e0bc06..36e55de92c5 100644 --- a/apps/files_external/lib/auth/authmechanism.php +++ b/apps/files_external/lib/auth/authmechanism.php @@ -95,6 +95,7 @@ class AuthMechanism implements \JsonSerializable { $data += $this->jsonSerializeIdentifier(); $data['scheme'] = $this->getScheme(); + $data['visibility'] = $this->getVisibility(); return $data; } diff --git a/apps/files_external/lib/auth/password/userprovided.php b/apps/files_external/lib/auth/password/userprovided.php index b0ff50a279a..1c2cc0a6d97 100644 --- a/apps/files_external/lib/auth/password/userprovided.php +++ b/apps/files_external/lib/auth/password/userprovided.php @@ -22,6 +22,7 @@ namespace OCA\Files_External\Lib\Auth\Password; use OCA\Files_External\Lib\DefinitionParameter; +use OCA\Files_External\Service\BackendService; use OCP\IL10N; use OCP\IUser; use OCA\Files_External\Lib\Auth\AuthMechanism; @@ -45,6 +46,7 @@ class UserProvided extends AuthMechanism { $this ->setIdentifier('password::userprovided') + ->setVisibility(BackendService::VISIBILITY_ADMIN) ->setScheme(self::SCHEME_PASSWORD) ->setText($l->t('User provided')) ->addParameters([ From 03c79ac24fdda8c32177aaf25851a4ad069e6517 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Jan 2016 16:57:20 +0100 Subject: [PATCH 06/10] remove custom controler for user provided password auth --- apps/files_external/appinfo/routes.php | 7 +- .../controller/usercredentialscontroller.php | 99 ------------------- .../userglobalstoragescontroller.php | 50 +++++++++- apps/files_external/js/settings.js | 36 +------ .../files_external/lib/auth/iuserprovided.php | 36 +++++++ .../lib/auth/password/userprovided.php | 9 +- apps/files_external/lib/storageconfig.php | 1 + 7 files changed, 97 insertions(+), 141 deletions(-) delete mode 100644 apps/files_external/controller/usercredentialscontroller.php create mode 100644 apps/files_external/lib/auth/iuserprovided.php diff --git a/apps/files_external/appinfo/routes.php b/apps/files_external/appinfo/routes.php index b30ad0a8542..c3149a300cf 100644 --- a/apps/files_external/appinfo/routes.php +++ b/apps/files_external/appinfo/routes.php @@ -44,12 +44,7 @@ namespace OCA\Files_External\AppInfo; 'url' => '/ajax/public_key.php', 'verb' => 'POST', 'requirements' => array() - ), - [ - 'name' => 'UserCredentials#store', - 'url' => '/usercredentials/{storageId}', - 'verb' => 'PUT' - ] + ) ) ) ); diff --git a/apps/files_external/controller/usercredentialscontroller.php b/apps/files_external/controller/usercredentialscontroller.php deleted file mode 100644 index 5153189d9c3..00000000000 --- a/apps/files_external/controller/usercredentialscontroller.php +++ /dev/null @@ -1,99 +0,0 @@ - - * - * @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\Controller; - -use OCA\Calendar\Sabre\Backend; -use OCA\Files_External\Lib\Auth\AuthMechanism; -use OCA\Files_External\Lib\Auth\Password\UserProvided; -use OCA\Files_external\Lib\StorageConfig; -use OCA\Files_External\Service\UserGlobalStoragesService; -use OCP\AppFramework\Controller; -use OCP\AppFramework\Http; -use OCP\AppFramework\Http\DataResponse; -use OCP\IL10N; -use OCP\IRequest; -use OCP\IUserSession; - -class UserCredentialsController extends StoragesController { - /** - * @var UserProvided - */ - private $authMechanism; - - /** - * @var IUserSession - */ - private $userSession; - - /** - * @var UserGlobalStoragesService - */ - private $globalStoragesService; - - public function __construct( - $appName, IRequest $request, - UserProvided $authMechanism, - IUserSession $userSession, - IL10N $l10n, - UserGlobalStoragesService $globalStoragesService - ) { - parent::__construct($appName, $request, $l10n, $globalStoragesService); - $this->authMechanism = $authMechanism; - $this->userSession = $userSession; - $this->globalStoragesService = $globalStoragesService; - } - - /** - * @param int $storageId - * @param string $username - * @param string $password - * - * @NoAdminRequired - * @return DataResponse - */ - public function store($storageId, $username, $password) { - $this->authMechanism->saveCredentials($this->userSession->getUser(), $storageId, $username, $password); - - $storage = $this->globalStoragesService->getStorage($storageId); - - $this->updateStorageStatus($storage); - - $storage->setBackendOptions([]); - $storage->setMountOptions([]); - $this->manipulateStorageConfig($storage); - - - return new DataResponse( - $storage, - Http::STATUS_OK - ); - } - - 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/controller/userglobalstoragescontroller.php b/apps/files_external/controller/userglobalstoragescontroller.php index 97b5c90e20c..5031d3c46bd 100644 --- a/apps/files_external/controller/userglobalstoragescontroller.php +++ b/apps/files_external/controller/userglobalstoragescontroller.php @@ -22,12 +22,12 @@ namespace OCA\Files_External\Controller; use OCA\Files_External\Lib\Auth\AuthMechanism; +use OCA\Files_External\Lib\Auth\IUserProvided; use OCA\Files_External\Lib\Auth\Password\UserProvided; use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; use \OCP\IRequest; use \OCP\IL10N; use \OCP\AppFramework\Http\DataResponse; -use \OCP\AppFramework\Controller; use \OCP\AppFramework\Http; use \OCA\Files_external\Service\UserGlobalStoragesService; use \OCA\Files_external\NotFoundException; @@ -129,6 +129,54 @@ class UserGlobalStoragesController extends StoragesController { ); } + /** + * Update an external storage entry. + * Only allows setting user provided backend fields + * + * @param int $id storage id + * @param array $backendOptions backend-specific options + * + * @return DataResponse + * + * @NoAdminRequired + */ + public function update( + $id, + $backendOptions + ) { + try { + $storage = $this->service->getStorage($id); + $authMechanism = $storage->getAuthMechanism(); + if ($authMechanism instanceof IUserProvided) { + $authMechanism->saveBackendOptions($this->userSession->getUser(), $id, $backendOptions); + $authMechanism->manipulateStorageConfig($storage, $this->userSession->getUser()); + } else { + return new DataResponse( + [ + 'message' => (string)$this->l10n->t('Storage with id "%i" is not user editable', array($id)) + ], + Http::STATUS_FORBIDDEN + ); + } + } catch (NotFoundException $e) { + return new DataResponse( + [ + 'message' => (string)$this->l10n->t('Storage with id "%i" not found', array($id)) + ], + Http::STATUS_NOT_FOUND + ); + } + + $this->updateStorageStatus($storage); + $this->sanitizeStorage($storage); + + return new DataResponse( + $storage, + Http::STATUS_OK + ); + + } + /** * Remove sensitive data from a StorageConfig before returning it to the user * diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 52b46db6cc0..94d0fc2f5a7 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -447,34 +447,7 @@ var UserGlobalStorageConfig = function (id) { UserGlobalStorageConfig.prototype = _.extend({}, StorageConfig.prototype, /** @lends OCA.External.Settings.UserStorageConfig.prototype */ { - _url: 'apps/files_external/userglobalstorages', - - /** - * Creates or saves the storage. - * - * @param {Function} [options.success] success callback, receives result as argument - * @param {Function} [options.error] error callback - */ - save: function (options) { - var self = this; - var url = OC.generateUrl('apps/files_external/usercredentials/{id}', {id: this.id}); - - $.ajax({ - type: 'PUT', - url: url, - contentType: 'application/json', - data: JSON.stringify({ - username: this.backendOptions.user, - password: this.backendOptions.password - }), - success: function (result) { - if (_.isFunction(options.success)) { - options.success(result); - } - }, - error: options.error - }); - } + _url: 'apps/files_external/userglobalstorages' }); /** @@ -914,9 +887,10 @@ MountConfigListView.prototype = _.extend({ var onCompletion = jQuery.Deferred(); $.each(result, function(i, storageParams) { var storageConfig; - var isUserProvidedAuth = storageParams.authMechanism === 'password::userprovided'; + console.log(storageParams); + var isUserGlobal = storageParams.type === 'system' && self._isPersonal; storageParams.mountPoint = storageParams.mountPoint.substr(1); // trim leading slash - if (isUserProvidedAuth) { + if (isUserGlobal) { storageConfig = new UserGlobalStorageConfig(); } else { storageConfig = new self._storageConfigClass(); @@ -935,7 +909,7 @@ MountConfigListView.prototype = _.extend({ $tr.find('.mountOptionsToggle, .remove').empty(); $tr.find('input:not(.user_provided), select:not(.user_provided)').attr('disabled', 'disabled'); - if (isUserProvidedAuth) { + if (isUserGlobal) { $tr.find('.configuration').find(':not(.user_provided)').remove(); } else { // userglobal storages do not expose configuration data diff --git a/apps/files_external/lib/auth/iuserprovided.php b/apps/files_external/lib/auth/iuserprovided.php new file mode 100644 index 00000000000..6b7eab4e2a7 --- /dev/null +++ b/apps/files_external/lib/auth/iuserprovided.php @@ -0,0 +1,36 @@ + + * + * @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; + +use OCP\IUser; + +/** + * For auth mechanisms where the user needs to provide credentials + */ +interface IUserProvided { + /** + * @param IUser $user the user for which to save the user provided options + * @param int $mountId the mount id to save the options for + * @param array $options the user provided options + */ + public function saveBackendOptions(IUser $user, $mountId, array $options); +} diff --git a/apps/files_external/lib/auth/password/userprovided.php b/apps/files_external/lib/auth/password/userprovided.php index 1c2cc0a6d97..e1c1352022f 100644 --- a/apps/files_external/lib/auth/password/userprovided.php +++ b/apps/files_external/lib/auth/password/userprovided.php @@ -21,6 +21,7 @@ namespace OCA\Files_External\Lib\Auth\Password; +use OCA\Files_External\Lib\Auth\IUserProvided; use OCA\Files_External\Lib\DefinitionParameter; use OCA\Files_External\Service\BackendService; use OCP\IL10N; @@ -34,7 +35,7 @@ use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; /** * User provided Username and Password */ -class UserProvided extends AuthMechanism { +class UserProvided extends AuthMechanism implements IUserProvided { const CREDENTIALS_IDENTIFIER_PREFIX = 'password::userprovided/'; @@ -62,10 +63,10 @@ class UserProvided extends AuthMechanism { return self::CREDENTIALS_IDENTIFIER_PREFIX . $storageId; } - public function saveCredentials(IUser $user, $id, $username, $password) { + public function saveBackendOptions(IUser $user, $id, array $options) { $this->credentialsManager->store($user->getUID(), $this->getCredentialsIdentifier($id), [ - 'user' => $username, - 'password' => $password + 'user' => $options['user'], // explicitly copy the fields we want instead of just passing the entire $options array + 'password' => $options['password'] // this way we prevent users from being able to modify any other field ]); } diff --git a/apps/files_external/lib/storageconfig.php b/apps/files_external/lib/storageconfig.php index 33646e603c3..7f716893842 100644 --- a/apps/files_external/lib/storageconfig.php +++ b/apps/files_external/lib/storageconfig.php @@ -406,6 +406,7 @@ class StorageConfig implements \JsonSerializable { if (!is_null($this->statusMessage)) { $result['statusMessage'] = $this->statusMessage; } + $result['type'] = ($this->getType() === self::MOUNT_TYPE_PERSONAl) ? 'personal': 'system'; return $result; } } From eb29bc64de8a60881ba2876a7c4f150711c89694 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 25 Jan 2016 16:24:33 +0100 Subject: [PATCH 07/10] set auth mechanism in tests --- apps/files_external/tests/js/settingsSpec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/files_external/tests/js/settingsSpec.js b/apps/files_external/tests/js/settingsSpec.js index 7631dc5a9b9..b2b5e1f57ec 100644 --- a/apps/files_external/tests/js/settingsSpec.js +++ b/apps/files_external/tests/js/settingsSpec.js @@ -104,6 +104,7 @@ describe('OCA.External.Settings tests', function() { 'configuration': { }, 'scheme': 'builtin', + 'visibility': 3 }, }); From efcf790eff211afc220e298383874733fb907181 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 26 Jan 2016 13:22:27 +0100 Subject: [PATCH 08/10] minor fixes --- .../controller/userglobalstoragescontroller.php | 5 ++--- apps/files_external/js/settings.js | 11 ++++++++--- apps/files_external/lib/failedcache.php | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/apps/files_external/controller/userglobalstoragescontroller.php b/apps/files_external/controller/userglobalstoragescontroller.php index 5031d3c46bd..6b39b3dc92d 100644 --- a/apps/files_external/controller/userglobalstoragescontroller.php +++ b/apps/files_external/controller/userglobalstoragescontroller.php @@ -23,7 +23,6 @@ namespace OCA\Files_External\Controller; use OCA\Files_External\Lib\Auth\AuthMechanism; use OCA\Files_External\Lib\Auth\IUserProvided; -use OCA\Files_External\Lib\Auth\Password\UserProvided; use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; use \OCP\IRequest; use \OCP\IL10N; @@ -186,11 +185,11 @@ class UserGlobalStoragesController extends StoragesController { $storage->setBackendOptions([]); $storage->setMountOptions([]); - if ($storage->getAuthMechanism() instanceof UserProvided) { + if ($storage->getAuthMechanism() instanceof IUserProvided) { try { $storage->getAuthMechanism()->manipulateStorageConfig($storage, $this->userSession->getUser()); } catch (InsufficientDataForMeaningfulAnswerException $e) { - + // not configured yet } } } diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 94d0fc2f5a7..ccb1e858fa0 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -206,6 +206,12 @@ StorageConfig.Status = { ERROR: 1, INDETERMINATE: 2 }; +StorageConfig.Visibility = { + NONE: 0, + PERSONAL: 1, + ADMIN: 2, + DEFAULT: 3 +}; /** * @memberof OCA.External.Settings */ @@ -809,7 +815,7 @@ MountConfigListView.prototype = _.extend({ $tr.find('.backend').data('identifier', backend.identifier); var selectAuthMechanism = $(''); - var neededVisibility = (this._isPersonal) ? 1 : 2; + var neededVisibility = (this._isPersonal) ? StorageConfig.Visibility.PERSONAL : StorageConfig.Visibility.ADMIN; $.each(this._allAuthMechanisms, function(authIdentifier, authMechanism) { if (backend.authSchemes[authMechanism.scheme] && (authMechanism.visibility & neededVisibility)) { selectAuthMechanism.append( @@ -887,7 +893,6 @@ MountConfigListView.prototype = _.extend({ var onCompletion = jQuery.Deferred(); $.each(result, function(i, storageParams) { var storageConfig; - console.log(storageParams); var isUserGlobal = storageParams.type === 'system' && self._isPersonal; storageParams.mountPoint = storageParams.mountPoint.substr(1); // trim leading slash if (isUserGlobal) { @@ -970,7 +975,7 @@ MountConfigListView.prototype = _.extend({ var newElement; var trimmedPlaceholder = placeholder; - var flags = ['@', '*', '!', '#', '&']; + var flags = ['@', '*', '!', '#', '&']; // used to determine what kind of parameter while(flags.indexOf(trimmedPlaceholder[0]) !== -1) { trimmedPlaceholder = trimmedPlaceholder.substr(1); } diff --git a/apps/files_external/lib/failedcache.php b/apps/files_external/lib/failedcache.php index e646e4fe677..9e24c12f4b5 100644 --- a/apps/files_external/lib/failedcache.php +++ b/apps/files_external/lib/failedcache.php @@ -1,6 +1,6 @@ + * @author Robin Appelman * * @copyright Copyright (c) 2016, ownCloud, Inc. * @license AGPL-3.0 From d0f56876877d5681a6faf5e8882c4de48a9bd4dc Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 26 Jan 2016 14:40:55 +0100 Subject: [PATCH 09/10] Dont set null values when validating storage definition --- apps/files_external/lib/definitionparameter.php | 8 +++++--- apps/files_external/lib/frontenddefinitiontrait.php | 8 +++++--- apps/files_external/tests/frontenddefinitiontraittest.php | 2 ++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/apps/files_external/lib/definitionparameter.php b/apps/files_external/lib/definitionparameter.php index 85c3dd08ad4..dc7985837f5 100644 --- a/apps/files_external/lib/definitionparameter.php +++ b/apps/files_external/lib/definitionparameter.php @@ -154,6 +154,10 @@ class DefinitionParameter implements \JsonSerializable { return $prefix . $this->getText(); } + public function isOptional() { + return $this->isFlagSet(self::FLAG_OPTIONAL) || $this->isFlagSet(self::FLAG_USER_PROVIDED); + } + /** * Validate a parameter value against this * Convert type as necessary @@ -162,8 +166,6 @@ class DefinitionParameter implements \JsonSerializable { * @return bool success */ public function validateValue(&$value) { - $optional = $this->isFlagSet(self::FLAG_OPTIONAL) || $this->isFlagSet(self::FLAG_USER_PROVIDED); - switch ($this->getType()) { case self::VALUE_BOOLEAN: if (!is_bool($value)) { @@ -180,7 +182,7 @@ class DefinitionParameter implements \JsonSerializable { } break; default: - if (!$value && !$optional) { + if (!$value && !$this->isOptional()) { return false; } break; diff --git a/apps/files_external/lib/frontenddefinitiontrait.php b/apps/files_external/lib/frontenddefinitiontrait.php index eedd433f2d7..fc47a9515f3 100644 --- a/apps/files_external/lib/frontenddefinitiontrait.php +++ b/apps/files_external/lib/frontenddefinitiontrait.php @@ -136,10 +136,12 @@ trait FrontendDefinitionTrait { public function validateStorageDefinition(StorageConfig $storage) { foreach ($this->getParameters() as $name => $parameter) { $value = $storage->getBackendOption($name); - if (!$parameter->validateValue($value)) { - return false; + if (!is_null($value) || !$parameter->isOptional()) { + if (!$parameter->validateValue($value)) { + return false; + } + $storage->setBackendOption($name, $value); } - $storage->setBackendOption($name, $value); } return true; } diff --git a/apps/files_external/tests/frontenddefinitiontraittest.php b/apps/files_external/tests/frontenddefinitiontraittest.php index 209b1abc7e0..27f49556330 100644 --- a/apps/files_external/tests/frontenddefinitiontraittest.php +++ b/apps/files_external/tests/frontenddefinitiontraittest.php @@ -61,6 +61,8 @@ class FrontendDefinitionTraitTest extends \Test\TestCase { ->getMock(); $param->method('getName') ->willReturn($name); + $param->method('isOptional') + ->willReturn(false); $param->expects($this->once()) ->method('validateValue') ->willReturn($valid); From 6b18134ceb0fd46d5b5796c1bd396a93865722c2 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 28 Jan 2016 13:07:19 +0100 Subject: [PATCH 10/10] inject logger --- .../controller/globalstoragescontroller.php | 8 ++++++-- .../controller/storagescontroller.php | 15 ++++++++++++--- .../controller/userglobalstoragescontroller.php | 7 +++++-- .../controller/userstoragescontroller.php | 8 ++++++-- .../controller/globalstoragescontrollertest.php | 3 ++- .../controller/userstoragescontrollertest.php | 3 ++- 6 files changed, 33 insertions(+), 11 deletions(-) diff --git a/apps/files_external/controller/globalstoragescontroller.php b/apps/files_external/controller/globalstoragescontroller.php index 64bbf0feb3f..069e41a96b8 100644 --- a/apps/files_external/controller/globalstoragescontroller.php +++ b/apps/files_external/controller/globalstoragescontroller.php @@ -24,6 +24,7 @@ namespace OCA\Files_External\Controller; use \OCP\IConfig; +use OCP\ILogger; use \OCP\IUserSession; use \OCP\IRequest; use \OCP\IL10N; @@ -46,18 +47,21 @@ class GlobalStoragesController extends StoragesController { * @param IRequest $request request object * @param IL10N $l10n l10n service * @param GlobalStoragesService $globalStoragesService storage service + * @param ILogger $logger */ public function __construct( $AppName, IRequest $request, IL10N $l10n, - GlobalStoragesService $globalStoragesService + GlobalStoragesService $globalStoragesService, + ILogger $logger ) { parent::__construct( $AppName, $request, $l10n, - $globalStoragesService + $globalStoragesService, + $logger ); } diff --git a/apps/files_external/controller/storagescontroller.php b/apps/files_external/controller/storagescontroller.php index b09774ef515..d71c4ff5ef7 100644 --- a/apps/files_external/controller/storagescontroller.php +++ b/apps/files_external/controller/storagescontroller.php @@ -25,6 +25,7 @@ namespace OCA\Files_External\Controller; use \OCP\IConfig; +use OCP\ILogger; use OCP\IUser; use \OCP\IUserSession; use \OCP\IRequest; @@ -60,6 +61,11 @@ abstract class StoragesController extends Controller { */ protected $service; + /** + * @var ILogger + */ + protected $logger; + /** * Creates a new storages controller. * @@ -67,16 +73,19 @@ abstract class StoragesController extends Controller { * @param IRequest $request request object * @param IL10N $l10n l10n service * @param StoragesService $storagesService storage service + * @param ILogger $logger */ public function __construct( $AppName, IRequest $request, IL10N $l10n, - StoragesService $storagesService + StoragesService $storagesService, + ILogger $logger ) { parent::__construct($AppName, $request); $this->l10n = $l10n; $this->service = $storagesService; + $this->logger = $logger; } /** @@ -115,7 +124,7 @@ abstract class StoragesController extends Controller { $priority ); } catch (\InvalidArgumentException $e) { - \OC::$server->getLogger()->logException($e); + $this->logger->logException($e); return new DataResponse( [ 'message' => (string)$this->l10n->t('Invalid backend or authentication mechanism class') @@ -129,7 +138,7 @@ abstract class StoragesController extends Controller { * Validate storage config * * @param StorageConfig $storage storage config - * + *1 * @return DataResponse|null returns response in case of validation error */ protected function validate(StorageConfig $storage) { diff --git a/apps/files_external/controller/userglobalstoragescontroller.php b/apps/files_external/controller/userglobalstoragescontroller.php index 6b39b3dc92d..c6b51d94047 100644 --- a/apps/files_external/controller/userglobalstoragescontroller.php +++ b/apps/files_external/controller/userglobalstoragescontroller.php @@ -24,6 +24,7 @@ namespace OCA\Files_External\Controller; use OCA\Files_External\Lib\Auth\AuthMechanism; use OCA\Files_External\Lib\Auth\IUserProvided; use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; +use OCP\ILogger; use \OCP\IRequest; use \OCP\IL10N; use \OCP\AppFramework\Http\DataResponse; @@ -57,13 +58,15 @@ class UserGlobalStoragesController extends StoragesController { IRequest $request, IL10N $l10n, UserGlobalStoragesService $userGlobalStoragesService, - IUserSession $userSession + IUserSession $userSession, + ILogger $logger ) { parent::__construct( $AppName, $request, $l10n, - $userGlobalStoragesService + $userGlobalStoragesService, + $logger ); $this->userSession = $userSession; } diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php index ccf8bc24e09..2a2a0bc63a6 100644 --- a/apps/files_external/controller/userstoragescontroller.php +++ b/apps/files_external/controller/userstoragescontroller.php @@ -25,6 +25,7 @@ namespace OCA\Files_External\Controller; use OCA\Files_External\Lib\Auth\AuthMechanism; use \OCP\IConfig; +use OCP\ILogger; use OCP\IUser; use \OCP\IUserSession; use \OCP\IRequest; @@ -55,19 +56,22 @@ class UserStoragesController extends StoragesController { * @param IL10N $l10n l10n service * @param UserStoragesService $userStoragesService storage service * @param IUserSession $userSession + * @param ILogger $logger */ public function __construct( $AppName, IRequest $request, IL10N $l10n, UserStoragesService $userStoragesService, - IUserSession $userSession + IUserSession $userSession, + ILogger $logger ) { parent::__construct( $AppName, $request, $l10n, - $userStoragesService + $userStoragesService, + $logger ); $this->userSession = $userSession; } diff --git a/apps/files_external/tests/controller/globalstoragescontrollertest.php b/apps/files_external/tests/controller/globalstoragescontrollertest.php index a3c911b511c..9256569f22a 100644 --- a/apps/files_external/tests/controller/globalstoragescontrollertest.php +++ b/apps/files_external/tests/controller/globalstoragescontrollertest.php @@ -41,7 +41,8 @@ class GlobalStoragesControllerTest extends StoragesControllerTest { 'files_external', $this->getMock('\OCP\IRequest'), $this->getMock('\OCP\IL10N'), - $this->service + $this->service, + $this->getMock('\OCP\ILogger') ); } } diff --git a/apps/files_external/tests/controller/userstoragescontrollertest.php b/apps/files_external/tests/controller/userstoragescontrollertest.php index 671e019fea0..342f6b85385 100644 --- a/apps/files_external/tests/controller/userstoragescontrollertest.php +++ b/apps/files_external/tests/controller/userstoragescontrollertest.php @@ -49,7 +49,8 @@ class UserStoragesControllerTest extends StoragesControllerTest { $this->getMock('\OCP\IRequest'), $this->getMock('\OCP\IL10N'), $this->service, - $this->getMock('\OCP\IUserSession') + $this->getMock('\OCP\IUserSession'), + $this->getMock('\OCP\ILogger') ); }