From 352b67fcea3a58a96c0cb5785a74856d2722b422 Mon Sep 17 00:00:00 2001 From: Sujith H Date: Tue, 6 Mar 2018 19:31:23 +0530 Subject: [PATCH] [stable10] Backport of Remove user from the storage setting or storage when user is deleted Remove user from the storage if there are multiple users associated with the storage during deletion of the user. Else remove the storage when user is deleted, since the storage is only associated with the user being deleted. Signed-off-by: Sujith H --- apps/files_external/lib/Lib/Backend/DAV.php | 4 +- .../Service/GlobalStoragesService.php | 33 ++++ .../External/Service/UserStoragesService.php | 20 +++ lib/private/User/User.php | 6 +- .../Service/IGlobalStoragesService.php | 8 + .../External/Service/IUserStoragesService.php | 8 +- .../GlobalStoragesServiceDeleteUserTest.php | 169 ++++++++++++++++++ .../Service/UserStoragesServiceTest.php | 62 +++++++ 8 files changed, 306 insertions(+), 4 deletions(-) create mode 100644 tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php diff --git a/apps/files_external/lib/Lib/Backend/DAV.php b/apps/files_external/lib/Lib/Backend/DAV.php index 3670e5543799..981338c91f53 100644 --- a/apps/files_external/lib/Lib/Backend/DAV.php +++ b/apps/files_external/lib/Lib/Backend/DAV.php @@ -24,11 +24,11 @@ use OCA\Files_External\Lib\LegacyDependencyCheckPolyfill; use OCP\Files\External\Auth\AuthMechanism; -use OCP\Files\External\Backend\Backend; +use OCP\Files\External\Backend\Backend as ExternalBackend; use OCP\Files\External\DefinitionParameter; use OCP\IL10N; -class DAV extends Backend { +class DAV extends ExternalBackend { use LegacyDependencyCheckPolyfill; diff --git a/lib/private/Files/External/Service/GlobalStoragesService.php b/lib/private/Files/External/Service/GlobalStoragesService.php index a6c17f95030a..59f3fe2f9dd7 100644 --- a/lib/private/Files/External/Service/GlobalStoragesService.php +++ b/lib/private/Files/External/Service/GlobalStoragesService.php @@ -31,6 +31,7 @@ use OCP\Files\External\IStorageConfig; use OCP\Files\External\IStoragesBackendService; use OCP\Files\External\Service\IGlobalStoragesService; +use OCP\IUser; /** * Service class to manage global external storages @@ -183,4 +184,36 @@ public function getStorageForAllUsers() { return array_combine($keys, $configs); } + + /** + * Deletes the external storages mounted to the user + * + * @param IUser $user + * @return bool + */ + public function deleteAllForUser($user) { + $userId = $user->getUID(); + $result = false; + //Get all valid storages + $mounts = $this->getStorages(); + foreach ($mounts as $mount) { + $applicableUsers = $mount->getApplicableUsers(); + $id = $mount->getId(); + if (\in_array($userId, $applicableUsers, true)) { + if (\count($applicableUsers) === 1) { + //As this storage is associated only with this user. + $this->removeStorage($id); + $result = true; + } else { + $storage = $this->getStorage($id); + $userIndex = \array_search($userId, $applicableUsers, true); + unset($applicableUsers[$userIndex]); + $storage->setApplicableUsers($applicableUsers); + $this->updateStorage($storage); + $result = true; + } + } + } + return $result; + } } diff --git a/lib/private/Files/External/Service/UserStoragesService.php b/lib/private/Files/External/Service/UserStoragesService.php index 966424ef0be6..863f9a3e1888 100644 --- a/lib/private/Files/External/Service/UserStoragesService.php +++ b/lib/private/Files/External/Service/UserStoragesService.php @@ -27,6 +27,7 @@ use OC\Files\Filesystem; use OCP\Files\Config\IUserMountCache; +use OCP\IUser; use OCP\IUserSession; use OCP\Files\External\IStorageConfig; @@ -56,6 +57,7 @@ public function __construct( IUserMountCache $userMountCache ) { $this->userSession = $userSession; + $this->userMountCache = $userMountCache; parent::__construct($backendService, $dbConfig, $userMountCache); } @@ -140,4 +142,22 @@ public function getVisibilityType() { protected function isApplicable(IStorageConfig $config) { return ($config->getApplicableUsers() === [$this->getUser()->getUID()]) && $config->getType() === IStorageConfig::MOUNT_TYPE_PERSONAl; } + + /** + * Deletes the storages mounted to a user + * @param IUser $user + * @return bool + */ + public function deleteAllMountsForUser(IUser $user) { + $getUserMounts = $this->userMountCache->getMountsForUser($user); + $result = false; + if (\count($getUserMounts) > 0) { + foreach ($getUserMounts as $userMount) { + $id = $userMount->getStorageId(); + $this->userMountCache->removeUserStorageMount($id, $user->getUID()); + $result = true; + } + } + return $result; + } } diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 6a88929b621a..b4fca8066308 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -43,7 +43,6 @@ use OCP\IUserBackend; use OCP\IUserSession; use OCP\User\IChangePasswordBackend; -use OCP\UserInterface; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\GenericEvent; @@ -227,6 +226,11 @@ public function delete() { // Delete the user's keys in preferences \OC::$server->getConfig()->deleteAllUserValues($this->getUID()); + // Delete all mount points for user + \OC::$server->getUserStoragesService()->deleteAllMountsForUser($this); + //Delete external storage or remove user from applicableUsers list + \OC::$server->getGlobalStoragesService()->deleteAllForUser($this); + // Delete user files in /data/ if ($homePath !== false) { // FIXME: this operates directly on FS, should use View instead... diff --git a/lib/public/Files/External/Service/IGlobalStoragesService.php b/lib/public/Files/External/Service/IGlobalStoragesService.php index bf92b99cbfae..c8e8fd7473f0 100644 --- a/lib/public/Files/External/Service/IGlobalStoragesService.php +++ b/lib/public/Files/External/Service/IGlobalStoragesService.php @@ -35,4 +35,12 @@ interface IGlobalStoragesService extends IStoragesService { * @since 10.0 */ public function getStorageForAllUsers(); + + /** + * Deletes the external storages mounted to the user + * @param \OCP\IUser $user + * @return bool + * @since 10.0.10 + */ + public function deleteAllForUser($user); } diff --git a/lib/public/Files/External/Service/IUserStoragesService.php b/lib/public/Files/External/Service/IUserStoragesService.php index f5a08ed81f0a..a13251e088ed 100644 --- a/lib/public/Files/External/Service/IUserStoragesService.php +++ b/lib/public/Files/External/Service/IUserStoragesService.php @@ -28,5 +28,11 @@ * @since 10.0 */ interface IUserStoragesService extends IStoragesService { - + /** + * Deletes the storages mounted to a user + * @param \OCP\IUser $user + * @return bool + * @since 10.0.10 + */ + public function deleteAllMountsForUser(\OCP\IUser $user); } diff --git a/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php b/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php new file mode 100644 index 000000000000..cecc7d05202a --- /dev/null +++ b/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php @@ -0,0 +1,169 @@ + + * + * @copyright Copyright (c) 2018, 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\Service; + +use OC\Files\Config\UserMountCache; +use OC\Files\External\Service\DBConfigService; +use OC\Files\External\Service\GlobalStoragesService; +use OC\Files\External\StoragesBackendService; +use OCA\Files_External\Lib\Backend\Backend; +use OCP\IUser; +use Test\TestCase; + +/** + * Class GlobalStoragesServiceDeleteUser + * + * @group DB + * @package Test\Files\External\Service + */ +class GlobalStoragesServiceDeleteUserTest extends TestCase { + public function setUp() { + parent::setUp(); + } + + public function tearDown() { + parent::tearDown(); + + //Remove all global storages created + $globalStorageService = \OC::$server->getGlobalStoragesService(); + foreach ($globalStorageService->getAllStorages() as $storage) { + $storageId = $storage->getId(); + $globalStorageService->removeStorage($storageId); + } + } + + public function providesDeleteAllUser() { + return [ + [ + [ + [ + 'applicableUsers' => ['user1', 'user2'], + 'applicableGroups' => ['group1'], + 'priority' => 12, + 'authMechanism' => 'identifier:\Auth\Mechanism', + 'storageClass' => 'OC\Files\Storage\DAV', + 'backendIdentifier' => 'identifier:\Test\Files\External\Backend\DummyBackend', + 'backendOptions' => [ + 'host' => 'http://localhost', + 'root' => 'test', + 'secure' => 'false', + 'user' => 'foo', + 'password' => 'foo' + ] + ], + [ + 'applicableUsers' => ['user1', 'user3'], + 'applicableGroups' => [], + 'priority' => 13, + 'authMechanism' => 'identifier:\Auth\Mechanism', + 'storageClass' => 'OC\Files\Storage\DAV', + 'backendIdentifier' => 'identifier:\Test\Files\External\Backend\DummyBackend2', + 'backendOptions' => [ + 'host' => 'http://localhost', + 'root' => 'test1', + 'secure' => 'false', + 'user' => 'foo', + 'password' => 'foo' + ] + ], + [ + 'applicableUsers' => ['user1', 'user4'], + 'applicableGroups' => [], + 'priority' => 14, + 'storageClass' => 'OC\Files\Storage\DAV', + 'authMechanism' => 'identifier:\Auth\Mechanism', + 'backendIdentifier' => 'identifier:\Test\Files\External\Backend\DummyBackend2', + 'backendOptions' => [ + 'host' => 'http://localhost', + 'root' => 'test2', + 'secure' => 'false', + 'user' => 'foo', + 'password' => 'foo' + ] + ], + //This storage shouldn't be available + [ + 'applicableUsers' => ['user1'], + 'applicableGroups' => [], + 'priority' => 15, + 'authMechanism' => 'identifier:\Auth\Mechanism', + 'storageClass' => 'OC\Files\Storage\DAV', + 'backendIdentifier' => 'identifier:\Test\Files\External\Backend\DummyBackend2', + 'backendOptions' => [ + 'host' => 'http://localhost', + 'root' => 'test3', + 'secure' => 'false', + 'user' => 'foo', + 'password' => 'foo' + ] + ], + ], 'user1' + ] + ]; + } + + /** + * @dataProvider providesDeleteAllUser + * @param $storageParams + */ + public function testDeleteAllForUser($storageParams, $userId) { + $backendService = new StoragesBackendService(\OC::$server->getConfig()); + $dbConfigService = new DBConfigService(\OC::$server->getDatabaseConnection(), \OC::$server->getCrypto()); + $userManager = \OC::$server->getUserManager(); + $userMountCache = new UserMountCache(\OC::$server->getDatabaseConnection(), $userManager, \OC::$server->getLogger()); + $service = new GlobalStoragesService($backendService, $dbConfigService, $userMountCache); + + $storageIds = []; + foreach ($storageParams as $storageParam) { + $backend = new Backend(); + $backend->setIdentifier($storageParam['backendIdentifier']); + $backendService->registerBackend($backend); + $storageConfig = $service->createStorage('/foo/', + $storageParam['backendIdentifier'], $storageParam['authMechanism'], + $storageParam['backendOptions'], null, + $storageParam['applicableUsers'], $storageParam['applicableGroups'], + $storageParam['priority']); + $storageConfig->setBackend($backend); + $backend->setStorageClass($storageParam['storageClass']); + + $newStorage = $service->addStorage($storageConfig); + $storageIds[] = $newStorage->getId(); + } + + $user = $this->createMock(IUser::class); + $user->expects($this->once()) + ->method('getUID') + ->willReturn($userId); + + $this->assertTrue($service->deleteAllForUser($user)); + + $storages = $service->getStorages(); + $newStorageIds = []; + foreach ($storages as $storage) { + $users = $storage->getApplicableUsers(); + $this->assertFalse(\in_array($userId, $users, true)); + $this->assertTrue(\in_array($storage->getId(), $storageIds, true)); + $newStorageIds[] = $storage->getId(); + } + $missingStorageId = \array_pop($storageIds); + $this->assertFalse(\in_array($missingStorageId, $newStorageIds, true)); + } +} diff --git a/tests/lib/Files/External/Service/UserStoragesServiceTest.php b/tests/lib/Files/External/Service/UserStoragesServiceTest.php index e84f661119fd..cc439df85979 100644 --- a/tests/lib/Files/External/Service/UserStoragesServiceTest.php +++ b/tests/lib/Files/External/Service/UserStoragesServiceTest.php @@ -23,12 +23,16 @@ */ namespace Test\Files\External\Service; +use OC\Files\Config\UserMountCache; use OC\Files\External\Service\GlobalStoragesService; use OC\Files\External\Service\UserStoragesService; use OC\Files\External\StorageConfig; use OC\Files\Filesystem; +use OC\Files\Mount\MountPoint; use OCP\Files\External\IStorageConfig; use OCP\Files\External\Service\IStoragesService; +use OCP\ILogger; +use OCP\IUser; use Test\Traits\UserTrait; /** @@ -204,4 +208,62 @@ public function testGetAdminStorage() { $this->service->getStorage($newStorage->getId()); } + + private function getStorage($storageId, $rootId) { + $storageCache = $this->getMockBuilder('\OC\Files\Cache\Storage') + ->disableOriginalConstructor() + ->getMock(); + $storageCache->expects($this->any()) + ->method('getNumericId') + ->will($this->returnValue($storageId)); + + $cache = $this->getMockBuilder('\OC\Files\Cache\Cache') + ->disableOriginalConstructor() + ->getMock(); + $cache->expects($this->any()) + ->method('getId') + ->will($this->returnValue($rootId)); + + $storage = $this->getMockBuilder('\OC\Files\Storage\Storage') + ->disableOriginalConstructor() + ->getMock(); + $storage->expects($this->any()) + ->method('getStorageCache') + ->will($this->returnValue($storageCache)); + $storage->expects($this->any()) + ->method('getCache') + ->will($this->returnValue($cache)); + + return $storage; + } + + public function testDeleteAllMountsForUser() { + $storage1 = $this->getStorage(10, 20); + $storage2 = $this->getStorage(12, 22); + + $mount1 = new MountPoint($storage1, '/foo/'); + $mount2 = new MountPoint($storage2, '/bar/'); + + $dbConnection = \OC::$server->getDatabaseConnection(); + $userManager = \OC::$server->getUserManager(); + $logger = $this->createMock(ILogger::class); + $userMountCache = new UserMountCache($dbConnection, $userManager, $logger); + $user1 = $userManager->createUser('user1', 'user1'); + $user2 = $userManager->createUser('user2', 'user2'); + + $userMountCache->registerMounts($user1, [$mount1, $mount2]); + + $userMountCache->registerMounts($user2, [$mount2]); + + $backendService = \OC::$server->getStoragesBackendService(); + $userSession = \OC::$server->getUserSession(); + $this->service = new UserStoragesService($backendService, $this->dbConfig, $userSession, $userMountCache); + $this->assertTrue($this->service->deleteAllMountsForUser($user1)); + $storarge1Result1 = $userMountCache->getMountsForStorageId(10); + $storarge1Result2 = $userMountCache->getMountsForStorageId(12); + $this->assertEquals(0, \count($storarge1Result1)); + $this->assertEquals(1, \count($storarge1Result2)); + $this->assertEquals(12, $storarge1Result2[0]->getStorageId()); + $this->assertEquals('/bar/', $storarge1Result2[0]->getMountPoint()); + } }