diff --git a/apps/files/lib/Command/TransferOwnership.php b/apps/files/lib/Command/TransferOwnership.php index e8bcf8f968f0..a9cc0c87a1b7 100644 --- a/apps/files/lib/Command/TransferOwnership.php +++ b/apps/files/lib/Command/TransferOwnership.php @@ -30,7 +30,6 @@ use OC\Share20\ProviderFactory; use OCP\Files\FileInfo; use OCP\Files\Mount\IMountManager; -use OCP\ILogger; use OCP\IUserManager; use OCP\Share\IManager; use OCP\Share\IShare; @@ -55,9 +54,6 @@ class TransferOwnership extends Command { /** @var Manager */ private $encryptionManager; - /** @var ILogger */ - private $logger; - /** @var ProviderFactory */ private $shareProviderFactory; @@ -85,12 +81,11 @@ class TransferOwnership extends Command { /** @var string */ private $finalTarget; - public function __construct(IUserManager $userManager, IManager $shareManager, IMountManager $mountManager, Manager $encryptionManager, ILogger $logger, ProviderFactory $shareProviderFactory) { + public function __construct(IUserManager $userManager, IManager $shareManager, IMountManager $mountManager, Manager $encryptionManager, ProviderFactory $shareProviderFactory) { $this->userManager = $userManager; $this->shareManager = $shareManager; $this->mountManager = $mountManager; $this->encryptionManager = $encryptionManager; - $this->logger = $logger; $this->shareProviderFactory = $shareProviderFactory; parent::__construct(); } @@ -135,7 +130,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { $this->inputPath = \ltrim($this->inputPath, '/'); // target user has to be ready - if (!\OC::$server->getEncryptionManager()->isReadyForUser($this->destinationUser)) { + if (!$this->encryptionManager->isReadyForUser($this->destinationUser)) { $output->writeln("The target user is not ready to accept files. The user has at least to be logged in once."); return 2; } @@ -196,7 +191,6 @@ protected function analyse(OutputInterface $output) { $output->writeln("Analysing files of $this->sourceUser ..."); $progress = new ProgressBar($output); $progress->start(); - $self = $this; $walkPath = "$this->sourceUser/files"; if (\strlen($this->inputPath) > 0) { if ($this->inputPath !== "$this->sourceUser/files") { @@ -257,11 +251,24 @@ private function collectUsersShares(OutputInterface $output) { $offset = 0; while (true) { $sharePage = $this->shareManager->getSharesBy($this->sourceUser, $shareType, null, true, 50, $offset); - $progress->advance(\count($sharePage)); if (empty($sharePage)) { break; } + if (\strlen($this->inputPath)>1) { + $inputPathWithEndingSlash = \rtrim($this->inputPath, '/') . '/'; + /** + * filter only shares within the source folder + */ + $sharePage = \array_filter($sharePage, function (IShare $share) use ($inputPathWithEndingSlash) { + /** + * trim sharePath, because `/` character trimmed with ltrim in inputPath + */ + $trimmedSharePath = \ltrim($share->getNode()->getPath(), '/'); + return $trimmedSharePath === $this->inputPath || (\strpos($trimmedSharePath, $inputPathWithEndingSlash) === 0); + }); + } + $progress->advance(\count($sharePage)); $this->shares = \array_merge($this->shares, $sharePage); $offset += 50; } diff --git a/apps/files/tests/Command/TransferOwnershipTest.php b/apps/files/tests/Command/TransferOwnershipTest.php new file mode 100644 index 000000000000..ed696b859a12 --- /dev/null +++ b/apps/files/tests/Command/TransferOwnershipTest.php @@ -0,0 +1,214 @@ + + * + * @copyright Copyright (c) 2019, 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 OCA\Files\Tests\Command; + +use OC\Encryption\Manager; +use OC\Share20\ProviderFactory; +use OCA\Files\Command\TransferOwnership; +use OCP\Files\Mount\IMountManager; +use OCP\IUser; +use OCP\IUserManager; +use OCP\Share; +use OCP\Share\IManager; +use PHPUnit\Framework\MockObject\MockObject; +use Symfony\Component\Console\Tester\CommandTester; +use Test\TestCase; +use Test\Traits\UserTrait; + +/** + * Class TransferOwnershipTest + * + * @group DB + * + * @package OCA\Files\Tests\Command + */ +class TransferOwnershipTest extends TestCase { + use UserTrait; + + /** + * @var IUserManager + */ + private $userManager; + + /** + * @var IManager + */ + private $shareManager; + + /** + * @var IMountManager + */ + private $mountManager; + + /** + * @var Manager | MockObject + */ + private $encryptionManager; + + /** + * @var ProviderFactory + */ + private $providerFactory; + + /** + * @var IUser + */ + private $sourceUser; + + /** + * @var IUser + */ + private $targetUser; + + /** + * @var CommandTester + */ + private $commandTester; + + protected function setup() { + parent::setUp(); + $this->userManager = \OC::$server->getUserManager(); + $this->shareManager = \OC::$server->getShareManager(); + $this->mountManager = \OC::$server->getMountManager(); + $this->encryptionManager = $this->createMock(Manager::class); + $this->providerFactory = new ProviderFactory(\OC::$server); + + $this->sourceUser = $this->createUser('source-user'); + $this->targetUser = $this->createUser('target-user'); + $this->loginAsUser('source-user'); + $this->logout(); + $this->loginAsUser('target-user'); + $this->logout(); + $this->createUser('share-receiver'); + $this->createTestFilesForSourceUser(); + + $command = new TransferOwnership( + $this->userManager, + $this->shareManager, + $this->mountManager, + $this->encryptionManager, + $this->providerFactory + ); + $this->commandTester = new CommandTester($command); + } + protected function tearDown() { + $this->tearDownUserTrait(); + $this->shareManager->userDeleted('share-receiver'); + $this->shareManager->userDeleted($this->sourceUser->getUID()); + $this->shareManager->userDeleted($this->targetUser->getUID()); + parent::tearDown(); + } + + /** + * Creates files and folder for source-user as the following tree: + * + * ├── file_in_user_root_folder + * ├── shared_file_in_user_root_folder (shared with share-receiver) + * ├── transfer + * │ ├── shared_file (shared with share-receiver) + * │ ├── test_file1 + * │ ├── test_file2 + * │ ├── sub_folder + * │ ├── shared_file_in_sub_folder (shared with share-receiver) + */ + private function createTestFilesForSourceUser() { + $userFolder = \OC::$server->getUserFolder($this->sourceUser->getUID()); + $userFolder->newFile('file_in_user_root_folder'); + $file = $userFolder->newFile('shared_file_in_user_root_folder'); + $share = $this->shareManager->newShare(); + $share->setNode($file) + ->setSharedBy('source-user') + ->setSharedWith('share-receiver') + ->setShareType(Share::SHARE_TYPE_USER) + ->setPermissions(19); + $this->shareManager->createShare($share); + + $userFolder->newFolder('transfer'); + $userFolder->newFile('transfer/test_file1'); + $userFolder->newFile('transfer/test_file2'); + $file = $userFolder->newFile('transfer/shared_file'); + $share = $this->shareManager->newShare(); + $share->setNode($file) + ->setSharedBy('source-user') + ->setSharedWith('share-receiver') + ->setShareType(Share::SHARE_TYPE_USER) + ->setPermissions(19); + $this->shareManager->createShare($share); + + $userFolder->newFolder('transfer/sub_folder'); + $file = $userFolder->newFile('transfer/sub_folder/shared_file_in_sub_folder'); + $share = $this->shareManager->newShare(); + $share->setNode($file) + ->setSharedBy('source-user') + ->setSharedWith('share-receiver') + ->setShareType(Share::SHARE_TYPE_USER) + ->setPermissions(19); + $this->shareManager->createShare($share); + } + + public function testTransferAllFiles() { + $this->encryptionManager->method('isReadyForUser')->willReturn(true); + $input = [ + 'source-user' => $this->sourceUser->getUID(), + 'destination-user' => $this->targetUser->getUID() + ]; + $this->commandTester->execute($input); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('Transferring files to target-user', $output); + $sourceShares = $this->shareManager->getSharesBy($this->sourceUser->getUID(), Share::SHARE_TYPE_USER); + $targetShares = $this->shareManager->getSharesBy($this->targetUser->getUID(), Share::SHARE_TYPE_USER); + $this->assertCount(0, $sourceShares); + $this->assertCount(3, $targetShares); + } + + public function folderPathProvider() { + return [ + ['transfer', 1, 2], + ['transfer/sub_folder', 2, 1] + ]; + } + + /** + * @dataProvider folderPathProvider + * + * @param string $path + * @param int $expectedSourceShareCount + * @param int $expectedTargetShareCount + */ + public function testTransferSpecificFolder($path, $expectedSourceShareCount, $expectedTargetShareCount) { + $this->encryptionManager->method('isReadyForUser')->willReturn(true); + $input = [ + 'source-user' => $this->sourceUser->getUID(), + 'destination-user' => $this->targetUser->getUID(), + '--path' => $path + ]; + $this->commandTester->execute($input); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('Transferring files to target-user', $output); + $sourceShares = $this->shareManager->getSharesBy($this->sourceUser->getUID(), Share::SHARE_TYPE_USER); + $targetShares = $this->shareManager->getSharesBy($this->targetUser->getUID(), Share::SHARE_TYPE_USER); + $this->assertCount($expectedSourceShareCount, $sourceShares); + $this->assertCount($expectedTargetShareCount, $targetShares); + } +} diff --git a/changelog/unreleased/36222 b/changelog/unreleased/36222 new file mode 100644 index 000000000000..4dae07acb461 --- /dev/null +++ b/changelog/unreleased/36222 @@ -0,0 +1,6 @@ +Bugfix: only collect shares inside the given path in files:transfer-ownership command + +Even path argument given, files:transfer-ownership command was trying to transfer all shares of sourceUser. +This situation caused random errors. We fixed this unintended behavior. + +https://github.com/owncloud/core/pull/36222