From a03da0a4747bf79be19c9baec83f807e2a3424be Mon Sep 17 00:00:00 2001 From: Luka Trovic Date: Fri, 21 Jun 2024 14:11:07 +0200 Subject: [PATCH] fix: check if able to access subfiles or folders Signed-off-by: Luka Trovic --- lib/private/Share20/Manager.php | 111 +++++++++++++++++++++--------- tests/lib/Share20/ManagerTest.php | 16 ++++- 2 files changed, 90 insertions(+), 37 deletions(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 1b52ecfce46c2..2acf8a98b6f46 100644 --- 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 index b4f61ba4b2498..7d1d0a18dd267 100644 --- 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,6 +550,7 @@ 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); @@ -554,7 +558,10 @@ public function testDeleteReshareWhenUserHasAnotherShareFromParentFolder() { $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]]);