diff --git a/apps/files_external/css/settings.css b/apps/files_external/css/settings.css index 6dfb012b15eb..cc924db27987 100644 --- a/apps/files_external/css/settings.css +++ b/apps/files_external/css/settings.css @@ -102,3 +102,13 @@ td.mountPoint, td.backend { width:160px; } #externalStorage .mountOptionsDropdown { margin-right: 40px; } + +#externalStorage td.configuration .error-invalid, +#externalStorage td.authentication .error-invalid { + white-space: normal; + +} + +#externalStorage tr.invalid { + background-color: #F2DEDE; +} diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 933e1ec01d70..0794a77e2bf9 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -809,6 +809,19 @@ MountConfigListView.prototype = _.extend({ newStorage: function(storageConfig, onCompletion) { var mountPoint = storageConfig.mountPoint; var backend = this._allBackends[storageConfig.backend]; + var isInvalidAuth = false; + + if (!backend) { + backend = { + name: 'Unknown backend: ' + storageConfig.backend, + invalid: true + }; + } + if (backend && storageConfig.authMechanism && !this._allAuthMechanisms[storageConfig.authMechanism]) { + // mark invalid to prevent editing + backend.invalid = true; + isInvalidAuth = true; + } // FIXME: Replace with a proper Handlebar template var $tr = this.$el.find('tr#addMountPoint'); @@ -836,6 +849,26 @@ MountConfigListView.prototype = _.extend({ $tr.addClass(backend.identifier); $tr.find('.backend').data('identifier', backend.identifier); + if (backend.invalid || isInvalidAuth) { + $tr.addClass('invalid'); + $tr.find('[name=mountPoint]').prop('disabled', true); + $tr.find('.applicable,.mountOptionsToggle').empty(); + this.updateStatus($tr, false, 'Unknown backend: ' + backend.name); + if (isInvalidAuth) { + $tr.find('td.authentication').append('' + + t('files_external', 'Unknown auth backend "{b}"', {b: storageConfig.authMechanism}) + + '' + ); + } + + $tr.find('td.configuration').append('' + + t('files_external', 'Please make sure that the app that provides this backend is installed and enabled') + + '' + ); + + return $tr; + } + var selectAuthMechanism = $(''); var neededVisibility = (this._isPersonal) ? StorageConfig.Visibility.PERSONAL : StorageConfig.Visibility.ADMIN; $.each(this._allAuthMechanisms, function(authIdentifier, authMechanism) { diff --git a/apps/files_external/lib/Command/ListCommand.php b/apps/files_external/lib/Command/ListCommand.php index 503c62af1368..15642ff789bd 100644 --- a/apps/files_external/lib/Command/ListCommand.php +++ b/apps/files_external/lib/Command/ListCommand.php @@ -34,6 +34,8 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; +use OCP\Files\External\Backend\InvalidBackend; +use OCP\Files\External\Auth\InvalidAuth; class ListCommand extends Base { /** @@ -192,7 +194,11 @@ public function listMounts($userId, array $mounts, InputInterface $input, Output 'enable_sharing' => false, 'encoding_compatibility' => false ]; - $rows = array_map(function (IStorageConfig $config) use ($userId, $defaultMountOptions, $full) { + $countInvalid = 0; + $rows = array_map(function (IStorageConfig $config) use ($userId, $defaultMountOptions, $full, &$countInvalid) { + if ($config->getBackend() instanceof InvalidBackend || $config->getAuthMechanism() instanceof InvalidAuth) { + $countInvalid++; + } $storageConfig = $config->getBackendOptions(); $keys = array_keys($storageConfig); $values = array_values($storageConfig); @@ -256,6 +262,14 @@ public function listMounts($userId, array $mounts, InputInterface $input, Output $table->setHeaders($headers); $table->setRows($rows); $table->render(); + + if ($countInvalid > 0) { + $output->writeln( + "Number of invalid storages found: $countInvalid.\n" . + "The listed configuration details are likely incomplete.\n" . + "Please make sure that all related apps that provide these storages are enabled or delete these." + ); + } } } diff --git a/apps/files_external/tests/Command/ListCommandTest.php b/apps/files_external/tests/Command/ListCommandTest.php index e1a279fdd841..97d89481a0e5 100644 --- a/apps/files_external/tests/Command/ListCommandTest.php +++ b/apps/files_external/tests/Command/ListCommandTest.php @@ -31,6 +31,7 @@ use OC\Files\External\StorageConfig; use OCP\Files\External\IStorageConfig; use Symfony\Component\Console\Output\BufferedOutput; +use OCP\Files\External\Backend\InvalidBackend; class ListCommandTest extends CommandTest { /** @@ -71,4 +72,26 @@ public function testListAuthIdentifier() { $this->assertNotEquals($output[0]['authentication_type'], $output[1]['authentication_type']); } + + public function testDisplayWarningForIncomplete() { + $l10n = $this->createMock('\OCP\IL10N', null, [], '', false); + $session = $this->createMock('\OCP\ISession'); + $crypto = $this->createMock('\OCP\Security\ICrypto'); + $instance = $this->getInstance(); + // FIXME: use mock of IStorageConfig + $mount1 = new StorageConfig(); + $mount1->setAuthMechanism(new Password()); + $mount1->setBackend(new InvalidBackend('InvalidId')); + $mount2 = new StorageConfig(); + $mount2->setAuthMechanism(new SessionCredentials($session, $crypto)); + $mount2->setBackend(new Local($l10n, new NullMechanism())); + $input = $this->getInput($instance); + $output = new BufferedOutput(); + + $instance->listMounts('', [$mount1, $mount2], $input, $output); + $output = $output->fetch(); + + $lines = explode($output, "\n"); + $this->assertRegexp('/Number of invalid storages found/', $output); + } } diff --git a/lib/private/Files/External/InvalidStorage.php b/lib/private/Files/External/InvalidStorage.php new file mode 100644 index 000000000000..f397fc1fbcb4 --- /dev/null +++ b/lib/private/Files/External/InvalidStorage.php @@ -0,0 +1,31 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Files\External; + +use OCP\Files\StorageNotAvailableException; +use OC\Files\Storage\FailedStorage; + +class InvalidStorage extends FailedStorage { + public function __construct($params) { + throw new StorageNotAvailableException(); + } +} diff --git a/lib/private/Files/External/Service/StoragesService.php b/lib/private/Files/External/Service/StoragesService.php index 88859e3a40d2..9485fbf8c799 100644 --- a/lib/private/Files/External/Service/StoragesService.php +++ b/lib/private/Files/External/Service/StoragesService.php @@ -37,6 +37,8 @@ use OCP\Files\External\IStoragesBackendService; use OCP\Files\External\NotFoundException; use OCP\Files\External\Service\IStoragesService; +use OCP\Files\External\Backend\InvalidBackend; +use OCP\Files\External\Auth\InvalidAuth; /** * Service class to manage external storages @@ -307,11 +309,11 @@ public function createStorage( ) { $backend = $this->backendService->getBackend($backendIdentifier); if (!$backend) { - throw new \InvalidArgumentException('Unable to get backend for ' . $backendIdentifier); + $backend = new InvalidBackend($backendIdentifier); } $authMechanism = $this->backendService->getAuthMechanism($authMechanismIdentifier); if (!$authMechanism) { - throw new \InvalidArgumentException('Unable to get authentication mechanism for ' . $authMechanismIdentifier); + $authMechanism = new InvalidAuth($authMechanismIdentifier); } $newStorage = $this->createConfig(); $newStorage->setMountPoint($mountPoint); @@ -389,11 +391,15 @@ public function updateStorage(IStorageConfig $updatedStorage) { $existingMount = $this->dbConfig->getMountById($id); if (!is_array($existingMount)) { - throw new NotFoundException('Storage with id "' . $id . '" not found while updating storage'); + throw new NotFoundException('Storage config with id "' . $id . '" not found while updating storage'); } $oldStorage = $this->getStorageConfigFromDBMount($existingMount); + if ($oldStorage->getBackend() instanceof InvalidBackend) { + throw new NotFoundException('Storage config with id "' . $id . '" and backend id "' . $oldStorage->getBackend()->getInvalidId() . '" cannot be edited due to missing backend'); + } + $removedUsers = array_diff($oldStorage->getApplicableUsers(), $updatedStorage->getApplicableUsers()); $removedGroups = array_diff($oldStorage->getApplicableGroups(), $updatedStorage->getApplicableGroups()); $addedUsers = array_diff($updatedStorage->getApplicableUsers(), $oldStorage->getApplicableUsers()); diff --git a/lib/public/Files/External/Auth/InvalidAuth.php b/lib/public/Files/External/Auth/InvalidAuth.php new file mode 100644 index 000000000000..52a3c71ddfc5 --- /dev/null +++ b/lib/public/Files/External/Auth/InvalidAuth.php @@ -0,0 +1,47 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCP\Files\External\Auth; + +/** + * Invalid authentication representing an auth mechanism + * that could not be resolved + * + * @since 10.0.5 + */ +class InvalidAuth extends AuthMechanism { + + /** + * Constructs a new InvalidAuth with the id of the invalid auth + * for display purposes + * + * @param string $invalidId invalid id + * + * @since 10.0.5 + */ + public function __construct($invalidId) { + $this->setIdentifier($invalidId) + ->setScheme(self::SCHEME_NULL) + ->setText('Unknown auth mechanism backend ' . $invalidId) + ; + } + +} diff --git a/lib/public/Files/External/Backend/InvalidBackend.php b/lib/public/Files/External/Backend/InvalidBackend.php new file mode 100644 index 000000000000..13eda1bb6dfd --- /dev/null +++ b/lib/public/Files/External/Backend/InvalidBackend.php @@ -0,0 +1,64 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCP\Files\External\Backend; + +use \OCP\IL10N; + +/** + * Invalid storage backend representing a backend + * that could not be resolved + * + * @since 10.0.5 + */ +class InvalidBackend extends Backend { + + /** @var string Invalid backend id */ + private $invalidId; + + /** + * Constructs a new InvalidBackend with the id of the invalid backend + * for display purposes + * + * @param string $invalidId id of the backend that did not exist + * + * @since 10.0.5 + */ + function __construct($invalidId) { + $this->invalidId = $invalidId; + $this->setIdentifier($invalidId) + ->setStorageClass('\OC\Files\External\InvalidStorage') + ->setText('Unknown storage backend ' . $invalidId) + ; + } + + /** + * Returns the invalid backend id + * + * @return string invalid backend id + * + * @since 10.0.5 + */ + public function getInvalidId() { + return $this->invalidId; + } +} + diff --git a/tests/lib/Files/External/InvalidStorageTest.php b/tests/lib/Files/External/InvalidStorageTest.php new file mode 100644 index 000000000000..19ee41ade997 --- /dev/null +++ b/tests/lib/Files/External/InvalidStorageTest.php @@ -0,0 +1,34 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH. + * @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 Test\Files\External; + +use Test\TestCase; +use OC\Files\External\InvalidStorage; + +class InvalidStorageTest extends TestCase { + + /** + * @expectedException OCP\Files\StorageNotAvailableException + */ + public function testCannotInstantiate() { + new InvalidStorage([]); + } +} diff --git a/tests/lib/Files/External/Service/StoragesServiceTest.php b/tests/lib/Files/External/Service/StoragesServiceTest.php index 0164bfcae701..13be1a73a97f 100644 --- a/tests/lib/Files/External/Service/StoragesServiceTest.php +++ b/tests/lib/Files/External/Service/StoragesServiceTest.php @@ -31,6 +31,9 @@ use OCP\Files\External\NotFoundException; use OCP\Files\External\IStoragesBackendService; use Test\TestCase; +use OCP\Files\External\Backend\InvalidBackend; +use OCP\Files\External\Auth\InvalidAuth; +use OCP\Files\External\Backend\Backend; /** * @group DB @@ -67,6 +70,11 @@ abstract class StoragesServiceTest extends TestCase { */ protected $mountCache; + /** + * @var Backend[] + */ + protected $backends; + public function setUp() { parent::setUp(); $this->dbConfig = new CleaningDBConfig(\OC::$server->getDatabaseConnection(), \OC::$server->getCrypto()); @@ -108,21 +116,24 @@ public function setUp() { $sftpBackend = $this->getBackendMock('\OCA\Files_External\Lib\Backend\SFTP', '\OCA\Files_External\Lib\Storage\SFTP'); $dummyBackend = $this->getBackendMock('\Test\Files\External\Backend\DummyBackend', '\Test\Files\External\Backend\DummyStorage'); - $backends = [ + $this->backends = [ 'identifier:\OCA\Files_External\Lib\Backend\SMB' => $this->getBackendMock('\OCA\Files_External\Lib\Backend\SMB', '\OCA\Files_External\Lib\Storage\SMB'), 'identifier:\OCA\Files_External\Lib\Backend\SFTP' => $sftpBackend, 'identifier:\Test\Files\External\Backend\DummyBackend' => $dummyBackend, 'identifier:sftp_alias' => $sftpBackend, ]; $this->backendService->method('getBackend') - ->will($this->returnCallback(function ($backendClass) use ($backends) { - if (isset($backends[$backendClass])) { - return $backends[$backendClass]; + ->will($this->returnCallback(function ($backendClass) { + if (isset($this->backends[$backendClass])) { + return $this->backends[$backendClass]; } return null; })); $this->backendService->method('getBackends') - ->will($this->returnValue($backends)); + ->will($this->returnCallback(function () { + // in case they changed + return $this->backends; + })); \OCP\Util::connectHook( Filesystem::CLASSNAME, @@ -349,27 +360,29 @@ public function testCreateStorage() { } /** - * @expectedException \InvalidArgumentException */ public function testCreateStorageInvalidClass() { - $this->service->createStorage( + $storageConfig = $this->service->createStorage( 'mount', 'identifier:\OC\Not\A\Backend', 'identifier:\Auth\Mechanism', [] ); + + $this->assertInstanceOf(InvalidBackend::class, $storageConfig->getBackend()); + } /** - * @expectedException \InvalidArgumentException */ public function testCreateStorageInvalidAuthMechanismClass() { - $this->service->createStorage( + $storageConfig = $this->service->createStorage( 'mount', 'identifier:\OCA\Files_External\Lib\Backend\SMB', 'identifier:\Not\An\Auth\Mechanism', [] ); + $this->assertInstanceOf(InvalidAuth::class, $storageConfig->getAuthMechanism()); } public function testGetStoragesBackendNotVisible() { @@ -486,4 +499,28 @@ public function testUpdateStorageMountPoint() { $this->assertEquals($newAuthMechanism, $savedStorage->getAuthMechanism()); $this->assertEquals('password2', $savedStorage->getBackendOption('password')); } + + /** + * @expectedException OCP\Files\External\NotFoundException + */ + public function testCannotEditInvalidBackend() { + $backend = $this->backendService->getBackend('identifier:\Test\Files\External\Backend\DummyBackend'); + $authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism'); + + $storage = new StorageConfig(); + $storage->setMountPoint('mountpoint'); + $storage->setBackend($backend); + $storage->setAuthMechanism($authMechanism); + $storage->setBackendOptions(['password' => 'testPassword']); + + $savedStorage = $this->service->addStorage($storage); + + // make it invalid + $this->backends['identifier:\Test\Files\External\Backend\DummyBackend'] = new InvalidBackend('identifier:\Test\Files\External\Backend\DummyBackend'); + + $updatedStorage = new StorageConfig($savedStorage->getId()); + $updatedStorage->setBackendOptions(['password' => 'password2']); + + $this->service->updateStorage($updatedStorage); + } } diff --git a/tests/lib/Files/External/Service/UserGlobalStoragesServiceTest.php b/tests/lib/Files/External/Service/UserGlobalStoragesServiceTest.php index 85a8435e13d9..f048f3e12e51 100644 --- a/tests/lib/Files/External/Service/UserGlobalStoragesServiceTest.php +++ b/tests/lib/Files/External/Service/UserGlobalStoragesServiceTest.php @@ -366,4 +366,9 @@ public function testUpdateStorageMountPoint() { // we don't test this here $this->assertTrue(true); } + + public function testCannotEditInvalidBackend() { + // we don't test this here + $this->assertTrue(true); + } }