LDAP: All LDAP servers should be tried even if one of them returns a connection error (grafana#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

Author: Jon Gyllenswärd

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,
+	)
+}

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