Multi-LDAP: Do not fail-fast on invalid credentials (grafana#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 grafana#19066

pkg/services/multildap/multildap.go

@@ -3,10 +3,14 @@ package multildap
 import (
 	"errors"
 
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/ldap"
 )
 
+// logger to log
+var logger = log.New("ldap")
+
 // GetConfig gets LDAP config
 var GetConfig = ldap.GetConfig
 
@@ -119,12 +123,18 @@ func (multiples *MultiLDAP) Login(query *models.LoginUserQuery) (
 			return user, nil
 		}
 
-		// Continue if we couldn't find the user
-		if err == ErrCouldNotFindUser {
-			continue
-		}
-
 		if err != nil {
+
+			if isSilentError(err) {
+				logger.Debug(
+					"unable to login with LDAP - skipping server",
+					"host", config.Host,
+					"port", config.Port,
+					"error", err,
+				)
+				continue
+			}
+
 			return nil, err
 		}
 	}
@@ -204,3 +214,17 @@ func (multiples *MultiLDAP) Users(logins []string) (
 
 	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()
 			})
 
+			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() {
 				mock := setup()