From 7aec7f6685db1ddeaa2574f2f83af6b2f7cee9e9 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Tue, 26 Nov 2019 16:37:57 +0100 Subject: [PATCH] respect shareapi_allow_share_dialog_user_enumeration in Principal backend for Sabre/DAV Signed-off-by: Georg Ehrke --- apps/dav/appinfo/v1/caldav.php | 1 + apps/dav/appinfo/v1/carddav.php | 1 + apps/dav/lib/Command/CreateCalendar.php | 3 +- apps/dav/lib/Connector/Sabre/Principal.php | 21 ++++ apps/dav/lib/RootCollection.php | 5 +- .../unit/CalDAV/AbstractCalDavBackend.php | 1 + .../tests/unit/CardDAV/CardDavBackendTest.php | 3 +- .../unit/Connector/Sabre/PrincipalTest.php | 96 +++++++++++++++++-- .../lib/AppInfo/Application.php | 3 +- .../lib/AppInfo/Application.php | 3 +- 10 files changed, 125 insertions(+), 12 deletions(-) diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index 8116453b11d49..f1086cc62b0e5 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -48,6 +48,7 @@ \OC::$server->getUserSession(), \OC::$server->getAppManager(), \OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class), + \OC::$server->getConfig(), 'principals/' ); $db = \OC::$server->getDatabaseConnection(); diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index 40ee12f1944ea..8c6b6fb2016e9 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -49,6 +49,7 @@ \OC::$server->getUserSession(), \OC::$server->getAppManager(), \OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class), + \OC::$server->getConfig(), 'principals/' ); $db = \OC::$server->getDatabaseConnection(); diff --git a/apps/dav/lib/Command/CreateCalendar.php b/apps/dav/lib/Command/CreateCalendar.php index a40bf48cc8e2c..9d565226bfb9a 100644 --- a/apps/dav/lib/Command/CreateCalendar.php +++ b/apps/dav/lib/Command/CreateCalendar.php @@ -80,7 +80,8 @@ protected function execute(InputInterface $input, OutputInterface $output) { \OC::$server->getShareManager(), \OC::$server->getUserSession(), \OC::$server->getAppManager(), - \OC::$server->query(ProxyMapper::class) + \OC::$server->query(ProxyMapper::class), + \OC::$server->getConfig() ); $random = \OC::$server->getSecureRandom(); $logger = \OC::$server->getLogger(); diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index 5c61b8371f2fe..880f082ec4208 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -40,6 +40,7 @@ use OCA\DAV\Traits\PrincipalProxyTrait; use OCP\App\IAppManager; use OCP\AppFramework\QueryException; +use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; @@ -79,6 +80,9 @@ class Principal implements BackendInterface { /** @var ProxyMapper */ private $proxyMapper; + /** @var IConfig */ + private $config; + /** * Principal constructor. * @@ -88,6 +92,7 @@ class Principal implements BackendInterface { * @param IUserSession $userSession * @param IAppManager $appManager * @param ProxyMapper $proxyMapper + * @param IConfig $config * @param string $principalPrefix */ public function __construct(IUserManager $userManager, @@ -96,6 +101,7 @@ public function __construct(IUserManager $userManager, IUserSession $userSession, IAppManager $appManager, ProxyMapper $proxyMapper, + IConfig $config, string $principalPrefix = 'principals/users/') { $this->userManager = $userManager; $this->groupManager = $groupManager; @@ -105,6 +111,7 @@ public function __construct(IUserManager $userManager, $this->principalPrefix = trim($principalPrefix, '/'); $this->hasGroups = $this->hasCircles = ($principalPrefix === 'principals/users/'); $this->proxyMapper = $proxyMapper; + $this->config = $config; } use PrincipalProxyTrait { @@ -240,6 +247,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; @@ -257,6 +266,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) { @@ -274,6 +289,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) { diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php index ed8297783d793..6e1d932eaccf2 100644 --- a/apps/dav/lib/RootCollection.php +++ b/apps/dav/lib/RootCollection.php @@ -62,9 +62,10 @@ public function __construct() { $shareManager, \OC::$server->getUserSession(), \OC::$server->getAppManager(), - $proxyMapper + $proxyMapper, + \OC::$server->getConfig() ); - $groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager, $l10n); + $groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager); $calendarResourcePrincipalBackend = new ResourcePrincipalBackend($db, $userSession, $groupManager, $logger, $proxyMapper); $calendarRoomPrincipalBackend = new RoomPrincipalBackend($db, $userSession, $groupManager, $logger, $proxyMapper); // as soon as debug mode is enabled we allow listing of principals diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php index 9f9a7c01337c8..6e74997d25e50 100644 --- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php +++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php @@ -86,6 +86,7 @@ public function setUp() { $this->createMock(IUserSession::class), $this->createMock(IAppManager::class), $this->createMock(ProxyMapper::class), + $this->createMock(IConfig::class), ]) ->setMethods(['getPrincipalByPath', 'getGroupMembership']) ->getMock(); diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php index aa18ef63a5cda..e9b8cebe2ae48 100644 --- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php +++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php @@ -134,6 +134,7 @@ public function setUp() { $this->createMock(IUserSession::class), $this->createMock(IAppManager::class), $this->createMock(ProxyMapper::class), + $this->createMock(IConfig::class), ]) ->setMethods(['getPrincipalByPath', 'getGroupMembership']) ->getMock(); @@ -396,7 +397,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); diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index 2a5c122ce9ea7..01dca6ac75781 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -65,6 +65,9 @@ class PrincipalTest extends TestCase { /** @var ProxyMapper | \PHPUnit_Framework_MockObject_MockObject */ private $proxyMapper; + /** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */ + private $config; + public function setUp() { $this->userManager = $this->createMock(IUserManager::class); $this->groupManager = $this->createMock(IGroupManager::class); @@ -72,6 +75,7 @@ public function setUp() { $this->userSession = $this->createMock(IUserSession::class); $this->appManager = $this->createMock(IAppManager::class); $this->proxyMapper = $this->createMock(ProxyMapper::class); + $this->config = $this->createMock(IConfig::class); $this->connector = new \OCA\DAV\Connector\Sabre\Principal( $this->userManager, @@ -79,7 +83,8 @@ public function setUp() { $this->shareManager, $this->userSession, $this->appManager, - $this->proxyMapper + $this->proxyMapper, + $this->config ); parent::setUp(); } @@ -403,11 +408,6 @@ public function testSetGroupMembershipProxy() { $this->connector->setGroupMemberSet('principals/users/foo/calendar-proxy-write', ['principals/users/bar']); } - - - - - public function testUpdatePrincipal() { $this->assertSame(0, $this->connector->updatePrincipal('foo', new PropPatch(array()))); } @@ -430,6 +430,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)); @@ -446,6 +451,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()) @@ -518,6 +525,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)); @@ -539,6 +551,78 @@ public function testSearchPrincipalByCalendarUserAddressSet() { ['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => 'user@example.com'])); } + 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('user2@foo.bar')); + $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('user2@foo.bar123')); + $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('user2@foo.bar456')); + + $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('user2@foo.bar')); + $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('user2@foo.bar123')); + $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('user2@foo.bar456')); + + $this->userManager->expects($this->at(0)) + ->method('getByEmail') + ->with('user2@foo.bar') + ->will($this->returnValue([$user2, $user3, $user4])); + + $this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users', + ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); + } + public function testFindByUriSharingApiDisabled() { $this->shareManager->expects($this->once()) ->method('shareApiEnabled') diff --git a/apps/files_trashbin/lib/AppInfo/Application.php b/apps/files_trashbin/lib/AppInfo/Application.php index 4baa82a6b4b55..e90fbb584e568 100644 --- a/apps/files_trashbin/lib/AppInfo/Application.php +++ b/apps/files_trashbin/lib/AppInfo/Application.php @@ -63,7 +63,8 @@ public function __construct (array $urlParams = []) { \OC::$server->getShareManager(), \OC::$server->getUserSession(), \OC::$server->getAppManager(), - \OC::$server->query(ProxyMapper::class) + \OC::$server->query(ProxyMapper::class), + \OC::$server->getConfig() ); }); diff --git a/apps/files_versions/lib/AppInfo/Application.php b/apps/files_versions/lib/AppInfo/Application.php index 919a9b42d7979..233a9813b57ef 100644 --- a/apps/files_versions/lib/AppInfo/Application.php +++ b/apps/files_versions/lib/AppInfo/Application.php @@ -53,7 +53,8 @@ public function __construct(array $urlParams = array()) { $server->getShareManager(), $server->getUserSession(), $server->getAppManager(), - $server->query(ProxyMapper::class) + $server->query(ProxyMapper::class), + \OC::$server->getConfig() ); });