mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Make dav respect disallowing sharing with groups 

Closes #25390

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
This commit is contained in:
Thomas Citharel 2021-02-15 16:12:33 +01:00
parent e4b5645883
commit 1c0d975654
No known key found for this signature in database
GPG Key ID: A061B9DDE0CA0773
4 changed files with 146 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
apps/dav/lib/DAV/GroupPrincipalBackend.php
View File

@ -196,9 +196,8 @@ class GroupPrincipalBackend implements BackendInterface {
if ($prefixPath !== self::PRINCIPAL_PREFIX) {
return [];
}
// If sharing is disabled, return the empty array
$shareAPIEnabled = $this->shareManager->shareApiEnabled();
if (!$shareAPIEnabled) {
// If sharing or group sharing is disabled, return the empty array
if (!$this->groupSharingEnabled()) {
return [];
}
@ -273,8 +272,7 @@ class GroupPrincipalBackend implements BackendInterface {
*/
public function findByUri($uri, $principalPrefix) {
// If sharing is disabled, return the empty array
$shareAPIEnabled = $this->shareManager->shareApiEnabled();
if (!$shareAPIEnabled) {
if (!$this->groupSharingEnabled()) {
return null;
}
@ -340,4 +338,11 @@ class GroupPrincipalBackend implements BackendInterface {
return $principal;
}
/**
* @return bool
*/
private function groupSharingEnabled(): bool {
return $this->shareManager->shareApiEnabled() && $this->shareManager->allowGroupSharing();
}
}

73
apps/dav/tests/unit/DAV/GroupPrincipalTest.php
View File

@ -38,19 +38,20 @@ use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Share\IManager;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\DAV\PropPatch;
class GroupPrincipalTest extends \Test\TestCase {
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
/** @var IConfig|MockObject */
private $config;
/** @var IGroupManager | \PHPUnit\Framework\MockObject\MockObject */
/** @var IGroupManager | MockObject */
private $groupManager;
/** @var IUserSession | \PHPUnit\Framework\MockObject\MockObject */
/** @var IUserSession | MockObject */
private $userSession;
/** @var IManager | \PHPUnit\Framework\MockObject\MockObject */
/** @var IManager | MockObject */
private $shareManager;
/** @var GroupPrincipalBackend */
@ -224,13 +225,22 @@ class GroupPrincipalTest extends \Test\TestCase {
/**
* @dataProvider searchPrincipalsDataProvider
* @param bool $sharingEnabled
* @param bool $groupSharingEnabled
* @param bool $groupsOnly
* @param string $test
* @param array $result
*/
public function testSearchPrincipals($sharingEnabled, $groupsOnly, $test, $result) {
public function testSearchPrincipals(bool $sharingEnabled, bool $groupSharingEnabled, bool $groupsOnly, string $test, array $result): void {
$this->shareManager->expects($this->once())
->method('shareAPIEnabled')
->willReturn($sharingEnabled);
if ($sharingEnabled) {
$this->shareManager->expects($sharingEnabled ? $this->once() : $this->never())
->method('allowGroupSharing')
->willReturn($groupSharingEnabled);
if ($sharingEnabled && $groupSharingEnabled) {
$this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly')
->willReturn($groupsOnly);
@ -264,7 +274,7 @@ class GroupPrincipalTest extends \Test\TestCase {
$group5 = $this->createMock(IGroup::class);
$group5->method('getGID')->willReturn('group5');
if ($sharingEnabled) {
if ($sharingEnabled && $groupSharingEnabled) {
$this->groupManager->expects($this->once())
->method('search')
->with('Foo')
@ -280,24 +290,35 @@ class GroupPrincipalTest extends \Test\TestCase {
public function searchPrincipalsDataProvider() {
return [
[true, false, 'allof', ['principals/groups/group1', 'principals/groups/group2', 'principals/groups/group3', 'principals/groups/group4', 'principals/groups/group5']],
[true, false, 'anyof', ['principals/groups/group1', 'principals/groups/group2', 'principals/groups/group3', 'principals/groups/group4', 'principals/groups/group5']],
[true, true, 'allof', ['principals/groups/group1', 'principals/groups/group2', 'principals/groups/group5']],
[true, true, 'anyof', ['principals/groups/group1', 'principals/groups/group2', 'principals/groups/group5']],
[false, false, 'allof', []],
[false, false, 'anyof', []],
[true, true, false, 'allof', ['principals/groups/group1', 'principals/groups/group2', 'principals/groups/group3', 'principals/groups/group4', 'principals/groups/group5']],
[true, true, false, 'anyof', ['principals/groups/group1', 'principals/groups/group2', 'principals/groups/group3', 'principals/groups/group4', 'principals/groups/group5']],
[true, true, true, 'allof', ['principals/groups/group1', 'principals/groups/group2', 'principals/groups/group5']],
[true, true, true, 'anyof', ['principals/groups/group1', 'principals/groups/group2', 'principals/groups/group5']],
[true, false, false, 'allof', []],
[false, true, false, 'anyof', []],
[false, false, false, 'allof', []],
[false, false, false, 'anyof', []],
];
}
/**
* @dataProvider findByUriDataProvider
* @param bool $sharingEnabled
* @param bool $groupSharingEnabled
* @param bool $groupsOnly
* @param string $findUri
* @param string|null $result
*/
public function testFindByUri($sharingEnabled, $groupsOnly, $findUri, $result) {
public function testFindByUri(bool $sharingEnabled, bool $groupSharingEnabled, bool $groupsOnly, string $findUri, ?string $result): void {
$this->shareManager->expects($this->once())
->method('shareAPIEnabled')
->willReturn($sharingEnabled);
if ($sharingEnabled) {
$this->shareManager->expects($sharingEnabled ? $this->once() : $this->never())
->method('allowGroupSharing')
->willReturn($groupSharingEnabled);
if ($sharingEnabled && $groupSharingEnabled) {
$this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly')
->willReturn($groupsOnly);
@ -325,19 +346,23 @@ class GroupPrincipalTest extends \Test\TestCase {
public function findByUriDataProvider() {
return [
[false, false, 'principal:principals/groups/group1', null],
[false, false, 'principal:principals/groups/group3', null],
[false, true, 'principal:principals/groups/group1', null],
[false, true, 'principal:principals/groups/group3', null],
[true, true, 'principal:principals/groups/group1', 'principals/groups/group1'],
[true, true, 'principal:principals/groups/group3', null],
[true, false, 'principal:principals/groups/group1', 'principals/groups/group1'],
[true, false, 'principal:principals/groups/group3', 'principals/groups/group3'],
[false, false, false, 'principal:principals/groups/group1', null],
[false, false, false, 'principal:principals/groups/group3', null],
[false, true, false, 'principal:principals/groups/group1', null],
[false, true, false, 'principal:principals/groups/group3', null],
[false, false, true, 'principal:principals/groups/group1', null],
[false, false, true, 'principal:principals/groups/group3', null],
[true, false, true, 'principal:principals/groups/group1', null],
[true, false, true, 'principal:principals/groups/group3', null],
[true, true, true, 'principal:principals/groups/group1', 'principals/groups/group1'],
[true, true, true, 'principal:principals/groups/group3', null],
[true, true, false, 'principal:principals/groups/group1', 'principals/groups/group1'],
[true, true, false, 'principal:principals/groups/group3', 'principals/groups/group3'],
];
}
/**
* @return Group|\PHPUnit\Framework\MockObject\MockObject
* @return Group|MockObject
*/
private function mockGroup($gid) {
$fooGroup = $this->createMock(Group::class);

50
apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php
View File

@ -41,6 +41,7 @@ use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\Share\IShare;
use OCP\Share\IManager;
use PHPUnit\Framework\MockObject\MockObject;
/**
* Class ShareesTest
@ -56,13 +57,13 @@ class ShareesAPIControllerTest extends TestCase {
/** @var string */
protected $uid;
/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
/** @var IRequest|MockObject */
protected $request;
/** @var IManager|\PHPUnit\Framework\MockObject\MockObject */
/** @var IManager|MockObject */
protected $shareManager;
/** @var ISearch|\PHPUnit\Framework\MockObject\MockObject */
/** @var ISearch|MockObject */
protected $collaboratorSearch;
protected function setUp(): void {
@ -72,10 +73,10 @@ class ShareesAPIControllerTest extends TestCase {
$this->request = $this->createMock(IRequest::class);
$this->shareManager = $this->createMock(IManager::class);
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject $configMock */
/** @var IConfig|MockObject $configMock */
$configMock = $this->createMock(IConfig::class);
/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject $urlGeneratorMock */
/** @var IURLGenerator|MockObject $urlGeneratorMock */
$urlGeneratorMock = $this->createMock(IURLGenerator::class);
$this->collaboratorSearch = $this->createMock(ISearch::class);
@ -91,7 +92,7 @@ class ShareesAPIControllerTest extends TestCase {
);
}
public function dataSearch() {
public function dataSearch(): array {
$noRemote = [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_EMAIL];
$allTypes = [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_REMOTE, IShare::TYPE_REMOTE_GROUP, IShare::TYPE_EMAIL];
@ -123,6 +124,9 @@ class ShareesAPIControllerTest extends TestCase {
[[
'itemType' => 'call',
], '', 'yes', true, true, true, $noRemote, false, true, true],
[[
'itemType' => 'call',
], '', 'yes', true, true, true, [0, 4], false, true, false],
[[
'itemType' => 'folder',
], '', 'yes', true, true, true, $allTypes, false, true, true],
@ -230,14 +234,14 @@ class ShareesAPIControllerTest extends TestCase {
* @param bool $allowGroupSharing
* @throws OCSBadRequestException
*/
public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $isRemoteGroupSharingEnabled, $emailSharingEnabled, $shareTypes, $shareWithGroupOnly, $shareeEnumeration, $allowGroupSharing) {
$search = isset($getData['search']) ? $getData['search'] : '';
$itemType = isset($getData['itemType']) ? $getData['itemType'] : 'irrelevant';
$page = isset($getData['page']) ? $getData['page'] : 1;
$perPage = isset($getData['perPage']) ? $getData['perPage'] : 200;
$shareType = isset($getData['shareType']) ? $getData['shareType'] : null;
public function testSearch(array $getData, string $apiSetting, string $enumSetting, 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;
$perPage = $getData['perPage'] ?? 200;
$shareType = $getData['shareType'] ?? null;
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject $config */
/** @var IConfig|MockObject $config */
$config = $this->createMock(IConfig::class);
$config->expects($this->exactly(1))
->method('getAppValue')
@ -246,19 +250,19 @@ class ShareesAPIControllerTest extends TestCase {
['files_sharing', 'lookupServerEnabled', 'yes', 'yes'],
]);
$this->shareManager->expects($itemType === 'file' || $itemType === 'folder' ? $this->once() : $this->never())
$this->shareManager->expects($this->once())
->method('allowGroupSharing')
->willReturn($allowGroupSharing);
/** @var string */
$uid = 'test123';
/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject $request */
/** @var IRequest|MockObject $request */
$request = $this->createMock(IRequest::class);
/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject $urlGenerator */
/** @var IURLGenerator|MockObject $urlGenerator */
$urlGenerator = $this->createMock(IURLGenerator::class);
/** @var \PHPUnit\Framework\MockObject\MockObject|\OCA\Files_Sharing\Controller\ShareesAPIController $sharees */
$sharees = $this->getMockBuilder('\OCA\Files_Sharing\Controller\ShareesAPIController')
/** @var MockObject|ShareesAPIController $sharees */
$sharees = $this->getMockBuilder(ShareesAPIController::class)
->setConstructorArgs([
$uid,
'files_sharing',
@ -304,7 +308,7 @@ class ShareesAPIControllerTest extends TestCase {
$this->assertInstanceOf(Http\DataResponse::class, $sharees->search($search, $itemType, $page, $perPage, $shareType));
}
public function dataSearchInvalid() {
public function dataSearchInvalid(): array {
return [
// Test invalid pagination
[[
@ -340,19 +344,19 @@ class ShareesAPIControllerTest extends TestCase {
$page = isset($getData['page']) ? $getData['page'] : 1;
$perPage = isset($getData['perPage']) ? $getData['perPage'] : 200;
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject $config */
/** @var IConfig|MockObject $config */
$config = $this->createMock(IConfig::class);
$config->expects($this->never())
->method('getAppValue');
/** @var string */
$uid = 'test123';
/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject $request */
/** @var IRequest|MockObject $request */
$request = $this->createMock(IRequest::class);
/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject $urlGenerator */
/** @var IURLGenerator|MockObject $urlGenerator */
$urlGenerator = $this->createMock(IURLGenerator::class);
/** @var \PHPUnit\Framework\MockObject\MockObject|\OCA\Files_Sharing\Controller\ShareesAPIController $sharees */
/** @var MockObject|ShareesAPIController $sharees */
$sharees = $this->getMockBuilder('\OCA\Files_Sharing\Controller\ShareesAPIController')
->setConstructorArgs([
$uid,

105
tests/lib/Collaboration/Collaborators/GroupPluginTest.php
View File

@ -125,13 +125,15 @@ class GroupPluginTest extends TestCase {
return $group;
}
public function dataGetGroups() {
public function dataGetGroups(): array {
return [
['test', false, true, [], [], [], [], true, false],
['test', false, false, [], [], [], [], true, false],
['test', false, true, false, [], [], [], [], true, false],
['test', false, false, false, [], [], [], [], true, false],
// group sharing disabled
['test', false, true, true, [], [], [], [], false, false],
// group without display name
[
'test', false, true,
'test', false, true, false,
[$this->getGroupMock('test1')],
[],
[],
@ -141,7 +143,7 @@ class GroupPluginTest extends TestCase {
],
// group with display name, search by id
[
'test', false, true,
'test', false, true, false,
[$this->getGroupMock('test1', 'Test One')],
[],
[],
@ -151,7 +153,7 @@ class GroupPluginTest extends TestCase {
],
// group with display name, search by display name
[
'one', false, true,
'one', false, true, false,
[$this->getGroupMock('test1', 'Test One')],
[],
[],
@ -161,7 +163,7 @@ class GroupPluginTest extends TestCase {
],
// group with display name, search by display name, exact expected
[
'Test One', false, true,
'Test One', false, true, false,
[$this->getGroupMock('test1', 'Test One')],
[],
[['label' => 'Test One', 'value' => ['shareType' => IShare::TYPE_GROUP, 'shareWith' => 'test1']]],
@ -170,7 +172,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', false, false,
'test', false, false, false,
[$this->getGroupMock('test1')],
[],
[],
@ -179,7 +181,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', false, true,
'test', false, true, false,
[
$this->getGroupMock('test'),
$this->getGroupMock('test1'),
@ -191,7 +193,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', false, false,
'test', false, false, false,
[
$this->getGroupMock('test'),
$this->getGroupMock('test1'),
@ -203,7 +205,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', false, true,
'test', false, true, false,
[
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
@ -218,7 +220,7 @@ class GroupPluginTest extends TestCase {
null,
],
[
'test', false, false,
'test', false, false, false,
[
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
@ -230,7 +232,7 @@ class GroupPluginTest extends TestCase {
null,
],
[
'test', false, true,
'test', false, true, false,
[
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
@ -247,7 +249,7 @@ class GroupPluginTest extends TestCase {
$this->getGroupMock('test'),
],
[
'test', false, false,
'test', false, false, false,
[
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
@ -260,10 +262,10 @@ class GroupPluginTest extends TestCase {
true,
$this->getGroupMock('test'),
],
['test', true, true, [], [], [], [], true, false],
['test', true, false, [], [], [], [], true, false],
['test', true, true, false, [], [], [], [], true, false],
['test', true, false, false, [], [], [], [], true, false],
[
'test', true, true,
'test', true, true, false,
[
$this->getGroupMock('test1'),
$this->getGroupMock('test2'),
@ -275,7 +277,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', true, false,
'test', true, false, false,
[
$this->getGroupMock('test1'),
$this->getGroupMock('test2'),
@ -287,7 +289,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', true, true,
'test', true, true, false,
[
$this->getGroupMock('test'),
$this->getGroupMock('test1'),
@ -299,7 +301,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', true, false,
'test', true, false, false,
[
$this->getGroupMock('test'),
$this->getGroupMock('test1'),
@ -311,7 +313,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', true, true,
'test', true, true, false,
[
$this->getGroupMock('test'),
$this->getGroupMock('test1'),
@ -323,7 +325,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', true, false,
'test', true, false, false,
[
$this->getGroupMock('test'),
$this->getGroupMock('test1'),
@ -335,7 +337,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', true, true,
'test', true, true, false,
[
$this->getGroupMock('test'),
$this->getGroupMock('test1'),
@ -347,7 +349,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', true, false,
'test', true, false, false,
[
$this->getGroupMock('test'),
$this->getGroupMock('test1'),
@ -359,7 +361,7 @@ class GroupPluginTest extends TestCase {
false,
],
[
'test', true, true,
'test', true, true, false,
[
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
@ -374,7 +376,7 @@ class GroupPluginTest extends TestCase {
null,
],
[
'test', true, false,
'test', true, false, false,
[
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
@ -386,7 +388,7 @@ class GroupPluginTest extends TestCase {
null,
],
[
'test', true, true,
'test', true, true, false,
[
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
@ -403,7 +405,7 @@ class GroupPluginTest extends TestCase {
$this->getGroupMock('test'),
],
[
'test', true, false,
'test', true, false, false,
[
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
@ -417,7 +419,7 @@ class GroupPluginTest extends TestCase {
$this->getGroupMock('test'),
],
[
'test', false, false,
'test', false, false, false,
[
$this->getGroupMock('test', null, true),
$this->getGroupMock('test1'),
@ -437,6 +439,7 @@ class GroupPluginTest extends TestCase {
* @param string $searchTerm
* @param bool $shareWithGroupOnly
* @param bool $shareeEnumeration
* @param bool $groupSharingDisabled
* @param array $groupResponse
* @param array $userGroupsResponse
* @param array $exactExpected
@ -445,35 +448,45 @@ class GroupPluginTest extends TestCase {
* @param bool|IGroup $singleGroup
*/
public function testSearch(
$searchTerm,
$shareWithGroupOnly,
$shareeEnumeration,
string $searchTerm,
bool $shareWithGroupOnly,
bool $shareeEnumeration,
bool $groupSharingDisabled,
array $groupResponse,
array $userGroupsResponse,
array $exactExpected,
array $expected,
$reachedEnd,
bool $reachedEnd,
$singleGroup
) {
$this->config->expects($this->any())
->method('getAppValue')
->willReturnCallback(
function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration) {
if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') {
return $shareWithGroupOnly ? 'yes' : 'no';
} elseif ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
return $shareeEnumeration ? 'yes' : 'no';
function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration, $groupSharingDisabled) {
if ($appName !== 'core') {
return $default;
}
switch ($key) {
case 'shareapi_only_share_with_group_members':
return $shareWithGroupOnly ? 'yes' : 'no';
case 'shareapi_allow_share_dialog_user_enumeration':
return $shareeEnumeration ? 'yes' : 'no';
case 'shareapi_allow_group_sharing':
return $groupSharingDisabled ? 'no' : 'yes';
default:
return $default;
}
return $default;
}
);
$this->instantiatePlugin();
$this->groupManager->expects($this->once())
->method('search')
->with($searchTerm, $this->limit, $this->offset)
->willReturn($groupResponse);
if (!$groupSharingDisabled) {
$this->groupManager->expects($this->once())
->method('search')
->with($searchTerm, $this->limit, $this->offset)
->willReturn($groupResponse);
}
if ($singleGroup !== false) {
$this->groupManager->expects($this->once())
@ -497,8 +510,10 @@ class GroupPluginTest extends TestCase {
$moreResults = $this->plugin->search($searchTerm, $this->limit, $this->offset, $this->searchResult);
$result = $this->searchResult->asArray();
$this->assertEquals($exactExpected, $result['exact']['groups']);
$this->assertEquals($expected, $result['groups']);
if (!$groupSharingDisabled) {
$this->assertEquals($exactExpected, $result['exact']['groups']);
$this->assertEquals($expected, $result['groups']);
}
$this->assertSame($reachedEnd, $moreResults);
}
}