Merge pull request #19569 from nextcloud/enh/noid/restrict-user-enumeration-to-groups

Restrict sharing user enumeration to groups
This commit is contained in:
Roeland Jago Douma 2020-03-20 13:40:47 +01:00 committed by GitHub
commit 2efa000705
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 515 additions and 78 deletions

View File

@ -40,7 +40,9 @@ class SystemAddressbook extends AddressBook {
} }
public function getChildren() { public function getChildren() {
if ($this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes') { $shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$restrictShareEnumeration = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'yes') === 'yes';
if (!$shareEnumeration || ($shareEnumeration && $restrictShareEnumeration)) {
return []; return [];
} }

View File

@ -245,7 +245,8 @@ class Principal implements BackendInterface {
return []; return [];
} }
$allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $allowEnumeration = $this->shareManager->allowEnumeration();
$limitEnumeration = $this->shareManager->limitEnumerationToGroups();
// If sharing is restricted to group members only, // If sharing is restricted to group members only,
// return only members that have groups in common // return only members that have groups in common
@ -259,6 +260,14 @@ class Principal implements BackendInterface {
$restrictGroups = $this->groupManager->getUserGroupIds($user); $restrictGroups = $this->groupManager->getUserGroupIds($user);
} }
$currentUserGroups = [];
if ($limitEnumeration) {
$currentUser = $this->userSession->getUser();
if ($currentUser) {
$currentUserGroups = $this->groupManager->getUserGroupIds($currentUser);
}
}
foreach ($searchProperties as $prop => $value) { foreach ($searchProperties as $prop => $value) {
switch ($prop) { switch ($prop) {
case '{http://sabredav.org/ns}email-address': case '{http://sabredav.org/ns}email-address':
@ -270,6 +279,15 @@ class Principal implements BackendInterface {
}); });
} }
if ($limitEnumeration) {
$users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) {
return !empty(array_intersect(
$this->groupManager->getUserGroupIds($user),
$currentUserGroups
)) || $user->getEMailAddress() === $value;
});
}
$results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) { $results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) {
// is sharing restricted to groups only? // is sharing restricted to groups only?
if ($restrictGroups !== false) { if ($restrictGroups !== false) {
@ -293,6 +311,15 @@ class Principal implements BackendInterface {
}); });
} }
if ($limitEnumeration) {
$users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) {
return !empty(array_intersect(
$this->groupManager->getUserGroupIds($user),
$currentUserGroups
)) || $user->getDisplayName() === $value;
});
}
$results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) { $results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) {
// is sharing restricted to groups only? // is sharing restricted to groups only?
if ($restrictGroups !== false) { if ($restrictGroups !== false) {

View File

@ -431,10 +431,9 @@ class PrincipalTest extends TestCase {
->will($this->returnValue($sharingEnabled)); ->will($this->returnValue($sharingEnabled));
if ($sharingEnabled) { if ($sharingEnabled) {
$this->config->expects($this->once()) $this->shareManager->expects($this->once())
->method('getAppValue') ->method('allowEnumeration')
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') ->willReturn(true);
->willReturn('yes');
$this->shareManager->expects($this->once()) $this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly') ->method('shareWithGroupMembersOnly')
@ -526,10 +525,9 @@ class PrincipalTest extends TestCase {
->method('shareAPIEnabled') ->method('shareAPIEnabled')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$this->config->expects($this->exactly(2)) $this->shareManager->expects($this->exactly(2))
->method('getAppValue') ->method('allowEnumeration')
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') ->willReturn(true);
->willReturn('yes');
$this->shareManager->expects($this->exactly(2)) $this->shareManager->expects($this->exactly(2))
->method('shareWithGroupMembersOnly') ->method('shareWithGroupMembersOnly')
@ -557,10 +555,9 @@ class PrincipalTest extends TestCase {
->method('shareAPIEnabled') ->method('shareAPIEnabled')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$this->config->expects($this->once()) $this->shareManager->expects($this->once())
->method('getAppValue') ->method('allowEnumeration')
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') ->willReturn(false);
->willReturn('no');
$this->shareManager->expects($this->once()) $this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly') ->method('shareWithGroupMembersOnly')
@ -593,10 +590,9 @@ class PrincipalTest extends TestCase {
->method('shareAPIEnabled') ->method('shareAPIEnabled')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$this->config->expects($this->once()) $this->shareManager->expects($this->once())
->method('getAppValue') ->method('allowEnumeration')
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') ->willReturn(false);
->willReturn('no');
$this->shareManager->expects($this->once()) $this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly') ->method('shareWithGroupMembersOnly')
@ -624,6 +620,128 @@ class PrincipalTest extends TestCase {
['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar']));
} }
public function testSearchPrincipalWithEnumerationLimitedDisplayname() {
$this->shareManager->expects($this->at(0))
->method('shareAPIEnabled')
->will($this->returnValue(true));
$this->shareManager->expects($this->at(1))
->method('allowEnumeration')
->willReturn(true);
$this->shareManager->expects($this->at(2))
->method('limitEnumerationToGroups')
->willReturn(true);
$this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly')
->will($this->returnValue(false));
$user2 = $this->createMock(IUser::class);
$user2->method('getUID')->will($this->returnValue('user2'));
$user2->method('getDisplayName')->will($this->returnValue('User 2'));
$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar'));
$user3 = $this->createMock(IUser::class);
$user3->method('getUID')->will($this->returnValue('user3'));
$user3->method('getDisplayName')->will($this->returnValue('User 22'));
$user3->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123'));
$user4 = $this->createMock(IUser::class);
$user4->method('getUID')->will($this->returnValue('user4'));
$user4->method('getDisplayName')->will($this->returnValue('User 222'));
$user4->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456'));
$this->userSession->expects($this->at(0))
->method('getUser')
->willReturn($user2);
$this->groupManager->expects($this->at(0))
->method('getUserGroupIds')
->willReturn(['group1']);
$this->groupManager->expects($this->at(1))
->method('getUserGroupIds')
->willReturn(['group1']);
$this->groupManager->expects($this->at(2))
->method('getUserGroupIds')
->willReturn(['group1']);
$this->groupManager->expects($this->at(3))
->method('getUserGroupIds')
->willReturn(['group2']);
$this->userManager->expects($this->at(0))
->method('searchDisplayName')
->with('User')
->willReturn([$user2, $user3, $user4]);
$this->assertEquals([
'principals/users/user2',
'principals/users/user3',
], $this->connector->searchPrincipals('principals/users',
['{DAV:}displayname' => 'User']));
}
public function testSearchPrincipalWithEnumerationLimitedMail() {
$this->shareManager->expects($this->at(0))
->method('shareAPIEnabled')
->will($this->returnValue(true));
$this->shareManager->expects($this->at(1))
->method('allowEnumeration')
->willReturn(true);
$this->shareManager->expects($this->at(2))
->method('limitEnumerationToGroups')
->willReturn(true);
$this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly')
->will($this->returnValue(false));
$user2 = $this->createMock(IUser::class);
$user2->method('getUID')->will($this->returnValue('user2'));
$user2->method('getDisplayName')->will($this->returnValue('User 2'));
$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar'));
$user3 = $this->createMock(IUser::class);
$user3->method('getUID')->will($this->returnValue('user3'));
$user3->method('getDisplayName')->will($this->returnValue('User 22'));
$user3->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123'));
$user4 = $this->createMock(IUser::class);
$user4->method('getUID')->will($this->returnValue('user4'));
$user4->method('getDisplayName')->will($this->returnValue('User 222'));
$user4->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456'));
$this->userSession->expects($this->at(0))
->method('getUser')
->willReturn($user2);
$this->groupManager->expects($this->at(0))
->method('getUserGroupIds')
->willReturn(['group1']);
$this->groupManager->expects($this->at(1))
->method('getUserGroupIds')
->willReturn(['group1']);
$this->groupManager->expects($this->at(2))
->method('getUserGroupIds')
->willReturn(['group1']);
$this->groupManager->expects($this->at(3))
->method('getUserGroupIds')
->willReturn(['group2']);
$this->userManager->expects($this->at(0))
->method('getByEmail')
->with('user')
->willReturn([$user2, $user3, $user4]);
$this->assertEquals([
'principals/users/user2',
'principals/users/user3'
], $this->connector->searchPrincipals('principals/users',
['{http://sabredav.org/ns}email-address' => 'user']));
}
public function testFindByUriSharingApiDisabled() { public function testFindByUriSharingApiDisabled() {
$this->shareManager->expects($this->once()) $this->shareManager->expects($this->once())
->method('shareApiEnabled') ->method('shareApiEnabled')

View File

@ -66,12 +66,6 @@ class ShareesAPIController extends OCSController {
/** @var IManager */ /** @var IManager */
protected $shareManager; protected $shareManager;
/** @var bool */
protected $shareWithGroupOnly = false;
/** @var bool */
protected $shareeEnumeration = true;
/** @var int */ /** @var int */
protected $offset = 0; protected $offset = 0;
@ -205,8 +199,6 @@ class ShareesAPIController extends OCSController {
} }
sort($shareTypes); sort($shareTypes);
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$this->limit = (int) $perPage; $this->limit = (int) $perPage;
$this->offset = $perPage * ($page - 1); $this->offset = $perPage * ($page - 1);

View File

@ -237,12 +237,10 @@ class ShareesAPIControllerTest extends TestCase {
/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject $config */ /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject $config */
$config = $this->createMock(IConfig::class); $config = $this->createMock(IConfig::class);
$config->expects($this->exactly(3)) $config->expects($this->exactly(1))
->method('getAppValue') ->method('getAppValue')
->with($this->anything(), $this->anything(), $this->anything()) ->with($this->anything(), $this->anything(), $this->anything())
->willReturnMap([ ->willReturnMap([
['core', 'shareapi_only_share_with_group_members', 'no', $apiSetting],
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', $enumSetting],
['files_sharing', 'lookupServerEnabled', 'yes', 'yes'], ['files_sharing', 'lookupServerEnabled', 'yes', 'yes'],
]); ]);
@ -302,9 +300,6 @@ class ShareesAPIControllerTest extends TestCase {
})); }));
$this->assertInstanceOf(Http\DataResponse::class, $sharees->search($search, $itemType, $page, $perPage, $shareType)); $this->assertInstanceOf(Http\DataResponse::class, $sharees->search($search, $itemType, $page, $perPage, $shareType));
$this->assertSame($shareWithGroupOnly, $this->invokePrivate($sharees, 'shareWithGroupOnly'));
$this->assertSame($shareeEnumeration, $this->invokePrivate($sharees, 'shareeEnumeration'));
} }
public function dataSearchInvalid() { public function dataSearchInvalid() {

View File

@ -142,6 +142,10 @@ $(document).ready(function(){
savePublicShareDisclaimerText(this.value); savePublicShareDisclaimerText(this.value);
}); });
$('#shareapi_allow_share_dialog_user_enumeration').on('change', function() {
$('#shareapi_restrict_user_enumeration_to_group_setting').toggleClass('hidden', !this.checked);
})
$('#allowLinks').change(function() { $('#allowLinks').change(function() {
$("#publicLinkSettings").toggleClass('hidden', !this.checked); $("#publicLinkSettings").toggleClass('hidden', !this.checked);
$('#setDefaultExpireDate').toggleClass('hidden', !(this.checked && $('#shareapiDefaultExpireDate')[0].checked)); $('#setDefaultExpireDate').toggleClass('hidden', !(this.checked && $('#shareapiDefaultExpireDate')[0].checked));

View File

@ -73,6 +73,7 @@ class Sharing implements ISettings {
'allowPublicUpload' => $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes'), 'allowPublicUpload' => $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes'),
'allowResharing' => $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes'), 'allowResharing' => $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes'),
'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'),
'restrictUserEnumerationToGroup' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no'),
'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(),
'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(), 'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(),
'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'),

View File

@ -109,11 +109,19 @@
<br /> <br />
<em><?php p($l->t('These groups will still be able to receive shares, but not to initiate them.')); ?></em> <em><?php p($l->t('These groups will still be able to receive shares, but not to initiate them.')); ?></em>
</p> </p>
<p class="<?php if ($_['shareAPIEnabled'] === 'no') p('hidden');?>"> <p class="<?php if ($_['shareAPIEnabled'] === 'no') p('hidden');?>">
<input type="checkbox" name="shareapi_allow_share_dialog_user_enumeration" value="1" id="shareapi_allow_share_dialog_user_enumeration" class="checkbox" <input type="checkbox" name="shareapi_allow_share_dialog_user_enumeration" value="1" id="shareapi_allow_share_dialog_user_enumeration" class="checkbox"
<?php if ($_['allowShareDialogUserEnumeration'] === 'yes') print_unescaped('checked="checked"'); ?> /> <?php if ($_['allowShareDialogUserEnumeration'] === 'yes') print_unescaped('checked="checked"'); ?> />
<label for="shareapi_allow_share_dialog_user_enumeration"><?php p($l->t('Allow username autocompletion in share dialog. If this is disabled the full username or email address needs to be entered.'));?></label><br /> <label for="shareapi_allow_share_dialog_user_enumeration"><?php p($l->t('Allow username autocompletion in share dialog. If this is disabled the full username or email address needs to be entered.'));?></label><br />
</p> </p>
<p id="shareapi_restrict_user_enumeration_to_group_setting" class="indent <?php if ($_['shareAPIEnabled'] === 'no' || $_['allowShareDialogUserEnumeration'] === 'no') p('hidden');?>">
<input type="checkbox" name="shareapi_restrict_user_enumeration_to_group" value="1" id="shareapi_restrict_user_enumeration_to_group" class="checkbox"
<?php if ($_['restrictUserEnumerationToGroup'] === 'yes') print_unescaped('checked="checked"'); ?> />
<label for="shareapi_restrict_user_enumeration_to_group"><?php p($l->t('Restrict username autocompletion to users within the same groups'));?></label><br />
</p>
<p> <p>
<input type="checkbox" id="publicShareDisclaimer" class="checkbox noJSAutoUpdate" <input type="checkbox" id="publicShareDisclaimer" class="checkbox noJSAutoUpdate"
<?php if ($_['publicShareDisclaimerText'] !== null) print_unescaped('checked="checked"'); ?> /> <?php if ($_['publicShareDisclaimerText'] !== null) print_unescaped('checked="checked"'); ?> />

View File

@ -94,55 +94,60 @@ class SharingTest extends TestCase {
$this->config $this->config
->expects($this->at(6)) ->expects($this->at(6))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_restrict_user_enumeration_to_group', 'no')
->willReturn('no');
$this->config
->expects($this->at(7))
->method('getAppValue')
->with('core', 'shareapi_enabled', 'yes') ->with('core', 'shareapi_enabled', 'yes')
->willReturn('yes'); ->willReturn('yes');
$this->config $this->config
->expects($this->at(7)) ->expects($this->at(8))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_default_expire_date', 'no') ->with('core', 'shareapi_default_expire_date', 'no')
->willReturn('no'); ->willReturn('no');
$this->config $this->config
->expects($this->at(8)) ->expects($this->at(9))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_expire_after_n_days', '7') ->with('core', 'shareapi_expire_after_n_days', '7')
->willReturn('7'); ->willReturn('7');
$this->config $this->config
->expects($this->at(9)) ->expects($this->at(10))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_enforce_expire_date', 'no') ->with('core', 'shareapi_enforce_expire_date', 'no')
->willReturn('no'); ->willReturn('no');
$this->config $this->config
->expects($this->at(10)) ->expects($this->at(11))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_exclude_groups', 'no') ->with('core', 'shareapi_exclude_groups', 'no')
->willReturn('no'); ->willReturn('no');
$this->config $this->config
->expects($this->at(11)) ->expects($this->at(12))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_public_link_disclaimertext', null) ->with('core', 'shareapi_public_link_disclaimertext', null)
->willReturn('Lorem ipsum'); ->willReturn('Lorem ipsum');
$this->config $this->config
->expects($this->at(12)) ->expects($this->at(13))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_enable_link_password_by_default', 'no') ->with('core', 'shareapi_enable_link_password_by_default', 'no')
->willReturn('yes'); ->willReturn('yes');
$this->config $this->config
->expects($this->at(13)) ->expects($this->at(14))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL) ->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL)
->willReturn(Constants::PERMISSION_ALL); ->willReturn(Constants::PERMISSION_ALL);
$this->config $this->config
->expects($this->at(14)) ->expects($this->at(15))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_default_internal_expire_date', 'no') ->with('core', 'shareapi_default_internal_expire_date', 'no')
->willReturn('no'); ->willReturn('no');
$this->config $this->config
->expects($this->at(15)) ->expects($this->at(16))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_internal_expire_after_n_days', '7') ->with('core', 'shareapi_internal_expire_after_n_days', '7')
->willReturn('7'); ->willReturn('7');
$this->config $this->config
->expects($this->at(16)) ->expects($this->at(17))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_enforce_internal_expire_date', 'no') ->with('core', 'shareapi_enforce_internal_expire_date', 'no')
->willReturn('no'); ->willReturn('no');
@ -156,6 +161,7 @@ class SharingTest extends TestCase {
'allowPublicUpload' => 'yes', 'allowPublicUpload' => 'yes',
'allowResharing' => 'yes', 'allowResharing' => 'yes',
'allowShareDialogUserEnumeration' => 'yes', 'allowShareDialogUserEnumeration' => 'yes',
'restrictUserEnumerationToGroup' => 'no',
'enforceLinkPassword' => false, 'enforceLinkPassword' => false,
'onlyShareWithGroupMembers' => false, 'onlyShareWithGroupMembers' => false,
'shareAPIEnabled' => 'yes', 'shareAPIEnabled' => 'yes',
@ -212,55 +218,60 @@ class SharingTest extends TestCase {
$this->config $this->config
->expects($this->at(6)) ->expects($this->at(6))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_restrict_user_enumeration_to_group', 'no')
->willReturn('no');
$this->config
->expects($this->at(7))
->method('getAppValue')
->with('core', 'shareapi_enabled', 'yes') ->with('core', 'shareapi_enabled', 'yes')
->willReturn('yes'); ->willReturn('yes');
$this->config $this->config
->expects($this->at(7)) ->expects($this->at(8))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_default_expire_date', 'no') ->with('core', 'shareapi_default_expire_date', 'no')
->willReturn('no'); ->willReturn('no');
$this->config $this->config
->expects($this->at(8)) ->expects($this->at(9))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_expire_after_n_days', '7') ->with('core', 'shareapi_expire_after_n_days', '7')
->willReturn('7'); ->willReturn('7');
$this->config $this->config
->expects($this->at(9)) ->expects($this->at(10))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_enforce_expire_date', 'no') ->with('core', 'shareapi_enforce_expire_date', 'no')
->willReturn('no'); ->willReturn('no');
$this->config $this->config
->expects($this->at(10)) ->expects($this->at(11))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_exclude_groups', 'no') ->with('core', 'shareapi_exclude_groups', 'no')
->willReturn('yes'); ->willReturn('yes');
$this->config $this->config
->expects($this->at(11)) ->expects($this->at(12))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_public_link_disclaimertext', null) ->with('core', 'shareapi_public_link_disclaimertext', null)
->willReturn('Lorem ipsum'); ->willReturn('Lorem ipsum');
$this->config $this->config
->expects($this->at(12)) ->expects($this->at(13))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_enable_link_password_by_default', 'no') ->with('core', 'shareapi_enable_link_password_by_default', 'no')
->willReturn('yes'); ->willReturn('yes');
$this->config $this->config
->expects($this->at(13)) ->expects($this->at(14))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL) ->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL)
->willReturn(Constants::PERMISSION_ALL); ->willReturn(Constants::PERMISSION_ALL);
$this->config $this->config
->expects($this->at(14)) ->expects($this->at(15))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_default_internal_expire_date', 'no') ->with('core', 'shareapi_default_internal_expire_date', 'no')
->willReturn('no'); ->willReturn('no');
$this->config $this->config
->expects($this->at(15)) ->expects($this->at(16))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_internal_expire_after_n_days', '7') ->with('core', 'shareapi_internal_expire_after_n_days', '7')
->willReturn('7'); ->willReturn('7');
$this->config $this->config
->expects($this->at(16)) ->expects($this->at(17))
->method('getAppValue') ->method('getAppValue')
->with('core', 'shareapi_enforce_internal_expire_date', 'no') ->with('core', 'shareapi_enforce_internal_expire_date', 'no')
->willReturn('no'); ->willReturn('no');
@ -275,6 +286,7 @@ class SharingTest extends TestCase {
'allowPublicUpload' => 'yes', 'allowPublicUpload' => 'yes',
'allowResharing' => 'yes', 'allowResharing' => 'yes',
'allowShareDialogUserEnumeration' => 'yes', 'allowShareDialogUserEnumeration' => 'yes',
'restrictUserEnumerationToGroup' => 'no',
'enforceLinkPassword' => false, 'enforceLinkPassword' => false,
'onlyShareWithGroupMembers' => false, 'onlyShareWithGroupMembers' => false,
'shareAPIEnabled' => 'yes', 'shareAPIEnabled' => 'yes',

View File

@ -52,6 +52,7 @@ class GroupPlugin implements ISearchPlugin {
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
} }
public function search($search, $limit, $offset, ISearchResult $searchResult) { public function search($search, $limit, $offset, ISearchResult $searchResult) {
@ -66,7 +67,7 @@ class GroupPlugin implements ISearchPlugin {
} }
$userGroups = []; $userGroups = [];
if (!empty($groups) && $this->shareWithGroupOnly) { if (!empty($groups) && ($this->shareWithGroupOnly || $this->shareeEnumerationInGroupOnly)) {
// Intersect all the groups that match with the groups this user is a member of // Intersect all the groups that match with the groups this user is a member of
$userGroups = $this->groupManager->getUserGroups($this->userSession->getUser()); $userGroups = $this->groupManager->getUserGroups($this->userSession->getUser());
$userGroups = array_map(function (IGroup $group) { return $group->getGID(); }, $userGroups); $userGroups = array_map(function (IGroup $group) { return $group->getGID(); }, $userGroups);
@ -93,6 +94,9 @@ class GroupPlugin implements ISearchPlugin {
], ],
]; ];
} else { } else {
if ($this->shareeEnumerationInGroupOnly && !in_array($group->getGID(), $userGroups, true)) {
continue;
}
$result['wide'][] = [ $result['wide'][] = [
'label' => $group->getDisplayName(), 'label' => $group->getDisplayName(),
'value' => [ 'value' => [

View File

@ -65,6 +65,8 @@ class MailPlugin implements ISearchPlugin {
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
} }
/** /**
@ -150,7 +152,18 @@ class MailPlugin implements ISearchPlugin {
continue; continue;
} }
if (!$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) { $addToWide = !$this->shareeEnumerationInGroupOnly;
if ($this->shareeEnumerationInGroupOnly) {
$addToWide = false;
$userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
foreach ($userGroups as $userGroup) {
if ($this->groupManager->isInGroup($contact['UID'], $userGroup)) {
$addToWide = true;
break;
}
}
}
if ($addToWide && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) {
$userResults['wide'][] = [ $userResults['wide'][] = [
'label' => $displayName, 'label' => $displayName,
'uuid' => $contact['UID'], 'uuid' => $contact['UID'],
@ -160,6 +173,7 @@ class MailPlugin implements ISearchPlugin {
'shareWith' => $cloud->getUser(), 'shareWith' => $cloud->getUser(),
], ],
]; ];
continue;
} }
} }
continue; continue;

View File

@ -36,11 +36,13 @@ use OCP\IUser;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\IUserSession; use OCP\IUserSession;
use OCP\Share; use OCP\Share;
use OCP\Share\IShare;
class UserPlugin implements ISearchPlugin { class UserPlugin implements ISearchPlugin {
/* @var bool */ /* @var bool */
protected $shareWithGroupOnly; protected $shareWithGroupOnly;
protected $shareeEnumeration; protected $shareeEnumeration;
protected $shareeEnumerationInGroupOnly;
/** @var IConfig */ /** @var IConfig */
private $config; private $config;
@ -60,11 +62,13 @@ class UserPlugin implements ISearchPlugin {
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
} }
public function search($search, $limit, $offset, ISearchResult $searchResult) { public function search($search, $limit, $offset, ISearchResult $searchResult) {
$result = ['wide' => [], 'exact' => []]; $result = ['wide' => [], 'exact' => []];
$users = []; $users = [];
$autoCompleteUsers = [];
$hasMoreResults = false; $hasMoreResults = false;
$userGroups = []; $userGroups = [];
@ -80,10 +84,32 @@ class UserPlugin implements ISearchPlugin {
} else { } else {
// Search in all users // Search in all users
$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset); $usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
$currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
foreach ($usersTmp as $user) { foreach ($usersTmp as $user) {
if ($user->isEnabled()) { // Don't keep deactivated users if ($user->isEnabled()) { // Don't keep deactivated users
$users[(string) $user->getUID()] = $user->getDisplayName(); $users[(string) $user->getUID()] = $user->getDisplayName();
$addToWideResults = false;
if ($this->shareeEnumeration && !$this->shareeEnumerationInGroupOnly) {
$addToWideResults = true;
}
if ($this->shareeEnumerationInGroupOnly) {
$commonGroups = array_intersect($currentUserGroups, $this->groupManager->getUserGroupIds($user));
if (!empty($commonGroups)) {
$addToWideResults = true;
}
}
if ($addToWideResults) {
$autoCompleteUsers[] = [
'label' => $user->getDisplayName(),
'value' => [
'shareType' => IShare::TYPE_USER,
'shareWith' => (string)$user->getUID(),
],
];
}
} }
} }
} }
@ -145,8 +171,9 @@ class UserPlugin implements ISearchPlugin {
} }
} }
if (!$this->shareeEnumeration) { // overwrite wide matches if they are limited
$result['wide'] = []; if (!$this->shareeEnumeration || $this->shareeEnumerationInGroupOnly) {
$result['wide'] = $autoCompleteUsers;
} }
$type = new SearchResultType('users'); $type = new SearchResultType('users');

