diff --git a/pkg/login/auth.go b/pkg/login/auth.go index 19232e5f441..7b384336d35 100644 --- a/pkg/login/auth.go +++ b/pkg/login/auth.go @@ -4,6 +4,7 @@ import ( "errors" "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" ) @@ -19,6 +20,8 @@ var ( ErrUserDisabled = errors.New("User is disabled") ) +var loginLogger = log.New("login") + func Init() { bus.AddHandler("auth", AuthenticateUser) } @@ -50,7 +53,10 @@ func AuthenticateUser(query *models.LoginUserQuery) error { } if err == ErrInvalidCredentials || err == ldap.ErrInvalidCredentials { - saveInvalidLoginAttempt(query) + if err := saveInvalidLoginAttempt(query); err != nil { + loginLogger.Error("Failed to save invalid login attempt", "err", err) + } + return ErrInvalidCredentials } diff --git a/pkg/login/auth_test.go b/pkg/login/auth_test.go index 06a78cc6d5d..a1284b3bc50 100644 --- a/pkg/login/auth_test.go +++ b/pkg/login/auth_test.go @@ -202,8 +202,9 @@ func mockLoginAttemptValidation(err error, sc *authScenarioContext) { } func mockSaveInvalidLoginAttempt(sc *authScenarioContext) { - saveInvalidLoginAttempt = func(query *models.LoginUserQuery) { + saveInvalidLoginAttempt = func(query *models.LoginUserQuery) error { sc.saveInvalidLoginAttemptWasCalled = true + return nil } } diff --git a/pkg/login/brute_force_login_protection.go b/pkg/login/brute_force_login_protection.go index d524c420540..6bb90d8ca13 100644 --- a/pkg/login/brute_force_login_protection.go +++ b/pkg/login/brute_force_login_protection.go @@ -34,9 +34,9 @@ var validateLoginAttempts = func(username string) error { return nil } -var saveInvalidLoginAttempt = func(query *m.LoginUserQuery) { +var saveInvalidLoginAttempt = func(query *m.LoginUserQuery) error { if setting.DisableBruteForceLoginProtection { - return + return nil } loginAttemptCommand := m.CreateLoginAttemptCommand{ @@ -44,5 +44,5 @@ var saveInvalidLoginAttempt = func(query *m.LoginUserQuery) { IpAddress: query.IpAddress, } - bus.Dispatch(&loginAttemptCommand) + return bus.Dispatch(&loginAttemptCommand) } diff --git a/pkg/login/brute_force_login_protection_test.go b/pkg/login/brute_force_login_protection_test.go index aca100760c7..38685a2aa2b 100644 --- a/pkg/login/brute_force_login_protection_test.go +++ b/pkg/login/brute_force_login_protection_test.go @@ -50,11 +50,12 @@ func TestLoginAttemptsValidation(t *testing.T) { return nil }) - saveInvalidLoginAttempt(&m.LoginUserQuery{ + err := saveInvalidLoginAttempt(&m.LoginUserQuery{ Username: "user", Password: "pwd", IpAddress: "192.168.1.1:56433", }) + So(err, ShouldBeNil) Convey("it should dispatch command", func() { So(createLoginAttemptCmd, ShouldNotBeNil) @@ -103,11 +104,12 @@ func TestLoginAttemptsValidation(t *testing.T) { return nil }) - saveInvalidLoginAttempt(&m.LoginUserQuery{ + err := saveInvalidLoginAttempt(&m.LoginUserQuery{ Username: "user", Password: "pwd", IpAddress: "192.168.1.1:56433", }) + So(err, ShouldBeNil) Convey("it should not dispatch command", func() { So(createLoginAttemptCmd, ShouldBeNil) diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index 3077b21432a..76bb7be2cbb 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -20,7 +20,7 @@ var isLDAPEnabled = multildap.IsEnabled var newLDAP = multildap.New // logger for the LDAP auth -var logger = log.New("login.ldap") +var ldapLogger = 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. @@ -40,7 +40,9 @@ var loginUsingLDAP = func(query *models.LoginUserQuery) (bool, error) { if err != nil { if err == ldap.ErrCouldNotFindUser { // Ignore the error since user might not be present anyway - DisableExternalUser(query.Username) + if err := DisableExternalUser(query.Username); err != nil { + ldapLogger.Debug("Failed to disable external user", "err", err) + } return true, ldap.ErrInvalidCredentials } @@ -75,7 +77,7 @@ func DisableExternalUser(username string) error { userInfo := userQuery.Result if !userInfo.IsDisabled { - logger.Debug( + ldapLogger.Debug( "Disabling external user", "user", userQuery.Result.Login, @@ -88,7 +90,7 @@ func DisableExternalUser(username string) error { } if err := bus.Dispatch(disableUserCmd); err != nil { - logger.Debug( + ldapLogger.Debug( "Error disabling external user", "user", userQuery.Result.Login,