Merge pull request #21790 from owncloud/ext-auth-user-provided

Add user provided auth mechanism

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'),

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

apps/files_external/controller/storagescontroller.php

@@ -25,6 +25,8 @@ namespace OCA\Files_External\Controller;
 
 
 use \OCP\IConfig;
+use OCP\ILogger;
+use OCP\IUser;
 use \OCP\IUserSession;
 use \OCP\IRequest;
 use \OCP\IL10N;
@@ -59,6 +61,11 @@ abstract class StoragesController extends Controller {
 	 */
 	protected $service;
 
+	/**
+	 * @var ILogger
+	 */
+	protected $logger;
+
 	/**
 	 * Creates a new storages controller.
 	 *
@@ -66,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;
 	}
 
 	/**
@@ -114,6 +124,7 @@ abstract class StoragesController extends Controller {
 				$priority
 			);
 		} catch (\InvalidArgumentException $e) {
+			$this->logger->logException($e);
 			return new DataResponse(
 				[
 					'message' => (string)$this->l10n->t('Invalid backend or authentication mechanism class')
@@ -127,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) {

apps/files_external/controller/userglobalstoragescontroller.php

@@ -22,10 +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\InsufficientDataForMeaningfulAnswerException;
+use OCP\ILogger;
 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;
@@ -56,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;
 	}
@@ -127,6 +131,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
 	 *
@@ -135,6 +187,14 @@ class UserGlobalStoragesController extends StoragesController {
 	protected function sanitizeStorage(StorageConfig $storage) {
 		$storage->setBackendOptions([]);
 		$storage->setMountOptions([]);
+
+		if ($storage->getAuthMechanism() instanceof IUserProvided) {
+			try {
+				$storage->getAuthMechanism()->manipulateStorageConfig($storage, $this->userSession->getUser());
+			} catch (InsufficientDataForMeaningfulAnswerException $e) {
+				// not configured yet
+			}
+		}
 	}
 
 }

apps/files_external/controller/userstoragescontroller.php

@@ -25,6 +25,8 @@ 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;
 use \OCP\IL10N;
@@ -54,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;
 	}

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
  */
@@ -357,6 +363,9 @@ StorageConfig.prototype = {
 		if (this.mountPoint === '') {
 			return false;
 		}
+		if (!this.backend) {
+			return false;
+		}
 		if (this.errors) {
 			return false;
 		}
@@ -432,6 +441,21 @@ 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'
+});
+
 /**
  * @class OCA.External.Settings.MountOptionsDropdown
  *
@@ -748,7 +772,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 +794,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');
@@ -790,8 +815,9 @@ MountConfigListView.prototype = _.extend({
 		$tr.find('.backend').data('identifier', backend.identifier);
 
 		var selectAuthMechanism = $('<select class="selectAuthMechanism"></select>');
+		var neededVisibility = (this._isPersonal) ? StorageConfig.Visibility.PERSONAL : StorageConfig.Visibility.ADMIN;
 		$.each(this._allAuthMechanisms, function(authIdentifier, authMechanism) {
-			if (backend.authSchemes[authMechanism.scheme]) {
+			if (backend.authSchemes[authMechanism.scheme] && (authMechanism.visibility & neededVisibility)) {
 				selectAuthMechanism.append(
 					$('<option value="'+authMechanism.identifier+'" data-scheme="'+authMechanism.scheme+'">'+authMechanism.name+'</option>')
 				);
@@ -805,7 +831,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 +892,14 @@ MountConfigListView.prototype = _.extend({
 				success: function(result) {
 					var onCompletion = jQuery.Deferred();
 					$.each(result, function(i, storageParams) {
+						var storageConfig;
+						var isUserGlobal = storageParams.type === 'system' && self._isPersonal;
 						storageParams.mountPoint = storageParams.mountPoint.substr(1); // trim leading slash
-						var storageConfig = new self._storageConfigClass();
+						if (isUserGlobal) {
+							storageConfig = new UserGlobalStorageConfig();
+						} else {
+							storageConfig = new self._storageConfigClass();
+						}
 						_.extend(storageConfig, storageParams);
 						var $tr = self.newStorage(storageConfig, onCompletion);
 
@@ -878,12 +910,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 (isUserGlobal) {
+							$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 +954,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 = $('<input type="password" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" placeholder="'+placeholder.substring(1)+'" />');
-		} else if (placeholder.indexOf('!') === 0) {
+
+		var trimmedPlaceholder = placeholder;
+		var flags = ['@', '*', '!', '#', '&']; // used to determine what kind of parameter
+		while(flags.indexOf(trimmedPlaceholder[0]) !== -1) {
+			trimmedPlaceholder = trimmedPlaceholder.substr(1);
+		}
+		if (hasFlag('*')) {
+			newElement = $('<input type="password" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" placeholder="'+ trimmedPlaceholder+'" />');
+		} else if (hasFlag('!')) {
 			var checkboxId = _.uniqueId('checkbox_');
-			newElement = $('<input type="checkbox" id="'+checkboxId+'" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" /><label for="'+checkboxId+'">'+placeholder.substring(1)+'</label>');
-		} else if (placeholder.indexOf('#') === 0) {
+			newElement = $('<input type="checkbox" id="'+checkboxId+'" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" /><label for="'+checkboxId+'">'+ trimmedPlaceholder+'</label>');
+		} else if (hasFlag('#')) {
 			newElement = $('<input type="hidden" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" />');
 		} else {
-			newElement = $('<input type="text" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" placeholder="'+placeholder+'" />');
+			newElement = $('<input type="text" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" placeholder="'+ trimmedPlaceholder+'" />');
 		}
 		highlightInput(newElement);
 		$td.append(newElement);
@@ -952,7 +1006,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 +1025,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 +1053,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 +1116,7 @@ MountConfigListView.prototype = _.extend({
 	saveStorageConfig:function($tr, callback, concurrentTimer) {
 		var self = this;
 		var storage = this.getStorageConfig($tr);
-		if (!storage.validate()) {
+		if (!storage || !storage.validate()) {
 			return false;
 		}
 

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

apps/files_external/lib/auth/iuserprovided.php

@@ -0,0 +1,36 @@
+<?php
+/**
+ * @author Robin Appelman <icewind@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;
+
+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);
+}

apps/files_external/lib/auth/password/userprovided.php

@@ -0,0 +1,88 @@
+<?php
+/**
+ * @author Robin Appelman <icewind@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 OCA\Files_External\Lib\Auth\IUserProvided;
+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;
+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 implements IUserProvided {
+
+	const CREDENTIALS_IDENTIFIER_PREFIX = 'password::userprovided/';
+
+	/** @var ICredentialsManager */
+	protected $credentialsManager;
+
+	public function __construct(IL10N $l, ICredentialsManager $credentialsManager) {
+		$this->credentialsManager = $credentialsManager;
+
+		$this
+			->setIdentifier('password::userprovided')
+			->setVisibility(BackendService::VISIBILITY_ADMIN)
+			->setScheme(self::SCHEME_PASSWORD)
+			->setText($l->t('User provided'))
+			->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) {
+		return self::CREDENTIALS_IDENTIFIER_PREFIX . $storageId;
+	}
+
+	public function saveBackendOptions(IUser $user, $id, array $options) {
+		$this->credentialsManager->store($user->getUID(), $this->getCredentialsIdentifier($id), [
+			'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
+		]);
+	}
+
+	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']);
+	}
+
+}

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,15 +144,20 @@ 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();
 	}
 
+	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
@@ -160,28 +166,26 @@ class DefinitionParameter implements \JsonSerializable {
 	 * @return bool success
 	 */
 	public function validateValue(&$value) {
-		$optional = $this->getFlags() & self::FLAG_OPTIONAL;
-
 		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 && !$this->isOptional()) {
 					return false;
 				}
-			}
-			break;
-		default:
-			if (!$value && !$optional) {
-				return false;
-			}
-			break;
+				break;
 		}
 		return true;
 	}

apps/files_external/lib/failedcache.php

@@ -0,0 +1,126 @@
+<?php
+/**
+ * @author Robin Appelman <icewind@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 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;
+	}
+}

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();
+	}
 }

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

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

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

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

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

apps/files_external/tests/js/settingsSpec.js

@@ -104,6 +104,7 @@ describe('OCA.External.Settings tests', function() {
 				'configuration': {
 				},
 				'scheme': 'builtin',
+				'visibility': 3
 			},
 		});