LoginAttempts: Reset attempts on successfull password reset (#59215)

* LoginAttempt: Add function to reset attemtps for username

* PasswordReset: Reset attempts on successful password reset

pkg/api/password.go

@@ -60,7 +60,11 @@ func (hs *HTTPServer) ResetPassword(c *models.ReqContext) response.Response {
 	}
 	query := models.ValidateResetPasswordCodeQuery{Code: form.Code}
 
+	// For now the only way to know the username to clear login attempts for is
+	// to set it in the function provided to NotificationService
+	var username string
 	getUserByLogin := func(ctx context.Context, login string) (*user.User, error) {
+		username = login
 		userQuery := user.GetUserByLoginQuery{LoginOrEmail: login}
 		usr, err := hs.userService.GetByLogin(ctx, &userQuery)
 		return usr, err
@@ -94,5 +98,9 @@ func (hs *HTTPServer) ResetPassword(c *models.ReqContext) response.Response {
 		return response.Error(500, "Failed to change user password", err)
 	}
 
+	if err := hs.loginAttemptService.Reset(c.Req.Context(), username); err != nil {
+		c.Logger.Warn("could not reset login attempts", "err", err, "username", username)
+	}
+
 	return response.Success("User password changed")
 }

pkg/services/loginattempt/login_attempt.go

@@ -10,6 +10,8 @@ type Service interface {
 	// Validate checks if username has to many login attempts inside a window.
 	// Will return true if provided username do not have too many attempts.
 	Validate(ctx context.Context, username string) (bool, error)
+	// Reset resets all login attempts attached to username
+	Reset(ctx context.Context, username string) error
 }
 
 type LoginAttempt struct {

pkg/services/loginattempt/loginattemptimpl/login_attempt.go

@@ -59,6 +59,10 @@ func (s *Service) Add(ctx context.Context, username, IPAddress string) error {
 	})
 }
 
