Merge pull request #53743 from nextcloud/backport/53304/stable31

[stable31]  fix(files_sharing): Hide own reshares

Author: sorbaugh

apps/files/lib/Command/TransferOwnership.php

@@ -58,7 +58,7 @@ protected function configure(): void {
 				'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,35 +88,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		}
 
 		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("<error> config.php: 'transfer-incoming-shares': wrong usage. Transfer aborted.</error>");
-						return self::FAILURE;
-					}
-					break;
-				default:
-					$output->writeln('<error>Option --transfer-incoming-shares: wrong usage. Transfer aborted.</error>');
-					return self::FAILURE;
-			}
-
 			$this->transferService->transfer(
 				$sourceUserObject,
 				$destinationUserObject,
 				ltrim($input->getOption('path'), '/'),
 				$output,
 				$input->getOption('move') === true,
 				false,
-				$includeIncoming
 			);
 		} catch (TransferOwnershipException $e) {
 			$output->writeln('<error>' . $e->getMessage() . '</error>');

apps/files/lib/Service/OwnershipTransferService.php

@@ -10,13 +10,15 @@
 namespace OCA\Files\Service;
 
 use Closure;
+use Exception;
 use OC\Files\Filesystem;
 use OC\Files\View;
 use OC\User\NoUserException;
 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;
@@ -69,7 +71,6 @@ public function transfer(
 		?OutputInterface $output = null,
 		bool $move = false,
 		bool $firstLogin = false,
-		bool $transferIncomingShares = false,
 	): void {
 		$output = $output ?? new NullOutput();
 		$sourceUid = $sourceUser->getUID();
@@ -154,29 +155,26 @@ public function transfer(
 		$sizeDifference = $sourceSize - $view->getFileInfo($finalTarget)->getSize();
 
 		// transfer the incoming shares
-		if ($transferIncomingShares === true) {
-			$sourceShares = $this->collectIncomingShares(
-				$sourceUid,
-				$output,
-				$view
-			);
-			$destinationShares = $this->collectIncomingShares(
-				$destinationUid,
-				$output,
-				$view,
-				true
-			);
-			$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
@@ -333,7 +331,7 @@ private function collectUsersShares(
 							return mb_strpos(
 								Filesystem::normalizePath($relativePath . '/', false),
 								$normalizedPath . '/') === 0;
-						} catch (\Exception $e) {
+						} catch (Exception $e) {
 							return false;
 						}
 					});
@@ -361,14 +359,16 @@ private function collectUsersShares(
 		}, $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 +377,19 @@ private function collectIncomingShares(string $sourceUid,
 			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;

apps/files_sharing/lib/MountProvider.php

@@ -53,7 +53,7 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader) {
 
 		// 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);

build/integration/features/bootstrap/CommandLineContext.php

@@ -108,19 +108,6 @@ public function transferringOwnershipPath($path, $user1, $user2) {
 		}
 	}
 
-	/**
-	 * @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$/
 	 */

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,18 +241,18 @@ 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
 			| uid_owner | user0 |
 			| 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