fix(sharing): add command to fix broken shares after ownership transferringbugfix/error-on-reshare-after-transfer-ownership

Signed-off-by: Luka Trovic <luka@nextcloud.com>

6 files changed, 208 insertions, 0 deletions

diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml
index dcf6621b183..06bf6cdc9e1 100644
--- a/apps/files_sharing/appinfo/info.xml
+++ b/apps/files_sharing/appinfo/info.xml
@@ -49,6 +49,7 @@ Turning the feature off removes shared files and folders on the server for all s
 		<command>OCA\Files_Sharing\Command\CleanupRemoteStorages</command>
 		<command>OCA\Files_Sharing\Command\ExiprationNotification</command>
 		<command>OCA\Files_Sharing\Command\DeleteOrphanShares</command>
+		<command>OCA\Files_Sharing\Command\FixShareOwners</command>
 	</commands>
 
 	<settings>
diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php
index e3c94a7ac1a..b42d1555263 100644
--- a/apps/files_sharing/composer/composer/autoload_classmap.php
+++ b/apps/files_sharing/composer/composer/autoload_classmap.php
@@ -27,6 +27,7 @@ return array(
     'OCA\\Files_Sharing\\Command\\CleanupRemoteStorages' => $baseDir . '/../lib/Command/CleanupRemoteStorages.php',
     'OCA\\Files_Sharing\\Command\\DeleteOrphanShares' => $baseDir . '/../lib/Command/DeleteOrphanShares.php',
     'OCA\\Files_Sharing\\Command\\ExiprationNotification' => $baseDir . '/../lib/Command/ExiprationNotification.php',
+    'OCA\\Files_Sharing\\Command\\FixShareOwners' => $baseDir . '/../lib/Command/FixShareOwners.php',
     'OCA\\Files_Sharing\\Controller\\AcceptController' => $baseDir . '/../lib/Controller/AcceptController.php',
     'OCA\\Files_Sharing\\Controller\\DeletedShareAPIController' => $baseDir . '/../lib/Controller/DeletedShareAPIController.php',
     'OCA\\Files_Sharing\\Controller\\ExternalSharesController' => $baseDir . '/../lib/Controller/ExternalSharesController.php',
diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php
index 597e65d96a2..bb364256947 100644
--- a/apps/files_sharing/composer/composer/autoload_static.php
+++ b/apps/files_sharing/composer/composer/autoload_static.php
@@ -42,6 +42,7 @@ class ComposerStaticInitFiles_Sharing
         'OCA\\Files_Sharing\\Command\\CleanupRemoteStorages' => __DIR__ . '/..' . '/../lib/Command/CleanupRemoteStorages.php',
         'OCA\\Files_Sharing\\Command\\DeleteOrphanShares' => __DIR__ . '/..' . '/../lib/Command/DeleteOrphanShares.php',
         'OCA\\Files_Sharing\\Command\\ExiprationNotification' => __DIR__ . '/..' . '/../lib/Command/ExiprationNotification.php',
+        'OCA\\Files_Sharing\\Command\\FixShareOwners' => __DIR__ . '/..' . '/../lib/Command/FixShareOwners.php',
         'OCA\\Files_Sharing\\Controller\\AcceptController' => __DIR__ . '/..' . '/../lib/Controller/AcceptController.php',
         'OCA\\Files_Sharing\\Controller\\DeletedShareAPIController' => __DIR__ . '/..' . '/../lib/Controller/DeletedShareAPIController.php',
         'OCA\\Files_Sharing\\Controller\\ExternalSharesController' => __DIR__ . '/..' . '/../lib/Controller/ExternalSharesController.php',
diff --git a/apps/files_sharing/lib/Command/FixShareOwners.php b/apps/files_sharing/lib/Command/FixShareOwners.php
new file mode 100644
index 00000000000..1cf5f82f5a8
--- /dev/null
+++ b/apps/files_sharing/lib/Command/FixShareOwners.php
@@ -0,0 +1,65 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Files_Sharing\Command;
+
+use OC\Core\Command\Base;
+use OCA\Files_Sharing\OrphanHelper;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class FixShareOwners extends Base {
+	public function __construct(
+		private readonly OrphanHelper $orphanHelper,
+	) {
+		parent::__construct();
+	}
+
+	protected function configure(): void {
+		$this
+			->setName('sharing:fix-share-owners')
+			->setDescription('Fix owner of broken shares after transfer ownership on old versions')
+			->addOption(
+				'dry-run',
+				null,
+				InputOption::VALUE_NONE,
+				'only show which shares would be updated'
+			);
+	}
+
+	public function execute(InputInterface $input, OutputInterface $output): int {
+		$shares = $this->orphanHelper->getAllShares();
+		$dryRun = $input->getOption('dry-run');
+		$count = 0;
+
+		foreach ($shares as $share) {
+			if ($this->orphanHelper->isShareValid($share['owner'], $share['fileid']) || !$this->orphanHelper->fileExists($share['fileid'])) {
+				continue;
+			}
+
+			$owner = $this->orphanHelper->findOwner($share['fileid']);
+
+			if ($owner !== null) {
+				if ($dryRun) {
+					$output->writeln("Share with id <info>{$share['id']}</info> (target: <info>{$share['target']}</info>) can be updated to owner <info>$owner</info>");
+				} else {
+					$this->orphanHelper->updateShareOwner($share['id'], $owner);
+					$output->writeln("Share with id <info>{$share['id']}</info> (target: <info>{$share['target']}</info>) updated to owner <info>$owner</info>");
+				}
+				$count++;
+			}
+		}
+
+		if ($count === 0) {
+			$output->writeln('No broken shares detected');
+		}
+
+		return static::SUCCESS;
+	}
+}
diff --git a/apps/files_sharing/lib/OrphanHelper.php b/apps/files_sharing/lib/OrphanHelper.php
index 220de619c87..4a52af0406c 100644
--- a/apps/files_sharing/lib/OrphanHelper.php
+++ b/apps/files_sharing/lib/OrphanHelper.php
@@ -10,6 +10,7 @@ namespace OCA\Files_Sharing;
 
 use OC\User\NoUserException;
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\Files\Config\IUserMountCache;
 use OCP\Files\IRootFolder;
 use OCP\IDBConnection;
 
@@ -17,6 +18,7 @@ class OrphanHelper {
 	public function __construct(
 		private IDBConnection $connection,
 		private IRootFolder $rootFolder,
+		private IUserMountCache $userMountCache,
 	) {
 	}
 
@@ -68,4 +70,26 @@ class OrphanHelper {
 			];
 		}
 	}
