Chore: Remove some bus from login package (login attempts) (#47310)

pkg/login/auth.go

@@ -47,7 +47,7 @@ func ProvideService(store sqlstore.Store, loginService login.Service) *Authentic
 
 // AuthenticateUser authenticates the user via username & password
 func (a *AuthenticatorService) AuthenticateUser(ctx context.Context, query *models.LoginUserQuery) error {
-	if err := validateLoginAttempts(ctx, query); err != nil {
+	if err := validateLoginAttempts(ctx, query, a.store); err != nil {
 		return err
 	}
 
@@ -75,7 +75,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); err != nil {
+		if err := saveInvalidLoginAttempt(ctx, query, a.store); err != nil {
 			loginLogger.Error("Failed to save invalid login attempt", "err", err)
 		}
 

pkg/login/auth_test.go

@@ -9,6 +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/sqlstore"
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -196,14 +197,14 @@ func mockLoginUsingLDAP(enabled bool, err error, sc *authScenarioContext) {
 }
 
 func mockLoginAttemptValidation(err error, sc *authScenarioContext) {
-	validateLoginAttempts = func(context.Context, *models.LoginUserQuery) error {
+	validateLoginAttempts = func(context.Context, *models.LoginUserQuery, sqlstore.Store) error {
 		sc.loginAttemptValidationWasCalled = true
 		return err
 	}
 }
 
 func mockSaveInvalidLoginAttempt(sc *authScenarioContext) {
-	saveInvalidLoginAttempt = func(ctx context.Context, query *models.LoginUserQuery) error {
+	saveInvalidLoginAttempt = func(ctx context.Context, query *models.LoginUserQuery, _ sqlstore.Store) error {
 		sc.saveInvalidLoginAttemptWasCalled = true
 		return nil
 	}

pkg/login/brute_force_login_protection.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"time"
 
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
 )
 
 var (
@@ -13,7 +13,7 @@ var (
 	loginAttemptsWindow           = time.Minute * 5
 )
 
-var validateLoginAttempts = func(ctx context.Context, query *models.LoginUserQuery) error {
+var validateLoginAttempts = func(ctx context.Context, query *models.LoginUserQuery, store sqlstore.Store) error {
 	if query.Cfg.DisableBruteForceLoginProtection {
 		return nil
 	}
@@ -23,7 +23,7 @@ var validateLoginAttempts = func(ctx context.Context, query *models.LoginUserQue
 		Since:    time.Now().Add(-loginAttemptsWindow),
 	}
 
-	if err := bus.Dispatch(ctx, &loginAttemptCountQuery); err != nil {
+	if err := store.GetUserLoginAttemptCount(ctx, &loginAttemptCountQuery); err != nil {
 		return err
 	}
 
@@ -34,7 +34,7 @@ var validateLoginAttempts = func(ctx context.Context, query *models.LoginUserQue
 	return nil
 }
 
-var saveInvalidLoginAttempt = func(ctx context.Context, query *models.LoginUserQuery) error {
+var saveInvalidLoginAttempt = func(ctx context.Context, query *models.LoginUserQuery, store sqlstore.Store) error {
 	if query.Cfg.DisableBruteForceLoginProtection {
 		return nil
 	}
@@ -44,5 +44,5 @@ var saveInvalidLoginAttempt = func(ctx context.Context, query *models.LoginUserQ
 		IpAddress: query.IpAddress,
 	}
 
-	return bus.Dispatch(ctx, &loginAttemptCommand)
+	return store.CreateLoginAttempt(ctx, &loginAttemptCommand)
 }

pkg/login/brute_force_login_protection_test.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -59,11 +59,12 @@ func TestValidateLoginAttempts(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			withLoginAttempts(t, tc.loginAttempts)
+			store := mockstore.NewSQLStoreMock()
+			store.ExpectedLoginAttempts = tc.loginAttempts
 
 			query := &models.LoginUserQuery{Username: "user", Cfg: tc.cfg}
 
-			err := validateLoginAttempts(context.Background(), query)
+			err := validateLoginAttempts(context.Background(), query, store)
 			require.Equal(t, tc.expected, err)
 		})
 	}
@@ -71,45 +72,31 @@ func TestValidateLoginAttempts(t *testing.T) {
 
 func TestSaveInvalidLoginAttempt(t *testing.T) {
 	t.Run("When brute force protection enabled", func(t *testing.T) {
-		t.Cleanup(func() { bus.ClearBusHandlers() })
+		store := mockstore.NewSQLStoreMock()
-
-		createLoginAttemptCmd := &models.CreateLoginAttemptCommand{}
-		bus.AddHandler("test", func(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error {
-			createLoginAttemptCmd = cmd
-			return nil
-		})
-
 		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, createLoginAttemptCmd)
+		require.NotNil(t, store.LastLoginAttemptCommand)
-		assert.Equal(t, "user", createLoginAttemptCmd.Username)
+		assert.Equal(t, "user", store.LastLoginAttemptCommand.Username)
-		assert.Equal(t, "192.168.1.1:56433", createLoginAttemptCmd.IpAddress)
+		assert.Equal(t, "192.168.1.1:56433", store.LastLoginAttemptCommand.IpAddress)
 	})
 
 	t.Run("When brute force protection disabled", func(t *testing.T) {
-		t.Cleanup(func() { bus.ClearBusHandlers() })
+		store := mockstore.NewSQLStoreMock()
-
-		var createLoginAttemptCmd *models.CreateLoginAttemptCommand
-		bus.AddHandler("test", func(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error {
-			createLoginAttemptCmd = cmd
-			return nil
-		})
-
 		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, createLoginAttemptCmd)
+		require.Nil(t, store.LastLoginAttemptCommand)
 	})
 }
 
@@ -126,11 +113,3 @@ func cfgWithBruteForceLoginProtectionEnabled(t *testing.T) *setting.Cfg {
 	require.False(t, cfg.DisableBruteForceLoginProtection)
 	return cfg
 }
-
-func withLoginAttempts(t *testing.T, loginAttempts int64) {
-	t.Helper()
-	bus.AddHandler("test", func(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error {
-		query.Result = loginAttempts
-		return nil
-	})
-}

pkg/services/sqlstore/mockstore/mockstore.go

@@ -12,8 +12,9 @@ type OrgListResponse []struct {
 	Response error
 }
 type SQLStoreMock struct {
-	LastGetAlertsQuery *models.GetAlertsQuery
+	LastGetAlertsQuery      *models.GetAlertsQuery
-	LatestUserId       int64
+	LastLoginAttemptCommand *models.CreateLoginAttemptCommand
+	LatestUserId            int64
 
 	ExpectedUser                   *models.User
 	ExpectedDatasource             *models.DataSource
@@ -40,7 +41,9 @@ type SQLStoreMock struct {
 	ExpectedPersistedDashboards    models.HitList
 	ExpectedSignedInUser           *models.SignedInUser
 	ExpectedUserStars              map[int64]bool
-	ExpectedError                  error
+	ExpectedLoginAttempts          int64
+
+	ExpectedError error
 }
 
 func NewSQLStoreMock() *SQLStoreMock {
@@ -130,6 +133,12 @@ func (m SQLStoreMock) DeleteOrphanedProvisionedDashboards(ctx context.Context, c
 }
 
 func (m *SQLStoreMock) CreateLoginAttempt(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error {
+	m.LastLoginAttemptCommand = cmd
+	return m.ExpectedError
+}
+
+func (m *SQLStoreMock) GetUserLoginAttemptCount(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error {
+	query.Result = m.ExpectedLoginAttempts
 	return m.ExpectedError
 }
 

pkg/services/sqlstore/store.go

@@ -26,6 +26,7 @@ type Store interface {
 	GetOrgById(context.Context, *models.GetOrgByIdQuery) error
 	GetOrgByNameHandler(ctx context.Context, query *models.GetOrgByNameQuery) error
 	CreateLoginAttempt(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error
+	GetUserLoginAttemptCount(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error
 	DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error
 	CreateUser(ctx context.Context, cmd models.CreateUserCommand) (*models.User, error)
 	GetUserById(ctx context.Context, query *models.GetUserByIdQuery) error