Merge pull request #5689 from nextcloud/fix-4117

LDAP: simplify returning the homePath and fixing #4117

3 files changed, 52 insertions, 23 deletions

diff --git a/apps/user_ldap/lib/User/Manager.php b/apps/user_ldap/lib/User/Manager.php
index ea4d071b646..743456d68e2 100644
--- a/apps/user_ldap/lib/User/Manager.php
+++ b/apps/user_ldap/lib/User/Manager.php
@@ -135,6 +135,19 @@ class Manager {
 	}
 
 	/**
+	 * 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
 	 * @return null
diff --git a/apps/user_ldap/lib/User/OfflineUser.php b/apps/user_ldap/lib/User/OfflineUser.php
index 0e60a29514e..942eee84cb7 100644
--- a/apps/user_ldap/lib/User/OfflineUser.php
+++ b/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;
diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php
index 3cc2fec740e..6c438391380 100644
--- a/apps/user_ldap/lib/User_LDAP.php
+++ b/apps/user_ldap/lib/User_LDAP.php
@@ -41,19 +41,20 @@ use OCA\User_LDAP\Exceptions\NotOnLDAP;
 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 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 		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 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 
 		//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 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 	 * @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 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 			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);