summaryrefslogtreecommitdiffstats
path: root/apps/user_ldap/lib
diff options
context:
space:
mode:
authorCôme Chilliet <come.chilliet@nextcloud.com>2022-10-20 16:45:43 +0200
committerCôme Chilliet <come.chilliet@nextcloud.com>2022-10-20 16:51:39 +0200
commit3dedfd34b92f22835dbe9e466c3776861fd613ab (patch)
tree0cf838344f25510b05bd6872bdbc0e733dccc2f2 /apps/user_ldap/lib
parent00c4c3d72332e54b588ccd8009c05cfe724b194a (diff)
downloadnextcloud-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.php9
-rw-r--r--apps/user_ldap/lib/Wizard.php194
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