Catch \Error in Transfer::restoreShares

This makes the command more fault tolerant. An \Error can happen when
e.g. the owner of a share is null.
If we don't catch this, the restore process will stop in an unknown
state.

Signed-off-by: Tobia De Koninck <LEDfan@users.noreply.github.com>

apps/files/lib/Command/TransferOwnership.php

@@ -77,7 +77,12 @@ class TransferOwnership extends Command {
 				InputOption::VALUE_REQUIRED,
 				'selectively provide the path to transfer. For example --path="folder_name"',
 				''
-			);
+			)->addOption(
+				'move',
+				null,
+				InputOption::VALUE_NONE,
+				'move data from source user to root directory of destination user, which must be empty'
+		);
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
@@ -99,7 +104,8 @@ class TransferOwnership extends Command {
 				$sourceUserObject,
 				$destinationUserObject,
 				ltrim($input->getOption('path'), '/'),
-				$output
+				$output,
+				$input->hasArgument('move')
 			);
 		} catch (TransferOwnershipException $e) {
 			$output->writeln("<error>" . $e->getMessage() . "</error>");

apps/files/lib/Service/OwnershipTransferService.php

@@ -71,24 +71,32 @@ class OwnershipTransferService {
 	 * @param IUser $destinationUser
 	 * @param string $path
 	 *
+	 * @param OutputInterface|null $output
+	 * @param bool $move
 	 * @throws TransferOwnershipException
+	 * @throws \OC\User\NoUserException
 	 */
 	public function transfer(IUser $sourceUser,
 							 IUser $destinationUser,
 							 string $path,
-							 ?OutputInterface $output = null): void {
+							 ?OutputInterface $output = null,
+							 bool $move = false): void {
 		$output = $output ?? new NullOutput();
 		$sourceUid = $sourceUser->getUID();
 		$destinationUid = $destinationUser->getUID();
 		$sourcePath = rtrim($sourceUid . '/files/' . $path, '/');
 
 		// target user has to be ready
-		if (!$this->encryptionManager->isReadyForUser($destinationUid)) {
+		if ($destinationUser->getLastLogin() === 0 || !$this->encryptionManager->isReadyForUser($destinationUid)) {
 			throw new TransferOwnershipException("The target user is not ready to accept files. The user has at least to have logged in once.", 2);
 		}
 
-		$date = date('Y-m-d H-i-s');
+		if ($move) {
-		$finalTarget = "$destinationUid/files/transferred from $sourceUid on $date";
+			$finalTarget = "$destinationUid/files/";
+		} else {
+			$date = date('Y-m-d H-i-s');
+			$finalTarget = "$destinationUid/files/transferred from $sourceUid on $date";
+		}
 
 		// setup filesystem
 		Filesystem::initMountPoints($sourceUid);
@@ -99,6 +107,11 @@ class OwnershipTransferService {
 			throw new TransferOwnershipException("Unknown path provided: $path", 1);
 		}
 
+		if ($move && (!$view->is_dir($finalTarget) || count($view->getDirectoryContent($finalTarget)) > 0)) {
+			throw new TransferOwnershipException("Destination path does not exists or is not empty", 1);
+		}
+
+
 		// analyse source folder
 		$this->analyse(
 			$sourceUid,
@@ -273,7 +286,7 @@ class OwnershipTransferService {
 				}
 			} catch (\OCP\Files\NotFoundException $e) {
 				$output->writeln('<error>Share with id ' . $share->getId() . ' points at deleted file, skipping</error>');
-			} catch (\Exception $e) {
+			} catch (\Throwable $e) {
 				$output->writeln('<error>Could not restore share with id ' . $share->getId() . ':' . $e->getTraceAsString() . '</error>');
 			}
 			$progress->advance();

lib/private/Share20/Manager.php

@@ -301,7 +301,7 @@ class Manager implements IManager {
 		$isFederatedShare = $share->getNode()->getStorage()->instanceOfStorage('\OCA\Files_Sharing\External\Storage');
 		$permissions = 0;
 		$mount = $share->getNode()->getMountPoint();
-		if (!$isFederatedShare && $share->getNode()->getOwner()->getUID() !== $share->getSharedBy()) {
+		if ($mount->getMountType() !== 'group' && !$isFederatedShare && $share->getNode()->getOwner()->getUID() !== $share->getSharedBy()) {
 			// When it's a reshare use the parent share permissions as maximum
 			$userMountPointId = $mount->getStorageRootId();
 			$userMountPoints = $userFolder->getById($userMountPointId);

tests/lib/Share20/ManagerTest.php

@@ -21,7 +21,6 @@
 
 namespace Test\Share20;
 
-use OC\Files\Mount\MoveableMount;
 use OC\HintException;
 use OC\Share20\DefaultShareProvider;
 use OC\Share20\Exception;
@@ -53,11 +52,13 @@ use OCP\Security\ISecureRandom;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IProviderFactory;
 use OCP\Share\IShare;
+use OC\Files\Mount\MountPoint;
 use OCP\Share\IShareProvider;
 use PHPUnit\Framework\MockObject\MockBuilder;
 use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
+use Test\TestMoveableMountPoint;
 
 /**
  * Class ManagerTest
@@ -624,8 +625,8 @@ class ManagerTest extends \Test\TestCase {
 		$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, null, null, null), 'A share requires permissions', true];
 		$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK,  $limitedPermssions, null, $user0, $user0, null, null, null), 'A share requires permissions', true];
 
-		$mount = $this->createMock(MoveableMount::class);
+		$movableMount = $this->createMock(TestMoveableMountPoint::class);
-		$limitedPermssions->method('getMountPoint')->willReturn($mount);
+		$limitedPermssions->method('getMountPoint')->willReturn($movableMount);
 
 
 		$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER,  $limitedPermssions, $user2, $user0, $user0, 31, null, null), 'Can’t increase permissions of path', true];
@@ -640,6 +641,8 @@ class ManagerTest extends \Test\TestCase {
 			->willReturn($owner);
 		$nonMoveableMountPermssions->method('getStorage')
 			->willReturn($storage);
+		$nonMoveableMountPoint = $this->createMock(MountPoint::class);
+		$nonMoveableMountPermssions->method('getMountPoint')->willReturn($nonMoveableMountPoint);
 
 		$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER,  $nonMoveableMountPermssions, $user2, $user0, $user0, 11, null, null), 'Can’t increase permissions of path', false];
 		$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $nonMoveableMountPermssions, $group0, $user0, $user0, 11, null, null), 'Can’t increase permissions of path', false];
@@ -660,6 +663,7 @@ class ManagerTest extends \Test\TestCase {
 			->willReturn($owner);
 		$allPermssions->method('getStorage')
 			->willReturn($storage);
+		$allPermssions->method('getMountPoint')->willReturn($movableMount);
 
 		$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER,  $allPermssions, $user2, $user0, $user0, 30, null, null), 'Shares need at least read permissions', true];
 		$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 2, null, null), 'Shares need at least read permissions', true];