Merge pull request #1654 from nextcloud/backport-1643-stable10

[stable10] cache loginName2UserName and cover the method with unit tests
This commit is contained in:
Morris Jobke 2016-10-10 09:15:25 +02:00 committed by GitHub
commit 466ebf8974
4 changed files with 176 additions and 26 deletions

View File

@ -54,6 +54,7 @@ class Access extends LDAPUtility implements IUserTools {
* @var \OCA\User_LDAP\Connection * @var \OCA\User_LDAP\Connection
*/ */
public $connection; public $connection;
/** @var Manager */
public $userManager; public $userManager;
//never ever check this var directly, always use getPagedSearchResultState //never ever check this var directly, always use getPagedSearchResultState
protected $pagedSearchedSuccessful; protected $pagedSearchedSuccessful;

View File

@ -0,0 +1,26 @@
<?php
/**
* @copyright Copyright (c) 2016 Arthur Schiwon <blizzz@arthur-schiwon.de>
*
* @author Arthur Schiwon <blizzz@arthur-schiwon.de>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\User_LDAP\Exceptions;
class NotOnLDAP extends \Exception {}

View File

@ -36,6 +36,7 @@
namespace OCA\User_LDAP; namespace OCA\User_LDAP;
use OC\User\NoUserException; use OC\User\NoUserException;
use OCA\User_LDAP\Exceptions\NotOnLDAP;
use OCA\User_LDAP\User\OfflineUser; use OCA\User_LDAP\User\OfflineUser;
use OCA\User_LDAP\User\User; use OCA\User_LDAP\User\User;
use OCP\IConfig; use OCP\IConfig;
@ -80,14 +81,26 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
* @return string|false * @return string|false
*/ */
public function loginName2UserName($loginName) { public function loginName2UserName($loginName) {
$cacheKey = 'loginName2UserName-'.$loginName;
$username = $this->access->connection->getFromCache($cacheKey);
if(!is_null($username)) {
return $username;
}
try { try {
$ldapRecord = $this->getLDAPUserByLoginName($loginName); $ldapRecord = $this->getLDAPUserByLoginName($loginName);
$user = $this->access->userManager->get($ldapRecord['dn'][0]); $user = $this->access->userManager->get($ldapRecord['dn'][0]);
if($user instanceof OfflineUser) { if($user instanceof OfflineUser) {
// this path is not really possible, however get() is documented
// to return User or OfflineUser so we are very defensive here.
$this->access->connection->writeToCache($cacheKey, false);
return false; return false;
} }
return $user->getUsername(); $username = $user->getUsername();
} catch (\Exception $e) { $this->access->connection->writeToCache($cacheKey, $username);
return $username;
} catch (NotOnLDAP $e) {
$this->access->connection->writeToCache($cacheKey, false);
return false; return false;
} }
} }
@ -107,14 +120,14 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
* *
* @param string $loginName * @param string $loginName
* @return array * @return array
* @throws \Exception * @throws NotOnLDAP
*/ */
public function getLDAPUserByLoginName($loginName) { public function getLDAPUserByLoginName($loginName) {
//find out dn of the user name //find out dn of the user name
$attrs = $this->access->userManager->getAttributes(); $attrs = $this->access->userManager->getAttributes();
$users = $this->access->fetchUsersByLoginName($loginName, $attrs); $users = $this->access->fetchUsersByLoginName($loginName, $attrs);
if(count($users) < 1) { if(count($users) < 1) {
throw new \Exception('No user available for the given login name on ' . throw new NotOnLDAP('No user available for the given login name on ' .
$this->access->connection->ldapHost . ':' . $this->access->connection->ldapPort); $this->access->connection->ldapHost . ':' . $this->access->connection->ldapPort);
} }
return $users[0]; return $users[0];

View File

@ -28,9 +28,10 @@
namespace OCA\User_LDAP\Tests; namespace OCA\User_LDAP\Tests;
use OCA\User_LDAP\Access;
use OCA\User_LDAP\Helper;
use OCA\User_LDAP\User_LDAP as UserLDAP; use OCA\User_LDAP\User_LDAP as UserLDAP;
use \OCA\User_LDAP\Access; use Test\TestCase;
use \OCA\User_LDAP\Connection;
/** /**
* Class Test_User_Ldap_Direct * Class Test_User_Ldap_Direct
@ -39,7 +40,7 @@ use \OCA\User_LDAP\Connection;
* *
* @package OCA\User_LDAP\Tests * @package OCA\User_LDAP\Tests
*/ */
class User_LDAPTest extends \Test\TestCase { class User_LDAPTest extends TestCase {
protected $backend; protected $backend;
protected $access; protected $access;
protected $configMock; protected $configMock;
@ -51,24 +52,22 @@ class User_LDAPTest extends \Test\TestCase {
\OC_Group::clearBackends(); \OC_Group::clearBackends();
} }
/**
* @return \PHPUnit_Framework_MockObject_MockObject|Access
*/
private function getAccessMock() { private function getAccessMock() {
static $conMethods; static $conMethods;
static $accMethods; static $accMethods;
static $uMethods;
if(is_null($conMethods) || is_null($accMethods)) { if(is_null($conMethods) || is_null($accMethods)) {
$conMethods = get_class_methods('\OCA\User_LDAP\Connection'); $conMethods = get_class_methods('\OCA\User_LDAP\Connection');
$accMethods = get_class_methods('\OCA\User_LDAP\Access'); $accMethods = get_class_methods('\OCA\User_LDAP\Access');
unset($accMethods[array_search('getConnection', $accMethods)]); unset($accMethods[array_search('getConnection', $accMethods)]);
$uMethods = get_class_methods('\OCA\User_LDAP\User\User');
unset($uMethods[array_search('getUsername', $uMethods)]);
unset($uMethods[array_search('getDN', $uMethods)]);
unset($uMethods[array_search('__construct', $uMethods)]);
} }
$lw = $this->getMock('\OCA\User_LDAP\ILDAPWrapper'); $lw = $this->getMock('\OCA\User_LDAP\ILDAPWrapper');
$connector = $this->getMock('\OCA\User_LDAP\Connection', $connector = $this->getMock(
$conMethods, '\OCA\User_LDAP\Connection', $conMethods, array($lw, null, null)
array($lw, null, null)); );
$this->configMock = $this->getMock('\OCP\IConfig'); $this->configMock = $this->getMock('\OCP\IConfig');
@ -76,7 +75,7 @@ class User_LDAPTest extends \Test\TestCase {
->disableOriginalConstructor() ->disableOriginalConstructor()
->getMock(); ->getMock();
$um = $this->getMockBuilder('\OCA\User_LDAP\User\Manager') $um = $this->getMockBuilder('OCA\User_LDAP\User\Manager')
->setMethods(['getDeletedUser']) ->setMethods(['getDeletedUser'])
->setConstructorArgs([ ->setConstructorArgs([
$this->configMock, $this->configMock,
@ -93,11 +92,12 @@ class User_LDAPTest extends \Test\TestCase {
->method('getDeletedUser') ->method('getDeletedUser')
->will($this->returnValue($offlineUser)); ->will($this->returnValue($offlineUser));
$helper = new \OCA\User_LDAP\Helper(); $helper = new Helper();
$access = $this->getMock('\OCA\User_LDAP\Access', $access = $this->getMock('\OCA\User_LDAP\Access',
$accMethods, $accMethods,
array($connector, $lw, $um, $helper)); array($connector, $lw, $um, $helper)
);
$um->setLdapAccess($access); $um->setLdapAccess($access);
@ -129,7 +129,7 @@ class User_LDAPTest extends \Test\TestCase {
/** /**
* Prepares the Access mock for checkPassword tests * Prepares the Access mock for checkPassword tests
* @param \OCA\User_LDAP\Access $access mock * @param Access|\PHPUnit_Framework_MockObject_MockObject $access mock
* @param bool $noDisplayName * @param bool $noDisplayName
* @return void * @return void
*/ */
@ -298,7 +298,7 @@ class User_LDAPTest extends \Test\TestCase {
/** /**
* Prepares the Access mock for getUsers tests * Prepares the Access mock for getUsers tests
* @param \OCA\User_LDAP\Access $access mock * @param Access $access mock
* @return void * @return void
*/ */
private function prepareAccessForGetUsers(&$access) { private function prepareAccessForGetUsers(&$access) {
@ -842,4 +842,114 @@ class User_LDAPTest extends \Test\TestCase {
$result = $backend->countUsers(); $result = $backend->countUsers();
$this->assertFalse($result); $this->assertFalse($result);
} }
public function testLoginName2UserNameSuccess() {
$loginName = 'Alice';
$username = 'alice';
$dn = 'uid=alice,dc=what,dc=ever';
$access = $this->getAccessMock();
$access->expects($this->once())
->method('fetchUsersByLoginName')
->with($this->equalTo($loginName))
->willReturn([['dn' => [$dn]]]);
$access->expects($this->once())
->method('stringResemblesDN')
->with($this->equalTo($dn))
->willReturn(true);
$access->expects($this->once())
->method('dn2username')
->with($this->equalTo($dn))
->willReturn($username);
$access->connection->expects($this->exactly(2))
->method('getFromCache')
->with($this->equalTo('loginName2UserName-'.$loginName))
->willReturnOnConsecutiveCalls(null, $username);
$access->connection->expects($this->once())
->method('writeToCache')
->with($this->equalTo('loginName2UserName-'.$loginName), $this->equalTo($username));
$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
$name = $backend->loginName2UserName($loginName);
$this->assertSame($username, $name);
// and once again to verify that caching works
$backend->loginName2UserName($loginName);
}
public function testLoginName2UserNameNoUsersOnLDAP() {
$loginName = 'Loki';
$access = $this->getAccessMock();
$access->expects($this->once())
->method('fetchUsersByLoginName')
->with($this->equalTo($loginName))
->willReturn([]);
$access->expects($this->never())
->method('stringResemblesDN');
$access->expects($this->never())
->method('dn2username');
$access->connection->expects($this->exactly(2))
->method('getFromCache')
->with($this->equalTo('loginName2UserName-'.$loginName))
->willReturnOnConsecutiveCalls(null, false);
$access->connection->expects($this->once())
->method('writeToCache')
->with($this->equalTo('loginName2UserName-'.$loginName), false);
$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
$name = $backend->loginName2UserName($loginName);
$this->assertSame(false, $name);
// and once again to verify that caching works
$backend->loginName2UserName($loginName);
}
public function testLoginName2UserNameOfflineUser() {
$loginName = 'Alice';
$dn = 'uid=alice,dc=what,dc=ever';
$offlineUser = $this->getMockBuilder('OCA\User_LDAP\User\OfflineUser')
->disableOriginalConstructor()
->getMock();
$access = $this->getAccessMock();
$access->expects($this->once())
->method('fetchUsersByLoginName')
->with($this->equalTo($loginName))
->willReturn([['dn' => [$dn]]]);
$access->expects($this->once())
->method('stringResemblesDN')
->with($this->equalTo($dn))
->willReturn(true);
$access->expects($this->once())
->method('dn2username')
->willReturn(false); // this is fake, but allows us to force-enter the OfflineUser path
$access->connection->expects($this->exactly(2))
->method('getFromCache')
->with($this->equalTo('loginName2UserName-'.$loginName))
->willReturnOnConsecutiveCalls(null, false);
$access->connection->expects($this->once())
->method('writeToCache')
->with($this->equalTo('loginName2UserName-'.$loginName), $this->equalTo(false));
$access->userManager->expects($this->once())
->method('getDeletedUser')
->will($this->returnValue($offlineUser));
$this->configMock->expects($this->once())
->method('getUserValue')
->willReturn(1);
$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
$name = $backend->loginName2UserName($loginName);
$this->assertSame(false, $name);
// and once again to verify that caching works
$backend->loginName2UserName($loginName);
}
} }