diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 4abc3a3f54c..ecb5a6eab1a 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1413,7 +1413,7 @@ class Manager implements IManager { } $share = null; try { - if ($this->shareApiAllowLinks()) { + if ($this->config->getAppValue('core', 'shareapi_allow_links', 'yes') === 'yes') { $provider = $this->factory->getProviderForType(IShare::TYPE_LINK); $share = $provider->getShareByToken($token); } @@ -1496,6 +1496,17 @@ class Manager implements IManager { } } } + + // For link and email shares, verify the share owner can still create such shares + if ($share->getShareType() === IShare::TYPE_LINK || $share->getShareType() === IShare::TYPE_EMAIL) { + $shareOwner = $this->userManager->get($share->getShareOwner()); + if ($shareOwner === null) { + throw new ShareNotFound($this->l->t('The requested share does not exist anymore')); + } + if (!$this->userCanCreateLinkShares($shareOwner)) { + throw new ShareNotFound($this->l->t('The requested share does not exist anymore')); + } + } } /** @@ -1742,14 +1753,15 @@ class Manager implements IManager { /** * Is public link sharing enabled * + * @param ?IUser $user User to check against group exclusions, defaults to current session user * @return bool */ - public function shareApiAllowLinks() { + public function shareApiAllowLinks(?IUser $user = null) { if ($this->config->getAppValue('core', 'shareapi_allow_links', 'yes') !== 'yes') { return false; } - $user = $this->userSession->getUser(); + $user = $user ?? $this->userSession->getUser(); if ($user) { $excludedGroups = json_decode($this->config->getAppValue('core', 'shareapi_allow_links_exclude_groups', '[]')); if ($excludedGroups) { @@ -1761,6 +1773,16 @@ class Manager implements IManager { return true; } + /** + * Check if a specific user can create link shares + * + * @param IUser $user The user to check + * @return bool + */ + protected function userCanCreateLinkShares(IUser $user): bool { + return $this->shareApiAllowLinks($user); + } + /** * Is password on public link requires * diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index f65f7a4c56b..97501282592 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -298,10 +298,12 @@ interface IManager { /** * Is public link sharing enabled * + * @param ?IUser $user User to check against group exclusions, defaults to current session user * @return bool * @since 9.0.0 + * @since 33.0.0 Added optional $user parameter */ - public function shareApiAllowLinks(); + public function shareApiAllowLinks(?IUser $user = null); /** * Is password on public link required diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 7c5e5a6e743..a7b93d35821 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -3325,21 +3325,29 @@ class ManagerTest extends \Test\TestCase { public function testGetShareByTokenPublicUploadDisabled(): void { $this->config - ->expects($this->exactly(3)) + ->expects($this->exactly(5)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'yes'], ['core', 'shareapi_allow_public_upload', 'yes', 'no'], ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], + ['core', 'shareapi_allow_links_exclude_groups', '[]', '[]'], ]); $share = $this->manager->newShare(); $share->setShareType(IShare::TYPE_LINK) ->setPermissions(Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE); $share->setSharedWith('sharedWith'); + $share->setShareOwner('shareOwner'); $folder = $this->createMock(\OC\Files\Node\Folder::class); $share->setNode($folder); + $shareOwner = $this->createMock(IUser::class); + $this->userManager->expects($this->once()) + ->method('get') + ->with('shareOwner') + ->willReturn($shareOwner); + $this->defaultProvider->expects($this->once()) ->method('getShareByToken') ->willReturn('validToken') @@ -3350,6 +3358,87 @@ class ManagerTest extends \Test\TestCase { $this->assertSame(Constants::PERMISSION_READ, $res->getPermissions()); } + public function testGetShareByTokenShareOwnerExcludedFromLinkShares(): void { + $this->expectException(ShareNotFound::class); + $this->expectExceptionMessage('The requested share does not exist anymore'); + + $this->config + ->expects($this->exactly(4)) + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], + ['core', 'shareapi_allow_links_exclude_groups', '[]', '["excludedGroup"]'], + ]); + + $this->l->expects($this->once()) + ->method('t') + ->willReturnArgument(0); + + $share = $this->manager->newShare(); + $share->setShareType(IShare::TYPE_LINK) + ->setPermissions(Constants::PERMISSION_READ); + $share->setShareOwner('shareOwner'); + $file = $this->createMock(File::class); + $share->setNode($file); + + $shareOwner = $this->createMock(IUser::class); + $this->userManager->expects($this->once()) + ->method('get') + ->with('shareOwner') + ->willReturn($shareOwner); + + $this->groupManager->expects($this->once()) + ->method('getUserGroupIds') + ->with($shareOwner) + ->willReturn(['excludedGroup', 'otherGroup']); + + $this->defaultProvider->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $this->manager->getShareByToken('token'); + } + + public function testGetShareByTokenShareOwnerNotExcludedFromLinkShares(): void { + $this->config + ->expects($this->exactly(4)) + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], + ['core', 'shareapi_allow_links_exclude_groups', '[]', '["excludedGroup"]'], + ]); + + $share = $this->manager->newShare(); + $share->setShareType(IShare::TYPE_LINK) + ->setPermissions(Constants::PERMISSION_READ); + $share->setShareOwner('shareOwner'); + $file = $this->createMock(File::class); + $share->setNode($file); + + $shareOwner = $this->createMock(IUser::class); + $this->userManager->expects($this->once()) + ->method('get') + ->with('shareOwner') + ->willReturn($shareOwner); + + $this->groupManager->expects($this->once()) + ->method('getUserGroupIds') + ->with($shareOwner) + ->willReturn(['allowedGroup', 'otherGroup']); + + $this->defaultProvider->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $res = $this->manager->getShareByToken('token'); + + $this->assertSame($share, $res); + } + public function testCheckPasswordNoLinkShare(): void { $share = $this->createMock(IShare::class); $share->method('getShareType')->willReturn(IShare::TYPE_USER);