Merge pull request #31073 from nextcloud/fix/user_ldap-fix-access-typing

Improve typing in OCA\User_LDAP\Access and reduce psalm errors

3 files changed, 47 insertions, 69 deletions

diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
index 65087040cc4..ed5e5bff9ce 100644
--- a/apps/user_ldap/lib/Access.php
+++ b/apps/user_ldap/lib/Access.php
@@ -52,7 +52,6 @@ use OC\ServerNotAvailableException;
 use OCA\User_LDAP\Exceptions\ConstraintViolationException;
 use OCA\User_LDAP\Exceptions\NoMoreResults;
 use OCA\User_LDAP\Mapping\AbstractMapping;
-use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\User\OfflineUser;
 use OCP\HintException;
@@ -74,17 +73,16 @@ class Access extends LDAPUtility {
 	public $connection;
 	/** @var Manager */
 	public $userManager;
-	//never ever check this var directly, always use getPagedSearchResultState
-	protected $pagedSearchedSuccessful;
-
 	/**
-	 * @var UserMapping $userMapper
+	 * never ever check this var directly, always use getPagedSearchResultState
+	 * @var ?bool
 	 */
+	protected $pagedSearchedSuccessful;
+
+	/** @var ?AbstractMapping */
 	protected $userMapper;
 
-	/**
-	 * @var AbstractMapping $userMapper
-	 */
+	/** @var ?AbstractMapping */
 	protected $groupMapper;
 
 	/**
@@ -121,17 +119,15 @@ class Access extends LDAPUtility {
 
 	/**
 	 * sets the User Mapper
-	 *
-	 * @param AbstractMapping $mapper
 	 */
-	public function setUserMapper(AbstractMapping $mapper) {
+	public function setUserMapper(AbstractMapping $mapper): void {
 		$this->userMapper = $mapper;
 	}
 
 	/**
 	 * @throws \Exception
 	 */
-	public function getUserMapper(): UserMapping {
+	public function getUserMapper(): AbstractMapping {
 		if (is_null($this->userMapper)) {
 			throw new \Exception('UserMapper was not assigned to this Access instance.');
 		}
@@ -140,20 +136,17 @@ class Access extends LDAPUtility {
 
 	/**
 	 * sets the Group Mapper
-	 *
-	 * @param AbstractMapping $mapper
 	 */
-	public function setGroupMapper(AbstractMapping $mapper) {
+	public function setGroupMapper(AbstractMapping $mapper): void {
 		$this->groupMapper = $mapper;
 	}
 
 	/**
 	 * returns the Group Mapper
 	 *
-	 * @return AbstractMapping
 	 * @throws \Exception
 	 */
-	public function getGroupMapper() {
+	public function getGroupMapper(): AbstractMapping {
 		if (is_null($this->groupMapper)) {
 			throw new \Exception('GroupMapper was not assigned to this Access instance.');
 		}
@@ -343,8 +336,8 @@ class Access extends LDAPUtility {
 	public function extractRangeData($result, $attribute) {
 		$keys = array_keys($result);
 		foreach ($keys as $key) {
-			if ($key !== $attribute && strpos($key, $attribute) === 0) {
-				$queryData = explode(';', $key);
+			if ($key !== $attribute && strpos((string)$key, $attribute) === 0) {
+				$queryData = explode(';', (string)$key);
 				if (strpos($queryData[1], 'range=') === 0) {
 					$high = substr($queryData[1], 1 + strpos($queryData[1], '-'));
 					$data = [
@@ -669,12 +662,10 @@ class Access extends LDAPUtility {
 	}
 
 	/**
-	 * @param array $ldapObjects as returned by fetchList()
-	 * @param bool $isUsers
-	 * @return array
+	 * @param array[] $ldapObjects as returned by fetchList()
 	 * @throws \Exception
 	 */
-	private function ldap2NextcloudNames($ldapObjects, $isUsers) {
+	private function ldap2NextcloudNames(array $ldapObjects, bool $isUsers): array {
 		if ($isUsers) {
 			$nameAttribute = $this->connection->ldapUserDisplayName;
 			$sndAttribute = $this->connection->ldapUserDisplayName2;
@@ -786,7 +777,7 @@ class Access extends LDAPUtility {
 	 * Instead of using this method directly, call
 	 * createAltInternalOwnCloudName($name, true)
 	 */
-	private function _createAltInternalOwnCloudNameForUsers($name) {
+	private function _createAltInternalOwnCloudNameForUsers(string $name) {
 		$attempts = 0;
 		//while loop is just a precaution. If a name is not generated within
 		//20 attempts, something else is very wrong. Avoids infinite loop.
@@ -813,8 +804,8 @@ class Access extends LDAPUtility {
 	 * numbering, e.g. Developers_42 when there are 41 other groups called
 	 * "Developers"
 	 */
-	private function _createAltInternalOwnCloudNameForGroups($name) {
-		$usedNames = $this->groupMapper->getNamesBySearch($name, "", '_%');
+	private function _createAltInternalOwnCloudNameForGroups(string $name) {
+		$usedNames = $this->getGroupMapper()->getNamesBySearch($name, "", '_%');
 		if (!$usedNames || count($usedNames) === 0) {
 			$lastNo = 1; //will become name_2
 		} else {
@@ -843,10 +834,10 @@ class Access extends LDAPUtility {
 	 * creates a unique name for internal Nextcloud use.
 	 *
 	 * @param string $name the display name of the object
-	 * @param boolean $isUser whether name should be created for a user (true) or a group (false)
+	 * @param bool $isUser whether name should be created for a user (true) or a group (false)
 	 * @return string|false with with the name to use in Nextcloud or false if unsuccessful
 	 */
-	private function createAltInternalOwnCloudName($name, $isUser) {
+	private function createAltInternalOwnCloudName(string $name, bool $isUser) {
 		// ensure there is space for the "_1234" suffix
 		if (strlen($name) > 59) {
 			$name = substr($name, 0, 59);
@@ -879,7 +870,7 @@ class Access extends LDAPUtility {
 	 * utilizing the login filter.
 	 *
 	 * @param string $loginName
-	 * @return int
+	 * @return false|int
 	 */
 	public function countUsersByLoginName($loginName) {
 		$loginName = $this->escapeFilterPart($loginName);
@@ -954,7 +945,7 @@ class Access extends LDAPUtility {
 	 * @param string|string[] $attr
 	 * @param int $limit
 	 * @param int $offset
-	 * @return array
+	 * @return array[]
 	 */
 	public function fetchListOfGroups($filter, $attr, $limit = null, $offset = null) {
 		$groupRecords = $this->searchGroups($filter, $attr, $limit, $offset);
@@ -965,7 +956,7 @@ class Access extends LDAPUtility {
 		}, []);
 		$idsByDn = $this->groupMapper->getListOfIdsByDn($listOfDNs);
 
-		array_walk($groupRecords, function ($record) use ($idsByDn) {
+		array_walk($groupRecords, function (array $record) use ($idsByDn) {
 			$newlyMapped = false;
 			$gid = $idsByDn[$record['dn'][0]] ?? null;
 			if ($gid === null) {
@@ -978,27 +969,17 @@ class Access extends LDAPUtility {
 		return $this->fetchList($groupRecords, $this->manyAttributes($attr));
 	}
 
-	/**
-	 * @param array $list
-	 * @param bool $manyAttributes
-	 * @return array
-	 */
-	private function fetchList($list, $manyAttributes) {
-		if (is_array($list)) {
-			if ($manyAttributes) {
-				return $list;
-			} else {
-				$list = array_reduce($list, function ($carry, $item) {
-					$attribute = array_keys($item)[0];
-					$carry[] = $item[$attribute][0];
-					return $carry;
-				}, []);
-				return array_unique($list, SORT_LOCALE_STRING);
-			}
+	private function fetchList(array $list, bool $manyAttributes): array {
+		if ($manyAttributes) {
+			return $list;
+		} else {
+			$list = array_reduce($list, function ($carry, $item) {
+				$attribute = array_keys($item)[0];
+				$carry[] = $item[$attribute][0];
+				return $carry;
+			}, []);
+			return array_unique($list, SORT_LOCALE_STRING);
 		}
-
-		//error cause actually, maybe throw an exception in future.
-		return [];
 	}
 
 	/**
@@ -1518,7 +1499,7 @@ class Access extends LDAPUtility {
 	 * @param string $operator either & or |
 	 * @return string the combined filter
 	 */
-	private function combineFilter($filters, $operator) {
+	private function combineFilter(array $filters, string $operator): string {
 		$combinedFilter = '(' . $operator;
 		foreach ($filters as $filter) {
 			if ($filter !== '' && $filter[0] !== '(') {
@@ -1559,12 +1540,12 @@ class Access extends LDAPUtility {
 	 * string into single words
 	 *
 	 * @param string $search the search term
-	 * @param string[] $searchAttributes needs to have at least two attributes,
+	 * @param string[]|null|'' $searchAttributes needs to have at least two attributes,
 	 * otherwise it does not make sense :)
 	 * @return string the final filter part to use in LDAP searches
 	 * @throws DomainException
 	 */
-	private function getAdvancedFilterPartForSearch($search, $searchAttributes) {
+	private function getAdvancedFilterPartForSearch(string $search, $searchAttributes): string {
 		if (!is_array($searchAttributes) || count($searchAttributes) < 2) {
 			throw new DomainException('searchAttributes must be an array with at least two string');
 		}
@@ -1586,12 +1567,12 @@ class Access extends LDAPUtility {
 	 * creates a filter part for searches
 	 *
 	 * @param string $search the search term
-	 * @param string[]|null $searchAttributes
+	 * @param string[]|null|'' $searchAttributes
 	 * @param string $fallbackAttribute a fallback attribute in case the user
 	 * did not define search attributes. Typically the display name attribute.
 	 * @return string the final filter part to use in LDAP searches
 	 */
-	private function getFilterPartForSearch($search, $searchAttributes, $fallbackAttribute) {
+	private function getFilterPartForSearch(string $search, $searchAttributes, string $fallbackAttribute): string {
 		$filter = [];
 		$haveMultiSearchAttributes = (is_array($searchAttributes) && count($searchAttributes) > 0);
 		if ($haveMultiSearchAttributes && strpos(trim($search), ' ') !== false) {
@@ -1623,10 +1604,8 @@ class Access extends LDAPUtility {
 	 * returns the search term depending on whether we are allowed
 	 * list users found by ldap with the current input appended by
 	 * a *
-	 *
-	 * @return string
 	 */
-	private function prepareSearchTerm($term) {
+	private function prepareSearchTerm(string $term): string {
 		$config = \OC::$server->getConfig();
 
 		$allowEnum = $config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes');
@@ -1735,7 +1714,7 @@ class Access extends LDAPUtility {
 	 * @return bool true on success, false otherwise
 	 * @throws ServerNotAvailableException
 	 */
-	private function detectUuidAttribute($dn, $isUser = true, $force = false, array $ldapRecord = null) {
+	private function detectUuidAttribute(string $dn, bool $isUser = true, bool $force = false, ?array $ldapRecord = null): bool {
 		if ($isUser) {
 			$uuidAttr = 'ldapUuidUserAttribute';
 			$uuidOverride = $this->connection->ldapExpertUUIDUserAttr;
@@ -1827,10 +1806,9 @@ class Access extends LDAPUtility {
 	 * converts a binary ObjectGUID into a string representation
 	 *
 	 * @param string $oguid the ObjectGUID in it's binary form as retrieved from AD
-	 * @return string
 	 * @link https://www.php.net/manual/en/function.ldap-get-values-len.php#73198
 	 */
-	private function convertObjectGUID2Str($oguid) {
+	private function convertObjectGUID2Str(string $oguid): string {
 		$hex_guid = bin2hex($oguid);
 		$hex_guid_to_guid_str = '';
 		for ($k = 1; $k <= 4; ++$k) {
@@ -1990,7 +1968,7 @@ class Access extends LDAPUtility {
 	 *
 	 * @throws ServerNotAvailableException
 	 */
-	private function abandonPagedSearch() {
+	private function abandonPagedSearch(): void {
 		if ($this->lastCookie === '') {
 			return;
 		}
diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php
index 6666da1e933..3cd6a340a56 100644
--- a/apps/user_ldap/lib/Connection.php
+++ b/apps/user_ldap/lib/Connection.php
@@ -260,7 +260,7 @@ class Connection extends LDAPUtility {
 		}
 		$key = $this->getCacheKey($key);
 
-		return json_decode(base64_decode($this->cache->get($key)), true);
+		return json_decode(base64_decode($this->cache->get($key) ?? ''), true);
 	}
 
 	/**
diff --git a/apps/user_ldap/lib/Helper.php b/apps/user_ldap/lib/Helper.php
index 650755842b6..437fab6b6a8 100644
--- a/apps/user_ldap/lib/Helper.php
+++ b/apps/user_ldap/lib/Helper.php
@@ -129,10 +129,10 @@ class Helper {
 		sort($serverConnections);
 		$lastKey = array_pop($serverConnections);
 		$lastNumber = (int)str_replace('s', '', $lastKey);
-		return 's' . str_pad($lastNumber + 1, 2, '0', STR_PAD_LEFT);
+		return 's' . str_pad((string)($lastNumber + 1), 2, '0', STR_PAD_LEFT);
 	}
 
-	private function getServersConfig($value) {
+	private function getServersConfig(string $value): array {
 		$regex = '/' . $value . '$/S';
 
 		$keys = $this->config->getAppKeys('user_ldap');
@@ -211,7 +211,7 @@ class Helper {
 	/**
 	 * sanitizes a DN received from the LDAP server
 	 *
-	 * @param array $dn the DN in question
+	 * @param array|string $dn the DN in question
 	 * @return array|string the sanitized DN
 	 */
 	public function sanitizeDN($dn) {
@@ -275,10 +275,10 @@ class Helper {
 	 * listens to a hook thrown by server2server sharing and replaces the given
 	 * login name by a username, if it matches an LDAP user.
 	 *
-	 * @param array $param
+	 * @param array $param contains a reference to a $uid var under 'uid' key
 	 * @throws \Exception
 	 */
-	public static function loginName2UserName($param) {
+	public static function loginName2UserName($param): void {
 		if (!isset($param['uid'])) {
 			throw new \Exception('key uid is expected to be set in $param');
 		}