From edf5493ae8d249bc2dc63be6af414f499cde5f6d Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 23 Jun 2025 08:24:13 +0200 Subject: [PATCH 1/3] fix(files): Limit transferring incoming shares to the selected path Signed-off-by: provokateurin --- .../lib/Service/OwnershipTransferService.php | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index b3a36ee13e5..fd98415df31 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -10,6 +10,7 @@ declare(strict_types=1); namespace OCA\Files\Service; use Closure; +use Exception; use OC\Files\Filesystem; use OC\Files\View; use OC\User\NoUserException; @@ -19,6 +20,7 @@ use OCA\Files_External\Config\ConfigAdapter; use OCP\Encryption\IManager as IEncryptionManager; use OCP\Files\Config\IHomeMountProvider; use OCP\Files\Config\IUserMountCache; +use OCP\Files\File; use OCP\Files\FileInfo; use OCP\Files\InvalidPathException; use OCP\Files\IRootFolder; @@ -161,13 +163,12 @@ class OwnershipTransferService { $sourceShares = $this->collectIncomingShares( $sourceUid, $output, - $view + $sourcePath, ); $destinationShares = $this->collectIncomingShares( $destinationUid, $output, - $view, - true + null, ); $this->transferIncomingShares( $sourceUid, @@ -344,7 +345,7 @@ class OwnershipTransferService { return mb_strpos( Filesystem::normalizePath($relativePath . '/', false), $normalizedPath . '/') === 0; - } catch (\Exception $e) { + } catch (Exception $e) { return false; } }); @@ -372,14 +373,16 @@ class OwnershipTransferService { }, $shares))); } - private function collectIncomingShares(string $sourceUid, + private function collectIncomingShares( + string $sourceUid, OutputInterface $output, - View $view, - bool $addKeys = false): array { + ?string $path, + ): array { $output->writeln("Collecting all incoming share information for files and folders of $sourceUid ..."); $shares = []; $progress = new ProgressBar($output); + $normalizedPath = Filesystem::normalizePath($path); $offset = 0; while (true) { @@ -388,14 +391,19 @@ class OwnershipTransferService { if (empty($sharePage)) { break; } - if ($addKeys) { - foreach ($sharePage as $singleShare) { - $shares[$singleShare->getNodeId()] = $singleShare; - } - } else { - foreach ($sharePage as $singleShare) { - $shares[] = $singleShare; - } + + if ($path !== null && $path !== "$sourceUid/files") { + $sharePage = array_filter($sharePage, static function (IShare $share) use ($sourceUid, $normalizedPath) { + try { + return str_starts_with(Filesystem::normalizePath($sourceUid . '/files' . $share->getTarget() . '/', false), $normalizedPath . '/'); + } catch (Exception) { + return false; + } + }); + } + + foreach ($sharePage as $share) { + $shares[$share->getNodeId()] = $share; } $offset += 50; From 2e5ccc7123d808c9d85bfcc20aa53e05ebf01088 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 3 Jun 2025 16:16:19 +0200 Subject: [PATCH 2/3] fix(files): Always transfer incoming shares when doing ownership transfer to prevent broken reshares Signed-off-by: provokateurin --- apps/files/lib/Command/TransferOwnership.php | 24 +-------- .../lib/Service/OwnershipTransferService.php | 43 ++++++++-------- .../features/bootstrap/CommandLineContext.php | 13 ----- .../files_features/transfer-ownership.feature | 51 ++++++------------- 4 files changed, 36 insertions(+), 95 deletions(-) diff --git a/apps/files/lib/Command/TransferOwnership.php b/apps/files/lib/Command/TransferOwnership.php index 104a8fb4985..f7663e26f28 100644 --- a/apps/files/lib/Command/TransferOwnership.php +++ b/apps/files/lib/Command/TransferOwnership.php @@ -64,7 +64,7 @@ class TransferOwnership extends Command { 'transfer-incoming-shares', null, InputOption::VALUE_OPTIONAL, - 'transfer incoming user file shares to destination user. Usage: --transfer-incoming-shares=1 (value required)', + 'Incoming shares are always transferred now, so this option does not affect the ownership transfer anymore', '2' )->addOption( 'include-external-storage', @@ -129,27 +129,6 @@ class TransferOwnership extends Command { } try { - $includeIncomingArgument = $input->getOption('transfer-incoming-shares'); - - switch ($includeIncomingArgument) { - case '0': - $includeIncoming = false; - break; - case '1': - $includeIncoming = true; - break; - case '2': - $includeIncoming = $this->config->getSystemValue('transferIncomingShares', false); - if (gettype($includeIncoming) !== 'boolean') { - $output->writeln(" config.php: 'transfer-incoming-shares': wrong usage. Transfer aborted."); - return self::FAILURE; - } - break; - default: - $output->writeln('Option --transfer-incoming-shares: wrong usage. Transfer aborted.'); - return self::FAILURE; - } - $this->transferService->transfer( $sourceUserObject, $destinationUserObject, @@ -157,7 +136,6 @@ class TransferOwnership extends Command { $output, $input->getOption('move') === true, false, - $includeIncoming, $includeExternalStorage, ); } catch (TransferOwnershipException $e) { diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index fd98415df31..d2ee03481be 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -72,7 +72,6 @@ class OwnershipTransferService { ?OutputInterface $output = null, bool $move = false, bool $firstLogin = false, - bool $transferIncomingShares = false, bool $includeExternalStorage = false, ): void { $output = $output ?? new NullOutput(); @@ -159,28 +158,26 @@ class OwnershipTransferService { $sizeDifference = $sourceSize - $view->getFileInfo($finalTarget)->getSize(); // transfer the incoming shares - if ($transferIncomingShares === true) { - $sourceShares = $this->collectIncomingShares( - $sourceUid, - $output, - $sourcePath, - ); - $destinationShares = $this->collectIncomingShares( - $destinationUid, - $output, - null, - ); - $this->transferIncomingShares( - $sourceUid, - $destinationUid, - $sourceShares, - $destinationShares, - $output, - $path, - $finalTarget, - $move - ); - } + $sourceShares = $this->collectIncomingShares( + $sourceUid, + $output, + $sourcePath, + ); + $destinationShares = $this->collectIncomingShares( + $destinationUid, + $output, + null, + ); + $this->transferIncomingShares( + $sourceUid, + $destinationUid, + $sourceShares, + $destinationShares, + $output, + $path, + $finalTarget, + $move + ); $destinationPath = $finalTarget . '/' . $path; // restore the shares diff --git a/build/integration/features/bootstrap/CommandLineContext.php b/build/integration/features/bootstrap/CommandLineContext.php index 5ea8d12a970..b2ca1a45bf7 100644 --- a/build/integration/features/bootstrap/CommandLineContext.php +++ b/build/integration/features/bootstrap/CommandLineContext.php @@ -108,19 +108,6 @@ class CommandLineContext implements \Behat\Behat\Context\Context { } } - /** - * @When /^transferring ownership of path "([^"]+)" from "([^"]+)" to "([^"]+)" with received shares$/ - */ - public function transferringOwnershipPathWithIncomingShares($path, $user1, $user2) { - $path = '--path=' . $path; - if ($this->runOcc(['files:transfer-ownership', $path, $user1, $user2, '--transfer-incoming-shares=1']) === 0) { - $this->lastTransferPath = $this->findLastTransferFolderForUser($user1, $user2); - } else { - // failure - $this->lastTransferPath = null; - } - } - /** * @When /^using received transfer folder of "([^"]+)" as dav path$/ */ diff --git a/build/integration/files_features/transfer-ownership.feature b/build/integration/files_features/transfer-ownership.feature index 4e5407cadbb..6f7a7944166 100644 --- a/build/integration/files_features/transfer-ownership.feature +++ b/build/integration/files_features/transfer-ownership.feature @@ -184,10 +184,10 @@ Feature: transfer-ownership And As an "user2" Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" And using old dav path - And as "user0" the folder "/test" exists + And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path - And as "user1" the folder "/test" does not exist - And As an "user0" + And as "user1" the folder "/test" exists + And As an "user1" And Getting info of last share And the OCS status code should be "100" And Share fields of last share match with @@ -210,13 +210,12 @@ Feature: transfer-ownership And user "user1" accepts last share When transferring ownership from "user0" to "user1" And the command was successful - And As an "user1" - Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" And using old dav path - And as "user0" the folder "/test" exists + Then as "user0" the folder "/test" does not exist + When As an "user1" And using received transfer folder of "user1" as dav path - And as "user1" the folder "/test" does not exist - And As an "user1" + Then as "user1" the folder "/test" exists + And Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" And Getting info of last share And the OCS status code should be "100" And Share fields of last share match with @@ -242,10 +241,10 @@ Feature: transfer-ownership And As an "user2" Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" And using old dav path - And as "user0" the folder "/test" exists + And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path - And as "user1" the folder "/test" does not exist - And As an "user0" + And as "user1" the folder "/test" exists + And As an "user1" And Getting info of last share And the OCS status code should be "100" And Share fields of last share match with @@ -253,7 +252,7 @@ Feature: transfer-ownership | uid_file_owner | user3 | | share_with | group1 | - Scenario: transferring ownership does not transfer received shares + Scenario: transferring ownership transfers received shares Given user "user0" exists And user "user1" exists And user "user2" exists @@ -264,16 +263,16 @@ Feature: transfer-ownership And the command was successful And As an "user1" And using received transfer folder of "user1" as dav path - Then as "user1" the folder "/test" does not exist + Then as "user1" the folder "/test" exists And using old dav path - And as "user0" the folder "/test" exists + And as "user0" the folder "/test" does not exist And As an "user2" And Getting info of last share And the OCS status code should be "100" And Share fields of last share match with | uid_owner | user2 | | uid_file_owner | user2 | - | share_with | user0 | + | share_with | user1 | @local_storage Scenario: transferring ownership does not transfer external storage @@ -516,26 +515,6 @@ Feature: transfer-ownership Then the command failed with exit code 1 And the command error output contains the text "Moving a storage (user0/files/test) into another storage (user1) is not allowed" - Scenario: transferring ownership does not transfer received shares - Given user "user0" exists - And user "user1" exists - And user "user2" exists - And User "user2" created a folder "/test" - And User "user0" created a folder "/sub" - And folder "/test" of user "user2" is shared with user "user0" with permissions 31 - And user "user0" accepts last share - And User "user0" moved folder "/test" to "/sub/test" - When transferring ownership of path "sub" from "user0" to "user1" - And the command was successful - And As an "user1" - And using received transfer folder of "user1" as dav path - Then as "user1" the folder "/sub" exists - And as "user1" the folder "/sub/test" does not exist - And using old dav path - And as "user0" the folder "/sub" does not exist - And Getting info of last share - And the OCS status code should be "404" - Scenario: transferring ownership transfers received shares into subdir when requested Given user "user0" exists And user "user1" exists @@ -548,7 +527,7 @@ Feature: transfer-ownership And User "user0" moved folder "/transfer-share" to "/sub/transfer-share" And folder "/do-not-transfer" of user "user2" is shared with user "user0" with permissions 31 And user "user0" accepts last share - When transferring ownership of path "sub" from "user0" to "user1" with received shares + When transferring ownership of path "sub" from "user0" to "user1" And the command was successful And As an "user1" And using received transfer folder of "user1" as dav path From 8e580f85485607f39fbbe42f274bbf253d5a9444 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 3 Jun 2025 17:40:51 +0200 Subject: [PATCH 3/3] fix(files_sharing): Hide own reshares Signed-off-by: provokateurin --- apps/files_sharing/lib/MountProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 91c392de6eb..24d14630303 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -55,7 +55,7 @@ class MountProvider implements IMountProvider { // filter out excluded shares and group shares that includes self $shares = array_filter($shares, function (IShare $share) use ($user) { - return $share->getPermissions() > 0 && $share->getShareOwner() !== $user->getUID(); + return $share->getPermissions() > 0 && $share->getShareOwner() !== $user->getUID() && $share->getSharedBy() !== $user->getUID(); }); $superShares = $this->buildSuperShares($shares, $user);