[Pkg-owncloud-commits] [owncloud] 36/63: Backport of #21133 to stable8

David Prévot taffit at moszumanska.debian.org
Tue Dec 22 16:50:58 UTC 2015


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

taffit pushed a commit to branch stable8.0
in repository owncloud.

commit 5cab4a939e8b05a9559ba03ba54ad2a93e680cab
Author: Arthur Schiwon <blizzz at owncloud.com>
Date:   Fri Dec 11 00:12:41 2015 +0100

    Backport of #21133 to stable8
    
    throw NoUserException in getHome when the requested user does not exist anymore
    
    look for DN changes before marking a user as deleted
    
    adjust unit test
    
    unit test on getHome in combination with OfflineUser
    
    fix find DN by UUID for AD
    
    adjust dealing with search results due to different format in 8.0
---
 apps/user_ldap/lib/access.php                  | 95 ++++++++++++++++++++++++++
 apps/user_ldap/lib/mapping/abstractmapping.php | 12 +++-
 apps/user_ldap/tests/user_ldap.php             | 63 ++++++++++++++++-
 apps/user_ldap/user_ldap.php                   | 52 ++++++++++++--
 4 files changed, 212 insertions(+), 10 deletions(-)

diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index d6d9743..1f7b4c1 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -1131,6 +1131,54 @@ class Access extends LDAPUtility implements user\IUserTools {
 	}
 
 	/**
+	 * reverse lookup of a DN given a known UUID
+	 *
+	 * @param string $uuid
+	 * @return string
+	 * @throws \Exception
+	 */
+	public function getUserDnByUuid($uuid) {
+		$uuidOverride = $this->connection->ldapExpertUUIDUserAttr;
+		$filter       = $this->connection->ldapUserFilter;
+		$base         = $this->connection->ldapBaseUsers;
+
+		if($this->connection->ldapUuidUserAttribute === 'auto' && empty($uuidOverride)) {
+			// Sacrebleu! The UUID attribute is unknown :( We need first an
+			// existing DN to be able to reliably detect it.
+			$result = $this->search($filter, $base, ['dn'], 1);
+			if(!isset($result[0])) {
+				throw new \Exception('Cannot determine UUID attribute');
+			}
+			$dn = $result[0];
+			if(!$this->detectUuidAttribute($dn, true)) {
+				throw new \Exception('Cannot determine UUID attribute');
+			}
+		} else {
+			// The UUID attribute is either known or an override is given.
+			// By calling this method we ensure that $this->connection->$uuidAttr
+			// is definitely set
+			if(!$this->detectUuidAttribute('', true)) {
+				throw new \Exception('Cannot determine UUID attribute');
+			}
+		}
+
+		$uuidAttr = $this->connection->ldapUuidUserAttribute;
+		if($uuidAttr === 'guid' || $uuidAttr === 'objectguid') {
+			$uuid = $this->formatGuid2ForFilterUser($uuid);
+		}
+
+		$filter = $uuidAttr . '=' . $uuid;
+		$result = $this->searchUsers($filter, ['dn'], 2);
+		if(is_array($result) && isset($result[0]) && count($result) === 1) {
+			// we put the count into account to make sure that this is
+			// really unique
+			return $result[0];
+		}
+
+		throw new \Exception('Cannot determine UUID attribute');
+	}
+
+	/**
 	 * auto-detects the directory's UUID attribute
 	 * @param string $dn a known DN used to check against
 	 * @param bool $isUser
@@ -1233,6 +1281,53 @@ class Access extends LDAPUtility implements user\IUserTools {
 	}
 
 	/**
+	 * the first three blocks of the string-converted GUID happen to be in
+	 * reverse order. In order to use it in a filter, this needs to be
+	 * corrected. Furthermore the dashes need to be replaced and \\ preprended
+	 * to every two hax figures.
+	 *
+	 * If an invalid string is passed, it will be returned without change.
+	 *
+	 * @param string $guid
+	 * @return string
+	 */
+	public function formatGuid2ForFilterUser($guid) {
+		if(!is_string($guid)) {
+			throw new \InvalidArgumentException('String expected');
+		}
+		$blocks = explode('-', $guid);
+		if(count($blocks) !== 5) {
+			/*
+			 * Why not throw an Exception instead? This method is a utility
+			 * called only when trying to figure out whether a "missing" known
+			 * LDAP user was or was not renamed on the LDAP server. And this
+			 * even on the use case that a reverse lookup is needed (UUID known,
+			 * not DN), i.e. when finding users (search dialog, users page,
+			 * login, …) this will not be fired. This occurs only if shares from
+			 * a users are supposed to be mounted who cannot be found. Throwing
+			 * an exception here would kill the experience for a valid, acting
+			 * user. Instead we write a log message.
+			 */
+			\OC::$server->getLogger()->info(
+				'Passed string does not resemble a valid GUID. Known UUID ' .
+				'({uuid}) probably does not match UUID configuration.',
+				[ 'app' => 'user_ldap', 'uuid' => $guid ]
+			);
+			return $guid;
+		}
+		for($i=0; $i < 3; $i++) {
+			$pairs = str_split($blocks[$i], 2);
+			$pairs = array_reverse($pairs);
+			$blocks[$i] = implode('', $pairs);
+		}
+		for($i=0; $i < 5; $i++) {
+			$pairs = str_split($blocks[$i], 2);
+			$blocks[$i] = '\\' . implode('\\', $pairs);
+		}
+		return implode('', $blocks);
+	}
+
+	/**
 	 * gets a SID of the domain of the given dn
 	 * @param string $dn
 	 * @return string|bool
diff --git a/apps/user_ldap/lib/mapping/abstractmapping.php b/apps/user_ldap/lib/mapping/abstractmapping.php
index cb9db83..ed31202 100644
--- a/apps/user_ldap/lib/mapping/abstractmapping.php
+++ b/apps/user_ldap/lib/mapping/abstractmapping.php
@@ -144,7 +144,7 @@ abstract class AbstractMapping {
 	}
 
 	/**
-	 * Gets the name based on the provided LDAP DN.
+	 * Gets the name based on the provided LDAP UUID.
 	 * @param string $uuid
 	 * @return string|false
 	 */
