From 9f91481324b26564dd099113bf9a690db6e269c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 11 Feb 2020 16:47:03 +0100 Subject: [PATCH 1/4] Add checkbox for TLS --- js/wizard/wizardTabElementary.js | 11 +++++++++++ lib/Wizard.php | 2 +- templates/part.wizard-server.php | 6 ++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/js/wizard/wizardTabElementary.js b/js/wizard/wizardTabElementary.js index 2f3c26453..e7538f33d 100644 --- a/js/wizard/wizardTabElementary.js +++ b/js/wizard/wizardTabElementary.js @@ -41,6 +41,10 @@ OCA = OCA || {}; setMethod: 'setPort', $relatedElements: $('.ldapDetectPort') }, + ldap_tls: { + $element: $('#ldap_tls'), + setMethod: 'setTLS' + }, ldap_dn: { $element: $('#ldap_dn'), setMethod: 'setAgentDN' @@ -119,6 +123,13 @@ OCA = OCA || {}; this.setElementValue(this.managedItems.ldap_port.$element, port); }, + /** + * updates the TLS configuration + */ + setTLS: function(tls) { + this.setElementValue(this.managedItems.ldap_tls.$element, tls); + }, + /** * updates the user (agent) DN text field * diff --git a/lib/Wizard.php b/lib/Wizard.php index 0b6ad7a1b..b64351133 100644 --- a/lib/Wizard.php +++ b/lib/Wizard.php @@ -1304,7 +1304,7 @@ private function getConnection() { $this->getLDAP()->setOption($cr, LDAP_OPT_PROTOCOL_VERSION, 3); $this->getLDAP()->setOption($cr, LDAP_OPT_REFERRALS, 0); $this->getLDAP()->setOption($cr, LDAP_OPT_NETWORK_TIMEOUT, self::LDAP_NW_TIMEOUT); - if ($this->configuration->ldapTLS === 1) { + if ($this->configuration->ldapTLS) { $this->getLDAP()->startTls($cr); } diff --git a/templates/part.wizard-server.php b/templates/part.wizard-server.php index c6ce5a542..0094c824e 100644 --- a/templates/part.wizard-server.php +++ b/templates/part.wizard-server.php @@ -55,6 +55,12 @@ class="ldapIconCopy icon-default-style" + +
+ + +
+
Date: Tue, 11 Feb 2020 18:14:11 +0100 Subject: [PATCH 2/4] Remove port auto-detection and minor fix in the template --- js/wizard/wizard.js | 1 - js/wizard/wizardDetectorPort.js | 44 ------- js/wizard/wizardTabElementary.js | 23 +--- lib/Controller/WizardController.php | 1 - lib/Wizard.php | 172 ---------------------------- templates/part.wizard-server.php | 5 +- templates/settings.php | 1 - 7 files changed, 3 insertions(+), 244 deletions(-) delete mode 100644 js/wizard/wizardDetectorPort.js diff --git a/js/wizard/wizard.js b/js/wizard/wizard.js index 62a3fccbd..aa470e52e 100644 --- a/js/wizard/wizard.js +++ b/js/wizard/wizard.js @@ -18,7 +18,6 @@ OCA = OCA || {}; detectorQueue.init(); var detectors = []; - detectors.push(new OCA.LDAP.Wizard.WizardDetectorPort()); detectors.push(new OCA.LDAP.Wizard.WizardDetectorBaseDN()); detectors.push(new OCA.LDAP.Wizard.WizardDetectorEmailAttribute()); detectors.push(new OCA.LDAP.Wizard.WizardDetectorUserDisplayNameAttribute()); diff --git a/js/wizard/wizardDetectorPort.js b/js/wizard/wizardDetectorPort.js deleted file mode 100644 index ba0751896..000000000 --- a/js/wizard/wizardDetectorPort.js +++ /dev/null @@ -1,44 +0,0 @@ - -/** - * Copyright (c) 2015, Arthur Schiwon - * This file is licensed under the Affero General Public License version 3 or later. - * See the COPYING-README file. - */ - -OCA = OCA || {}; - -(function() { - - /** - * @classdesc a Port Detector. It executes the auto-detection of the port - * by the ownCloud server, if requirements are met. - * - * @constructor - */ - var WizardDetectorPort = OCA.LDAP.Wizard.WizardDetectorGeneric.subClass({ - /** @inheritdoc */ - init: function() { - this.setTargetKey('ldap_port'); - this.runsOnRequest = true; - }, - - /** - * runs the detector, if port is not set. - * - * @param {OCA.LDAP.Wizard.ConfigModel} model - * @param {string} configID - the configuration prefix - * @returns {boolean|jqXHR} - * @abstract - */ - run: function(model, configID) { - model.notifyAboutDetectionStart('ldap_port'); - var params = OC.buildQueryString({ - action: 'guessPortAndTLS', - ldap_serverconfig_chooser: configID - }); - return model.callWizard(params, this.processResult, this); - } - }); - - OCA.LDAP.Wizard.WizardDetectorPort = WizardDetectorPort; -})(); diff --git a/js/wizard/wizardTabElementary.js b/js/wizard/wizardTabElementary.js index e7538f33d..978dbcd8f 100644 --- a/js/wizard/wizardTabElementary.js +++ b/js/wizard/wizardTabElementary.js @@ -38,8 +38,7 @@ OCA = OCA || {}; }, ldap_port: { $element: $('#ldap_port'), - setMethod: 'setPort', - $relatedElements: $('.ldapDetectPort') + setMethod: 'setPort' }, ldap_tls: { $element: $('#ldap_tls'), @@ -69,8 +68,7 @@ OCA = OCA || {}; } }; this.setManagedItems(items); - _.bindAll(this, 'onPortButtonClick', 'onBaseDNButtonClick', 'onBaseDNTestButtonClick'); - this.managedItems.ldap_port.$relatedElements.click(this.onPortButtonClick); + _.bindAll(this, 'onBaseDNButtonClick', 'onBaseDNTestButtonClick'); this.managedItems.ldap_base.$detectButton.click(this.onBaseDNButtonClick); this.managedItems.ldap_base.$testButton.click(this.onBaseDNTestButtonClick); }, @@ -107,11 +105,6 @@ OCA = OCA || {}; */ setHost: function(host) { this.setElementValue(this.managedItems.ldap_host.$element, host); - if(host) { - this.enableElement(this.managedItems.ldap_port.$relatedElements); - } else { - this.disableElement(this.managedItems.ldap_port.$relatedElements); - } }, /** @@ -216,7 +209,6 @@ OCA = OCA || {}; */ onConfigSwitch: function(view, configuration) { this.baseDNTestTriggered = false; - view.disableElement(view.managedItems.ldap_port.$relatedElements); view.onConfigLoaded(view, configuration); }, @@ -301,17 +293,6 @@ OCA = OCA || {}; } }, - /** - * request to count the users with the current filter - * - * @param {Event} event - */ - onPortButtonClick: function(event) { - event.preventDefault(); - this.configModel.executeAfterSet(this.configModel.requestWizard, this.configModel, 'ldap_port'); - // this.configModel.requestWizard('ldap_port'); - }, - /** * request to count the users with the current filter * diff --git a/lib/Controller/WizardController.php b/lib/Controller/WizardController.php index 17d9c72b2..c9e100771 100644 --- a/lib/Controller/WizardController.php +++ b/lib/Controller/WizardController.php @@ -107,7 +107,6 @@ public function cast($ldap_serverconfig_chooser, $action, $cfgkey = null, $cfgva $config = new Configuration($this->config, $prefix); switch ($action) { - case 'guessPortAndTLS': case 'guessBaseDN': case 'detectEmailAttribute': case 'detectUserDisplayNameAttribute': diff --git a/lib/Wizard.php b/lib/Wizard.php index b64351133..c22de9b36 100644 --- a/lib/Wizard.php +++ b/lib/Wizard.php @@ -652,60 +652,6 @@ public function testLoginName($loginName) { return $this->result; } - /** - * Tries to determine the port, requires given Host, User DN and Password - * @return WizardResult|false WizardResult on success, false otherwise - * @throws \Exception - */ - public function guessPortAndTLS() { - if (!$this->checkRequirements(['ldapHost', - ])) { - return false; - } - $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']; - $t = $setting['tls']; - \OCP\Util::writeLog('user_ldap', 'Wiz: trying port '. $p . ', TLS '. $t, \OCP\Util::DEBUG); - //connectAndBind may throw Exception, it needs to be catched by the - //callee of this method - - try { - $settingsFound = $this->connectAndBind($p, $t); - } catch (\Exception $e) { - // any reply other than -1 (= cannot connect) is already okay, - // because then we found the server - // unavailable startTLS returns -11 - if ($e->getCode() > 0) { - $settingsFound = true; - } else { - throw $e; - } - } - - if ($settingsFound === true) { - $config = [ - 'ldapPort' => $p, - 'ldapTLS' => (int)$t - ]; - $this->configuration->setConfiguration($config); - \OCP\Util::writeLog('user_ldap', 'Wiz: detected Port ' . $p, \OCP\Util::DEBUG); - $this->result->addChange('ldap_port', $p); - return $this->result; - } - } - - //custom port, undetected (we do not brute force) - return false; - } - /** * tries to determine a base dn from User DN or LDAP Host * @return WizardResult|false WizardResult on success, false otherwise @@ -1023,80 +969,6 @@ private function composeLdapFilter($filterType) { return $filter; } - /** - * Connects and Binds to an LDAP Server - * @param int $port the port to connect with - * @param bool $tls whether startTLS is to be used - * @param bool $ncc - * @return bool - * @throws \Exception - */ - private function connectAndBind($port = 389, $tls = false, $ncc = false) { - if ($ncc) { - //No certificate check - //FIXME: undo afterwards - \putenv('LDAPTLS_REQCERT=never'); - } - - //connect, does not really trigger any server communication - \OCP\Util::writeLog('user_ldap', 'Wiz: Checking Host Info ', \OCP\Util::DEBUG); - $host = $this->configuration->ldapHost; - $hostInfo = \parse_url($host); - if (!$hostInfo) { - throw new \Exception($this->l->t('Invalid Host')); - } - \OCP\Util::writeLog('user_ldap', 'Wiz: Attempting to connect ', \OCP\Util::DEBUG); - $cr = $this->getLDAP()->connect($host, $port); - if (!\is_resource($cr)) { - throw new \Exception($this->l->t('Invalid Host')); - } - - \OCP\Util::writeLog('user_ldap', 'Wiz: Setting LDAP Options ', \OCP\Util::DEBUG); - //set LDAP options - $this->getLDAP()->setOption($cr, LDAP_OPT_PROTOCOL_VERSION, 3); - $this->getLDAP()->setOption($cr, LDAP_OPT_REFERRALS, 0); - $this->getLDAP()->setOption($cr, LDAP_OPT_NETWORK_TIMEOUT, self::LDAP_NW_TIMEOUT); - - try { - if ($tls) { - $isTlsWorking = @$this->getLDAP()->startTls($cr); - if (!$isTlsWorking) { - return false; - } - } - - \OCP\Util::writeLog('user_ldap', 'Wiz: Attemping to Bind ', \OCP\Util::DEBUG); - //interesting part: do the bind! - $login = $this->getLDAP()->bind($cr, - $this->configuration->ldapAgentName, - $this->configuration->ldapAgentPassword - ); - $errNo = $this->getLDAP()->errno($cr); - $error = \ldap_error($cr); - $this->getLDAP()->unbind($cr); - } catch (ServerNotAvailableException $e) { - return false; - } - - if ($login === true) { - $this->getLDAP()->unbind($cr); - if ($ncc) { - throw new \Exception('Certificate cannot be validated.'); - } - \OCP\Util::writeLog('user_ldap', 'Wiz: Bind successful to Port '. $port . ' TLS ' . \intval($tls), \OCP\Util::DEBUG); - return true; - } - - if ($errNo === -1 || ($errNo === 2 && $ncc)) { - //host, port or TLS wrong - return false; - } - if ($errNo === 2) { - return $this->connectAndBind($port, $tls, true); - } - throw new \Exception($error, $errNo); - } - /** * checks whether a valid combination of agent and password has been * provided (either two values or nothing for anonymous connect) @@ -1318,48 +1190,4 @@ private function getConnection() { return false; } - - /** - * @return array - */ - private function getDefaultLdapPortSettings() { - static $settings = [ - ['port' => 7636, 'tls' => false], - ['port' => 636, 'tls' => false], - ['port' => 7389, 'tls' => true], - ['port' => 389, 'tls' => true], - ['port' => 7389, 'tls' => false], - ['port' => 389, 'tls' => false], - ]; - return $settings; - } - - /** - * @return array - */ - private function getPortSettingsToTry() { - //389 ← LDAP / Unencrypted or StartTLS - //636 ← LDAPS / SSL - //7xxx ← UCS. need to be checked first, because both ports may be open - $host = $this->configuration->ldapHost; - $port = (int)$this->configuration->ldapPort; - $portSettings = []; - - //In case the port is already provided, we will check this first - if ($port > 0) { - $hostInfo = \parse_url($host); - if (!(\is_array($hostInfo) - && isset($hostInfo['scheme']) - && \stripos($hostInfo['scheme'], 'ldaps') !== false)) { - $portSettings[] = ['port' => $port, 'tls' => true]; - } - $portSettings[] =['port' => $port, 'tls' => false]; - } - - //default ports - $portSettings = \array_merge($portSettings, - $this->getDefaultLdapPortSettings()); - - return $portSettings; - } } diff --git a/templates/part.wizard-server.php b/templates/part.wizard-server.php index 0094c824e..825b46ac8 100644 --- a/templates/part.wizard-server.php +++ b/templates/part.wizard-server.php @@ -48,16 +48,13 @@ class="ldapIconCopy icon-default-style" -
- +
diff --git a/templates/settings.php b/templates/settings.php index 9e1d2792a..daa773c41 100644 --- a/templates/settings.php +++ b/templates/settings.php @@ -18,7 +18,6 @@ 'wizard/wizardTabExpert', 'wizard/wizardDetectorQueue', 'wizard/wizardDetectorGeneric', - 'wizard/wizardDetectorPort', 'wizard/wizardDetectorBaseDN', 'wizard/wizardDetectorFeatureAbstract', 'wizard/wizardDetectorUserObjectClasses', From 60a3ad956756af361c3fe9ca1a4a1a1d9424bb97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 12 Feb 2020 13:46:26 +0100 Subject: [PATCH 3/4] Include operation timeout to prevent getting stuck if TLS connection cannot be verified because of missing CA certificate --- lib/Connection.php | 1 + lib/Wizard.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/Connection.php b/lib/Connection.php index 53e22cc2c..4259236a5 100644 --- a/lib/Connection.php +++ b/lib/Connection.php @@ -598,6 +598,7 @@ private function doConnect($host, $port) { } // Set network timeout threshold to avoid long delays when ldap server cannot be resolved $this->getLDAP()->setOption($this->ldapConnectionRes, LDAP_OPT_NETWORK_TIMEOUT, \intval($this->configuration->ldapNetworkTimeout)); + $this->getLDAP()->setOption($this->ldapConnectionRes, LDAP_OPT_TIMEOUT, \intval($this->configuration->ldapNetworkTimeout)); if (!$this->getLDAP()->isResource($this->ldapConnectionRes)) { $this->ldapConnectionRes = null; // to indicate it really is not set, connect() might have set it to false throw new ServerNotAvailableException("Connect to $host:$port failed"); diff --git a/lib/Wizard.php b/lib/Wizard.php index c22de9b36..40ca9c108 100644 --- a/lib/Wizard.php +++ b/lib/Wizard.php @@ -1176,6 +1176,7 @@ private function getConnection() { $this->getLDAP()->setOption($cr, LDAP_OPT_PROTOCOL_VERSION, 3); $this->getLDAP()->setOption($cr, LDAP_OPT_REFERRALS, 0); $this->getLDAP()->setOption($cr, LDAP_OPT_NETWORK_TIMEOUT, self::LDAP_NW_TIMEOUT); + $this->getLDAP()->setOption($cr, LDAP_OPT_TIMEOUT, self::LDAP_NW_TIMEOUT); if ($this->configuration->ldapTLS) { $this->getLDAP()->startTls($cr); } From f3cf582dc86835a58d97b9434e363851589b4a2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 12 Feb 2020 14:57:49 +0100 Subject: [PATCH 4/4] Adjust tests, remove restriction --- tests/unit/WizardTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/WizardTest.php b/tests/unit/WizardTest.php index 81d2e22c5..98b7b94b0 100644 --- a/tests/unit/WizardTest.php +++ b/tests/unit/WizardTest.php @@ -125,8 +125,7 @@ private function prepareLdapWrapperForConnections() { //dummy value, usually invalid ->will($this->returnValue(true)); - $this->ldap->expects($this->exactly(3)) - ->method('setOption') + $this->ldap->method('setOption') ->will($this->returnValue(true)); $this->ldap->expects($this->once())