Feature: LDAP refactoring (#16950)

* incapsulates multipleldap logic under one module

* abstracts users upsert and get logic

* changes some of the text error messages and import sort sequence

* heavily refactors the LDAP module – LDAP module now only deals with LDAP related behaviour

* integrates affected auth_proxy module and their tests

* refactoring of the auth_proxy logic

pkg/services/ldap/hooks.go

@@ -1,5 +0,0 @@
-package ldap
-
-var (
-	hookDial func(*Auth) error
-)

pkg/services/ldap/ldap.go

@@ -8,39 +8,38 @@ import (
 	"io/ioutil"
 	"strings"
 
-	"github.com/davecgh/go-spew/spew"
-	LDAP "gopkg.in/ldap.v3"
+	"gopkg.in/ldap.v3"
 
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/log"
-	models "github.com/grafana/grafana/pkg/models"
-	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/models"
 )
 
 // IConnection is interface for LDAP connection manipulation
 type IConnection interface {
 	Bind(username, password string) error
 	UnauthenticatedBind(username string) error
-	Search(*LDAP.SearchRequest) (*LDAP.SearchResult, error)
+	Add(*ldap.AddRequest) error
+	Del(*ldap.DelRequest) error
+	Search(*ldap.SearchRequest) (*ldap.SearchResult, error)
 	StartTLS(*tls.Config) error
 	Close()
 }
 
-// IAuth is interface for LDAP authorization
-type IAuth interface {
-	Login(query *models.LoginUserQuery) error
-	SyncUser(query *models.LoginUserQuery) error
-	GetGrafanaUserFor(
-		ctx *models.ReqContext,
-		user *UserInfo,
-	) (*models.User, error)
-	Users() ([]*UserInfo, error)
+// IServer is interface for LDAP authorization
+type IServer interface {
+	Login(*models.LoginUserQuery) (*models.ExternalUserInfo, error)
+	Add(string, map[string][]string) error
+	Remove(string) error
+	Users([]string) ([]*models.ExternalUserInfo, error)
+	ExtractGrafanaUser(*UserInfo) (*models.ExternalUserInfo, error)
+	Dial() error
+	Close()
 }
 
-// Auth is basic struct of LDAP authorization
-type Auth struct {
-	server            *ServerConfig
-	conn              IConnection
+// Server is basic struct of LDAP authorization
+type Server struct {
+	config            *ServerConfig
+	connection        IConnection
 	requireSecondBind bool
 	log               log.Logger
 }
@@ -52,28 +51,24 @@ var (
 )
 
 var dial = func(network, addr string) (IConnection, error) {
-	return LDAP.Dial(network, addr)
+	return ldap.Dial(network, addr)
 }
 
 // New creates the new LDAP auth
-func New(server *ServerConfig) IAuth {
-	return &Auth{
-		server: server,
+func New(config *ServerConfig) IServer {
+	return &Server{
+		config: config,
 		log:    log.New("ldap"),
 	}
 }
 
 // Dial dials in the LDAP
-func (auth *Auth) Dial() error {
-	if hookDial != nil {
-		return hookDial(auth)
-	}
-
+func (server *Server) Dial() error {
 	var err error
 	var certPool *x509.CertPool
-	if auth.server.RootCACert != "" {
+	if server.config.RootCACert != "" {
 		certPool = x509.NewCertPool()
-		for _, caCertFile := range strings.Split(auth.server.RootCACert, " ") {
+		for _, caCertFile := range strings.Split(server.config.RootCACert, " ") {
 			pem, err := ioutil.ReadFile(caCertFile)
 			if err != nil {
 				return err
@@ -84,35 +79,35 @@ func (auth *Auth) Dial() error {
 		}
 	}
 	var clientCert tls.Certificate
-	if auth.server.ClientCert != "" && auth.server.ClientKey != "" {
-		clientCert, err = tls.LoadX509KeyPair(auth.server.ClientCert, auth.server.ClientKey)
+	if server.config.ClientCert != "" && server.config.ClientKey != "" {
+		clientCert, err = tls.LoadX509KeyPair(server.config.ClientCert, server.config.ClientKey)
 		if err != nil {
 			return err
 		}
 	}
-	for _, host := range strings.Split(auth.server.Host, " ") {
-		address := fmt.Sprintf("%s:%d", host, auth.server.Port)
-		if auth.server.UseSSL {
+	for _, host := range strings.Split(server.config.Host, " ") {
+		address := fmt.Sprintf("%s:%d", host, server.config.Port)
+		if server.config.UseSSL {
 			tlsCfg := &tls.Config{
-				InsecureSkipVerify: auth.server.SkipVerifySSL,
+				InsecureSkipVerify: server.config.SkipVerifySSL,
 				ServerName:         host,
 				RootCAs:            certPool,
 			}
 			if len(clientCert.Certificate) > 0 {
 				tlsCfg.Certificates = append(tlsCfg.Certificates, clientCert)
 			}
-			if auth.server.StartTLS {
-				auth.conn, err = dial("tcp", address)
+			if server.config.StartTLS {
+				server.connection, err = dial("tcp", address)
 				if err == nil {
-					if err = auth.conn.StartTLS(tlsCfg); err == nil {
+					if err = server.connection.StartTLS(tlsCfg); err == nil {
 						return nil
 					}
 				}
 			} else {
-				auth.conn, err = LDAP.DialTLS("tcp", address, tlsCfg)
+				server.connection, err = ldap.DialTLS("tcp", address, tlsCfg)
 			}
 		} else {
-			auth.conn, err = dial("tcp", address)
+			server.connection, err = dial("tcp", address)
 		}
 
 		if err == nil {
@@ -122,91 +117,206 @@ func (auth *Auth) Dial() error {
 	return err
 }
 
-// Login logs in the user
-func (auth *Auth) Login(query *models.LoginUserQuery) error {
-	// connect to ldap server
-	if err := auth.Dial(); err != nil {
-		return err
-	}
-	defer auth.conn.Close()
+// Close closes the LDAP connection
+func (server *Server) Close() {
+	server.connection.Close()
+}
 
-	// perform initial authentication
-	if err := auth.initialBind(query.Username, query.Password); err != nil {
-		return err
-	}
+// Login intialBinds the user, search it and then serialize it
+func (server *Server) Login(query *models.LoginUserQuery) (
+	*models.ExternalUserInfo, error,
+) {
 
-	// find user entry & attributes
-	user, err := auth.searchForUser(query.Username)
+	// Perform initial authentication
+	err := server.intialBind(query.Username, query.Password)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	auth.log.Debug("Ldap User found", "info", spew.Sdump(user))
+	// Find user entry & attributes
+	users, err := server.Users([]string{query.Username})
+	if err != nil {
+		return nil, err
+	}
 
-	// check if a second user bind is needed
-	if auth.requireSecondBind {
-		err = auth.secondBind(user, query.Password)
+	// If we couldn't find the user -
+	// we should show incorrect credentials err
+	if len(users) == 0 {
+		return nil, ErrInvalidCredentials
+	}
+
+	// Check if a second user bind is needed
+	user := users[0]
+	if server.requireSecondBind {
+		err = server.secondBind(user, query.Password)
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
 
-	grafanaUser, err := auth.GetGrafanaUserFor(query.ReqContext, user)
+	return user, nil
+}
+
+// Add adds stuff to LDAP
+func (server *Server) Add(dn string, values map[string][]string) error {
+	err := server.intialBind(
+		server.config.BindDN,
+		server.config.BindPassword,
+	)
+	if err != nil {
+		return err
+	}
+
+	attributes := make([]ldap.Attribute, 0)
+	for key, value := range values {
+		attributes = append(attributes, ldap.Attribute{
+			Type: key,
+			Vals: value,
+		})
+	}
+
+	request := &ldap.AddRequest{
+		DN:         dn,
+		Attributes: attributes,
+	}
+
+	err = server.connection.Add(request)
 	if err != nil {
 		return err
 	}
 
-	query.User = grafanaUser
 	return nil
 }
 
-// SyncUser syncs user with Grafana
-func (auth *Auth) SyncUser(query *models.LoginUserQuery) error {
-	// connect to ldap server
-	err := auth.Dial()
-	if err != nil {
-		return err
-	}
-	defer auth.conn.Close()
-
-	err = auth.serverBind()
+// Remove removes stuff from LDAP
+func (server *Server) Remove(dn string) error {
+	err := server.intialBind(
+		server.config.BindDN,
+		server.config.BindPassword,
+	)
 	if err != nil {
 		return err
 	}
 
-	// find user entry & attributes
-	user, err := auth.searchForUser(query.Username)
-	if err != nil {
-		auth.log.Error("Failed searching for user in ldap", "error", err)
-		return err
-	}
-
-	auth.log.Debug("Ldap User found", "info", spew.Sdump(user))
-
-	grafanaUser, err := auth.GetGrafanaUserFor(query.ReqContext, user)
+	request := ldap.NewDelRequest(dn, nil)
+	err = server.connection.Del(request)
 	if err != nil {
 		return err
 	}
 
-	query.User = grafanaUser
 	return nil
 }
 
-func (auth *Auth) GetGrafanaUserFor(
-	ctx *models.ReqContext,
-	user *UserInfo,
-) (*models.User, error) {
+// Users gets LDAP users
+func (server *Server) Users(logins []string) (
+	[]*models.ExternalUserInfo,
+	error,
+) {
+	var result *ldap.SearchResult
+	var err error
+	var config = server.config
+
+	for _, base := range config.SearchBaseDNs {
+		result, err = server.connection.Search(
+			server.getSearchRequest(base, logins),
+		)
+		if err != nil {
+			return nil, err
+		}
+
+		if len(result.Entries) > 0 {
+			break
+		}
+	}
+
+	serializedUsers, err := server.serializeUsers(result)
+	if err != nil {
+		return nil, err
+	}
+
+	return serializedUsers, nil
+}
+
+// ExtractGrafanaUser extracts external user info from LDAP user
+func (server *Server) ExtractGrafanaUser(user *UserInfo) (*models.ExternalUserInfo, error) {
+	result := server.buildGrafanaUser(user)
+	if err := server.validateGrafanaUser(result); err != nil {
+		return nil, err
+	}
+
+	return result, nil
+}
+
+// validateGrafanaUser validates user access.
+// If there are no ldap group mappings access is true
+// otherwise a single group must match
+func (server *Server) validateGrafanaUser(user *models.ExternalUserInfo) error {
+	if len(server.config.Groups) > 0 && len(user.OrgRoles) < 1 {
+		server.log.Error(
+			"user does not belong in any of the specified LDAP groups",
+			"username", user.Login,
+			"groups", user.Groups,
+		)
+		return ErrInvalidCredentials
+	}
+
+	return nil
+}
+
+// getSearchRequest returns LDAP search request for users
+func (server *Server) getSearchRequest(
+	base string,
+	logins []string,
+) *ldap.SearchRequest {
+	attributes := []string{}
+
+	inputs := server.config.Attr
+	attributes = appendIfNotEmpty(
+		attributes,
+		inputs.Username,
+		inputs.Surname,
+		inputs.Email,
+		inputs.Name,
+		inputs.MemberOf,
+	)
+
+	search := ""
+	for _, login := range logins {
+		query := strings.Replace(
+			server.config.SearchFilter,
+			"%s", ldap.EscapeFilter(login),
+			-1,
+		)
+
+		search = search + query
+	}
+
+	filter := fmt.Sprintf("(|%s)", search)
+
+	return &ldap.SearchRequest{
+		BaseDN:       base,
+		Scope:        ldap.ScopeWholeSubtree,
+		DerefAliases: ldap.NeverDerefAliases,
+		Attributes:   attributes,
+		Filter:       filter,
+	}
+}
+
+// buildGrafanaUser extracts info from UserInfo model to ExternalUserInfo
+func (server *Server) buildGrafanaUser(user *UserInfo) *models.ExternalUserInfo {
 	extUser := &models.ExternalUserInfo{
 		AuthModule: "ldap",
 		AuthId:     user.DN,
-		Name:       fmt.Sprintf("%s %s", user.FirstName, user.LastName),
-		Login:      user.Username,
-		Email:      user.Email,
-		Groups:     user.MemberOf,
-		OrgRoles:   map[int64]models.RoleType{},
+		Name: strings.TrimSpace(
+			fmt.Sprintf("%s %s", user.FirstName, user.LastName),
+		),
+		Login:    user.Username,
+		Email:    user.Email,
+		Groups:   user.MemberOf,
+		OrgRoles: map[int64]models.RoleType{},
 	}
 
-	for _, group := range auth.server.Groups {
+	for _, group := range server.config.Groups {
 		// only use the first match for each org
 		if extUser.OrgRoles[group.OrgId] != "" {
 			continue
@@ -220,49 +330,28 @@ func (auth *Auth) GetGrafanaUserFor(
 		}
 	}
 
-	// validate that the user has access
-	// if there are no ldap group mappings access is true
-	// otherwise a single group must match
-	if len(auth.server.Groups) > 0 && len(extUser.OrgRoles) < 1 {
-		auth.log.Info(
-			"Ldap Auth: user does not belong in any of the specified ldap groups",
-			"username", user.Username,
-			"groups", user.MemberOf,
-		)
-		return nil, ErrInvalidCredentials
-	}
-
-	// add/update user in grafana
-	upsertUserCmd := &models.UpsertUserCommand{
-		ReqContext:    ctx,
-		ExternalUser:  extUser,
-		SignupAllowed: setting.LdapAllowSignup,
-	}
-
-	err := bus.Dispatch(upsertUserCmd)
-	if err != nil {
-		return nil, err
-	}
-
-	return upsertUserCmd.Result, nil
+	return extUser
 }
 
-func (auth *Auth) serverBind() error {
+func (server *Server) serverBind() error {
 	bindFn := func() error {
-		return auth.conn.Bind(auth.server.BindDN, auth.server.BindPassword)
+		return server.connection.Bind(
+			server.config.BindDN,
+			server.config.BindPassword,
+		)
 	}
 
-	if auth.server.BindPassword == "" {
+	if server.config.BindPassword == "" {
 		bindFn = func() error {
-			return auth.conn.UnauthenticatedBind(auth.server.BindDN)
+			return server.connection.UnauthenticatedBind(server.config.BindDN)
 		}
 	}
 
 	// bind_dn and bind_password to bind
 	if err := bindFn(); err != nil {
-		auth.log.Info("LDAP initial bind failed, %v", err)
+		server.log.Info("LDAP initial bind failed, %v", err)
 
-		if ldapErr, ok := err.(*LDAP.Error); ok {
+		if ldapErr, ok := err.(*ldap.Error); ok {
 			if ldapErr.ResultCode == 49 {
 				return ErrInvalidCredentials
 			}
@@ -273,11 +362,15 @@ func (auth *Auth) serverBind() error {
 	return nil
 }
 
-func (auth *Auth) secondBind(user *UserInfo, userPassword string) error {
-	if err := auth.conn.Bind(user.DN, userPassword); err != nil {
-		auth.log.Info("Second bind failed", "error", err)
+func (server *Server) secondBind(
+	user *models.ExternalUserInfo,
+	userPassword string,
+) error {
+	err := server.connection.Bind(user.AuthId, userPassword)
+	if err != nil {
+		server.log.Info("Second bind failed", "error", err)
 
-		if ldapErr, ok := err.(*LDAP.Error); ok {
+		if ldapErr, ok := err.(*ldap.Error); ok {
 			if ldapErr.ResultCode == 49 {
 				return ErrInvalidCredentials
 			}
@@ -288,31 +381,31 @@ func (auth *Auth) secondBind(user *UserInfo, userPassword string) error {
 	return nil
 }
 
-func (auth *Auth) initialBind(username, userPassword string) error {
-	if auth.server.BindPassword != "" || auth.server.BindDN == "" {
-		userPassword = auth.server.BindPassword
-		auth.requireSecondBind = true
+func (server *Server) intialBind(username, userPassword string) error {
+	if server.config.BindPassword != "" || server.config.BindDN == "" {
+		userPassword = server.config.BindPassword
+		server.requireSecondBind = true
 	}
 
-	bindPath := auth.server.BindDN
+	bindPath := server.config.BindDN
 	if strings.Contains(bindPath, "%s") {
-		bindPath = fmt.Sprintf(auth.server.BindDN, username)
+		bindPath = fmt.Sprintf(server.config.BindDN, username)
 	}
 
 	bindFn := func() error {
-		return auth.conn.Bind(bindPath, userPassword)
+		return server.connection.Bind(bindPath, userPassword)
 	}
 
 	if userPassword == "" {
 		bindFn = func() error {
-			return auth.conn.UnauthenticatedBind(bindPath)
+			return server.connection.UnauthenticatedBind(bindPath)
 		}
 	}
 
 	if err := bindFn(); err != nil {
-		auth.log.Info("Initial bind failed", "error", err)
+		server.log.Info("Initial bind failed", "error", err)
 
-		if ldapErr, ok := err.(*LDAP.Error); ok {
+		if ldapErr, ok := err.(*ldap.Error); ok {
 			if ldapErr.ResultCode == 49 {
 				return ErrInvalidCredentials
 			}
@@ -323,199 +416,124 @@ func (auth *Auth) initialBind(username, userPassword string) error {
 	return nil
 }
 
-func (auth *Auth) searchForUser(username string) (*UserInfo, error) {
-	var searchResult *LDAP.SearchResult
-	var err error
-
-	for _, searchBase := range auth.server.SearchBaseDNs {
-		attributes := make([]string, 0)
-		inputs := auth.server.Attr
-		attributes = appendIfNotEmpty(attributes,
-			inputs.Username,
-			inputs.Surname,
-			inputs.Email,
-			inputs.Name,
-			inputs.MemberOf)
-
-		searchReq := LDAP.SearchRequest{
-			BaseDN:       searchBase,
-			Scope:        LDAP.ScopeWholeSubtree,
-			DerefAliases: LDAP.NeverDerefAliases,
-			Attributes:   attributes,
-			Filter: strings.Replace(
-				auth.server.SearchFilter,
-				"%s", LDAP.EscapeFilter(username),
-				-1,
-			),
-		}
-
-		auth.log.Debug("Ldap Search For User Request", "info", spew.Sdump(searchReq))
-
-		searchResult, err = auth.conn.Search(&searchReq)
-		if err != nil {
-			return nil, err
-		}
-
-		if len(searchResult.Entries) > 0 {
-			break
-		}
-	}
-
-	if len(searchResult.Entries) == 0 {
-		return nil, ErrInvalidCredentials
-	}
-
-	if len(searchResult.Entries) > 1 {
-		return nil, errors.New("Ldap search matched more than one entry, please review your filter setting")
-	}
-
+// requestMemberOf use this function when POSIX LDAP schema does not support memberOf, so it manually search the groups
+func (server *Server) requestMemberOf(searchResult *ldap.SearchResult) ([]string, error) {
 	var memberOf []string
-	if auth.server.GroupSearchFilter == "" {
-		memberOf = getLdapAttrArray(auth.server.Attr.MemberOf, searchResult)
-	} else {
-		// If we are using a POSIX LDAP schema it won't support memberOf, so we manually search the groups
-		var groupSearchResult *LDAP.SearchResult
-		for _, groupSearchBase := range auth.server.GroupSearchBaseDNs {
-			var filter_replace string
-			if auth.server.GroupSearchFilterUserAttribute == "" {
-				filter_replace = getLdapAttr(auth.server.Attr.Username, searchResult)
-			} else {
-				filter_replace = getLdapAttr(auth.server.GroupSearchFilterUserAttribute, searchResult)
-			}
 
-			filter := strings.Replace(
-				auth.server.GroupSearchFilter, "%s",
-				LDAP.EscapeFilter(filter_replace),
-				-1,
-			)
-
-			auth.log.Info("Searching for user's groups", "filter", filter)
-
-			// support old way of reading settings
-			groupIdAttribute := auth.server.Attr.MemberOf
-			// but prefer dn attribute if default settings are used
-			if groupIdAttribute == "" || groupIdAttribute == "memberOf" {
-				groupIdAttribute = "dn"
-			}
-
-			groupSearchReq := LDAP.SearchRequest{
-				BaseDN:       groupSearchBase,
-				Scope:        LDAP.ScopeWholeSubtree,
-				DerefAliases: LDAP.NeverDerefAliases,
-				Attributes:   []string{groupIdAttribute},
-				Filter:       filter,
-			}
-
-			groupSearchResult, err = auth.conn.Search(&groupSearchReq)
-			if err != nil {
-				return nil, err
-			}
-
-			if len(groupSearchResult.Entries) > 0 {
-				for i := range groupSearchResult.Entries {
-					memberOf = append(memberOf, getLdapAttrN(groupIdAttribute, groupSearchResult, i))
-				}
-				break
-			}
+	for _, groupSearchBase := range server.config.GroupSearchBaseDNs {
+		var filterReplace string
+		if server.config.GroupSearchFilterUserAttribute == "" {
+			filterReplace = getLdapAttr(server.config.Attr.Username, searchResult)
+		} else {
+			filterReplace = getLdapAttr(server.config.GroupSearchFilterUserAttribute, searchResult)
 		}
-	}
 
-	return &UserInfo{
-		DN:        searchResult.Entries[0].DN,
-		LastName:  getLdapAttr(auth.server.Attr.Surname, searchResult),
-		FirstName: getLdapAttr(auth.server.Attr.Name, searchResult),
-		Username:  getLdapAttr(auth.server.Attr.Username, searchResult),
-		Email:     getLdapAttr(auth.server.Attr.Email, searchResult),
-		MemberOf:  memberOf,
-	}, nil
-}
-
-func (ldap *Auth) Users() ([]*UserInfo, error) {
-	var result *LDAP.SearchResult
-	var err error
-	server := ldap.server
-
-	if err := ldap.Dial(); err != nil {
-		return nil, err
-	}
-	defer ldap.conn.Close()
-
-	for _, base := range server.SearchBaseDNs {
-		attributes := make([]string, 0)
-		inputs := server.Attr
-		attributes = appendIfNotEmpty(
-			attributes,
-			inputs.Username,
-			inputs.Surname,
-			inputs.Email,
-			inputs.Name,
-			inputs.MemberOf,
+		filter := strings.Replace(
+			server.config.GroupSearchFilter, "%s",
+			ldap.EscapeFilter(filterReplace),
+			-1,
 		)
 
-		req := LDAP.SearchRequest{
-			BaseDN:       base,
-			Scope:        LDAP.ScopeWholeSubtree,
-			DerefAliases: LDAP.NeverDerefAliases,
-			Attributes:   attributes,
+		server.log.Info("Searching for user's groups", "filter", filter)
 
-			// Doing a star here to get all the users in one go
-			Filter: strings.Replace(server.SearchFilter, "%s", "*", -1),
+		// support old way of reading settings
+		groupIDAttribute := server.config.Attr.MemberOf
+		// but prefer dn attribute if default settings are used
+		if groupIDAttribute == "" || groupIDAttribute == "memberOf" {
+			groupIDAttribute = "dn"
 		}
 
-		result, err = ldap.conn.Search(&req)
+		groupSearchReq := ldap.SearchRequest{
+			BaseDN:       groupSearchBase,
+			Scope:        ldap.ScopeWholeSubtree,
+			DerefAliases: ldap.NeverDerefAliases,
+			Attributes:   []string{groupIDAttribute},
+			Filter:       filter,
+		}
+
+		groupSearchResult, err := server.connection.Search(&groupSearchReq)
 		if err != nil {
 			return nil, err
 		}
 
-		if len(result.Entries) > 0 {
+		if len(groupSearchResult.Entries) > 0 {
+			for i := range groupSearchResult.Entries {
+				memberOf = append(memberOf, getLdapAttrN(groupIDAttribute, groupSearchResult, i))
+			}
 			break
 		}
 	}
 
-	return ldap.serializeUsers(result), nil
+	return memberOf, nil
 }
 
-func (ldap *Auth) serializeUsers(users *LDAP.SearchResult) []*UserInfo {
-	var serialized []*UserInfo
+// serializeUsers serializes the users
+// from LDAP result to ExternalInfo struct
+func (server *Server) serializeUsers(
+	users *ldap.SearchResult,
+) ([]*models.ExternalUserInfo, error) {
+	var serialized []*models.ExternalUserInfo
 
 	for index := range users.Entries {
-		serialize := &UserInfo{
+		memberOf, err := server.getMemberOf(users)
+		if err != nil {
+			return nil, err
+		}
+
+		userInfo := &UserInfo{
 			DN: getLdapAttrN(
 				"dn",
 				users,
 				index,
 			),
 			LastName: getLdapAttrN(
-				ldap.server.Attr.Surname,
+				server.config.Attr.Surname,
 				users,
 				index,
 			),
 			FirstName: getLdapAttrN(
-				ldap.server.Attr.Name,
+				server.config.Attr.Name,
 				users,
 				index,
 			),
 			Username: getLdapAttrN(
-				ldap.server.Attr.Username,
+				server.config.Attr.Username,
 				users,
 				index,
 			),
 			Email: getLdapAttrN(
-				ldap.server.Attr.Email,
-				users,
-				index,
-			),
-			MemberOf: getLdapAttrArrayN(
-				ldap.server.Attr.MemberOf,
+				server.config.Attr.Email,
 				users,
 				index,
 			),
+			MemberOf: memberOf,
 		}
 
-		serialized = append(serialized, serialize)
+		serialized = append(
+			serialized,
+			server.buildGrafanaUser(userInfo),
+		)
 	}
 
-	return serialized
+	return serialized, nil
+}
+
+// getMemberOf finds memberOf property or request it
+func (server *Server) getMemberOf(search *ldap.SearchResult) (
+	[]string, error,
+) {
+	if server.config.GroupSearchFilter == "" {
+		memberOf := getLdapAttrArray(server.config.Attr.MemberOf, search)
+
+		return memberOf, nil
+	}
+
+	memberOf, err := server.requestMemberOf(search)
+	if err != nil {
+		return nil, err
+	}
+
+	return memberOf, nil
 }
 
 func appendIfNotEmpty(slice []string, values ...string) []string {
@@ -527,11 +545,11 @@ func appendIfNotEmpty(slice []string, values ...string) []string {
 	return slice
 }
 
-func getLdapAttr(name string, result *LDAP.SearchResult) string {
+func getLdapAttr(name string, result *ldap.SearchResult) string {
 	return getLdapAttrN(name, result, 0)
 }
 
-func getLdapAttrN(name string, result *LDAP.SearchResult, n int) string {
+func getLdapAttrN(name string, result *ldap.SearchResult, n int) string {
 	if strings.ToLower(name) == "dn" {
 		return result.Entries[n].DN
 	}
@@ -545,11 +563,11 @@ func getLdapAttrN(name string, result *LDAP.SearchResult, n int) string {
 	return ""
 }
 
-func getLdapAttrArray(name string, result *LDAP.SearchResult) []string {
+func getLdapAttrArray(name string, result *ldap.SearchResult) []string {
 	return getLdapAttrArrayN(name, result, 0)
 }
 
-func getLdapAttrArrayN(name string, result *LDAP.SearchResult, n int) []string {
+func getLdapAttrArrayN(name string, result *ldap.SearchResult, n int) []string {
 	for _, attr := range result.Entries[n].Attributes {
 		if attr.Name == name {
 			return attr.Values

pkg/services/ldap/ldap_helpers_test.go

@@ -0,0 +1,140 @@
+package ldap
+
+import (
+	"testing"
+
+	. "github.com/smartystreets/goconvey/convey"
+	"gopkg.in/ldap.v3"
+
+	"github.com/grafana/grafana/pkg/infra/log"
+)
+
+func TestLDAPHelpers(t *testing.T) {
+	Convey("serializeUsers()", t, func() {
+		Convey("simple case", func() {
+			server := &Server{
+				config: &ServerConfig{
+					Attr: AttributeMap{
+						Username: "username",
+						Name:     "name",
+						MemberOf: "memberof",
+						Email:    "email",
+					},
+					SearchBaseDNs: []string{"BaseDNHere"},
+				},
+				connection: &mockConnection{},
+				log:        log.New("test-logger"),
+			}
+
+			entry := ldap.Entry{
+				DN: "dn", Attributes: []*ldap.EntryAttribute{
+					{Name: "username", Values: []string{"roelgerrits"}},
+					{Name: "surname", Values: []string{"Gerrits"}},
+					{Name: "email", Values: []string{"roel@test.com"}},
+					{Name: "name", Values: []string{"Roel"}},
+					{Name: "memberof", Values: []string{"admins"}},
+				}}
+			users := &ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
+
+			result, err := server.serializeUsers(users)
+
+			So(err, ShouldBeNil)
+			So(result[0].Login, ShouldEqual, "roelgerrits")
+			So(result[0].Email, ShouldEqual, "roel@test.com")
+			So(result[0].Groups, ShouldContain, "admins")
+		})
+
+		Convey("without lastname", func() {
+			server := &Server{
+				config: &ServerConfig{
+					Attr: AttributeMap{
+						Username: "username",
+						Name:     "name",
+						MemberOf: "memberof",
+						Email:    "email",
+					},
+					SearchBaseDNs: []string{"BaseDNHere"},
+				},
+				connection: &mockConnection{},
+				log:        log.New("test-logger"),
+			}
+
+			entry := ldap.Entry{
+				DN: "dn", Attributes: []*ldap.EntryAttribute{
+					{Name: "username", Values: []string{"roelgerrits"}},
+					{Name: "email", Values: []string{"roel@test.com"}},
+					{Name: "name", Values: []string{"Roel"}},
+					{Name: "memberof", Values: []string{"admins"}},
+				}}
+			users := &ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
+
+			result, err := server.serializeUsers(users)
+
+			So(err, ShouldBeNil)
+			So(result[0].Name, ShouldEqual, "Roel")
+		})
+	})
+
+	Convey("serverBind()", t, func() {
+		Convey("Given bind dn and password configured", func() {
+			connection := &mockConnection{}
+			var actualUsername, actualPassword string
+			connection.bindProvider = func(username, password string) error {
+				actualUsername = username
+				actualPassword = password
+				return nil
+			}
+			server := &Server{
+				connection: connection,
+				config: &ServerConfig{
+					BindDN:       "o=users,dc=grafana,dc=org",
+					BindPassword: "bindpwd",
+				},
+			}
+			err := server.serverBind()
+			So(err, ShouldBeNil)
+			So(actualUsername, ShouldEqual, "o=users,dc=grafana,dc=org")
+			So(actualPassword, ShouldEqual, "bindpwd")
+		})
+
+		Convey("Given bind dn configured", func() {
+			connection := &mockConnection{}
+			unauthenticatedBindWasCalled := false
+			var actualUsername string
+			connection.unauthenticatedBindProvider = func(username string) error {
+				unauthenticatedBindWasCalled = true
+				actualUsername = username
+				return nil
+			}
+			server := &Server{
+				connection: connection,
+				config: &ServerConfig{
+					BindDN: "o=users,dc=grafana,dc=org",
+				},
+			}
+			err := server.serverBind()
+			So(err, ShouldBeNil)
+			So(unauthenticatedBindWasCalled, ShouldBeTrue)
+			So(actualUsername, ShouldEqual, "o=users,dc=grafana,dc=org")
+		})
+
+		Convey("Given empty bind dn and password", func() {
+			connection := &mockConnection{}
+			unauthenticatedBindWasCalled := false
+			var actualUsername string
+			connection.unauthenticatedBindProvider = func(username string) error {
+				unauthenticatedBindWasCalled = true
+				actualUsername = username
+				return nil
+			}
+			server := &Server{
+				connection: connection,
+				config:     &ServerConfig{},
+			}
+			err := server.serverBind()
+			So(err, ShouldBeNil)
+			So(unauthenticatedBindWasCalled, ShouldBeTrue)
+			So(actualUsername, ShouldBeEmpty)
+		})
+	})
+}

pkg/services/ldap/ldap_login_test.go

@@ -7,23 +7,94 @@ import (
 	"gopkg.in/ldap.v3"
 
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/user"
 )
 
-func TestLdapLogin(t *testing.T) {
-	Convey("Login using ldap", t, func() {
-		AuthScenario("When login with invalid credentials", func(scenario *scenarioContext) {
-			conn := &mockLdapConn{}
+func TestLDAPLogin(t *testing.T) {
+	Convey("Login()", t, func() {
+		authScenario("When user is log in and updated", func(sc *scenarioContext) {
+			// arrange
+			mockConnection := &mockConnection{}
+
+			auth := &Server{
+				config: &ServerConfig{
+					Host:       "",
+					RootCACert: "",
+					Groups: []*GroupToOrgRole{
+						{GroupDN: "*", OrgRole: "Admin"},
+					},
+					Attr: AttributeMap{
+						Username: "username",
+						Surname:  "surname",
+						Email:    "email",
+						Name:     "name",
+						MemberOf: "memberof",
+					},
+					SearchBaseDNs: []string{"BaseDNHere"},
+				},
+				connection: mockConnection,
+				log:        log.New("test-logger"),
+			}
+
+			entry := ldap.Entry{
+				DN: "dn", Attributes: []*ldap.EntryAttribute{
+					{Name: "username", Values: []string{"roelgerrits"}},
+					{Name: "surname", Values: []string{"Gerrits"}},
+					{Name: "email", Values: []string{"roel@test.com"}},
+					{Name: "name", Values: []string{"Roel"}},
+					{Name: "memberof", Values: []string{"admins"}},
+				}}
+			result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
+			mockConnection.setSearchResult(&result)
+
+			query := &models.LoginUserQuery{
+				Username: "roelgerrits",
+			}
+
+			sc.userQueryReturns(&models.User{
+				Id:    1,
+				Email: "roel@test.net",
+				Name:  "Roel Gerrits",
+				Login: "roelgerrits",
+			})
+			sc.userOrgsQueryReturns([]*models.UserOrgDTO{})
+
+			// act
+			extUser, _ := auth.Login(query)
+			userInfo, err := user.Upsert(&user.UpsertArgs{
+				SignupAllowed: true,
+				ExternalUser:  extUser,
+			})
+
+			// assert
+
+			// Check absence of the error
+			So(err, ShouldBeNil)
+
+			// User should be searched in ldap
+			So(mockConnection.searchCalled, ShouldBeTrue)
+
+			// Info should be updated (email differs)
+			So(userInfo.Email, ShouldEqual, "roel@test.com")
+
+			// User should have admin privileges
+			So(sc.addOrgUserCmd.Role, ShouldEqual, "Admin")
+		})
+
+		authScenario("When login with invalid credentials", func(scenario *scenarioContext) {
+			connection := &mockConnection{}
 			entry := ldap.Entry{}
 			result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
-			conn.setSearchResult(&result)
+			connection.setSearchResult(&result)
 
-			conn.bindProvider = func(username, password string) error {
+			connection.bindProvider = func(username, password string) error {
 				return &ldap.Error{
 					ResultCode: 49,
 				}
 			}
-			auth := &Auth{
-				server: &ServerConfig{
+			auth := &Server{
+				config: &ServerConfig{
 					Attr: AttributeMap{
 						Username: "username",
 						Name:     "name",
@@ -31,19 +102,19 @@ func TestLdapLogin(t *testing.T) {
 					},
 					SearchBaseDNs: []string{"BaseDNHere"},
 				},
-				conn: conn,
-				log:  log.New("test-logger"),
+				connection: connection,
+				log:        log.New("test-logger"),
 			}
 
-			err := auth.Login(scenario.loginUserQuery)
+			_, err := auth.Login(scenario.loginUserQuery)
 
 			Convey("it should return invalid credentials error", func() {
 				So(err, ShouldEqual, ErrInvalidCredentials)
 			})
 		})
 
-		AuthScenario("When login with valid credentials", func(scenario *scenarioContext) {
-			conn := &mockLdapConn{}
+		authScenario("When login with valid credentials", func(scenario *scenarioContext) {
+			connection := &mockConnection{}
 			entry := ldap.Entry{
 				DN: "dn", Attributes: []*ldap.EntryAttribute{
 					{Name: "username", Values: []string{"markelog"}},
@@ -54,13 +125,13 @@ func TestLdapLogin(t *testing.T) {
 				},
 			}
 			result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
-			conn.setSearchResult(&result)
+			connection.setSearchResult(&result)
 
-			conn.bindProvider = func(username, password string) error {
+			connection.bindProvider = func(username, password string) error {
 				return nil
 			}
-			auth := &Auth{
-				server: &ServerConfig{
+			auth := &Server{
+				config: &ServerConfig{
 					Attr: AttributeMap{
 						Username: "username",
 						Name:     "name",
@@ -68,19 +139,14 @@ func TestLdapLogin(t *testing.T) {
 					},
 					SearchBaseDNs: []string{"BaseDNHere"},
 				},
-				conn: conn,
-				log:  log.New("test-logger"),
+				connection: connection,
+				log:        log.New("test-logger"),
 			}
 
-			err := auth.Login(scenario.loginUserQuery)
+			resp, err := auth.Login(scenario.loginUserQuery)
 
-			Convey("it should not return error", func() {
-				So(err, ShouldBeNil)
-			})
-
-			Convey("it should get user", func() {
-				So(scenario.loginUserQuery.User.Login, ShouldEqual, "markelog")
-			})
+			So(err, ShouldBeNil)
+			So(resp.Login, ShouldEqual, "markelog")
 		})
 	})
 }

pkg/services/ldap/ldap_test.go

@@ -1,496 +1,157 @@
 package ldap
 
 import (
-	"context"
 	"testing"
 
 	. "github.com/smartystreets/goconvey/convey"
-	"gopkg.in/ldap.v3"
+	ldap "gopkg.in/ldap.v3"
 
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/log"
-	m "github.com/grafana/grafana/pkg/models"
 )
 
 func TestAuth(t *testing.T) {
-	Convey("initialBind", t, func() {
-		Convey("Given bind dn and password configured", func() {
-			conn := &mockLdapConn{}
-			var actualUsername, actualPassword string
-			conn.bindProvider = func(username, password string) error {
-				actualUsername = username
-				actualPassword = password
-				return nil
-			}
-			Auth := &Auth{
-				conn: conn,
-				server: &ServerConfig{
-					BindDN:       "cn=%s,o=users,dc=grafana,dc=org",
-					BindPassword: "bindpwd",
-				},
-			}
-			err := Auth.initialBind("user", "pwd")
-			So(err, ShouldBeNil)
-			So(Auth.requireSecondBind, ShouldBeTrue)
-			So(actualUsername, ShouldEqual, "cn=user,o=users,dc=grafana,dc=org")
-			So(actualPassword, ShouldEqual, "bindpwd")
-		})
+	Convey("Add()", t, func() {
+		connection := &mockConnection{}
 
-		Convey("Given bind dn configured", func() {
-			conn := &mockLdapConn{}
-			var actualUsername, actualPassword string
-			conn.bindProvider = func(username, password string) error {
-				actualUsername = username
-				actualPassword = password
-				return nil
-			}
-			Auth := &Auth{
-				conn: conn,
-				server: &ServerConfig{
-					BindDN: "cn=%s,o=users,dc=grafana,dc=org",
-				},
-			}
-			err := Auth.initialBind("user", "pwd")
-			So(err, ShouldBeNil)
-			So(Auth.requireSecondBind, ShouldBeFalse)
-			So(actualUsername, ShouldEqual, "cn=user,o=users,dc=grafana,dc=org")
-			So(actualPassword, ShouldEqual, "pwd")
-		})
-
-		Convey("Given empty bind dn and password", func() {
-			conn := &mockLdapConn{}
-			unauthenticatedBindWasCalled := false
-			var actualUsername string
-			conn.unauthenticatedBindProvider = func(username string) error {
-				unauthenticatedBindWasCalled = true
-				actualUsername = username
-				return nil
-			}
-			Auth := &Auth{
-				conn:   conn,
-				server: &ServerConfig{},
-			}
-			err := Auth.initialBind("user", "pwd")
-			So(err, ShouldBeNil)
-			So(Auth.requireSecondBind, ShouldBeTrue)
-			So(unauthenticatedBindWasCalled, ShouldBeTrue)
-			So(actualUsername, ShouldBeEmpty)
-		})
-	})
-
-	Convey("serverBind", t, func() {
-		Convey("Given bind dn and password configured", func() {
-			conn := &mockLdapConn{}
-			var actualUsername, actualPassword string
-			conn.bindProvider = func(username, password string) error {
-				actualUsername = username
-				actualPassword = password
-				return nil
-			}
-			Auth := &Auth{
-				conn: conn,
-				server: &ServerConfig{
-					BindDN:       "o=users,dc=grafana,dc=org",
-					BindPassword: "bindpwd",
-				},
-			}
-			err := Auth.serverBind()
-			So(err, ShouldBeNil)
-			So(actualUsername, ShouldEqual, "o=users,dc=grafana,dc=org")
-			So(actualPassword, ShouldEqual, "bindpwd")
-		})
-
-		Convey("Given bind dn configured", func() {
-			conn := &mockLdapConn{}
-			unauthenticatedBindWasCalled := false
-			var actualUsername string
-			conn.unauthenticatedBindProvider = func(username string) error {
-				unauthenticatedBindWasCalled = true
-				actualUsername = username
-				return nil
-			}
-			Auth := &Auth{
-				conn: conn,
-				server: &ServerConfig{
-					BindDN: "o=users,dc=grafana,dc=org",
-				},
-			}
-			err := Auth.serverBind()
-			So(err, ShouldBeNil)
-			So(unauthenticatedBindWasCalled, ShouldBeTrue)
-			So(actualUsername, ShouldEqual, "o=users,dc=grafana,dc=org")
-		})
-
-		Convey("Given empty bind dn and password", func() {
-			conn := &mockLdapConn{}
-			unauthenticatedBindWasCalled := false
-			var actualUsername string
-			conn.unauthenticatedBindProvider = func(username string) error {
-				unauthenticatedBindWasCalled = true
-				actualUsername = username
-				return nil
-			}
-			Auth := &Auth{
-				conn:   conn,
-				server: &ServerConfig{},
-			}
-			err := Auth.serverBind()
-			So(err, ShouldBeNil)
-			So(unauthenticatedBindWasCalled, ShouldBeTrue)
-			So(actualUsername, ShouldBeEmpty)
-		})
-	})
-
-	Convey("When translating ldap user to grafana user", t, func() {
-
-		var user1 = &m.User{}
-
-		bus.AddHandlerCtx("test", func(ctx context.Context, cmd *m.UpsertUserCommand) error {
-			cmd.Result = user1
-			cmd.Result.Login = "torkelo"
-			return nil
-		})
-
-		Convey("Given no ldap group map match", func() {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{{}},
-			})
-			_, err := Auth.GetGrafanaUserFor(nil, &UserInfo{})
-
-			So(err, ShouldEqual, ErrInvalidCredentials)
-		})
-
-		AuthScenario("Given wildcard group match", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "*", OrgRole: "Admin"},
-				},
-			})
-
-			sc.userQueryReturns(user1)
-
-			result, err := Auth.GetGrafanaUserFor(nil, &UserInfo{})
-			So(err, ShouldBeNil)
-			So(result, ShouldEqual, user1)
-		})
-
-		AuthScenario("Given exact group match", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=users", OrgRole: "Admin"},
-				},
-			})
-
-			sc.userQueryReturns(user1)
-
-			result, err := Auth.GetGrafanaUserFor(nil, &UserInfo{MemberOf: []string{"cn=users"}})
-			So(err, ShouldBeNil)
-			So(result, ShouldEqual, user1)
-		})
-
-		AuthScenario("Given group match with different case", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=users", OrgRole: "Admin"},
-				},
-			})
-
-			sc.userQueryReturns(user1)
-
-			result, err := Auth.GetGrafanaUserFor(nil, &UserInfo{MemberOf: []string{"CN=users"}})
-			So(err, ShouldBeNil)
-			So(result, ShouldEqual, user1)
-		})
-
-		AuthScenario("Given no existing grafana user", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=admin", OrgRole: "Admin"},
-					{GroupDN: "cn=editor", OrgRole: "Editor"},
-					{GroupDN: "*", OrgRole: "Viewer"},
-				},
-			})
-
-			sc.userQueryReturns(nil)
-
-			result, err := Auth.GetGrafanaUserFor(nil, &UserInfo{
-				DN:       "torkelo",
-				Username: "torkelo",
-				Email:    "my@email.com",
-				MemberOf: []string{"cn=editor"},
-			})
-
-			So(err, ShouldBeNil)
-
-			Convey("Should return new user", func() {
-				So(result.Login, ShouldEqual, "torkelo")
-			})
-
-			Convey("Should set isGrafanaAdmin to false by default", func() {
-				So(result.IsAdmin, ShouldBeFalse)
-			})
-
-		})
-
-	})
-
-	Convey("When syncing ldap groups to grafana org roles", t, func() {
-		AuthScenario("given no current user orgs", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=users", OrgRole: "Admin"},
-				},
-			})
-
-			sc.userOrgsQueryReturns([]*m.UserOrgDTO{})
-			_, err := Auth.GetGrafanaUserFor(nil, &UserInfo{
-				MemberOf: []string{"cn=users"},
-			})
-
-			Convey("Should create new org user", func() {
-				So(err, ShouldBeNil)
-				So(sc.addOrgUserCmd, ShouldNotBeNil)
-				So(sc.addOrgUserCmd.Role, ShouldEqual, m.ROLE_ADMIN)
-			})
-		})
-
-		AuthScenario("given different current org role", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=users", OrgId: 1, OrgRole: "Admin"},
-				},
-			})
-
-			sc.userOrgsQueryReturns([]*m.UserOrgDTO{{OrgId: 1, Role: m.ROLE_EDITOR}})
-			_, err := Auth.GetGrafanaUserFor(nil, &UserInfo{
-				MemberOf: []string{"cn=users"},
-			})
-
-			Convey("Should update org role", func() {
-				So(err, ShouldBeNil)
-				So(sc.updateOrgUserCmd, ShouldNotBeNil)
-				So(sc.updateOrgUserCmd.Role, ShouldEqual, m.ROLE_ADMIN)
-				So(sc.setUsingOrgCmd.OrgId, ShouldEqual, 1)
-			})
-		})
-
-		AuthScenario("given current org role is removed in ldap", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=users", OrgId: 2, OrgRole: "Admin"},
-				},
-			})
-
-			sc.userOrgsQueryReturns([]*m.UserOrgDTO{
-				{OrgId: 1, Role: m.ROLE_EDITOR},
-				{OrgId: 2, Role: m.ROLE_EDITOR},
-			})
-			_, err := Auth.GetGrafanaUserFor(nil, &UserInfo{
-				MemberOf: []string{"cn=users"},
-			})
-
-			Convey("Should remove org role", func() {
-				So(err, ShouldBeNil)
-				So(sc.removeOrgUserCmd, ShouldNotBeNil)
-				So(sc.setUsingOrgCmd.OrgId, ShouldEqual, 2)
-			})
-		})
-
-		AuthScenario("given org role is updated in config", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=admin", OrgId: 1, OrgRole: "Admin"},
-					{GroupDN: "cn=users", OrgId: 1, OrgRole: "Viewer"},
-				},
-			})
-
-			sc.userOrgsQueryReturns([]*m.UserOrgDTO{{OrgId: 1, Role: m.ROLE_EDITOR}})
-			_, err := Auth.GetGrafanaUserFor(nil, &UserInfo{
-				MemberOf: []string{"cn=users"},
-			})
-
-			Convey("Should update org role", func() {
-				So(err, ShouldBeNil)
-				So(sc.removeOrgUserCmd, ShouldBeNil)
-				So(sc.updateOrgUserCmd, ShouldNotBeNil)
-				So(sc.setUsingOrgCmd.OrgId, ShouldEqual, 1)
-			})
-		})
-
-		AuthScenario("given multiple matching ldap groups", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=admins", OrgId: 1, OrgRole: "Admin"},
-					{GroupDN: "*", OrgId: 1, OrgRole: "Viewer"},
-				},
-			})
-
-			sc.userOrgsQueryReturns([]*m.UserOrgDTO{{OrgId: 1, Role: m.ROLE_ADMIN}})
-			_, err := Auth.GetGrafanaUserFor(nil, &UserInfo{
-				MemberOf: []string{"cn=admins"},
-			})
-
-			Convey("Should take first match, and ignore subsequent matches", func() {
-				So(err, ShouldBeNil)
-				So(sc.updateOrgUserCmd, ShouldBeNil)
-				So(sc.setUsingOrgCmd.OrgId, ShouldEqual, 1)
-			})
-		})
-
-		AuthScenario("given multiple matching ldap groups and no existing groups", func(sc *scenarioContext) {
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=admins", OrgId: 1, OrgRole: "Admin"},
-					{GroupDN: "*", OrgId: 1, OrgRole: "Viewer"},
-				},
-			})
-
-			sc.userOrgsQueryReturns([]*m.UserOrgDTO{})
-			_, err := Auth.GetGrafanaUserFor(nil, &UserInfo{
-				MemberOf: []string{"cn=admins"},
-			})
-
-			Convey("Should take first match, and ignore subsequent matches", func() {
-				So(err, ShouldBeNil)
-				So(sc.addOrgUserCmd.Role, ShouldEqual, m.ROLE_ADMIN)
-				So(sc.setUsingOrgCmd.OrgId, ShouldEqual, 1)
-			})
-
-			Convey("Should not update permissions unless specified", func() {
-				So(err, ShouldBeNil)
-				So(sc.updateUserPermissionsCmd, ShouldBeNil)
-			})
-		})
-
-		AuthScenario("given ldap groups with grafana_admin=true", func(sc *scenarioContext) {
-			trueVal := true
-
-			Auth := New(&ServerConfig{
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "cn=admins", OrgId: 1, OrgRole: "Admin", IsGrafanaAdmin: &trueVal},
-				},
-			})
-
-			sc.userOrgsQueryReturns([]*m.UserOrgDTO{})
-			_, err := Auth.GetGrafanaUserFor(nil, &UserInfo{
-				MemberOf: []string{"cn=admins"},
-			})
-
-			Convey("Should create user with admin set to true", func() {
-				So(err, ShouldBeNil)
-				So(sc.updateUserPermissionsCmd.IsGrafanaAdmin, ShouldBeTrue)
-			})
-		})
-	})
-
-	Convey("When calling SyncUser", t, func() {
-		mockLdapConnection := &mockLdapConn{}
-
-		auth := &Auth{
-			server: &ServerConfig{
-				Host:       "",
-				RootCACert: "",
-				Groups: []*GroupToOrgRole{
-					{GroupDN: "*", OrgRole: "Admin"},
-				},
-				Attr: AttributeMap{
-					Username: "username",
-					Surname:  "surname",
-					Email:    "email",
-					Name:     "name",
-					MemberOf: "memberof",
-				},
+		auth := &Server{
+			config: &ServerConfig{
 				SearchBaseDNs: []string{"BaseDNHere"},
 			},
-			conn: mockLdapConnection,
-			log:  log.New("test-logger"),
+			connection: connection,
+			log:        log.New("test-logger"),
 		}
 
-		dialCalled := false
-		dial = func(network, addr string) (IConnection, error) {
-			dialCalled = true
-			return mockLdapConnection, nil
-		}
+		Convey("Adds user", func() {
+			err := auth.Add(
+				"cn=ldap-tuz,ou=users,dc=grafana,dc=org",
+				map[string][]string{
+					"mail":         {"ldap-viewer@grafana.com"},
+					"userPassword": {"grafana"},
+					"objectClass": {
+						"person",
+						"top",
+						"inetOrgPerson",
+						"organizationalPerson",
+					},
+					"sn": {"ldap-tuz"},
+					"cn": {"ldap-tuz"},
+				},
+			)
 
-		entry := ldap.Entry{
-			DN: "dn", Attributes: []*ldap.EntryAttribute{
-				{Name: "username", Values: []string{"roelgerrits"}},
-				{Name: "surname", Values: []string{"Gerrits"}},
-				{Name: "email", Values: []string{"roel@test.com"}},
-				{Name: "name", Values: []string{"Roel"}},
-				{Name: "memberof", Values: []string{"admins"}},
-			}}
-		result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
-		mockLdapConnection.setSearchResult(&result)
+			hasMail := false
+			hasUserPassword := false
+			hasObjectClass := false
+			hasSN := false
+			hasCN := false
 
-		AuthScenario("When ldapUser found call syncInfo and orgRoles", func(sc *scenarioContext) {
-			// arrange
-			query := &m.LoginUserQuery{
-				Username: "roelgerrits",
+			So(err, ShouldBeNil)
+			So(connection.addParams.Controls, ShouldBeNil)
+			So(connection.addCalled, ShouldBeTrue)
+			So(
+				connection.addParams.DN,
+				ShouldEqual,
+				"cn=ldap-tuz,ou=users,dc=grafana,dc=org",
+			)
+
+			attrs := connection.addParams.Attributes
+			for _, value := range attrs {
+				if value.Type == "mail" {
+					So(value.Vals, ShouldContain, "ldap-viewer@grafana.com")
+					hasMail = true
+				}
+
+				if value.Type == "userPassword" {
+					hasUserPassword = true
+					So(value.Vals, ShouldContain, "grafana")
+				}
+
+				if value.Type == "objectClass" {
+					hasObjectClass = true
+					So(value.Vals, ShouldContain, "person")
+					So(value.Vals, ShouldContain, "top")
+					So(value.Vals, ShouldContain, "inetOrgPerson")
+					So(value.Vals, ShouldContain, "organizationalPerson")
+				}
+
+				if value.Type == "sn" {
+					hasSN = true
+					So(value.Vals, ShouldContain, "ldap-tuz")
+				}
+
+				if value.Type == "cn" {
+					hasCN = true
+					So(value.Vals, ShouldContain, "ldap-tuz")
+				}
 			}
 
-			hookDial = nil
+			So(hasMail, ShouldBeTrue)
+			So(hasUserPassword, ShouldBeTrue)
+			So(hasObjectClass, ShouldBeTrue)
+			So(hasSN, ShouldBeTrue)
+			So(hasCN, ShouldBeTrue)
+		})
+	})
 
-			sc.userQueryReturns(&m.User{
-				Id:    1,
-				Email: "roel@test.net",
-				Name:  "Roel Gerrits",
-				Login: "roelgerrits",
-			})
-			sc.userOrgsQueryReturns([]*m.UserOrgDTO{})
+	Convey("Remove()", t, func() {
+		connection := &mockConnection{}
 
-			// act
-			syncErrResult := auth.SyncUser(query)
+		auth := &Server{
+			config: &ServerConfig{
+				SearchBaseDNs: []string{"BaseDNHere"},
+			},
+			connection: connection,
+			log:        log.New("test-logger"),
+		}
+
+		Convey("Removes the user", func() {
+			dn := "cn=ldap-tuz,ou=users,dc=grafana,dc=org"
+			err := auth.Remove(dn)
+
+			So(err, ShouldBeNil)
+			So(connection.delCalled, ShouldBeTrue)
+			So(connection.delParams.Controls, ShouldBeNil)
+			So(connection.delParams.DN, ShouldEqual, dn)
+		})
+	})
+
+	Convey("Users()", t, func() {
+		Convey("find one user", func() {
+			mockConnection := &mockConnection{}
+			entry := ldap.Entry{
+				DN: "dn", Attributes: []*ldap.EntryAttribute{
+					{Name: "username", Values: []string{"roelgerrits"}},
+					{Name: "surname", Values: []string{"Gerrits"}},
+					{Name: "email", Values: []string{"roel@test.com"}},
+					{Name: "name", Values: []string{"Roel"}},
+					{Name: "memberof", Values: []string{"admins"}},
+				}}
+			result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
+			mockConnection.setSearchResult(&result)
+
+			// Set up attribute map without surname and email
+			server := &Server{
+				config: &ServerConfig{
+					Attr: AttributeMap{
+						Username: "username",
+						Name:     "name",
+						MemberOf: "memberof",
+					},
+					SearchBaseDNs: []string{"BaseDNHere"},
+				},
+				connection: mockConnection,
+				log:        log.New("test-logger"),
+			}
+
+			searchResult, err := server.Users([]string{"roelgerrits"})
+
+			So(err, ShouldBeNil)
+			So(searchResult, ShouldNotBeNil)
 
-			// assert
-			So(dialCalled, ShouldBeTrue)
-			So(syncErrResult, ShouldBeNil)
 			// User should be searched in ldap
-			So(mockLdapConnection.searchCalled, ShouldBeTrue)
-			// Info should be updated (email differs)
-			So(sc.updateUserCmd.Email, ShouldEqual, "roel@test.com")
-			// User should have admin privileges
-			So(sc.addOrgUserCmd.UserId, ShouldEqual, 1)
-			So(sc.addOrgUserCmd.Role, ShouldEqual, "Admin")
+			So(mockConnection.searchCalled, ShouldBeTrue)
+
+			// No empty attributes should be added to the search request
+			So(len(mockConnection.searchAttributes), ShouldEqual, 3)
 		})
 	})
-
-	Convey("When searching for a user and not all five attributes are mapped", t, func() {
-		mockLdapConnection := &mockLdapConn{}
-		entry := ldap.Entry{
-			DN: "dn", Attributes: []*ldap.EntryAttribute{
-				{Name: "username", Values: []string{"roelgerrits"}},
-				{Name: "surname", Values: []string{"Gerrits"}},
-				{Name: "email", Values: []string{"roel@test.com"}},
-				{Name: "name", Values: []string{"Roel"}},
-				{Name: "memberof", Values: []string{"admins"}},
-			}}
-		result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
-		mockLdapConnection.setSearchResult(&result)
-
-		// Set up attribute map without surname and email
-		Auth := &Auth{
-			server: &ServerConfig{
-				Attr: AttributeMap{
-					Username: "username",
-					Name:     "name",
-					MemberOf: "memberof",
-				},
-				SearchBaseDNs: []string{"BaseDNHere"},
-			},
-			conn: mockLdapConnection,
-			log:  log.New("test-logger"),
-		}
-
-		searchResult, err := Auth.searchForUser("roelgerrits")
-
-		So(err, ShouldBeNil)
-		So(searchResult, ShouldNotBeNil)
-
-		// User should be searched in ldap
-		So(mockLdapConnection.searchCalled, ShouldBeTrue)
-
-		// No empty attributes should be added to the search request
-		So(len(mockLdapConnection.searchAttributes), ShouldEqual, 3)
-	})
 }

pkg/services/ldap/settings.go

@@ -13,10 +13,12 @@ import (
 	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
+// Config holds list of connections to LDAP
 type Config struct {
 	Servers []*ServerConfig `toml:"servers"`
 }
 
+// ServerConfig holds connection data to LDAP
 type ServerConfig struct {
 	Host          string       `toml:"host"`
 	Port          int          `toml:"port"`
@@ -108,11 +110,11 @@ func readConfig(configFile string) (*Config, error) {
 
 	_, err := toml.DecodeFile(configFile, result)
 	if err != nil {
-		return nil, errutil.Wrap("Failed to load ldap config file", err)
+		return nil, errutil.Wrap("Failed to load LDAP config file", err)
 	}
 
 	if len(result.Servers) == 0 {
-		return nil, xerrors.New("ldap enabled but no ldap servers defined in config file")
+		return nil, xerrors.New("LDAP enabled but no LDAP servers defined in config file")
 	}
 
 	// set default org id

pkg/services/ldap/test.go

@@ -12,15 +12,22 @@ import (
 	"github.com/grafana/grafana/pkg/services/login"
 )
 
-type mockLdapConn struct {
-	result                      *ldap.SearchResult
-	searchCalled                bool
-	searchAttributes            []string
+type mockConnection struct {
+	searchResult     *ldap.SearchResult
+	searchCalled     bool
+	searchAttributes []string
+
+	addParams *ldap.AddRequest
+	addCalled bool
+
+	delParams *ldap.DelRequest
+	delCalled bool
+
 	bindProvider                func(username, password string) error
 	unauthenticatedBindProvider func(username string) error
 }
 
-func (c *mockLdapConn) Bind(username, password string) error {
+func (c *mockConnection) Bind(username, password string) error {
 	if c.bindProvider != nil {
 		return c.bindProvider(username, password)
 	}
@@ -28,7 +35,7 @@ func (c *mockLdapConn) Bind(username, password string) error {
 	return nil
 }
 
-func (c *mockLdapConn) UnauthenticatedBind(username string) error {
+func (c *mockConnection) UnauthenticatedBind(username string) error {
 	if c.unauthenticatedBindProvider != nil {
 		return c.unauthenticatedBindProvider(username)
 	}
@@ -36,23 +43,35 @@ func (c *mockLdapConn) UnauthenticatedBind(username string) error {
 	return nil
 }
 
-func (c *mockLdapConn) Close() {}
+func (c *mockConnection) Close() {}
 
-func (c *mockLdapConn) setSearchResult(result *ldap.SearchResult) {
-	c.result = result
+func (c *mockConnection) setSearchResult(result *ldap.SearchResult) {
+	c.searchResult = result
 }
 
-func (c *mockLdapConn) Search(sr *ldap.SearchRequest) (*ldap.SearchResult, error) {
+func (c *mockConnection) Search(sr *ldap.SearchRequest) (*ldap.SearchResult, error) {
 	c.searchCalled = true
 	c.searchAttributes = sr.Attributes
-	return c.result, nil
+	return c.searchResult, nil
 }
 
-func (c *mockLdapConn) StartTLS(*tls.Config) error {
+func (c *mockConnection) Add(request *ldap.AddRequest) error {
+	c.addCalled = true
+	c.addParams = request
 	return nil
 }
 
-func AuthScenario(desc string, fn scenarioFunc) {
+func (c *mockConnection) Del(request *ldap.DelRequest) error {
+	c.delCalled = true
+	c.delParams = request
+	return nil
+}
+
+func (c *mockConnection) StartTLS(*tls.Config) error {
+	return nil
+}
+
+func authScenario(desc string, fn scenarioFunc) {
 	Convey(desc, func() {
 		defer bus.ClearBusHandlers()
 
@@ -64,10 +83,6 @@ func AuthScenario(desc string, fn scenarioFunc) {
 			},
 		}
 
-		hookDial = func(auth *Auth) error {
-			return nil
-		}
-
 		loginService := &login.LoginService{
 			Bus: bus.GetBus(),
 		}