diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 9360046bc248e..b03608f987265 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1343,7 +1343,7 @@ public function getSharesBy($userId, $shareType, $path = null, $reshares = false $added = 0; foreach ($shares as $share) { try { - $this->checkExpireDate($share); + $this->checkShare($share); } catch (ShareNotFound $e) { //Ignore since this basically means the share is deleted continue; @@ -1402,7 +1402,7 @@ public function getSharedWith($userId, $shareType, $node = null, $limit = 50, $o // remove all shares which are already expired foreach ($shares as $key => $share) { try { - $this->checkExpireDate($share); + $this->checkShare($share); } catch (ShareNotFound $e) { unset($shares[$key]); } @@ -1448,7 +1448,7 @@ public function getShareById($id, $recipient = null) { $share = $provider->getShareById($id, $recipient); - $this->checkExpireDate($share); + $this->checkShare($share); return $share; } @@ -1532,7 +1532,7 @@ public function getShareByToken($token) { throw new ShareNotFound($this->l->t('The requested share does not exist anymore')); } - $this->checkExpireDate($share); + $this->checkShare($share); /* * Reduce the permissions for link or email shares if public upload is not enabled @@ -1545,11 +1545,25 @@ public function getShareByToken($token) { return $share; } - protected function checkExpireDate($share) { + /** + * Check expire date and disabled owner + * + * @throws ShareNotFound + */ + protected function checkShare(IShare $share): void { if ($share->isExpired()) { $this->deleteShare($share); 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) { + $user = $this->userManager->get($uid); + if ($user?->isEnabled() === false) { + throw new ShareNotFound($this->l->t('The requested share comes from a disabled user')); + } + } + } } /** diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index e8704600e2a86..1f3bcd405a10f 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -2729,10 +2729,12 @@ public function testGetSharesByExpiredLinkShares() { public function testGetShareByToken() { $this->config - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getAppValue') - ->with('core', 'shareapi_allow_links', 'yes') - ->willReturn('yes'); + ->willReturnMap([ + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], + ]); $factory = $this->createMock(IProviderFactory::class); @@ -2774,10 +2776,12 @@ public function testGetShareByToken() { public function testGetShareByTokenRoom() { $this->config - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getAppValue') - ->with('core', 'shareapi_allow_links', 'yes') - ->willReturn('no'); + ->willReturnMap([ + ['core', 'shareapi_allow_links', 'yes', 'no'], + ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], + ]); $factory = $this->createMock(IProviderFactory::class); @@ -2826,10 +2830,12 @@ public function testGetShareByTokenRoom() { public function testGetShareByTokenWithException() { $this->config - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getAppValue') - ->with('core', 'shareapi_allow_links', 'yes') - ->willReturn('yes'); + ->willReturnMap([ + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], + ]); $factory = $this->createMock(IProviderFactory::class); @@ -2876,6 +2882,61 @@ public function testGetShareByTokenWithException() { } + public function testGetShareByTokenHideDisabledUser() { + $this->expectException(\OCP\Share\Exceptions\ShareNotFound::class); + $this->expectExceptionMessage('The requested share comes from a disabled user'); + + $this->config + ->expects($this->exactly(2)) + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', 'no', 'yes'], + ]); + + $this->l->expects($this->once()) + ->method('t') + ->willReturnArgument(0); + + $manager = $this->createManagerMock() + ->setMethods(['deleteShare']) + ->getMock(); + + $date = new \DateTime(); + $date->setTime(0, 0, 0); + $date->add(new \DateInterval('P2D')); + $share = $this->manager->newShare(); + $share->setExpirationDate($date); + $share->setShareOwner('owner'); + $share->setSharedBy('sharedBy'); + + $sharedBy = $this->createMock(IUser::class); + $owner = $this->createMock(IUser::class); + + $this->userManager->method('get')->willReturnMap([ + ['sharedBy', $sharedBy], + ['owner', $owner], + ]); + + $owner->expects($this->once()) + ->method('isEnabled') + ->willReturn(true); + $sharedBy->expects($this->once()) + ->method('isEnabled') + ->willReturn(false); + + $this->defaultProvider->expects($this->once()) + ->method('getShareByToken') + ->with('expiredToken') + ->willReturn($share); + + $manager->expects($this->never()) + ->method('deleteShare'); + + $manager->getShareByToken('expiredToken'); + } + + public function testGetShareByTokenExpired() { $this->expectException(\OCP\Share\Exceptions\ShareNotFound::class); $this->expectExceptionMessage('The requested share does not exist anymore'); @@ -2913,10 +2974,12 @@ public function testGetShareByTokenExpired() { public function testGetShareByTokenNotExpired() { $this->config - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getAppValue') - ->with('core', 'shareapi_allow_links', 'yes') - ->willReturn('yes'); + ->willReturnMap([ + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], + ]); $date = new \DateTime(); $date->setTime(0, 0, 0); @@ -2948,11 +3011,12 @@ public function testGetShareByTokenWithPublicLinksDisabled() { public function testGetShareByTokenPublicUploadDisabled() { $this->config - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'yes'], ['core', 'shareapi_allow_public_upload', 'yes', 'no'], + ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], ]); $share = $this->manager->newShare();