diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index 4e06ddc72ce..bf2b23a9e24 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -47,6 +47,7 @@ var loginUsingLDAP = func(ctx context.Context, query *models.LoginUserQuery, log ldapLogger.Debug("Failed to disable external user", "err", err) } + // Return invalid credentials if we couldn't find the user anywhere return true, ldap.ErrInvalidCredentials } diff --git a/pkg/services/multildap/multildap.go b/pkg/services/multildap/multildap.go index 3fa28c908c8..b9ea8bd9ef3 100644 --- a/pkg/services/multildap/multildap.go +++ b/pkg/services/multildap/multildap.go @@ -106,6 +106,8 @@ func (multiples *MultiLDAP) Login(query *models.LoginUserQuery) ( return nil, ErrNoLDAPServers } + ldapSilentErrors := []error{} + for index, config := range multiples.configs { server := newLDAP(config) @@ -122,12 +124,9 @@ func (multiples *MultiLDAP) Login(query *models.LoginUserQuery) ( defer server.Close() user, err := server.Login(query) - // FIXME - if user != nil { - return user, nil - } if err != nil { if isSilentError(err) { + ldapSilentErrors = append(ldapSilentErrors, err) logger.Debug( "unable to login with LDAP - skipping server", "host", config.Host, @@ -139,10 +138,21 @@ func (multiples *MultiLDAP) Login(query *models.LoginUserQuery) ( return nil, err } + + if user != nil { + return user, nil + } } - // Return invalid credentials if we couldn't find the user anywhere - return nil, ErrInvalidCredentials + // Return ErrInvalidCredentials in case any of the errors was ErrInvalidCredentials (means that the authentication has failed at least once) + for _, ldapErr := range ldapSilentErrors { + if errors.Is(ldapErr, ErrInvalidCredentials) { + return nil, ErrInvalidCredentials + } + } + + // Return ErrCouldNotFindUser if all of the configured LDAP servers returned with ErrCouldNotFindUser + return nil, ErrCouldNotFindUser } // User attempts to find an user by login/username by searching into all of the configured LDAP servers. Then, if the user is found it returns the user alongisde the server it was found. diff --git a/pkg/services/multildap/multildap_test.go b/pkg/services/multildap/multildap_test.go index aa103c828ed..cbf33532b2d 100644 --- a/pkg/services/multildap/multildap_test.go +++ b/pkg/services/multildap/multildap_test.go @@ -99,6 +99,7 @@ func TestMultiLDAP(t *testing.T) { t.Run("Should call underlying LDAP methods", func(t *testing.T) { mock := setup() + mock.loginErrReturn = ErrInvalidCredentials multi := New([]*ldap.ServerConfig{ {}, {}, @@ -109,7 +110,7 @@ func TestMultiLDAP(t *testing.T) { require.Equal(t, 2, mock.loginCalledTimes) require.Equal(t, 2, mock.closeCalledTimes) - require.Equal(t, ErrInvalidCredentials, err) + require.Equal(t, ldap.ErrInvalidCredentials, err) teardown() }) @@ -150,7 +151,7 @@ func TestMultiLDAP(t *testing.T) { require.Equal(t, 2, mock.loginCalledTimes) require.Equal(t, 2, mock.closeCalledTimes) - require.Equal(t, ErrInvalidCredentials, err) + require.Equal(t, ErrCouldNotFindUser, err) teardown() })