decouple userExists from userExistsOnLDAP check

allows to mark users as offline right away, avoids a gap of being not a user and causing weird side effects Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
nextcloud · Nov 29, 2019 · 9d3b107 · 9d3b107
1 parent 532f547
commit 9d3b107
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 15 deletions.
diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
@@ -175,6 +175,21 @@ public function update() {
 		}
 	}
 
+	/**
+	 * marks a user as deleted
+	 *
+	 * @throws \OCP\PreConditionNotMetException
+	 */
+	public function markUser() {
+		$curValue = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '0');
+		if($curValue === '1') {
+			// the user is already marked, do not write to DB again
+			return;
+		}
+		$this->config->setUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '1');
+		$this->config->setUserValue($this->getUsername(), 'user_ldap', 'foundDeleted', (string)time());
+	}
+
 	/**
 	 * processes results from LDAP for attributes as returned by getAttributesToRead()
 	 * @param array $ldapEntry the user entry as retrieved from LDAP

diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php
@@ -296,25 +296,35 @@ public function userExistsOnLDAP($user) {
 		if(is_null($user)) {
 			return false;
 		}
+		$uid = $user instanceof User ? $user->getUsername() : $user->getOCName();
+		$cacheKey = 'userExistsOnLDAP' . $uid;
+		$userExists = $this->access->connection->getFromCache($cacheKey);
+		if(!is_null($userExists)) {
+			return (bool)$userExists;
+		}
 
 		$dn = $user->getDN();
 		//check if user really still exists by reading its entry
 		if(!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapUserFilter))) {
 			try {
 				$uuid = $this->access->getUserMapper()->getUUIDByDN($dn);
 				if (!$uuid) {
+					$this->access->connection->writeToCache($cacheKey, false);
 					return false;
 				}
 				$newDn = $this->access->getUserDnByUuid($uuid);
 				//check if renamed user is still valid by reapplying the ldap filter
 				if ($newDn === $dn || !is_array($this->access->readAttribute($newDn, '', $this->access->connection->ldapUserFilter))) {
+					$this->access->connection->writeToCache($cacheKey, false);
 					return false;
 				}
 				$this->access->getUserMapper()->setDNbyUUID($newDn, $uuid);
+				$this->access->connection->writeToCache($cacheKey, true);
 				return true;
 			} catch (ServerNotAvailableException $e) {
 				throw $e;
 			} catch (\Exception $e) {
+				$this->access->connection->writeToCache($cacheKey, false);
 				return false;
 			}
 		}
@@ -323,6 +333,7 @@ public function userExistsOnLDAP($user) {
 			$user->unmark();
 		}
 
+		$this->access->connection->writeToCache($cacheKey, true);
 		return true;
 	}
 
@@ -345,15 +356,10 @@ public function userExists($uid) {
 				$this->access->connection->ldapHost, ILogger::DEBUG);
 			$this->access->connection->writeToCache('userExists'.$uid, false);
 			return false;
-		} else if($user instanceof OfflineUser) {
-			//express check for users marked as deleted. Returning true is
-			//necessary for cleanup
-			return true;
 		}
 
-		$result = $this->userExistsOnLDAP($user);
-		$this->access->connection->writeToCache('userExists'.$uid, $result);
-		return $result;
+		$this->access->connection->writeToCache('userExists'.$uid, true);
+		return true;
 	}
 
 	/**

diff --git a/apps/user_ldap/lib/User_Proxy.php b/apps/user_ldap/lib/User_Proxy.php
@@ -37,7 +37,8 @@
 use OCP\Notification\IManager as INotificationManager;
 
 class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP {
-	private $backends = array();
+	private $backends = [];
+	/** @var User_LDAP */
 	private $refBackend = null;
 
 	/**
@@ -49,9 +50,14 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface,
 	 * @param INotificationManager $notificationManager
 	 * @param IUserSession $userSession
 	 */
-	public function __construct(array $serverConfigPrefixes, ILDAPWrapper $ldap, IConfig $ocConfig,
-		INotificationManager $notificationManager, IUserSession $userSession,
-								UserPluginManager $userPluginManager) {
+	public function __construct(
+		array $serverConfigPrefixes,
+		ILDAPWrapper $ldap,
+		IConfig $ocConfig,
+		INotificationManager $notificationManager,
+		IUserSession $userSession,
+		UserPluginManager $userPluginManager
+	) {
 		parent::__construct($ldap);
 		foreach($serverConfigPrefixes as $configPrefix) {
 			$this->backends[$configPrefix] =
@@ -105,13 +111,13 @@ protected function callOnLastSeenOn($uid, $method, $parameters, $passOnWhen) {
 					&& method_exists($this->getAccess($prefix), $method)) {
 					$instance = $this->getAccess($prefix);
 				}
-				$result = call_user_func_array(array($instance, $method), $parameters);
+				$result = call_user_func_array([$instance, $method], $parameters);
 				if($result === $passOnWhen) {
 					//not found here, reset cache to null if user vanished
 					//because sometimes methods return false with a reason
 					$userExists = call_user_func_array(
-						array($this->backends[$prefix], 'userExists'),
-						array($uid)
+						[$this->backends[$prefix], 'userExistsOnLDAP'],
+						[$uid]
 					);
 					if(!$userExists) {
 						$this->writeToCache($cacheKey, null);
@@ -170,7 +176,22 @@ public function getUsers($search = '', $limit = 10, $offset = 0) {
 	 * @return boolean
 	 */
 	public function userExists($uid) {
-		return $this->handleRequest($uid, 'userExists', array($uid));
+		$existsOnLDAP = false;
+		$existsLocally = $this->handleRequest($uid, 'userExists', array($uid));
+		if($existsLocally) {
+			$existsOnLDAP = $this->userExistsOnLDAP($uid);
+		}
+		if($existsLocally && !$existsOnLDAP) {
+			try {
+				$user = $this->getLDAPAccess($uid)->userManager->get($uid);
+				if($user instanceof User) {
+					$user->markUser();
+				}
+			} catch (\Exception $e) {
+				// ignore
+			}
+		}
+		return $existsLocally;
 	}
 
 	/**