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
This commit is contained in:
Jon Gyllenswärd 2019-11-05 08:58:59 +01:00 committed by GitHub
parent 5210a8f2f4
commit cce5557145
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 84 additions and 6 deletions

View File

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

View File

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