Merge pull request #512 from owncloud/easier_tls

Easier tls

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());

js/wizard/wizardTabElementary.js

@@ -38,8 +38,11 @@ OCA = OCA || {};
 				},
 				ldap_port: {
 					$element: $('#ldap_port'),
-					setMethod: 'setPort',
-					$relatedElements: $('.ldapDetectPort')
+					setMethod: 'setPort'
+				},
+				ldap_tls: {
+					$element: $('#ldap_tls'),
+					setMethod: 'setTLS'
 				},
 				ldap_dn: {
 					$element: $('#ldap_dn'),
@@ -65,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);
 		},
@@ -103,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);
-			}
 		},
 
 		/**
@@ -119,6 +116,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
 		 *
@@ -205,7 +209,6 @@ OCA = OCA || {};
 		 */
 		onConfigSwitch: function(view, configuration) {
 			this.baseDNTestTriggered = false;
-			view.disableElement(view.managedItems.ldap_port.$relatedElements);
 			view.onConfigLoaded(view, configuration);
 		},
 
@@ -290,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
 		 *

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");

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':

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)
@@ -1304,7 +1176,8 @@ 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) {
+		$this->getLDAP()->setOption($cr, LDAP_OPT_TIMEOUT, self::LDAP_NW_TIMEOUT);
+		if ($this->configuration->ldapTLS) {
 			$this->getLDAP()->startTls($cr);
 		}
 
@@ -1318,48 +1191,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;
-	}
 }

templates/part.wizard-server.php

@@ -48,13 +48,16 @@ class="ldapIconCopy icon-default-style"
 						<span class="hostPortCombinatorSpan">
 							<input type="number" id="ldap_port" name="ldap_port"
 								placeholder="<?php p($l->t('Port'));?>" />
-							<button class="ldapDetectPort" name="ldapDetectPort" type="button">
-								<?php p($l->t('Detect Port'));?>
-							</button>
 						</span>
 					</div>
 				</div>
 			</div>
+
+			<div class="tablerow">
+				<input type="checkbox" id="ldap_tls" name="ldap_tls" title="<?php p($l->t('Enable StartTLS support (also known as LDAP over TLS) for the connection.  Note that this is different than LDAPS (LDAP over SSL) which doesn\'t need this checkbox checked. You\'ll need to import the LDAP server\'s certificate in your %s server.', $theme->getName()));?>" value="1">
+				<label for="ldap_tls"><?php p($l->t('Use StartTLS support'));?></label>
+			</div>
+
 			<div class="tablerow">
 				<input type="text" id="ldap_dn" name="ldap_dn"
 				class="tablecell"

templates/settings.php

@@ -18,7 +18,6 @@
 	'wizard/wizardTabExpert',
 	'wizard/wizardDetectorQueue',
 	'wizard/wizardDetectorGeneric',
-	'wizard/wizardDetectorPort',
 	'wizard/wizardDetectorBaseDN',
 	'wizard/wizardDetectorFeatureAbstract',
 	'wizard/wizardDetectorUserObjectClasses',

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())