respect shareapi_allow_share_dialog_user_enumeration in Principal backend for Sabre/DAV

Signed-off-by: Georg Ehrke <developer@georgehrke.com>
This commit is contained in:
Georg Ehrke 2019-11-26 16:37:57 +01:00
parent 9fce87b2df
commit c3748cfee3
No known key found for this signature in database
GPG Key ID: 9D98FD9380A1CB43
10 changed files with 128 additions and 15 deletions

View File

@ -48,6 +48,7 @@ $principalBackend = new Principal(
\OC::$server->getUserSession(), \OC::$server->getUserSession(),
\OC::$server->getAppManager(), \OC::$server->getAppManager(),
\OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class), \OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class),
\OC::$server->getConfig(),
'principals/' 'principals/'
); );
$db = \OC::$server->getDatabaseConnection(); $db = \OC::$server->getDatabaseConnection();

View File

@ -49,6 +49,7 @@ $principalBackend = new Principal(
\OC::$server->getUserSession(), \OC::$server->getUserSession(),
\OC::$server->getAppManager(), \OC::$server->getAppManager(),
\OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class), \OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class),
\OC::$server->getConfig(),
'principals/' 'principals/'
); );
$db = \OC::$server->getDatabaseConnection(); $db = \OC::$server->getDatabaseConnection();

View File

@ -81,7 +81,8 @@ class CreateCalendar extends Command {
\OC::$server->getShareManager(), \OC::$server->getShareManager(),
\OC::$server->getUserSession(), \OC::$server->getUserSession(),
\OC::$server->getAppManager(), \OC::$server->getAppManager(),
\OC::$server->query(ProxyMapper::class) \OC::$server->query(ProxyMapper::class),
\OC::$server->getConfig()
); );
$random = \OC::$server->getSecureRandom(); $random = \OC::$server->getSecureRandom();
$logger = \OC::$server->getLogger(); $logger = \OC::$server->getLogger();

View File

