Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable16] respect shareapi_allow_share_dialog_user_enumeration in Principal bac… #18230

Merged
merged 1 commit into from
Mar 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions apps/dav/lib/Connector/Sabre/Principal.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class Principal implements BackendInterface {
* @param IGroupManager $groupManager
* @param IShareManager $shareManager
* @param IUserSession $userSession
* @param IAppManager $appManager
* @param IConfig $config
* @param string $principalPrefix
*/
Expand Down Expand Up @@ -239,6 +240,8 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
return [];
}

$allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';

// If sharing is restricted to group members only,
// return only members that have groups in common
$restrictGroups = false;
Expand All @@ -256,6 +259,12 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
case '{http://sabredav.org/ns}email-address':
$users = $this->userManager->getByEmail($value);

if (!$allowEnumeration) {
$users = \array_filter($users, static function(IUser $user) use ($value) {
return $user->getEMailAddress() === $value;
});
}

$results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) {
// is sharing restricted to groups only?
if ($restrictGroups !== false) {
Expand All @@ -273,6 +282,12 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
case '{DAV:}displayname':
$users = $this->userManager->searchDisplayName($value);

if (!$allowEnumeration) {
$users = \array_filter($users, static function(IUser $user) use ($value) {
return $user->getDisplayName() === $value;
});
}

$results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) {
// is sharing restricted to groups only?
if ($restrictGroups !== false) {
Expand Down
2 changes: 1 addition & 1 deletion apps/dav/lib/RootCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function __construct() {
$config,
\OC::$server->getAppManager()
);
$groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager, $l10n);
$groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager);
$calendarResourcePrincipalBackend = new ResourcePrincipalBackend($db, $userSession, $groupManager, $logger);
$calendarRoomPrincipalBackend = new RoomPrincipalBackend($db, $userSession, $groupManager, $logger);
// as soon as debug mode is enabled we allow listing of principals
Expand Down
2 changes: 1 addition & 1 deletion apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ public function testMultipleUIDDenied() {
// create a card
$uri0 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri0, $this->vcardTest0);

// create another card with same uid
$uri1 = $this->getUniqueID('card');
$this->expectException(BadRequest::class);
Expand Down
84 changes: 84 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,11 @@ public function testSearchPrincipals($sharingEnabled, $groupsOnly, $test, $resul
->will($this->returnValue($sharingEnabled));

if ($sharingEnabled) {
$this->config->expects($this->once())
->method('getAppValue')
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
->willReturn('yes');

$this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly')
->will($this->returnValue($groupsOnly));
Expand All @@ -324,6 +329,8 @@ public function testSearchPrincipals($sharingEnabled, $groupsOnly, $test, $resul
->will($this->returnValue(['group1', 'group2', 'group5']));
}
} else {
$this->config->expects($this->never())
->method('getAppValue');
$this->shareManager->expects($this->never())
->method('shareWithGroupMembersOnly');
$this->groupManager->expects($this->never())
Expand Down Expand Up @@ -396,6 +403,11 @@ public function testSearchPrincipalByCalendarUserAddressSet() {
->method('shareAPIEnabled')
->will($this->returnValue(true));

$this->config->expects($this->exactly(2))
->method('getAppValue')
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
->willReturn('yes');

$this->shareManager->expects($this->exactly(2))
->method('shareWithGroupMembersOnly')
->will($this->returnValue(false));
Expand All @@ -417,6 +429,78 @@ public function testSearchPrincipalByCalendarUserAddressSet() {
['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => '[email protected]']));
}

public function testSearchPrincipalWithEnumerationDisabledDisplayname() {
$this->shareManager->expects($this->once())
->method('shareAPIEnabled')
->will($this->returnValue(true));

$this->config->expects($this->once())
->method('getAppValue')
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
->willReturn('no');

$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('[email protected]'));
$user3 = $this->createMock(IUser::class);
$user3->method('getUID')->will($this->returnValue('user3'));
$user2->method('getDisplayName')->will($this->returnValue('User 22'));
$user2->method('getEMailAddress')->will($this->returnValue('[email protected]'));
$user4 = $this->createMock(IUser::class);
$user4->method('getUID')->will($this->returnValue('user4'));
$user2->method('getDisplayName')->will($this->returnValue('User 222'));
$user2->method('getEMailAddress')->will($this->returnValue('[email protected]'));

$this->userManager->expects($this->at(0))
->method('searchDisplayName')
->with('User 2')
->will($this->returnValue([$user2, $user3, $user4]));

$this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users',
['{DAV:}displayname' => 'User 2']));
}

public function testSearchPrincipalWithEnumerationDisabledEmail() {
$this->shareManager->expects($this->once())
->method('shareAPIEnabled')
->will($this->returnValue(true));

$this->config->expects($this->once())
->method('getAppValue')
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
->willReturn('no');

$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('[email protected]'));
$user3 = $this->createMock(IUser::class);
$user3->method('getUID')->will($this->returnValue('user3'));
$user2->method('getDisplayName')->will($this->returnValue('User 22'));
$user2->method('getEMailAddress')->will($this->returnValue('[email protected]'));
$user4 = $this->createMock(IUser::class);
$user4->method('getUID')->will($this->returnValue('user4'));
$user2->method('getDisplayName')->will($this->returnValue('User 222'));
$user2->method('getEMailAddress')->will($this->returnValue('[email protected]'));

$this->userManager->expects($this->at(0))
->method('getByEmail')
->with('[email protected]')
->will($this->returnValue([$user2, $user3, $user4]));

$this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users',
['{http://sabredav.org/ns}email-address' => '[email protected]']));
}

public function testFindByUriSharingApiDisabled() {
$this->shareManager->expects($this->once())
->method('shareApiEnabled')
Expand Down