fix: Fix promotion of reshares from subsubfolders

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

2 files changed, 101 insertions, 26 deletions

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 2fb080d07dc..2bc639607d8 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -1197,16 +1197,23 @@ class Manager implements IManager {
 		foreach ($userIds as $userId) {
 			foreach ($shareTypes as $shareType) {
 				$provider = $this->factory->getProviderForType($shareType);
-				$shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0);
-				foreach ($shares as $child) {
-					$reshareRecords[] = $child;
-				}
-			}
-
-			if ($node instanceof Folder) {
-				$sharesInFolder = $this->getSharesInFolder($userId, $node, false);
-
-				foreach ($sharesInFolder as $shares) {
+				if ($node instanceof Folder) {
+					/* We need to get all shares by this user to get subshares */
+					$shares = $provider->getSharesBy($userId, $shareType, null, false, -1, 0);
+
+					foreach ($shares as $share) {
+						try {
+							$path = $share->getNode()->getPath();
+						} catch (NotFoundException) {
+							/* Ignore share of non-existing node */
+							continue;
+						}
+						if (str_starts_with($path, $node->getPath() . '/') || ($path === $node->getPath())) {
+							$reshareRecords[] = $share;
+						}
+					}
+				} else {
+					$shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0);
 					foreach ($shares as $child) {
 						$reshareRecords[] = $child;
 					}
diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index 0aace03a99a..517f236ef28 100644
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -486,34 +486,92 @@ class ManagerTest extends \Test\TestCase {
 		$this->assertSame($shares, $result);
 	}
 
-	public function testPromoteReshareWhenUserHasOneShare(): void {
+	public function testPromoteReshareFile(): void {
+		$manager = $this->createManagerMock()
+			->setMethods(['updateShare', 'getSharesInFolder', 'generalCreateChecks'])
+			->getMock();
+
+		$file = $this->createMock(File::class);
+
+		$share = $this->createMock(IShare::class);
+		$share->method('getShareType')->willReturn(IShare::TYPE_USER);
+		$share->method('getNodeType')->willReturn('folder');
+		$share->method('getSharedWith')->willReturn('userB');
+		$share->method('getNode')->willReturn($file);
+
+		$reShare = $this->createMock(IShare::class);
+		$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
+		$reShare->method('getSharedBy')->willReturn('userB');
+		$reShare->method('getSharedWith')->willReturn('userC');
+		$reShare->method('getNode')->willReturn($file);
+
+		$this->defaultProvider->method('getSharesBy')
+			->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare, $file) {
+				$this->assertEquals($file, $node);
+				if ($shareType === IShare::TYPE_USER) {
+					return match($userId) {
+						'userB' => [$reShare],
+					};
+				} else {
+					return [];
+				}
+			});
+		$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
+
+		$manager->expects($this->exactly(1))->method('updateShare')->with($reShare);
+
+		self::invokePrivate($manager, 'promoteReshares', [$share]);
+	}
+
+	public function testPromoteReshare(): void {
 		$manager = $this->createManagerMock()
 			->setMethods(['updateShare', 'getSharesInFolder', 'generalCreateChecks'])
 			->getMock();
 
 		$folder = $this->createMock(Folder::class);
+		$folder->method('getPath')->willReturn('/path/to/folder');
+
+		$subFolder = $this->createMock(Folder::class);
+		$subFolder->method('getPath')->willReturn('/path/to/folder/sub');
+
+		$otherFolder = $this->createMock(Folder::class);
+		$otherFolder->method('getPath')->willReturn('/path/to/otherfolder/');
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getShareType')->willReturn(IShare::TYPE_USER);
 		$share->method('getNodeType')->willReturn('folder');
-		$share->method('getSharedWith')->willReturn('UserB');
+		$share->method('getSharedWith')->willReturn('userB');
 		$share->method('getNode')->willReturn($folder);
 
 		$reShare = $this->createMock(IShare::class);
-		$reShare->method('getSharedBy')->willReturn('UserB');
-		$reShare->method('getSharedWith')->willReturn('UserC');
+		$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
+		$reShare->method('getSharedBy')->willReturn('userB');
+		$reShare->method('getSharedWith')->willReturn('userC');
 		$reShare->method('getNode')->willReturn($folder);
 
 		$reShareInSubFolder = $this->createMock(IShare::class);
-		$reShareInSubFolder->method('getSharedBy')->willReturn('UserB');
+		$reShareInSubFolder->method('getShareType')->willReturn(IShare::TYPE_USER);
+		$reShareInSubFolder->method('getSharedBy')->willReturn('userB');
+		$reShareInSubFolder->method('getNode')->willReturn($subFolder);
 
-		$manager->method('getSharesInFolder')->willReturn([$reShareInSubFolder]);
-		$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
+		$reShareInOtherFolder = $this->createMock(IShare::class);
+		$reShareInOtherFolder->method('getShareType')->willReturn(IShare::TYPE_USER);
+		$reShareInOtherFolder->method('getSharedBy')->willReturn('userB');
+		$reShareInOtherFolder->method('getNode')->willReturn($otherFolder);
 
 		$this->defaultProvider->method('getSharesBy')
-			->willReturn([$reShare]);
+			->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare, $reShareInSubFolder, $reShareInOtherFolder) {
+				if ($shareType === IShare::TYPE_USER) {
+					return match($userId) {
+						'userB' => [$reShare,$reShareInSubFolder,$reShareInOtherFolder],
+					};
+				} else {
+					return [];
+				}
+			});
+		$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
 
-		$manager->expects($this->atLeast(2))->method('updateShare')->withConsecutive([$reShare], [$reShareInSubFolder]);
+		$manager->expects($this->exactly(2))->method('updateShare')->withConsecutive([$reShare], [$reShareInSubFolder]);
 
 		self::invokePrivate($manager, 'promoteReshares', [$share]);
 	}
