From ee02016dc10896f215431fd22e229473fb1b75c8 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 15 Mar 2024 15:51:15 +0100 Subject: [PATCH] fix(files_sharing): ShareesAPI - Return empty response when user is not allowed to share Resolves: https://github.com/nextcloud/server/issues/20950 Signed-off-by: Ferdinand Thiessen --- .../lib/Controller/ShareesAPIController.php | 33 ++--- .../Controller/ShareesAPIControllerTest.php | 114 ++++++++++++------ 2 files changed, 83 insertions(+), 64 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index 00bc85e4a969d..de601607f2c9b 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -64,18 +64,6 @@ */ class ShareesAPIController extends OCSController { - /** @var string */ - protected $userId; - - /** @var IConfig */ - protected $config; - - /** @var IURLGenerator */ - protected $urlGenerator; - - /** @var IManager */ - protected $shareManager; - /** @var int */ protected $offset = 0; @@ -105,8 +93,6 @@ class ShareesAPIController extends OCSController { ]; protected $reachedEndFor = []; - /** @var ISearch */ - private $collaboratorSearch; /** * @param string $UserId @@ -118,20 +104,15 @@ class ShareesAPIController extends OCSController { * @param ISearch $collaboratorSearch */ public function __construct( - $UserId, string $appName, IRequest $request, - IConfig $config, - IURLGenerator $urlGenerator, - IManager $shareManager, - ISearch $collaboratorSearch + protected string $userId, + protected IConfig $config, + protected IURLGenerator $urlGenerator, + protected IManager $shareManager, + protected ISearch $collaboratorSearch, ) { parent::__construct($appName, $request); - $this->userId = $UserId; - $this->config = $config; - $this->urlGenerator = $urlGenerator; - $this->shareManager = $shareManager; - $this->collaboratorSearch = $collaboratorSearch; } /** @@ -158,6 +139,10 @@ public function search(string $search = '', string $itemType = null, int $page = return new DataResponse($this->result); } + if ($this->shareManager->sharingDisabledForUser($this->userId)) { + return new DataResponse($this->result); + } + // never return more than the max. number of results configured in the config.php $maxResults = $this->config->getSystemValueInt('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT); if ($maxResults > 0) { diff --git a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php index 5953ab0d89022..b56e57d272adc 100644 --- a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php @@ -33,6 +33,7 @@ use OCA\Files_Sharing\Controller\ShareesAPIController; use OCA\Files_Sharing\Tests\TestCase; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\Collaboration\Collaborators\ISearch; use OCP\IConfig; @@ -65,15 +66,16 @@ class ShareesAPIControllerTest extends TestCase { /** @var ISearch|MockObject */ protected $collaboratorSearch; + /** @var IConfig|MockObject */ + protected $config; + protected function setUp(): void { parent::setUp(); $this->uid = 'test123'; $this->request = $this->createMock(IRequest::class); $this->shareManager = $this->createMock(IManager::class); - - /** @var IConfig|MockObject $configMock */ - $configMock = $this->createMock(IConfig::class); + $this->config = $this->createMock(IConfig::class); /** @var IURLGenerator|MockObject $urlGeneratorMock */ $urlGeneratorMock = $this->createMock(IURLGenerator::class); @@ -81,10 +83,10 @@ protected function setUp(): void { $this->collaboratorSearch = $this->createMock(ISearch::class); $this->sharees = new ShareesAPIController( - $this->uid, 'files_sharing', $this->request, - $configMock, + $this->uid, + $this->config, $urlGeneratorMock, $this->shareManager, $this->collaboratorSearch @@ -96,124 +98,124 @@ public function dataSearch(): array { $allTypes = [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_REMOTE, IShare::TYPE_REMOTE_GROUP, IShare::TYPE_EMAIL]; return [ - [[], '', 'yes', true, true, true, $noRemote, false, true, true], + [[], '', 'yes', false, true, true, true, $noRemote, false, true, true], // Test itemType [[ 'search' => '', - ], '', 'yes', true, true, true, $noRemote, false, true, true], + ], '', 'yes', false, true, true, true, $noRemote, false, true, true], [[ 'search' => 'foobar', - ], '', 'yes', true, true, true, $noRemote, false, true, true], + ], '', 'yes', false, true, true, true, $noRemote, false, true, true], [[ 'search' => 0, - ], '', 'yes', true, true, true, $noRemote, false, true, true], + ], '', 'yes', false, true, true, true, $noRemote, false, true, true], // Test itemType [[ 'itemType' => '', - ], '', 'yes', true, true, true, $noRemote, false, true, true], + ], '', 'yes', false, true, true, true, $noRemote, false, true, true], [[ 'itemType' => 'folder', - ], '', 'yes', true, true, true, $allTypes, false, true, true], + ], '', 'yes', false, true, true, true, $allTypes, false, true, true], [[ 'itemType' => 0, - ], '', 'yes', true, true , true, $noRemote, false, true, true], + ], '', 'yes', false, true, true , true, $noRemote, false, true, true], // Test shareType [[ 'itemType' => 'call', - ], '', 'yes', true, true, true, $noRemote, false, true, true], + ], '', 'yes', false, true, true, true, $noRemote, false, true, true], [[ 'itemType' => 'call', - ], '', 'yes', true, true, true, [0, 4], false, true, false], + ], '', 'yes', false, true, true, true, [0, 4], false, true, false], [[ 'itemType' => 'folder', - ], '', 'yes', true, true, true, $allTypes, false, true, true], + ], '', 'yes', false, true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'shareType' => 0, - ], '', 'yes', true, true, false, [0], false, true, true], + ], '', 'yes', false, true, true, false, [0], false, true, true], [[ 'itemType' => 'folder', 'shareType' => '0', - ], '', 'yes', true, true, false, [0], false, true, true], + ], '', 'yes', false, true, true, false, [0], false, true, true], [[ 'itemType' => 'folder', 'shareType' => 1, - ], '', 'yes', true, true, false, [1], false, true, true], + ], '', 'yes', false, true, true, false, [1], false, true, true], [[ 'itemType' => 'folder', 'shareType' => 12, - ], '', 'yes', true, true, false, [], false, true, true], + ], '', 'yes', false, true, true, false, [], false, true, true], [[ 'itemType' => 'folder', 'shareType' => 'foobar', - ], '', 'yes', true, true, true, $allTypes, false, true, true], + ], '', 'yes', false, true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'shareType' => [0, 1, 2], - ], '', 'yes', false, false, false, [0, 1], false, true, true], + ], '', 'yes', false, false, false, false, [0, 1], false, true, true], [[ 'itemType' => 'folder', 'shareType' => [0, 1], - ], '', 'yes', false, false, false, [0, 1], false, true, true], + ], '', 'yes', false, false, false, false, [0, 1], false, true, true], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', true, true, true, $allTypes, false, true, true], + ], '', 'yes', false, true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', false, false, false, [0, 1], false, true, true], + ], '', 'yes', false, false, false, false, [0, 1], false, true, true], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', true, false, false, [0, 6], false, true, false], + ], '', 'yes', false, true, false, false, [0, 6], false, true, false], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', false, false, true, [0, 4], false, true, false], + ], '', 'yes', false, false, false, true, [0, 4], false, true, false], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', true, true, false, [0, 6, 9], false, true, false], + ], '', 'yes', false, true, true, false, [0, 6, 9], false, true, false], // Test pagination [[ 'itemType' => 'folder', 'page' => 1, - ], '', 'yes', true, true, true, $allTypes, false, true, true], + ], '', 'yes', false, true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'page' => 10, - ], '', 'yes', true, true, true, $allTypes, false, true, true], + ], '', 'yes', false, true, true, true, $allTypes, false, true, true], // Test perPage [[ 'itemType' => 'folder', 'perPage' => 1, - ], '', 'yes', true, true, true, $allTypes, false, true, true], + ], '', 'yes', false, true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'perPage' => 10, - ], '', 'yes', true, true, true, $allTypes, false, true, true], + ], '', 'yes', false, true, true, true, $allTypes, false, true, true], // Test $shareWithGroupOnly setting [[ 'itemType' => 'folder', - ], 'no', 'yes', true, true, true, $allTypes, false, true, true], + ], 'no', 'yes', false, true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', - ], 'yes', 'yes', true, true, true, $allTypes, true, true, true], + ], 'yes', 'yes', false, true, true, true, $allTypes, true, true, true], // Test $shareeEnumeration setting [[ 'itemType' => 'folder', - ], 'no', 'yes', true, true, true, $allTypes, false, true, true], + ], 'no', 'yes', false, true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', - ], 'no', 'no', true, true, true, $allTypes, false, false, true], + ], 'no', 'no', false, true, true, true, $allTypes, false, false, true], ]; } @@ -233,7 +235,19 @@ public function dataSearch(): array { * @param bool $allowGroupSharing * @throws OCSBadRequestException */ - public function testSearch(array $getData, string $apiSetting, string $enumSetting, bool $remoteSharingEnabled, bool $isRemoteGroupSharingEnabled, bool $emailSharingEnabled, array $shareTypes, bool $shareWithGroupOnly, bool $shareeEnumeration, bool $allowGroupSharing) { + public function testSearch( + array $getData, + string $apiSetting, + string $enumSetting, + bool $sharingDisabledForUser, + bool $remoteSharingEnabled, + bool $isRemoteGroupSharingEnabled, + bool $emailSharingEnabled, + array $shareTypes, + bool $shareWithGroupOnly, + bool $shareeEnumeration, + bool $allowGroupSharing, + ) { $search = $getData['search'] ?? ''; $itemType = $getData['itemType'] ?? 'irrelevant'; $page = $getData['page'] ?? 1; @@ -263,15 +277,15 @@ public function testSearch(array $getData, string $apiSetting, string $enumSetti /** @var MockObject|ShareesAPIController $sharees */ $sharees = $this->getMockBuilder(ShareesAPIController::class) ->setConstructorArgs([ - $uid, 'files_sharing', $request, + $uid, $config, $urlGenerator, $this->shareManager, $this->collaboratorSearch ]) - ->setMethods(['isRemoteSharingAllowed', 'shareProviderExists', 'isRemoteGroupSharingAllowed']) + ->onlyMethods(['isRemoteSharingAllowed', 'isRemoteGroupSharingAllowed']) ->getMock(); $expectedShareTypes = $shareTypes; @@ -293,6 +307,10 @@ public function testSearch(array $getData, string $apiSetting, string $enumSetti ->with($itemType) ->willReturn($isRemoteGroupSharingEnabled); + $this->shareManager->expects($this->any()) + ->method('sharingDisabledForUser') + ->with($uid) + ->willReturn($sharingDisabledForUser); $this->shareManager->expects($this->any()) ->method('shareProviderExists') @@ -358,15 +376,15 @@ public function testSearchInvalid($getData, $message) { /** @var MockObject|ShareesAPIController $sharees */ $sharees = $this->getMockBuilder('\OCA\Files_Sharing\Controller\ShareesAPIController') ->setConstructorArgs([ - $uid, 'files_sharing', $request, + $uid, $config, $urlGenerator, $this->shareManager, $this->collaboratorSearch ]) - ->setMethods(['isRemoteSharingAllowed']) + ->onlyMethods(['isRemoteSharingAllowed']) ->getMock(); $sharees->expects($this->never()) ->method('isRemoteSharingAllowed'); @@ -401,6 +419,22 @@ public function testIsRemoteSharingAllowed($itemType, $expected) { $this->assertSame($expected, $this->invokePrivate($this->sharees, 'isRemoteSharingAllowed', [$itemType])); } + public function testSearchSharingDisabled() { + $this->shareManager->expects($this->once()) + ->method('sharingDisabledForUser') + ->with($this->uid) + ->willReturn(true); + + $this->config->expects($this->once()) + ->method('getSystemValueInt') + ->with('sharing.minSearchStringLength', 0) + ->willReturn(0); + + $this->shareManager->expects($this->never()) + ->method('allowGroupSharing'); + + $this->assertInstanceOf(DataResponse::class, $this->sharees->search('', null, 1, 10, [], false)); + } public function testSearchNoItemType() { $this->expectException(\OCP\AppFramework\OCS\OCSBadRequestException::class);