Merge pull request #35229 from nextcloud/feat/user_ldap-specific-server-for-background

Use a dedicated LDAP host and port for background jobs if configured

3 files changed, 38 insertions, 28 deletions

diff --git a/apps/user_ldap/lib/Configuration.php b/apps/user_ldap/lib/Configuration.php
index f2090291d32..59fac50b90b 100644
--- a/apps/user_ldap/lib/Configuration.php
+++ b/apps/user_ldap/lib/Configuration.php
@@ -68,6 +68,8 @@ class Configuration {
 		'ldapPort' => null,
 		'ldapBackupHost' => null,
 		'ldapBackupPort' => null,
+		'ldapBackgroundHost' => null,
+		'ldapBackgroundPort' => null,
 		'ldapBase' => null,
 		'ldapBaseUsers' => null,
 		'ldapBaseGroups' => null,
@@ -278,7 +280,7 @@ class Configuration {
 						$value = implode("\n", $value);
 					}
 					break;
-				//following options are not stored but detected, skip them
+					//following options are not stored but detected, skip them
 				case 'ldapIgnoreNamingRules':
 				case 'ldapUuidUserAttribute':
 				case 'ldapUuidGroupAttribute':
@@ -367,8 +369,8 @@ class Configuration {
 			$defaults = $this->getDefaults();
 		}
 		return \OC::$server->getConfig()->getAppValue('user_ldap',
-										$this->configPrefix.$varName,
-										$defaults[$varName]);
+			$this->configPrefix.$varName,
+			$defaults[$varName]);
 	}
 
 	/**
@@ -413,6 +415,8 @@ class Configuration {
 			'ldap_port' => '',
 			'ldap_backup_host' => '',
 			'ldap_backup_port' => '',
+			'ldap_background_host' => '',
+			'ldap_background_port' => '',
 			'ldap_override_main_server' => '',
 			'ldap_dn' => '',
 			'ldap_agent_password' => '',
@@ -478,6 +482,8 @@ class Configuration {
 			'ldap_port' => 'ldapPort',
 			'ldap_backup_host' => 'ldapBackupHost',
 			'ldap_backup_port' => 'ldapBackupPort',
+			'ldap_background_host' => 'ldapBackgroundHost',
+			'ldap_background_port' => 'ldapBackgroundPort',
 			'ldap_override_main_server' => 'ldapOverrideMainServer',
 			'ldap_dn' => 'ldapAgentName',
 			'ldap_agent_password' => 'ldapAgentPassword',
diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php
index 0ebca44082e..95ddd3fc51c 100644
--- a/apps/user_ldap/lib/Connection.php
+++ b/apps/user_ldap/lib/Connection.php
@@ -598,19 +598,26 @@ class Connection extends LDAPUtility {
 				}
 			}
 
-			$isOverrideMainServer = ($this->configuration->ldapOverrideMainServer
-				|| $this->getFromCache('overrideMainServer'));
-			$isBackupHost = (trim($this->configuration->ldapBackupHost) !== "");
+			$hasBackupHost = (trim($this->configuration->ldapBackupHost ?? '') !== '');
+			$hasBackgroundHost = (trim($this->configuration->ldapBackgroundHost ?? '') !== '');
+			$useBackgroundHost = (\OC::$CLI && $hasBackgroundHost);
+			$overrideCacheKey = ($useBackgroundHost ? 'overrideBackgroundServer' : 'overrideMainServer');
+			$forceBackupHost = ($this->configuration->ldapOverrideMainServer || $this->getFromCache($overrideCacheKey));
 			$bindStatus = false;
-			try {
-				if (!$isOverrideMainServer) {
-					$this->doConnect($this->configuration->ldapHost,
-						$this->configuration->ldapPort);
+			if (!$forceBackupHost) {
+				try {
+					$host = $this->configuration->ldapHost ?? '';
+					$port = $this->configuration->ldapPort ?? '';
+					if ($useBackgroundHost) {
+						$host = $this->configuration->ldapBackgroundHost ?? '';
+						$port = $this->configuration->ldapBackgroundPort ?? '';
+					}
+					$this->doConnect($host, $port);
 					return $this->bind();
-				}
-			} catch (ServerNotAvailableException $e) {
-				if (!$isBackupHost) {
-					throw $e;
+				} catch (ServerNotAvailableException $e) {
+					if (!$hasBackupHost) {
+						throw $e;
+					}
 				}
 				$this->logger->warning(
 					'Main LDAP not reachable, connecting to backup',
@@ -620,19 +627,16 @@ class Connection extends LDAPUtility {
 				);
 			}
 
-			//if LDAP server is not reachable, try the Backup (Replica!) Server
-			if ($isBackupHost || $isOverrideMainServer) {
-				$this->doConnect($this->configuration->ldapBackupHost,
-					$this->configuration->ldapBackupPort);
-				$this->bindResult = [];
-				$bindStatus = $this->bind();
-				$error = $this->ldap->isResource($this->ldapConnectionRes) ?
-					$this->ldap->errno($this->ldapConnectionRes) : -1;
-				if ($bindStatus && $error === 0 && !$this->getFromCache('overrideMainServer')) {
-					//when bind to backup server succeeded and failed to main server,
-					//skip contacting him until next cache refresh
-					$this->writeToCache('overrideMainServer', true);
-				}
+			// if LDAP server is not reachable, try the Backup (Replica!) Server
+			$this->doConnect($this->configuration->ldapBackupHost ?? '', $this->configuration->ldapBackupPort ?? '');
+			$this->bindResult = [];
+			$bindStatus = $this->bind();
+			$error = $this->ldap->isResource($this->ldapConnectionRes) ?
+				$this->ldap->errno($this->ldapConnectionRes) : -1;
+			if ($bindStatus && $error === 0 && !$forceBackupHost) {
+				//when bind to backup server succeeded and failed to main server,
+				//skip contacting him until next cache refresh
+				$this->writeToCache($overrideCacheKey, true);
 			}
 
 			return $bindStatus;
diff --git a/apps/user_ldap/tests/ConnectionTest.php b/apps/user_ldap/tests/ConnectionTest.php
index 3b771c6d04f..b4e26c922ed 100644
--- a/apps/user_ldap/tests/ConnectionTest.php
+++ b/apps/user_ldap/tests/ConnectionTest.php
@@ -121,7 +121,7 @@ class ConnectionTest extends \Test\TestCase {
 			->willReturn(0);
 
 		// Not called often enough? Then, the fallback to the backup server is broken.
-		$this->connection->expects($this->exactly(4))
+		$this->connection->expects($this->exactly(2))
 			->method('getFromCache')
 			->with('overrideMainServer')
 			->will($this->onConsecutiveCalls(false, false, true, true));