diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index 15aff7be5c3..5160beb406c 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -48,8 +48,8 @@ type Server struct { // - with the username and password setup in the config // - or, anonymously func (server *Server) Bind() error { - if server.shouldAuthAdmin() { - if err := server.AuthAdmin(); err != nil { + if server.shouldAdminBind() { + if err := server.AdminBind(); err != nil { return err } } else { @@ -75,7 +75,7 @@ var ( ErrCouldNotFindUser = errors.New("Can't find user in LDAP") ) -// New creates the new LDAP auth +// New creates the new LDAP connection func New(config *ServerConfig) IServer { return &Server{ Config: config, @@ -84,6 +84,7 @@ func New(config *ServerConfig) IServer { } // Dial dials in the LDAP +// TODO: decrease cyclomatic complexity func (server *Server) Dial() error { var err error var certPool *x509.CertPool @@ -144,16 +145,20 @@ func (server *Server) Close() { } // Login the user. -// There is several cases - -// 1. First we check if we need to authenticate the admin user. -// That user should have search privileges. -// 2. For some configurations it is allowed to search the -// user without any authenfication, in such case we -// perform "unauthenticated bind". -// -- -// After either first or second step is done we find the user DN -// by its username, after that, we then combine it with user password and -// then try to authentificate that user +// There are several cases - +// 1. "admin" user +// Bind the "admin" user (defined in Grafana config file) which has the search privileges +// in LDAP server, then we search the targeted user through that bind, then the second +// perform the bind via passed login/password. +// 2. Single bind +// // If all the users meant to be used with Grafana have the ability to search in LDAP server +// then we bind with LDAP server with targeted login/password +// and then search for the said user in order to retrive all the information about them +// 3. Unauthenticated bind +// For some LDAP configurations it is allowed to search the +// user without login/password binding with LDAP server, in such case +// we will perform "unauthenticated bind", then search for the +// targeted user and then perform the bind with passed login/password. func (server *Server) Login(query *models.LoginUserQuery) ( *models.ExternalUserInfo, error, ) { @@ -161,13 +166,16 @@ func (server *Server) Login(query *models.LoginUserQuery) ( var authAndBind bool // Check if we can use a search user - if server.shouldAuthAdmin() { - if err := server.AuthAdmin(); err != nil { + if server.shouldAdminBind() { + if err := server.AdminBind(); err != nil { return nil, err } } else if server.shouldSingleBind() { authAndBind = true - err = server.UserBind(server.singleBindDN(query.Username), query.Password) + err = server.UserBind( + server.singleBindDN(query.Username), + query.Password, + ) if err != nil { return nil, err } @@ -206,38 +214,24 @@ func (server *Server) Login(query *models.LoginUserQuery) ( return user, nil } +// shouldAdminBind checks if we should use +// admin username & password for LDAP bind +func (server *Server) shouldAdminBind() bool { + return server.Config.BindPassword != "" +} + +// singleBindDN combines the bind with the username +// in order to get the proper path func (server *Server) singleBindDN(username string) string { return fmt.Sprintf(server.Config.BindDN, username) } +// shouldSingleBind checks if we can use "single bind" approach func (server *Server) shouldSingleBind() bool { return strings.Contains(server.Config.BindDN, "%s") } -// getUsersIteration is a helper function for Users() method. -// It divides the users by equal parts for the anticipated requests -func getUsersIteration(logins []string, fn func(int, int) error) error { - lenLogins := len(logins) - iterations := int( - math.Ceil( - float64(lenLogins) / float64(UsersMaxRequest), - ), - ) - - for i := 1; i < iterations+1; i++ { - previous := float64(UsersMaxRequest * (i - 1)) - current := math.Min(float64(i*UsersMaxRequest), float64(lenLogins)) - - err := fn(int(previous), int(current)) - if err != nil { - return err - } - } - - return nil -} - -// Users gets LDAP users +// Users gets LDAP users by logins func (server *Server) Users(logins []string) ( []*models.ExternalUserInfo, error, @@ -273,6 +267,29 @@ func (server *Server) Users(logins []string) ( return serializedUsers, nil } +// getUsersIteration is a helper function for Users() method. +// It divides the users by equal parts for the anticipated requests +func getUsersIteration(logins []string, fn func(int, int) error) error { + lenLogins := len(logins) + iterations := int( + math.Ceil( + float64(lenLogins) / float64(UsersMaxRequest), + ), + ) + + for i := 1; i < iterations+1; i++ { + previous := float64(UsersMaxRequest * (i - 1)) + current := math.Min(float64(i*UsersMaxRequest), float64(lenLogins)) + + err := fn(int(previous), int(current)) + if err != nil { + return err + } + } + + return nil +} + // users is helper method for the Users() func (server *Server) users(logins []string) ( []*ldap.Entry, @@ -304,7 +321,7 @@ func (server *Server) users(logins []string) ( 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", + "User does not belong in any of the specified LDAP groups", "username", user.Login, "groups", user.Groups, ) @@ -397,18 +414,12 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*models.ExternalUserIn return extUser, nil } -// shouldAuthAdmin checks if we should use -// admin username & password for LDAP bind -func (server *Server) shouldAuthAdmin() bool { - return server.Config.BindPassword != "" -} - -// UserBind authenticates the connection with the LDAP server +// UserBind binds the user with the LDAP server func (server *Server) UserBind(username, password string) error { err := server.userBind(username, password) if err != nil { server.log.Error( - fmt.Sprintf("Cannot authentificate user %s in LDAP", username), + fmt.Sprintf("Cannot bind user %s with LDAP", username), "error", err, ) @@ -418,8 +429,8 @@ func (server *Server) UserBind(username, password string) error { return nil } -// AuthAdmin authentificates LDAP admin user -func (server *Server) AuthAdmin() error { +// AdminBind binds "admin" user with LDAP +func (server *Server) AdminBind() error { err := server.userBind(server.Config.BindDN, server.Config.BindPassword) if err != nil { server.log.Error( @@ -433,7 +444,7 @@ func (server *Server) AuthAdmin() error { return nil } -// userBind authenticates the connection with the LDAP server +// userBind binds the user with the LDAP server func (server *Server) userBind(path, password string) error { err := server.Connection.Bind(path, password) if err != nil { @@ -448,7 +459,8 @@ func (server *Server) userBind(path, password string) error { return nil } -// requestMemberOf use this function when POSIX LDAP schema does not support memberOf, so it manually search the groups +// requestMemberOf use this function when POSIX LDAP +// schema does not support memberOf, so it manually search the groups func (server *Server) requestMemberOf(entry *ldap.Entry) ([]string, error) { var memberOf []string var config = server.Config diff --git a/pkg/services/ldap/ldap_private_test.go b/pkg/services/ldap/ldap_private_test.go index d089eab04b9..ff8bc430ed9 100644 --- a/pkg/services/ldap/ldap_private_test.go +++ b/pkg/services/ldap/ldap_private_test.go @@ -182,7 +182,7 @@ func TestLDAPPrivateMethods(t *testing.T) { }) }) - Convey("shouldAuthAdmin()", t, func() { + Convey("shouldAdminBind()", t, func() { Convey("it should require admin userBind", func() { server := &Server{ Config: &ServerConfig{ @@ -190,7 +190,7 @@ func TestLDAPPrivateMethods(t *testing.T) { }, } - result := server.shouldAuthAdmin() + result := server.shouldAdminBind() So(result, ShouldBeTrue) }) @@ -201,9 +201,46 @@ func TestLDAPPrivateMethods(t *testing.T) { }, } - result := server.shouldAuthAdmin() + result := server.shouldAdminBind() So(result, ShouldBeFalse) }) }) + Convey("shouldSingleBind()", t, func() { + Convey("it should allow single bind", func() { + server := &Server{ + Config: &ServerConfig{ + BindDN: "cn=%s,dc=grafana,dc=org", + }, + } + + result := server.shouldSingleBind() + So(result, ShouldBeTrue) + }) + + Convey("it should not allow single bind", func() { + server := &Server{ + Config: &ServerConfig{ + BindDN: "cn=admin,dc=grafana,dc=org", + }, + } + + result := server.shouldSingleBind() + So(result, ShouldBeFalse) + }) + }) + + Convey("singleBindDN()", t, func() { + Convey("it should allow single bind", func() { + server := &Server{ + Config: &ServerConfig{ + BindDN: "cn=%s,dc=grafana,dc=org", + }, + } + + result := server.singleBindDN("test") + So(result, ShouldEqual, "cn=test,dc=grafana,dc=org") + }) + }) + } diff --git a/pkg/services/ldap/ldap_test.go b/pkg/services/ldap/ldap_test.go index 5a87d89bdaa..9f89bf43833 100644 --- a/pkg/services/ldap/ldap_test.go +++ b/pkg/services/ldap/ldap_test.go @@ -146,7 +146,7 @@ func TestPublicAPI(t *testing.T) { }) }) - Convey("AuthAdmin()", t, func() { + Convey("AdminBind()", t, func() { Convey("Should use admin DN and password", func() { connection := &MockConnection{} var actualUsername, actualPassword string @@ -166,7 +166,7 @@ func TestPublicAPI(t *testing.T) { }, } - err := server.AuthAdmin() + err := server.AdminBind() So(err, ShouldBeNil) So(actualUsername, ShouldEqual, dn) @@ -193,7 +193,7 @@ func TestPublicAPI(t *testing.T) { log: log.New("test-logger"), } - err := server.AuthAdmin() + err := server.AdminBind() So(err, ShouldEqual, expected) }) })