[Pkg-owncloud-commits] [owncloud] 01/27: Backport #18469 (read all relevant user attributes on login and user search, in one query)

David Prévot taffit at moszumanska.debian.org
Wed Oct 28 17:03:04 UTC 2015


This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to annotated tag v8.1.4RC2
in repository owncloud.

commit c1953f96760163efb62f0bb8e244016d8e32dba7
Author: Arthur Schiwon <blizzz at owncloud.com>
Date:   Fri Aug 21 00:55:42 2015 +0200

    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 ee3191b..4b45a15 100644
--- a/apps/user_ldap/group_ldap.php
+++ b/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();
 		}
diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index 44237b5..54f959b 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -537,6 +537,16 @@ class Access extends LDAPUtility implements user\IUserTools {
 	}
 
 	/**
+	 * 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 @@ 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);
+		}
 	}
 
 	/**
diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php
index b70e057..86f0b89 100644
--- a/apps/user_ldap/lib/user/manager.php
+++ b/apps/user_ldap/lib/user/manager.php
@@ -127,6 +127,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
 	 * @return bool
diff --git a/apps/user_ldap/lib/user/user.php b/apps/user_ldap/lib/user/user.php
index ac5d8f5..6498cdf 100644
--- a/apps/user_ldap/lib/user/user.php
+++ b/apps/user_ldap/lib/user/user.php
@@ -139,6 +139,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
 	 */
@@ -155,6 +218,68 @@ class User {
 	}
 
 	/**
+	 * 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)) {
diff --git a/apps/user_ldap/tests/access.php b/apps/user_ldap/tests/access.php
index c4cd203..5bf1a65 100644
--- a/apps/user_ldap/tests/access.php
+++ b/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);
+	}
 }
diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php
index 805238e..7007afd 100644
--- a/apps/user_ldap/tests/group_ldap.php
+++ b/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));
 
diff --git a/apps/user_ldap/tests/user/manager.php b/apps/user_ldap/tests/user/manager.php
index d659323..98e4863 100644
--- a/apps/user_ldap/tests/user/manager.php
+++ b/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));
+	}
+
 }
diff --git a/apps/user_ldap/tests/user/user.php b/apps/user_ldap/tests/user/user.php
index 80baafc..1c41eb7 100644
--- a/apps/user_ldap/tests/user/user.php
+++ b/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 at 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 2b99e1c..69a76c0 100644
--- a/apps/user_ldap/tests/user_ldap.php
+++ b/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);
 	}
 
 	/**
diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php
index caff30a..f38cac2 100644
--- a/apps/user_ldap/user_ldap.php
+++ b/apps/user_ldap/user_ldap.php
@@ -101,16 +101,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($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 @@ 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);
@@ -266,39 +259,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) {
-			$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;
 	}
 
 	/**

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git



More information about the Pkg-owncloud-commits mailing list