[BUGFIX] Fix crash when no emails or groups are retrieved from LDAP. (#651)

* [BUGFIX] Fix crash when no emails or groups are retrieved from LDAP.

If group or email attribute configured by user in configuration is not
found in user object the list of attributes in LDAP search result is empty.
This change introduces a check before accessing the first element of the
list which previously led to out of bound access.

Fixes #647.

* [MISC] Change log level of LDAP connection creation to trace.

internal/authentication/ldap_user_provider.go

@@ -44,7 +44,7 @@ func (p *LDAPUserProvider) connect(userDN string, password string) (LDAPConnecti
 	}
 
 	if url.Scheme == "ldaps" {
-		logging.Logger().Debug("LDAP client starts a TLS session")
+		logging.Logger().Trace("LDAP client starts a TLS session")
 		conn, err := p.connectionFactory.DialTLS("tcp", url.Host, &tls.Config{
 			InsecureSkipVerify: p.configuration.SkipVerify,
 		})
@@ -53,7 +53,7 @@ func (p *LDAPUserProvider) connect(userDN string, password string) (LDAPConnecti
 		}
 		newConnection = conn
 	} else {
-		logging.Logger().Debug("LDAP client starts a session over raw TCP")
+		logging.Logger().Trace("LDAP client starts a session over raw TCP")
 		conn, err := p.connectionFactory.Dial("tcp", url.Host)
 		if err != nil {
 			return nil, err
@@ -214,8 +214,11 @@ func (p *LDAPUserProvider) GetDetails(username string) (*UserDetails, error) {
 	}
 
 	groups := make([]string, 0)
-
 	for _, res := range sr.Entries {
+		if len(res.Attributes) == 0 {
+			logging.Logger().Warningf("No groups retrieved from LDAP for user %s", username)
+			break
+		}
 		// append all values of the document. Normally there should be only one per document.
 		groups = append(groups, res.Attributes[0].Values...)
 	}
@@ -240,6 +243,10 @@ func (p *LDAPUserProvider) GetDetails(username string) (*UserDetails, error) {
 	emails := make([]string, 0)
 
 	for _, res := range sr.Entries {
+		if len(res.Attributes) == 0 {
+			logging.Logger().Warningf("No email retrieved from LDAP for user %s", username)
+			break
+		}
 		// append all values of the document. Normally there should be only one per document.
 		emails = append(emails, res.Attributes[0].Values...)
 	}

internal/authentication/ldap_user_provider_test.go

@@ -134,3 +134,111 @@ func TestShouldEscapeUserInput(t *testing.T) {
 
 	ldapClient.getUserAttribute(mockConn, "john=abc", "dn")
 }
+
+func createSearchResultWithAttributes(attributes ...*ldap.EntryAttribute) *ldap.SearchResult {
+	return &ldap.SearchResult{
+		Entries: []*ldap.Entry{
+			&ldap.Entry{Attributes: attributes},
+		},
+	}
+}
+
+func createSearchResultWithAttributeValues(values ...string) *ldap.SearchResult {
+	return createSearchResultWithAttributes(&ldap.EntryAttribute{
+		Values: values,
+	})
+}
+
+func TestShouldNotCrashWhenGroupsAreNotRetrievedFromLDAP(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockFactory := NewMockLDAPConnectionFactory(ctrl)
+	mockConn := NewMockLDAPConnection(ctrl)
+
+	ldapClient := NewLDAPUserProviderWithFactory(schema.LDAPAuthenticationBackendConfiguration{
+		URL:               "ldap://127.0.0.1:389",
+		User:              "cn=admin,dc=example,dc=com",
+		Password:          "password",
+		UsersFilter:       "uid={0}",
+		AdditionalUsersDN: "ou=users",
+		BaseDN:            "dc=example,dc=com",
+	}, mockFactory)
+
+	mockFactory.EXPECT().
+		Dial(gomock.Eq("tcp"), gomock.Eq("127.0.0.1:389")).
+		Return(mockConn, nil).Times(2)
+
+	mockConn.EXPECT().
+		Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
+		Return(nil).
+		Times(2)
+
+	mockConn.EXPECT().
+		Close().Times(2)
+
+	searchGroups := mockConn.EXPECT().
+		Search(gomock.Any()).
+		Return(createSearchResultWithAttributes(), nil)
+	searchUserDN := mockConn.EXPECT().
+		Search(gomock.Any()).
+		Return(createSearchResultWithAttributeValues("uid=john,dc=example,dc=com"), nil)
+	searchEmails := mockConn.EXPECT().
+		Search(gomock.Any()).
+		Return(createSearchResultWithAttributeValues("test@example.com"), nil)
+
+	gomock.InOrder(searchGroups, searchUserDN, searchEmails)
+
+	details, err := ldapClient.GetDetails("john")
+	require.NoError(t, err)
+
+	assert.ElementsMatch(t, details.Groups, []string{})
+	assert.ElementsMatch(t, details.Emails, []string{"test@example.com"})
+}
+
+func TestShouldNotCrashWhenEmailsAreNotRetrievedFromLDAP(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockFactory := NewMockLDAPConnectionFactory(ctrl)
+	mockConn := NewMockLDAPConnection(ctrl)
+
+	ldapClient := NewLDAPUserProviderWithFactory(schema.LDAPAuthenticationBackendConfiguration{
+		URL:               "ldap://127.0.0.1:389",
+		User:              "cn=admin,dc=example,dc=com",
+		Password:          "password",
+		UsersFilter:       "uid={0}",
+		AdditionalUsersDN: "ou=users",
+		BaseDN:            "dc=example,dc=com",
+	}, mockFactory)
+
+	mockFactory.EXPECT().
+		Dial(gomock.Eq("tcp"), gomock.Eq("127.0.0.1:389")).
+		Return(mockConn, nil).Times(2)
+
+	mockConn.EXPECT().
+		Bind(gomock.Eq("cn=admin,dc=example,dc=com"), gomock.Eq("password")).
+		Return(nil).
+		Times(2)
+
+	mockConn.EXPECT().
+		Close().Times(2)
+
+	searchGroups := mockConn.EXPECT().
+		Search(gomock.Any()).
+		Return(createSearchResultWithAttributeValues("group1", "group2"), nil)
+	searchUserDN := mockConn.EXPECT().
+		Search(gomock.Any()).
+		Return(createSearchResultWithAttributeValues("uid=john,dc=example,dc=com"), nil)
+	searchEmails := mockConn.EXPECT().
+		Search(gomock.Any()).
+		Return(createSearchResultWithAttributes(), nil)
+
+	gomock.InOrder(searchGroups, searchUserDN, searchEmails)
+
+	details, err := ldapClient.GetDetails("john")
+	require.NoError(t, err)
+
+	assert.ElementsMatch(t, details.Groups, []string{"group1", "group2"})
+	assert.ElementsMatch(t, details.Emails, []string{})
+}