+
+	public function findOwner(int $fileId): ?string {
+		$mounts = $this->userMountCache->getMountsForFileId($fileId);
+		if (!$mounts) {
+			return null;
+		}
+		foreach ($mounts as $mount) {
+			$userHomeMountPoint = '/' . $mount->getUser()->getUID() . '/';
+			if ($mount->getMountPoint() === $userHomeMountPoint) {
+				return $mount->getUser()->getUID();
+			}
+		}
+		return null;
+	}
+
+	public function updateShareOwner(int $shareId, string $owner): void {
+		$query = $this->connection->getQueryBuilder();
+		$query->update('share')
+			->set('uid_owner', $query->createNamedParameter($owner))
+			->where($query->expr()->eq('id', $query->createNamedParameter($shareId, IQueryBuilder::PARAM_INT)));
+		$query->executeStatement();
+	}
 }
diff --git a/apps/files_sharing/tests/Command/FixShareOwnersTest.php b/apps/files_sharing/tests/Command/FixShareOwnersTest.php
new file mode 100644
index 00000000000..939fad03d7c
--- /dev/null
+++ b/apps/files_sharing/tests/Command/FixShareOwnersTest.php
@@ -0,0 +1,116 @@
+<?php
+/**
+ * SPDX-FileCopyrightText: 2017-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+namespace OCA\Files_Sharing\Tests\Command;
+
+use OCA\Files_Sharing\Command\FixShareOwners;
+use OCA\Files_Sharing\OrphanHelper;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Test\TestCase;
+
+/**
+ * Class FixShareOwnersTest
+ *
+ * @package OCA\Files_Sharing\Tests\Command
+ */
+class FixShareOwnersTest extends TestCase {
+	/**
+	 * @var FixShareOwners
+	 */
+	private $command;
+
+	/**
+	 * @var OrphanHelper|\PHPUnit\Framework\MockObject\MockObject
+	 */
+	private $orphanHelper;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->orphanHelper = $this->createMock(OrphanHelper::class);
+		$this->command = new FixShareOwners($this->orphanHelper);
+	}
+
+	public function testExecuteNoSharesDetected() {
+		$this->orphanHelper->expects($this->once())
+			->method('getAllShares')
+			->willReturn([
+				['id' => 1, 'owner' => 'user1', 'fileid' => 1, 'target' => 'target1'],
+				['id' => 2, 'owner' => 'user2', 'fileid' => 2, 'target' => 'target2'],
+			]);
+		$this->orphanHelper->expects($this->exactly(2))
+			->method('isShareValid')
+			->willReturn(true);
+
+		$input = $this->createMock(InputInterface::class);
+		$output = $this->createMock(OutputInterface::class);
+
+		$output->expects($this->once())
+			->method('writeln')
+			->with('No broken shares detected');
+		$this->command->execute($input, $output);
+	}
+
+	public function testExecuteSharesDetected() {
+		$this->orphanHelper->expects($this->once())
+			->method('getAllShares')
+			->willReturn([
+				['id' => 1, 'owner' => 'user1', 'fileid' => 1, 'target' => 'target1'],
+				['id' => 2, 'owner' => 'user2', 'fileid' => 2, 'target' => 'target2'],
+			]);
+		$this->orphanHelper->expects($this->exactly(2))
+			->method('isShareValid')
+			->willReturnOnConsecutiveCalls(true, false);
+		$this->orphanHelper->expects($this->once())
+			->method('fileExists')
+			->willReturn(true);
+		$this->orphanHelper->expects($this->once())
+			->method('findOwner')
+			->willReturn('newOwner');
+		$this->orphanHelper->expects($this->once())
+			->method('updateShareOwner');
+
+		$input = $this->createMock(InputInterface::class);
+		$output = $this->createMock(OutputInterface::class);
+
+		$output->expects($this->once())
+			->method('writeln')
+			->with('Share with id <info>2</info> (target: <info>target2</info>) updated to owner <info>newOwner</info>');
+		$this->command->execute($input, $output);
+	}
+
+	public function testExecuteSharesDetectedDryRun() {
+		$this->orphanHelper->expects($this->once())
+			->method('getAllShares')
+			->willReturn([
+				['id' => 1, 'owner' => 'user1', 'fileid' => 1, 'target' => 'target1'],
+				['id' => 2, 'owner' => 'user2', 'fileid' => 2, 'target' => 'target2'],
+			]);
+		$this->orphanHelper->expects($this->exactly(2))
+			->method('isShareValid')
+			->willReturnOnConsecutiveCalls(true, false);
+		$this->orphanHelper->expects($this->once())
+			->method('fileExists')
+			->willReturn(true);
+		$this->orphanHelper->expects($this->once())
+			->method('findOwner')
+			->willReturn('newOwner');
+		$this->orphanHelper->expects($this->never())
+			->method('updateShareOwner');
+
+		$input = $this->createMock(InputInterface::class);
+		$output = $this->createMock(OutputInterface::class);
+
+		$output->expects($this->once())
+			->method('writeln')
+			->with('Share with id <info>2</info> (target: <info>target2</info>) can be updated to owner <info>newOwner</info>');
+		$input->expects($this->once())
+			->method('getOption')
+			->with('dry-run')
+			->willReturn(true);
+		$this->command->execute($input, $output);
+	}
+}