Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP backup server not called when LDAP main server fails #10160

Closed
GitHubUser4234 opened this issue Jul 9, 2018 · 3 comments
Closed

LDAP backup server not called when LDAP main server fails #10160

GitHubUser4234 opened this issue Jul 9, 2018 · 3 comments

Comments

@GitHubUser4234
Copy link
Contributor

GitHubUser4234 commented Jul 9, 2018

Hi,

#10031 does its job well, but unfortunately it seems to create another issue, which is that the LDAP backup server is not called when the LDAP main server fails. To reproduce, make sure that main and backup LDAP server settings point to LDAP servers that are accessible. Then change the value of the main server setting to some invalid domain/port. Now LDAP users cannot login to Nextcloud anymore. Then replace the apps/user_ldap/lib/Connection.php with a version before the #10031 patch, Nextcloud is working normally again.

Looks like in apps/user_ldap/lib/Connection.php there is an infinite loop (establishConnection() => bind() => getConnectionResource() => establishConnection()) once the primary server becomes inaccessible.

Please have a look, thanks!

@nextcloud-bot
Copy link
Member

GitMate.io thinks possibly related issues are #10031 (LDAP backup server should not be queried when auth fails), #3074 (Can't add LDAP server), #10032 ([stable13] LDAP backup server should not be queried when auth fails), #772 (LDAP Users not mapped and authentication fails), and #272 (LDAP Users not mapped and authentication fails).

@juliushaertl
Copy link
Member

I could reproduce this and I prepared a fix at #10227

The infinite loop was caused by calling bind even if there were no connection details set. Without those there was no connection resource available so it tried to get those for the main server again by calling establishConnection().

@GitHubUser4234 Could you try if the following patch fixes the issue for you:

diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php
index 039e46aeef..8bbe034585 100644
--- a/apps/user_ldap/lib/Connection.php
+++ b/apps/user_ldap/lib/Connection.php
@@ -564,8 +564,8 @@ class Connection extends LDAPUtility {
 				if (!$isOverrideMainServer) {
 					$this->doConnect($this->configuration->ldapHost,
 						$this->configuration->ldapPort);
+					return $this->bind();
 				}
-				return $this->bind();
 			} catch (ServerNotAvailableException $e) {
 				if(!$isBackupHost) {
 					throw $e;

@MorrisJobke MorrisJobke added this to the Nextcloud 14 milestone Jul 13, 2018
@MorrisJobke
Copy link
Member

Fixed by #10227

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants