Skip to content

Commit

Permalink
fix(files_sharing): ShareesAPI - Return empty response when user is n…
Browse files Browse the repository at this point in the history 
…ot allowed to share

Resolves: #20950

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
@susnux @backportbot
susnux authored and backportbot[bot] committed Mar 15, 2024
1 parent fd54328 commit ee02016
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 64 deletions.
33 changes: 9 additions & 24 deletions apps/files_sharing/lib/Controller/ShareesAPIController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -105,8 +93,6 @@ class ShareesAPIController extends OCSController {
];

protected $reachedEndFor = [];
/** @var ISearch */
private $collaboratorSearch;

/**
* @param string $UserId
Expand All @@ -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;
}

/**
Expand All @@ -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) {
Expand Down
114 changes: 74 additions & 40 deletions apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -65,26 +66,27 @@ 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);

$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
Expand All @@ -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],

];
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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')
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ee02016

Please sign in to comment.