From bff24d10dee527e8dbc5ebb7e12d0695a2648f2f Mon Sep 17 00:00:00 2001 From: Luka Trovic Date: Thu, 7 Nov 2024 19:36:36 +0100 Subject: [PATCH] fix(sharing): add command to fix broken shares after ownership transferring Signed-off-by: Luka Trovic --- apps/files_sharing/appinfo/info.xml | 1 + .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Command/FixShareOwners.php | 65 ++++++++++ apps/files_sharing/lib/OrphanHelper.php | 24 ++++ .../tests/Command/FixShareOwnersTest.php | 116 ++++++++++++++++++ 6 files changed, 208 insertions(+) create mode 100644 apps/files_sharing/lib/Command/FixShareOwners.php create mode 100644 apps/files_sharing/tests/Command/FixShareOwnersTest.php diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml index dcf6621b18347..4d911b7ee5305 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 OCA\Files_Sharing\Command\CleanupRemoteStorages OCA\Files_Sharing\Command\ExiprationNotification OCA\Files_Sharing\Command\DeleteOrphanShares + OCA\Files_Sharing\Command\FixBrokenShares diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index e3c94a7ac1a94..b42d155526317 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -27,6 +27,7 @@ '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 597e65d96a261..bb3642569474c 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 0000000000000..55d61c4ccd213 --- /dev/null +++ b/apps/files_sharing/lib/Command/FixShareOwners.php @@ -0,0 +1,65 @@ +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 {$share['id']} (target: {$share['target']}) can be updated to owner $owner"); + } else { + $this->orphanHelper->updateShareOwner($share['id'], $owner); + $output->writeln("Share with id {$share['id']} (target: {$share['target']}) updated to owner $owner"); + } + $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 220de619c876d..4a52af0406cfa 100644 --- a/apps/files_sharing/lib/OrphanHelper.php +++ b/apps/files_sharing/lib/OrphanHelper.php @@ -10,6 +10,7 @@ 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 @@ public function getAllShares() { ]; } } + + 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 0000000000000..939fad03d7cce --- /dev/null +++ b/apps/files_sharing/tests/Command/FixShareOwnersTest.php @@ -0,0 +1,116 @@ +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 2 (target: target2) updated to owner newOwner'); + $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 2 (target: target2) can be updated to owner newOwner'); + $input->expects($this->once()) + ->method('getOption') + ->with('dry-run') + ->willReturn(true); + $this->command->execute($input, $output); + } +}