Skip to content

Commit 9cd7ece

Browse files
skjnldsvskjnldsv
authored
Merge pull request #18120 from nextcloud/bugfix/11087/dav_principal_backend_respect_sharing_settings
respect shareapi_allow_share_dialog_user_enumeration in Principal backend for Sabre/DAV
2 parents 2b19da8 + c3748cf commit 9cd7ece

File tree

10 files changed

+128
-15
lines changed

10 files changed

+128
-15
lines changed

‎apps/dav/appinfo/v1/caldav.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
\OC::$server->getUserSession(),
4949
\OC::$server->getAppManager(),
5050
\OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class),
51+
\OC::$server->getConfig(),
5152
'principals/'
5253
);
5354
$db = \OC::$server->getDatabaseConnection();

‎apps/dav/appinfo/v1/carddav.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
\OC::$server->getUserSession(),
5050
\OC::$server->getAppManager(),
5151
\OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class),
52+
\OC::$server->getConfig(),
5253
'principals/'
5354
);
5455
$db = \OC::$server->getDatabaseConnection();

‎apps/dav/lib/Command/CreateCalendar.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ protected function execute(InputInterface $input, OutputInterface $output) {
8181
\OC::$server->getShareManager(),
8282
\OC::$server->getUserSession(),
8383
\OC::$server->getAppManager(),
84-
\OC::$server->query(ProxyMapper::class)
84+
\OC::$server->query(ProxyMapper::class),
85+
\OC::$server->getConfig()
8586
);
8687
$random = \OC::$server->getSecureRandom();
8788
$logger = \OC::$server->getLogger();

