pkg/login: Check errors (#19714)

* pkg/login: Check errors
* pkg/login: Introduce "login" logger

Co-Authored-By: Marcus Efraimsson <marcus.efraimsson@gmail.com>

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
 	}
 

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
 	}
 }
 

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)
 }

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)

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,