ensure that users are cached when they are retrieved

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
Arthur Schiwon 2017-12-07 17:49:33 +01:00
parent 7d529c081a
commit 991190b994
No known key found for this signature in database
GPG Key ID: 7424F1874854DF23
3 changed files with 128 additions and 7 deletions

View File

@ -830,6 +830,7 @@ class Access extends LDAPUtility implements IUserTools {
$recordsToUpdate = array_filter($ldapRecords, function($record) use ($isBackgroundJobModeAjax) { $recordsToUpdate = array_filter($ldapRecords, function($record) use ($isBackgroundJobModeAjax) {
$newlyMapped = false; $newlyMapped = false;
$uid = $this->dn2ocname($record['dn'][0], null, true, $newlyMapped, $record); $uid = $this->dn2ocname($record['dn'][0], null, true, $newlyMapped, $record);
$this->cacheUserExists($uid);
return ($uid !== false) && ($newlyMapped || $isBackgroundJobModeAjax); return ($uid !== false) && ($newlyMapped || $isBackgroundJobModeAjax);
}); });
} }
@ -854,7 +855,6 @@ class Access extends LDAPUtility implements IUserTools {
if($ocName === false) { if($ocName === false) {
continue; continue;
} }
$this->cacheUserExists($ocName);
$user = $this->userManager->get($ocName); $user = $this->userManager->get($ocName);
if($user instanceof OfflineUser) { if($user instanceof OfflineUser) {
$user->unmark(); $user->unmark();
@ -1019,11 +1019,15 @@ class Access extends LDAPUtility implements IUserTools {
/** /**
* retrieved. Results will according to the order in the array. * retrieved. Results will according to the order in the array.
*
* @param $filter
* @param $base
* @param null $attr
* @param int $limit optional, maximum results to be counted * @param int $limit optional, maximum results to be counted
* @param int $offset optional, a starting point * @param int $offset optional, a starting point
* @return array|false array with the search result as first value and pagedSearchOK as * @return array|false array with the search result as first value and pagedSearchOK as
* second | false if not successful * second | false if not successful
* @throws \OC\ServerNotAvailableException * @throws ServerNotAvailableException
*/ */
private function executeSearch($filter, $base, &$attr = null, $limit = null, $offset = null) { private function executeSearch($filter, $base, &$attr = null, $limit = null, $offset = null) {
if(!is_null($attr) && !is_array($attr)) { if(!is_null($attr) && !is_array($attr)) {
@ -1169,6 +1173,7 @@ class Access extends LDAPUtility implements IUserTools {
/** /**
* Executes an LDAP search * Executes an LDAP search
*
* @param string $filter the LDAP filter for the search * @param string $filter the LDAP filter for the search
* @param array $base an array containing the LDAP subtree(s) that shall be searched * @param array $base an array containing the LDAP subtree(s) that shall be searched
* @param string|string[] $attr optional, array, one or more attributes that shall be * @param string|string[] $attr optional, array, one or more attributes that shall be
@ -1176,6 +1181,7 @@ class Access extends LDAPUtility implements IUserTools {
* @param int $offset * @param int $offset
* @param bool $skipHandling * @param bool $skipHandling
* @return array with the search result * @return array with the search result
* @throws ServerNotAvailableException
*/ */
public function search($filter, $base, $attr = null, $limit = null, $offset = null, $skipHandling = false) { public function search($filter, $base, $attr = null, $limit = null, $offset = null, $skipHandling = false) {
$limitPerPage = intval($this->connection->ldapPagingSize); $limitPerPage = intval($this->connection->ldapPagingSize);
@ -1189,12 +1195,12 @@ class Access extends LDAPUtility implements IUserTools {
* loops through until we get $continue equals true and * loops through until we get $continue equals true and
* $findings['count'] < $limit * $findings['count'] < $limit
*/ */
$findings = array(); $findings = [];
$savedoffset = $offset; $savedoffset = $offset;
do { do {
$search = $this->executeSearch($filter, $base, $attr, $limitPerPage, $offset); $search = $this->executeSearch($filter, $base, $attr, $limitPerPage, $offset);
if($search === false) { if($search === false) {
return array(); return [];
} }
list($sr, $pagedSearchOK) = $search; list($sr, $pagedSearchOK) = $search;
$cr = $this->connection->getConnectionResource(); $cr = $this->connection->getConnectionResource();
@ -1231,7 +1237,7 @@ class Access extends LDAPUtility implements IUserTools {
} }
if(!is_null($attr)) { if(!is_null($attr)) {
$selection = array(); $selection = [];
$i = 0; $i = 0;
foreach($findings as $item) { foreach($findings as $item) {
if(!is_array($item)) { if(!is_array($item)) {
@ -1239,7 +1245,6 @@ class Access extends LDAPUtility implements IUserTools {
} }
$item = \OCP\Util::mb_array_change_key_case($item, MB_CASE_LOWER, 'UTF-8'); $item = \OCP\Util::mb_array_change_key_case($item, MB_CASE_LOWER, 'UTF-8');
foreach($attr as $key) { foreach($attr as $key) {
$key = mb_strtolower($key, 'UTF-8');
if(isset($item[$key])) { if(isset($item[$key])) {
if(is_array($item[$key]) && isset($item[$key]['count'])) { if(is_array($item[$key]) && isset($item[$key]['count'])) {
unset($item[$key]['count']); unset($item[$key]['count']);

View File

@ -229,7 +229,7 @@ class Helper {
/** /**
* sanitizes a DN received from the LDAP server * sanitizes a DN received from the LDAP server
* @param array $dn the DN in question * @param array $dn the DN in question
* @return array the sanitized DN * @return array|string the sanitized DN
*/ */
public function sanitizeDN($dn) { public function sanitizeDN($dn) {
//treating multiple base DNs //treating multiple base DNs

View File

@ -60,6 +60,8 @@ use Test\TestCase;
* @package OCA\User_LDAP\Tests * @package OCA\User_LDAP\Tests
*/ */
class AccessTest extends TestCase { class AccessTest extends TestCase {
/** @var UserMapping|\PHPUnit_Framework_MockObject_MockObject */
protected $userMapper;
/** @var Connection|\PHPUnit_Framework_MockObject_MockObject */ /** @var Connection|\PHPUnit_Framework_MockObject_MockObject */
private $connection; private $connection;
/** @var LDAP|\PHPUnit_Framework_MockObject_MockObject */ /** @var LDAP|\PHPUnit_Framework_MockObject_MockObject */
@ -79,6 +81,7 @@ class AccessTest extends TestCase {
$this->userManager = $this->createMock(Manager::class); $this->userManager = $this->createMock(Manager::class);
$this->helper = $this->createMock(Helper::class); $this->helper = $this->createMock(Helper::class);
$this->config = $this->createMock(IConfig::class); $this->config = $this->createMock(IConfig::class);
$this->userMapper = $this->createMock(UserMapping::class);
$this->access = new Access( $this->access = new Access(
$this->connection, $this->connection,
@ -87,6 +90,7 @@ class AccessTest extends TestCase {
$this->helper, $this->helper,
$this->config $this->config
); );
$this->access->setUserMapper($this->userMapper);
} }
private function getConnectorAndLdapMock() { private function getConnectorAndLdapMock() {
@ -217,6 +221,7 @@ class AccessTest extends TestCase {
/** /**
* @dataProvider dnInputDataProvider * @dataProvider dnInputDataProvider
* @param array $case
*/ */
public function testStringResemblesDN($case) { public function testStringResemblesDN($case) {
list($lw, $con, $um, $helper) = $this->getConnectorAndLdapMock(); list($lw, $con, $um, $helper) = $this->getConnectorAndLdapMock();
@ -440,6 +445,7 @@ class AccessTest extends TestCase {
->method('__get') ->method('__get')
->willReturn(false); ->willReturn(false);
/** @noinspection PhpUnhandledExceptionInspection */
$this->access->setPassword('CN=foo', 'MyPassword'); $this->access->setPassword('CN=foo', 'MyPassword');
} }
@ -458,6 +464,7 @@ class AccessTest extends TestCase {
->with($connection) ->with($connection)
->willReturn(false); ->willReturn(false);
/** @noinspection PhpUnhandledExceptionInspection */
$this->assertFalse($this->access->setPassword('CN=foo', 'MyPassword')); $this->assertFalse($this->access->setPassword('CN=foo', 'MyPassword'));
} }
@ -485,6 +492,7 @@ class AccessTest extends TestCase {
->with($connection, 'CN=foo', 'MyPassword') ->with($connection, 'CN=foo', 'MyPassword')
->willThrowException(new ConstraintViolationException()); ->willThrowException(new ConstraintViolationException());
/** @noinspection PhpUnhandledExceptionInspection */
$this->access->setPassword('CN=foo', 'MyPassword'); $this->access->setPassword('CN=foo', 'MyPassword');
} }
@ -508,6 +516,114 @@ class AccessTest extends TestCase {
->with($connection, 'CN=foo', 'MyPassword') ->with($connection, 'CN=foo', 'MyPassword')
->willReturn(true); ->willReturn(true);
/** @noinspection PhpUnhandledExceptionInspection */
$this->assertTrue($this->access->setPassword('CN=foo', 'MyPassword')); $this->assertTrue($this->access->setPassword('CN=foo', 'MyPassword'));
} }
protected function prepareMocksForSearchTests(
$base,
$fakeConnection,
$fakeSearchResultResource,
$fakeLdapEntries
) {
$this->connection
->expects($this->any())
->method('getConnectionResource')
->willReturn($fakeConnection);
$this->connection->expects($this->any())
->method('__get')
->willReturnCallback(function($key) use ($base) {
if(stripos($key, 'base') !== false) {
return $base;
}
return null;
});
$this->ldap
->expects($this->any())
->method('isResource')
->willReturnCallback(function ($resource) use ($fakeConnection) {
return $resource === $fakeConnection;
});
$this->ldap
->expects($this->any())
->method('errno')
->willReturn(0);
$this->ldap
->expects($this->once())
->method('search')
->willReturn([$fakeSearchResultResource]);
$this->ldap
->expects($this->exactly(count($base)))
->method('getEntries')
->willReturn($fakeLdapEntries);
$this->helper->expects($this->any())
->method('sanitizeDN')
->willReturnArgument(0);
}
public function testSearchNoPagedSearch() {
// scenario: no pages search, 1 search base
$filter = 'objectClass=nextcloudUser';
$base = ['ou=zombies,dc=foobar,dc=nextcloud,dc=com'];
$fakeConnection = new \stdClass();
$fakeSearchResultResource = new \stdClass();
$fakeLdapEntries = [
'count' => 2,
[
'dn' => 'uid=sgarth,' . $base[0],
],
[
'dn' => 'uid=wwilson,' . $base[0],
]
];
$expected = $fakeLdapEntries;
unset($expected['count']);
$this->prepareMocksForSearchTests($base, $fakeConnection, $fakeSearchResultResource, $fakeLdapEntries);
/** @noinspection PhpUnhandledExceptionInspection */
$result = $this->access->search($filter, $base);
$this->assertSame($expected, $result);
}
public function testFetchListOfUsers() {
$filter = 'objectClass=nextcloudUser';
$base = ['ou=zombies,dc=foobar,dc=nextcloud,dc=com'];
$attrs = ['dn', 'uid'];
$fakeConnection = new \stdClass();
$fakeSearchResultResource = new \stdClass();
$fakeLdapEntries = [
'count' => 2,
[
'dn' => 'uid=sgarth,' . $base[0],
'uid' => [ 'sgarth' ],
],
[
'dn' => 'uid=wwilson,' . $base[0],
'uid' => [ 'wwilson' ],
]
];
$expected = $fakeLdapEntries;
unset($expected['count']);
array_walk($expected, function(&$v) {
$v['dn'] = [$v['dn']]; // dn is translated into an array internally for consistency
});
$this->prepareMocksForSearchTests($base, $fakeConnection, $fakeSearchResultResource, $fakeLdapEntries);
$this->connection->expects($this->exactly($fakeLdapEntries['count']))
->method('writeToCache')
->with($this->stringStartsWith('userExists'), true);
/** @noinspection PhpUnhandledExceptionInspection */
$list = $this->access->fetchListOfUsers($filter, $attrs);
$this->assertSame($expected, $list);
}
} }