@ -40,6 +40,7 @@ use OCA\DAV\CalDAV\Proxy\ProxyMapper;
use OCA\DAV\Traits\PrincipalProxyTrait; use OCA\DAV\Traits\PrincipalProxyTrait;
use OCP\App\IAppManager; use OCP\App\IAppManager;
use OCP\AppFramework\QueryException; use OCP\AppFramework\QueryException;
use OCP\IConfig;
use OCP\IGroup; use OCP\IGroup;
use OCP\IGroupManager; use OCP\IGroupManager;
use OCP\IUser; use OCP\IUser;
@ -79,6 +80,9 @@ class Principal implements BackendInterface {
/** @var ProxyMapper */ /** @var ProxyMapper */
private $proxyMapper; private $proxyMapper;
/** @var IConfig */
private $config;
/** /**
* Principal constructor. * Principal constructor.
* *
@ -88,6 +92,7 @@ class Principal implements BackendInterface {
* @param IUserSession $userSession * @param IUserSession $userSession
* @param IAppManager $appManager * @param IAppManager $appManager
* @param ProxyMapper $proxyMapper * @param ProxyMapper $proxyMapper
* @param IConfig $config
* @param string $principalPrefix * @param string $principalPrefix
*/ */
public function __construct(IUserManager $userManager, public function __construct(IUserManager $userManager,
@ -96,6 +101,7 @@ class Principal implements BackendInterface {
IUserSession $userSession, IUserSession $userSession,
IAppManager $appManager, IAppManager $appManager,
ProxyMapper $proxyMapper, ProxyMapper $proxyMapper,
IConfig $config,
string $principalPrefix = 'principals/users/') { string $principalPrefix = 'principals/users/') {
$this->userManager = $userManager; $this->userManager = $userManager;
$this->groupManager = $groupManager; $this->groupManager = $groupManager;
@ -105,6 +111,7 @@ class Principal implements BackendInterface {
$this->principalPrefix = trim($principalPrefix, '/'); $this->principalPrefix = trim($principalPrefix, '/');
$this->hasGroups = $this->hasCircles = ($principalPrefix === 'principals/users/'); $this->hasGroups = $this->hasCircles = ($principalPrefix === 'principals/users/');
$this->proxyMapper = $proxyMapper; $this->proxyMapper = $proxyMapper;
$this->config = $config;
} }
use PrincipalProxyTrait { use PrincipalProxyTrait {
@ -240,6 +247,8 @@ class Principal implements BackendInterface {
return []; return [];
} }
$allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
// 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
$restrictGroups = false; $restrictGroups = false;
@ -257,6 +266,12 @@ class Principal implements BackendInterface {
case '{http://sabredav.org/ns}email-address': case '{http://sabredav.org/ns}email-address':
$users = $this->userManager->getByEmail($value); $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) { $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) {
@ -274,6 +289,12 @@ class Principal implements BackendInterface {
case '{DAV:}displayname': case '{DAV:}displayname':
$users = $this->userManager->searchDisplayName($value); $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) { $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

@ -63,9 +63,10 @@ class RootCollection extends SimpleCollection {
$shareManager, $shareManager,
\OC::$server->getUserSession(), \OC::$server->getUserSession(),
\OC::$server->getAppManager(), \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); $calendarResourcePrincipalBackend = new ResourcePrincipalBackend($db, $userSession, $groupManager, $logger, $proxyMapper);
$calendarRoomPrincipalBackend = new RoomPrincipalBackend($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 // as soon as debug mode is enabled we allow listing of principals

View File

@ -86,6 +86,7 @@ abstract class AbstractCalDavBackend extends TestCase {
$this->createMock(IUserSession::class), $this->createMock(IUserSession::class),
$this->createMock(IAppManager::class), $this->createMock(IAppManager::class),
$this->createMock(ProxyMapper::class), $this->createMock(ProxyMapper::class),
$this->createMock(IConfig::class),
]) ])
->setMethods(['getPrincipalByPath', 'getGroupMembership']) ->setMethods(['getPrincipalByPath', 'getGroupMembership'])
->getMock(); ->getMock();

View File

@ -134,6 +134,7 @@ class CardDavBackendTest extends TestCase {
$this->createMock(IUserSession::class), $this->createMock(IUserSession::class),
$this->createMock(IAppManager::class), $this->createMock(IAppManager::class),
$this->createMock(ProxyMapper::class), $this->createMock(ProxyMapper::class),
$this->createMock(IConfig::class),
]) ])
->setMethods(['getPrincipalByPath', 'getGroupMembership']) ->setMethods(['getPrincipalByPath', 'getGroupMembership'])
->getMock(); ->getMock();
@ -396,7 +397,7 @@ class CardDavBackendTest extends TestCase {
// create a card // create a card
$uri0 = $this->getUniqueID('card'); $uri0 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri0, $this->vcardTest0); $this->backend->createCard($bookId, $uri0, $this->vcardTest0);
// create another card with same uid // create another card with same uid
$uri1 = $this->getUniqueID('card'); $uri1 = $this->getUniqueID('card');
$this->expectException(BadRequest::class); $this->expectException(BadRequest::class);

View File

@ -65,6 +65,9 @@ class PrincipalTest extends TestCase {
/** @var ProxyMapper | \PHPUnit_Framework_MockObject_MockObject */ /** @var ProxyMapper | \PHPUnit_Framework_MockObject_MockObject */
private $proxyMapper; private $proxyMapper;
/** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */
private $config;
protected function setUp(): void { protected function setUp(): void {
$this->userManager = $this->createMock(IUserManager::class); $this->userManager = $this->createMock(IUserManager::class);
$this->groupManager = $this->createMock(IGroupManager::class); $this->groupManager = $this->createMock(IGroupManager::class);
@ -72,6 +75,7 @@ class PrincipalTest extends TestCase {
$this->userSession = $this->createMock(IUserSession::class); $this->userSession = $this->createMock(IUserSession::class);
$this->appManager = $this->createMock(IAppManager::class); $this->appManager = $this->createMock(IAppManager::class);
$this->proxyMapper = $this->createMock(ProxyMapper::class); $this->proxyMapper = $this->createMock(ProxyMapper::class);
$this->config = $this->createMock(IConfig::class);
$this->connector = new \OCA\DAV\Connector\Sabre\Principal( $this->connector = new \OCA\DAV\Connector\Sabre\Principal(
$this->userManager, $this->userManager,
@ -79,7 +83,8 @@ class PrincipalTest extends TestCase {
$this->shareManager, $this->shareManager,
$this->userSession, $this->userSession,
$this->appManager, $this->appManager,
$this->proxyMapper $this->proxyMapper,
$this->config
); );
parent::setUp(); parent::setUp();
} }
@ -209,7 +214,7 @@ class PrincipalTest extends TestCase {
$this->assertSame([], $response); $this->assertSame([], $response);
} }
public function testGetGroupMemberSetEmpty() { public function testGetGroupMemberSetEmpty() {
$this->expectException(\Sabre\DAV\Exception::class); $this->expectException(\Sabre\DAV\Exception::class);
$this->expectExceptionMessage('Principal not found'); $this->expectExceptionMessage('Principal not found');
@ -334,7 +339,7 @@ class PrincipalTest extends TestCase {
$this->assertSame($expectedResponse, $response); $this->assertSame($expectedResponse, $response);
} }
public function testGetGroupMembershipEmpty() { public function testGetGroupMembershipEmpty() {
$this->expectException(\Sabre\DAV\Exception::class); $this->expectException(\Sabre\DAV\Exception::class);
$this->expectExceptionMessage('Principal not found'); $this->expectExceptionMessage('Principal not found');
@ -348,7 +353,7 @@ class PrincipalTest extends TestCase {
$this->connector->getGroupMembership('principals/users/foo'); $this->connector->getGroupMembership('principals/users/foo');
} }
public function testSetGroupMembership() { public function testSetGroupMembership() {
$this->expectException(\Sabre\DAV\Exception::class); $this->expectException(\Sabre\DAV\Exception::class);
$this->expectExceptionMessage('Setting members of the group is not supported yet'); $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']); $this->connector->setGroupMemberSet('principals/users/foo/calendar-proxy-write', ['principals/users/bar']);
} }
public function testUpdatePrincipal() { public function testUpdatePrincipal() {
$this->assertSame(0, $this->connector->updatePrincipal('foo', new PropPatch(array()))); $this->assertSame(0, $this->connector->updatePrincipal('foo', new PropPatch(array())));
} }
@ -430,6 +430,11 @@ class PrincipalTest extends TestCase {
->will($this->returnValue($sharingEnabled)); ->will($this->returnValue($sharingEnabled));
if ($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()) $this->shareManager->expects($this->once())
->method('shareWithGroupMembersOnly') ->method('shareWithGroupMembersOnly')
->will($this->returnValue($groupsOnly)); ->will($this->returnValue($groupsOnly));
@ -446,6 +451,8 @@ class PrincipalTest extends TestCase {
->will($this->returnValue(['group1', 'group2', 'group5'])); ->will($this->returnValue(['group1', 'group2', 'group5']));
} }
} else { } else {
$this->config->expects($this->never())
->method('getAppValue');
$this->shareManager->expects($this->never()) $this->shareManager->expects($this->never())
->method('shareWithGroupMembersOnly'); ->method('shareWithGroupMembersOnly');
$this->groupManager->expects($this->never()) $this->groupManager->expects($this->never())
@ -518,6 +525,11 @@ class PrincipalTest extends TestCase {
->method('shareAPIEnabled') ->method('shareAPIEnabled')
->will($this->returnValue(true)); ->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)) $this->shareManager->expects($this->exactly(2))
->method('shareWithGroupMembersOnly') ->method('shareWithGroupMembersOnly')
->will($this->returnValue(false)); ->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'])); ['{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() { public function testFindByUriSharingApiDisabled() {
$this->shareManager->expects($this->once()) $this->shareManager->expects($this->once())
->method('shareApiEnabled') ->method('shareApiEnabled')

View File

@ -58,7 +58,8 @@ class Application extends App {
\OC::$server->getShareManager(), \OC::$server->getShareManager(),
\OC::$server->getUserSession(), \OC::$server->getUserSession(),
\OC::$server->getAppManager(), \OC::$server->getAppManager(),
\OC::$server->query(ProxyMapper::class) \OC::$server->query(ProxyMapper::class),
\OC::$server->getConfig()
); );
}); });

View File

@ -66,7 +66,8 @@ class Application extends App {
$server->getShareManager(), $server->getShareManager(),
$server->getUserSession(), $server->getUserSession(),
$server->getAppManager(), $server->getAppManager(),
$server->query(ProxyMapper::class) $server->query(ProxyMapper::class),
\OC::$server->getConfig()
); );
}); });