From 3a7fc80948e630c0f88edccba54d66acacf6d724 Mon Sep 17 00:00:00 2001 From: Serge Zaitsev Date: Wed, 6 Apr 2022 08:45:01 +0200 Subject: [PATCH] Chore: Remove last bus parts from login package (#47313) * Chore: Remove last bus parts from login package * fix middleware tests --- pkg/login/auth.go | 2 +- pkg/login/auth_test.go | 2 +- pkg/login/grafana_login.go | 6 ++--- pkg/login/grafana_login_test.go | 24 +++++++++----------- pkg/login/ldap_login.go | 3 +-- pkg/middleware/middleware_basic_auth_test.go | 19 ++++------------ pkg/services/sqlstore/mockstore/mockstore.go | 1 + 7 files changed, 23 insertions(+), 34 deletions(-) diff --git a/pkg/login/auth.go b/pkg/login/auth.go index 429317ec4c7..ae76ee3bec5 100644 --- a/pkg/login/auth.go +++ b/pkg/login/auth.go @@ -55,7 +55,7 @@ func (a *AuthenticatorService) AuthenticateUser(ctx context.Context, query *mode return err } - err := loginUsingGrafanaDB(ctx, query) + err := loginUsingGrafanaDB(ctx, query, a.store) if err == nil || (!errors.Is(err, models.ErrUserNotFound) && !errors.Is(err, ErrInvalidCredentials) && !errors.Is(err, ErrUserDisabled)) { query.AuthModule = "grafana" diff --git a/pkg/login/auth_test.go b/pkg/login/auth_test.go index 5ebd4098936..2dad43286c4 100644 --- a/pkg/login/auth_test.go +++ b/pkg/login/auth_test.go @@ -183,7 +183,7 @@ type authScenarioContext struct { type authScenarioFunc func(sc *authScenarioContext) func mockLoginUsingGrafanaDB(err error, sc *authScenarioContext) { - loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery) error { + loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery, _ sqlstore.Store) error { sc.grafanaLoginWasCalled = true return err } diff --git a/pkg/login/grafana_login.go b/pkg/login/grafana_login.go index 593cfc1c85a..cae3c97be2c 100644 --- a/pkg/login/grafana_login.go +++ b/pkg/login/grafana_login.go @@ -4,8 +4,8 @@ import ( "context" "crypto/subtle" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/util" ) @@ -21,10 +21,10 @@ var validatePassword = func(providedPassword string, userPassword string, userSa return nil } -var loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery) error { +var loginUsingGrafanaDB = func(ctx context.Context, query *models.LoginUserQuery, store sqlstore.Store) error { userQuery := models.GetUserByLoginQuery{LoginOrEmail: query.Username} - if err := bus.Dispatch(ctx, &userQuery); err != nil { + if err := store.GetUserByLogin(ctx, &userQuery); err != nil { return err } diff --git a/pkg/login/grafana_login_test.go b/pkg/login/grafana_login_test.go index 28fea924c9b..fb9b1b1cd16 100644 --- a/pkg/login/grafana_login_test.go +++ b/pkg/login/grafana_login_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/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -13,7 +13,7 @@ import ( func TestLoginUsingGrafanaDB(t *testing.T) { grafanaLoginScenario(t, "When login with non-existing user", func(sc *grafanaLoginScenarioContext) { sc.withNonExistingUser() - err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery) + err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) require.EqualError(t, err, models.ErrUserNotFound.Error()) assert.False(t, sc.validatePasswordCalled) @@ -22,7 +22,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) { grafanaLoginScenario(t, "When login with invalid credentials", func(sc *grafanaLoginScenarioContext) { sc.withInvalidPassword() - err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery) + err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) require.EqualError(t, err, ErrInvalidCredentials.Error()) @@ -32,7 +32,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) { grafanaLoginScenario(t, "When login with valid credentials", func(sc *grafanaLoginScenarioContext) { sc.withValidCredentials() - err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery) + err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) require.NoError(t, err) assert.True(t, sc.validatePasswordCalled) @@ -44,7 +44,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) { grafanaLoginScenario(t, "When login with disabled user", func(sc *grafanaLoginScenarioContext) { sc.withDisabledUser() - err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery) + err := loginUsingGrafanaDB(context.Background(), sc.loginUserQuery, sc.store) require.EqualError(t, err, ErrUserDisabled.Error()) assert.False(t, sc.validatePasswordCalled) @@ -53,6 +53,7 @@ func TestLoginUsingGrafanaDB(t *testing.T) { } type grafanaLoginScenarioContext struct { + store *mockstore.SQLStoreMock loginUserQuery *models.LoginUserQuery validatePasswordCalled bool } @@ -66,6 +67,7 @@ func grafanaLoginScenario(t *testing.T, desc string, fn grafanaLoginScenarioFunc origValidatePassword := validatePassword sc := &grafanaLoginScenarioContext{ + store: mockstore.NewSQLStoreMock(), loginUserQuery: &models.LoginUserQuery{ Username: "user", Password: "pwd", @@ -95,14 +97,10 @@ func mockPasswordValidation(valid bool, sc *grafanaLoginScenarioContext) { } func (sc *grafanaLoginScenarioContext) getUserByLoginQueryReturns(user *models.User) { - bus.AddHandler("test", func(ctx context.Context, query *models.GetUserByLoginQuery) error { - if user == nil { - return models.ErrUserNotFound - } - - query.Result = user - return nil - }) + sc.store.ExpectedUser = user + if user == nil { + sc.store.ExpectedError = models.ErrUserNotFound + } } func (sc *grafanaLoginScenarioContext) withValidCredentials() { diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index 629b0c8d14a..b52f431cd06 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -4,7 +4,6 @@ import ( "context" "errors" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/ldap" @@ -59,7 +58,7 @@ var loginUsingLDAP = func(ctx context.Context, query *models.LoginUserQuery, log ExternalUser: externalUser, SignupAllowed: setting.LDAPAllowSignup, } - err = bus.Dispatch(ctx, upsert) + err = loginService.UpsertUser(ctx, upsert) if err != nil { return true, err } diff --git a/pkg/middleware/middleware_basic_auth_test.go b/pkg/middleware/middleware_basic_auth_test.go index e9a8a7a4ebd..1c4fbca2ee0 100644 --- a/pkg/middleware/middleware_basic_auth_test.go +++ b/pkg/middleware/middleware_basic_auth_test.go @@ -79,21 +79,12 @@ func TestMiddlewareBasicAuth(t *testing.T) { const password = "MyPass" const salt = "Salt" - login.ProvideService(sc.sqlStore, &logintest.LoginServiceFake{}) - - bus.AddHandler("user-query", func(ctx context.Context, query *models.GetUserByLoginQuery) error { - encoded, err := util.EncodePassword(password, salt) - if err != nil { - return err - } - query.Result = &models.User{ - Password: encoded, - Id: id, - Salt: salt, - } - return nil - }) + encoded, err := util.EncodePassword(password, salt) + require.NoError(t, err) + sc.mockSQLStore.ExpectedUser = &models.User{Password: encoded, Id: id, Salt: salt} + sc.mockSQLStore.ExpectedSignedInUser = &models.SignedInUser{UserId: id} + login.ProvideService(sc.mockSQLStore, &logintest.LoginServiceFake{}) bus.AddHandler("get-sign-user", func(ctx context.Context, query *models.GetSignedInUserQuery) error { query.Result = &models.SignedInUser{UserId: query.UserId} return nil diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index 4a87e0c0fcb..403089ffdae 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -156,6 +156,7 @@ func (m *SQLStoreMock) GetUserById(ctx context.Context, query *models.GetUserByI } func (m *SQLStoreMock) GetUserByLogin(ctx context.Context, query *models.GetUserByLoginQuery) error { + query.Result = m.ExpectedUser return m.ExpectedError }