[stable12] LDAP: simplify returning the homePath and fixing #4117

apps/user_ldap/lib/User/Manager.php

@@ -134,6 +134,19 @@ private function createAndCache($dn, $uid) {
 		return $user;
 	}
 
+	/**
+	 * removes a user entry from the cache
+	 * @param $uid
+	 */
+	public function invalidate($uid) {
+		if(!isset($this->usersByUid[$uid])) {
+			return;
+		}
+		$dn = $this->usersByUid[$uid]->getDN();
+		unset($this->usersByUid[$uid]);
+		unset($this->usersByDN[$dn]);
+	}
+
 	/**
 	 * @brief checks whether the Access instance has been set
 	 * @throws \Exception if Access has not been set

apps/user_ldap/lib/User/OfflineUser.php

@@ -25,6 +25,8 @@
 namespace OCA\User_LDAP\User;
 
 use OCA\User_LDAP\Mapping\UserMapping;
+use OCP\IConfig;
+use OCP\IDBConnection;
 
 class OfflineUser {
 	/**
@@ -60,11 +62,11 @@ class OfflineUser {
 	 */
 	protected $hasActiveShares;
 	/**
-	 * @var \OCP\IConfig $config
+	 * @var IConfig $config
 	 */
 	protected $config;
 	/**
-	 * @var \OCP\IDBConnection $db
+	 * @var IDBConnection $db
 	 */
 	protected $db;
 	/**
@@ -74,11 +76,11 @@ class OfflineUser {
 
 	/**
 	 * @param string $ocName
-	 * @param \OCP\IConfig $config
-	 * @param \OCP\IDBConnection $db
+	 * @param IConfig $config
+	 * @param IDBConnection $db
 	 * @param \OCA\User_LDAP\Mapping\UserMapping $mapping
 	 */
