LoginAttempt: Move logic around login attempts into the service (#58962)

* LoginAttemps: Remove from sqlstore mock

* LoginAttemps: Move from models package to service package

* LoginAttemps: Implement functionallity from brute force login in service

* LoginAttemps: Call service

* LoginAttempts: Update name and remove internal functions

* LoginAttempts: Add tests

* LoginAttempt: Add service fake

* LoginAttempt: Register service as a background_services and remove job
from cleanup service

* LoginAttemps: Remove result from command struct

* LoginAttempt: No longer pass pointers

pkg/cmd/grafana-cli/runner/wire.go

@@ -82,6 +82,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/login/authinfoservice"
 	authinfodatabase "github.com/grafana/grafana/pkg/services/login/authinfoservice/database"
 	"github.com/grafana/grafana/pkg/services/login/loginservice"
+	"github.com/grafana/grafana/pkg/services/loginattempt"
 	"github.com/grafana/grafana/pkg/services/loginattempt/loginattemptimpl"
 	"github.com/grafana/grafana/pkg/services/ngalert"
 	ngmetrics "github.com/grafana/grafana/pkg/services/ngalert/metrics"
@@ -227,6 +228,7 @@ var wireSet = wire.NewSet(
 	loginpkg.ProvideService,
 	wire.Bind(new(loginpkg.Authenticator), new(*loginpkg.AuthenticatorService)),
 	loginattemptimpl.ProvideService,
+	wire.Bind(new(loginattempt.Service), new(*loginattemptimpl.Service)),
 	datasourceproxy.ProvideService,
 	search.ProvideService,
 	searchV2.ProvideService,

pkg/login/auth.go

@@ -50,16 +50,19 @@ func ProvideService(store db.DB, loginService login.Service, loginAttemptService
 
 // AuthenticateUser authenticates the user via username & password
 func (a *AuthenticatorService) AuthenticateUser(ctx context.Context, query *models.LoginUserQuery) error {
-	if err := validateLoginAttempts(ctx, query, a.loginAttemptService); err != nil {
+	ok, err := a.loginAttemptService.Validate(ctx, query.Username)
+	if err != nil {
 		return err
 	}
+	if !ok {
+		return ErrTooManyLoginAttempts
+	}
 
 	if err := validatePasswordSet(query.Password); err != nil {
 		return err
 	}
 
 	isGrafanaLoginEnabled := !query.Cfg.DisableLogin
-	var err error
 
 	if isGrafanaLoginEnabled {
 		err = loginUsingGrafanaDB(ctx, query, a.userService)
@@ -84,7 +87,7 @@ func (a *AuthenticatorService) AuthenticateUser(ctx context.Context, query *mode
 	}
 
 	if errors.Is(err, ErrInvalidCredentials) || errors.Is(err, ldap.ErrInvalidCredentials) {
-		if err := saveInvalidLoginAttempt(ctx, query, a.loginAttemptService); err != nil {
+		if err := a.loginAttemptService.Add(ctx, query.Username, query.IpAddress); err != nil {
 			loginLogger.Error("Failed to save invalid login attempt", "err", err)
 		}
 

pkg/login/auth_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/ldap"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/login/logintest"
-	"github.com/grafana/grafana/pkg/services/loginattempt"
+	"github.com/grafana/grafana/pkg/services/loginattempt/loginattempttest"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/stretchr/testify/assert"
@@ -18,16 +18,15 @@ import (
 
 func TestAuthenticateUser(t *testing.T) {
 	authScenario(t, "When a user authenticates without setting a password", func(sc *authScenarioContext) {
-		mockLoginAttemptValidation(nil, sc)
 		mockLoginUsingGrafanaDB(nil, sc)
 		mockLoginUsingLDAP(false, nil, sc)
 
-		loginQuery := models.LoginUserQuery{
+		loginAttemptService := &loginattempttest.FakeLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
+		err := a.AuthenticateUser(context.Background(), &models.LoginUserQuery{
 			Username: "user",
 			Password: "",
-		}
-		a := AuthenticatorService{loginAttemptService: nil, loginService: &logintest.LoginServiceFake{}}
-		err := a.AuthenticateUser(context.Background(), &loginQuery)
+		})
 
 		require.EqualError(t, err, ErrPasswordEmpty.Error())
 		assert.False(t, sc.grafanaLoginWasCalled)
@@ -36,164 +35,154 @@ func TestAuthenticateUser(t *testing.T) {
 	})
 
 	authScenario(t, "When user authenticates with no auth provider enabled", func(sc *authScenarioContext) {
-		mockLoginAttemptValidation(nil, sc)
 		sc.loginUserQuery.Cfg.DisableLogin = true
 
-		a := AuthenticatorService{loginAttemptService: nil, loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.EqualError(t, err, ErrNoAuthProvider.Error())
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.False(t, sc.grafanaLoginWasCalled)
 		assert.False(t, sc.ldapLoginWasCalled)
-		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
 		assert.Equal(t, "", sc.loginUserQuery.AuthModule)
+		assert.False(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 
 	authScenario(t, "When a user authenticates having too many login attempts", func(sc *authScenarioContext) {
-		mockLoginAttemptValidation(ErrTooManyLoginAttempts, sc)
 		mockLoginUsingGrafanaDB(nil, sc)
 		mockLoginUsingLDAP(true, nil, sc)
-		mockSaveInvalidLoginAttempt(sc)
 
-		a := AuthenticatorService{loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: false}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.EqualError(t, err, ErrTooManyLoginAttempts.Error())
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.False(t, sc.grafanaLoginWasCalled)
 		assert.False(t, sc.ldapLoginWasCalled)
-		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
 		assert.Empty(t, sc.loginUserQuery.AuthModule)
+		assert.False(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 
 	authScenario(t, "When grafana user authenticate with valid credentials", func(sc *authScenarioContext) {
-		mockLoginAttemptValidation(nil, sc)
 		mockLoginUsingGrafanaDB(nil, sc)
 		mockLoginUsingLDAP(true, ErrInvalidCredentials, sc)
-		mockSaveInvalidLoginAttempt(sc)
 
-		a := AuthenticatorService{loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.NoError(t, err)
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.False(t, sc.ldapLoginWasCalled)
-		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
 		assert.Equal(t, "grafana", sc.loginUserQuery.AuthModule)
+		assert.False(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 
 	authScenario(t, "When grafana user authenticate and unexpected error occurs", func(sc *authScenarioContext) {
 		customErr := errors.New("custom")
-		mockLoginAttemptValidation(nil, sc)
 		mockLoginUsingGrafanaDB(customErr, sc)
 		mockLoginUsingLDAP(true, ErrInvalidCredentials, sc)
-		mockSaveInvalidLoginAttempt(sc)
 
-		a := AuthenticatorService{loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.EqualError(t, err, customErr.Error())
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.False(t, sc.ldapLoginWasCalled)
-		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
 		assert.Equal(t, "grafana", sc.loginUserQuery.AuthModule)
+		assert.False(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 
 	authScenario(t, "When a non-existing grafana user authenticate and ldap disabled", func(sc *authScenarioContext) {
-		mockLoginAttemptValidation(nil, sc)
 		mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc)
 		mockLoginUsingLDAP(false, nil, sc)
-		mockSaveInvalidLoginAttempt(sc)
 
-		a := AuthenticatorService{loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.EqualError(t, err, user.ErrUserNotFound.Error())
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.True(t, sc.ldapLoginWasCalled)
-		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
 		assert.Empty(t, sc.loginUserQuery.AuthModule)
+		assert.False(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 
 	authScenario(t, "When a non-existing grafana user authenticate and invalid ldap credentials", func(sc *authScenarioContext) {
-		mockLoginAttemptValidation(nil, sc)
 		mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc)
 		mockLoginUsingLDAP(true, ldap.ErrInvalidCredentials, sc)
-		mockSaveInvalidLoginAttempt(sc)
 
-		a := AuthenticatorService{loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.EqualError(t, err, ErrInvalidCredentials.Error())
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.True(t, sc.ldapLoginWasCalled)
-		assert.True(t, sc.saveInvalidLoginAttemptWasCalled)
 		assert.Equal(t, login.LDAPAuthModule, sc.loginUserQuery.AuthModule)
+		assert.True(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 
 	authScenario(t, "When a non-existing grafana user authenticate and valid ldap credentials", func(sc *authScenarioContext) {
-		mockLoginAttemptValidation(nil, sc)
 		mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc)
 		mockLoginUsingLDAP(true, nil, sc)
-		mockSaveInvalidLoginAttempt(sc)
 
-		a := AuthenticatorService{loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.NoError(t, err)
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.True(t, sc.ldapLoginWasCalled)
-		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
 		assert.Equal(t, login.LDAPAuthModule, sc.loginUserQuery.AuthModule)
+		assert.False(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 
 	authScenario(t, "When a non-existing grafana user authenticate and ldap returns unexpected error", func(sc *authScenarioContext) {
 		customErr := errors.New("custom")
-		mockLoginAttemptValidation(nil, sc)
 		mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc)
 		mockLoginUsingLDAP(true, customErr, sc)
-		mockSaveInvalidLoginAttempt(sc)
 
-		a := AuthenticatorService{loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.EqualError(t, err, customErr.Error())
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.True(t, sc.ldapLoginWasCalled)
-		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
 		assert.Equal(t, login.LDAPAuthModule, sc.loginUserQuery.AuthModule)
+		assert.False(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 
 	authScenario(t, "When grafana user authenticate with invalid credentials and invalid ldap credentials", func(sc *authScenarioContext) {
-		mockLoginAttemptValidation(nil, sc)
 		mockLoginUsingGrafanaDB(ErrInvalidCredentials, sc)
 		mockLoginUsingLDAP(true, ldap.ErrInvalidCredentials, sc)
-		mockSaveInvalidLoginAttempt(sc)
 
-		a := AuthenticatorService{loginService: &logintest.LoginServiceFake{}}
+		loginAttemptService := &loginattempttest.MockLoginAttemptService{ExpectedValid: true}
+		a := AuthenticatorService{loginAttemptService: loginAttemptService, loginService: &logintest.LoginServiceFake{}}
 		err := a.AuthenticateUser(context.Background(), sc.loginUserQuery)
 
 		require.EqualError(t, err, ErrInvalidCredentials.Error())
-		assert.True(t, sc.loginAttemptValidationWasCalled)
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.True(t, sc.ldapLoginWasCalled)
-		assert.True(t, sc.saveInvalidLoginAttemptWasCalled)
+		assert.True(t, loginAttemptService.AddCalled)
+		assert.True(t, loginAttemptService.ValidateCalled)
 	})
 }
 
 type authScenarioContext struct {
-	loginUserQuery                   *models.LoginUserQuery
-	grafanaLoginWasCalled            bool
-	ldapLoginWasCalled               bool
-	loginAttemptValidationWasCalled  bool
-	saveInvalidLoginAttemptWasCalled bool
+	loginUserQuery        *models.LoginUserQuery
+	grafanaLoginWasCalled bool
+	ldapLoginWasCalled    bool
 }
 
 type authScenarioFunc func(sc *authScenarioContext)
@@ -212,28 +201,12 @@ func mockLoginUsingLDAP(enabled bool, err error, sc *authScenarioContext) {
 	}
 }
 
-func mockLoginAttemptValidation(err error, sc *authScenarioContext) {
-	validateLoginAttempts = func(context.Context, *models.LoginUserQuery, loginattempt.Service) error {
-		sc.loginAttemptValidationWasCalled = true
-		return err
-	}
-}
-
-func mockSaveInvalidLoginAttempt(sc *authScenarioContext) {
-	saveInvalidLoginAttempt = func(ctx context.Context, query *models.LoginUserQuery, _ loginattempt.Service) error {
-		sc.saveInvalidLoginAttemptWasCalled = true
-		return nil
-	}
-}
-
 func authScenario(t *testing.T, desc string, fn authScenarioFunc) {
 	t.Helper()
 
 	t.Run(desc, func(t *testing.T) {
 		origLoginUsingGrafanaDB := loginUsingGrafanaDB
 		origLoginUsingLDAP := loginUsingLDAP
-		origValidateLoginAttempts := validateLoginAttempts
-		origSaveInvalidLoginAttempt := saveInvalidLoginAttempt
 		cfg := setting.Cfg{DisableLogin: false}
 		sc := &authScenarioContext{
 			loginUserQuery: &models.LoginUserQuery{
@@ -247,8 +220,6 @@ func authScenario(t *testing.T, desc string, fn authScenarioFunc) {
 		t.Cleanup(func() {
 			loginUsingGrafanaDB = origLoginUsingGrafanaDB
 			loginUsingLDAP = origLoginUsingLDAP
-			validateLoginAttempts = origValidateLoginAttempts
-			saveInvalidLoginAttempt = origSaveInvalidLoginAttempt
 		})
 
 		fn(sc)

pkg/login/brute_force_login_protection.go

@@ -1,48 +0,0 @@
-package login
-
-import (
-	"context"
-	"time"
-
-	"github.com/grafana/grafana/pkg/models"
-	"github.com/grafana/grafana/pkg/services/loginattempt"
-)
-
-var (
-	maxInvalidLoginAttempts int64 = 5
-	loginAttemptsWindow           = time.Minute * 5
-)
-
-var validateLoginAttempts = func(ctx context.Context, query *models.LoginUserQuery, loginAttemptService loginattempt.Service) error {
-	if query.Cfg.DisableBruteForceLoginProtection {
-		return nil
-	}
-
-	loginAttemptCountQuery := models.GetUserLoginAttemptCountQuery{
-		Username: query.Username,
-		Since:    time.Now().Add(-loginAttemptsWindow),
-	}
-
-	if err := loginAttemptService.GetUserLoginAttemptCount(ctx, &loginAttemptCountQuery); err != nil {
-		return err
-	}
-
-	if loginAttemptCountQuery.Result >= maxInvalidLoginAttempts {
-		return ErrTooManyLoginAttempts
-	}
-
-	return nil
-}
-
-var saveInvalidLoginAttempt = func(ctx context.Context, query *models.LoginUserQuery, loginAttemptService loginattempt.Service) error {
-	if query.Cfg.DisableBruteForceLoginProtection {
-		return nil
-	}
-
-	loginAttemptCommand := models.CreateLoginAttemptCommand{
-		Username:  query.Username,
-		IpAddress: query.IpAddress,
-	}
-
-	return loginAttemptService.CreateLoginAttempt(ctx, &loginAttemptCommand)
-}

pkg/login/brute_force_login_protection_test.go

@@ -1,116 +0,0 @@
-package login
-
-import (
-	"context"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"github.com/grafana/grafana/pkg/models"
-	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
-	"github.com/grafana/grafana/pkg/setting"
-)
-
-func TestValidateLoginAttempts(t *testing.T) {
-	testCases := []struct {
-		name          string
-		loginAttempts int64
-		cfg           *setting.Cfg
-		expected      error
-	}{
-		{
-			name:          "When brute force protection enabled and user login attempt count is less than max",
-			loginAttempts: maxInvalidLoginAttempts - 1,
-			cfg:           cfgWithBruteForceLoginProtectionEnabled(t),
-			expected:      nil,
-		},
-		{
-			name:          "When brute force protection enabled and user login attempt count equals max",
-			loginAttempts: maxInvalidLoginAttempts,
-			cfg:           cfgWithBruteForceLoginProtectionEnabled(t),
-			expected:      ErrTooManyLoginAttempts,
-		},
-		{
-			name:          "When brute force protection enabled and user login attempt count is greater than max",
-			loginAttempts: maxInvalidLoginAttempts + 1,
-			cfg:           cfgWithBruteForceLoginProtectionEnabled(t),
-			expected:      ErrTooManyLoginAttempts,
-		},
-
-		{
-			name:          "When brute force protection disabled and user login attempt count is less than max",
-			loginAttempts: maxInvalidLoginAttempts - 1,
-			cfg:           cfgWithBruteForceLoginProtectionDisabled(t),
-			expected:      nil,
-		},
-		{
-			name:          "When brute force protection disabled and user login attempt count equals max",
-			loginAttempts: maxInvalidLoginAttempts,
-			cfg:           cfgWithBruteForceLoginProtectionDisabled(t),
-			expected:      nil,
-		},
-		{
-			name:          "When brute force protection disabled and user login attempt count is greater than max",
-			loginAttempts: maxInvalidLoginAttempts + 1,
-			cfg:           cfgWithBruteForceLoginProtectionDisabled(t),
-			expected:      nil,
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			store := mockstore.NewSQLStoreMock()
-			store.ExpectedLoginAttempts = tc.loginAttempts
-
-			query := &models.LoginUserQuery{Username: "user", Cfg: tc.cfg}
-
-			err := validateLoginAttempts(context.Background(), query, store)
-			require.Equal(t, tc.expected, err)
-		})
-	}
-}
-
-func TestSaveInvalidLoginAttempt(t *testing.T) {
-	t.Run("When brute force protection enabled", func(t *testing.T) {
-		store := mockstore.NewSQLStoreMock()
-		err := saveInvalidLoginAttempt(context.Background(), &models.LoginUserQuery{
-			Username:  "user",
-			Password:  "pwd",
-			IpAddress: "192.168.1.1:56433",
-			Cfg:       cfgWithBruteForceLoginProtectionEnabled(t),
-		}, store)
-		require.NoError(t, err)
-
-		require.NotNil(t, store.LastLoginAttemptCommand)
-		assert.Equal(t, "user", store.LastLoginAttemptCommand.Username)
-		assert.Equal(t, "192.168.1.1:56433", store.LastLoginAttemptCommand.IpAddress)
-	})
-
-	t.Run("When brute force protection disabled", func(t *testing.T) {
-		store := mockstore.NewSQLStoreMock()
-		err := saveInvalidLoginAttempt(context.Background(), &models.LoginUserQuery{
-			Username:  "user",
-			Password:  "pwd",
-			IpAddress: "192.168.1.1:56433",
-			Cfg:       cfgWithBruteForceLoginProtectionDisabled(t),
-		}, store)
-		require.NoError(t, err)
-
-		require.Nil(t, store.LastLoginAttemptCommand)
-	})
-}
-
-func cfgWithBruteForceLoginProtectionDisabled(t *testing.T) *setting.Cfg {
-	t.Helper()
-	cfg := setting.NewCfg()
-	cfg.DisableBruteForceLoginProtection = true
-	return cfg
-}
-
-func cfgWithBruteForceLoginProtectionEnabled(t *testing.T) *setting.Cfg {
-	t.Helper()
-	cfg := setting.NewCfg()
-	require.False(t, cfg.DisableBruteForceLoginProtection)
-	return cfg
-}

pkg/models/login_attempt.go

@@ -1,36 +0,0 @@
-package models
-
-import (
-	"time"
-)
-
-type LoginAttempt struct {
-	Id        int64
-	Username  string
-	IpAddress string
-	Created   int64
-}
-
-// ---------------------
-// COMMANDS
-
-type CreateLoginAttemptCommand struct {
-	Username  string
-	IpAddress string
-
-	Result LoginAttempt
-}
-
-type DeleteOldLoginAttemptsCommand struct {
-	OlderThan   time.Time
-	DeletedRows int64
-}
-
-// ---------------------
-// QUERIES
-
-type GetUserLoginAttemptCountQuery struct {
-	Username string
-	Since    time.Time
-	Result   int64
-}

pkg/server/backgroundsvcs/background_services.go

@@ -18,6 +18,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/live"
 	"github.com/grafana/grafana/pkg/services/live/pushhttp"
 	"github.com/grafana/grafana/pkg/services/login/authinfoservice"
+	"github.com/grafana/grafana/pkg/services/loginattempt/loginattemptimpl"
 	"github.com/grafana/grafana/pkg/services/ngalert"
 	"github.com/grafana/grafana/pkg/services/notifications"
 	plugindashboardsservice "github.com/grafana/grafana/pkg/services/plugindashboards/service"
@@ -46,7 +47,7 @@ func ProvideBackgroundServiceRegistry(
 	thumbnailsService thumbs.Service, StorageService store.StorageService, searchService searchV2.SearchService, entityEventsService store.EntityEventsService,
 	saService *samanager.ServiceAccountsService, authInfoService *authinfoservice.Implementation,
 	grpcServerProvider grpcserver.Provider,
-	secretMigrationProvider secretsMigrations.SecretMigrationProvider,
+	secretMigrationProvider secretsMigrations.SecretMigrationProvider, loginAttemptService *loginattemptimpl.Service,
 	// Need to make sure these are initialized, is there a better place to put them?
 	_ dashboardsnapshots.Service, _ *alerting.AlertNotificationService,
 	_ serviceaccounts.Service, _ *guardian.Provider,
@@ -81,6 +82,7 @@ func ProvideBackgroundServiceRegistry(
 		authInfoService,
 		processManager,
 		secretMigrationProvider,
+		loginAttemptService,
 	)
 }
 

pkg/server/wire.go

@@ -84,6 +84,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/login/authinfoservice"
 	authinfodatabase "github.com/grafana/grafana/pkg/services/login/authinfoservice/database"
 	"github.com/grafana/grafana/pkg/services/login/loginservice"
+	"github.com/grafana/grafana/pkg/services/loginattempt"
 	"github.com/grafana/grafana/pkg/services/loginattempt/loginattemptimpl"
 	"github.com/grafana/grafana/pkg/services/navtree/navtreeimpl"
 	"github.com/grafana/grafana/pkg/services/ngalert"
@@ -363,6 +364,7 @@ var wireBasicSet = wire.NewSet(
 	teamimpl.ProvideService,
 	tempuserimpl.ProvideService,
 	loginattemptimpl.ProvideService,
+	wire.Bind(new(loginattempt.Service), new(*loginattemptimpl.Service)),
 	secretsMigrations.ProvideDataSourceMigrationService,
 	secretsMigrations.ProvideMigrateToPluginService,
 	secretsMigrations.ProvideMigrateFromPluginService,

pkg/services/cleanup/cleanup.go

@@ -20,7 +20,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/annotations"
 	"github.com/grafana/grafana/pkg/services/dashboardsnapshots"
 	dashver "github.com/grafana/grafana/pkg/services/dashboardversion"
-	"github.com/grafana/grafana/pkg/services/loginattempt"
 	"github.com/grafana/grafana/pkg/services/ngalert/image"
 	"github.com/grafana/grafana/pkg/services/queryhistory"
 	"github.com/grafana/grafana/pkg/services/shorturls"
@@ -31,7 +30,7 @@ import (
 func ProvideService(cfg *setting.Cfg, serverLockService *serverlock.ServerLockService,
 	shortURLService shorturls.Service, sqlstore db.DB, queryHistoryService queryhistory.Service,
 	dashboardVersionService dashver.Service, dashSnapSvc dashboardsnapshots.Service, deleteExpiredImageService *image.DeleteExpiredService,
-	loginAttemptService loginattempt.Service, tempUserService tempuser.Service, tracer tracing.Tracer, annotationCleaner annotations.Cleaner) *CleanUpService {
+	tempUserService tempuser.Service, tracer tracing.Tracer, annotationCleaner annotations.Cleaner) *CleanUpService {
 	s := &CleanUpService{
 		Cfg:                       cfg,
 		ServerLockService:         serverLockService,
@@ -42,7 +41,6 @@ func ProvideService(cfg *setting.Cfg, serverLockService *serverlock.ServerLockSe
 		dashboardVersionService:   dashboardVersionService,
 		dashboardSnapshotService:  dashSnapSvc,
 		deleteExpiredImageService: deleteExpiredImageService,
-		loginAttemptService:       loginAttemptService,
 		tempUserService:           tempUserService,
 		tracer:                    tracer,
 		annotationCleaner:         annotationCleaner,
@@ -61,7 +59,6 @@ type CleanUpService struct {
 	dashboardVersionService   dashver.Service
 	dashboardSnapshotService  dashboardsnapshots.Service
 	deleteExpiredImageService *image.DeleteExpiredService
-	loginAttemptService       loginattempt.Service
 	tempUserService           tempuser.Service
 	annotationCleaner         annotations.Cleaner
 }
@@ -106,7 +103,6 @@ func (srv *CleanUpService) clean(ctx context.Context) {
 		{"expire old user invites", srv.expireOldUserInvites},
 		{"delete stale short URLs", srv.deleteStaleShortURLs},
 		{"delete stale query history", srv.deleteStaleQueryHistory},
-		{"delete old login attempts", srv.deleteOldLoginAttempts},
 	}
 
 	logger := srv.log.FromContext(ctx)
@@ -227,33 +223,6 @@ func (srv *CleanUpService) deleteExpiredImages(ctx context.Context) {
 	}
 }
 
-func (srv *CleanUpService) deleteOldLoginAttempts(ctx context.Context) {
-	logger := srv.log.FromContext(ctx)
-	err := srv.ServerLockService.LockAndExecute(ctx, "delete old login attempts",
-		time.Minute*10, func(context.Context) {
-			srv.deleteOldLoginAttemptsWithoutLock(ctx)
-		})
-	if err != nil {
-		logger.Error("failed to lock and execute cleanup of old login attempts", "error", err)
-	}
-}
-
-func (srv *CleanUpService) deleteOldLoginAttemptsWithoutLock(ctx context.Context) {
-	logger := srv.log.FromContext(ctx)
-	if srv.Cfg.DisableBruteForceLoginProtection {
-		return
-	}
-
-	cmd := models.DeleteOldLoginAttemptsCommand{
-		OlderThan: time.Now().Add(time.Minute * -10),
-	}
-	if err := srv.loginAttemptService.DeleteOldLoginAttempts(ctx, &cmd); err != nil {
-		logger.Error("Problem deleting expired login attempts", "error", err.Error())
-	} else {
-		logger.Debug("Deleted expired login attempts", "rows affected", cmd.DeletedRows)
-	}
-}
-
 func (srv *CleanUpService) expireOldUserInvites(ctx context.Context) {
 	logger := srv.log.FromContext(ctx)
 	maxInviteLifetime := srv.Cfg.UserInviteMaxLifetime

pkg/services/loginattempt/login_attempt.go

@@ -2,12 +2,19 @@ package loginattempt
 
 import (
 	"context"
-
-	"github.com/grafana/grafana/pkg/models"
 )
 
 type Service interface {
-	CreateLoginAttempt(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error
-	DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error
-	GetUserLoginAttemptCount(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error
+	// Add adds a new login attempt record for provided username
+	Add(ctx context.Context, username, IPAddress string) error
+	// 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)
+}
+
+type LoginAttempt struct {
+	Id        int64
+	Username  string
+	IpAddress string
+	Created   int64
 }

pkg/services/loginattempt/loginattemptimpl/login_attempt.go

@@ -5,40 +5,95 @@ import (
 	"time"
 
 	"github.com/grafana/grafana/pkg/infra/db"
-	"github.com/grafana/grafana/pkg/models"
-	"github.com/grafana/grafana/pkg/services/loginattempt"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/infra/serverlock"
+	"github.com/grafana/grafana/pkg/setting"
 )
 
-type Service struct {
-	store store
-}
+const (
+	maxInvalidLoginAttempts int64 = 5
+	loginAttemptsWindow           = time.Minute * 5
+)
 
-func ProvideService(db db.DB) loginattempt.Service {
+func ProvideService(db db.DB, cfg *setting.Cfg, lock *serverlock.ServerLockService) *Service {
 	return &Service{
-		store: &xormStore{db: db, now: time.Now},
+		&xormStore{db: db, now: time.Now},
+		cfg,
+		lock,
+		log.New("login_attempt"),
 	}
 }
 
-func (s *Service) CreateLoginAttempt(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error {
-	err := s.store.CreateLoginAttempt(ctx, cmd)
-	if err != nil {
-		return err
-	}
-	return nil
+type Service struct {
+	store  store
+	cfg    *setting.Cfg
+	lock   *serverlock.ServerLockService
+	logger log.Logger
 }
 
-func (s *Service) DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error {
-	err := s.store.DeleteOldLoginAttempts(ctx, cmd)
-	if err != nil {
-		return err
+func (s *Service) Run(ctx context.Context) error {
+	// no need to run clean up job if it is disabled
+	if s.cfg.DisableBruteForceLoginProtection {
+		return nil
+	}
+
+	ticker := time.NewTicker(time.Minute * 10)
+	for {
+		select {
+		case <-ticker.C:
+			s.cleanup(ctx)
+		case <-ctx.Done():
+			return ctx.Err()
+		}
 	}
-	return nil
 }
 
-func (s *Service) GetUserLoginAttemptCount(ctx context.Context, cmd *models.GetUserLoginAttemptCountQuery) error {
-	err := s.store.GetUserLoginAttemptCount(ctx, cmd)
-	if err != nil {
-		return err
+func (s *Service) Add(ctx context.Context, username, IPAddress string) error {
+	if s.cfg.DisableBruteForceLoginProtection {
+		return nil
+	}
+
+	return s.store.CreateLoginAttempt(ctx, CreateLoginAttemptCommand{
+		Username:  username,
+		IpAddress: IPAddress,
+	})
+}
+
+func (s *Service) Validate(ctx context.Context, username string) (bool, error) {
+	if s.cfg.DisableBruteForceLoginProtection {
+		return true, nil
+	}
+
+	loginAttemptCountQuery := GetUserLoginAttemptCountQuery{
+		Username: username,
+		Since:    time.Now().Add(-loginAttemptsWindow),
+	}
+
+	count, err := s.store.GetUserLoginAttemptCount(ctx, loginAttemptCountQuery)
+	if err != nil {
+		return false, err
+	}
+
+	if count >= maxInvalidLoginAttempts {
+		return false, nil
+	}
+
+	return true, nil
+}
+
+func (s *Service) cleanup(ctx context.Context) {
+	err := s.lock.LockAndExecute(ctx, "delete old login attempts", time.Minute*10, func(context.Context) {
+		cmd := DeleteOldLoginAttemptsCommand{
+			OlderThan: time.Now().Add(time.Minute * -10),
+		}
+		if deletedLogs, err := s.store.DeleteOldLoginAttempts(ctx, cmd); err != nil {
+			s.logger.Error("Problem deleting expired login attempts", "error", err.Error())
+		} else {
+			s.logger.Debug("Deleted expired login attempts", "rows affected", deletedLogs)
+		}
+	})
+
+	if err != nil {
+		s.logger.Error("failed to lock and execute cleanup of old login attempts", "error", err)
 	}
-	return nil
 }

pkg/services/loginattempt/loginattemptimpl/login_attempt_test.go

@@ -0,0 +1,98 @@
+package loginattemptimpl
+
+import (
+	"context"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/setting"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestService_Validate(t *testing.T) {
+	testCases := []struct {
+		name          string
+		loginAttempts int64
+		disabled      bool
+		expected      bool
+		expectedErr   error
+	}{
+		{
+			name:          "When brute force protection enabled and user login attempt count is less than max",
+			loginAttempts: maxInvalidLoginAttempts - 1,
+			expected:      true,
+			expectedErr:   nil,
+		},
+		{
+			name:          "When brute force protection enabled and user login attempt count equals max",
+			loginAttempts: maxInvalidLoginAttempts,
+			expected:      false,
+			expectedErr:   nil,
+		},
+		{
+			name:          "When brute force protection enabled and user login attempt count is greater than max",
+			loginAttempts: maxInvalidLoginAttempts + 1,
+			expected:      false,
+			expectedErr:   nil,
+		},
+
+		{
+			name:          "When brute force protection disabled and user login attempt count is less than max",
+			loginAttempts: maxInvalidLoginAttempts - 1,
+			disabled:      true,
+			expected:      true,
+			expectedErr:   nil,
+		},
+		{
+			name:          "When brute force protection disabled and user login attempt count equals max",
+			loginAttempts: maxInvalidLoginAttempts,
+			disabled:      true,
+			expected:      true,
+			expectedErr:   nil,
+		},
+		{
+			name:          "When brute force protection disabled and user login attempt count is greater than max",
+			loginAttempts: maxInvalidLoginAttempts + 1,
+			disabled:      true,
+			expected:      true,
+			expectedErr:   nil,
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := setting.NewCfg()
+			cfg.DisableBruteForceLoginProtection = tt.disabled
+			service := &Service{
+				store: fakeStore{
+					ExpectedCount: tt.loginAttempts,
+					ExpectedErr:   tt.expectedErr,
+				},
+				cfg: cfg,
+			}
+
+			ok, err := service.Validate(context.Background(), "test")
+			assert.Equal(t, tt.expected, ok)
+			assert.Equal(t, tt.expectedErr, err)
+		})
+	}
+}
+
+var _ store = new(fakeStore)
+
+type fakeStore struct {
+	ExpectedErr         error
+	ExpectedCount       int64
+	ExpectedDeletedRows int64
+}
+
+func (f fakeStore) GetUserLoginAttemptCount(ctx context.Context, query GetUserLoginAttemptCountQuery) (int64, error) {
+	return f.ExpectedCount, f.ExpectedErr
+}
+
+func (f fakeStore) CreateLoginAttempt(ctx context.Context, command CreateLoginAttemptCommand) error {
+	return f.ExpectedErr
+}
+
+func (f fakeStore) DeleteOldLoginAttempts(ctx context.Context, command DeleteOldLoginAttemptsCommand) (int64, error) {
+	return f.ExpectedDeletedRows, f.ExpectedErr
+}

pkg/services/loginattempt/loginattemptimpl/models.go

@@ -0,0 +1,23 @@
+package loginattemptimpl
+
+import (
+	"time"
+
+	"github.com/grafana/grafana/pkg/services/loginattempt"
+)
+
+type CreateLoginAttemptCommand struct {
+	Username  string
+	IpAddress string
+
+	Result loginattempt.LoginAttempt
+}
+
+type GetUserLoginAttemptCountQuery struct {
+	Username string
+	Since    time.Time
+}
+
+type DeleteOldLoginAttemptsCommand struct {
+	OlderThan time.Time
+}

pkg/services/loginattempt/loginattemptimpl/store.go

@@ -6,7 +6,7 @@ import (
 	"time"
 
 	"github.com/grafana/grafana/pkg/infra/db"
-	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/loginattempt"
 )
 
 type xormStore struct {
@@ -15,14 +15,14 @@ type xormStore struct {
 }
 
 type store interface {
-	CreateLoginAttempt(context.Context, *models.CreateLoginAttemptCommand) error
-	DeleteOldLoginAttempts(context.Context, *models.DeleteOldLoginAttemptsCommand) error
-	GetUserLoginAttemptCount(context.Context, *models.GetUserLoginAttemptCountQuery) error
+	CreateLoginAttempt(ctx context.Context, cmd CreateLoginAttemptCommand) error
+	DeleteOldLoginAttempts(ctx context.Context, cmd DeleteOldLoginAttemptsCommand) (int64, error)
+	GetUserLoginAttemptCount(ctx context.Context, query GetUserLoginAttemptCountQuery) (int64, error)
 }
 
-func (xs *xormStore) CreateLoginAttempt(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error {
+func (xs *xormStore) CreateLoginAttempt(ctx context.Context, cmd CreateLoginAttemptCommand) error {
 	return xs.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
-		loginAttempt := models.LoginAttempt{
+		loginAttempt := loginattempt.LoginAttempt{
 			Username:  cmd.Username,
 			IpAddress: cmd.IpAddress,
 			Created:   xs.now().Unix(),
@@ -38,8 +38,9 @@ func (xs *xormStore) CreateLoginAttempt(ctx context.Context, cmd *models.CreateL
 	})
 }
 
-func (xs *xormStore) DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error {
-	return xs.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
+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())
@@ -59,31 +60,38 @@ func (xs *xormStore) DeleteOldLoginAttempts(ctx context.Context, cmd *models.Del
 
 		sql = "DELETE FROM login_attempt WHERE id <= ?"
 
-		if result, err := sess.Exec(sql, maxId); err != nil {
-			return err
-		} else if cmd.DeletedRows, err = result.RowsAffected(); err != nil {
-			return err
-		}
-
-		return nil
-	})
-}
-
-func (xs *xormStore) GetUserLoginAttemptCount(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error {
-	return xs.db.WithDbSession(ctx, func(dbSession *db.Session) error {
-		loginAttempt := new(models.LoginAttempt)
-		total, err := dbSession.
-			Where("username = ?", query.Username).
-			And("created >= ?", query.Since.Unix()).
-			Count(loginAttempt)
-
+		deleteResult, err := sess.Exec(sql, maxId)
 		if err != nil {
 			return err
 		}
 
-		query.Result = total
+		deletedRows, err = deleteResult.RowsAffected()
+		if err != nil {
+			return err
+		}
 		return nil
 	})
+	return deletedRows, 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 {
+		var queryErr error
+		loginAttempt := new(loginattempt.LoginAttempt)
+		total, queryErr = dbSession.
+			Where("username = ?", query.Username).
+			And("created >= ?", query.Since.Unix()).
+			Count(loginAttempt)
+
+		if queryErr != nil {
+			return queryErr
+		}
+
+		return nil
+	})
+
+	return total, err
 }
 
 func toInt64(i interface{}) int64 {

pkg/services/loginattempt/loginattemptimpl/store_test.go

@@ -8,15 +8,12 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/infra/db"
-	"github.com/grafana/grafana/pkg/models"
-	"github.com/grafana/grafana/pkg/services/loginattempt"
 )
 
 func TestIntegrationLoginAttemptsQuery(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}
-	var loginAttemptService loginattempt.Service
 	user := "user"
 
 	beginningOfTime := time.Date(2017, 10, 22, 8, 0, 0, 0, time.Local)
@@ -25,58 +22,60 @@ func TestIntegrationLoginAttemptsQuery(t *testing.T) {
 
 	for _, test := range []struct {
 		Name   string
-		Query  models.GetUserLoginAttemptCountQuery
+		Query  GetUserLoginAttemptCountQuery
 		Err    error
 		Result int64
 	}{
 		{
 			"Should return a total count of zero login attempts when comparing since beginning of time + 2min and 1s",
-			models.GetUserLoginAttemptCountQuery{Username: user, Since: timePlusTwoMinutes.Add(time.Second * 1)}, nil, 0,
+			GetUserLoginAttemptCountQuery{Username: user, Since: timePlusTwoMinutes.Add(time.Second * 1)}, nil, 0,
 		},
 		{
 			"Should return a total count of zero login attempts when comparing since beginning of time + 2min and 1s",
-			models.GetUserLoginAttemptCountQuery{Username: user, Since: timePlusTwoMinutes.Add(time.Second * 1)}, nil, 0,
+			GetUserLoginAttemptCountQuery{Username: user, Since: timePlusTwoMinutes.Add(time.Second * 1)}, nil, 0,
 		},
 		{
 			"Should return the total count of login attempts since beginning of time",
-			models.GetUserLoginAttemptCountQuery{Username: user, Since: beginningOfTime}, nil, 3,
+			GetUserLoginAttemptCountQuery{Username: user, Since: beginningOfTime}, nil, 3,
 		},
 		{
 			"Should return the total count of login attempts since beginning of time + 1min",
-			models.GetUserLoginAttemptCountQuery{Username: user, Since: timePlusOneMinute}, nil, 2,
+			GetUserLoginAttemptCountQuery{Username: user, Since: timePlusOneMinute}, nil, 2,
 		},
 		{
 			"Should return the total count of login attempts since beginning of time + 2min",
-			models.GetUserLoginAttemptCountQuery{Username: user, Since: timePlusTwoMinutes}, nil, 1,
+			GetUserLoginAttemptCountQuery{Username: user, Since: timePlusTwoMinutes}, nil, 1,
 		},
 	} {
 		mockTime := beginningOfTime
-		loginAttemptService = &Service{
-			store: &xormStore{
-				db:  db.InitTestDB(t),
-				now: func() time.Time { return mockTime },
-			},
+		s := &xormStore{
+			db:  db.InitTestDB(t),
+			now: func() time.Time { return mockTime },
 		}
-		err := loginAttemptService.CreateLoginAttempt(context.Background(), &models.CreateLoginAttemptCommand{
+
+		err := s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
 			IpAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
+
 		mockTime = timePlusOneMinute
-		err = loginAttemptService.CreateLoginAttempt(context.Background(), &models.CreateLoginAttemptCommand{
+		err = s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
 			IpAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
+
 		mockTime = timePlusTwoMinutes
-		err = loginAttemptService.CreateLoginAttempt(context.Background(), &models.CreateLoginAttemptCommand{
+		err = s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
 			IpAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
-		err = loginAttemptService.GetUserLoginAttemptCount(context.Background(), &test.Query)
+
+		count, err := s.GetUserLoginAttemptCount(context.Background(), test.Query)
 		require.Equal(t, test.Err, err, test.Name)
-		require.Equal(t, test.Result, test.Query.Result, test.Name)
+		require.Equal(t, test.Result, count, test.Name)
 	}
 }
 
@@ -84,7 +83,6 @@ func TestIntegrationLoginAttemptsDelete(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}
-	var loginAttemptService loginattempt.Service
 	user := "user"
 
 	beginningOfTime := time.Date(2017, 10, 22, 8, 0, 0, 0, time.Local)
@@ -93,53 +91,55 @@ func TestIntegrationLoginAttemptsDelete(t *testing.T) {
 
 	for _, test := range []struct {
 		Name        string
-		Cmd         models.DeleteOldLoginAttemptsCommand
+		Cmd         DeleteOldLoginAttemptsCommand
 		Err         error
 		DeletedRows int64
 	}{
 		{
 			"Should return deleted rows older than beginning of time",
-			models.DeleteOldLoginAttemptsCommand{OlderThan: beginningOfTime}, nil, 0,
+			DeleteOldLoginAttemptsCommand{OlderThan: beginningOfTime}, nil, 0,
 		},
 		{
 			"Should return deleted rows older than beginning of time + 1min",
-			models.DeleteOldLoginAttemptsCommand{OlderThan: timePlusOneMinute}, nil, 1,
+			DeleteOldLoginAttemptsCommand{OlderThan: timePlusOneMinute}, nil, 1,
 		},
 		{
 			"Should return deleted rows older than beginning of time + 2min",
-			models.DeleteOldLoginAttemptsCommand{OlderThan: timePlusTwoMinutes}, nil, 2,
+			DeleteOldLoginAttemptsCommand{OlderThan: timePlusTwoMinutes}, nil, 2,
 		},
 		{
 			"Should return deleted rows older than beginning of time + 2min and 1s",
-			models.DeleteOldLoginAttemptsCommand{OlderThan: timePlusTwoMinutes.Add(time.Second * 1)}, nil, 3,
+			DeleteOldLoginAttemptsCommand{OlderThan: timePlusTwoMinutes.Add(time.Second * 1)}, nil, 3,
 		},
 	} {
 		mockTime := beginningOfTime
-		loginAttemptService = &Service{
-			store: &xormStore{
-				db:  db.InitTestDB(t),
-				now: func() time.Time { return mockTime },
-			},
+		s := &xormStore{
+			db:  db.InitTestDB(t),
+			now: func() time.Time { return mockTime },
 		}
-		err := loginAttemptService.CreateLoginAttempt(context.Background(), &models.CreateLoginAttemptCommand{
+
+		err := s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
 			IpAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
+
 		mockTime = timePlusOneMinute
-		err = loginAttemptService.CreateLoginAttempt(context.Background(), &models.CreateLoginAttemptCommand{
+		err = s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
 			IpAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
+
 		mockTime = timePlusTwoMinutes
-		err = loginAttemptService.CreateLoginAttempt(context.Background(), &models.CreateLoginAttemptCommand{
+		err = s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
 			IpAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
-		err = loginAttemptService.DeleteOldLoginAttempts(context.Background(), &test.Cmd)
+
+		deletedRows, err := s.DeleteOldLoginAttempts(context.Background(), test.Cmd)
 		require.Equal(t, test.Err, err, test.Name)
-		require.Equal(t, test.DeletedRows, test.Cmd.DeletedRows, test.Name)
+		require.Equal(t, test.DeletedRows, deletedRows, test.Name)
 	}
 }

pkg/services/loginattempt/loginattempttest/fake.go

@@ -0,0 +1,22 @@
+package loginattempttest
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/services/loginattempt"
+)
+
+var _ loginattempt.Service = new(FakeLoginAttemptService)
+
+type FakeLoginAttemptService struct {
+	ExpectedValid bool
+	ExpectedErr   error
+}
+
+func (f FakeLoginAttemptService) Add(ctx context.Context, username, IPAddress 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

@@ -0,0 +1,27 @@
+package loginattempttest
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/services/loginattempt"
+)
+
+var _ loginattempt.Service = new(MockLoginAttemptService)
+
+type MockLoginAttemptService struct {
+	AddCalled      bool
+	ValidateCalled bool
+
+	ExpectedValid bool
+	ExpectedErr   error
+}
+
+func (f *MockLoginAttemptService) Add(ctx context.Context, username, IPAddress string) error {
+	f.AddCalled = true
+	return f.ExpectedErr
+}
+
+func (f *MockLoginAttemptService) Validate(ctx context.Context, username string) (bool, error) {
+	f.ValidateCalled = true
+	return f.ExpectedValid, f.ExpectedErr
+}

pkg/services/sqlstore/mockstore/mockstore.go

@@ -17,8 +17,7 @@ type OrgListResponse []struct {
 	Response error
 }
 type SQLStoreMock struct {
-	LastGetAlertsQuery      *models.GetAlertsQuery
-	LastLoginAttemptCommand *models.CreateLoginAttemptCommand
+	LastGetAlertsQuery *models.GetAlertsQuery
 
 	ExpectedUser                   *user.User
 	ExpectedTeamsByUser            []*models.TeamDTO
@@ -28,7 +27,6 @@ type SQLStoreMock struct {
 	ExpectedDataSourcesAccessStats []*models.DataSourceAccessStats
 	ExpectedNotifierUsageStats     []*models.NotifierUsageStats
 	ExpectedSignedInUser           *user.SignedInUser
-	ExpectedLoginAttempts          int64
 
 	ExpectedError error
 }
@@ -130,11 +128,6 @@ func (m *SQLStoreMock) GetSqlxSession() *session.SessionDB {
 	return nil
 }
 
-func (m *SQLStoreMock) CreateLoginAttempt(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error {
-	m.LastLoginAttemptCommand = cmd
-	return m.ExpectedError
-}
-
 func (m *SQLStoreMock) GetAlertById(ctx context.Context, query *models.GetAlertByIdQuery) error {
 	query.Result = m.ExpectedAlert
 	return m.ExpectedError
@@ -144,19 +137,10 @@ func (m *SQLStoreMock) GetAlertNotificationUidWithId(ctx context.Context, query
 	return m.ExpectedError
 }
 
-func (m *SQLStoreMock) DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error {
-	return m.ExpectedError
-}
-
 func (m *SQLStoreMock) GetAlertNotificationsWithUidToSend(ctx context.Context, query *models.GetAlertNotificationsWithUidToSendQuery) error {
 	return m.ExpectedError
 }
 
-func (m *SQLStoreMock) GetUserLoginAttemptCount(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error {
-	query.Result = m.ExpectedLoginAttempts
-	return m.ExpectedError
-}
-
 func (m *SQLStoreMock) GetAlertStatesForDashboard(ctx context.Context, query *models.GetAlertStatesForDashboardQuery) error {
 	return m.ExpectedError
 }