Multi-LDAP: Do not fail-fast on invalid credentials (#19261)

* Multi-LDAP: Do not fail-fast on invalid credentials

When configuring LDAP authentication, it is very common to have multiple
servers configured. When using user bind (authenticating with LDAP using
the same credentials as the user authenticating to Grafana) we don't
expect all the users to be on all LDAP servers.

Because of this use-case, we should not fail-fast when authenticating on
multiple LDAP server configurations. Instead, we should continue to try
the credentials with the next LDAP server configured.

Fixes #19066

pkg/services/multildap/multildap.go

@@ -3,10 +3,14 @@ package multildap
 import (
 import (
 	"errors"
 	"errors"
 
 
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/ldap"
 	"github.com/grafana/grafana/pkg/services/ldap"
 )
 )
 
 
+// logger to log
+var logger = log.New("ldap")
+
 // GetConfig gets LDAP config
 // GetConfig gets LDAP config
 var GetConfig = ldap.GetConfig
 var GetConfig = ldap.GetConfig
 
 
@@ -119,12 +123,18 @@ func (multiples *MultiLDAP) Login(query *models.LoginUserQuery) (
 			return user, nil
 			return user, nil
 		}
 		}
 
 
-		// Continue if we couldn't find the user
-		if err == ErrCouldNotFindUser {
+		if err != nil {
+
+			if isSilentError(err) {
+				logger.Debug(
+					"unable to login with LDAP - skipping server",
+					"host", config.Host,
+					"port", config.Port,
+					"error", err,
+				)
 				continue
 				continue
 			}
 			}
 
 
-		if err != nil {
 			return nil, err
 			return nil, err
 		}
 		}
 	}
 	}
@@ -204,3 +214,17 @@ func (multiples *MultiLDAP) Users(logins []string) (
 
 
 	return result, nil
 	return result, nil
 }
 }
+
+// isSilentError evaluates an error and tells whenever we should fail the LDAP request
+// immediately or if we should continue into other LDAP servers
+func isSilentError(err error) bool {
+	continueErrs := []error{ErrInvalidCredentials, ErrCouldNotFindUser}
+
+	for _, cerr := range continueErrs {
+		if err == cerr {
+			return true
+		}
+	}
+
+	return false
+}

pkg/services/multildap/multildap_test.go

@@ -152,6 +152,25 @@ func TestMultiLDAP(t *testing.T) {
 				teardown()
 				teardown()
 			})
 			})
 
 
+			Convey("Should still try to auth with the second server after receiving an invalid credentials error from the first", func() {
+				mock := setup()
+
+				mock.loginErrReturn = ErrInvalidCredentials
+
+				multi := New([]*ldap.ServerConfig{
+					{}, {},
+				})
+				_, err := multi.Login(&models.LoginUserQuery{})
+
+				So(mock.dialCalledTimes, ShouldEqual, 2)
+				So(mock.loginCalledTimes, ShouldEqual, 2)
+				So(mock.closeCalledTimes, ShouldEqual, 2)
+
+				So(err, ShouldEqual, ErrInvalidCredentials)
+
+				teardown()
+			})
+
 			Convey("Should return unknown error", func() {
 			Convey("Should return unknown error", func() {
 				mock := setup()
 				mock := setup()