View File

@ -107,12 +107,13 @@ class ContactsStore implements IContactsStore {
array $entries, array $entries,
$filter) { $filter) {
$disallowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes'; $disallowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes';
$restrictEnumeration = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'yes') === 'yes';
$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes'; $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes';
// whether to filter out local users // whether to filter out local users
$skipLocal = false; $skipLocal = false;
// whether to filter out all users which doesn't have the same group as the current user // whether to filter out all users which doesn't have the same group as the current user
$ownGroupsOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $ownGroupsOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes' || $restrictEnumeration;
$selfGroups = $this->groupManager->getUserGroupIds($self); $selfGroups = $this->groupManager->getUserGroupIds($self);

View File

@ -1764,6 +1764,15 @@ class Manager implements IManager {
return $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes'; return $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes';
} }
public function allowEnumeration(): bool {
return $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
}
public function limitEnumerationToGroups(): bool {
return $this->allowEnumeration() &&
$this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
}
/** /**
* Copied from \OC_Util::isSharingDisabledForUser * Copied from \OC_Util::isSharingDisabledForUser
* *

View File

@ -368,6 +368,22 @@ interface IManager {
*/ */
public function allowGroupSharing(); public function allowGroupSharing();
/**
* Check if user enumeration is allowed
*
* @return bool
* @since 19.0.0
*/
public function allowEnumeration(): bool;
/**
* Check if user enumeration is limited to the users groups
*
* @return bool
* @since 19.0.0
*/
public function limitEnumerationToGroups(): bool;
/** /**
* Check if sharing is disabled for the given user * Check if sharing is disabled for the given user
* *

View File

@ -89,7 +89,27 @@ class UserPluginTest extends TestCase {
); );
} }
public function getUserMock($uid, $displayName, $enabled = true) { public function mockConfig($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup) {
$this->config->expects($this->any())
->method('getAppValue')
->willReturnCallback(
function($appName, $key, $default)
use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup)
{
if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') {
return $shareWithGroupOnly ? 'yes' : 'no';
} else if ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
return $shareeEnumeration ? 'yes' : 'no';
} else if ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_group') {
return $shareeEnumerationLimitToGroup ? 'yes' : 'no';
}
return $default;
}
);
}
public function getUserMock($uid, $displayName, $enabled = true, $groups = []) {
$user = $this->createMock(IUser::class); $user = $this->createMock(IUser::class);
$user->expects($this->any()) $user->expects($this->any())
@ -383,21 +403,7 @@ class UserPluginTest extends TestCase {
$reachedEnd, $reachedEnd,
$singleUser $singleUser
) { ) {
$this->config->expects($this->any()) $this->mockConfig($shareWithGroupOnly, $shareeEnumeration, false);
->method('getAppValue')
->willReturnCallback(
function($appName, $key, $default)
use ($shareWithGroupOnly, $shareeEnumeration)
{
if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') {
return $shareWithGroupOnly ? 'yes' : 'no';
} else if ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
return $shareeEnumeration ? 'yes' : 'no';
}
return $default;
}
);
$this->instantiatePlugin(); $this->instantiatePlugin();
$this->session->expects($this->any()) $this->session->expects($this->any())
@ -493,4 +499,110 @@ class UserPluginTest extends TestCase {
$this->plugin->takeOutCurrentUser($users); $this->plugin->takeOutCurrentUser($users);
$this->assertSame($expectedUIDs, array_keys($users)); $this->assertSame($expectedUIDs, array_keys($users));
} }
public function dataSearchEnumeration() {
return [
[
'test',
['groupA'],
[
[ 'uid' => 'test1', 'groups' => ['groupA'] ],
[ 'uid' => 'test2', 'groups' => ['groupB'] ]
],
['test1']
],
[
'test',
['groupA'],
[
[ 'uid' => 'test1', 'groups' => ['groupA'] ],
[ 'uid' => 'test2', 'groups' => ['groupB', 'groupA'] ]
],
['test1', 'test2']
],
[
'test',
['groupA'],
[
[ 'uid' => 'test1', 'groups' => ['groupA', 'groupC'] ],
[ 'uid' => 'test2', 'groups' => ['groupB', 'groupA'] ]
],
['test1', 'test2']
],
[
'test',
['groupC', 'groupB'],
[
[ 'uid' => 'test1', 'groups' => ['groupA', 'groupC'] ],
[ 'uid' => 'test2', 'groups' => ['groupB', 'groupA'] ]
],
['test1', 'test2']
],
[
'test',
[],
[
[ 'uid' => 'test1', 'groups' => ['groupA'] ],
[ 'uid' => 'test2', 'groups' => ['groupB', 'groupA'] ]
],
[]
],
[
'test',
['groupC', 'groupB'],
[
[ 'uid' => 'test1', 'groups' => [] ],
[ 'uid' => 'test2', 'groups' => [] ]
],
[]
],
];
}
/**
* @dataProvider dataSearchEnumeration
*/
public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers, $result) {
$this->mockConfig(false, true, true);
$userResults = array_map(function ($user) {
return $this->getUserMock($user['uid'], $user['uid']);
}, $matchingUsers);
$mappedResult = array_map(function ($user) {
return ['label' => $user, 'value' => [ 'shareType' => 0, 'shareWith' => $user ]];
}, $result);
$this->userManager->expects($this->once())
->method('searchDisplayName')
->willReturn($userResults);
$this->session->expects($this->any())
->method('getUser')
->willReturn($this->getUserMock('test', 'foo'));
// current user
$this->groupManager->expects($this->at(0))
->method('getUserGroupIds')
->willReturn($userGroups);
$this->groupManager->expects($this->any())
->method('getUserGroupIds')
->willReturnCallback(function ($user) use ($matchingUsers) {
$neededObject = array_filter(
$matchingUsers,
function ($e) use ($user) {
return $user->getUID() === $e['uid'];
}
);
if (count($neededObject) > 0) {
return array_shift($neededObject)['groups'];
}
return [];
});
$this->instantiatePlugin();
$this->plugin->search($search, $this->limit, $this->offset, $this->searchResult);
$result = $this->searchResult->asArray();
$this->assertEquals([], $result['exact']['users']);
$this->assertEquals($mappedResult, $result['users']);
}
} }

