From d4cce4c6f0e2ae21475433950a19d40a9055bac1 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 3 Nov 2016 12:14:44 +0100 Subject: [PATCH] Handle invalid ext storage backend to keep mount point visible Keep mount point visible and also ext storage config visible when dealing with configs relating to storage backends or auth mechanisms that were provided by an app that is currently disabled. --- apps/files_external/css/settings.css | 10 +++ apps/files_external/js/settings.js | 33 ++++++++++ .../lib/Command/ListCommand.php | 16 ++++- .../tests/Command/ListCommandTest.php | 23 +++++++ lib/private/Files/External/InvalidStorage.php | 31 +++++++++ .../External/Service/StoragesService.php | 12 +++- .../Files/External/Auth/InvalidAuth.php | 47 ++++++++++++++ .../Files/External/Backend/InvalidBackend.php | 64 +++++++++++++++++++ .../lib/Files/External/InvalidStorageTest.php | 34 ++++++++++ .../External/Service/StoragesServiceTest.php | 55 +++++++++++++--- .../Service/UserGlobalStoragesServiceTest.php | 5 ++ 11 files changed, 317 insertions(+), 13 deletions(-) create mode 100644 lib/private/Files/External/InvalidStorage.php create mode 100644 lib/public/Files/External/Auth/InvalidAuth.php create mode 100644 lib/public/Files/External/Backend/InvalidBackend.php create mode 100644 tests/lib/Files/External/InvalidStorageTest.php 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); + } }