diff --git a/build/integration/sharing_features/sharing-v1-part2.feature b/build/integration/sharing_features/sharing-v1-part2.feature index 8cc97fe71ee58..e6984bbd330d9 100644 --- a/build/integration/sharing_features/sharing-v1-part2.feature +++ b/build/integration/sharing_features/sharing-v1-part2.feature @@ -923,6 +923,34 @@ Feature: sharing Then the OCS status code should be "403" And the HTTP status code should be "200" + Scenario: Rename a received share after deleting the source file of a share made by the receiver + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And file "textfile0.txt" of user "user1" is shared with user "user2" + And user "user2" accepts last share + And User "user1" deletes file "/textfile0.txt" + When file "textfile1.txt" of user "user0" is shared with user "user1" + And user "user1" accepts last share + And User "user1" moves file "/textfile1 (2).txt" to "/shared_file.txt" + Then the HTTP status code should be "201" + And as "user1" the file "/shared_file.txt" exists + + Scenario: Rename a received share after deleting the source file of a reshare made by the receiver + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And file "textfile0.txt" of user "user0" is shared with user "user1" + And user "user1" accepts last share + And file "textfile0 (2).txt" of user "user1" is shared with user "user2" + And user "user2" accepts last share + And User "user0" deletes file "/textfile0.txt" + When file "textfile1.txt" of user "user0" is shared with user "user1" + And user "user1" accepts last share + And User "user1" moves file "/textfile1 (2).txt" to "/shared_file.txt" + Then the HTTP status code should be "201" + And as "user1" the file "/shared_file.txt" exists + Scenario: Keep usergroup shares (#22143) Given As an "admin" And user "user0" exists diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 1006a0f24bf76..1943d520b9b08 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1460,6 +1460,15 @@ protected function checkShare(IShare $share): void { $this->deleteShare($share); throw new ShareNotFound($this->l->t('The requested share does not exist anymore')); } + + try { + $share->getNode(); + // Ignore share, file is still accessible + } catch (NotFoundException) { + // Access lost, but maybe only temporarily, so don't delete the share right away + throw new ShareNotFound($this->l->t('The requested share does not exist anymore')); + } + if ($this->config->getAppValue('files_sharing', 'hide_disabled_user_shares', 'no') === 'yes') { $uids = array_unique([$share->getShareOwner(),$share->getSharedBy()]); foreach ($uids as $uid) { diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index c780d29bfed66..3475f4e2659de 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -24,6 +24,7 @@ use OCP\Files\Mount\IMountManager; use OCP\Files\Mount\IMountPoint; use OCP\Files\Node; +use OCP\Files\NotFoundException; use OCP\Files\Storage; use OCP\HintException; use OCP\IConfig; @@ -666,6 +667,24 @@ public function testGetShareById(): void { } + public function testGetShareByIdNodeAccessible(): void { + $share = $this->createMock(IShare::class); + $share + ->expects($this->once()) + ->method('getNode') + ->willThrowException(new NotFoundException()); + + $this->defaultProvider + ->expects($this->once()) + ->method('getShareById') + ->with(42) + ->willReturn($share); + + $this->expectException(ShareNotFound::class); + $this->manager->getShareById('default:42'); + } + + public function testGetExpiredShareById() { $this->expectException(\OCP\Share\Exceptions\ShareNotFound::class); @@ -2874,10 +2893,11 @@ public function testCreateShareOfIncomingFederatedShare() { } public function testGetSharesBy() { - $share = $this->manager->newShare(); - $node = $this->createMock(Folder::class); + $share = $this->manager->newShare(); + $share->setNode($node); + $this->defaultProvider->expects($this->once()) ->method('getSharesBy') ->with( @@ -2904,6 +2924,8 @@ public function testGetSharesBy() { * deleted (as they are evaluated). but share 8 should still be there. */ public function testGetSharesByExpiredLinkShares() { + $node = $this->createMock(File::class); + $manager = $this->createManagerMock() ->setMethods(['deleteShare']) ->getMock(); @@ -2917,6 +2939,7 @@ public function testGetSharesByExpiredLinkShares() { for ($i = 0; $i < 8; $i++) { $share = $this->manager->newShare(); $share->setId($i); + $share->setNode($node); $shares[] = $share; } @@ -2937,8 +2960,6 @@ public function testGetSharesByExpiredLinkShares() { $shares2[] = clone $shares[$i]; } - $node = $this->createMock(File::class); - /* * Simulate the getSharesBy call. */ @@ -3100,8 +3121,10 @@ public function testGetShareByTokenHideDisabledUser() { $date = new \DateTime(); $date->setTime(0, 0, 0); $date->add(new \DateInterval('P2D')); + $node = $this->createMock(File::class); $share = $this->manager->newShare(); $share->setExpirationDate($date); + $share->setNode($node); $share->setShareOwner('owner'); $share->setSharedBy('sharedBy'); @@ -3179,8 +3202,10 @@ public function testGetShareByTokenNotExpired() { $date = new \DateTime(); $date->setTime(0, 0, 0); $date->add(new \DateInterval('P2D')); + $node = $this->createMock(Folder::class); $share = $this->manager->newShare(); $share->setExpirationDate($date); + $share->setNode($node); $this->defaultProvider->expects($this->once()) ->method('getShareByToken')