[stable29] fix(user_ldap): Avoid extra LDAP request when mapping a user for the first time

apps/user_ldap/lib/Access.php

@@ -168,6 +168,56 @@
 		return $this->connection;
 	}
 
+	/**
+	 * Reads several attributes for an LDAP record identified by a DN and a filter
+	 * No support for ranged attributes.
+	 *
+	 * @param string $dn the record in question
+	 * @param array $attrs the attributes that shall be retrieved
+	 *                     if empty, just check the record's existence
+	 * @param string $filter
+	 * @return array|false an array of values on success or an empty
+	 *                     array if $attr is empty, false otherwise
+	 * @throws ServerNotAvailableException
+	 */
+	public function readAttributes(string $dn, array $attrs, string $filter = 'objectClass=*'): array|false {
+		if (!$this->checkConnection()) {
+			$this->logger->warning(
+				'No LDAP Connector assigned, access impossible for readAttribute.',
+				['app' => 'user_ldap']
+			);
+			return false;
+		}
+		$cr = $this->connection->getConnectionResource();
+		$attrs = array_map(
+			fn (string $attr): string => mb_strtolower($attr, 'UTF-8'),
+			$attrs,
+		);
+
+		$values = [];
+		$record = $this->executeRead($dn, $attrs, $filter);
+		if (is_bool($record)) {
+			// when an exists request was run and it was successful, an empty
+			// array must be returned
+			return $record ? [] : false;
+		}
+
+		$result = [];
+		foreach ($attrs as $attr) {
+			$values = $this->extractAttributeValuesFromResult($record, $attr);
+			if (!empty($values)) {
+				$result[$attr] = $values;
+			}
+		}
+
+		if (!empty($result)) {
+			return $result;
+		}
+
+		$this->logger->debug('Requested attributes {attrs} not found for ' . $dn, ['app' => 'user_ldap', 'attrs' => $attrs]);
+		return false;
+	}
+
 	/**
 	 * reads a given attribute for an LDAP record identified by a DN
 	 *
@@ -248,9 +298,9 @@
 	 *                    returned data on a successful usual operation
 	 * @throws ServerNotAvailableException
 	 */
