diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 12d71b1528..96b6bae64b 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -54,6 +54,7 @@ class Access extends LDAPUtility implements IUserTools { * @var \OCA\User_LDAP\Connection */ public $connection; + /** @var Manager */ public $userManager; //never ever check this var directly, always use getPagedSearchResultState protected $pagedSearchedSuccessful; diff --git a/apps/user_ldap/lib/Exceptions/NotOnLDAP.php b/apps/user_ldap/lib/Exceptions/NotOnLDAP.php new file mode 100644 index 0000000000..41fb6c9e71 --- /dev/null +++ b/apps/user_ldap/lib/Exceptions/NotOnLDAP.php @@ -0,0 +1,26 @@ + + * + * @author Arthur Schiwon + * + * @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 . + * + */ + +namespace OCA\User_LDAP\Exceptions; + +class NotOnLDAP extends \Exception {} diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 13e61c63c0..e765814930 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -36,6 +36,7 @@ namespace OCA\User_LDAP; use OC\User\NoUserException; +use OCA\User_LDAP\Exceptions\NotOnLDAP; use OCA\User_LDAP\User\OfflineUser; use OCA\User_LDAP\User\User; use OCP\IConfig; @@ -80,14 +81,26 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn * @return string|false */ public function loginName2UserName($loginName) { + $cacheKey = 'loginName2UserName-'.$loginName; + $username = $this->access->connection->getFromCache($cacheKey); + if(!is_null($username)) { + return $username; + } + try { $ldapRecord = $this->getLDAPUserByLoginName($loginName); $user = $this->access->userManager->get($ldapRecord['dn'][0]); 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 $user->getUsername(); - } catch (\Exception $e) { + $username = $user->getUsername(); + $this->access->connection->writeToCache($cacheKey, $username); + return $username; + } catch (NotOnLDAP $e) { + $this->access->connection->writeToCache($cacheKey, false); return false; } } @@ -107,14 +120,14 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn * * @param string $loginName * @return array - * @throws \Exception + * @throws NotOnLDAP */ public function getLDAPUserByLoginName($loginName) { //find out dn of the user name $attrs = $this->access->userManager->getAttributes(); $users = $this->access->fetchUsersByLoginName($loginName, $attrs); 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); } return $users[0]; diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php index f2be36ed12..5859e51ec6 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -35,6 +35,8 @@ use OCA\User_LDAP\Helper; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\LogWrapper; use OCA\User_LDAP\User\Manager; +use OCA\User_LDAP\User\OfflineUser; +use OCA\User_LDAP\User\User; use OCA\User_LDAP\User_LDAP as UserLDAP; use OCP\IAvatarManager; use OCP\IConfig; @@ -62,13 +64,10 @@ class User_LDAPTest extends TestCase { \OC_Group::clearBackends(); } + /** + * @return \PHPUnit_Framework_MockObject_MockObject|Access + */ private function getAccessMock() { - static $accMethods; - - if(is_null($accMethods)) { - $accMethods = get_class_methods(Access::class); - unset($accMethods[array_search('getConnection', $accMethods)]); - } $lw = $this->createMock(ILDAPWrapper::class); $connector = $this->getMockBuilder(Connection::class) ->setMethodsExcept(['getConnection']) @@ -81,7 +80,6 @@ class User_LDAPTest extends TestCase { ->disableOriginalConstructor() ->getMock(); - /** @var Manager|\PHPUnit_Framework_MockObject_MockObject $um */ $um = $this->getMockBuilder(Manager::class) ->setMethods(['getDeletedUser']) ->setConstructorArgs([ @@ -136,7 +134,7 @@ class User_LDAPTest extends TestCase { /** * Prepares the Access mock for checkPassword tests - * @param Access $access mock + * @param Access|\PHPUnit_Framework_MockObject_MockObject $access mock * @param bool $noDisplayName * @return void */ @@ -849,4 +847,116 @@ class User_LDAPTest extends TestCase { $result = $backend->countUsers(); $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->createMock(IConfig::class)); + $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->createMock(IConfig::class)); + $name = $backend->loginName2UserName($loginName); + $this->assertSame(false, $name); + + // and once again to verify that caching works + $backend->loginName2UserName($loginName); + } + + public function testLoginName2UserNameOfflineUser() { + $loginName = 'Alice'; + $username = 'alice'; + $dn = 'uid=alice,dc=what,dc=ever'; + + $offlineUser = $this->getMockBuilder(OfflineUser::class) + ->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)); + + //$config = $this->createMock(IConfig::class); + $this->configMock->expects($this->once()) + ->method('getUserValue') + ->willReturn(1); + + $backend = new UserLDAP($access, $this->createMock(IConfig::class)); + $name = $backend->loginName2UserName($loginName); + $this->assertSame(false, $name); + + // and once again to verify that caching works + $backend->loginName2UserName($loginName); + } + }