@@ -153,6 +153,16 @@ abstract class AbstractMapping {
 	}
 
 	/**
+	 * Gets the UUID based on the provided LDAP DN
+	 * @param string $dn
+	 * @return false|string
+	 * @throws \Exception
+	 */
+	public function getUUIDByDN($dn) {
+		return $this->getXbyY('directory_uuid', 'ldap_dn', $dn);
+	}
+
+	/**
 	 * gets a piece of the mapping list
 	 * @param int $offset
 	 * @param int $limit
diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php
index f8ac4a1..59f5e8f 100644
--- a/apps/user_ldap/tests/user_ldap.php
+++ b/apps/user_ldap/tests/user_ldap.php
@@ -57,14 +57,25 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 									$conMethods,
 									array($lw, null, null));
 
-		$um = new \OCA\user_ldap\lib\user\Manager(
-				$this->getMock('\OCP\IConfig'),
+
+		$offlineUser = $this->getMockBuilder('\OCA\user_ldap\lib\user\OfflineUser')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$um = $this->getMockBuilder('\OCA\user_ldap\lib\user\Manager')
+			->setMethods(['getDeletedUser'])
+			->setConstructorArgs([
 				$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')
-			);
+			  ])
+			->getMock();
+
+		$um->expects($this->any())
+			->method('getDeletedUser')
+			->will($this->returnValue($offlineUser));
 
 		$access = $this->getMock('\OCA\user_ldap\lib\Access',
 								 $accMethods,
@@ -643,6 +654,44 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 		$this->assertFalse($result);
 	}
 
+	/**
+	 * @expectedException \OC\User\NoUserException
+	 */
+	public function testGetHomeDeletedUser() {
+		$access = $this->getAccessMock();
+		$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
+		$this->prepareMockForUserExists($access);
+
+		$access->connection->expects($this->any())
+				->method('__get')
+				->will($this->returnCallback(function($name) {
+					if($name === 'homeFolderNamingRule') {
+						return 'attr:testAttribute';
+					}
+					return null;
+				}));
+
+		$access->expects($this->any())
+				->method('readAttribute')
+				->will($this->returnValue([]));
+
+		$userMapper = $this->getMockBuilder('\OCA\User_LDAP\Mapping\UserMapping')
+				->disableOriginalConstructor()
+				->getMock();
+
+		$access->expects($this->any())
+				->method('getUserMapper')
+				->will($this->returnValue($userMapper));
+
+		$this->configMock->expects($this->any())
+			->method('getUserValue')
+			->will($this->returnValue(true));
+
+		//no path at all – triggers OC default behaviour
+		$result = $backend->getHome('newyorker');
+		$this->assertFalse($result);
+	}
+
 	private function prepareAccessForGetDisplayName(&$access) {
 		$access->connection->expects($this->any())
 			   ->method('__get')
@@ -668,6 +717,14 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 							return false;
 				   }
 			   }));