-	public function __construct($ocName, \OCP\IConfig $config, \OCP\IDBConnection $db, UserMapping $mapping) {
+	public function __construct($ocName, IConfig $config, IDBConnection $db, UserMapping $mapping) {
 		$this->ocName = $ocName;
 		$this->config = $config;
 		$this->db = $db;

apps/user_ldap/lib/User_LDAP.php

@@ -41,19 +41,20 @@
 use OCA\User_LDAP\User\OfflineUser;
 use OCA\User_LDAP\User\User;
 use OCP\IConfig;
+use OCP\IUser;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Util;
 
 class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP {
-	/** @var string[] $homesToKill */
-	protected $homesToKill = array();
-
 	/** @var \OCP\IConfig */
 	protected $ocConfig;
 
 	/** @var INotificationManager */
 	protected $notificationManager;
 
+	/** @var string */
+	protected $currentUserInDeletionProcess;
+
 	/**
 	 * @param Access $access
 	 * @param \OCP\IConfig $ocConfig
@@ -63,6 +64,24 @@ public function __construct(Access $access, IConfig $ocConfig, INotificationMana
 		parent::__construct($access);
 		$this->ocConfig = $ocConfig;
 		$this->notificationManager = $notificationManager;
+		$this->registerHooks();
+	}
+
+	protected function registerHooks() {
+		Util::connectHook('OC_User','pre_deleteUser', $this, 'preDeleteUser');
+		Util::connectHook('OC_User','post_deleteUser', $this, 'postDeleteUser');
+	}
+
+	public function preDeleteUser(array $param) {
+		$user = $param[0];
+		if(!$user instanceof IUser) {
+			throw new \RuntimeException('IUser expected');
+		}
+		$this->currentUserInDeletionProcess = $user->getUID();
+	}
+
+	public function postDeleteUser() {
+		$this->currentUserInDeletionProcess = null;
 	}
 
 	/**
@@ -359,10 +378,8 @@ public function deleteUser($uid) {
 
 		//Get Home Directory out of user preferences so we can return it later,
 		//necessary for removing directories as done by OC_User.
-		$home = $this->ocConfig->getUserValue($uid, 'user_ldap', 'homePath', '');
-		$this->homesToKill[$uid] = $home;
 		$this->access->getUserMapper()->unmap($uid);
-
+		$this->access->userManager->invalidate($uid);
 		return true;
 	}
 
@@ -375,11 +392,6 @@ public function deleteUser($uid) {
 	 * @throws \Exception
 	 */
 	public function getHome($uid) {
-		if(isset($this->homesToKill[$uid]) && !empty($this->homesToKill[$uid])) {
-			//a deleted user who needs some clean up
-			return $this->homesToKill[$uid];
-		}
-
 		// user Exists check required as it is not done in user proxy!
 		if(!$this->userExists($uid)) {
 			return false;
@@ -391,16 +403,18 @@ public function getHome($uid) {
 			return $path;
 		}
 
+		// early return path if it is a deleted user
 		$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($this->currentUserInDeletionProcess === $user->getUID()) {
+				return $user->getHomePath();
+			} else {
+				throw new NoUserException($uid . ' is not a valid user anymore');
+			}
+		} else if ($user === null) {
+			throw new NoUserException($uid . ' is not a valid user anymore');
 		}
+
 		$path = $user->getHomePath();
 		$this->access->cacheUserHome($uid, $path);
 

apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserCleanUp.php

@@ -0,0 +1,102 @@
+<?php
+/**
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ *
+ * @author Arthur Schiwon <blizzz@arthur-schiwon.de>
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\User_LDAP\Tests\Integration\Lib\User;
+
+use OC\User\NoUserException;
+use OCA\User_LDAP\Jobs\CleanUp;
+use OCA\User_LDAP\Mapping\UserMapping;
+use OCA\User_LDAP\Tests\Integration\AbstractIntegrationTest;
+use OCA\User_LDAP\User_LDAP;
+
+require_once __DIR__ . '/../../Bootstrap.php';
+
+class IntegrationTestUserCleanUp extends AbstractIntegrationTest {
+	/** @var  UserMapping */
+	protected $mapping;
+
+	/**
+	 * prepares the LDAP environment and sets up a test configuration for
+	 * the LDAP backend.
+	 */
+	public function init() {
+		require(__DIR__ . '/../../setup-scripts/createExplicitUsers.php');
+		parent::init();
+		$this->mapping = new UserMapping(\OC::$server->getDatabaseConnection());
+		$this->mapping->clear();
+		$this->access->setUserMapper($this->mapping);
+
+		$userBackend  = new User_LDAP($this->access, \OC::$server->getConfig(), \OC::$server->getNotificationManager());
+		\OC_User::useBackend($userBackend);
+	}
+
+	/**
+	 * adds a map entry for the user, so we know the username
+	 *
+	 * @param $dn
+	 * @param $username
+	 */
+	private function prepareUser($dn, $username) {
+		// assigns our self-picked oc username to the dn
+		$this->mapping->map($dn, $username, 'fakeUUID-' . $username);
+	}
+
+	private function deleteUserFromLDAP($dn) {
+		$cr = $this->connection->getConnectionResource();
+		ldap_delete($cr, $dn);
+	}
+
+	/**
+	 * tests whether a display name consisting of two parts is created correctly
+	 *
+	 * @return bool
+	 */
+	protected function case1() {
+		$username = 'alice1337';
+		$dn = 'uid=alice,ou=Users,' . $this->base;
+		$this->prepareUser($dn, $username);
+
+		$user = \OC::$server->getUserManager()->get($username);
+		if($user === null) {
+			return false;
+		}
+
+		$this->deleteUserFromLDAP($dn);
+
+		$job = new CleanUp();
+		$job->run([]);
+
+		$user->delete();
+
+		return null === \OC::$server->getUserManager()->get($username);
+	}
+}
+
+/** @var string $host */
+/** @var int $port */
+/** @var string $adn */
+/** @var string $apwd */
+/** @var string $bdn */
+$test = new IntegrationTestUserCleanUp($host, $port, $adn, $apwd, $bdn);
+$test->init();
+$test->run();

apps/user_ldap/tests/User/UserTest.php

@@ -295,7 +295,7 @@ public function testUpdateQuotaToDefaultAllProvided() {
 	}
 
 	public function testUpdateQuotaToNoneAllProvided() {
-		list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		list($access, $connection) =