fix(authentication): ldap connection left open (#2179)

The recent ldap changes in cb71df5 left a connection to the LDAP server open at startup. This resolves this which prevents an ugly log message and unnecessary open sockets.
This commit is contained in:
James Elliott 2021-07-13 21:12:50 +10:00 committed by GitHub
parent 69bfc28a60
commit f292050822
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 331 additions and 97 deletions

View File

@ -107,6 +107,8 @@ func (p *LDAPUserProvider) checkServer() (err error) {
return err
}
defer conn.Close()
searchRequest := ldap.NewSearchRequest("", ldap.ScopeBaseObject, ldap.NeverDerefAliases,
1, 0, false, "(objectClass=*)", []string{ldapSupportedExtensionAttribute}, nil)

View File

@ -9,6 +9,7 @@ import (
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/text/encoding/unicode"
"github.com/authelia/authelia/internal/configuration/schema"
"github.com/authelia/authelia/internal/utils"
@ -28,14 +29,16 @@ func TestShouldCreateRawConnectionWhenSchemeIsLDAP(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
gomock.InOrder(dialURL, connBind)
_, err := ldapClient.connect("cn=admin,dc=example,dc=com", "password")
require.NoError(t, err)
@ -55,14 +58,16 @@ func TestShouldCreateTLSConnectionWhenSchemeIsLDAPS(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldaps://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
gomock.InOrder(dialURL, connBind)
_, err := ldapClient.connect("cn=admin,dc=example,dc=com", "password")
require.NoError(t, err)
@ -177,15 +182,15 @@ func TestShouldCheckLDAPServerExtensions(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
mockConn.EXPECT().
searchOIDs := mockConn.EXPECT().
Search(NewExtendedSearchRequestMatcher("(objectClass=*)", "", ldap.ScopeBaseObject, ldap.NeverDerefAliases, false, []string{ldapSupportedExtensionAttribute})).
Return(&ldap.SearchResult{
Entries: []*ldap.Entry{
@ -201,6 +206,10 @@ func TestShouldCheckLDAPServerExtensions(t *testing.T) {
},
}, nil)
connClose := mockConn.EXPECT().Close()
gomock.InOrder(dialURL, connBind, searchOIDs, connClose)
err := ldapClient.checkServer()
assert.NoError(t, err)
@ -229,15 +238,15 @@ func TestShouldNotEnablePasswdModifyExtension(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
mockConn.EXPECT().
searchOIDs := mockConn.EXPECT().
Search(NewExtendedSearchRequestMatcher("(objectClass=*)", "", ldap.ScopeBaseObject, ldap.NeverDerefAliases, false, []string{ldapSupportedExtensionAttribute})).
Return(&ldap.SearchResult{
Entries: []*ldap.Entry{
@ -253,6 +262,10 @@ func TestShouldNotEnablePasswdModifyExtension(t *testing.T) {
},
}, nil)
connClose := mockConn.EXPECT().Close()
gomock.InOrder(dialURL, connBind, searchOIDs, connClose)
err := ldapClient.checkServer()
assert.NoError(t, err)
@ -313,18 +326,22 @@ func TestShouldReturnCheckServerSearchError(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
mockConn.EXPECT().
searchOIDs := mockConn.EXPECT().
Search(NewExtendedSearchRequestMatcher("(objectClass=*)", "", ldap.ScopeBaseObject, ldap.NeverDerefAliases, false, []string{ldapSupportedExtensionAttribute})).
Return(nil, errors.New("could not perform the search"))
connClose := mockConn.EXPECT().Close()
gomock.InOrder(dialURL, connBind, searchOIDs, connClose)
err := ldapClient.checkServer()
assert.EqualError(t, err, "could not perform the search")
@ -447,20 +464,20 @@ func TestShouldNotCrashWhenGroupsAreNotRetrievedFromLDAP(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
mockConn.EXPECT().
Close()
connClose := mockConn.EXPECT().Close()
searchGroups := mockConn.EXPECT().
Search(gomock.Any()).
Return(createSearchResultWithAttributes(), nil)
searchProfile := mockConn.EXPECT().
Search(gomock.Any()).
Return(&ldap.SearchResult{
@ -485,7 +502,7 @@ func TestShouldNotCrashWhenGroupsAreNotRetrievedFromLDAP(t *testing.T) {
},
}, nil)
gomock.InOrder(searchProfile, searchGroups)
gomock.InOrder(dialURL, connBind, searchProfile, searchGroups, connClose)
details, err := ldapClient.GetDetails("john")
require.NoError(t, err)
@ -516,20 +533,20 @@ func TestShouldNotCrashWhenEmailsAreNotRetrievedFromLDAP(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
mockConn.EXPECT().
Close()
connClose := mockConn.EXPECT().Close()
searchGroups := mockConn.EXPECT().
Search(gomock.Any()).
Return(createSearchResultWithAttributeValues("group1", "group2"), nil)
searchProfile := mockConn.EXPECT().
Search(gomock.Any()).
Return(&ldap.SearchResult{
@ -546,7 +563,7 @@ func TestShouldNotCrashWhenEmailsAreNotRetrievedFromLDAP(t *testing.T) {
},
}, nil)
gomock.InOrder(searchProfile, searchGroups)
gomock.InOrder(dialURL, connBind, searchProfile, searchGroups, connClose)
details, err := ldapClient.GetDetails("john")
require.NoError(t, err)
@ -578,20 +595,20 @@ func TestShouldReturnUsernameFromLDAP(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
mockConn.EXPECT().
Close()
connClose := mockConn.EXPECT().Close()
searchGroups := mockConn.EXPECT().
Search(gomock.Any()).
Return(createSearchResultWithAttributeValues("group1", "group2"), nil)
searchProfile := mockConn.EXPECT().
Search(gomock.Any()).
Return(&ldap.SearchResult{
@ -616,7 +633,7 @@ func TestShouldReturnUsernameFromLDAP(t *testing.T) {
},
}, nil)
gomock.InOrder(searchProfile, searchGroups)
gomock.InOrder(dialURL, connBind, searchProfile, searchGroups, connClose)
details, err := ldapClient.GetDetails("john")
require.NoError(t, err)
@ -627,7 +644,7 @@ func TestShouldReturnUsernameFromLDAP(t *testing.T) {
assert.Equal(t, details.Username, "John")
}
func TestShouldUpdateUserPassword(t *testing.T) {
func TestShouldUpdateUserPasswordPasswdModifyExtension(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
@ -655,15 +672,15 @@ func TestShouldUpdateUserPassword(t *testing.T) {
"password",
)
gomock.InOrder(
mockFactory.EXPECT().
dialURLOIDs := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil),
mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil),
Return(mockConn, nil)
mockConn.EXPECT().
connBindOIDs := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
searchOIDs := mockConn.EXPECT().
Search(NewExtendedSearchRequestMatcher("(objectClass=*)", "", ldap.ScopeBaseObject, ldap.NeverDerefAliases, false, []string{ldapSupportedExtensionAttribute})).
Return(&ldap.SearchResult{
Entries: []*ldap.Entry{
@ -677,15 +694,21 @@ func TestShouldUpdateUserPassword(t *testing.T) {
},
},
},
}, nil),
}, nil)
mockFactory.EXPECT().
connCloseOIDs := mockConn.EXPECT().Close()
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil),
mockConn.EXPECT().
Return(mockConn, nil)
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil),
mockConn.EXPECT().
Return(nil)
connClose := mockConn.EXPECT().Close()
searchProfile := mockConn.EXPECT().
Search(gomock.Any()).
Return(&ldap.SearchResult{
Entries: []*ldap.Entry{
@ -707,14 +730,223 @@ func TestShouldUpdateUserPassword(t *testing.T) {
},
},
},
}, nil),
mockConn.EXPECT().
}, nil)
passwdModify := mockConn.EXPECT().
PasswordModify(pwdModifyRequest).
Return(nil),
mockConn.EXPECT().
Close(),
Return(nil)
gomock.InOrder(dialURLOIDs, connBindOIDs, searchOIDs, connCloseOIDs, dialURL, connBind, searchProfile, passwdModify, connClose)
err := ldapClient.checkServer()
require.NoError(t, err)
err = ldapClient.UpdatePassword("john", "password")
require.NoError(t, err)
}
func TestShouldUpdateUserPasswordActiveDirectory(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockFactory := NewMockLDAPConnectionFactory(ctrl)
mockConn := NewMockLDAPConnection(ctrl)
ldapClient := newLDAPUserProvider(
schema.LDAPAuthenticationBackendConfiguration{
Implementation: "activedirectory",
URL: "ldap://127.0.0.1:389",
User: "cn=admin,dc=example,dc=com",
Password: "password",
UsernameAttribute: "sAMAccountName",
MailAttribute: "mail",
DisplayNameAttribute: "displayName",
UsersFilter: "cn={input}",
AdditionalUsersDN: "ou=users",
BaseDN: "dc=example,dc=com",
},
nil,
mockFactory)
utf16 := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM)
pwdEncoded, _ := utf16.NewEncoder().String("\"password\"")
modifyRequest := ldap.NewModifyRequest(
"cn=test,dc=example,dc=com",
nil,
)
modifyRequest.Replace("unicodePwd", []string{pwdEncoded})
dialURLOIDs := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
connBindOIDs := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
searchOIDs := mockConn.EXPECT().
Search(NewExtendedSearchRequestMatcher("(objectClass=*)", "", ldap.ScopeBaseObject, ldap.NeverDerefAliases, false, []string{ldapSupportedExtensionAttribute})).
Return(&ldap.SearchResult{
Entries: []*ldap.Entry{
{
DN: "",
Attributes: []*ldap.EntryAttribute{
{
Name: ldapSupportedExtensionAttribute,
Values: []string{},
},
},
},
},
}, nil)
connCloseOIDs := mockConn.EXPECT().Close()
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
connClose := mockConn.EXPECT().Close()
searchProfile := mockConn.EXPECT().
Search(gomock.Any()).
Return(&ldap.SearchResult{
Entries: []*ldap.Entry{
{
DN: "cn=test,dc=example,dc=com",
Attributes: []*ldap.EntryAttribute{
{
Name: "displayname",
Values: []string{"John Doe"},
},
{
Name: "mail",
Values: []string{"test@example.com"},
},
{
Name: "uid",
Values: []string{"John"},
},
},
},
},
}, nil)
passwdModify := mockConn.EXPECT().
Modify(modifyRequest).
Return(nil)
gomock.InOrder(dialURLOIDs, connBindOIDs, searchOIDs, connCloseOIDs, dialURL, connBind, searchProfile, passwdModify, connClose)
err := ldapClient.checkServer()
require.NoError(t, err)
err = ldapClient.UpdatePassword("john", "password")
require.NoError(t, err)
}
func TestShouldUpdateUserPasswordBasic(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockFactory := NewMockLDAPConnectionFactory(ctrl)
mockConn := NewMockLDAPConnection(ctrl)
ldapClient := newLDAPUserProvider(
schema.LDAPAuthenticationBackendConfiguration{
Implementation: "custom",
URL: "ldap://127.0.0.1:389",
User: "uid=admin,dc=example,dc=com",
Password: "password",
UsernameAttribute: "uid",
MailAttribute: "mail",
DisplayNameAttribute: "displayName",
UsersFilter: "uid={input}",
AdditionalUsersDN: "ou=users",
BaseDN: "dc=example,dc=com",
},
nil,
mockFactory)
modifyRequest := ldap.NewModifyRequest(
"uid=test,dc=example,dc=com",
nil,
)
modifyRequest.Replace("userPassword", []string{"password"})
dialURLOIDs := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
connBindOIDs := mockConn.EXPECT().
Bind(gomock.Eq("uid=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
searchOIDs := mockConn.EXPECT().
Search(NewExtendedSearchRequestMatcher("(objectClass=*)", "", ldap.ScopeBaseObject, ldap.NeverDerefAliases, false, []string{ldapSupportedExtensionAttribute})).
Return(&ldap.SearchResult{
Entries: []*ldap.Entry{
{
DN: "",
Attributes: []*ldap.EntryAttribute{
{
Name: ldapSupportedExtensionAttribute,
Values: []string{},
},
},
},
},
}, nil)
connCloseOIDs := mockConn.EXPECT().Close()
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
connBind := mockConn.EXPECT().
Bind(gomock.Eq("uid=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
connClose := mockConn.EXPECT().Close()
searchProfile := mockConn.EXPECT().
Search(gomock.Any()).
Return(&ldap.SearchResult{
Entries: []*ldap.Entry{
{
DN: "uid=test,dc=example,dc=com",
Attributes: []*ldap.EntryAttribute{
{
Name: "displayName",
Values: []string{"John Doe"},
},
{
Name: "mail",
Values: []string{"test@example.com"},
},
{
Name: "uid",
Values: []string{"John"},
},
},
},
},
}, nil)
passwdModify := mockConn.EXPECT().
Modify(modifyRequest).
Return(nil)
gomock.InOrder(dialURLOIDs, connBindOIDs, searchOIDs, connCloseOIDs, dialURL, connBind, searchProfile, passwdModify, connClose)
err := ldapClient.checkServer()
require.NoError(t, err)
@ -780,8 +1012,7 @@ func TestShouldCheckValidUserPassword(t *testing.T) {
mockConn.EXPECT().
Bind(gomock.Eq("uid=test,dc=example,dc=com"), gomock.Eq("password")).
Return(nil),
mockConn.EXPECT().
Close().Times(2),
mockConn.EXPECT().Close().Times(2),
)
valid, err := ldapClient.CheckUserPassword("john", "password")
@ -848,8 +1079,7 @@ func TestShouldCheckInvalidUserPassword(t *testing.T) {
mockConn.EXPECT().
Bind(gomock.Eq("uid=test,dc=example,dc=com"), gomock.Eq("password")).
Return(errors.New("Invalid username or password")),
mockConn.EXPECT().
Close(),
mockConn.EXPECT().Close(),
)
valid, err := ldapClient.CheckUserPassword("john", "password")
@ -881,23 +1111,23 @@ func TestShouldCallStartTLSWhenEnabled(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
mockConn.EXPECT().
connStartTLS := mockConn.EXPECT().
StartTLS(ldapClient.tlsConfig)
mockConn.EXPECT().
Close()
connClose := mockConn.EXPECT().Close()
searchGroups := mockConn.EXPECT().
Search(gomock.Any()).
Return(createSearchResultWithAttributes(), nil)
searchProfile := mockConn.EXPECT().
Search(gomock.Any()).
Return(&ldap.SearchResult{
@ -922,7 +1152,7 @@ func TestShouldCallStartTLSWhenEnabled(t *testing.T) {
},
}, nil)
gomock.InOrder(searchProfile, searchGroups)
gomock.InOrder(dialURL, connStartTLS, connBind, searchProfile, searchGroups, connClose)
details, err := ldapClient.GetDetails("john")
require.NoError(t, err)
@ -989,23 +1219,23 @@ func TestShouldCallStartTLSWithInsecureSkipVerifyWhenSkipVerifyTrue(t *testing.T
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldap://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connBind := mockConn.EXPECT().
Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
Return(nil)
mockConn.EXPECT().
connStartTLS := mockConn.EXPECT().
StartTLS(ldapClient.tlsConfig)
mockConn.EXPECT().
Close()
connClose := mockConn.EXPECT().Close()
searchGroups := mockConn.EXPECT().
Search(gomock.Any()).
Return(createSearchResultWithAttributes(), nil)
searchProfile := mockConn.EXPECT().
Search(gomock.Any()).
Return(&ldap.SearchResult{
@ -1030,7 +1260,7 @@ func TestShouldCallStartTLSWithInsecureSkipVerifyWhenSkipVerifyTrue(t *testing.T
},
}, nil)
gomock.InOrder(searchProfile, searchGroups)
gomock.InOrder(dialURL, connStartTLS, connBind, searchProfile, searchGroups, connClose)
details, err := ldapClient.GetDetails("john")
require.NoError(t, err)
@ -1067,14 +1297,16 @@ func TestShouldReturnLDAPSAlreadySecuredWhenStartTLSAttempted(t *testing.T) {
nil,
mockFactory)
mockFactory.EXPECT().
dialURL := mockFactory.EXPECT().
DialURL(gomock.Eq("ldaps://127.0.0.1:389"), gomock.Any()).
Return(mockConn, nil)
mockConn.EXPECT().
connStartTLS := mockConn.EXPECT().
StartTLS(ldapClient.tlsConfig).
Return(errors.New("LDAP Result Code 200 \"Network Error\": ldap: already encrypted"))
gomock.InOrder(dialURL, connStartTLS)
_, err := ldapClient.GetDetails("john")
assert.EqualError(t, err, "LDAP Result Code 200 \"Network Error\": ldap: already encrypted")
}