View File

@ -177,16 +177,21 @@ class ContactsStoreTest extends TestCase {
->willReturn('yes'); ->willReturn('yes');
$this->config->expects($this->at(1)) $this->config->expects($this->at(1))
->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('yes'))
->willReturn('no');
$this->config->expects($this->at(2))
->method('getAppValue') ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no'))
->willReturn('yes'); ->willReturn('yes');
$this->config->expects($this->at(2)) $this->config->expects($this->at(3))
->method('getAppValue') ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no')) ->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no'))
->willReturn('yes'); ->willReturn('yes');
$this->config->expects($this->at(3)) $this->config->expects($this->at(4))
->method('getAppValue') ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups_list'), $this->equalTo('')) ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups_list'), $this->equalTo(''))
->willReturn('["group1", "group5", "group6"]'); ->willReturn('["group1", "group5", "group6"]');
@ -223,16 +228,20 @@ class ContactsStoreTest extends TestCase {
$this->assertCount(0, $entries); $this->assertCount(0, $entries);
} }
public function testGetContactsOnlyIfInTheSameGroup() { public function testGetContactsOnlyShareIfInTheSameGroup() {
$this->config->expects($this->at(0))->method('getAppValue') $this->config->expects($this->at(0))->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes'))
->willReturn('yes'); ->willReturn('yes');
$this->config->expects($this->at(1)) ->method('getAppValue') $this->config->expects($this->at(1)) ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('yes'))
->willReturn('no');
$this->config->expects($this->at(2)) ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no'))
->willReturn('no'); ->willReturn('no');
$this->config->expects($this->at(2)) $this->config->expects($this->at(3))
->method('getAppValue') ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no')) ->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no'))
->willReturn('yes'); ->willReturn('yes');
@ -305,6 +314,92 @@ class ContactsStoreTest extends TestCase {
$this->assertEquals('contact', $entries[2]->getProperty('UID')); $this->assertEquals('contact', $entries[2]->getProperty('UID'));
} }
public function testGetContactsOnlyEnumerateIfInTheSameGroup() {
$this->config->expects($this->at(0))->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes'))
->willReturn('yes');
$this->config->expects($this->at(1)) ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('yes'))
->willReturn('yes');
$this->config->expects($this->at(2)) ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no'))
->willReturn('no');
$this->config->expects($this->at(3))
->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no'))
->willReturn('no');
/** @var IUser|PHPUnit_Framework_MockObject_MockObject $currentUser */
$currentUser = $this->createMock(IUser::class);
$currentUser->expects($this->once())
->method('getUID')
->willReturn('user001');
$this->groupManager->expects($this->at(0))
->method('getUserGroupIds')
->with($this->equalTo($currentUser))
->willReturn(['group1', 'group2', 'group3']);
$user1 = $this->createMock(IUser::class);
$this->userManager->expects($this->at(0))
->method('get')
->with('user1')
->willReturn($user1);
$this->groupManager->expects($this->at(1))
->method('getUserGroupIds')
->with($this->equalTo($user1))
->willReturn(['group1']);
$user2 = $this->createMock(IUser::class);
$this->userManager->expects($this->at(1))
->method('get')
->with('user2')
->willReturn($user2);
$this->groupManager->expects($this->at(2))
->method('getUserGroupIds')
->with($this->equalTo($user2))
->willReturn(['group2', 'group3']);
$user3 = $this->createMock(IUser::class);
$this->userManager->expects($this->at(2))
->method('get')
->with('user3')
->willReturn($user3);
$this->groupManager->expects($this->at(3))
->method('getUserGroupIds')
->with($this->equalTo($user3))
->willReturn(['group8', 'group9']);
$this->contactsManager->expects($this->once())
->method('search')
->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
->willReturn([
[
'UID' => 'user1',
'isLocalSystemBook' => true
],
[
'UID' => 'user2',
'isLocalSystemBook' => true
],
[
'UID' => 'user3',
'isLocalSystemBook' => true
],
[
'UID' => 'contact',
],
]);
$entries = $this->contactsStore->getContacts($currentUser, '');
$this->assertCount(3, $entries);
$this->assertEquals('user1', $entries[0]->getProperty('UID'));
$this->assertEquals('user2', $entries[1]->getProperty('UID'));
$this->assertEquals('contact', $entries[2]->getProperty('UID'));
}
public function testGetContactsWithFilter() { public function testGetContactsWithFilter() {
$this->config->expects($this->at(0))->method('getAppValue') $this->config->expects($this->at(0))->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes'))