diff options
author | Côme Chilliet <come.chilliet@nextcloud.com> | 2022-10-20 16:45:43 +0200 |
---|---|---|
committer | Côme Chilliet <come.chilliet@nextcloud.com> | 2022-10-20 16:51:39 +0200 |
commit | 3dedfd34b92f22835dbe9e466c3776861fd613ab (patch) | |
tree | 0cf838344f25510b05bd6872bdbc0e733dccc2f2 /apps/user_ldap/lib | |
parent | 00c4c3d72332e54b588ccd8009c05cfe724b194a (diff) | |
download | nextcloud-server-3dedfd34b92f22835dbe9e466c3776861fd613ab.tar.gz nextcloud-server-3dedfd34b92f22835dbe9e466c3776861fd613ab.zip |
Clean up LDAP Wizard typing
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Diffstat (limited to 'apps/user_ldap/lib')
-rw-r--r-- | apps/user_ldap/lib/ILDAPWrapper.php | 9 | ||||
-rw-r--r-- | apps/user_ldap/lib/Wizard.php | 194 |
2 files changed, 99 insertions, 104 deletions
diff --git a/apps/user_ldap/lib/ILDAPWrapper.php b/apps/user_ldap/lib/ILDAPWrapper.php index b5c5568348e..6ec88effa5f 100644 --- a/apps/user_ldap/lib/ILDAPWrapper.php +++ b/apps/user_ldap/lib/ILDAPWrapper.php @@ -30,7 +30,6 @@ namespace OCA\User_LDAP; interface ILDAPWrapper { - //LDAP functions in use /** @@ -48,7 +47,7 @@ interface ILDAPWrapper { * connect to an LDAP server * @param string $host The host to connect to * @param string $port The port to connect to - * @return mixed a link resource on success, otherwise false + * @return resource|\LDAP\Connection|false a link resource on success, otherwise false */ public function connect($host, $port); @@ -106,7 +105,7 @@ interface ILDAPWrapper { * Get attributes from a search result entry * @param resource|\LDAP\Connection $link LDAP link resource * @param resource|\LDAP\ResultEntry $result LDAP result resource - * @return array containing the results, false on error + * @return array|false containing the results, false on error * */ public function getAttributes($link, $result); @@ -114,7 +113,7 @@ interface ILDAPWrapper { * Get the DN of a result entry * @param resource|\LDAP\Connection $link LDAP link resource * @param resource|\LDAP\ResultEntry $result LDAP result resource - * @return string containing the DN, false on error + * @return string|false containing the DN, false on error */ public function getDN($link, $result); @@ -122,7 +121,7 @@ interface ILDAPWrapper { * Get all result entries * @param resource|\LDAP\Connection $link LDAP link resource * @param resource|\LDAP\Result $result LDAP result resource - * @return array containing the results, false on error + * @return array|false containing the results, false on error */ public function getEntries($link, $result); diff --git a/apps/user_ldap/lib/Wizard.php b/apps/user_ldap/lib/Wizard.php index 19de55c091d..346ac7a61a6 100644 --- a/apps/user_ldap/lib/Wizard.php +++ b/apps/user_ldap/lib/Wizard.php @@ -19,6 +19,7 @@ * @author Tobias Perschon <tobias@perschon.at> * @author Victor Dubiniuk <dubiniuk@owncloud.com> * @author Xuanwo <xuanwo@yunify.com> + * @author Côme Chilliet <come.chilliet@nextcloud.com> * * @license AGPL-3.0 * @@ -35,22 +36,22 @@ * along with this program. If not, see <http://www.gnu.org/licenses/> * */ + namespace OCA\User_LDAP; use OC\ServerNotAvailableException; +use OCP\IL10N; +use OCP\L10N\IFactory as IL10NFactory; use Psr\Log\LoggerInterface; class Wizard extends LDAPUtility { - /** @var \OCP\IL10N */ - protected static $l; - protected $access; + protected static ?IL10N $l = null; + protected Access $access; + /** @var resource|\LDAP\Connection|null */ protected $cr; - protected $configuration; - protected $result; - protected $resultCache = []; - - /** @var LoggerInterface */ - protected $logger; + protected Configuration $configuration; + protected WizardResult $result; + protected LoggerInterface $logger; public const LRESULT_PROCESSED_OK = 2; public const LRESULT_PROCESSED_INVALID = 3; @@ -65,17 +66,15 @@ class Wizard extends LDAPUtility { public const LDAP_NW_TIMEOUT = 4; - /** - * Constructor - * @param Configuration $configuration an instance of Configuration - * @param ILDAPWrapper $ldap an instance of ILDAPWrapper - * @param Access $access - */ - public function __construct(Configuration $configuration, ILDAPWrapper $ldap, Access $access) { + public function __construct( + Configuration $configuration, + ILDAPWrapper $ldap, + Access $access + ) { parent::__construct($ldap); $this->configuration = $configuration; - if (is_null(Wizard::$l)) { - Wizard::$l = \OC::$server->getL10N('user_ldap'); + if (is_null(static::$l)) { + static::$l = \OC::$server->get(IL10NFactory::class)->get('user_ldap'); } $this->access = $access; $this->result = new WizardResult(); @@ -93,7 +92,6 @@ class Wizard extends LDAPUtility { * * @param string $filter the LDAP search filter * @param string $type a string being either 'users' or 'groups'; - * @return int * @throws \Exception */ public function countEntries(string $filter, string $type): int { @@ -120,6 +118,9 @@ class Wizard extends LDAPUtility { return (int)$result; } + /** + * @return WizardResult|false + */ public function countGroups() { $filter = $this->configuration->ldapGroupFilter; @@ -153,10 +154,9 @@ class Wizard extends LDAPUtility { } /** - * @return WizardResult * @throws \Exception */ - public function countUsers() { + public function countUsers(): WizardResult { $filter = $this->access->getFilterForUserCount(); $usersTotal = $this->countEntries($filter, 'users'); @@ -176,32 +176,26 @@ class Wizard extends LDAPUtility { /** * counts any objects in the currently set base dn * - * @return WizardResult * @throws \Exception */ - public function countInBaseDN() { + public function countInBaseDN(): WizardResult { // we don't need to provide a filter in this case $total = $this->countEntries('', 'objects'); - if ($total === false) { - throw new \Exception('invalid results received'); - } $this->result->addChange('ldap_test_base', $total); return $this->result; } /** * counts users with a specified attribute - * @param string $attr - * @param bool $existsCheck - * @return int|bool + * @return int|false */ - public function countUsersWithAttribute($attr, $existsCheck = false) { + public function countUsersWithAttribute(string $attr, bool $existsCheck = false) { if (!$this->checkRequirements(['ldapHost', 'ldapPort', 'ldapBase', 'ldapUserFilter', ])) { - return false; + return false; } $filter = $this->access->combineFilterWithAnd([ @@ -209,7 +203,7 @@ class Wizard extends LDAPUtility { $attr . '=*' ]); - $limit = ($existsCheck === false) ? null : 1; + $limit = $existsCheck ? null : 1; return $this->access->countUsers($filter, ['dn'], $limit); } @@ -217,7 +211,7 @@ class Wizard extends LDAPUtility { /** * detects the display name attribute. If a setting is already present that * returns at least one hit, the detection will be canceled. - * @return WizardResult|bool + * @return WizardResult|false * @throws \Exception */ public function detectUserDisplayNameAttribute() { @@ -308,7 +302,7 @@ class Wizard extends LDAPUtility { } /** - * @return WizardResult + * @return WizardResult|false * @throws \Exception */ public function determineAttributes() { @@ -317,11 +311,15 @@ class Wizard extends LDAPUtility { 'ldapBase', 'ldapUserFilter', ])) { - return false; + return false; } $attributes = $this->getUserAttributes(); + if (!is_array($attributes)) { + throw new \Exception('Failed to determine user attributes'); + } + natcasesort($attributes); $attributes = array_values($attributes); @@ -337,7 +335,7 @@ class Wizard extends LDAPUtility { /** * detects the available LDAP attributes - * @return array|false The instance's WizardResult instance + * @return array|false * @throws \Exception */ private function getUserAttributes() { @@ -346,7 +344,7 @@ class Wizard extends LDAPUtility { 'ldapBase', 'ldapUserFilter', ])) { - return false; + return false; } $cr = $this->getConnection(); if (!$cr) { @@ -359,8 +357,12 @@ class Wizard extends LDAPUtility { if (!$this->ldap->isResource($rr)) { return false; } + /** @var resource|\LDAP\Result $rr */ $er = $this->ldap->firstEntry($cr, $rr); $attributes = $this->ldap->getAttributes($cr, $er); + if ($attributes === false) { + return false; + } $pureAttributes = []; for ($i = 0; $i < $attributes['count']; $i++) { $pureAttributes[] = $attributes[$i]; @@ -390,18 +392,15 @@ class Wizard extends LDAPUtility { /** * detects the available LDAP groups - * @param string $dbKey - * @param string $confKey - * @param bool $testMemberOf * @return WizardResult|false the instance's WizardResult instance * @throws \Exception */ - private function determineGroups($dbKey, $confKey, $testMemberOf = true) { + private function determineGroups(string $dbKey, string $confKey, bool $testMemberOf = true) { if (!$this->checkRequirements(['ldapHost', 'ldapPort', 'ldapBase', ])) { - return false; + return false; } $cr = $this->getConnection(); if (!$cr) { @@ -424,12 +423,9 @@ class Wizard extends LDAPUtility { /** * fetches all groups from LDAP and adds them to the result object * - * @param string $dbKey - * @param string $confKey - * @return array $groupEntries * @throws \Exception */ - public function fetchGroups($dbKey, $confKey) { + public function fetchGroups(string $dbKey, string $confKey): array { $obclasses = ['posixGroup', 'group', 'zimbraDistributionList', 'groupOfNames', 'groupOfUniqueNames']; $filterParts = []; @@ -451,7 +447,7 @@ class Wizard extends LDAPUtility { // detection will fail later $result = $this->access->searchGroups($filter, ['cn', 'dn'], $limit, $offset); foreach ($result as $item) { - if (!isset($item['cn']) && !is_array($item['cn']) && !isset($item['cn'][0])) { + if (!isset($item['cn']) || !is_array($item['cn']) || !isset($item['cn'][0])) { // just in case - no issue known continue; } @@ -476,12 +472,15 @@ class Wizard extends LDAPUtility { return $groupEntries; } + /** + * @return WizardResult|false + */ public function determineGroupMemberAssoc() { if (!$this->checkRequirements(['ldapHost', 'ldapPort', 'ldapGroupFilter', ])) { - return false; + return false; } $attribute = $this->detectGroupMemberAssoc(); if ($attribute === false) { @@ -522,7 +521,7 @@ class Wizard extends LDAPUtility { /** * detects the available object classes - * @return WizardResult + * @return WizardResult|false * @throws \Exception */ public function determineUserObjectClasses() { @@ -530,7 +529,7 @@ class Wizard extends LDAPUtility { 'ldapPort', 'ldapBase', ])) { - return false; + return false; } $cr = $this->getConnection(); if (!$cr) { @@ -602,7 +601,7 @@ class Wizard extends LDAPUtility { } /** - * @return bool|WizardResult + * @return WizardResult|false * @throws \Exception */ public function getUserLoginFilter() { @@ -624,11 +623,10 @@ class Wizard extends LDAPUtility { } /** - * @return bool|WizardResult - * @param string $loginName + * @return WizardResult|false * @throws \Exception */ - public function testLoginName($loginName) { + public function testLoginName(string $loginName) { if (!$this->checkRequirements(['ldapHost', 'ldapPort', 'ldapBase', @@ -641,6 +639,7 @@ class Wizard extends LDAPUtility { if (!$this->ldap->isResource($cr)) { throw new \Exception('connection error'); } + /** @var resource|\LDAP\Connection $cr */ if (mb_strpos($this->access->connection->ldapLoginFilter, '%uid', 0, 'UTF-8') === false) { @@ -670,10 +669,6 @@ class Wizard extends LDAPUtility { $this->checkHost(); $portSettings = $this->getPortSettingsToTry(); - if (!is_array($portSettings)) { - throw new \Exception(print_r($portSettings, true)); - } - //proceed from the best configuration and return on first success foreach ($portSettings as $setting) { $p = $setting['port']; @@ -742,7 +737,7 @@ class Wizard extends LDAPUtility { //this did not help :( //Let's see whether we can parse the Host URL and convert the domain to //a base DN - $helper = new Helper(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection()); + $helper = \OC::$server->get(Helper::class); $domain = $helper->getDomainFromURL($this->configuration->ldapHost); if (!$domain) { return false; @@ -768,7 +763,7 @@ class Wizard extends LDAPUtility { * @param string $value the (detected) value * */ - private function applyFind($key, $value) { + private function applyFind(string $key, string $value): void { $this->result->addChange($key, $value); $this->configuration->setConfiguration([$key => $value]); } @@ -778,7 +773,7 @@ class Wizard extends LDAPUtility { * field. In this case the port will be stripped off, but also stored as * setting. */ - private function checkHost() { + private function checkHost(): void { $host = $this->configuration->ldapHost; $hostInfo = parse_url($host); @@ -787,14 +782,14 @@ class Wizard extends LDAPUtility { $port = $hostInfo['port']; $host = str_replace(':'.$port, '', $host); $this->applyFind('ldap_host', $host); - $this->applyFind('ldap_port', $port); + $this->applyFind('ldap_port', (string)$port); } } /** * tries to detect the group member association attribute which is * one of 'uniqueMember', 'memberUid', 'member', 'gidNumber' - * @return string|false, string with the attribute name, false on error + * @return string|false string with the attribute name, false on error * @throws \Exception */ private function detectGroupMemberAssoc() { @@ -812,6 +807,7 @@ class Wizard extends LDAPUtility { if (!$this->ldap->isResource($rr)) { return false; } + /** @var resource|\LDAP\Result $rr */ $er = $this->ldap->firstEntry($cr, $rr); while ($this->ldap->isResource($er)) { $this->ldap->getDN($cr, $er); @@ -840,7 +836,7 @@ class Wizard extends LDAPUtility { * @return bool true on success, false otherwise * @throws \Exception */ - private function testBaseDN($base) { + private function testBaseDN(string $base): bool { $cr = $this->getConnection(); if (!$cr) { throw new \Exception('Could not connect to LDAP'); @@ -858,6 +854,7 @@ class Wizard extends LDAPUtility { ); return false; } + /** @var resource|\LDAP\Result $rr */ $entries = $this->ldap->countEntries($cr, $rr); return ($entries !== false) && ($entries > 0); } @@ -871,7 +868,7 @@ class Wizard extends LDAPUtility { * @return bool true if it does, false otherwise * @throws \Exception */ - private function testMemberOf() { + private function testMemberOf(): bool { $cr = $this->getConnection(); if (!$cr) { throw new \Exception('Could not connect to LDAP'); @@ -885,13 +882,12 @@ class Wizard extends LDAPUtility { /** * creates an LDAP Filter from given configuration - * @param integer $filterType int, for which use case the filter shall be created + * @param int $filterType int, for which use case the filter shall be created * can be any of self::LFILTER_USER_LIST, self::LFILTER_LOGIN or * self::LFILTER_GROUP_LIST - * @return string|false string with the filter on success, false otherwise * @throws \Exception */ - private function composeLdapFilter($filterType) { + private function composeLdapFilter(int $filterType): string { $filter = ''; $parts = 0; switch ($filterType) { @@ -921,6 +917,7 @@ class Wizard extends LDAPUtility { if (!$this->ldap->isResource($rr)) { continue; } + /** @var resource|\LDAP\Result $rr */ $er = $this->ldap->firstEntry($cr, $rr); $attrs = $this->ldap->getAttributes($cr, $er); $dn = $this->ldap->getDN($cr, $er); @@ -980,6 +977,9 @@ class Wizard extends LDAPUtility { $loginpart = '=%uid'; $filterUsername = ''; $userAttributes = $this->getUserAttributes(); + if ($userAttributes === false) { + throw new \Exception('Failed to get user attributes'); + } $userAttributes = array_change_key_case(array_flip($userAttributes)); $parts = 0; @@ -1044,10 +1044,9 @@ class Wizard extends LDAPUtility { * * @param int $port the port to connect with * @param bool $tls whether startTLS is to be used - * @return bool * @throws \Exception */ - private function connectAndBind($port, $tls) { + private function connectAndBind(int $port, bool $tls): bool { //connect, does not really trigger any server communication $host = $this->configuration->ldapHost; $hostInfo = parse_url($host); @@ -1058,10 +1057,11 @@ class Wizard extends LDAPUtility { 'Wiz: Attempting to connect', ['app' => 'user_ldap'] ); - $cr = $this->ldap->connect($host, $port); + $cr = $this->ldap->connect($host, (string)$port); if (!$this->ldap->isResource($cr)) { throw new \Exception(self::$l->t('Invalid Host')); } + /** @var resource|\LDAP\Connection $cr */ //set LDAP options $this->ldap->setOption($cr, LDAP_OPT_PROTOCOL_VERSION, 3); @@ -1086,7 +1086,7 @@ class Wizard extends LDAPUtility { $this->configuration->ldapAgentPassword ); $errNo = $this->ldap->errno($cr); - $error = ldap_error($cr); + $error = $this->ldap->error($cr); $this->ldap->unbind($cr); } catch (ServerNotAvailableException $e) { return false; @@ -1110,9 +1110,9 @@ class Wizard extends LDAPUtility { /** * checks whether a valid combination of agent and password has been * provided (either two values or nothing for anonymous connect) - * @return bool, true if everything is fine, false otherwise + * @return bool true if everything is fine, false otherwise */ - private function checkAgentRequirements() { + private function checkAgentRequirements(): bool { $agent = $this->configuration->ldapAgentName; $pwd = $this->configuration->ldapAgentPassword; @@ -1122,11 +1122,7 @@ class Wizard extends LDAPUtility { ; } - /** - * @param array $reqs - * @return bool - */ - private function checkRequirements($reqs) { + private function checkRequirements(array $reqs): bool { $this->checkAgentRequirements(); foreach ($reqs as $option) { $value = $this->configuration->$option; @@ -1148,7 +1144,7 @@ class Wizard extends LDAPUtility { * yields most result entries * @return array|false an array with the values on success, false otherwise */ - public function cumulativeSearchOnAttribute($filters, $attr, $dnReadLimit = 3, &$maxF = null) { + public function cumulativeSearchOnAttribute(array $filters, string $attr, int $dnReadLimit = 3, ?string &$maxF = null) { $dnRead = []; $foundItems = []; $maxEntries = 0; @@ -1161,6 +1157,7 @@ class Wizard extends LDAPUtility { if (!$this->ldap->isResource($cr)) { return false; } + /** @var resource|\LDAP\Connection $cr */ $lastFilter = null; if (isset($filters[count($filters) - 1])) { $lastFilter = $filters[count($filters) - 1]; @@ -1175,6 +1172,7 @@ class Wizard extends LDAPUtility { if (!$this->ldap->isResource($rr)) { continue; } + /** @var resource|\LDAP\Result $rr */ $entries = $this->ldap->countEntries($cr, $rr); $getEntryFunc = 'firstEntry'; if (($entries !== false) && ($entries > 0)) { @@ -1192,16 +1190,17 @@ class Wizard extends LDAPUtility { $rr = $entry; //will be expected by nextEntry next round $attributes = $this->ldap->getAttributes($cr, $entry); $dn = $this->ldap->getDN($cr, $entry); - if ($dn === false || in_array($dn, $dnRead)) { + if ($attributes === false || $dn === false || in_array($dn, $dnRead)) { continue; } $newItems = []; - $state = $this->getAttributeValuesFromEntry($attributes, + $state = $this->getAttributeValuesFromEntry( + $attributes, $attr, - $newItems); + $newItems + ); $dnReadCount++; $foundItems = array_merge($foundItems, $newItems); - $this->resultCache[$dn][$attr] = $newItems; $dnRead[] = $dn; } while (($state === self::LRESULT_PROCESSED_SKIP || $this->ldap->isResource($entry)) @@ -1221,10 +1220,10 @@ class Wizard extends LDAPUtility { * Configuration class * @param bool $po whether the objectClass with most result entries * shall be pre-selected via the result - * @return array|false list of found items. + * @return array list of found items. * @throws \Exception */ - private function determineFeature($objectclasses, $attr, $dbkey, $confkey, $po = false) { + private function determineFeature(array $objectclasses, string $attr, string $dbkey, string $confkey, bool $po = false): array { $cr = $this->getConnection(); if (!$cr) { throw new \Exception('Could not connect to LDAP'); @@ -1271,12 +1270,11 @@ class Wizard extends LDAPUtility { * @param array $result the return value from ldap_get_attributes * @param string $attribute the attribute values to look for * @param array &$known new values will be appended here - * @return int, state on of the class constants LRESULT_PROCESSED_OK, + * @return int state on of the class constants LRESULT_PROCESSED_OK, * LRESULT_PROCESSED_INVALID or LRESULT_PROCESSED_SKIP */ - private function getAttributeValuesFromEntry($result, $attribute, &$known) { - if (!is_array($result) - || !isset($result['count']) + private function getAttributeValuesFromEntry(array $result, string $attribute, array &$known): int { + if (!isset($result['count']) || !$result['count'] > 0) { return self::LRESULT_PROCESSED_INVALID; } @@ -1300,7 +1298,7 @@ class Wizard extends LDAPUtility { } /** - * @return bool|mixed + * @return resource|\LDAP\Connection|false a link resource on success, otherwise false */ private function getConnection() { if (!is_null($this->cr)) { @@ -1312,6 +1310,10 @@ class Wizard extends LDAPUtility { $this->configuration->ldapPort ); + if ($cr === false) { + return false; + } + $this->ldap->setOption($cr, LDAP_OPT_PROTOCOL_VERSION, 3); $this->ldap->setOption($cr, LDAP_OPT_REFERRALS, 0); $this->ldap->setOption($cr, LDAP_OPT_NETWORK_TIMEOUT, self::LDAP_NW_TIMEOUT); @@ -1330,10 +1332,7 @@ class Wizard extends LDAPUtility { return false; } - /** - * @return array - */ - private function getDefaultLdapPortSettings() { + private function getDefaultLdapPortSettings(): array { static $settings = [ ['port' => 7636, 'tls' => false], ['port' => 636, 'tls' => false], @@ -1345,10 +1344,7 @@ class Wizard extends LDAPUtility { return $settings; } - /** - * @return array - */ - private function getPortSettingsToTry() { + private function getPortSettingsToTry(): array { //389 ← LDAP / Unencrypted or StartTLS //636 ← LDAPS / SSL //7xxx ← UCS. need to be checked first, because both ports may be open |