From 189bf102cf1bf1619a9245f0e2bc38d8921ed883 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Tue, 22 Nov 2022 11:37:18 +0100 Subject: [PATCH] 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 | 2 + pkg/login/auth.go | 9 +- pkg/login/auth_test.go | 117 +++++++----------- pkg/login/brute_force_login_protection.go | 48 ------- .../brute_force_login_protection_test.go | 116 ----------------- pkg/models/login_attempt.go | 36 ------ .../backgroundsvcs/background_services.go | 4 +- pkg/server/wire.go | 2 + pkg/services/cleanup/cleanup.go | 33 +---- pkg/services/loginattempt/login_attempt.go | 17 ++- .../loginattemptimpl/login_attempt.go | 99 +++++++++++---- .../loginattemptimpl/login_attempt_test.go | 98 +++++++++++++++ .../loginattempt/loginattemptimpl/models.go | 23 ++++ .../loginattempt/loginattemptimpl/store.go | 62 ++++++---- .../loginattemptimpl/store_test.go | 70 +++++------ .../loginattempt/loginattempttest/fake.go | 22 ++++ .../loginattempt/loginattempttest/mock.go | 27 ++++ pkg/services/sqlstore/mockstore/mockstore.go | 18 +-- 18 files changed, 388 insertions(+), 415 deletions(-) delete mode 100644 pkg/login/brute_force_login_protection.go delete mode 100644 pkg/login/brute_force_login_protection_test.go delete mode 100644 pkg/models/login_attempt.go create mode 100644 pkg/services/loginattempt/loginattemptimpl/login_attempt_test.go create mode 100644 pkg/services/loginattempt/loginattemptimpl/models.go create mode 100644 pkg/services/loginattempt/loginattempttest/fake.go create mode 100644 pkg/services/loginattempt/loginattempttest/mock.go diff --git a/pkg/cmd/grafana-cli/runner/wire.go b/pkg/cmd/grafana-cli/runner/wire.go index 7fd0801876d..3276c5cc6ff 100644 --- a/pkg/cmd/grafana-cli/runner/wire.go +++ b/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, diff --git a/pkg/login/auth.go b/pkg/login/auth.go index 7839d48bc67..382b4b4f288 100644 --- a/pkg/login/auth.go +++ b/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) } diff --git a/pkg/login/auth_test.go b/pkg/login/auth_test.go index dfa0a629db0..edc8f4ed3d8 100644 --- a/pkg/login/auth_test.go +++ b/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) diff --git a/pkg/login/brute_force_login_protection.go b/pkg/login/brute_force_login_protection.go deleted file mode 100644 index 0c44067a5f2..00000000000 --- a/pkg/login/brute_force_login_protection.go +++ /dev/null @@ -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) -} diff --git a/pkg/login/brute_force_login_protection_test.go b/pkg/login/brute_force_login_protection_test.go deleted file mode 100644 index 71d20fa02d0..00000000000 --- a/pkg/login/brute_force_login_protection_test.go +++ /dev/null @@ -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 -} diff --git a/pkg/models/login_attempt.go b/pkg/models/login_attempt.go deleted file mode 100644 index 6e0976bc506..00000000000 --- a/pkg/models/login_attempt.go +++ /dev/null @@ -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 -} diff --git a/pkg/server/backgroundsvcs/background_services.go b/pkg/server/backgroundsvcs/background_services.go index 2582d680327..66181694739 100644 --- a/pkg/server/backgroundsvcs/background_services.go +++ b/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, ) } diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 0c46d0529d3..7c4dfeadb54 100644 --- a/pkg/server/wire.go +++ b/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, diff --git a/pkg/services/cleanup/cleanup.go b/pkg/services/cleanup/cleanup.go index d8889d37912..a04dfdd125a 100644 --- a/pkg/services/cleanup/cleanup.go +++ b/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 diff --git a/pkg/services/loginattempt/login_attempt.go b/pkg/services/loginattempt/login_attempt.go index cbf581a3d59..897ebbccf1f 100644 --- a/pkg/services/loginattempt/login_attempt.go +++ b/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 } diff --git a/pkg/services/loginattempt/loginattemptimpl/login_attempt.go b/pkg/services/loginattempt/loginattemptimpl/login_attempt.go index c9329d4cc84..11bba09a786 100644 --- a/pkg/services/loginattempt/loginattemptimpl/login_attempt.go +++ b/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) +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 err + 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 } diff --git a/pkg/services/loginattempt/loginattemptimpl/login_attempt_test.go b/pkg/services/loginattempt/loginattemptimpl/login_attempt_test.go new file mode 100644 index 00000000000..ae4de8b931f --- /dev/null +++ b/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 +} diff --git a/pkg/services/loginattempt/loginattemptimpl/models.go b/pkg/services/loginattempt/loginattemptimpl/models.go new file mode 100644 index 00000000000..b372e2df6c9 --- /dev/null +++ b/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 +} diff --git a/pkg/services/loginattempt/loginattemptimpl/store.go b/pkg/services/loginattempt/loginattemptimpl/store.go index 2881bba4e2a..6721eefb99b 100644 --- a/pkg/services/loginattempt/loginattemptimpl/store.go +++ b/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 { diff --git a/pkg/services/loginattempt/loginattemptimpl/store_test.go b/pkg/services/loginattempt/loginattemptimpl/store_test.go index e0c5af8dafb..12f4ed58bfb 100644 --- a/pkg/services/loginattempt/loginattemptimpl/store_test.go +++ b/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) } } diff --git a/pkg/services/loginattempt/loginattempttest/fake.go b/pkg/services/loginattempt/loginattempttest/fake.go new file mode 100644 index 00000000000..1168989af98 --- /dev/null +++ b/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 +} diff --git a/pkg/services/loginattempt/loginattempttest/mock.go b/pkg/services/loginattempt/loginattempttest/mock.go new file mode 100644 index 00000000000..a74c37fd3d9 --- /dev/null +++ b/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 +} diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index 1b4757b2e93..9ce5d0990ea 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/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 }