Skip to content

Commit

Permalink
Merge pull request #31421 from nextcloud/fix/user_ldap-fix-ldap-conne…
Browse files Browse the repository at this point in the history 
…ction-resets

user_ldap fix ldap connection resets
  • Loading branch information
@come-nc
come-nc authored Mar 17, 2022
2 parents d9c0799 + df29acb commit 475a859
Show file tree
Hide file tree
Showing 12 changed files with 189 additions and 247 deletions.
202 changes: 69 additions & 133 deletions apps/user_ldap/lib/Access.php
View file

Large diffs are not rendered by default.

86 changes: 29 additions & 57 deletions apps/user_ldap/lib/Configuration.php
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 475a859

Please sign in to comment.