@@ -524,23 +582,24 @@ class ManagerTest extends \Test\TestCase {
 			->getMock();
 
 		$folder = $this->createMock(Folder::class);
+		$folder->method('getPath')->willReturn('/path/to/folder');
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getShareType')->willReturn(IShare::TYPE_USER);
 		$share->method('getNodeType')->willReturn('folder');
-		$share->method('getSharedWith')->willReturn('UserB');
+		$share->method('getSharedWith')->willReturn('userB');
 		$share->method('getNode')->willReturn($folder);
 
 		$reShare = $this->createMock(IShare::class);
 		$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
 		$reShare->method('getNodeType')->willReturn('folder');
-		$reShare->method('getSharedBy')->willReturn('UserB');
+		$reShare->method('getSharedBy')->willReturn('userB');
 		$reShare->method('getNode')->willReturn($folder);
 
 		$this->defaultProvider->method('getSharesBy')->willReturn([$reShare]);
-		$manager->method('getSharesInFolder')->willReturn([]);
 		$manager->method('generalCreateChecks')->willReturn(true);
 
+		/* No share is promoted because generalCreateChecks does not throw */
 		$manager->expects($this->never())->method('updateShare');
 
 		self::invokePrivate($manager, 'promoteReshares', [$share]);
@@ -552,6 +611,7 @@ class ManagerTest extends \Test\TestCase {
 			->getMock();
 
 		$folder = $this->createMock(Folder::class);
+		$folder->method('getPath')->willReturn('/path/to/folder');
 
 		$userA = $this->createMock(IUser::class);
 		$userA->method('getUID')->willReturn('userA');
@@ -566,13 +626,13 @@ class ManagerTest extends \Test\TestCase {
 		$reShare1 = $this->createMock(IShare::class);
 		$reShare1->method('getShareType')->willReturn(IShare::TYPE_USER);
 		$reShare1->method('getNodeType')->willReturn('folder');
-		$reShare1->method('getSharedBy')->willReturn('UserB');
+		$reShare1->method('getSharedBy')->willReturn('userB');
 		$reShare1->method('getNode')->willReturn($folder);
 
 		$reShare2 = $this->createMock(IShare::class);
 		$reShare2->method('getShareType')->willReturn(IShare::TYPE_USER);
 		$reShare2->method('getNodeType')->willReturn('folder');
-		$reShare2->method('getSharedBy')->willReturn('UserC');
+		$reShare2->method('getSharedBy')->willReturn('userC');
 		$reShare2->method('getNode')->willReturn($folder);
 
 		$userB = $this->createMock(IUser::class);
@@ -584,11 +644,19 @@ class ManagerTest extends \Test\TestCase {
 		$this->groupManager->method('get')->with('Group')->willReturn($group);
 
 		$this->defaultProvider->method('getSharesBy')
-			->willReturn([]);
+			->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare1, $reShare2) {
+				if ($shareType === IShare::TYPE_USER) {
+					return match($userId) {
+						'userB' => [$reShare1],
+						'userC' => [$reShare2],
+					};
+				} else {
+					return [];
+				}
+			});
 		$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
 
 		$manager->method('getSharedWith')->willReturn([]);
-		$manager->expects($this->exactly(2))->method('getSharesInFolder')->willReturnOnConsecutiveCalls([[$reShare1]], [[$reShare2]]);
 
 		$manager->expects($this->exactly(2))->method('updateShare')->withConsecutive([$reShare1], [$reShare2]);