respect shareapi_allow_share_dialog_user_enumeration in Principal backend for Sabre/DAV

Signed-off-by: Georg Ehrke <developer@georgehrke.com>

diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php
index 8116453b11d49a60e1f6dde4438019ade09b75c4..f1086cc62b0e5b3d49f1c1b96363df0a14c1cf17 100644 (file)
--- a/apps/dav/appinfo/v1/caldav.php
+++ b/apps/dav/appinfo/v1/caldav.php
@@ -48,6 +48,7 @@ $principalBackend = new Principal(
        \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 40ee12f1944eab56bc194589d804be6d8e0386e5..8c6b6fb2016e9af2b9e57e7a3186053652c409f0 100644 (file)
--- a/apps/dav/appinfo/v1/carddav.php
+++ b/apps/dav/appinfo/v1/carddav.php
@@ -49,6 +49,7 @@ $principalBackend = new Principal(
        \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 c6bfffb2425838fc391a9e7df49a037df5a53dc8..1efe420236892c75077d0b6311ddc75403f1b18d 100644 (file)
--- a/apps/dav/lib/Command/CreateCalendar.php
+++ b/apps/dav/lib/Command/CreateCalendar.php
@@ -81,7 +81,8 @@ class CreateCalendar extends Command {
                        \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 5c61b8371f2fe0afd28a73e6f84e9231a661ceb5..880f082ec42085b92c2bbc910c309be67888e701 100644 (file)
--- a/apps/dav/lib/Connector/Sabre/Principal.php
+++ b/apps/dav/lib/Connector/Sabre/Principal.php
@@ -40,6 +40,7 @@ use OCA\DAV\CalDAV\Proxy\ProxyMapper;
 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 @@ class Principal implements BackendInterface {
                                                                IUserSession $userSession,
                                                                IAppManager $appManager,
                                                                ProxyMapper $proxyMapper,
+                                                               IConfig $config,
                                                                string $principalPrefix = 'principals/users/') {
                $this->userManager = $userManager;
                $this->groupManager = $groupManager;
@@ -105,6 +111,7 @@ class Principal implements BackendInterface {
                $this->principalPrefix = trim($principalPrefix, '/');
                $this->hasGroups = $this->hasCircles = ($principalPrefix === 'principals/users/');
                $this->proxyMapper = $proxyMapper;
+               $this->config = $config;
        }
 
        use PrincipalProxyTrait {
@@ -240,6 +247,8 @@ class Principal implements BackendInterface {
                        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 @@ class Principal implements BackendInterface {
                                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 @@ class Principal implements BackendInterface {
                                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 8c66609ed66bfd3e731f279e50a61c7f4c8306aa..6458f2e1dc2712cddfaf68dc75008a0a766b0871 100644 (file)
--- a/apps/dav/lib/RootCollection.php
+++ b/apps/dav/lib/RootCollection.php
@@ -63,9 +63,10 @@ class RootCollection extends SimpleCollection {
                        $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 8772f511486160ccc72f62f93e080e3a2045374e..9449ab27b00e972d62dc52d8febc04bfb9cfe921 100644 (file)
--- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php
+++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php
@@ -86,6 +86,7 @@ abstract class AbstractCalDavBackend extends TestCase {
                                $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 f2a85cb9dbb249738d9cdd57759562b0749d6fa3..d00d03d8944653318bb5bd252d52da03078c8021 100644 (file)
--- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
+++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
@@ -134,6 +134,7 @@ class CardDavBackendTest extends TestCase {
                                $this->createMock(IUserSession::class),
                                $this->createMock(IAppManager::class),
                                $this->createMock(ProxyMapper::class),
+                               $this->createMock(IConfig::class),
                                ])
                        ->setMethods(['getPrincipalByPath', 'getGroupMembership'])
                        ->getMock();
@@ -396,7 +397,7 @@ class CardDavBackendTest extends TestCase {
                // 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 7836191450b052a12034de3758997bf1a99c0d67..cb268a943ec15cc8925caf83b3f8789563bbfee1 100644 (file)
--- 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;
+
        protected function setUp(): void {
                $this->userManager = $this->createMock(IUserManager::class);
                $this->groupManager = $this->createMock(IGroupManager::class);
@@ -72,6 +75,7 @@ class PrincipalTest extends TestCase {
                $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 @@ class PrincipalTest extends TestCase {
                        $this->shareManager,
                        $this->userSession,
                        $this->appManager,
-                       $this->proxyMapper
+                       $this->proxyMapper,
+                       $this->config
                );
                parent::setUp();
        }
@@ -209,7 +214,7 @@ class PrincipalTest extends TestCase {
                $this->assertSame([], $response);
        }
 
-       
+
        public function testGetGroupMemberSetEmpty() {
                $this->expectException(\Sabre\DAV\Exception::class);
                $this->expectExceptionMessage('Principal not found');
@@ -334,7 +339,7 @@ class PrincipalTest extends TestCase {
                $this->assertSame($expectedResponse, $response);
        }
 
-       
+
        public function testGetGroupMembershipEmpty() {
                $this->expectException(\Sabre\DAV\Exception::class);
                $this->expectExceptionMessage('Principal not found');
@@ -348,7 +353,7 @@ class PrincipalTest extends TestCase {
                $this->connector->getGroupMembership('principals/users/foo');
        }
 
-       
+
        public function testSetGroupMembership() {
                $this->expectException(\Sabre\DAV\Exception::class);
                $this->expectExceptionMessage('Setting members of the group is not supported yet');
@@ -403,11 +408,6 @@ class PrincipalTest extends TestCase {
                $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 @@ class PrincipalTest extends TestCase {
                        ->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 @@ class PrincipalTest extends TestCase {
                                        ->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 @@ class PrincipalTest extends TestCase {
                        ->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 @@ class PrincipalTest extends TestCase {
                        ['{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 08fdae18d9dd2742f255aa982adb6816d4112c4c..dde400da5cacb982e4fcb5e614d299c481036047 100644 (file)
--- a/apps/files_trashbin/lib/AppInfo/Application.php
+++ b/apps/files_trashbin/lib/AppInfo/Application.php
@@ -58,7 +58,8 @@ class Application extends App {
                                \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 25dc7fe5973afb6eda179632f2a9437acd878339..a7e88faedf69da34099255541f3343d6548234e7 100644 (file)
--- a/apps/files_versions/lib/AppInfo/Application.php
+++ b/apps/files_versions/lib/AppInfo/Application.php
@@ -66,7 +66,8 @@ class Application extends App {
                                $server->getShareManager(),
                                $server->getUserSession(),
                                $server->getAppManager(),
-                               $server->query(ProxyMapper::class)
+                               $server->query(ProxyMapper::class),
+                               \OC::$server->getConfig()
                        );
                });