Merge pull request #18469 from owncloud/ldap-batch-read-attrs-ng

read all relevant user attributes on login and user search, in one qu…

apps/user_ldap/group_ldap.php

@@ -31,6 +31,7 @@ namespace OCA\user_ldap;
 
 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 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 			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();
 		}

apps/user_ldap/lib/access.php

@@ -536,6 +536,16 @@ class Access extends LDAPUtility implements user\IUserTools {
 		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
@@ -656,7 +666,24 @@ class Access extends LDAPUtility implements user\IUserTools {
 	 * @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);
+		}
 	}
 
 	/**

apps/user_ldap/lib/user/manager.php

@@ -126,6 +126,43 @@ class Manager {
 		}
 	}
 
+	/**
+	 * 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

apps/user_ldap/lib/user/user.php

@@ -138,6 +138,69 @@ class User {
 		}
 	}
 
+	/**
+	 * 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 @@ class User {
 		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 @@ class User {
 	 * @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 @@ class User {
 	}
 
 	/**
-	 * @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)) {

apps/user_ldap/tests/access.php

@@ -29,7 +29,7 @@ use \OCA\user_ldap\lib\Connection;
 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 @@ class Test_Access extends \Test\TestCase {
 	}
 
 	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 @@ class Test_Access extends \Test\TestCase {
 	}
 
 	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 @@ class Test_Access extends \Test\TestCase {
 	}
 
 	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 @@ class Test_Access extends \Test\TestCase {
 
 	/** @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 @@ class Test_Access extends \Test\TestCase {
 	}
 
 	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 @@ class Test_Access extends \Test\TestCase {
 	}
 
 	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 @@ class Test_Access extends \Test\TestCase {
 	}
 
 	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 @@ class Test_Access extends \Test\TestCase {
 	}
 
 	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 @@ class Test_Access extends \Test\TestCase {
 	}
 
 	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 @@ class Test_Access extends \Test\TestCase {
 			$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);
+	}
 }

apps/user_ldap/tests/group_ldap.php

@@ -53,6 +53,10 @@ class Test_Group_Ldap extends \Test\TestCase {
 								 $accMethods,
 								 array($connector, $lw, $um));
 
+		$access->expects($this->any())
+			->method('getConnection')
+			->will($this->returnValue($connector));
+
 		return $access;
 	}
 
@@ -391,7 +395,7 @@ class Test_Group_Ldap extends \Test\TestCase {
 
 		$access->connection->hasPrimaryGroups = false;
 
-		$access->expects($this->once())
+		$access->expects($this->any())
 			->method('username2dn')
 			->will($this->returnValue($dn));
 

apps/user_ldap/tests/user/manager.php

@@ -37,6 +37,16 @@ class Test_User_Manager extends \Test\TestCase {
 		$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 @@ class Test_User_Manager extends \Test\TestCase {
 		$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));
+	}
+
 }

apps/user_ldap/tests/user/user.php

@@ -221,7 +221,7 @@ class Test_User_User extends \Test\TestCase {
 		$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 @@ class Test_User_User extends \Test\TestCase {
 			->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 @@ class Test_User_User extends \Test\TestCase {
 			->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 @@ class Test_User_User extends \Test\TestCase {
 		//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();
+	}
 }

apps/user_ldap/tests/user_ldap.php

@@ -34,6 +34,7 @@ use \OCA\user_ldap\lib\ILDAPWrapper;
 class Test_User_Ldap_Direct extends \Test\TestCase {
 	protected $backend;
 	protected $access;
+	protected $configMock;
 
 	protected function setUp() {
 		parent::setUp();
@@ -61,8 +62,9 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 									$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 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 		$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 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 						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);
 	}
 
 	/**

apps/user_ldap/user_ldap.php

@@ -98,9 +98,8 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 	 */
 	public function getLDAPUserByLoginName($loginName) {
 		//find out dn of the user name
-		$attrs = array($this->access->connection->ldapUserDisplayName, 'dn',
-			'uid', 'samaccountname');
-		$users = $this->access->fetchUsersByLoginName($loginName, $attrs);
+		$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.');
 		}
@@ -137,16 +136,9 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 				return false;
 			}
 
+			$this->access->cacheUserExists($user->getUsername());
+			$user->processAttributes($ldapRecord);
 			$user->markLogin();
-			if(isset($ldapRecord[$this->access->connection->ldapUserDisplayName])) {
-				$dpn = $ldapRecord[$this->access->connection->ldapUserDisplayName];
-				$user->storeDisplayName($dpn);
-			}
-			if(isset($ldapRecord['uid'])) {
-				$user->storeLDAPUserName($ldapRecord['uid']);
-			} else if(isset($ldapRecord['samaccountname'])) {
-				$user->storeLDAPUserName($ldapRecord['samaccountname']);
-			}
 
 			return $user->getUsername();
 		}
@@ -188,7 +180,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 		//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);
@@ -302,44 +294,12 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 		if($this->access->connection->isCached($cacheKey)) {
 			return $this->access->connection->getFromCache($cacheKey);
 		}
-		if(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($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;
-			}
-			if($this->ocConfig->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: ' . $uid);
-			}
-		}
 
-		//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;
 	}
 
 	/**