diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 68babd0ec26de..97f8fc44b3d86 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -82,7 +82,6 @@ * This class is the communication hub for all sharing related operations. */ class Manager implements IManager { - /** @var IProviderFactory */ private $factory; private LoggerInterface $logger; @@ -669,7 +668,6 @@ protected function linkCreateChecks(IShare $share) { * @param IShare $share */ protected function setLinkParent(IShare $share) { - // No sense in checking if the method is not there. if (method_exists($share, 'setParent')) { $storage = $share->getNode()->getStorage(); @@ -1350,7 +1348,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; @@ -1409,7 +1407,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]); } @@ -1455,7 +1453,7 @@ public function getShareById($id, $recipient = null) { $share = $provider->getShareById($id, $recipient); - $this->checkExpireDate($share); + $this->checkShare($share); return $share; } @@ -1539,7 +1537,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 @@ -1552,11 +1550,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 !== null) && ($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 a7bb6cbcefc05..893d785794861 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -552,7 +552,7 @@ public function testVerifyPasswordHook() { /** @var ValidatePasswordPolicyEvent $event */ $this->assertSame('password', $event->getPassword()); } - ); + ); $result = self::invokePrivate($this->manager, 'verifyPassword', ['password']); $this->assertNull($result); @@ -575,7 +575,7 @@ public function testVerifyPasswordHookFails() { $this->assertSame('password', $event->getPassword()); throw new HintException('message', 'password not accepted'); } - ); + ); self::invokePrivate($this->manager, 'verifyPassword', ['password']); } @@ -2715,10 +2715,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); @@ -2761,10 +2763,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); @@ -2814,10 +2818,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); @@ -2865,6 +2871,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'); @@ -2902,10 +2963,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); @@ -2937,11 +3000,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();