LDAP: Search all DNs for users (#38891)

pkg/services/ldap/ldap.go

@@ -12,9 +12,10 @@ import (
 	"strings"
 
 	"github.com/davecgh/go-spew/spew"
+	"gopkg.in/ldap.v3"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
-	"gopkg.in/ldap.v3"
 )
 
 // IConnection is interface for LDAP connection manipulation
@@ -252,16 +253,11 @@ func (server *Server) Users(logins []string) (
 	[]*models.ExternalUserInfo,
 	error,
 ) {
-	var users []*ldap.Entry
+	var users [][]*ldap.Entry
 	err := getUsersIteration(logins, func(previous, current int) error {
-		entries, err := server.users(logins[previous:current])
+		var err error
-		if err != nil {
+		users, err = server.users(logins[previous:current])
 		return err
-		}
-
-		users = append(users, entries...)
-
-		return nil
 	})
 	if err != nil {
 		return nil, err
@@ -308,13 +304,15 @@ func getUsersIteration(logins []string, fn func(int, int) error) error {
 
 // users is helper method for the Users()
 func (server *Server) users(logins []string) (
-	[]*ldap.Entry,
+	[][]*ldap.Entry,
 	error,
 ) {
 	var result *ldap.SearchResult
 	var Config = server.Config
 	var err error
 
+	var entries = make([][]*ldap.Entry, 0, len(Config.SearchBaseDNs))
+
 	for _, base := range Config.SearchBaseDNs {
 		result, err = server.Connection.Search(
 			server.getSearchRequest(base, logins),
@@ -324,11 +322,11 @@ func (server *Server) users(logins []string) (
 		}
 
 		if len(result.Entries) > 0 {
-			break
+			entries = append(entries, result.Entries)
 		}
 	}
 
-	return result.Entries, nil
+	return entries, nil
 }
 
 // validateGrafanaUser validates user access.
@@ -557,18 +555,27 @@ func (server *Server) requestMemberOf(entry *ldap.Entry) ([]string, error) {
 // serializeUsers serializes the users
 // from LDAP result to ExternalInfo struct
 func (server *Server) serializeUsers(
-	entries []*ldap.Entry,
+	entries [][]*ldap.Entry,
 ) ([]*models.ExternalUserInfo, error) {
 	var serialized []*models.ExternalUserInfo
+	var users = map[string]struct{}{}
 
-	for _, user := range entries {
+	for _, dn := range entries {
+		for _, user := range dn {
 			extUser, err := server.buildGrafanaUser(user)
 			if err != nil {
 				return nil, err
 			}
 
+			if _, exists := users[extUser.Login]; exists {
+				// ignore duplicates
+				continue
+			}
+			users[extUser.Login] = struct{}{}
+
 			serialized = append(serialized, extUser)
 		}
+	}
 
 	return serialized, nil
 }

pkg/services/ldap/ldap_helpers_test.go

@@ -1,137 +1,89 @@
 package ldap
 
 import (
+	"fmt"
 	"testing"
 
-	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/assert"
 	"gopkg.in/ldap.v3"
 )
 
-func TestLDAPHelpers(t *testing.T) {
+func TestIsMemberOf(t *testing.T) {
-	Convey("isMemberOf()", t, func() {
+	tests := []struct {
-		Convey("Wildcard", func() {
+		memberOf []string
-			result := isMemberOf([]string{}, "*")
+		group    string
-			So(result, ShouldBeTrue)
+		expected bool
-		})
+	}{
-
+		{memberOf: []string{}, group: "*", expected: true},
-		Convey("Should find one", func() {
+		{memberOf: []string{"one", "Two", "three"}, group: "two", expected: true},
-			result := isMemberOf([]string{"one", "Two", "three"}, "two")
+		{memberOf: []string{"one", "Two", "three"}, group: "twos", expected: false},
-			So(result, ShouldBeTrue)
-		})
-
-		Convey("Should not find one", func() {
-			result := isMemberOf([]string{"one", "Two", "three"}, "twos")
-			So(result, ShouldBeFalse)
-		})
-	})
-
-	Convey("getUsersIteration()", t, func() {
-		Convey("it should execute twice for 600 users", func() {
-			logins := make([]string, 600)
-			i := 0
-
-			result := getUsersIteration(logins, func(previous, current int) error {
-				i++
-
-				if i == 1 {
-					So(previous, ShouldEqual, 0)
-					So(current, ShouldEqual, 500)
-				} else {
-					So(previous, ShouldEqual, 500)
-					So(current, ShouldEqual, 600)
 	}
 
-				return nil
+	for _, tc := range tests {
+		t.Run(fmt.Sprintf("isMemberOf(%v, \"%s\") = %v", tc.memberOf, tc.group, tc.expected), func(t *testing.T) {
+			assert.Equal(t, tc.expected, isMemberOf(tc.memberOf, tc.group))
 		})
+	}
+}
 
-			So(i, ShouldEqual, 2)
+func TestGetUsersIteration(t *testing.T) {
-			So(result, ShouldBeNil)
+	const pageSize = UsersMaxRequest
-		})
+	iterations := map[int]int{
-
+		0:    0,
-		Convey("it should execute three times for 1500 users", func() {
+		400:  1,
-			logins := make([]string, 1500)
+		600:  2,
-			i := 0
+		1500: 3,
-
-			result := getUsersIteration(logins, func(previous, current int) error {
-				i++
-				switch i {
-				case 1:
-					So(previous, ShouldEqual, 0)
-					So(current, ShouldEqual, 500)
-				case 2:
-					So(previous, ShouldEqual, 500)
-					So(current, ShouldEqual, 1000)
-				default:
-					So(previous, ShouldEqual, 1000)
-					So(current, ShouldEqual, 1500)
 	}
 
-				return nil
+	for userCount, expectedIterations := range iterations {
-			})
+		t.Run(fmt.Sprintf("getUserIteration iterates %d times for %d users", expectedIterations, userCount), func(t *testing.T) {
+			logins := make([]string, userCount)
 
-			So(i, ShouldEqual, 3)
-			So(result, ShouldBeNil)
-		})
-
-		Convey("it should execute once for 400 users", func() {
-			logins := make([]string, 400)
 			i := 0
+			_ = getUsersIteration(logins, func(first int, last int) error {
+				assert.Equal(t, pageSize*i, first)
 
-			result := getUsersIteration(logins, func(previous, current int) error {
+				expectedLast := pageSize*i + pageSize
-				i++
+				if expectedLast > userCount {
-				if i == 1 {
+					expectedLast = userCount
-					So(previous, ShouldEqual, 0)
-					So(current, ShouldEqual, 400)
 				}
 
-				return nil
+				assert.Equal(t, expectedLast, last)
-			})
 
-			So(i, ShouldEqual, 1)
-			So(result, ShouldBeNil)
-		})
-
-		Convey("it should not execute for 0 users", func() {
-			logins := make([]string, 0)
-			i := 0
-
-			result := getUsersIteration(logins, func(previous, current int) error {
 				i++
 				return nil
 			})
 
-			So(i, ShouldEqual, 0)
+			assert.Equal(t, expectedIterations, i)
-			So(result, ShouldBeNil)
-		})
 		})
+	}
+}
 
-	Convey("getAttribute()", t, func() {
+func TestGetAttribute(t *testing.T) {
-		Convey("Should get DN", func() {
+	t.Run("DN", func(t *testing.T) {
 		entry := &ldap.Entry{
 			DN: "test",
 		}
 
 		result := getAttribute("dn", entry)
-
+		assert.Equal(t, "test", result)
-			So(result, ShouldEqual, "test")
 	})
 
-		Convey("Should get username", func() {
+	t.Run("username", func(t *testing.T) {
-			value := []string{"roelgerrits"}
+		value := "roelgerrits"
 		entry := &ldap.Entry{
 			Attributes: []*ldap.EntryAttribute{
 				{
-						Name: "username", Values: value,
+					Name: "username", Values: []string{value},
 				},
 			},
 		}
 
 		result := getAttribute("username", entry)
-
+		assert.Equal(t, value, result)
-			So(result, ShouldEqual, value[0])
 	})
 
-		Convey("Should not get anything", func() {
+	t.Run("no result", func(t *testing.T) {
 		value := []string{"roelgerrits"}
 		entry := &ldap.Entry{
 			Attributes: []*ldap.EntryAttribute{
@@ -142,23 +94,22 @@ func TestLDAPHelpers(t *testing.T) {
 		}
 
 		result := getAttribute("username", entry)
-
+		assert.Empty(t, result)
-			So(result, ShouldEqual, "")
-		})
 	})
+}
 
-	Convey("getArrayAttribute()", t, func() {
+func TestGetArrayAttribute(t *testing.T) {
-		Convey("Should get DN", func() {
+	t.Run("DN", func(t *testing.T) {
 		entry := &ldap.Entry{
 			DN: "test",
 		}
 
 		result := getArrayAttribute("dn", entry)
 
-			So(result, ShouldResemble, []string{"test"})
+		assert.EqualValues(t, []string{"test"}, result)
 	})
 
-		Convey("Should get username", func() {
+	t.Run("username", func(t *testing.T) {
 		value := []string{"roelgerrits"}
 		entry := &ldap.Entry{
 			Attributes: []*ldap.EntryAttribute{
@@ -170,10 +121,10 @@ func TestLDAPHelpers(t *testing.T) {
 
 		result := getArrayAttribute("username", entry)
 
-			So(result, ShouldResemble, value)
+		assert.EqualValues(t, value, result)
 	})
 
-		Convey("Should not get anything", func() {
+	t.Run("no result", func(t *testing.T) {
 		value := []string{"roelgerrits"}
 		entry := &ldap.Entry{
 			Attributes: []*ldap.EntryAttribute{
@@ -185,7 +136,6 @@ func TestLDAPHelpers(t *testing.T) {
 
 		result := getArrayAttribute("something", entry)
 
-			So(result, ShouldResemble, []string{})
+		assert.Empty(t, result)
-		})
 	})
 }

pkg/services/ldap/ldap_login_test.go

@@ -4,22 +4,22 @@ import (
 	"errors"
 	"testing"
 
-	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"gopkg.in/ldap.v3"
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 )
 
-func TestLDAPLogin(t *testing.T) {
+var defaultLogin = &models.LoginUserQuery{
-	defaultLogin := &models.LoginUserQuery{
 	Username:  "user",
 	Password:  "pwd",
 	IpAddress: "192.168.1.1:56433",
-	}
+}
 
-	Convey("Login()", t, func() {
+func TestServer_Login_UserBind_Fail(t *testing.T) {
-		Convey("Should get invalid credentials when userBind fails", func() {
 	connection := &MockConnection{}
 	entry := ldap.Entry{}
 	result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
@@ -40,10 +40,10 @@ func TestLDAPLogin(t *testing.T) {
 
 	_, err := server.Login(defaultLogin)
 
-			So(err, ShouldEqual, ErrInvalidCredentials)
+	assert.ErrorIs(t, err, ErrInvalidCredentials)
-		})
+}
 
-		Convey("Returns an error when search didn't find anything", func() {
+func TestServer_Login_Search_NoResult(t *testing.T) {
 	connection := &MockConnection{}
 	result := ldap.SearchResult{Entries: []*ldap.Entry{}}
 	connection.setSearchResult(&result)
@@ -60,11 +60,10 @@ func TestLDAPLogin(t *testing.T) {
 	}
 
 	_, err := server.Login(defaultLogin)
+	assert.ErrorIs(t, err, ErrCouldNotFindUser)
+}
 
-			So(err, ShouldEqual, ErrCouldNotFindUser)
+func TestServer_Login_Search_Error(t *testing.T) {
-		})
-
-		Convey("When search returns an error", func() {
 	connection := &MockConnection{}
 	expected := errors.New("Killa-gorilla")
 	connection.setSearchError(expected)
@@ -81,11 +80,10 @@ func TestLDAPLogin(t *testing.T) {
 	}
 
 	_, err := server.Login(defaultLogin)
+	assert.ErrorIs(t, err, expected)
+}
 
-			So(err, ShouldEqual, expected)
+func TestServer_Login_ValidCredentials(t *testing.T) {
-		})
-
-		Convey("When login with valid credentials", func() {
 	connection := &MockConnection{}
 	entry := ldap.Entry{
 		DN: "dn", Attributes: []*ldap.EntryAttribute{
@@ -116,12 +114,14 @@ func TestLDAPLogin(t *testing.T) {
 	}
 
 	resp, err := server.Login(defaultLogin)
+	require.NoError(t, err)
+	assert.Equal(t, "markelog", resp.Login)
+}
 
-			So(err, ShouldBeNil)
+// TestServer_Login_UnauthenticatedBind tests that unauthenticated bind
-			So(resp.Login, ShouldEqual, "markelog")
+// is called when there is no admin password or user wildcard in the
-		})
+// bind_dn.
-
+func TestServer_Login_UnauthenticatedBind(t *testing.T) {
-		Convey("Should perform unauthenticated bind without admin", func() {
 	connection := &MockConnection{}
 	entry := ldap.Entry{
 		DN: "test",
@@ -141,13 +141,12 @@ func TestLDAPLogin(t *testing.T) {
 	}
 
 	user, err := server.Login(defaultLogin)
+	require.NoError(t, err)
+	assert.Equal(t, "test", user.AuthId)
+	assert.True(t, connection.UnauthenticatedBindCalled)
+}
 
-			So(err, ShouldBeNil)
+func TestServer_Login_AuthenticatedBind(t *testing.T) {
-			So(user.AuthId, ShouldEqual, "test")
-			So(connection.UnauthenticatedBindCalled, ShouldBeTrue)
-		})
-
-		Convey("Should perform authenticated binds", func() {
 	connection := &MockConnection{}
 	entry := ldap.Entry{
 		DN: "test",
@@ -186,19 +185,19 @@ func TestLDAPLogin(t *testing.T) {
 	}
 
 	user, err := server.Login(defaultLogin)
+	require.NoError(t, err)
 
-			So(err, ShouldBeNil)
+	assert.Equal(t, "test", user.AuthId)
+	assert.True(t, connection.BindCalled)
 
-			So(user.AuthId, ShouldEqual, "test")
+	assert.Equal(t, "killa", adminUsername)
-			So(connection.BindCalled, ShouldBeTrue)
+	assert.Equal(t, "gorilla", adminPassword)
 
-			So(adminUsername, ShouldEqual, "killa")
+	assert.Equal(t, "test", username)
-			So(adminPassword, ShouldEqual, "gorilla")
+	assert.Equal(t, "pwd", password)
+}
 
-			So(username, ShouldEqual, "test")
+func TestServer_Login_UserWildcardBind(t *testing.T) {
-			So(password, ShouldEqual, "pwd")
-		})
-		Convey("Should bind with user if %s exists in the bind_dn", func() {
 	connection := &MockConnection{}
 	entry := ldap.Entry{
 		DN: "test",
@@ -223,12 +222,9 @@ func TestLDAPLogin(t *testing.T) {
 	}
 
 	_, err := server.Login(defaultLogin)
+	require.NoError(t, err)
 
-			So(err, ShouldBeNil)
+	assert.Equal(t, "cn=user,ou=users,dc=grafana,dc=org", authBindUser)
-
+	assert.Equal(t, "pwd", authBindPassword)
-			So(authBindUser, ShouldEqual, "cn=user,ou=users,dc=grafana,dc=org")
+	assert.True(t, connection.BindCalled)
-			So(authBindPassword, ShouldEqual, "pwd")
-			So(connection.BindCalled, ShouldBeTrue)
-		})
-	})
 }

pkg/services/ldap/ldap_private_test.go

@@ -3,15 +3,35 @@ package ldap
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
+	"github.com/stretchr/testify/assert"
+
+	"gopkg.in/ldap.v3"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
-	. "github.com/smartystreets/goconvey/convey"
-	"gopkg.in/ldap.v3"
 )
 
-func TestLDAPPrivateMethods(t *testing.T) {
+func TestServer_getSearchRequest(t *testing.T) {
-	Convey("getSearchRequest()", t, func() {
+	expected := &ldap.SearchRequest{
-		Convey("with enabled GroupSearchFilterUserAttribute setting", func() {
+		BaseDN:       "killa",
+		Scope:        2,
+		DerefAliases: 0,
+		SizeLimit:    0,
+		TimeLimit:    0,
+		TypesOnly:    false,
+		Filter:       "(|)",
+		Attributes: []string{
+			"username",
+			"email",
+			"name",
+			"memberof",
+			"gansta",
+		},
+		Controls: nil,
+	}
+
 	server := &Server{
 		Config: &ServerConfig{
 			Attr: AttributeMap{
@@ -28,28 +48,11 @@ func TestLDAPPrivateMethods(t *testing.T) {
 
 	result := server.getSearchRequest("killa", []string{"gorilla"})
 
-			So(result, ShouldResemble, &ldap.SearchRequest{
+	assert.EqualValues(t, expected, result)
-				BaseDN:       "killa",
+}
-				Scope:        2,
-				DerefAliases: 0,
-				SizeLimit:    0,
-				TimeLimit:    0,
-				TypesOnly:    false,
-				Filter:       "(|)",
-				Attributes: []string{
-					"username",
-					"email",
-					"name",
-					"memberof",
-					"gansta",
-				},
-				Controls: nil,
-			})
-		})
-	})
 
-	Convey("serializeUsers()", t, func() {
+func TestSerializeUsers(t *testing.T) {
-		Convey("simple case", func() {
+	t.Run("simple case", func(t *testing.T) {
 		server := &Server{
 			Config: &ServerConfig{
 				Attr: AttributeMap{
@@ -74,17 +77,17 @@ func TestLDAPPrivateMethods(t *testing.T) {
 				{Name: "memberof", Values: []string{"admins"}},
 			},
 		}
-			users := []*ldap.Entry{&entry}
+		users := [][]*ldap.Entry{{&entry}}
 
 		result, err := server.serializeUsers(users)
+		require.NoError(t, err)
 
-			So(err, ShouldBeNil)
+		assert.Equal(t, "roelgerrits", result[0].Login)
-			So(result[0].Login, ShouldEqual, "roelgerrits")
+		assert.Equal(t, "roel@test.com", result[0].Email)
-			So(result[0].Email, ShouldEqual, "roel@test.com")
+		assert.Contains(t, result[0].Groups, "admins")
-			So(result[0].Groups, ShouldContain, "admins")
 	})
 
-		Convey("without lastname", func() {
+	t.Run("without lastname", func(t *testing.T) {
 		server := &Server{
 			Config: &ServerConfig{
 				Attr: AttributeMap{
@@ -108,16 +111,16 @@ func TestLDAPPrivateMethods(t *testing.T) {
 				{Name: "memberof", Values: []string{"admins"}},
 			},
 		}
-			users := []*ldap.Entry{&entry}
+		users := [][]*ldap.Entry{{&entry}}
 
 		result, err := server.serializeUsers(users)
+		require.NoError(t, err)
 
-			So(err, ShouldBeNil)
+		assert.False(t, result[0].IsDisabled)
-			So(result[0].IsDisabled, ShouldBeFalse)
+		assert.Equal(t, "Roel", result[0].Name)
-			So(result[0].Name, ShouldEqual, "Roel")
 	})
 
-		Convey("a user without matching groups should be marked as disabled", func() {
+	t.Run("mark user without matching group as disabled", func(t *testing.T) {
 		server := &Server{
 			Config: &ServerConfig{
 				Groups: []*GroupToOrgRole{{
@@ -136,39 +139,18 @@ func TestLDAPPrivateMethods(t *testing.T) {
 				{Name: "memberof", Values: []string{"admins"}},
 			},
 		}
-			users := []*ldap.Entry{&entry}
+		users := [][]*ldap.Entry{{&entry}}
 
 		result, err := server.serializeUsers(users)
+		require.NoError(t, err)
 
-			So(err, ShouldBeNil)
+		assert.Len(t, result, 1)
-			So(len(result), ShouldEqual, 1)
+		assert.True(t, result[0].IsDisabled)
-			So(result[0].IsDisabled, ShouldBeTrue)
-		})
 	})
+}
 
-	Convey("validateGrafanaUser()", t, func() {
+func TestServer_validateGrafanaUser(t *testing.T) {
-		Convey("Returns error when user does not belong in any of the specified LDAP groups", func() {
+	t.Run("no group config", func(t *testing.T) {
-			server := &Server{
-				Config: &ServerConfig{
-					Groups: []*GroupToOrgRole{
-						{
-							OrgId: 1,
-						},
-					},
-				},
-				log: logger.New("test"),
-			}
-
-			user := &models.ExternalUserInfo{
-				Login: "markelog",
-			}
-
-			result := server.validateGrafanaUser(user)
-
-			So(result, ShouldEqual, ErrInvalidCredentials)
-		})
-
-		Convey("Does not return error when group config is empty", func() {
 		server := &Server{
 			Config: &ServerConfig{
 				Groups: []*GroupToOrgRole{},
@@ -180,12 +162,11 @@ func TestLDAPPrivateMethods(t *testing.T) {
 			Login: "markelog",
 		}
 
-			result := server.validateGrafanaUser(user)
+		err := server.validateGrafanaUser(user)
-
+		require.NoError(t, err)
-			So(result, ShouldBeNil)
 	})
 
-		Convey("Does not return error when groups are there", func() {
+	t.Run("user in group", func(t *testing.T) {
 		server := &Server{
 			Config: &ServerConfig{
 				Groups: []*GroupToOrgRole{
@@ -204,70 +185,70 @@ func TestLDAPPrivateMethods(t *testing.T) {
 			},
 		}
 
-			result := server.validateGrafanaUser(user)
+		err := server.validateGrafanaUser(user)
-
+		require.NoError(t, err)
-			So(result, ShouldBeNil)
-		})
 	})
 
-	Convey("shouldAdminBind()", t, func() {
+	t.Run("user not in group", func(t *testing.T) {
-		Convey("it should require admin userBind", func() {
 		server := &Server{
 			Config: &ServerConfig{
-					BindPassword: "test",
+				Groups: []*GroupToOrgRole{
+					{
+						OrgId: 1,
 					},
+				},
+			},
+			log: logger.New("test"),
 		}
 
-			result := server.shouldAdminBind()
+		user := &models.ExternalUserInfo{
-			So(result, ShouldBeTrue)
+			Login: "markelog",
-		})
-
-		Convey("it should not require admin userBind", func() {
-			server := &Server{
-				Config: &ServerConfig{
-					BindPassword: "",
-				},
 		}
 
-			result := server.shouldAdminBind()
+		err := server.validateGrafanaUser(user)
-			So(result, ShouldBeFalse)
+		require.ErrorIs(t, err, ErrInvalidCredentials)
-		})
 	})
+}
 
-	Convey("shouldSingleBind()", t, func() {
+func TestServer_binds(t *testing.T) {
-		Convey("it should allow single bind", func() {
+	t.Run("single bind with cn wildcard", func(t *testing.T) {
 		server := &Server{
 			Config: &ServerConfig{
 				BindDN: "cn=%s,dc=grafana,dc=org",
 			},
 		}
 
-			result := server.shouldSingleBind()
+		assert.True(t, server.shouldSingleBind())
-			So(result, ShouldBeTrue)
+		assert.Equal(t, "cn=test,dc=grafana,dc=org", server.singleBindDN("test"))
 	})
 
-		Convey("it should not allow single bind", func() {
+	t.Run("don't single bind", func(t *testing.T) {
 		server := &Server{
 			Config: &ServerConfig{
 				BindDN: "cn=admin,dc=grafana,dc=org",
 			},
 		}
 
-			result := server.shouldSingleBind()
+		assert.False(t, server.shouldSingleBind())
-			So(result, ShouldBeFalse)
-		})
 	})
 
-	Convey("singleBindDN()", t, func() {
+	t.Run("admin user bind", func(t *testing.T) {
-		Convey("it should allow single bind", func() {
 		server := &Server{
 			Config: &ServerConfig{
-					BindDN: "cn=%s,dc=grafana,dc=org",
+				BindPassword: "test",
 			},
 		}
 
-			result := server.singleBindDN("test")
+		assert.True(t, server.shouldAdminBind())
-			So(result, ShouldEqual, "cn=test,dc=grafana,dc=org")
 	})
+
+	t.Run("don't admin user bind", func(t *testing.T) {
+		server := &Server{
+			Config: &ServerConfig{
+				BindPassword: "",
+			},
+		}
+
+		assert.False(t, server.shouldAdminBind())
 	})
 }

pkg/services/ldap/ldap_test.go

@@ -2,27 +2,27 @@ package ldap
 
 import (
 	"errors"
+	"fmt"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/stretchr/testify/assert"
-	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/require"
 	"gopkg.in/ldap.v3"
+
+	"github.com/grafana/grafana/pkg/infra/log"
 )
 
-func TestPublicAPI(t *testing.T) {
+func TestNew(t *testing.T) {
-	Convey("New()", t, func() {
-		Convey("Should return ", func() {
 	result := New(&ServerConfig{
 		Attr:          AttributeMap{},
 		SearchBaseDNs: []string{"BaseDNHere"},
 	})
 
-			So(result, ShouldImplement, (*IServer)(nil))
+	assert.Implements(t, (*IServer)(nil), result)
-		})
+}
-	})
 
-	Convey("Close()", t, func() {
+func TestServer_Close(t *testing.T) {
-		Convey("Should close the connection", func() {
+	t.Run("close the connection", func(t *testing.T) {
 		connection := &MockConnection{}
 
 		server := &Server{
@@ -33,11 +33,11 @@ func TestPublicAPI(t *testing.T) {
 			Connection: connection,
 		}
 
-			So(server.Close, ShouldNotPanic)
+		assert.NotPanics(t, server.Close)
-			So(connection.CloseCalled, ShouldBeTrue)
+		assert.True(t, connection.CloseCalled)
 	})
 
-		Convey("Should panic if no connection is established", func() {
+	t.Run("panic if no connection", func(t *testing.T) {
 		server := &Server{
 			Config: &ServerConfig{
 				Attr:          AttributeMap{},
@@ -46,12 +46,13 @@ func TestPublicAPI(t *testing.T) {
 			Connection: nil,
 		}
 
-			So(server.Close, ShouldPanic)
+		assert.Panics(t, server.Close)
 	})
-	})
+}
-	Convey("Users()", t, func() {
+
-		Convey("Finds one user", func() {
+func TestServer_Users(t *testing.T) {
-			MockConnection := &MockConnection{}
+	t.Run("one user", func(t *testing.T) {
+		conn := &MockConnection{}
 		entry := ldap.Entry{
 			DN: "dn", Attributes: []*ldap.EntryAttribute{
 				{Name: "username", Values: []string{"roelgerrits"}},
@@ -61,7 +62,7 @@ func TestPublicAPI(t *testing.T) {
 				{Name: "memberof", Values: []string{"admins"}},
 			}}
 		result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
-			MockConnection.setSearchResult(&result)
+		conn.setSearchResult(&result)
 
 		// Set up attribute map without surname and email
 		server := &Server{
@@ -73,64 +74,157 @@ func TestPublicAPI(t *testing.T) {
 				},
 				SearchBaseDNs: []string{"BaseDNHere"},
 			},
-				Connection: MockConnection,
+			Connection: conn,
 			log:        log.New("test-logger"),
 		}
 
 		searchResult, err := server.Users([]string{"roelgerrits"})
 
-			So(err, ShouldBeNil)
+		require.NoError(t, err)
-			So(searchResult, ShouldNotBeNil)
+		assert.NotNil(t, searchResult)
 
 		// User should be searched in ldap
-			So(MockConnection.SearchCalled, ShouldBeTrue)
+		assert.True(t, conn.SearchCalled)
-
 		// No empty attributes should be added to the search request
-			So(len(MockConnection.SearchAttributes), ShouldEqual, 3)
+		assert.Len(t, conn.SearchAttributes, 3)
 	})
 
-		Convey("Handles a error", func() {
+	t.Run("error", func(t *testing.T) {
 		expected := errors.New("Killa-gorilla")
-			MockConnection := &MockConnection{}
+		conn := &MockConnection{}
-			MockConnection.setSearchError(expected)
+		conn.setSearchError(expected)
 
 		// Set up attribute map without surname and email
 		server := &Server{
 			Config: &ServerConfig{
 				SearchBaseDNs: []string{"BaseDNHere"},
 			},
-				Connection: MockConnection,
+			Connection: conn,
 			log:        log.New("test-logger"),
 		}
 
 		_, err := server.Users([]string{"roelgerrits"})
 
-			So(err, ShouldEqual, expected)
+		assert.ErrorIs(t, err, expected)
 	})
 
-		Convey("Should return empty slice if none were found", func() {
+	t.Run("no user", func(t *testing.T) {
-			MockConnection := &MockConnection{}
+		conn := &MockConnection{}
 		result := ldap.SearchResult{Entries: []*ldap.Entry{}}
-			MockConnection.setSearchResult(&result)
+		conn.setSearchResult(&result)
 
 		// Set up attribute map without surname and email
 		server := &Server{
 			Config: &ServerConfig{
 				SearchBaseDNs: []string{"BaseDNHere"},
 			},
-				Connection: MockConnection,
+			Connection: conn,
 			log:        log.New("test-logger"),
 		}
 
 		searchResult, err := server.Users([]string{"roelgerrits"})
 
-			So(err, ShouldBeNil)
+		require.NoError(t, err)
-			So(searchResult, ShouldBeEmpty)
+		assert.Empty(t, searchResult)
-		})
 	})
 
-	Convey("UserBind()", t, func() {
+	t.Run("multiple DNs", func(t *testing.T) {
-		Convey("Should use provided DN and password", func() {
+		conn := &MockConnection{}
+		serviceDN := "dc=svc,dc=example,dc=org"
+		serviceEntry := ldap.Entry{
+			DN: "dn", Attributes: []*ldap.EntryAttribute{
+				{Name: "username", Values: []string{"imgrenderer"}},
+				{Name: "name", Values: []string{"Image renderer"}},
+			}}
+		services := ldap.SearchResult{Entries: []*ldap.Entry{&serviceEntry}}
+
+		userDN := "dc=users,dc=example,dc=org"
+		userEntry := ldap.Entry{
+			DN: "dn", Attributes: []*ldap.EntryAttribute{
+				{Name: "username", Values: []string{"grot"}},
+				{Name: "name", Values: []string{"Grot"}},
+			}}
+		users := ldap.SearchResult{Entries: []*ldap.Entry{&userEntry}}
+
+		conn.setSearchFunc(func(request *ldap.SearchRequest) (*ldap.SearchResult, error) {
+			switch request.BaseDN {
+			case userDN:
+				return &users, nil
+			case serviceDN:
+				return &services, nil
+			default:
+				return nil, fmt.Errorf("test case not defined for baseDN: '%s'", request.BaseDN)
+			}
+		})
+
+		server := &Server{
+			Config: &ServerConfig{
+				Attr: AttributeMap{
+					Username: "username",
+					Name:     "name",
+				},
+				SearchBaseDNs: []string{serviceDN, userDN},
+			},
+			Connection: conn,
+			log:        log.New("test-logger"),
+		}
+
+		searchResult, err := server.Users([]string{"imgrenderer", "grot"})
+		require.NoError(t, err)
+
+		assert.Len(t, searchResult, 2)
+	})
+
+	t.Run("same user in multiple DNs", func(t *testing.T) {
+		conn := &MockConnection{}
+		firstDN := "dc=users1,dc=example,dc=org"
+		firstEntry := ldap.Entry{
+			DN: "dn", Attributes: []*ldap.EntryAttribute{
+				{Name: "username", Values: []string{"grot"}},
+				{Name: "name", Values: []string{"Grot the First"}},
+			}}
+		firsts := ldap.SearchResult{Entries: []*ldap.Entry{&firstEntry}}
+
+		secondDN := "dc=users2,dc=example,dc=org"
+		secondEntry := ldap.Entry{
+			DN: "dn", Attributes: []*ldap.EntryAttribute{
+				{Name: "username", Values: []string{"grot"}},
+				{Name: "name", Values: []string{"Grot the Second"}},
+			}}
+		seconds := ldap.SearchResult{Entries: []*ldap.Entry{&secondEntry}}
+
+		conn.setSearchFunc(func(request *ldap.SearchRequest) (*ldap.SearchResult, error) {
+			switch request.BaseDN {
+			case secondDN:
+				return &seconds, nil
+			case firstDN:
+				return &firsts, nil
+			default:
+				return nil, fmt.Errorf("test case not defined for baseDN: '%s'", request.BaseDN)
+			}
+		})
+
+		server := &Server{
+			Config: &ServerConfig{
+				Attr: AttributeMap{
+					Username: "username",
+					Name:     "name",
+				},
+				SearchBaseDNs: []string{firstDN, secondDN},
+			},
+			Connection: conn,
+			log:        log.New("test-logger"),
+		}
+
+		res, err := server.Users([]string{"grot"})
+		require.NoError(t, err)
+		require.Len(t, res, 1)
+		assert.Equal(t, "Grot the First", res[0].Name)
+	})
+}
+
+func TestServer_UserBind(t *testing.T) {
+	t.Run("use provided DN and password", func(t *testing.T) {
 		connection := &MockConnection{}
 		var actualUsername, actualPassword string
 		connection.BindProvider = func(username, password string) error {
@@ -148,12 +242,12 @@ func TestPublicAPI(t *testing.T) {
 		dn := "cn=user,ou=users,dc=grafana,dc=org"
 		err := server.UserBind(dn, "pwd")
 
-			So(err, ShouldBeNil)
+		require.NoError(t, err)
-			So(actualUsername, ShouldEqual, dn)
+		assert.Equal(t, dn, actualUsername)
-			So(actualPassword, ShouldEqual, "pwd")
+		assert.Equal(t, "pwd", actualPassword)
 	})
 
-		Convey("Should handle an error", func() {
+	t.Run("error", func(t *testing.T) {
 		connection := &MockConnection{}
 		expected := &ldap.Error{
 			ResultCode: uint16(25),
@@ -169,12 +263,12 @@ func TestPublicAPI(t *testing.T) {
 			log: log.New("test-logger"),
 		}
 		err := server.UserBind("user", "pwd")
-			So(err, ShouldEqual, expected)
+		assert.ErrorIs(t, err, expected)
-		})
 	})
+}
 
-	Convey("AdminBind()", t, func() {
+func TestServer_AdminBind(t *testing.T) {
-		Convey("Should use admin DN and password", func() {
+	t.Run("use admin DN and password", func(t *testing.T) {
 		connection := &MockConnection{}
 		var actualUsername, actualPassword string
 		connection.BindProvider = func(username, password string) error {
@@ -194,13 +288,13 @@ func TestPublicAPI(t *testing.T) {
 		}
 
 		err := server.AdminBind()
+		require.NoError(t, err)
 
-			So(err, ShouldBeNil)
+		assert.Equal(t, dn, actualUsername)
-			So(actualUsername, ShouldEqual, dn)
+		assert.Equal(t, "pwd", actualPassword)
-			So(actualPassword, ShouldEqual, "pwd")
 	})
 
-		Convey("Should handle an error", func() {
+	t.Run("error", func(t *testing.T) {
 		connection := &MockConnection{}
 		expected := &ldap.Error{
 			ResultCode: uint16(25),
@@ -221,7 +315,6 @@ func TestPublicAPI(t *testing.T) {
 		}
 
 		err := server.AdminBind()
-			So(err, ShouldEqual, expected)
+		assert.ErrorIs(t, err, expected)
-		})
 	})
 }

pkg/services/ldap/testing.go

@@ -6,10 +6,11 @@ import (
 	"gopkg.in/ldap.v3"
 )
 
+type searchFunc = func(request *ldap.SearchRequest) (*ldap.SearchResult, error)
+
 // MockConnection struct for testing
 type MockConnection struct {
-	SearchResult     *ldap.SearchResult
+	SearchFunc       searchFunc
-	SearchError      error
 	SearchCalled     bool
 	SearchAttributes []string
 
@@ -56,11 +57,19 @@ func (c *MockConnection) Close() {
 }
 
 func (c *MockConnection) setSearchResult(result *ldap.SearchResult) {
-	c.SearchResult = result
+	c.SearchFunc = func(request *ldap.SearchRequest) (*ldap.SearchResult, error) {
+		return result, nil
+	}
 }
 
 func (c *MockConnection) setSearchError(err error) {
-	c.SearchError = err
+	c.SearchFunc = func(request *ldap.SearchRequest) (*ldap.SearchResult, error) {
+		return nil, err
+	}
+}
+
+func (c *MockConnection) setSearchFunc(fn searchFunc) {
+	c.SearchFunc = fn
 }
 
 // Search mocks Search connection function
@@ -68,11 +77,7 @@ func (c *MockConnection) Search(sr *ldap.SearchRequest) (*ldap.SearchResult, err
 	c.SearchCalled = true
 	c.SearchAttributes = sr.Attributes
 
-	if c.SearchError != nil {
+	return c.SearchFunc(sr)
-		return nil, c.SearchError
-	}
-
-	return c.SearchResult, nil
 }
 
 // Add mocks Add connection function