From cce5557145d10c5306c5d03162043583e180114f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jon=20Gyllensw=C3=A4rd?= <jon.gyllensward@grafana.com>
Date: Tue, 5 Nov 2019 08:58:59 +0100
Subject: [PATCH] LDAP: All LDAP servers should be tried even if one of them
 returns a connection error (#20077)

All ldap servers are now being tried and the first one that gives back an answer is used if a previous one is failing. Applies to login and syncing
---
 pkg/services/multildap/multildap.go      | 39 +++++++++++++++---
 pkg/services/multildap/multildap_test.go | 51 ++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/pkg/services/multildap/multildap.go b/pkg/services/multildap/multildap.go
index fa84c852344..90964ddd7cb 100644
--- a/pkg/services/multildap/multildap.go
+++ b/pkg/services/multildap/multildap.go
@@ -109,11 +109,17 @@ func (multiples *MultiLDAP) Login(query *models.LoginUserQuery) (
 		return nil, ErrNoLDAPServers
 	}
 
-	for _, config := range multiples.configs {
+	for index, config := range multiples.configs {
 		server := newLDAP(config)
 
 		if err := server.Dial(); err != nil {
-			return nil, err
+			logDialFailure(err, config)
+
+			// Only return an error if it is the last server so we can try next server
+			if index == len(multiples.configs)-1 {
+				return nil, err
+			}
+			continue
 		}
 
 		defer server.Close()
@@ -155,11 +161,17 @@ func (multiples *MultiLDAP) User(login string) (
 	}
 
 	search := []string{login}
-	for _, config := range multiples.configs {
+	for index, config := range multiples.configs {
 		server := newLDAP(config)
 
 		if err := server.Dial(); err != nil {
-			return nil, *config, err
+			logDialFailure(err, config)
+
+			// Only return an error if it is the last server so we can try next server
+			if index == len(multiples.configs)-1 {
+				return nil, *config, err
+			}
+			continue
 		}
 
 		defer server.Close()
@@ -192,11 +204,17 @@ func (multiples *MultiLDAP) Users(logins []string) (
 		return nil, ErrNoLDAPServers
 	}
 
-	for _, config := range multiples.configs {
+	for index, config := range multiples.configs {
 		server := newLDAP(config)
 
 		if err := server.Dial(); err != nil {
-			return nil, err
+			logDialFailure(err, config)
+
+			// Only return an error if it is the last server so we can try next server
+			if index == len(multiples.configs)-1 {
+				return nil, err
+			}
+			continue
 		}
 
 		defer server.Close()
@@ -228,3 +246,12 @@ func isSilentError(err error) bool {
 
 	return false
 }
+
+func logDialFailure(err error, config *ldap.ServerConfig) {
+	logger.Debug(
+		"unable to dial LDAP server",
+		"host", config.Host,
+		"port", config.Port,
+		"error", err,
+	)
+}
diff --git a/pkg/services/multildap/multildap_test.go b/pkg/services/multildap/multildap_test.go
index 81fcdca732a..467bebd4dd6 100644
--- a/pkg/services/multildap/multildap_test.go
+++ b/pkg/services/multildap/multildap_test.go
@@ -171,6 +171,24 @@ func TestMultiLDAP(t *testing.T) {
 				teardown()
 			})
 
+			Convey("Should still try to auth with the second server after receiving a dial error from the first", func() {
+				mock := setup()
+
+				expectedError := errors.New("Dial error")
+				mock.dialErrReturn = expectedError
+
+				multi := New([]*ldap.ServerConfig{
+					{}, {},
+				})
+				_, err := multi.Login(&models.LoginUserQuery{})
+
+				So(mock.dialCalledTimes, ShouldEqual, 2)
+
+				So(err, ShouldEqual, expectedError)
+
+				teardown()
+			})
+
 			Convey("Should return unknown error", func() {
 				mock := setup()
 
@@ -287,9 +305,42 @@ func TestMultiLDAP(t *testing.T) {
 
 				teardown()
 			})
+
+			Convey("Should still try to auth with the second server after receiving a dial error from the first", func() {
+				mock := setup()
+
+				expectedError := errors.New("Dial error")
+				mock.dialErrReturn = expectedError
+
+				multi := New([]*ldap.ServerConfig{
+					{}, {},
+				})
+				_, _, err := multi.User("test")
+
+				So(mock.dialCalledTimes, ShouldEqual, 2)
+				So(err, ShouldEqual, expectedError)
+
+				teardown()
+			})
 		})
 
 		Convey("Users()", func() {
+			Convey("Should still try to auth with the second server after receiving a dial error from the first", func() {
+				mock := setup()
+
+				expectedError := errors.New("Dial error")
+				mock.dialErrReturn = expectedError
+
+				multi := New([]*ldap.ServerConfig{
+					{}, {},
+				})
+				_, err := multi.Users([]string{"test"})
+
+				So(mock.dialCalledTimes, ShouldEqual, 2)
+				So(err, ShouldEqual, expectedError)
+
+				teardown()
+			})
 			Convey("Should return error for absent config list", func() {
 				setup()