Merge pull request #35229 from nextcloud/feat/user_ldap-specific-serv…

…er-for-background Use a dedicated LDAP host and port for background jobs if configured
nextcloud · Dec 20, 2022 · 4d04030 · 4d04030
2 parents e50efc6 + 6b7ffcd
commit 4d04030
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 28 deletions.
diff --git 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 @@ public function saveConfiguration(): void {
  $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 @@ protected function getValue(string $varName): string {
  $defaults = $this->getDefaults();
  }
  return \OC::$server->getConfig()->getAppValue('user_ldap',
-  $this->configPrefix.$varName,
-  $defaults[$varName]);
+ $this->configPrefix.$varName,
+ $defaults[$varName]);
  }
 
  /**
@@ -413,6 +415,8 @@ public function getDefaults(): array {
  '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 @@ public function getConfigTranslationArray(): array {
  '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
@@ -598,19 +598,26 @@ private function establishConnection() {
  }
  }
 
- $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 @@ private function establishConnection() {
  );
  }
 
- //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
@@ -121,7 +121,7 @@ public function testUseBackupServer() {
  ->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));