diff options
author | Luka Trovic <89908051+luka-nextcloud@users.noreply.github.com> | 2024-12-19 14:01:06 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-12-19 14:01:06 +0100 |
commit | 021b335065a3d9451bc1638e34f48a2f99049c30 (patch) | |
tree | 43f3397e2bc28d9b98554174407d715dc165eecd /apps | |
parent | e7035078924888ddcd23a0402f9bf12079c0c8ac (diff) | |
parent | 317c9b2c0557dccbc4b3923685dcc2dfa4b71802 (diff) | |
download | nextcloud-server-021b335065a3d9451bc1638e34f48a2f99049c30.tar.gz nextcloud-server-021b335065a3d9451bc1638e34f48a2f99049c30.zip |
Merge pull request #49612 from nextcloud/backport/43025/stable30
[stable30] fix: Add command to update re-share if shared-by user has been revoked
Diffstat (limited to 'apps')
-rw-r--r-- | apps/files_sharing/appinfo/info.xml | 1 | ||||
-rw-r--r-- | apps/files_sharing/composer/composer/autoload_classmap.php | 1 | ||||
-rw-r--r-- | apps/files_sharing/composer/composer/autoload_static.php | 1 | ||||
-rw-r--r-- | apps/files_sharing/lib/Command/FixShareOwners.php | 65 | ||||
-rw-r--r-- | apps/files_sharing/lib/OrphanHelper.php | 28 | ||||
-rw-r--r-- | apps/files_sharing/tests/Command/FixShareOwnersTest.php | 116 |
6 files changed, 211 insertions, 1 deletions
diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml index ccaccc688b4..b70da4979cc 100644 --- a/apps/files_sharing/appinfo/info.xml +++ b/apps/files_sharing/appinfo/info.xml @@ -48,6 +48,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 e4aa9c7b16a..872898a295b 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 36a2622f56e..a6bec0f9d23 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 94fe4f08318..713d0c64b82 100644 --- a/apps/files_sharing/lib/OrphanHelper.php +++ b/apps/files_sharing/lib/OrphanHelper.php @@ -10,19 +10,23 @@ 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; class OrphanHelper { private IDBConnection $connection; private IRootFolder $rootFolder; + private IUserMountCache $userMountCache; public function __construct( IDBConnection $connection, - IRootFolder $rootFolder + IRootFolder $rootFolder, + IUserMountCache $userMountCache ) { $this->connection = $connection; $this->rootFolder = $rootFolder; + $this->userMountCache = $userMountCache; } public function isShareValid(string $owner, int $fileId): bool { @@ -73,4 +77,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); + } +} |