diff --git a/apps/user_ldap/ajax/testConfiguration.php b/apps/user_ldap/ajax/testConfiguration.php index b91eab8fd92b4..b77439fa3e812 100644 --- a/apps/user_ldap/ajax/testConfiguration.php +++ b/apps/user_ldap/ajax/testConfiguration.php @@ -1,5 +1,6 @@ getConfiguration(); if ($conf['ldap_configuration_active'] === '0') { //needs to be true, otherwise it will also fail with an irritating message $conf['ldap_configuration_active'] = '1'; - $configurationOk = $connection->setConfiguration($conf); } - if ($configurationOk) { + try { + $connection->setConfiguration($conf, throw: true); + } catch (ConfigurationIssueException $e) { + $configurationError = $e->getHint(); + } + if ($configurationError === '') { //Configuration is okay /* * Closing the session since it won't be used from this point on. There might be a potential @@ -64,7 +69,7 @@ } } else { \OC_JSON::error(['message' - => $l->t('Invalid configuration. Please have a look at the logs for further details.')]); + => $l->t('Invalid configuration: %s', $configurationError)]); } } catch (\Exception $e) { \OC_JSON::error(['message' => $e->getMessage()]); diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 48472f36b65e1..36259880928e2 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -36,6 +36,7 @@ 'OCA\\User_LDAP\\Events\\GroupBackendRegistered' => $baseDir . '/../lib/Events/GroupBackendRegistered.php', 'OCA\\User_LDAP\\Events\\UserBackendRegistered' => $baseDir . '/../lib/Events/UserBackendRegistered.php', 'OCA\\User_LDAP\\Exceptions\\AttributeNotSet' => $baseDir . '/../lib/Exceptions/AttributeNotSet.php', + 'OCA\\User_LDAP\\Exceptions\\ConfigurationIssueException' => $baseDir . '/../lib/Exceptions/ConfigurationIssueException.php', 'OCA\\User_LDAP\\Exceptions\\ConstraintViolationException' => $baseDir . '/../lib/Exceptions/ConstraintViolationException.php', 'OCA\\User_LDAP\\Exceptions\\NoMoreResults' => $baseDir . '/../lib/Exceptions/NoMoreResults.php', 'OCA\\User_LDAP\\Exceptions\\NotOnLDAP' => $baseDir . '/../lib/Exceptions/NotOnLDAP.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index fc67caabce635..be985838393f4 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -51,6 +51,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Events\\GroupBackendRegistered' => __DIR__ . '/..' . '/../lib/Events/GroupBackendRegistered.php', 'OCA\\User_LDAP\\Events\\UserBackendRegistered' => __DIR__ . '/..' . '/../lib/Events/UserBackendRegistered.php', 'OCA\\User_LDAP\\Exceptions\\AttributeNotSet' => __DIR__ . '/..' . '/../lib/Exceptions/AttributeNotSet.php', + 'OCA\\User_LDAP\\Exceptions\\ConfigurationIssueException' => __DIR__ . '/..' . '/../lib/Exceptions/ConfigurationIssueException.php', 'OCA\\User_LDAP\\Exceptions\\ConstraintViolationException' => __DIR__ . '/..' . '/../lib/Exceptions/ConstraintViolationException.php', 'OCA\\User_LDAP\\Exceptions\\NoMoreResults' => __DIR__ . '/..' . '/../lib/Exceptions/NoMoreResults.php', 'OCA\\User_LDAP\\Exceptions\\NotOnLDAP' => __DIR__ . '/..' . '/../lib/Exceptions/NotOnLDAP.php', diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index e7f2a41364326..14dfcdb1bc56b 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -8,11 +8,14 @@ namespace OCA\User_LDAP; use OC\ServerNotAvailableException; +use OCA\User_LDAP\Exceptions\ConfigurationIssueException; use OCP\ICache; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IDBConnection; +use OCP\IL10N; use OCP\Server; +use OCP\Util; use Psr\Log\LoggerInterface; /** @@ -134,8 +137,8 @@ class Connection extends LDAPUtility { */ protected $bindResult = []; - /** @var LoggerInterface */ - protected $logger; + protected LoggerInterface $logger; + private IL10N $l10n; /** * Constructor @@ -157,6 +160,7 @@ public function __construct( $this->doNotValidate = !in_array($this->configPrefix, $helper->getServerConfigurationPrefixes()); $this->logger = Server::get(LoggerInterface::class); + $this->l10n = Util::getL10N('user_ldap'); } public function __destruct() { @@ -332,16 +336,17 @@ private function readConfiguration(bool $force = false): void { * 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 + * @param bool $throw if true, throw ConfigurationIssueException with details instead of returning false * @return bool true if config validates, false otherwise. Check with $setParameters for detailed success on single parameters */ - public function setConfiguration($config, &$setParameters = null): bool { + public function setConfiguration(array $config, ?array &$setParameters = null, bool $throw = false): bool { if (is_null($setParameters)) { $setParameters = []; } $this->doNotValidate = false; $this->configuration->setConfiguration($config, $setParameters); if (count($setParameters) > 0) { - $this->configured = $this->validateConfiguration(); + $this->configured = $this->validateConfiguration($throw); } @@ -447,10 +452,10 @@ private function doSoftValidation(): void { } } - private function doCriticalValidation(): bool { - $configurationOK = true; - $errorStr = 'Configuration Error (prefix ' . $this->configPrefix . '): '; - + /** + * @throws ConfigurationIssueException + */ + private function doCriticalValidation(): void { //options that shall not be empty $options = ['ldapHost', 'ldapUserDisplayName', 'ldapGroupDisplayName', 'ldapLoginFilter']; @@ -483,10 +488,9 @@ private function doCriticalValidation(): bool { $subj = $key; break; } - $configurationOK = false; - $this->logger->warning( - $errorStr . 'No ' . $subj . ' given!', - ['app' => 'user_ldap'] + throw new ConfigurationIssueException( + 'No ' . $subj . ' given!', + $this->l10n->t('Mandatory field "%s" left empty', $subj), ); } } @@ -494,47 +498,76 @@ private function doCriticalValidation(): bool { //combinations $agent = $this->configuration->ldapAgentName; $pwd = $this->configuration->ldapAgentPassword; - if ( - ($agent === '' && $pwd !== '') - || ($agent !== '' && $pwd === '') - ) { - $this->logger->warning( - $errorStr . 'either no password is given for the user ' . - 'agent or a password is given, but not an LDAP agent.', - ['app' => 'user_ldap'] + if ($agent === '' && $pwd !== '') { + throw new ConfigurationIssueException( + 'A password is given, but not an LDAP agent', + $this->l10n->t('A password is given, but not an LDAP agent'), + ); + } + if ($agent !== '' && $pwd === '') { + throw new ConfigurationIssueException( + 'No password is given for the user agent', + $this->l10n->t('No password is given for the user agent'), ); - $configurationOK = false; } $base = $this->configuration->ldapBase; $baseUsers = $this->configuration->ldapBaseUsers; $baseGroups = $this->configuration->ldapBaseGroups; - if (empty($base) && empty($baseUsers) && empty($baseGroups)) { - $this->logger->warning( - $errorStr . 'Not a single Base DN given.', - ['app' => 'user_ldap'] + if (empty($base)) { + throw new ConfigurationIssueException( + 'Not a single Base DN given', + $this->l10n->t('No LDAP base DN was given'), ); - $configurationOK = false; } - if (mb_strpos((string)$this->configuration->ldapLoginFilter, '%uid', 0, 'UTF-8') - === false) { - $this->logger->warning( - $errorStr . 'login filter does not contain %uid place holder.', - ['app' => 'user_ldap'] + if (!empty($baseUsers) && !$this->checkBasesAreValid($baseUsers, $base)) { + throw new ConfigurationIssueException( + 'User base is not in root base', + $this->l10n->t('User base DN is not a subnode of global base DN'), + ); + } + + if (!empty($baseGroups) && !$this->checkBasesAreValid($baseGroups, $base)) { + throw new ConfigurationIssueException( + 'Group base is not in root base', + $this->l10n->t('Group base DN is not a subnode of global base DN'), ); - $configurationOK = false; } - return $configurationOK; + if (mb_strpos((string)$this->configuration->ldapLoginFilter, '%uid', 0, 'UTF-8') === false) { + throw new ConfigurationIssueException( + 'Login filter does not contain %uid place holder.', + $this->l10n->t('Login filter does not contain %uid place holder'), + ); + } + } + + /** + * Checks that all bases are subnodes of one of the root bases + */ + private function checkBasesAreValid(array $bases, array $rootBases): bool { + foreach ($bases as $base) { + $ok = false; + foreach ($rootBases as $rootBase) { + if (str_ends_with($base, $rootBase)) { + $ok = true; + break; + } + } + if (!$ok) { + return false; + } + } + return true; } /** * Validates the user specified configuration * @return bool true if configuration seems OK, false otherwise */ - private function validateConfiguration(): bool { + private function validateConfiguration(bool $throw = false): bool { if ($this->doNotValidate) { //don't do a validation if it is a new configuration with pure //default values. Will be allowed on changes via __set or @@ -548,7 +581,19 @@ private function validateConfiguration(): bool { //second step: critical checks. If left empty or filled wrong, mark as //not configured and give a warning. - return $this->doCriticalValidation(); + try { + $this->doCriticalValidation(); + return true; + } catch (ConfigurationIssueException $e) { + if ($throw) { + throw $e; + } + $this->logger->warning( + 'Configuration Error (prefix ' . $this->configPrefix . '): ' . $e->getMessage(), + ['exception' => $e] + ); + return false; + } } diff --git a/apps/user_ldap/lib/Exceptions/ConfigurationIssueException.php b/apps/user_ldap/lib/Exceptions/ConfigurationIssueException.php new file mode 100644 index 0000000000000..efeb426b13d4a --- /dev/null +++ b/apps/user_ldap/lib/Exceptions/ConfigurationIssueException.php @@ -0,0 +1,15 @@ +