Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

user_ldap fix ldap connection resets #31421

Merged
merged 6 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 65 additions & 129 deletions apps/user_ldap/lib/Access.php

Large diffs are not rendered by default.

86 changes: 29 additions & 57 deletions apps/user_ldap/lib/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,23 @@ class Configuration {
public const LDAP_SERVER_FEATURE_AVAILABLE = 'available';
public const LDAP_SERVER_FEATURE_UNAVAILABLE = 'unavailable';

protected $configPrefix = null;
/**
* @var string
*/
protected $configPrefix;
/**
* @var bool
*/
protected $configRead = false;
/**
* @var string[] pre-filled with one reference key so that at least one entry is written on save request and
* the config ID is registered
*/
protected $unsavedChanges = ['ldapConfigurationActive' => 'ldapConfigurationActive'];

//settings
/**
* @var array<string, mixed> settings
*/
protected $config = [
'ldapHost' => null,
'ldapPort' => null,
Expand Down Expand Up @@ -115,11 +123,7 @@ class Configuration {
'ldapMatchingRuleInChainState' => self::LDAP_SERVER_FEATURE_UNKNOWN,
];

/**
* @param string $configPrefix
* @param bool $autoRead
*/
public function __construct($configPrefix, $autoRead = true) {
public function __construct(string $configPrefix, bool $autoRead = true) {
$this->configPrefix = $configPrefix;
if ($autoRead) {
$this->readConfiguration();
Expand All @@ -145,10 +149,7 @@ public function __set($name, $value) {
$this->setConfiguration([$name => $value]);
}

/**
* @return array
*/
public function getConfiguration() {
public function getConfiguration(): array {
return $this->config;
}

Expand All @@ -159,13 +160,8 @@ public function getConfiguration() {
* @param array $config array that holds the config parameters in an associated
* array
* @param array &$applied optional; array where the set fields will be given to
* @return false|null
*/
public function setConfiguration($config, &$applied = null) {
if (!is_array($config)) {
return false;
}

public function setConfiguration(array $config, array &$applied = null): void {
$cta = $this->getConfigTranslationArray();
foreach ($config as $inputKey => $val) {
if (strpos($inputKey, '_') !== false && array_key_exists($inputKey, $cta)) {
Expand Down Expand Up @@ -207,11 +203,10 @@ public function setConfiguration($config, &$applied = null) {
}
$this->unsavedChanges[$key] = $key;
}
return null;
}

public function readConfiguration() {
if (!$this->configRead && !is_null($this->configPrefix)) {
public function readConfiguration(): void {
if (!$this->configRead) {
$cta = array_flip($this->getConfigTranslationArray());
foreach ($this->config as $key => $val) {
if (!isset($cta[$key])) {
Expand Down Expand Up @@ -260,7 +255,7 @@ public function readConfiguration() {
/**
* saves the current config changes in the database
*/
public function saveConfiguration() {
public function saveConfiguration(): void {
$cta = array_flip($this->getConfigTranslationArray());
foreach ($this->unsavedChanges as $key) {
$value = $this->config[$key];
Expand Down Expand Up @@ -293,7 +288,7 @@ public function saveConfiguration() {
}
$this->saveValue($cta[$key], $value);
}
$this->saveValue('_lastChange', time());
$this->saveValue('_lastChange', (string)time());
$this->unsavedChanges = [];
}

Expand All @@ -318,7 +313,7 @@ protected function getMultiLine($varName) {
* @param string $varName name of config-key
* @param array|string $value to set
*/
protected function setMultiLine($varName, $value) {
protected function setMultiLine(string $varName, $value): void {
if (empty($value)) {
$value = '';
} elseif (!is_array($value)) {
Expand Down Expand Up @@ -349,36 +344,20 @@ protected function setMultiLine($varName, $value) {
$this->setRawValue($varName, $finalValue);
}

/**
* @param string $varName
* @return string
*/
protected function getPwd($varName) {
protected function getPwd(string $varName): string {
return base64_decode($this->getValue($varName));
}

/**
* @param string $varName
* @return string
*/
protected function getLcValue($varName) {
protected function getLcValue(string $varName): string {
return mb_strtolower($this->getValue($varName), 'UTF-8');
}

/**
* @param string $varName
* @return string
*/
protected function getSystemValue($varName) {
protected function getSystemValue(string $varName): string {
//FIXME: if another system value is added, softcode the default value
return \OC::$server->getConfig()->getSystemValue($varName, false);
}

/**
* @param string $varName
* @return string
*/
protected function getValue($varName) {
protected function getValue(string $varName): string {
static $defaults;
if (is_null($defaults)) {
$defaults = $this->getDefaults();
Expand All @@ -394,7 +373,7 @@ protected function getValue($varName) {
* @param string $varName name of config key
* @param mixed $value to set
*/
protected function setValue($varName, $value) {
protected function setValue(string $varName, $value): void {
if (is_string($value)) {
$value = trim($value);
}
Expand All @@ -407,16 +386,11 @@ protected function setValue($varName, $value) {
* @param string $varName name of config key
* @param mixed $value to set
*/
protected function setRawValue($varName, $value) {
protected function setRawValue(string $varName, $value): void {
$this->config[$varName] = $value;
}

/**
* @param string $varName
* @param string $value
* @return bool
*/
protected function saveValue($varName, $value) {
protected function saveValue(string $varName, string $value): bool {
\OC::$server->getConfig()->setAppValue(
'user_ldap',
$this->configPrefix.$varName,
Expand All @@ -429,7 +403,7 @@ protected function saveValue($varName, $value) {
* @return array an associative array with the default values. Keys are correspond
* to config-value entries in the database table
*/
public function getDefaults() {
public function getDefaults(): array {
return [
'ldap_host' => '',
'ldap_port' => '',
Expand Down Expand Up @@ -492,7 +466,7 @@ public function getDefaults() {
/**
* @return array that maps internal variable names to database fields
*/
public function getConfigTranslationArray() {
public function getConfigTranslationArray(): array {
//TODO: merge them into one representation
static $array = [
'ldap_host' => 'ldapHost',
Expand Down Expand Up @@ -554,18 +528,16 @@ public function getConfigTranslationArray() {
}

/**
* @param string $rule
* @return array
* @throws \RuntimeException
*/
public function resolveRule($rule) {
public function resolveRule(string $rule): array {
if ($rule === 'avatar') {
return $this->getAvatarAttributes();
}
throw new \RuntimeException('Invalid rule');
}

public function getAvatarAttributes() {
public function getAvatarAttributes(): array {
$value = $this->ldapUserAvatarRule ?: self::AVATAR_PREFIX_DEFAULT;
$defaultAttributes = ['jpegphoto', 'thumbnailphoto'];

Expand Down
42 changes: 33 additions & 9 deletions apps/user_ldap/lib/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,25 @@ class Connection extends LDAPUtility {
* @var resource|\LDAP\Connection|null
*/
private $ldapConnectionRes = null;

/**
* @var string
*/
private $configPrefix;

/**
* @var ?string
*/
private $configID;

/**
* @var bool
*/
private $configured = false;
//whether connection should be kept on __destruct

/**
* @var bool whether connection should be kept on __destruct
*/
private $dontDestruct = false;

/**
Expand All @@ -94,33 +109,42 @@ class Connection extends LDAPUtility {
*/
public $hasGidNumber = true;

//cache handler
protected $cache;
/**
* @var \OCP\ICache|null
*/
protected $cache = null;

/** @var Configuration settings handler **/
protected $configuration;

/**
* @var bool
*/
protected $doNotValidate = false;

/**
* @var bool
*/
protected $ignoreValidation = false;

/**
* @var array{dn?: mixed, hash?: string, result?: bool}
*/
protected $bindResult = [];

/** @var LoggerInterface */
protected $logger;

/**
* Constructor
* @param ILDAPWrapper $ldap
* @param string $configPrefix a string with the prefix for the configkey column (appconfig table)
* @param string|null $configID a string with the value for the appid column (appconfig table) or null for on-the-fly connections
*/
public function __construct(ILDAPWrapper $ldap, $configPrefix = '', $configID = 'user_ldap') {
public function __construct(ILDAPWrapper $ldap, string $configPrefix = '', ?string $configID = 'user_ldap') {
parent::__construct($ldap);
$this->configPrefix = $configPrefix;
$this->configID = $configID;
$this->configuration = new Configuration($configPrefix,
!is_null($configID));
$this->configuration = new Configuration($configPrefix, !is_null($configID));
$memcache = \OC::$server->getMemCacheFactory();
if ($memcache->isAvailable()) {
$this->cache = $memcache->createDistributed();
Expand Down Expand Up @@ -304,9 +328,9 @@ private function readConfiguration($force = false) {
* set LDAP configuration with values delivered by an array, not read from configuration
* @param array $config array that holds the config parameters in an associated array
* @param array &$setParameters optional; array where the set fields will be given to
* @return boolean true if config validates, false otherwise. Check with $setParameters for detailed success on single parameters
* @return bool true if config validates, false otherwise. Check with $setParameters for detailed success on single parameters
*/
public function setConfiguration($config, &$setParameters = null) {
public function setConfiguration($config, &$setParameters = null): bool {
if (is_null($setParameters)) {
$setParameters = [];
}
Expand Down
22 changes: 10 additions & 12 deletions apps/user_ldap/lib/Group_LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -863,20 +863,18 @@ private function getGroupsByMember(string $dn, array &$seen = null): array {

$groups = $this->access->fetchListOfGroups($filter,
[strtolower($this->access->connection->ldapGroupMemberAssocAttr), $this->access->connection->ldapGroupDisplayName, 'dn']);
if (is_array($groups)) {
$fetcher = function ($dn) use (&$seen) {
if (is_array($dn) && isset($dn['dn'][0])) {
$dn = $dn['dn'][0];
}
return $this->getGroupsByMember($dn, $seen);
};

if (empty($dn)) {
$dn = "";
$fetcher = function ($dn) use (&$seen) {
if (is_array($dn) && isset($dn['dn'][0])) {
$dn = $dn['dn'][0];
}
return $this->getGroupsByMember($dn, $seen);
};

$allGroups = $this->walkNestedGroups($dn, $fetcher, $groups, $seen);
if (empty($dn)) {
$dn = "";
}

$allGroups = $this->walkNestedGroups($dn, $fetcher, $groups, $seen);
$visibleGroups = $this->filterValidGroups($allGroups);
return array_intersect_key($allGroups, $visibleGroups);
}
Expand Down Expand Up @@ -1349,7 +1347,7 @@ public function getDisplayName(string $gid): string {
$this->access->groupname2dn($gid),
$this->access->connection->ldapGroupDisplayName);

if ($displayName && (count($displayName) > 0)) {
if (($displayName !== false) && (count($displayName) > 0)) {
$displayName = $displayName[0];
$this->access->connection->writeToCache($cacheKey, $displayName);
return $displayName;
Expand Down
6 changes: 3 additions & 3 deletions apps/user_ldap/lib/ILDAPWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function controlPagedResult($link, $pageSize, $isCritical);
* Retrieve the LDAP pagination cookie
* @param resource|\LDAP\Connection $link LDAP link resource
* @param resource|\LDAP\Result $result LDAP result resource
* @param string $cookie structure sent by LDAP server
* @param string &$cookie structure sent by LDAP server
* @return bool true on success, false otherwise
*
* Corresponds to ldap_control_paged_result_response
Expand Down Expand Up @@ -178,8 +178,8 @@ public function modReplace($link, $userDN, $password);
/**
* Sets the value of the specified option to be $value
* @param resource|\LDAP\Connection $link LDAP link resource
* @param string $option a defined LDAP Server option
* @param int $value the new value for the option
* @param int $option a defined LDAP Server option
* @param mixed $value the new value for the option
* @return bool true on success, false otherwise
*/
public function setOption($link, $option, $value);
Expand Down
5 changes: 1 addition & 4 deletions apps/user_ldap/lib/Mapping/AbstractMapping.php
Original file line number Diff line number Diff line change
Expand Up @@ -280,12 +280,9 @@ public function getListOfIdsByDn(array $fdns): array {
/**
* Searches mapped names by the giving string in the name column
*
* @param string $search
* @param string $prefixMatch
* @param string $postfixMatch
* @return string[]
*/
public function getNamesBySearch($search, $prefixMatch = "", $postfixMatch = "") {
public function getNamesBySearch(string $search, string $prefixMatch = "", string $postfixMatch = ""): array {
$statement = $this->dbc->prepare('
SELECT `owncloud_name`
FROM `' . $this->getTableName() . '`
Expand Down
7 changes: 3 additions & 4 deletions apps/user_ldap/lib/User/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,8 @@ public function getUsername() {

/**
* returns the home directory of the user if specified by LDAP settings
* @param string $valueFromLDAP
* @return bool|string
* @param ?string $valueFromLDAP
* @return false|string
* @throws \Exception
*/
public function getHomePath($valueFromLDAP = null) {
Expand All @@ -278,8 +278,7 @@ public function getHomePath($valueFromLDAP = null) {
&& strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0
&& $this->access->connection->homeFolderNamingRule !== 'attr:') {
$attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:'));
$homedir = $this->access->readAttribute(
$this->access->username2dn($this->getUsername()), $attr);
$homedir = $this->access->readAttribute($this->access->username2dn($this->getUsername()), $attr);
if ($homedir && isset($homedir[0])) {
$path = $homedir[0];
}
Expand Down
Loading