From c1953f96760163efb62f0bb8e244016d8e32dba7 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 21 Aug 2015 00:55:42 +0200 Subject: [PATCH 01/13] Backport #18469 (read all relevant user attributes on login and user search, in one query) read all relevant user attributes on login and user search, in one query. saves us some. Conflicts: apps/user_ldap/user_ldap.php adjust to nested group fix do not throw exception when no attribute is specified --- apps/user_ldap/group_ldap.php | 7 +- apps/user_ldap/lib/access.php | 29 +++- apps/user_ldap/lib/user/manager.php | 37 ++++++ apps/user_ldap/lib/user/user.php | 183 ++++++++++++++++++++++---- apps/user_ldap/tests/access.php | 67 ++++++++-- apps/user_ldap/tests/group_ldap.php | 6 +- apps/user_ldap/tests/user/manager.php | 42 ++++++ apps/user_ldap/tests/user/user.php | 168 ++++++++++++++++++++++- apps/user_ldap/tests/user_ldap.php | 18 ++- apps/user_ldap/user_ldap.php | 50 ++----- 10 files changed, 514 insertions(+), 93 deletions(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index ee3191bec954..4b45a157cd92 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -31,6 +31,7 @@ use OCA\user_ldap\lib\Access; use OCA\user_ldap\lib\BackendUtility; +use OCA\user_ldap\lib\user\User; class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { protected $enabled = false; @@ -195,7 +196,11 @@ private function _getGroupDNsFromMemberOf($DN, &$seen = null) { return array(); } $seen[$DN] = 1; - $groups = $this->access->readAttribute($DN, 'memberOf'); + $user = $this->access->userManager->get($DN); + if(!$user instanceof User) { + return array(); + } + $groups = $user->getMemberOfGroups(); if (!is_array($groups)) { return array(); } diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 44237b523938..54f959b6a5e4 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -536,6 +536,16 @@ private function ldap2ownCloudNames($ldapObjects, $isUsers) { return $ownCloudNames; } + /** + * caches the user display name + * @param string $ocName the internal ownCloud username + * @param string|false $home the home directory path + */ + public function cacheUserHome($ocName, $home) { + $cacheKey = 'getHome'.$ocName; + $this->connection->writeToCache($cacheKey, $home); + } + /** * caches a user as existing * @param string $ocName the internal ownCloud username @@ -657,7 +667,24 @@ public function fetchUsersByLoginName($loginName, $attributes = array('dn')) { * @return array */ public function fetchListOfUsers($filter, $attr, $limit = null, $offset = null) { - return $this->fetchList($this->searchUsers($filter, $attr, $limit, $offset), (count($attr) > 1)); + $ldapRecords = $this->searchUsers($filter, $attr, $limit, $offset); + $this->batchApplyUserAttributes($ldapRecords); + return $this->fetchList($ldapRecords, (count($attr) > 1)); + } + + /** + * provided with an array of LDAP user records the method will fetch the + * user object and requests it to process the freshly fetched attributes and + * and their values + * @param array $ldapRecords + */ + public function batchApplyUserAttributes(array $ldapRecords){ + foreach($ldapRecords as $userRecord) { + $ocName = $this->dn2ocname($userRecord['dn'], $userRecord[$this->connection->ldapUserDisplayName]); + $this->cacheUserExists($ocName); + $user = $this->userManager->get($ocName); + $user->processAttributes($userRecord); + } } /** diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php index b70e057741f7..86f0b8991d7f 100644 --- a/apps/user_ldap/lib/user/manager.php +++ b/apps/user_ldap/lib/user/manager.php @@ -126,6 +126,43 @@ private function checkAccess() { } } + /** + * returns a list of attributes that will be processed further, e.g. quota, + * email, displayname, or others. + * @param bool $minimal - optional, set to true to skip attributes with big + * payload + * @return string[] + */ + public function getAttributes($minimal = false) { + $attributes = array('dn', 'uid', 'samaccountname', 'memberof'); + $possible = array( + $this->access->getConnection()->ldapQuotaAttribute, + $this->access->getConnection()->ldapEmailAttribute, + $this->access->getConnection()->ldapUserDisplayName, + ); + foreach($possible as $attr) { + if(!is_null($attr)) { + $attributes[] = $attr; + } + } + + $homeRule = $this->access->getConnection()->homeFolderNamingRule; + if(strpos($homeRule, 'attr:') === 0) { + $attributes[] = substr($homeRule, strlen('attr:')); + } + + if(!$minimal) { + // attributes that are not really important but may come with big + // payload. + $attributes = array_merge($attributes, array( + 'jpegphoto', + 'thumbnailphoto' + )); + } + + return $attributes; + } + /** * Checks whether the specified user is marked as deleted * @param string $id the ownCloud user name diff --git a/apps/user_ldap/lib/user/user.php b/apps/user_ldap/lib/user/user.php index ac5d8f5a374c..6498cdf913f8 100644 --- a/apps/user_ldap/lib/user/user.php +++ b/apps/user_ldap/lib/user/user.php @@ -138,6 +138,69 @@ public function update() { } } + /** + * processes results from LDAP for attributes as returned by getAttributesToRead() + * @param array $ldapEntry the user entry as retrieved from LDAP + */ + public function processAttributes($ldapEntry) { + $this->markRefreshTime(); + //Quota + $attr = strtolower($this->connection->ldapQuotaAttribute); + if(isset($ldapEntry[$attr])) { + $this->updateQuota($ldapEntry[$attr]); + } + unset($attr); + + //Email + $attr = strtolower($this->connection->ldapEmailAttribute); + if(isset($ldapEntry[$attr])) { + $this->updateEmail($ldapEntry[$attr]); + } + unset($attr); + + //displayName + $attr = strtolower($this->connection->ldapUserDisplayName); + if(isset($ldapEntry[$attr])) { + $displayName = $ldapEntry[$attr]; + if(!empty($displayName)) { + $this->storeDisplayName($displayName); + $this->access->cacheUserDisplayName($this->getUsername(), $displayName); + } + } + unset($attr); + + // LDAP Username, needed for s2s sharing + if(isset($ldapEntry['uid'])) { + $this->storeLDAPUserName($ldapEntry['uid']); + } else if(isset($ldapEntry['samaccountname'])) { + $this->storeLDAPUserName($ldapEntry['samaccountname']); + } + //homePath + if(strpos($this->connection->homeFolderNamingRule, 'attr:') === 0) { + $attr = strtolower(substr($this->connection->homeFolderNamingRule, strlen('attr:'))); + if(isset($ldapEntry[$attr])) { + $this->access->cacheUserHome( + $this->getUsername(), $this->getHomePath($ldapEntry[$attr])); + } + } + //memberOf groups + $cacheKey = 'getMemberOf'.$this->getUsername(); + $groups = false; + if(isset($ldapEntry['memberof'])) { + $groups = $ldapEntry['memberof']; + } + $this->connection->writeToCache($cacheKey, $groups); + //Avatar + $attrs = array('jpegphoto', 'thumbnailphoto'); + foreach ($attrs as $attr) { + if(isset($ldapEntry[$attr])) { + $this->avatarImage = $ldapEntry[$attr]; + $this->updateAvatar(); + break; + } + } + } + /** * @brief returns the LDAP DN of the user * @return string @@ -154,6 +217,68 @@ public function getUsername() { return $this->uid; } + /** + * returns the home directory of the user if specified by LDAP settings + * @param string $valueFromLDAP + * @return bool|string + * @throws \Exception + */ + public function getHomePath($valueFromLDAP = null) { + $path = $valueFromLDAP; + $attr = null; + + if( is_null($path) + && strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0 + && $this->access->connection->homeFolderNamingRule !== 'attr:') + { + $attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:')); + $homedir = $this->access->readAttribute( + $this->access->username2dn($this->getUsername()), $attr); + if ($homedir && isset($homedir[0])) { + $path = $homedir[0]; + } + } + + if(!empty($path)) { + //if attribute's value is an absolute path take this, otherwise append it to data dir + //check for / at the beginning or pattern c:\ resp. c:/ + if( '/' !== $path[0] + && !(3 < strlen($path) && ctype_alpha($path[0]) + && $path[1] === ':' && ('\\' === $path[2] || '/' === $path[2])) + ) { + $path = $this->config->getSystemValue('datadirectory', + \OC::$SERVERROOT.'/data' ) . '/' . $path; + } + //we need it to store it in the DB as well in case a user gets + //deleted so we can clean up afterwards + $this->config->setUserValue( + $this->getUsername(), 'user_ldap', 'homePath', $path + ); + return $path; + } + + if( !is_null($attr) + && $this->config->getAppValue('user_ldap', 'enforce_home_folder_naming_rule', true) + ) { + // a naming rule attribute is defined, but it doesn't exist for that LDAP user + throw new \Exception('Home dir attribute can\'t be read from LDAP for uid: ' . $this->getUsername()); + } + + //false will apply default behaviour as defined and done by OC_User + $this->config->setUserValue($this->getUsername(), 'user_ldap', 'homePath', ''); + return false; + } + + public function getMemberOfGroups() { + $cacheKey = 'getMemberOf'.$this->getUsername(); + if($this->connection->isCached($cacheKey)) { + return $this->connection->getFromCache($cacheKey); + } + $groupDNs = $this->access->readAttribute($this->getDN(), 'memberOf'); + $this->connection->writeToCache($cacheKey, $groupDNs); + return $groupDNs; + } + /** * @brief reads the image from LDAP that shall be used as Avatar * @return string data (provided by LDAP) | false @@ -189,7 +314,7 @@ public function markLogin() { * @brief marks the time when user features like email have been updated * @return null */ - private function markRefreshTime() { + public function markRefreshTime() { $this->config->setUserValue( $this->uid, 'user_ldap', self::USER_PREFKEY_LASTREFRESH, time()); } @@ -252,48 +377,52 @@ private function wasRefreshed($feature) { } /** - * @brief fetches the email from LDAP and stores it as ownCloud user value + * fetches the email from LDAP and stores it as ownCloud user value + * @param string $valueFromLDAP if known, to save an LDAP read request * @return null */ - public function updateEmail() { + public function updateEmail($valueFromLDAP = null) { if($this->wasRefreshed('email')) { return; } - - $email = null; - $emailAttribute = $this->connection->ldapEmailAttribute; - if(!empty($emailAttribute)) { - $aEmail = $this->access->readAttribute($this->dn, $emailAttribute); - if($aEmail && (count($aEmail) > 0)) { - $email = $aEmail[0]; - } - if(!is_null($email)) { - $this->config->setUserValue( - $this->uid, 'settings', 'email', $email); + $email = $valueFromLDAP; + if(is_null($valueFromLDAP)) { + $emailAttribute = $this->connection->ldapEmailAttribute; + if(!empty($emailAttribute)) { + $aEmail = $this->access->readAttribute($this->dn, $emailAttribute); + if(is_array($aEmail) && (count($aEmail) > 0)) { + $email = $aEmail[0]; + } } } + if(!is_null($email)) { + $this->config->setUserValue( + $this->uid, 'settings', 'email', $email); + } } /** - * @brief fetches the quota from LDAP and stores it as ownCloud user value + * fetches the quota from LDAP and stores it as ownCloud user value + * @param string $valueFromLDAP the quota attribute's value can be passed, + * to save the readAttribute request * @return null */ - public function updateQuota() { + public function updateQuota($valueFromLDAP = null) { if($this->wasRefreshed('quota')) { return; } - - $quota = null; + //can be null $quotaDefault = $this->connection->ldapQuotaDefault; - $quotaAttribute = $this->connection->ldapQuotaAttribute; - if(!empty($quotaDefault)) { - $quota = $quotaDefault; - } - if(!empty($quotaAttribute)) { - $aQuota = $this->access->readAttribute($this->dn, $quotaAttribute); - - if($aQuota && (count($aQuota) > 0)) { - $quota = $aQuota[0]; + $quota = !is_null($valueFromLDAP) + ? $valueFromLDAP + : $quotaDefault !== '' ? $quotaDefault : null; + if(is_null($valueFromLDAP)) { + $quotaAttribute = $this->connection->ldapQuotaAttribute; + if(!empty($quotaAttribute)) { + $aQuota = $this->access->readAttribute($this->dn, $quotaAttribute); + if($aQuota && (count($aQuota) > 0)) { + $quota = $aQuota[0]; + } } } if(!is_null($quota)) { diff --git a/apps/user_ldap/tests/access.php b/apps/user_ldap/tests/access.php index c4cd2036461c..5bf1a65bd51e 100644 --- a/apps/user_ldap/tests/access.php +++ b/apps/user_ldap/tests/access.php @@ -29,7 +29,7 @@ use \OCA\user_ldap\lib\ILDAPWrapper; class Test_Access extends \Test\TestCase { - private function getConnecterAndLdapMock() { + private function getConnectorAndLdapMock() { static $conMethods; static $accMethods; static $umMethods; @@ -56,7 +56,7 @@ private function getConnecterAndLdapMock() { } public function testEscapeFilterPartValidChars() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $access = new Access($con, $lw, $um); $input = 'okay'; @@ -64,7 +64,7 @@ public function testEscapeFilterPartValidChars() { } public function testEscapeFilterPartEscapeWildcard() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $access = new Access($con, $lw, $um); $input = '*'; @@ -73,7 +73,7 @@ public function testEscapeFilterPartEscapeWildcard() { } public function testEscapeFilterPartEscapeWildcard2() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $access = new Access($con, $lw, $um); $input = 'foo*bar'; @@ -83,7 +83,7 @@ public function testEscapeFilterPartEscapeWildcard2() { /** @dataProvider convertSID2StrSuccessData */ public function testConvertSID2StrSuccess(array $sidArray, $sidExpected) { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $access = new Access($con, $lw, $um); $sidBinary = implode('', $sidArray); @@ -118,7 +118,7 @@ public function convertSID2StrSuccessData() { } public function testConvertSID2StrInputError() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $access = new Access($con, $lw, $um); $sidIllegal = 'foobar'; @@ -128,7 +128,7 @@ public function testConvertSID2StrInputError() { } public function testGetDomainDNFromDNSuccess() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $access = new Access($con, $lw, $um); $inputDN = 'uid=zaphod,cn=foobar,dc=my,dc=server,dc=com'; @@ -143,7 +143,7 @@ public function testGetDomainDNFromDNSuccess() { } public function testGetDomainDNFromDNError() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $access = new Access($con, $lw, $um); $inputDN = 'foobar'; @@ -178,7 +178,7 @@ private function getResemblesDNInputData() { } public function testStringResemblesDN() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $access = new Access($con, $lw, $um); $cases = $this->getResemblesDNInputData(); @@ -199,7 +199,7 @@ public function testStringResemblesDN() { } public function testStringResemblesDNLDAPmod() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); $lw = new \OCA\user_ldap\lib\LDAP(); $access = new Access($con, $lw, $um); @@ -213,4 +213,51 @@ public function testStringResemblesDNLDAPmod() { $this->assertSame($case['expectedResult'], $access->stringResemblesDN($case['input'])); } } + + public function testCacheUserHome() { + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); + $access = new Access($con, $lw, $um); + + $con->expects($this->once()) + ->method('writeToCache'); + + $access->cacheUserHome('foobar', '/foobars/path'); + } + + public function testBatchApplyUserAttributes() { + list($lw, $con, $um) = $this->getConnectorAndLdapMock(); + $access = new Access($con, $lw, $um); + $mapperMock = $this->getMockBuilder('\OCA\User_LDAP\Mapping\UserMapping') + ->disableOriginalConstructor() + ->getMock(); + $userMock = $this->getMockBuilder('\OCA\user_ldap\lib\user\User') + ->disableOriginalConstructor() + ->getMock(); + + $access->setUserMapper($mapperMock); + + $data = array( + array( + 'dn' => 'foobar', + $con->ldapUserDisplayName => 'barfoo' + ), + array( + 'dn' => 'foo', + $con->ldapUserDisplayName => 'bar' + ), + array( + 'dn' => 'raboof', + $con->ldapUserDisplayName => 'oofrab' + ) + ); + + $userMock->expects($this->exactly(count($data))) + ->method('processAttributes'); + + $um->expects($this->exactly(count($data))) + ->method('get') + ->will($this->returnValue($userMock)); + + $access->batchApplyUserAttributes($data); + } } diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php index 805238e7d379..7007afd51621 100644 --- a/apps/user_ldap/tests/group_ldap.php +++ b/apps/user_ldap/tests/group_ldap.php @@ -53,6 +53,10 @@ private function getAccessMock() { $accMethods, array($connector, $lw, $um)); + $access->expects($this->any()) + ->method('getConnection') + ->will($this->returnValue($connector)); + return $access; } @@ -391,7 +395,7 @@ public function testGetUserGroupsMemberOf() { $access->connection->hasPrimaryGroups = false; - $access->expects($this->once()) + $access->expects($this->any()) ->method('username2dn') ->will($this->returnValue($dn)); diff --git a/apps/user_ldap/tests/user/manager.php b/apps/user_ldap/tests/user/manager.php index d659323fc7f8..98e48638d8b2 100644 --- a/apps/user_ldap/tests/user/manager.php +++ b/apps/user_ldap/tests/user/manager.php @@ -37,6 +37,16 @@ private function getTestInstances() { $image = $this->getMock('\OCP\Image'); $dbc = $this->getMock('\OCP\IDBConnection'); + $connection = new \OCA\user_ldap\lib\Connection( + $lw = $this->getMock('\OCA\user_ldap\lib\ILDAPWrapper'), + '', + null + ); + + $access->expects($this->any()) + ->method('getConnection') + ->will($this->returnValue($connection)); + return array($access, $config, $filesys, $image, $log, $avaMgr, $dbc); } @@ -206,4 +216,36 @@ public function testGetByUidNotExisting() { $this->assertNull($user); } + public function testGetAttributesAll() { + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = + $this->getTestInstances(); + + $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc); + $manager->setLdapAccess($access); + + $connection = $access->getConnection(); + $connection->setConfiguration(array('ldapEmailAttribute' => 'mail')); + + $attributes = $manager->getAttributes(); + + $this->assertTrue(in_array('dn', $attributes)); + $this->assertTrue(in_array($access->getConnection()->ldapEmailAttribute, $attributes)); + $this->assertTrue(in_array('jpegphoto', $attributes)); + $this->assertTrue(in_array('thumbnailphoto', $attributes)); + } + + public function testGetAttributesMinimal() { + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = + $this->getTestInstances(); + + $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc); + $manager->setLdapAccess($access); + + $attributes = $manager->getAttributes(true); + + $this->assertTrue(in_array('dn', $attributes)); + $this->assertTrue(!in_array('jpegphoto', $attributes)); + $this->assertTrue(!in_array('thumbnailphoto', $attributes)); + } + } diff --git a/apps/user_ldap/tests/user/user.php b/apps/user_ldap/tests/user/user.php index 80baafcb2504..1c41eb71ec27 100644 --- a/apps/user_ldap/tests/user/user.php +++ b/apps/user_ldap/tests/user/user.php @@ -221,7 +221,7 @@ public function testUpdateQuotaDefaultProvided() { $connection->expects($this->at(0)) ->method('__get') ->with($this->equalTo('ldapQuotaDefault')) - ->will($this->returnValue('23 GB')); + ->will($this->returnValue('25 GB')); $connection->expects($this->at(1)) ->method('__get') @@ -242,7 +242,7 @@ public function testUpdateQuotaDefaultProvided() { ->with($this->equalTo('alice'), $this->equalTo('files'), $this->equalTo('quota'), - $this->equalTo('23 GB')) + $this->equalTo('25 GB')) ->will($this->returnValue(true)); $uid = 'alice'; @@ -278,14 +278,14 @@ public function testUpdateQuotaIndividualProvided() { ->method('readAttribute') ->with($this->equalTo('uid=alice,dc=foo,dc=bar'), $this->equalTo('myquota')) - ->will($this->returnValue(array('23 GB'))); + ->will($this->returnValue(array('27 GB'))); $config->expects($this->once()) ->method('setUserValue') ->with($this->equalTo('alice'), $this->equalTo('files'), $this->equalTo('quota'), - $this->equalTo('23 GB')) + $this->equalTo('27 GB')) ->will($this->returnValue(true)); $uid = 'alice'; @@ -679,4 +679,164 @@ public function testGetAvatarImageProvided() { //photo is returned $photo = $user->getAvatarImage(); } + + public function testProcessAttributes() { + list(, $config, $filesys, $image, $log, $avaMgr, $dbc) = + $this->getTestInstances(); + + list($access, $connection) = + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); + + $uid = 'alice'; + $dn = 'uid=alice'; + + $requiredMethods = array( + 'markRefreshTime', + 'updateQuota', + 'updateEmail', + 'storeDisplayName', + 'storeLDAPUserName', + 'getHomePath', + 'updateAvatar' + ); + + $userMock = $this->getMockBuilder('OCA\user_ldap\lib\user\User') + ->setConstructorArgs(array($uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr)) + ->setMethods($requiredMethods) + ->getMock(); + + $connection->setConfiguration(array( + 'homeFolderNamingRule' => 'homeDirectory' + )); + + $connection->expects($this->any()) + ->method('__get') + //->will($this->returnArgument(0)); + ->will($this->returnCallback(function($name) { + if($name === 'homeFolderNamingRule') { + return 'attr:homeDirectory'; + } + return $name; + })); + + $record = array( + strtolower($connection->ldapQuotaAttribute) => array('4096'), + strtolower($connection->ldapEmailAttribute) => array('alice@wonderland.org'), + strtolower($connection->ldapUserDisplayName) => array('Aaaaalice'), + 'uid' => array($uid), + 'homedirectory' => array('Alice\'s Folder'), + 'memberof' => array('cn=groupOne', 'cn=groupTwo'), + 'jpegphoto' => array('here be an image') + ); + + foreach($requiredMethods as $method) { + $userMock->expects($this->once()) + ->method($method); + } + + $userMock->processAttributes($record); + } + + public function emptyHomeFolderAttributeValueProvider() { + return array( + 'empty' => array(''), + 'prefixOnly' => array('attr:'), + ); + } + + /** + * @dataProvider emptyHomeFolderAttributeValueProvider + */ + public function testGetHomePathNotConfigured($attributeValue) { + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = + $this->getTestInstances(); + + list($access, $connection) = + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); + + $connection->expects($this->any()) + ->method('__get') + ->with($this->equalTo('homeFolderNamingRule')) + ->will($this->returnValue($attributeValue)); + + $access->expects($this->never()) + ->method('readAttribute'); + + $config->expects($this->never()) + ->method('getAppValue'); + + $uid = 'alice'; + $dn = 'uid=alice,dc=foo,dc=bar'; + + $user = new User( + $uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr); + + $path = $user->getHomePath(); + $this->assertSame($path, false); + } + + public function testGetHomePathConfiguredNotAvailableAllowed() { + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = + $this->getTestInstances(); + + list($access, $connection) = + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); + + $connection->expects($this->any()) + ->method('__get') + ->with($this->equalTo('homeFolderNamingRule')) + ->will($this->returnValue('attr:foobar')); + + $access->expects($this->once()) + ->method('readAttribute') + ->will($this->returnValue(false)); + + // asks for "enforce_home_folder_naming_rule" + $config->expects($this->once()) + ->method('getAppValue') + ->will($this->returnValue(false)); + + $uid = 'alice'; + $dn = 'uid=alice,dc=foo,dc=bar'; + + $user = new User( + $uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr); + + $path = $user->getHomePath(); + + $this->assertSame($path, false); + } + + /** + * @expectedException \Exception + */ + public function testGetHomePathConfiguredNotAvailableNotAllowed() { + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = + $this->getTestInstances(); + + list($access, $connection) = + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); + + $connection->expects($this->any()) + ->method('__get') + ->with($this->equalTo('homeFolderNamingRule')) + ->will($this->returnValue('attr:foobar')); + + $access->expects($this->once()) + ->method('readAttribute') + ->will($this->returnValue(false)); + + // asks for "enforce_home_folder_naming_rule" + $config->expects($this->once()) + ->method('getAppValue') + ->will($this->returnValue(true)); + + $uid = 'alice'; + $dn = 'uid=alice,dc=foo,dc=bar'; + + $user = new User( + $uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr); + + $user->getHomePath(); + } } diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index 2b99e1c2dc8e..69a76c0b7acd 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -34,6 +34,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { protected $backend; protected $access; + protected $configMock; protected function setUp() { parent::setUp(); @@ -61,8 +62,9 @@ private function getAccessMock() { $conMethods, array($lw, null, null)); + $this->configMock = $this->getMock('\OCP\IConfig'); $um = new \OCA\user_ldap\lib\user\Manager( - $this->getMock('\OCP\IConfig'), + $this->configMock, $this->getMock('\OCA\user_ldap\lib\FilesystemHelper'), $this->getMock('\OCA\user_ldap\lib\LogWrapper'), $this->getMock('\OCP\IAvatarManager'), @@ -586,6 +588,13 @@ public function testGetHomeRelative() { $backend = new UserLDAP($access, $config); $this->prepareMockForUserExists($access); + $dataDir = \OC::$server->getConfig()->getSystemValue( + 'datadirectory', \OC::$SERVERROOT.'/data'); + + $this->configMock->expects($this->once()) + ->method('getSystemValue') + ->will($this->returnValue($dataDir)); + $access->connection->expects($this->any()) ->method('__get') ->will($this->returnCallback(function($name) { @@ -609,14 +618,9 @@ public function testGetHomeRelative() { return false; } })); - //datadir-relativ path - $datadir = '/my/data/dir'; - $config->expects($this->once()) - ->method('getSystemValue') - ->will($this->returnValue($datadir)); $result = $backend->getHome('ladyofshadows'); - $this->assertEquals($datadir.'/susannah/', $result); + $this->assertEquals($dataDir.'/susannah/', $result); } /** diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index caff30a0e601..f38cac21212d 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -101,16 +101,9 @@ public function checkPassword($uid, $password) { return false; } + $this->access->cacheUserExists($user->getUsername()); + $user->processAttributes($ldapRecord); $user->markLogin(); - if(isset($users[0][$this->access->connection->ldapUserDisplayName])) { - $dpn = $users[0][$this->access->connection->ldapUserDisplayName]; - $user->storeDisplayName($dpn); - } - if(isset($users[0]['uid'])) { - $user->storeLDAPUserName($users[0]['uid']); - } else if(isset($users[0]['samaccountname'])) { - $user->storeLDAPUserName($users[0]['samaccountname']); - } return $user->getUsername(); } @@ -152,7 +145,7 @@ public function getUsers($search = '', $limit = 10, $offset = 0) { //do the search and translate results to owncloud names $ldap_users = $this->access->fetchListOfUsers( $filter, - array($this->access->connection->ldapUserDisplayName, 'dn'), + $this->access->userManager->getAttributes(true), $limit, $offset); $ldap_users = $this->access->ownCloudUserNames($ldap_users); \OCP\Util::writeLog('user_ldap', 'getUsers: '.count($ldap_users). ' Users found', \OCP\Util::DEBUG); @@ -266,39 +259,12 @@ public function getHome($uid) { if($this->access->connection->isCached($cacheKey)) { return $this->access->connection->getFromCache($cacheKey); } - if(strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0) { - $attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:')); - $homedir = $this->access->readAttribute( - $this->access->username2dn($uid), $attr); - if($homedir && isset($homedir[0])) { - $path = $homedir[0]; - //if attribute's value is an absolute path take this, otherwise append it to data dir - //check for / at the beginning or pattern c:\ resp. c:/ - if( - '/' === $path[0] - || (3 < strlen($path) && ctype_alpha($path[0]) - && $path[1] === ':' && ('\\' === $path[2] || '/' === $path[2])) - ) { - $homedir = $path; - } else { - $homedir = $this->ocConfig->getSystemValue('datadirectory', - \OC::$SERVERROOT.'/data' ) . '/' . $homedir[0]; - } - $this->access->connection->writeToCache($cacheKey, $homedir); - //we need it to store it in the DB as well in case a user gets - //deleted so we can clean up afterwards - $this->ocConfig->setUserValue( - $uid, 'user_ldap', 'homePath', $homedir - ); - //TODO: if home directory changes, the old one needs to be removed. - return $homedir; - } - } - //false will apply default behaviour as defined and done by OC_User - $this->access->connection->writeToCache($cacheKey, false); - $this->ocConfig->setUserValue($uid, 'user_ldap', 'homePath', ''); - return false; + $user = $this->access->userManager->get($uid); + $path = $user->getHomePath(); + $this->access->cacheUserHome($uid, $path); + + return $path; } /** From 0a9904963d3b26c74693847ff64568ee828799bb Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 28 Sep 2015 22:58:10 +0200 Subject: [PATCH 02/13] stable8.1 related adjustments to #18469 backport --- apps/user_ldap/lib/access.php | 12 +++++++++++- apps/user_ldap/user_ldap.php | 28 ++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 54f959b6a5e4..4890563eb539 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -680,10 +680,20 @@ public function fetchListOfUsers($filter, $attr, $limit = null, $offset = null) */ public function batchApplyUserAttributes(array $ldapRecords){ foreach($ldapRecords as $userRecord) { + if(!isset($userRecord[$this->connection->ldapUserDisplayName])) { + // displayName is obligatory + continue; + } $ocName = $this->dn2ocname($userRecord['dn'], $userRecord[$this->connection->ldapUserDisplayName]); + if(!$ocName) { + // no user name, skip. + continue; + } $this->cacheUserExists($ocName); $user = $this->userManager->get($ocName); - $user->processAttributes($userRecord); + if(!is_null($user)) { + $user->processAttributes($userRecord); + } } } diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index f38cac21212d..7d4d6cd2cb22 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -70,6 +70,23 @@ public function canChangeAvatar($uid) { return false; } + /** + * returns an LDAP record based on a given login name + * + * @param string $loginName + * @return array + * @throws \Exception + */ + public function getLDAPUserByLoginName($loginName) { + //find out dn of the user name + $attrs = $this->access->userManager->getAttributes(); + $users = $this->access->fetchUsersByLoginName($loginName, $attrs, 1); + if(count($users) < 1) { + throw new \Exception('No user available for the given login name.'); + } + return $users[0]; + } + /** * Check if the password is correct * @param string $uid The username @@ -79,15 +96,14 @@ public function canChangeAvatar($uid) { * Check if the password is correct without logging in the user */ public function checkPassword($uid, $password) { - //find out dn of the user name - $attrs = array($this->access->connection->ldapUserDisplayName, 'dn', - 'uid', 'samaccountname'); - $users = $this->access->fetchUsersByLoginName($uid, $attrs); - if(count($users) < 1) { + try { + $ldapRecord = $this->getLDAPUserByLoginName($uid); + } catch(\Exception $e) { return false; } - $dn = $users[0]['dn']; + $dn = $ldapRecord['dn']; $user = $this->access->userManager->get($dn); + if(!$user instanceof User) { \OCP\Util::writeLog('user_ldap', 'LDAP Login: Could not get user object for DN ' . $dn . From a05ead505d5cd6b29119e96c95d83b6d0bdb0f98 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 29 Sep 2015 16:19:45 +0200 Subject: [PATCH 03/13] fix update quota with known value --- apps/user_ldap/lib/user/user.php | 6 ++--- apps/user_ldap/tests/user/user.php | 39 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/apps/user_ldap/lib/user/user.php b/apps/user_ldap/lib/user/user.php index 6498cdf913f8..1f48a480843e 100644 --- a/apps/user_ldap/lib/user/user.php +++ b/apps/user_ldap/lib/user/user.php @@ -413,9 +413,9 @@ public function updateQuota($valueFromLDAP = null) { } //can be null $quotaDefault = $this->connection->ldapQuotaDefault; - $quota = !is_null($valueFromLDAP) - ? $valueFromLDAP - : $quotaDefault !== '' ? $quotaDefault : null; + $quota = $quotaDefault !== '' ? $quotaDefault : null; + $quota = !is_null($valueFromLDAP) ? $valueFromLDAP : $quota; + if(is_null($valueFromLDAP)) { $quotaAttribute = $this->connection->ldapQuotaAttribute; if(!empty($quotaAttribute)) { diff --git a/apps/user_ldap/tests/user/user.php b/apps/user_ldap/tests/user/user.php index 1c41eb71ec27..19581d835d1f 100644 --- a/apps/user_ldap/tests/user/user.php +++ b/apps/user_ldap/tests/user/user.php @@ -370,6 +370,45 @@ public function testUpdateQuotaNoneConfigured() { $user->updateQuota(); } + public function testUpdateQuotaFromValue() { + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = + $this->getTestInstances(); + + list($access, $connection) = + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); + + $readQuota = '19 GB'; + + $connection->expects($this->at(0)) + ->method('__get') + ->with($this->equalTo('ldapQuotaDefault')) + ->will($this->returnValue('')); + + $connection->expects($this->once(1)) + ->method('__get') + ->with($this->equalTo('ldapQuotaDefault')) + ->will($this->returnValue(null)); + + $access->expects($this->never()) + ->method('readAttribute'); + + $config->expects($this->once()) + ->method('setUserValue') + ->with($this->equalTo('alice'), + $this->equalTo('files'), + $this->equalTo('quota'), + $this->equalTo($readQuota)) + ->will($this->returnValue(true)); + + $uid = 'alice'; + $dn = 'uid=alice,dc=foo,dc=bar'; + + $user = new User( + $uid, $dn, $access, $config, $filesys, $image, $log, $avaMgr); + + $user->updateQuota($readQuota); + } + //the testUpdateAvatar series also implicitely tests getAvatarImage public function testUpdateAvatarJpegPhotoProvided() { list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = From 7ccd52db253ad6909092e87e89a17d8c078e01f4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 1 Oct 2015 17:54:40 +0200 Subject: [PATCH 04/13] lowercase configured displayname attribute so isset works - all attribute names coming from ldap are lowercased for easy comparison --- apps/user_ldap/lib/access.php | 6 ++++-- apps/user_ldap/tests/access.php | 16 +++++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 4890563eb539..c1c6a8fac45b 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -679,12 +679,14 @@ public function fetchListOfUsers($filter, $attr, $limit = null, $offset = null) * @param array $ldapRecords */ public function batchApplyUserAttributes(array $ldapRecords){ + $displayNameAttribute = strtolower($this->connection->ldapUserDisplayName); foreach($ldapRecords as $userRecord) { - if(!isset($userRecord[$this->connection->ldapUserDisplayName])) { + if(!isset($userRecord[$displayNameAttribute])) { // displayName is obligatory continue; } - $ocName = $this->dn2ocname($userRecord['dn'], $userRecord[$this->connection->ldapUserDisplayName]); + + $ocName = $this->dn2ocname($userRecord['dn'], $userRecord[$displayNameAttribute]); if(!$ocName) { // no user name, skip. continue; diff --git a/apps/user_ldap/tests/access.php b/apps/user_ldap/tests/access.php index 5bf1a65bd51e..5c535720fec5 100644 --- a/apps/user_ldap/tests/access.php +++ b/apps/user_ldap/tests/access.php @@ -230,24 +230,34 @@ public function testBatchApplyUserAttributes() { $mapperMock = $this->getMockBuilder('\OCA\User_LDAP\Mapping\UserMapping') ->disableOriginalConstructor() ->getMock(); + + $mapperMock->expects($this->any()) + ->method('getNameByDN') + ->will($this->returnValue('a_username')); + $userMock = $this->getMockBuilder('\OCA\user_ldap\lib\user\User') ->disableOriginalConstructor() ->getMock(); + $access->connection->expects($this->any()) + ->method('__get') + ->will($this->returnValue('displayName')); + $access->setUserMapper($mapperMock); + $displayNameAttribute = strtolower($access->connection->ldapUserDisplayName); $data = array( array( 'dn' => 'foobar', - $con->ldapUserDisplayName => 'barfoo' + $displayNameAttribute => 'barfoo' ), array( 'dn' => 'foo', - $con->ldapUserDisplayName => 'bar' + $displayNameAttribute => 'bar' ), array( 'dn' => 'raboof', - $con->ldapUserDisplayName => 'oofrab' + $displayNameAttribute => 'oofrab' ) ); From 6ccc5a6cd34deea57f0ae7a64c10a16e7ca5c30f Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 7 Oct 2015 18:57:49 +0200 Subject: [PATCH 05/13] allow an attribute to return more than one value --- apps/user_ldap/lib/access.php | 41 +++++++++++------------------------ apps/user_ldap/lib/wizard.php | 2 +- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index c1c6a8fac45b..0005bad2660f 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -720,6 +720,9 @@ private function fetchList($list, $manyAttributes) { if($manyAttributes) { return $list; } else { + $list = array_reduce($list, function($carry, $item) { + $carry[] = $item[0]; + }, array()); return array_unique($list, SORT_LOCALE_STRING); } } @@ -992,44 +995,26 @@ private function search($filter, $base, $attr = null, $limit = null, $offset = n if(!is_null($attr)) { $selection = array(); - $multiArray = false; - if(count($attr) > 1) { - $multiArray = true; - $i = 0; - } + $i = 0; foreach($findings as $item) { if(!is_array($item)) { continue; } $item = \OCP\Util::mb_array_change_key_case($item, MB_CASE_LOWER, 'UTF-8'); - - if($multiArray) { - foreach($attr as $key) { - $key = mb_strtolower($key, 'UTF-8'); - if(isset($item[$key])) { - if($key !== 'dn') { - $selection[$i][$key] = $this->resemblesDN($key) ? - $this->sanitizeDN($item[$key][0]) - : $item[$key][0]; - } else { - $selection[$i][$key] = $this->sanitizeDN($item[$key]); - } - } - - } - $i++; - } else { - //tribute to case insensitivity - $key = mb_strtolower($attr[0], 'UTF-8'); - + foreach($attr as $key) { + $key = mb_strtolower($key, 'UTF-8'); if(isset($item[$key])) { - if($this->resemblesDN($key)) { - $selection[] = $this->sanitizeDN($item[$key]); + if($key !== 'dn') { + $selection[$i][$key] = $this->resemblesDN($key) ? + $this->sanitizeDN($item[$key]) + : $item[$key]; } else { - $selection[] = $item[$key]; + $selection[$i][$key] = $this->sanitizeDN($item[$key]); } } + } + $i++; } $findings = $selection; } diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 6ca84c8718fe..2b0b888c42e8 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -434,7 +434,7 @@ public function fetchGroups($dbKey, $confKey) { // detection will fail later $result = $this->access->searchGroups($filter, array('cn', 'dn'), $limit, $offset); foreach($result as $item) { - $groupNames[] = $item['cn']; + $groupNames[] = $item['cn'][0]; $groupEntries[] = $item; } $offset += $limit; From 5412899665ce44b4092e5a990e5e2efa5bc24a06 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 8 Oct 2015 20:32:15 +0200 Subject: [PATCH 06/13] adjust handling changed return array structure from search() and fetchList() --- apps/user_ldap/group_ldap.php | 2 +- apps/user_ldap/lib/access.php | 23 ++++++++++++++++++----- apps/user_ldap/lib/user/user.php | 17 ++++++++++------- apps/user_ldap/lib/wizard.php | 4 ++++ 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 4b45a157cd92..a7386534a75f 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -653,7 +653,7 @@ public function countUsersInGroup($gid, $search = '') { $this->access->connection->ldapLoginFilter, 'UTF-8'), $this->access->getFilterPartForUserSearch($search) )); - $ldap_users = $this->access->fetchListOfUsers($filter, 'dn'); + $ldap_users = $this->access->fetchListOfUsers($filter, 'dn', 1); if(count($ldap_users) < 1) { continue; } diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 0005bad2660f..585e0dc5b908 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -486,7 +486,7 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true) { /** * gives back the user names as they are used ownClod internally - * @param array $ldapUsers an array with the ldap Users result in style of array ( array ('dn' => foo, 'uid' => bar), ... ) + * @param array $ldapUsers as returned by fetchList() * @return array an array with the user names to use in ownCloud * * gives back the user names as they are used ownClod internally @@ -497,7 +497,7 @@ public function ownCloudUserNames($ldapUsers) { /** * gives back the group names as they are used ownClod internally - * @param array $ldapGroups an array with the ldap Groups result in style of array ( array ('dn' => foo, 'cn' => bar), ... ) + * @param array $ldapGroups as returned by fetchList() * @return array an array with the group names to use in ownCloud * * gives back the group names as they are used ownClod internally @@ -507,7 +507,7 @@ public function ownCloudGroupNames($ldapGroups) { } /** - * @param array $ldapObjects + * @param array $ldapObjects as returned by fetchList() * @param bool $isUsers * @return array */ @@ -520,7 +520,15 @@ private function ldap2ownCloudNames($ldapObjects, $isUsers) { $ownCloudNames = array(); foreach($ldapObjects as $ldapObject) { - $nameByLDAP = isset($ldapObject[$nameAttribute]) ? $ldapObject[$nameAttribute] : null; + $nameByLDAP = null; + if( isset($ldapObject[$nameAttribute]) + && is_array($ldapObject[$nameAttribute]) + && isset($ldapObject[$nameAttribute][0]) + ) { + // might be set, but not necessarily. if so, we use it. + $nameByLDAP = $ldapObject[$nameAttribute][0]; + } + $ocName = $this->dn2ocname($ldapObject['dn'], $nameByLDAP, $isUsers); if($ocName) { $ownCloudNames[] = $ocName; @@ -528,7 +536,9 @@ private function ldap2ownCloudNames($ldapObjects, $isUsers) { //cache the user names so it does not need to be retrieved //again later (e.g. sharing dialogue). $this->cacheUserExists($ocName); - $this->cacheUserDisplayName($ocName, $nameByLDAP); + if(!is_null($nameByLDAP)) { + $this->cacheUserDisplayName($ocName, $nameByLDAP); + } } } continue; @@ -1004,6 +1014,9 @@ private function search($filter, $base, $attr = null, $limit = null, $offset = n foreach($attr as $key) { $key = mb_strtolower($key, 'UTF-8'); if(isset($item[$key])) { + if(is_array($item[$key]) && isset($item[$key]['count'])) { + unset($item[$key]['count']); + } if($key !== 'dn') { $selection[$i][$key] = $this->resemblesDN($key) ? $this->sanitizeDN($item[$key]) diff --git a/apps/user_ldap/lib/user/user.php b/apps/user_ldap/lib/user/user.php index 1f48a480843e..637b95db8b41 100644 --- a/apps/user_ldap/lib/user/user.php +++ b/apps/user_ldap/lib/user/user.php @@ -147,21 +147,21 @@ public function processAttributes($ldapEntry) { //Quota $attr = strtolower($this->connection->ldapQuotaAttribute); if(isset($ldapEntry[$attr])) { - $this->updateQuota($ldapEntry[$attr]); + $this->updateQuota($ldapEntry[$attr][0]); } unset($attr); //Email $attr = strtolower($this->connection->ldapEmailAttribute); if(isset($ldapEntry[$attr])) { - $this->updateEmail($ldapEntry[$attr]); + $this->updateEmail($ldapEntry[$attr][0]); } unset($attr); //displayName $attr = strtolower($this->connection->ldapUserDisplayName); if(isset($ldapEntry[$attr])) { - $displayName = $ldapEntry[$attr]; + $displayName = $ldapEntry[$attr][0]; if(!empty($displayName)) { $this->storeDisplayName($displayName); $this->access->cacheUserDisplayName($this->getUsername(), $displayName); @@ -171,18 +171,20 @@ public function processAttributes($ldapEntry) { // LDAP Username, needed for s2s sharing if(isset($ldapEntry['uid'])) { - $this->storeLDAPUserName($ldapEntry['uid']); + $this->storeLDAPUserName($ldapEntry['uid'][0]); } else if(isset($ldapEntry['samaccountname'])) { - $this->storeLDAPUserName($ldapEntry['samaccountname']); + $this->storeLDAPUserName($ldapEntry['samaccountname'][0]); } + //homePath if(strpos($this->connection->homeFolderNamingRule, 'attr:') === 0) { $attr = strtolower(substr($this->connection->homeFolderNamingRule, strlen('attr:'))); if(isset($ldapEntry[$attr])) { $this->access->cacheUserHome( - $this->getUsername(), $this->getHomePath($ldapEntry[$attr])); + $this->getUsername(), $this->getHomePath($ldapEntry[$attr][0])); } } + //memberOf groups $cacheKey = 'getMemberOf'.$this->getUsername(); $groups = false; @@ -190,11 +192,12 @@ public function processAttributes($ldapEntry) { $groups = $ldapEntry['memberof']; } $this->connection->writeToCache($cacheKey, $groups); + //Avatar $attrs = array('jpegphoto', 'thumbnailphoto'); foreach ($attrs as $attr) { if(isset($ldapEntry[$attr])) { - $this->avatarImage = $ldapEntry[$attr]; + $this->avatarImage = $ldapEntry[$attr][0]; $this->updateAvatar(); break; } diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 2b0b888c42e8..7bf191e3be56 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -434,6 +434,10 @@ public function fetchGroups($dbKey, $confKey) { // detection will fail later $result = $this->access->searchGroups($filter, array('cn', 'dn'), $limit, $offset); foreach($result as $item) { + if(!isset($item['cn']) && !is_array($item['cn']) && !isset($item['cn'][0])) { + // just in case - no issue known + continue; + } $groupNames[] = $item['cn'][0]; $groupEntries[] = $item; } From ba13cecb76439a91ec82c70f9f81e7db82ea89f1 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 8 Oct 2015 21:18:34 +0200 Subject: [PATCH 07/13] treat dn as any other attribute when building the search() return array --- apps/user_ldap/group_ldap.php | 2 +- apps/user_ldap/lib/access.php | 6 +++--- apps/user_ldap/user_ldap.php | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index a7386534a75f..e2b4666a63ac 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -489,7 +489,7 @@ private function getGroupsByMember($dn, &$seen = null) { array($this->access->connection->ldapGroupDisplayName, 'dn')); if (is_array($groups)) { foreach ($groups as $groupobj) { - $groupDN = $groupobj['dn']; + $groupDN = $groupobj['dn'][0]; $allGroups[$groupDN] = $groupobj; $nestedGroups = $this->access->connection->ldapNestedGroups; if (!empty($nestedGroups)) { diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 585e0dc5b908..e23f0a039215 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -529,7 +529,7 @@ private function ldap2ownCloudNames($ldapObjects, $isUsers) { $nameByLDAP = $ldapObject[$nameAttribute][0]; } - $ocName = $this->dn2ocname($ldapObject['dn'], $nameByLDAP, $isUsers); + $ocName = $this->dn2ocname($ldapObject['dn'][0], $nameByLDAP, $isUsers); if($ocName) { $ownCloudNames[] = $ocName; if($isUsers) { @@ -696,7 +696,7 @@ public function batchApplyUserAttributes(array $ldapRecords){ continue; } - $ocName = $this->dn2ocname($userRecord['dn'], $userRecord[$displayNameAttribute]); + $ocName = $this->dn2ocname($userRecord['dn'][0], $userRecord[$displayNameAttribute]); if(!$ocName) { // no user name, skip. continue; @@ -1022,7 +1022,7 @@ private function search($filter, $base, $attr = null, $limit = null, $offset = n $this->sanitizeDN($item[$key]) : $item[$key]; } else { - $selection[$i][$key] = $this->sanitizeDN($item[$key]); + $selection[$i][$key] = [$this->sanitizeDN($item[$key])]; } } diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index 7d4d6cd2cb22..97fac7d85b54 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -101,7 +101,7 @@ public function checkPassword($uid, $password) { } catch(\Exception $e) { return false; } - $dn = $ldapRecord['dn']; + $dn = $ldapRecord['dn'][0]; $user = $this->access->userManager->get($dn); if(!$user instanceof User) { From f9be35c931b2399391616b1396db2f30378fa112 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 9 Oct 2015 14:12:59 +0200 Subject: [PATCH 08/13] fix primary group retrieval --- apps/user_ldap/group_ldap.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index e2b4666a63ac..48200bf57f41 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -245,7 +245,7 @@ public function primaryGroupID2Name($gid, $dn) { if(empty($result)) { return false; } - $dn = $result[0]; + $dn = $result[0]['dn'][0]; //and now the group name //NOTE once we have separate ownCloud group IDs and group names we can From cde21db620a0f6a3d43548adafce3dfa6412f30a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 9 Oct 2015 14:30:49 +0200 Subject: [PATCH 09/13] adjust tests --- apps/user_ldap/tests/group_ldap.php | 4 ++-- apps/user_ldap/tests/user_ldap.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php index 7007afd51621..9b7165001e12 100644 --- a/apps/user_ldap/tests/group_ldap.php +++ b/apps/user_ldap/tests/group_ldap.php @@ -144,7 +144,7 @@ public function testPrimaryGroupID2NameSuccess() { $access->expects($this->once()) ->method('searchGroups') - ->will($this->returnValue(array('cn=foo,dc=barfoo,dc=bar'))); + ->will($this->returnValue([['dn' => ['cn=foo,dc=barfoo,dc=bar']]])); $access->expects($this->once()) ->method('dn2groupname') @@ -220,7 +220,7 @@ public function testPrimaryGroupID2NameNoName() { $access->expects($this->once()) ->method('searchGroups') - ->will($this->returnValue(array('cn=foo,dc=barfoo,dc=bar'))); + ->will($this->returnValue([['dn' => ['cn=foo,dc=barfoo,dc=bar']]])); $access->expects($this->once()) ->method('dn2groupname') diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index 69a76c0b7acd..0f70c43fc11f 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -124,7 +124,7 @@ private function prepareAccessForCheckPassword(&$access, $noDisplayName = false) ->method('fetchListOfUsers') ->will($this->returnCallback(function($filter) { if($filter === 'roland') { - return array(array('dn' => 'dnOfRoland,dc=test')); + return array(array('dn' => ['dnOfRoland,dc=test'])); } return array(); })); @@ -133,7 +133,7 @@ private function prepareAccessForCheckPassword(&$access, $noDisplayName = false) ->method('fetchUsersByLoginName') ->will($this->returnCallback(function($uid) { if($uid === 'roland') { - return array(array('dn' => 'dnOfRoland,dc=test')); + return array(array('dn' => ['dnOfRoland,dc=test'])); } return array(); })); From bb973c2d763fd95be97e6c0240640f7c9e993154 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 9 Oct 2015 20:07:35 +0200 Subject: [PATCH 10/13] adjust fetchList with a single requested attribute accordingly --- apps/user_ldap/lib/access.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index e23f0a039215..483ca232277e 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -731,7 +731,9 @@ private function fetchList($list, $manyAttributes) { return $list; } else { $list = array_reduce($list, function($carry, $item) { - $carry[] = $item[0]; + $attribute = array_keys($item)[0]; + $carry[] = $item[$attribute][0]; + return $carry; }, array()); return array_unique($list, SORT_LOCALE_STRING); } From c5b28e079571cc0c6c63bba1226832ed46cd5159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Fortier?= Date: Thu, 15 Oct 2015 20:30:27 -0400 Subject: [PATCH 11/13] Revert "adjust to nested group fix This reverts commit 845485cfe, which fixes #19816 regression. --- apps/user_ldap/group_ldap.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 48200bf57f41..351fa1348680 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -31,7 +31,6 @@ use OCA\user_ldap\lib\Access; use OCA\user_ldap\lib\BackendUtility; -use OCA\user_ldap\lib\user\User; class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { protected $enabled = false; @@ -196,11 +195,7 @@ private function _getGroupDNsFromMemberOf($DN, &$seen = null) { return array(); } $seen[$DN] = 1; - $user = $this->access->userManager->get($DN); - if(!$user instanceof User) { - return array(); - } - $groups = $user->getMemberOfGroups(); + $groups = $this->access->readAttribute($DN, 'memberOf'); if (!is_array($groups)) { return array(); } From 643cba065abfa8e0253ecfe97948285dc5bfdea7 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 16 Oct 2015 10:35:40 +0200 Subject: [PATCH 12/13] Fix style --- apps/user_ldap/group_ldap.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 351fa1348680..5847ce64859c 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -207,7 +207,7 @@ private function _getGroupDNsFromMemberOf($DN, &$seen = null) { $subGroups = $this->_getGroupDNsFromMemberOf($group, $seen); $allGroups = array_merge($allGroups, $subGroups); } - } + } return $allGroups; } From cc9d5e44b159ab21ece870a51ab5a3a3484dea12 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 16 Oct 2015 10:35:47 +0200 Subject: [PATCH 13/13] Adjust unit tests --- apps/user_ldap/tests/group_ldap.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php index 9b7165001e12..5d2764a69d99 100644 --- a/apps/user_ldap/tests/group_ldap.php +++ b/apps/user_ldap/tests/group_ldap.php @@ -41,14 +41,9 @@ private function getAccessMock() { $connector = $this->getMock('\OCA\user_ldap\lib\Connection', $conMethods, array($lw, null, null)); - $um = new \OCA\user_ldap\lib\user\Manager( - $this->getMock('\OCP\IConfig'), - $this->getMock('\OCA\user_ldap\lib\FilesystemHelper'), - $this->getMock('\OCA\user_ldap\lib\LogWrapper'), - $this->getMock('\OCP\IAvatarManager'), - $this->getMock('\OCP\Image'), - $this->getMock('\OCP\IDBConnection') - ); + $um = $this->getMockBuilder('\OCA\user_ldap\lib\user\Manager') + ->disableOriginalConstructor() + ->getMock(); $access = $this->getMock('\OCA\user_ldap\lib\Access', $accMethods, array($connector, $lw, $um));