Skip to content

user_ldap fix ldap connection resets #31421

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.

Sign up for GitHub

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

Merged
come-nc merged 6 commits into from
Mar 17, 2022
Merged

user_ldap fix ldap connection resets #31421

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8349530
Fix executeRead when connection is reset because of a timeout
come-nc Mar 3, 2022
fb63484
Improve typing in user_ldap to detect problems early
come-nc Mar 3, 2022
e80eb79
Remove redundant is_array check
come-nc Mar 3, 2022
0c5bd58
Fix types in OCA\User_LDAP\Configuration
come-nc Mar 3, 2022
dab5ea9
Fix unit tests
come-nc Mar 3, 2022
df29acb
Set $limit as int as well in Access::count private method
come-nc Mar 8, 2022
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
Prev Previous commit
Next Next commit
Fix types in OCA\User_LDAP\Configuration 
Signed-off-by: Côme Chilliet <[email protected]>
  • Loading branch information
@come-nc
come-nc committed Mar 3, 2022
commit 0c5bd588ed4528d46df244a686b6f91299766836
86 changes: 29 additions & 57 deletions apps/user_ldap/lib/Configuration.php
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions apps/user_ldap/lib/Connection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,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