+
+		$userMapper = $this->getMockBuilder('\OCA\User_LDAP\Mapping\UserMapping')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$access->expects($this->any())
+			->method('getUserMapper')
+			->will($this->returnValue($userMapper));
 	}
 
 	public function testGetDisplayName() {
diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php
index 6f00ba1..3fe00ac 100644
--- a/apps/user_ldap/user_ldap.php
+++ b/apps/user_ldap/user_ldap.php
@@ -25,6 +25,7 @@
 
 namespace OCA\user_ldap;
 
+use OC\User\NoUserException;
 use OCA\user_ldap\lib\BackendUtility;
 use OCA\user_ldap\lib\Access;
 use OCA\user_ldap\lib\user\OfflineUser;
@@ -159,15 +160,18 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 
 	/**
 	 * checks whether a user is still available on LDAP
+	 *
 	 * @param string|\OCA\User_LDAP\lib\user\User $user either the ownCloud user
 	 * name or an instance of that user
 	 * @return bool
+	 * @throws \Exception
+	 * @throws \OC\ServerNotAvailableException
 	 */
 	public function userExistsOnLDAP($user) {
 		if(is_string($user)) {
 			$user = $this->access->userManager->get($user);
 		}
-		if(!$user instanceof User) {
+		if(is_null($user)) {
 			return false;
 		}
 
@@ -178,7 +182,22 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 			if(is_null($lcr)) {
 				throw new \Exception('No LDAP Connection to server ' . $this->access->connection->ldapHost);
 			}
-			return false;
+
+			try {
+				$uuid = $this->access->getUserMapper()->getUUIDByDN($dn);
+				if(!$uuid) {
+					return false;
+				}
+				$newDn = $this->access->getUserDnByUuid($uuid);
+				$this->access->getUserMapper()->setDNbyUUID($newDn, $uuid);
+				return true;
+			} catch (\Exception $e) {
+				return false;
+			}
+		}
+
+		if($user instanceof OfflineUser) {
+			$user->unmark();
 		}
 
 		return true;
@@ -243,10 +262,13 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 	}
 
 	/**
-	* get the user's home directory
-	* @param string $uid the username
-	* @return string|bool
-	*/
+	 * get the user's home directory
+	 *
+	 * @param string $uid the username
+	 * @return bool|string
+	 * @throws NoUserException
+	 * @throws \Exception
+	 */
 	public function getHome($uid) {
 		if(isset($this->homesToKill[$uid]) && !empty($this->homesToKill[$uid])) {
 			//a deleted user who needs some clean up
@@ -262,6 +284,24 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 		if($this->access->connection->isCached($cacheKey)) {
 			return $this->access->connection->getFromCache($cacheKey);
 		}
+
+		$user = $this->access->userManager->get($uid);
+		if(is_null($user) || ($user instanceof OfflineUser && !$this->userExistsOnLDAP($user->getOCName()))) {
+			throw new NoUserException($uid . ' is not a valid user anymore');
+		}
+		if($user instanceof OfflineUser) {
+			// apparently this user survived the userExistsOnLDAP check,
+			// we request the user instance again in order to retrieve a User
+			// instance instead
+			$user = $this->access->userManager->get($uid);
+			if($user instanceof OfflineUser) {
+				// should not happen, still be better safe than sorry
+				$path = $user->getHomePath();
+				$this->access->connection->writeToCache($cacheKey, $path);
+				return $path;
+			}
+		}
+
 		if(strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0 &&
 			$this->access->connection->homeFolderNamingRule !== 'attr:') {
 			$attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:'));

-- 
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