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 3d65375307
commit 7aec7f6685
No known key found for this signature in database
GPG key ID: 9D98FD9380A1CB43
10 changed files with 125 additions and 12 deletions

View file

@ -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();

View file

@ -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();

View file

@ -80,7 +80,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();

View file

@ -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) {

View file

@ -62,9 +62,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

View file

@ -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();

View file

@ -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);

View file

@ -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 @@ 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();
}
@ -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')

View file

@ -63,7 +63,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()
);
});

View file

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