‎apps/dav/lib/Connector/Sabre/Principal.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
use OCA\DAV\Traits\PrincipalProxyTrait;
4141
use OCP\App\IAppManager;
4242
use OCP\AppFramework\QueryException;
43+
use OCP\IConfig;
4344
use OCP\IGroup;
4445
use OCP\IGroupManager;
4546
use OCP\IUser;
@@ -79,6 +80,9 @@ class Principal implements BackendInterface {
7980
/** @var ProxyMapper */
8081
private $proxyMapper;
8182

83+
/** @var IConfig */
84+
private $config;
85+
8286
/**
8387
* Principal constructor.
8488
*
@@ -88,6 +92,7 @@ class Principal implements BackendInterface {
8892
* @param IUserSession $userSession
8993
* @param IAppManager $appManager
9094
* @param ProxyMapper $proxyMapper
95+
* @param IConfig $config
9196
* @param string $principalPrefix
9297
*/
9398
public function __construct(IUserManager $userManager,
@@ -96,6 +101,7 @@ public function __construct(IUserManager $userManager,
96101
IUserSession $userSession,
97102
IAppManager $appManager,
98103
ProxyMapper $proxyMapper,
104+
IConfig $config,
99105
string $principalPrefix = 'principals/users/') {
100106
$this->userManager = $userManager;
101107
$this->groupManager = $groupManager;
@@ -105,6 +111,7 @@ public function __construct(IUserManager $userManager,
105111
$this->principalPrefix = trim($principalPrefix, '/');
106112
$this->hasGroups = $this->hasCircles = ($principalPrefix === 'principals/users/');
107113
$this->proxyMapper = $proxyMapper;
114+
$this->config = $config;
108115
}
109116

110117
use PrincipalProxyTrait {
@@ -240,6 +247,8 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
240247
return [];
241248
}
242249

250+
$allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
251+
243252
// If sharing is restricted to group members only,
244253
// return only members that have groups in common
245254
$restrictGroups = false;
@@ -257,6 +266,12 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
257266
case '{http://sabredav.org/ns}email-address':
258267
$users = $this->userManager->getByEmail($value);
259268

269+
if (!$allowEnumeration) {
270+
$users = \array_filter($users, static function(IUser $user) use ($value) {
271+
return $user->getEMailAddress() === $value;
272+
});
273+
}
274+
260275
$results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) {
261276
// is sharing restricted to groups only?
262277
if ($restrictGroups !== false) {
@@ -274,6 +289,12 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
274289
case '{DAV:}displayname':
275290
$users = $this->userManager->searchDisplayName($value);
276291

292+
if (!$allowEnumeration) {
293+
$users = \array_filter($users, static function(IUser $user) use ($value) {
294+
return $user->getDisplayName() === $value;
295+
});
296+
}
297+
277298
$results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) {
278299
// is sharing restricted to groups only?
279300
if ($restrictGroups !== false) {

‎apps/dav/lib/RootCollection.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,10 @@ public function __construct() {
6363
$shareManager,
6464
\OC::$server->getUserSession(),
6565
\OC::$server->getAppManager(),
66-
$proxyMapper
66+
$proxyMapper,
67+
\OC::$server->getConfig()
6768
);
68-
$groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager, $l10n);
69+
$groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager);
6970
$calendarResourcePrincipalBackend = new ResourcePrincipalBackend($db, $userSession, $groupManager, $logger, $proxyMapper);
7071
$calendarRoomPrincipalBackend = new RoomPrincipalBackend($db, $userSession, $groupManager, $logger, $proxyMapper);
7172
// as soon as debug mode is enabled we allow listing of principals

‎apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ protected function setUp(): void {
8686
$this->createMock(IUserSession::class),
8787
$this->createMock(IAppManager::class),
8888
$this->createMock(ProxyMapper::class),
89+
$this->createMock(IConfig::class),
8990
])
9091
->setMethods(['getPrincipalByPath', 'getGroupMembership'])
9192
->getMock();

‎apps/dav/tests/unit/CardDAV/CardDavBackendTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ protected function setUp(): void {
134134
$this->createMock(IUserSession::class),
135135
$this->createMock(IAppManager::class),
136136
$this->createMock(ProxyMapper::class),
137+
$this->createMock(IConfig::class),
137138
])
138139
->setMethods(['getPrincipalByPath', 'getGroupMembership'])
139140
->getMock();
@@ -396,7 +397,7 @@ public function testMultipleUIDDenied() {
396397
// create a card
397398
$uri0 = $this->getUniqueID('card');
398399
$this->backend->createCard($bookId, $uri0, $this->vcardTest0);
399-
400+
400401
// create another card with same uid
401402
$uri1 = $this->getUniqueID('card');
402403
$this->expectException(BadRequest::class);

‎apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php‎

Lines changed: 93 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,26 @@ class PrincipalTest extends TestCase {
6565
/** @var ProxyMapper | \PHPUnit_Framework_MockObject_MockObject */
6666
private $proxyMapper;
6767

68+
/** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */
69+
private $config;
70+
6871
protected function setUp(): void {
6972
$this->userManager = $this->createMock(IUserManager::class);
7073
$this->groupManager = $this->createMock(IGroupManager::class);
7174
$this->shareManager = $this->createMock(IManager::class);
7275
$this->userSession = $this->createMock(IUserSession::class);
7376
$this->appManager = $this->createMock(IAppManager::class);
7477
$this->proxyMapper = $this->createMock(ProxyMapper::class);
78+
$this->config = $this->createMock(IConfig::class);
7579

7680
$this->connector = new \OCA\DAV\Connector\Sabre\Principal(
7781
$this->userManager,
7882
$this->groupManager,
7983
$this->shareManager,
8084
$this->userSession,
8185
$this->appManager,
82-
$this->proxyMapper
86+
$this->proxyMapper,
87+
$this->config
8388
);
8489
parent::setUp();
8590
}
@@ -209,7 +214,7 @@ public function testGetGroupMemberSet() {
209214
$this->assertSame([], $response);
210215
}
211216

212-
217+
213218
public function testGetGroupMemberSetEmpty() {
214219
$this->expectException(\Sabre\DAV\Exception::class);
215220
$this->expectExceptionMessage('Principal not found');
@@ -334,7 +339,7 @@ public function testGetGroupMembership() {
334339
$this->assertSame($expectedResponse, $response);
335340
}
336341

337-
342+
338343
public function testGetGroupMembershipEmpty() {
339344
$this->expectException(\Sabre\DAV\Exception::class);
340345
$this->expectExceptionMessage('Principal not found');
@@ -348,7 +353,7 @@ public function testGetGroupMembershipEmpty() {
348353
$this->connector->getGroupMembership('principals/users/foo');
349354
}
350355

351-
356+
352357
public function testSetGroupMembership() {
353358
$this->expectException(\Sabre\DAV\Exception::class);
354359
$this->expectExceptionMessage('Setting members of the group is not supported yet');
@@ -403,11 +408,6 @@ public function testSetGroupMembershipProxy() {
403408
$this->connector->setGroupMemberSet('principals/users/foo/calendar-proxy-write', ['principals/users/bar']);
404409
}
405410

406-
407-
408-
409-
410-
411411
public function testUpdatePrincipal() {
412412
$this->assertSame(0, $this->connector->updatePrincipal('foo', new PropPatch(array())));
413413
}
@@ -430,6 +430,11 @@ public function testSearchPrincipals($sharingEnabled, $groupsOnly, $test, $resul
430430
->will($this->returnValue($sharingEnabled));
431431

432432
if ($sharingEnabled) {
433+
$this->config->expects($this->once())
434+
->method('getAppValue')
435+
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
436+
->willReturn('yes');
437+
433438
$this->shareManager->expects($this->once())
434439
->method('shareWithGroupMembersOnly')
435440
->will($this->returnValue($groupsOnly));
@@ -446,6 +451,8 @@ public function testSearchPrincipals($sharingEnabled, $groupsOnly, $test, $resul
446451
->will($this->returnValue(['group1', 'group2', 'group5']));
447452
}
448453
} else {
454+
$this->config->expects($this->never())
455+
->method('getAppValue');
449456
$this->shareManager->expects($this->never())
450457
->method('shareWithGroupMembersOnly');
451458
$this->groupManager->expects($this->never())
@@ -518,6 +525,11 @@ public function testSearchPrincipalByCalendarUserAddressSet() {
518525
->method('shareAPIEnabled')
519526
->will($this->returnValue(true));
520527

528+
$this->config->expects($this->exactly(2))
529+
->method('getAppValue')
530+
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
531+
->willReturn('yes');
532+
521533
$this->shareManager->expects($this->exactly(2))
522534
->method('shareWithGroupMembersOnly')
523535
->will($this->returnValue(false));
@@ -539,6 +551,78 @@ public function testSearchPrincipalByCalendarUserAddressSet() {
539551
['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => 'user@example.com']));
540552
}
541553

554+
public function testSearchPrincipalWithEnumerationDisabledDisplayname() {
555+
$this->shareManager->expects($this->once())
556+
->method('shareAPIEnabled')
557+
->will($this->returnValue(true));
558+
559+
$this->config->expects($this->once())
560+
->method('getAppValue')
561+
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
562+
->willReturn('no');
563+
564+
$this->shareManager->expects($this->once())
565+
->method('shareWithGroupMembersOnly')
566+
->will($this->returnValue(false));
567+
568+
$user2 = $this->createMock(IUser::class);
569+
$user2->method('getUID')->will($this->returnValue('user2'));
570+
$user2->method('getDisplayName')->will($this->returnValue('User 2'));
571+
$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar'));
572+
$user3 = $this->createMock(IUser::class);
573+
$user3->method('getUID')->will($this->returnValue('user3'));
574+
$user2->method('getDisplayName')->will($this->returnValue('User 22'));
575+
$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123'));
576+
$user4 = $this->createMock(IUser::class);
577+
$user4->method('getUID')->will($this->returnValue('user4'));
578+
$user2->method('getDisplayName')->will($this->returnValue('User 222'));
579+
$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456'));
580+
581+
$this->userManager->expects($this->at(0))
582+
->method('searchDisplayName')
583+
->with('User 2')
584+
->will($this->returnValue([$user2, $user3, $user4]));
585+
586+
$this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users',
587+
['{DAV:}displayname' => 'User 2']));
588+
}
589+
590+
public function testSearchPrincipalWithEnumerationDisabledEmail() {
591+
$this->shareManager->expects($this->once())
592+
->method('shareAPIEnabled')
593+
->will($this->returnValue(true));
594+
595+
$this->config->expects($this->once())
596+
->method('getAppValue')
597+
->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
598+
->willReturn('no');
599+
600+
$this->shareManager->expects($this->once())
601+
->method('shareWithGroupMembersOnly')
602+
->will($this->returnValue(false));
603+
604+
$user2 = $this->createMock(IUser::class);
605+
$user2->method('getUID')->will($this->returnValue('user2'));
606+
$user2->method('getDisplayName')->will($this->returnValue('User 2'));
607+
$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar'));
608+
$user3 = $this->createMock(IUser::class);
609+
$user3->method('getUID')->will($this->returnValue('user3'));
610+
$user2->method('getDisplayName')->will($this->returnValue('User 22'));
611+
$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123'));
612+
$user4 = $this->createMock(IUser::class);
613+
$user4->method('getUID')->will($this->returnValue('user4'));
614+
$user2->method('getDisplayName')->will($this->returnValue('User 222'));
615+
$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456'));
616+
617+
$this->userManager->expects($this->at(0))
618+
->method('getByEmail')
619+
->with('user2@foo.bar')
620+
->will($this->returnValue([$user2, $user3, $user4]));
621+
622+
$this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users',
623+
['{http://sabredav.org/ns}email-address' => 'user2@foo.bar']));
624+
}
625+
542626
public function testFindByUriSharingApiDisabled() {
543627
$this->shareManager->expects($this->once())
544628
->method('shareApiEnabled')

‎apps/files_trashbin/lib/AppInfo/Application.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ public function __construct (array $urlParams = []) {
5858
\OC::$server->getShareManager(),
5959
\OC::$server->getUserSession(),
6060
\OC::$server->getAppManager(),
61-
\OC::$server->query(ProxyMapper::class)
61+
\OC::$server->query(ProxyMapper::class),
62+
\OC::$server->getConfig()
6263
);
6364
});
6465

‎apps/files_versions/lib/AppInfo/Application.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ public function __construct(array $urlParams = []) {
6666
$server->getShareManager(),
6767
$server->getUserSession(),
6868
$server->getAppManager(),
69-
$server->query(ProxyMapper::class)
69+
$server->query(ProxyMapper::class),
70+
\OC::$server->getConfig()
7071
);
7172
});
7273

0 commit comments

Comments
 (0)