From 918473d7f2e5d8b0f16997724d153ca0d2fd30d3 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 7f6681a9b89..ffccc550e2a 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; @@ -17,6 +18,7 @@ use OCA\Encryption\Util; use OCA\Files\Exception\TransferOwnershipException; use OCP\Encryption\IManager as IEncryptionManager; use OCP\Files\Config\IUserMountCache; +use OCP\Files\File; use OCP\Files\FileInfo; use OCP\Files\IHomeStorage; use OCP\Files\InvalidPathException; @@ -158,13 +160,12 @@ class OwnershipTransferService { $sourceShares = $this->collectIncomingShares( $sourceUid, $output, - $view + $sourcePath, ); $destinationShares = $this->collectIncomingShares( $destinationUid, $output, - $view, - true + null, ); $this->transferIncomingShares( $sourceUid, @@ -333,7 +334,7 @@ class OwnershipTransferService { return mb_strpos( Filesystem::normalizePath($relativePath . '/', false), $normalizedPath . '/') === 0; - } catch (\Exception $e) { + } catch (Exception $e) { return false; } }); @@ -361,14 +362,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) { @@ -377,14 +380,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 987af4193d58baa956dfd4c6d561d1b1be462f1d 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 edc73e62c38..c235fc88960 100644 --- a/apps/files/lib/Command/TransferOwnership.php +++ b/apps/files/lib/Command/TransferOwnership.php @@ -58,7 +58,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' ); } @@ -88,27 +88,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, @@ -116,7 +95,6 @@ class TransferOwnership extends Command { $output, $input->getOption('move') === true, false, - $includeIncoming ); } catch (TransferOwnershipException $e) { $output->writeln('' . $e->getMessage() . ''); diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index ffccc550e2a..0517305f13a 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -71,7 +71,6 @@ class OwnershipTransferService { ?OutputInterface $output = null, bool $move = false, bool $firstLogin = false, - bool $transferIncomingShares = false, ): void { $output = $output ?? new NullOutput(); $sourceUid = $sourceUser->getUID(); @@ -156,28 +155,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 0cf229551635e2ee1e6a5c6c3fbb7af422188ee8 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 ad9498371d3..f66d56ed538 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -53,7 +53,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);