+func (s *Service) Reset(ctx context.Context, username string) error {
+	return s.store.DeleteLoginAttempts(ctx, DeleteLoginAttemptsCommand{username})
+}
+
 func (s *Service) Validate(ctx context.Context, username string) (bool, error) {
 	if s.cfg.DisableBruteForceLoginProtection {
 		return true, nil

pkg/services/loginattempt/loginattemptimpl/login_attempt_test.go

@@ -96,3 +96,7 @@ func (f fakeStore) CreateLoginAttempt(ctx context.Context, command CreateLoginAt
 func (f fakeStore) DeleteOldLoginAttempts(ctx context.Context, command DeleteOldLoginAttemptsCommand) (int64, error) {
 	return f.ExpectedDeletedRows, f.ExpectedErr
 }
+
+func (f fakeStore) DeleteLoginAttempts(ctx context.Context, cmd DeleteLoginAttemptsCommand) error {
+	return f.ExpectedErr
+}

pkg/services/loginattempt/loginattemptimpl/models.go

@@ -21,3 +21,7 @@ type GetUserLoginAttemptCountQuery struct {
 type DeleteOldLoginAttemptsCommand struct {
 	OlderThan time.Time
 }
+
+type DeleteLoginAttemptsCommand struct {
+	Username string
+}

pkg/services/loginattempt/loginattemptimpl/store.go

@@ -2,7 +2,6 @@ package loginattemptimpl
 
 import (
 	"context"
-	"strconv"
 	"time"
 
 	"github.com/grafana/grafana/pkg/infra/db"
@@ -17,6 +16,7 @@ type xormStore struct {
 type store interface {
 	CreateLoginAttempt(ctx context.Context, cmd CreateLoginAttemptCommand) error
 	DeleteOldLoginAttempts(ctx context.Context, cmd DeleteOldLoginAttemptsCommand) (int64, error)
+	DeleteLoginAttempts(ctx context.Context, cmd DeleteLoginAttemptsCommand) error
 	GetUserLoginAttemptCount(ctx context.Context, query GetUserLoginAttemptCountQuery) (int64, error)
 }
 
@@ -41,26 +41,7 @@ func (xs *xormStore) CreateLoginAttempt(ctx context.Context, cmd CreateLoginAtte
 func (xs *xormStore) DeleteOldLoginAttempts(ctx context.Context, cmd DeleteOldLoginAttemptsCommand) (int64, error) {
 	var deletedRows int64
 	err := xs.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
-		var maxId int64
-		sql := "SELECT max(id) as id FROM login_attempt WHERE created < ?"
-		result, err := sess.Query(sql, cmd.OlderThan.Unix())
-		if err != nil {
-			return err
-		}
-		if len(result) == 0 || result[0] == nil {
-			return nil
-		}
-
-		// TODO: why don't we know the type of ID?
-		maxId = toInt64(result[0]["id"])
-
-		if maxId == 0 {
-			return nil
-		}
-
-		sql = "DELETE FROM login_attempt WHERE id <= ?"
-
-		deleteResult, err := sess.Exec(sql, maxId)
+		deleteResult, err := sess.Exec("DELETE FROM login_attempt WHERE created < ?", cmd.OlderThan.Unix())
 		if err != nil {
 			return err
 		}
@@ -74,6 +55,13 @@ func (xs *xormStore) DeleteOldLoginAttempts(ctx context.Context, cmd DeleteOldLo
 	return deletedRows, err
 }
 
+func (xs *xormStore) DeleteLoginAttempts(ctx context.Context, cmd DeleteLoginAttemptsCommand) error {
+	return xs.db.WithDbSession(ctx, func(sess *db.Session) error {
+		_, err := sess.Exec("DELETE FROM login_attempt WHERE username = ?", cmd.Username)
+		return err
+	})
+}
+
 func (xs *xormStore) GetUserLoginAttemptCount(ctx context.Context, query GetUserLoginAttemptCountQuery) (int64, error) {
 	var total int64
 	err := xs.db.WithDbSession(ctx, func(dbSession *db.Session) error {
@@ -93,16 +81,3 @@ func (xs *xormStore) GetUserLoginAttemptCount(ctx context.Context, query GetUser
 
 	return total, err
 }
-
-func toInt64(i interface{}) int64 {
-	switch i := i.(type) {
-	case []byte:
-		n, _ := strconv.ParseInt(string(i), 10, 64)
-		return n
-	case int:
-		return int64(i)
-	case int64:
-		return i
-	}
-	return 0
-}

pkg/services/loginattempt/loginattempttest/fake.go

@@ -17,6 +17,10 @@ func (f FakeLoginAttemptService) Add(ctx context.Context, username, IPAddress st
 	return f.ExpectedErr
 }
 
+func (f FakeLoginAttemptService) Reset(ctx context.Context, username string) error {
+	return f.ExpectedErr
+}
+
 func (f FakeLoginAttemptService) Validate(ctx context.Context, username string) (bool, error) {
 	return f.ExpectedValid, f.ExpectedErr
 }

pkg/services/loginattempt/loginattempttest/mock.go

@@ -10,6 +10,7 @@ var _ loginattempt.Service = new(MockLoginAttemptService)
 
 type MockLoginAttemptService struct {
 	AddCalled      bool
+	ResetCalled    bool
 	ValidateCalled bool
 
 	ExpectedValid bool
@@ -21,6 +22,11 @@ func (f *MockLoginAttemptService) Add(ctx context.Context, username, IPAddress s
 	return f.ExpectedErr
 }
 
+func (f *MockLoginAttemptService) Reset(ctx context.Context, username string) error {
+	f.ResetCalled = true
+	return f.ExpectedErr
+}
+
 func (f *MockLoginAttemptService) Validate(ctx context.Context, username string) (bool, error) {
 	f.ValidateCalled = true
 	return f.ExpectedValid, f.ExpectedErr