fix: check if able to access subfiles or folders

Signed-off-by: Luka Trovic <[email protected]>
nextcloud · Jun 21, 2024 · a03da0a · a03da0a
1 parent 032be69
commit a03da0a
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 37 deletions.
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
@@ -1126,29 +1126,9 @@ public function deleteReshare(IShare $share) {
 			return;
 		}
 
-		// If the user has another shares, we don't delete the shares by this user
-		if ($share->getShareType() === IShare::TYPE_USER) {
-			$groupShares = $this->getSharedWith($share->getSharedWith(), IShare::TYPE_GROUP, $node, -1, 0);
-
-			if (count($groupShares) !== 0) {
-				return;
-			}
-
-			// Check shares of parent folders
-			try {
-				$parentNode = $node->getParent();
-				while ($parentNode) {
-					$groupShares = $this->getSharedWith($share->getSharedWith(), IShare::TYPE_GROUP, $parentNode, -1, 0);
-					$userShares = $this->getSharedWith($share->getSharedWith(), IShare::TYPE_USER, $parentNode, -1, 0);
-
-					if (count($groupShares) !== 0 || count($userShares) !== 0) {
-						return;
-					}
-
-					$parentNode = $parentNode->getParent();
-				}
-			} catch (NotFoundException) {
-			}
+		// If the sharedWith user/group has another shares, we don't delete the shares by this user/group
+		if  ($this->hasAnotherShares($share->getSharedWith(), $node, [$share->getId()])) {
+			return;
 		}
 
 		// Delete re-share records (shared by "share with user") inside folder
@@ -1157,6 +1137,9 @@ public function deleteReshare(IShare $share) {
 
 			foreach ($sharesInFolder as $shares) {
 				foreach ($shares as $child) {
+					if ($this->hasAnotherShares($share->getSharedWith(), $child->getNode(), [$share->getId()])) {
+						continue;
+					}
 					$this->deleteShare($child);
 				}
 			}
@@ -1176,6 +1159,9 @@ public function deleteReshare(IShare $share) {
 				$provider = $this->factory->getProviderForType($shareType);
 				$shares = $provider->getSharesBy($share->getSharedWith(), $shareType, $node, false, -1, 0);
 				foreach ($shares as $child) {
+					if ($this->hasAnotherShares($share->getSharedWith(), $child->getNode(), [$share->getId()])) {
+						continue;
+					}
 					$this->deleteShare($child);
 				}
 			}
@@ -1187,15 +1173,13 @@ public function deleteReshare(IShare $share) {
 			$users = $group->getUsers();
 
 			foreach ($users as $user) {
+				// Skip if share owner is member of shared group
 				if ($user->getUID() === $share->getShareOwner()) {
 					continue;
 				}
 
-				$anotherShares = $this->getSharedWith($user->getUID(), IShare::TYPE_USER, $node, -1, 0);
-				$groupShares = $this->getSharedWith($user->getUID(), IShare::TYPE_GROUP, $node, -1, 0);
-
 				// If the user has another shares, we don't delete the shares by this user
-				if (count($anotherShares) !== 0 || count($groupShares) > 1) {
+				if ($this->hasAnotherShares($user->getUID(), $node, [$share->getId()])) {
 					continue;
 				}
 
@@ -1204,22 +1188,81 @@ public function deleteReshare(IShare $share) {
 
 					foreach ($sharesInFolder as $shares) {
 						foreach ($shares as $child) {
+							if ($this->hasAnotherShares($user->getUID(), $child->getNode(), [$share->getId()])) {
+								continue;
+							}
 							$this->deleteShare($child);
 						}
 					}
-				} else {
-					foreach ($shareTypes as $shareType) {
-						$provider = $this->factory->getProviderForType($shareType);
-						$shares = $provider->getSharesBy($user->getUID(), $shareType, $node, false, -1, 0);
-						foreach ($shares as $child) {
-							$this->deleteShare($child);
-						}
+				}
+
+				foreach ($shareTypes as $shareType) {
+					$provider = $this->factory->getProviderForType($shareType);
+					$shares = $provider->getSharesBy($user->getUID(), $shareType, $node, false, -1, 0);
+					foreach ($shares as $child) {
+						$this->deleteShare($child);
 					}
 				}
 			}
 		}
 	}
 
+	public function hasAnotherShares($userId, Node $node, $exceptShareIds = []) {
+		$shares = [];
+
+		$groupShares = $this->getSharedWith($userId, IShare::TYPE_GROUP, $node, -1, 0);
+		foreach ($groupShares as $share) {
+			if (!in_array($share->getId(), $exceptShareIds)) {
+				$shares[] = $share;
+			}
+		}
+		if (count($shares)) {
+			return true;
+		}
+
+		$userShares = $this->getSharedWith($userId, IShare::TYPE_USER, $node, -1, 0);
+		foreach ($userShares as $share) {
+			if (!in_array($share->getId(), $exceptShareIds)) {
+				$shares[] = $share;
+			}
+		}
+		if (count($shares)) {
+			return true;
+		}
+
+		// Check shares of parent folders
+		try {
+			$parentNode = $node->getParent();
+			while ($parentNode) {
+				$groupShares = $this->getSharedWith($userId, IShare::TYPE_GROUP, $parentNode, -1, 0);
+				foreach ($groupShares as $share) {
+					if (!in_array($share->getId(), $exceptShareIds)) {
+						$shares[] = $share;
+					}
+				}
+				if (count($shares)) {
+					return true;
+				}
+
+				$userShares = $this->getSharedWith($userId, IShare::TYPE_USER, $parentNode, -1, 0);
+				foreach ($userShares as $share) {
+					if (!in_array($share->getId(), $exceptShareIds)) {
+						$shares[] = $share;
+					}
+				}
+				if (count($shares)) {
+					return true;
+				}
+
+				$parentNode = $parentNode->getParent();
+			}
+		} catch (NotFoundException) {
+			return false;
+		}
+
+		return false;
+	}
+
 	/**
 	 * Delete a share
 	 *

diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
@@ -473,8 +473,9 @@ public function testDeleteChildren() {
 
 	public function testDeleteReshareWhenUserHasOneShare() {
 		$manager = $this->createManagerMock()
-			->setMethods(['deleteShare', 'getSharesInFolder', 'getSharedWith'])
+			->setMethods(['deleteShare', 'getSharesInFolder', 'getSharedWith', 'hasAnotherShares'])
 			->getMock();
+		$manager->method('hasAnotherShares')->willReturn(false);
 
 		$folder = $this->createMock(Folder::class);
 		$folder->method('getParent')->willReturn(null);
@@ -487,6 +488,7 @@ public function testDeleteReshareWhenUserHasOneShare() {
 
 		$reShare = $this->createMock(IShare::class);
 		$reShare->method('getSharedBy')->willReturn('UserB');
+		$reShare->method('getNode')->willReturn($folder);
 
 		$reShareInSubFolder = $this->createMock(IShare::class);
 		$reShareInSubFolder->method('getSharedBy')->willReturn('UserB');
@@ -504,8 +506,9 @@ public function testDeleteReshareWhenUserHasOneShare() {
 
 	public function testDeleteReshareWhenUserHasAnotherShare() {
 		$manager = $this->createManagerMock()
-			->setMethods(['deleteShare', 'getSharesInFolder', 'getSharedWith'])
+			->setMethods(['deleteShare', 'getSharesInFolder', 'getSharedWith', 'hasAnotherShares'])
 			->getMock();
+		$manager->method('hasAnotherShares')->willReturn(true);
 
 		$folder = $this->createMock(Folder::class);
 		$folder->method('getParent')->willReturn(null);
@@ -547,14 +550,18 @@ public function testDeleteReshareWhenUserHasAnotherShareFromParentFolder() {
 		$share->method('getNodeType')->willReturn('folder');
 		$share->method('getSharedWith')->willReturn('UserB');
 		$share->method('getNode')->willReturn($folder);
+		$share->method('getId')->willReturn(1);
 
 		$reShare = $this->createMock(IShare::class);
 		$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
 		$reShare->method('getNodeType')->willReturn('folder');
 		$reShare->method('getSharedBy')->willReturn('UserB');
 		$reShare->method('getNode')->willReturn($folder);
 
-		$manager->expects($this->exactly(3))->method('getSharedWith')->willReturnOnConsecutiveCalls([], [1], []);
+		$shareOfParent = $this->createMock(IShare::class);
+		$shareOfParent->method('getId')->willReturn(2);
+
+		$manager->expects($this->exactly(3))->method('getSharedWith')->willReturnOnConsecutiveCalls([], [], [$shareOfParent]);
 		$manager->method('getSharesInFolder')->willReturn([]);
 
 		$this->defaultProvider->method('getSharesBy')
@@ -606,6 +613,9 @@ public function testDeleteReshareOfUsersInGroupShare() {
 		$group->method('getUsers')->willReturn([$userB, $userC]);
 		$this->groupManager->method('get')->with('Group')->willReturn($group);
 
+		$this->defaultProvider->method('getSharesBy')
+			->willReturn([]);
+
 		$manager->method('getSharedWith')->willReturn([]);
 		$manager->expects($this->exactly(2))->method('getSharesInFolder')->willReturnOnConsecutiveCalls([[$reShare1]], [[$reShare2]]);