diff --git a/conf/defaults.ini b/conf/defaults.ini index c9eff7d047c..5c892840a67 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -413,7 +413,7 @@ allow_sign_up = true # LDAP backround sync (Enterprise only) # At 1 am every day sync_cron = "0 0 1 * * *" -active_sync_enabled = false +active_sync_enabled = true #################################### SMTP / Emailing ##################### [smtp] diff --git a/conf/sample.ini b/conf/sample.ini index 39b81f897aa..fc6c0b11caa 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -378,7 +378,7 @@ # LDAP backround sync (Enterprise only) # At 1 am every day ;sync_cron = "0 0 1 * * *" -;active_sync_enabled = false +;active_sync_enabled = true #################################### SMTP / Emailing ########################## [smtp] diff --git a/docs/sources/auth/enhanced_ldap.md b/docs/sources/auth/enhanced_ldap.md index 1a9aa9d7241..885f0478f80 100644 --- a/docs/sources/auth/enhanced_ldap.md +++ b/docs/sources/auth/enhanced_ldap.md @@ -44,7 +44,7 @@ a user as member of a team and it will not be removed when the user signs in. Th ## Active LDAP Synchronization -> Only available in Grafana Enterprise v6.3+ +> Only available in Grafana Enterprise v6.3+ In the open source version of Grafana, user data from LDAP will be synchronized only during the login process when authenticating using LDAP. @@ -60,8 +60,11 @@ With this feature you can configure Grafana to actively sync users with LDAP ser # @weekly | Run once a week, midnight between Sat/Sun | 0 0 0 * * 0 # @daily (or @midnight) | Run once a day, midnight | 0 0 0 * * * # @hourly | Run once an hour, beginning of hour | 0 0 * * * * -sync_cron = "@hourly" +sync_cron = "0 0 1 * * *" # This is default value (At 1 am every day) # This cron expression format uses 6 space-separated fields (including seconds), for example # sync_cron = "* */10 * * * *" # This will run the LDAP Synchronization every 10th minute, which is also the minimal interval between the grafana sync times i.e. you cannot set it for every 9th minute + +# You can also disable active LDAP synchronization +active_sync_enabled = true # enabled by default ``` diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index 4312355f924..daf0dfe93f6 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -2,7 +2,9 @@ package login import ( "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/multildap" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util/errutil" @@ -17,6 +19,9 @@ var isLDAPEnabled = multildap.IsEnabled // newLDAP creates multiple LDAP instance var newLDAP = multildap.New +// logger for the LDAP auth +var logger = log.New("login.ldap") + // loginUsingLDAP logs in user using LDAP. It returns whether LDAP is enabled and optional error and query arg will be // populated with the logged in user if successful. var loginUsingLDAP = func(query *models.LoginUserQuery) (bool, error) { @@ -33,6 +38,13 @@ var loginUsingLDAP = func(query *models.LoginUserQuery) (bool, error) { externalUser, err := newLDAP(config.Servers).Login(query) if err != nil { + if err == ldap.ErrCouldNotFindUser { + // Ignore the error since user might not be present anyway + disableExternalUser(query.Username) + + return true, ldap.ErrInvalidCredentials + } + return true, err } @@ -48,3 +60,43 @@ var loginUsingLDAP = func(query *models.LoginUserQuery) (bool, error) { return true, nil } + +// disableExternalUser marks external user as disabled in Grafana db +func disableExternalUser(username string) error { + // Check if external user exist in Grafana + userQuery := &models.GetExternalUserInfoByLoginQuery{ + LoginOrEmail: username, + } + + if err := bus.Dispatch(userQuery); err != nil { + return err + } + + userInfo := userQuery.Result + if !userInfo.IsDisabled { + + logger.Debug( + "Disabling external user", + "user", + userQuery.Result.Login, + ) + + // Mark user as disabled in grafana db + disableUserCmd := &models.DisableUserCommand{ + UserId: userQuery.Result.UserId, + IsDisabled: true, + } + + if err := bus.Dispatch(disableUserCmd); err != nil { + logger.Debug( + "Error disabling external user", + "user", + userQuery.Result.Login, + "message", + err.Error(), + ) + return err + } + } + return nil +} diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index bea06b45a2d..0e4404340b5 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -12,7 +12,6 @@ import ( "github.com/davecgh/go-spew/spew" "gopkg.in/ldap.v3" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" ) @@ -53,6 +52,9 @@ var ( // ErrInvalidCredentials is returned if username and password do not match ErrInvalidCredentials = errors.New("Invalid Username or Password") + + // ErrCouldNotFindUser is returned when username hasn't been found (not username+password) + ErrCouldNotFindUser = errors.New("Can't find user in LDAP") ) // New creates the new LDAP auth @@ -123,14 +125,35 @@ func (server *Server) Close() { server.Connection.Close() } -// Login user by searching and serializing it +// 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 func (server *Server) Login(query *models.LoginUserQuery) ( *models.ExternalUserInfo, error, ) { - // Authentication - err := server.Auth(query.Username, query.Password) - if err != nil { - return nil, err + var err error + + // Do we need to authenticate the "admin" user first? + // Admin user should have access for the user search in LDAP server + if server.shouldAuthAdmin() { + if err := server.AuthAdmin(); err != nil { + return nil, err + } + + // Or if anyone can perform the search in LDAP? + } else { + err := server.Connection.UnauthenticatedBind(server.Config.BindDN) + if err != nil { + return nil, err + } } // Find user entry & attributes @@ -142,8 +165,7 @@ func (server *Server) Login(query *models.LoginUserQuery) ( // If we couldn't find the user - // we should show incorrect credentials err if len(users) == 0 { - server.disableExternalUser(query.Username) - return nil, ErrInvalidCredentials + return nil, ErrCouldNotFindUser } user := users[0] @@ -151,6 +173,12 @@ func (server *Server) Login(query *models.LoginUserQuery) ( return nil, err } + // Authenticate user + err = server.Auth(user.AuthId, query.Password) + if err != nil { + return nil, err + } + return user, nil } @@ -252,34 +280,6 @@ func (server *Server) validateGrafanaUser(user *models.ExternalUserInfo) error { return nil } -// disableExternalUser marks external user as disabled in Grafana db -func (server *Server) disableExternalUser(username string) error { - // Check if external user exist in Grafana - userQuery := &models.GetExternalUserInfoByLoginQuery{ - LoginOrEmail: username, - } - - if err := bus.Dispatch(userQuery); err != nil { - return err - } - - userInfo := userQuery.Result - if !userInfo.IsDisabled { - server.log.Debug("Disabling external user", "user", userQuery.Result.Login) - // Mark user as disabled in grafana db - disableUserCmd := &models.DisableUserCommand{ - UserId: userQuery.Result.UserId, - IsDisabled: true, - } - - if err := bus.Dispatch(disableUserCmd); err != nil { - server.log.Debug("Error disabling external user", "user", userQuery.Result.Login, "message", err.Error()) - return err - } - } - return nil -} - // getSearchRequest returns LDAP search request for users func (server *Server) getSearchRequest( base string, @@ -360,32 +360,46 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*models.ExternalUserIn return extUser, nil } -// shouldBindAdmin checks if we should use +// shouldAuthAdmin checks if we should use // admin username & password for LDAP bind -func (server *Server) shouldBindAdmin() bool { +func (server *Server) shouldAuthAdmin() bool { return server.Config.BindPassword != "" } -// Auth authentificates user in LDAP. -// It might not use passed password and username, -// since they can be overwritten with admin config values - -// see "bind_dn" and "bind_password" options in LDAP config +// Auth authentificates user in LDAP func (server *Server) Auth(username, password string) error { - path := server.Config.BindDN - - if server.shouldBindAdmin() { - password = server.Config.BindPassword - } else { - path = fmt.Sprintf(path, username) + err := server.auth(username, password) + if err != nil { + server.log.Error( + fmt.Sprintf("Cannot authentificate user %s in LDAP", username), + "error", + err, + ) + return err } - bindFn := func() error { - return server.Connection.Bind(path, password) + return nil +} + +// AuthAdmin authentificates LDAP admin user +func (server *Server) AuthAdmin() error { + err := server.auth(server.Config.BindDN, server.Config.BindPassword) + if err != nil { + server.log.Error( + "Cannot authentificate admin user in LDAP", + "error", + err, + ) + return err } - if err := bindFn(); err != nil { - server.log.Error("Cannot authentificate in LDAP", "err", err) + return nil +} +// auth is helper for several types of LDAP authentification +func (server *Server) auth(path, password string) error { + err := server.Connection.Bind(path, password) + if err != nil { if ldapErr, ok := err.(*ldap.Error); ok { if ldapErr.ResultCode == 49 { return ErrInvalidCredentials diff --git a/pkg/services/ldap/ldap_login_test.go b/pkg/services/ldap/ldap_login_test.go index 162cba53c91..1828eeb9d7b 100644 --- a/pkg/services/ldap/ldap_login_test.go +++ b/pkg/services/ldap/ldap_login_test.go @@ -4,10 +4,11 @@ import ( "errors" "testing" - "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" "gopkg.in/ldap.v3" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/models" ) func TestLDAPLogin(t *testing.T) { @@ -24,7 +25,7 @@ func TestLDAPLogin(t *testing.T) { result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}} connection.setSearchResult(&result) - connection.bindProvider = func(username, password string) error { + connection.BindProvider = func(username, password string) error { return &ldap.Error{ ResultCode: 49, } @@ -42,12 +43,12 @@ func TestLDAPLogin(t *testing.T) { So(err, ShouldEqual, ErrInvalidCredentials) }) - Convey("Returns an error when search hasn't find anything", func() { + Convey("Returns an error when search didn't find anything", func() { connection := &MockConnection{} result := ldap.SearchResult{Entries: []*ldap.Entry{}} connection.setSearchResult(&result) - connection.bindProvider = func(username, password string) error { + connection.BindProvider = func(username, password string) error { return nil } server := &Server{ @@ -60,7 +61,7 @@ func TestLDAPLogin(t *testing.T) { _, err := server.Login(defaultLogin) - So(err, ShouldEqual, ErrInvalidCredentials) + So(err, ShouldEqual, ErrCouldNotFindUser) }) Convey("When search returns an error", func() { @@ -68,7 +69,7 @@ func TestLDAPLogin(t *testing.T) { expected := errors.New("Killa-gorilla") connection.setSearchError(expected) - connection.bindProvider = func(username, password string) error { + connection.BindProvider = func(username, password string) error { return nil } server := &Server{ @@ -98,7 +99,7 @@ func TestLDAPLogin(t *testing.T) { result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}} connection.setSearchResult(&result) - connection.bindProvider = func(username, password string) error { + connection.BindProvider = func(username, password string) error { return nil } server := &Server{ @@ -119,5 +120,83 @@ func TestLDAPLogin(t *testing.T) { So(err, ShouldBeNil) So(resp.Login, ShouldEqual, "markelog") }) + + Convey("Should perform unauthentificate bind without admin", func() { + connection := &MockConnection{} + entry := ldap.Entry{ + DN: "test", + } + result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}} + connection.setSearchResult(&result) + + connection.UnauthenticatedBindProvider = func() error { + return nil + } + server := &Server{ + Config: &ServerConfig{ + SearchBaseDNs: []string{"BaseDNHere"}, + }, + Connection: connection, + log: log.New("test-logger"), + } + + user, err := server.Login(defaultLogin) + + So(err, ShouldBeNil) + So(user.AuthId, ShouldEqual, "test") + So(connection.UnauthenticatedBindCalled, ShouldBeTrue) + }) + + Convey("Should perform authentificate binds", func() { + connection := &MockConnection{} + entry := ldap.Entry{ + DN: "test", + } + result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}} + connection.setSearchResult(&result) + + adminUsername := "" + adminPassword := "" + username := "" + password := "" + + i := 0 + connection.BindProvider = func(name, pass string) error { + i++ + if i == 1 { + adminUsername = name + adminPassword = pass + } + + if i == 2 { + username = name + password = pass + } + + return nil + } + server := &Server{ + Config: &ServerConfig{ + BindDN: "killa", + BindPassword: "gorilla", + SearchBaseDNs: []string{"BaseDNHere"}, + }, + Connection: connection, + log: log.New("test-logger"), + } + + user, err := server.Login(defaultLogin) + + So(err, ShouldBeNil) + + So(user.AuthId, ShouldEqual, "test") + So(connection.BindCalled, ShouldBeTrue) + + So(adminUsername, ShouldEqual, "killa") + So(adminPassword, ShouldEqual, "gorilla") + + So(username, ShouldEqual, "test") + So(password, ShouldEqual, "pwd") + }) }) } diff --git a/pkg/services/ldap/ldap_private_test.go b/pkg/services/ldap/ldap_private_test.go index d7e8c57aeb9..f1ec97c176b 100644 --- a/pkg/services/ldap/ldap_private_test.go +++ b/pkg/services/ldap/ldap_private_test.go @@ -143,4 +143,29 @@ func TestLDAPPrivateMethods(t *testing.T) { So(result, ShouldBeNil) }) }) + + Convey("shouldAuthAdmin()", t, func() { + Convey("it should require admin auth", func() { + server := &Server{ + Config: &ServerConfig{ + BindPassword: "test", + }, + } + + result := server.shouldAuthAdmin() + So(result, ShouldBeTrue) + }) + + Convey("it should not require admin auth", func() { + server := &Server{ + Config: &ServerConfig{ + BindPassword: "", + }, + } + + result := server.shouldAuthAdmin() + So(result, ShouldBeFalse) + }) + }) + } diff --git a/pkg/services/ldap/ldap_test.go b/pkg/services/ldap/ldap_test.go index 3aca78f2e9e..8207e0d3187 100644 --- a/pkg/services/ldap/ldap_test.go +++ b/pkg/services/ldap/ldap_test.go @@ -103,10 +103,10 @@ func TestPublicAPI(t *testing.T) { }) Convey("Auth()", t, func() { - Convey("Should ignore passsed username and password", func() { + Convey("Should use provided DN and password", func() { connection := &MockConnection{} var actualUsername, actualPassword string - connection.bindProvider = func(username, password string) error { + connection.BindProvider = func(username, password string) error { actualUsername = username actualPassword = password return nil @@ -114,33 +114,15 @@ func TestPublicAPI(t *testing.T) { server := &Server{ Connection: connection, Config: &ServerConfig{ - BindDN: "cn=admin,dc=grafana,dc=org", - BindPassword: "bindpwd", + BindDN: "cn=admin,dc=grafana,dc=org", }, } - err := server.Auth("user", "pwd") - So(err, ShouldBeNil) - So(actualUsername, ShouldEqual, "cn=admin,dc=grafana,dc=org") - So(actualPassword, ShouldEqual, "bindpwd") - }) - Convey("Given bind dn 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: "cn=%s,o=users,dc=grafana,dc=org", - }, - } - err := server.Auth("user", "pwd") + dn := "cn=user,ou=users,dc=grafana,dc=org" + err := server.Auth(dn, "pwd") + So(err, ShouldBeNil) - So(actualUsername, ShouldEqual, "cn=user,o=users,dc=grafana,dc=org") + So(actualUsername, ShouldEqual, dn) So(actualPassword, ShouldEqual, "pwd") }) @@ -149,13 +131,13 @@ func TestPublicAPI(t *testing.T) { expected := &ldap.Error{ ResultCode: uint16(25), } - connection.bindProvider = func(username, password string) error { + connection.BindProvider = func(username, password string) error { return expected } server := &Server{ Connection: connection, Config: &ServerConfig{ - BindDN: "cn=%s,o=users,dc=grafana,dc=org", + BindDN: "cn=%s,ou=users,dc=grafana,dc=org", }, log: log.New("test-logger"), } @@ -163,4 +145,56 @@ func TestPublicAPI(t *testing.T) { So(err, ShouldEqual, expected) }) }) + + Convey("AuthAdmin()", t, func() { + Convey("Should use admin DN and password", func() { + connection := &MockConnection{} + var actualUsername, actualPassword string + connection.BindProvider = func(username, password string) error { + actualUsername = username + actualPassword = password + return nil + } + + dn := "cn=admin,dc=grafana,dc=org" + + server := &Server{ + Connection: connection, + Config: &ServerConfig{ + BindPassword: "pwd", + BindDN: dn, + }, + } + + err := server.AuthAdmin() + + So(err, ShouldBeNil) + So(actualUsername, ShouldEqual, dn) + So(actualPassword, ShouldEqual, "pwd") + }) + + Convey("Should handle an error", func() { + connection := &MockConnection{} + expected := &ldap.Error{ + ResultCode: uint16(25), + } + connection.BindProvider = func(username, password string) error { + return expected + } + + dn := "cn=admin,dc=grafana,dc=org" + + server := &Server{ + Connection: connection, + Config: &ServerConfig{ + BindPassword: "pwd", + BindDN: dn, + }, + log: log.New("test-logger"), + } + + err := server.AuthAdmin() + So(err, ShouldEqual, expected) + }) + }) } diff --git a/pkg/services/ldap/testing.go b/pkg/services/ldap/testing.go index fda9fd95a8c..7e0b33fcbd5 100644 --- a/pkg/services/ldap/testing.go +++ b/pkg/services/ldap/testing.go @@ -19,14 +19,19 @@ type MockConnection struct { DelParams *ldap.DelRequest DelCalled bool - bindProvider func(username, password string) error - unauthenticatedBindProvider func(username string) error + UnauthenticatedBindCalled bool + BindCalled bool + + BindProvider func(username, password string) error + UnauthenticatedBindProvider func() error } // Bind mocks Bind connection function func (c *MockConnection) Bind(username, password string) error { - if c.bindProvider != nil { - return c.bindProvider(username, password) + c.BindCalled = true + + if c.BindProvider != nil { + return c.BindProvider(username, password) } return nil @@ -34,8 +39,10 @@ func (c *MockConnection) Bind(username, password string) error { // UnauthenticatedBind mocks UnauthenticatedBind connection function func (c *MockConnection) UnauthenticatedBind(username string) error { - if c.unauthenticatedBindProvider != nil { - return c.unauthenticatedBindProvider(username) + c.UnauthenticatedBindCalled = true + + if c.UnauthenticatedBindProvider != nil { + return c.UnauthenticatedBindProvider() } return nil