-	public function executeRead(string $dn, string $attribute, string $filter) {
+	public function executeRead(string $dn, string|array $attribute, string $filter) {
 		$dn = $this->helper->DNasBaseParameter($dn);
-		$rr = @$this->invokeLDAPMethod('read', $dn, $filter, [$attribute]);
+		$rr = @$this->invokeLDAPMethod('read', $dn, $filter, (is_string($attribute) ? [$attribute] : $attribute));
 		if (!$this->ldap->isResource($rr)) {
 			if ($attribute !== '') {
 				//do not throw this message on userExists check, irritates
@@ -472,15 +522,15 @@
 	 * @return string|false with with the name to use in Nextcloud
 	 * @throws \Exception
 	 */
-	public function dn2username($fdn, $ldapName = null) {
+	public function dn2username($fdn) {
 		//To avoid bypassing the base DN settings under certain circumstances
 		//with the group support, check whether the provided DN matches one of
 		//the given Bases
 		if (!$this->isDNPartOfBase($fdn, $this->connection->ldapBaseUsers)) {
 			return false;
 		}
 
-		return $this->dn2ocname($fdn, $ldapName, true);
+		return $this->dn2ocname($fdn, null, true);
 	}
 
 	/**
@@ -504,12 +554,8 @@
 		$newlyMapped = false;
 		if ($isUser) {
 			$mapper = $this->getUserMapper();
-			$nameAttribute = $this->connection->ldapUserDisplayName;
-			$filter = $this->connection->ldapUserFilter;
 		} else {
 			$mapper = $this->getGroupMapper();
-			$nameAttribute = $this->connection->ldapGroupDisplayName;
-			$filter = $this->connection->ldapGroupFilter;
 		}
 
 		//let's try to retrieve the Nextcloud name from the mappings table
@@ -523,6 +569,36 @@
 			return false;
 		}
 
+		if ($isUser) {
+			$nameAttribute = strtolower($this->connection->ldapUserDisplayName);
+			$filter = $this->connection->ldapUserFilter;
+			$uuidAttr = 'ldapUuidUserAttribute';
+			$uuidOverride = $this->connection->ldapExpertUUIDUserAttr;
+			$usernameAttribute = strtolower($this->connection->ldapExpertUsernameAttr);
+			$attributesToRead = [$nameAttribute,$usernameAttribute];
+			// TODO fetch also display name attributes and cache them if the user is mapped
+		} else {
+			$nameAttribute = strtolower($this->connection->ldapGroupDisplayName);
+			$filter = $this->connection->ldapGroupFilter;
+			$uuidAttr = 'ldapUuidGroupAttribute';
+			$uuidOverride = $this->connection->ldapExpertUUIDGroupAttr;
+			$attributesToRead = [$nameAttribute];
+		}
+
+		if ($this->detectUuidAttribute($fdn, $isUser, false, $record)) {
+			$attributesToRead[] = $this->connection->$uuidAttr;
+		}
+
+		if ($record === null) {
+			/* No record was passed, fetch it */
+			$record = $this->readAttributes($fdn, $attributesToRead, $filter);
+			if ($record === false) {
+				$this->logger->debug('Cannot read attributes for ' . $fdn . '. Skipping.', ['filter' => $filter]);
+				$intermediates[($isUser ? 'user-' : 'group-') . $fdn] = true;
+				return false;
+			}
+		}
+
 		//second try: get the UUID and check if it is known. Then, update the DN and return the name.
 		$uuid = $this->getUUID($fdn, $isUser, $record);
 		if (is_string($uuid)) {
@@ -537,20 +613,9 @@
 			return false;
 		}
 
-		if (is_null($ldapName)) {
-			$ldapName = $this->readAttribute($fdn, $nameAttribute, $filter);
-			if (!isset($ldapName[0]) || empty($ldapName[0])) {
-				$this->logger->debug('No or empty name for ' . $fdn . ' with filter ' . $filter . '.', ['app' => 'user_ldap']);
-				$intermediates[($isUser ? 'user-' : 'group-') . $fdn] = true;
-				return false;
-			}
-			$ldapName = $ldapName[0];
-		}
-
 		if ($isUser) {
-			$usernameAttribute = (string)$this->connection->ldapExpertUsernameAttr;
 			if ($usernameAttribute !== '') {
-				$username = $this->readAttribute($fdn, $usernameAttribute);
+				$username = $record[$usernameAttribute];
 				if (!isset($username[0]) || empty($username[0])) {
 					$this->logger->debug('No or empty username (' . $usernameAttribute . ') for ' . $fdn . '.', ['app' => 'user_ldap']);
 					return false;
@@ -572,6 +637,15 @@
 				return false;
 			}
 		} else {
+			if (is_null($ldapName)) {
+				$ldapName = $record[$nameAttribute];
+				if (!isset($ldapName[0]) || empty($ldapName[0])) {
+					$this->logger->debug('No or empty name for ' . $fdn . ' with filter ' . $filter . '.', ['app' => 'user_ldap']);
+					$intermediates['group-' . $fdn] = true;
+					return false;
+				}
+				$ldapName = $ldapName[0];
+			}
 			$intName = $this->sanitizeGroupIDCandidate($ldapName);
 		}
 
@@ -589,6 +663,7 @@
 			$this->connection->setConfiguration(['ldapCacheTTL' => $originalTTL]);
 			$newlyMapped = $this->mapAndAnnounceIfApplicable($mapper, $fdn, $intName, $uuid, $isUser);
 			if ($newlyMapped) {
+				$this->logger->debug('Mapped {fdn} as {name}', ['fdn' => $fdn,'name' => $intName]);
 				return $intName;
 			}
 		}
@@ -603,7 +678,6 @@
 						'fdn' => $fdn,
 						'altName' => $altName,
 						'intName' => $intName,
-						'app' => 'user_ldap',
 					]
 				);
 				$newlyMapped = true;
@@ -728,6 +802,7 @@
 	 */
 	public function cacheUserExists(string $ocName): void {
 		$this->connection->writeToCache('userExists' . $ocName, true);
+		$this->connection->writeToCache('userExistsOnLDAP' . $ocName, true);
 	}
 
 	/**
@@ -1743,10 +1818,10 @@
 	public function getUUID(string $dn, bool $isUser = true, ?array $ldapRecord = null) {
 		if ($isUser) {
 			$uuidAttr = 'ldapUuidUserAttribute';
 			$uuidOverride = $this->connection->ldapExpertUUIDUserAttr;
 		} else {
 			$uuidAttr = 'ldapUuidGroupAttribute';
 			$uuidOverride = $this->connection->ldapExpertUUIDGroupAttr;
 		}
 
 		$uuid = false;

apps/user_ldap/lib/Configuration.php

@@ -252,6 +252,27 @@ public function readConfiguration(): void {
 						break;
 					case 'ldapUserDisplayName2':
 					case 'ldapGroupDisplayName':
+					case 'ldapGidNumber':
+					case 'ldapGroupMemberAssocAttr':
+					case 'ldapQuotaAttribute':
+					case 'ldapEmailAttribute':
+					case 'ldapUuidUserAttribute':
+					case 'ldapUuidGroupAttribute':
+					case 'ldapExpertUsernameAttr':
+					case 'ldapExpertUUIDUserAttr':
+					case 'ldapExpertUUIDGroupAttr':
+					case 'ldapExtStorageHomeAttribute':
+					case 'ldapAttributePhone':
+					case 'ldapAttributeWebsite':
+					case 'ldapAttributeAddress':
+					case 'ldapAttributeTwitter':
+					case 'ldapAttributeFediverse':
+					case 'ldapAttributeOrganisation':
+					case 'ldapAttributeRole':
+					case 'ldapAttributeHeadline':
+					case 'ldapAttributeBiography':
+					case 'ldapAttributeBirthDate':
+					case 'ldapAttributeAnniversaryDate':
 						$readMethod = 'getLcValue';
 						break;
 					case 'ldapUserDisplayName':

apps/user_ldap/lib/User/Manager.php

@@ -128,6 +128,7 @@ public function invalidate($uid) {
 	/**
 	 * @brief checks whether the Access instance has been set
 	 * @throws \Exception if Access has not been set
+	 * @psalm-assert !null $this->access
 	 * @return null
 	 */
 	private function checkAccess() {
@@ -258,4 +259,37 @@ public function get($id) {
 
 		return $this->createInstancyByUserName($id);
 	}
+
+	/**
+	 * @brief Checks whether a User object by its DN or Nextcloud username exists
+	 * @param string $id the DN or username of the user
+	 * @throws \Exception when connection could not be established
+	 */
+	public function exists($id): bool {
+		$this->checkAccess();
+		$this->logger->debug('Checking if {id} exists', ['id' => $id]);
+		if (isset($this->usersByDN[$id])) {
+			return true;
+		} elseif (isset($this->usersByUid[$id])) {
+			return true;
+		}
+
+		if ($this->access->stringResemblesDN($id)) {
+			$this->logger->debug('{id} looks like a dn', ['id' => $id]);
+			$uid = $this->access->dn2username($id);
+			if ($uid !== false) {
+				return true;
+			}
+		}
+
+		// Most likely a uid. Check whether it is a deleted user
+		if ($this->isDeletedUser($id)) {
+			return true;
+		}
+		$dn = $this->access->username2dn($id);
+		if ($dn !== false) {
+			return true;
+		}
+		return false;
+	}
 }

apps/user_ldap/lib/User_LDAP.php

@@ -355,10 +355,9 @@ public function userExists($uid) {
 		if (!is_null($userExists)) {
 			return (bool)$userExists;
 		}
-		//getting dn, if false the user does not exist. If dn, he may be mapped only, requires more checking.
-		$user = $this->access->userManager->get($uid);
+		$userExists = $this->access->userManager->exists($uid);
 
-		if (is_null($user)) {
+		if (!$userExists) {
 			$this->logger->debug(
 				'No DN found for '.$uid.' on '.$this->access->connection->ldapHost,
 				['app' => 'user_ldap']
@@ -644,7 +643,6 @@ public function createUser($username, $password) {
 							$uuid,
 							true
 						);
-						$this->access->cacheUserExists($username);
 					} else {
 						$this->logger->warning(
 							'Failed to map created LDAP user with userid {userid}, because UUID could not be determined',

apps/user_ldap/tests/AccessTest.php

@@ -632,7 +632,8 @@ public function testFetchListOfUsers() {
 
 		$this->prepareMocksForSearchTests($base, $fakeConnection, $fakeSearchResultResource, $fakeLdapEntries);
 
-		$this->connection->expects($this->exactly($fakeLdapEntries['count']))
+		// Called twice per user, for userExists and userExistsOnLdap
+		$this->connection->expects($this->exactly(2 * $fakeLdapEntries['count']))
 			->method('writeToCache')
 			->with($this->stringStartsWith('userExists'), true);