diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index eacaa039dc3..a683e2a13ce 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -201,67 +201,6 @@ func (ss *SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQu }) } -func (ss *SQLStore) UpdateUser(ctx context.Context, cmd *models.UpdateUserCommand) error { - if ss.Cfg.CaseInsensitiveLogin { - cmd.Login = strings.ToLower(cmd.Login) - cmd.Email = strings.ToLower(cmd.Email) - } - - return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { - user := user.User{ - Name: cmd.Name, - Email: cmd.Email, - Login: cmd.Login, - Theme: cmd.Theme, - Updated: TimeNow(), - } - - if _, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Update(&user); err != nil { - return err - } - - if ss.Cfg.CaseInsensitiveLogin { - if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil { - return err - } - } - - sess.publishAfterCommit(&events.UserUpdated{ - Timestamp: user.Created, - Id: user.ID, - Name: user.Name, - Login: user.Login, - Email: user.Email, - }) - - return nil - }) -} - -func (ss *SQLStore) ChangeUserPassword(ctx context.Context, cmd *models.ChangeUserPasswordCommand) error { - return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { - user := user.User{ - Password: cmd.NewPassword, - Updated: TimeNow(), - } - - _, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Update(&user) - return err - }) -} - -func (ss *SQLStore) UpdateUserLastSeenAt(ctx context.Context, cmd *models.UpdateUserLastSeenAtCommand) error { - return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { - user := user.User{ - ID: cmd.UserId, - LastSeenAt: TimeNow(), - } - - _, err := sess.ID(cmd.UserId).Update(&user) - return err - }) -} - func (ss *SQLStore) SetUsingOrg(ctx context.Context, cmd *models.SetUsingOrgCommand) error { getOrgsForUserCmd := &models.GetUserOrgListQuery{UserId: cmd.UserId} if err := ss.GetUserOrgList(ctx, getOrgsForUserCmd); err != nil { diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 51e1d56993c..2094a8263c5 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -12,73 +12,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestIntegrationUserUpdate(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } - - ss := InitTestDB(t) - - users := createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand { - return &user.CreateUserCommand{ - Email: fmt.Sprint("USER", i, "@test.com"), - Name: fmt.Sprint("USER", i), - Login: fmt.Sprint("loginUSER", i), - IsDisabled: false, - } - }) - - ss.Cfg.CaseInsensitiveLogin = true - - t.Run("Testing DB - update generates duplicate user", func(t *testing.T) { - err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{ - Login: "loginuser2", - UserId: users[0].ID, - }) - - require.Error(t, err) - }) - - t.Run("Testing DB - update lowercases existing user", func(t *testing.T) { - err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{ - Login: "loginUSER0", - Email: "USER0@test.com", - UserId: users[0].ID, - }) - require.NoError(t, err) - - query := models.GetUserByIdQuery{Id: users[0].ID} - err = ss.GetUserById(context.Background(), &query) - require.NoError(t, err) - - require.Equal(t, "loginuser0", query.Result.Login) - require.Equal(t, "user0@test.com", query.Result.Email) - }) - - t.Run("Testing DB - no user info provided", func(t *testing.T) { - err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{ - Login: "", - Email: "", - Name: "Change Name", - UserId: users[3].ID, - }) - require.NoError(t, err) - - query := models.GetUserByIdQuery{Id: users[3].ID} - err = ss.GetUserById(context.Background(), &query) - require.NoError(t, err) - - // Changed - require.Equal(t, "Change Name", query.Result.Name) - - // Unchanged - require.Equal(t, "loginUSER3", query.Result.Login) - require.Equal(t, "USER3@test.com", query.Result.Email) - }) - - ss.Cfg.CaseInsensitiveLogin = false -} - func TestIntegrationUserDataAccess(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index e986352254b..f2ed0c5c8dd 100644 --- a/pkg/services/user/userimpl/store.go +++ b/pkg/services/user/userimpl/store.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strings" + "time" "github.com/grafana/grafana/pkg/events" "github.com/grafana/grafana/pkg/infra/log" @@ -23,6 +24,9 @@ type store interface { CaseInsensitiveLoginConflict(context.Context, string, string) error GetByLogin(context.Context, *user.GetUserByLoginQuery) (*user.User, error) GetByEmail(context.Context, *user.GetUserByEmailQuery) (*user.User, error) + Update(context.Context, *user.UpdateUserCommand) error + ChangePassword(context.Context, *user.ChangeUserPasswordCommand) error + UpdateLastSeenAt(context.Context, *user.UpdateUserLastSeenAtCommand) error } type sqlStore struct { @@ -246,3 +250,64 @@ func (ss *sqlStore) userCaseInsensitiveLoginConflict(ctx context.Context, sess * return nil } + +func (ss *sqlStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) error { + if ss.cfg.CaseInsensitiveLogin { + cmd.Login = strings.ToLower(cmd.Login) + cmd.Email = strings.ToLower(cmd.Email) + } + + return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + user := user.User{ + Name: cmd.Name, + Email: cmd.Email, + Login: cmd.Login, + Theme: cmd.Theme, + Updated: time.Now(), + } + + if _, err := sess.ID(cmd.UserID).Where(ss.notServiceAccountFilter()).Update(&user); err != nil { + return err + } + + if ss.cfg.CaseInsensitiveLogin { + if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil { + return err + } + } + + sess.PublishAfterCommit(&events.UserUpdated{ + Timestamp: user.Created, + Id: user.ID, + Name: user.Name, + Login: user.Login, + Email: user.Email, + }) + + return nil + }) +} + +func (ss *sqlStore) ChangePassword(ctx context.Context, cmd *user.ChangeUserPasswordCommand) error { + return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + user := user.User{ + Password: cmd.NewPassword, + Updated: time.Now(), + } + + _, err := sess.ID(cmd.UserID).Where(ss.notServiceAccountFilter()).Update(&user) + return err + }) +} + +func (ss *sqlStore) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLastSeenAtCommand) error { + return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + user := user.User{ + ID: cmd.UserID, + LastSeenAt: time.Now(), + } + + _, err := sess.ID(cmd.UserID).Update(&user) + return err + }) +} diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index 9e865038c2b..bf1c8cef735 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -2,6 +2,7 @@ package userimpl import ( "context" + "fmt" "testing" "time" @@ -194,4 +195,97 @@ func TestIntegrationUserDataAccess(t *testing.T) { ss.Cfg.CaseInsensitiveLogin = false }) + + t.Run("Change user password", func(t *testing.T) { + err := userStore.ChangePassword(context.Background(), &user.ChangeUserPasswordCommand{}) + require.NoError(t, err) + }) + + t.Run("update last seen at", func(t *testing.T) { + err := userStore.UpdateLastSeenAt(context.Background(), &user.UpdateUserLastSeenAtCommand{}) + require.NoError(t, err) + }) +} + +func TestIntegrationUserUpdate(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + ss := sqlstore.InitTestDB(t) + userStore := ProvideStore(ss, setting.NewCfg()) + + users := createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand { + return &user.CreateUserCommand{ + Email: fmt.Sprint("USER", i, "@test.com"), + Name: fmt.Sprint("USER", i), + Login: fmt.Sprint("loginUSER", i), + IsDisabled: false, + } + }) + + userStore.cfg.CaseInsensitiveLogin = true + + t.Run("Testing DB - update generates duplicate user", func(t *testing.T) { + err := userStore.Update(context.Background(), &user.UpdateUserCommand{ + Login: "loginuser2", + UserID: users[0].ID, + }) + + require.Error(t, err) + }) + + t.Run("Testing DB - update lowercases existing user", func(t *testing.T) { + err := userStore.Update(context.Background(), &user.UpdateUserCommand{ + Login: "loginUSER0", + Email: "USER0@test.com", + UserID: users[0].ID, + }) + require.NoError(t, err) + + result, err := userStore.GetByID(context.Background(), users[0].ID) + require.NoError(t, err) + + require.Equal(t, "loginuser0", result.Login) + require.Equal(t, "user0@test.com", result.Email) + }) + + t.Run("Testing DB - no user info provided", func(t *testing.T) { + err := userStore.Update(context.Background(), &user.UpdateUserCommand{ + Login: "", + Email: "", + Name: "Change Name", + UserID: users[3].ID, + }) + require.NoError(t, err) + + // query := user.GetUserByIDQuery{ID: users[3].ID} + result, err := userStore.GetByID(context.Background(), users[3].ID) + require.NoError(t, err) + + // Changed + require.Equal(t, "Change Name", result.Name) + + // Unchanged + require.Equal(t, "loginUSER3", result.Login) + require.Equal(t, "USER3@test.com", result.Email) + }) + + ss.Cfg.CaseInsensitiveLogin = false +} + +func createFiveTestUsers(t *testing.T, sqlStore *sqlstore.SQLStore, fn func(i int) *user.CreateUserCommand) []user.User { + t.Helper() + + users := []user.User{} + for i := 0; i < 5; i++ { + cmd := fn(i) + + user, err := sqlStore.CreateUser(context.Background(), *cmd) + users = append(users, *user) + + require.Nil(t, err) + } + + return users } diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index e713e1be78b..1e1a9df869e 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -160,34 +160,16 @@ func (s *Service) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuer return s.store.GetByEmail(ctx, query) } -// TODO: remove wrapper around sqlstore func (s *Service) Update(ctx context.Context, cmd *user.UpdateUserCommand) error { - q := &models.UpdateUserCommand{ - Name: cmd.Name, - Email: cmd.Email, - Login: cmd.Login, - Theme: cmd.Theme, - UserId: cmd.UserID, - } - return s.sqlStore.UpdateUser(ctx, q) + return s.store.Update(ctx, cmd) } -// TODO: remove wrapper around sqlstore func (s *Service) ChangePassword(ctx context.Context, cmd *user.ChangeUserPasswordCommand) error { - q := &models.ChangeUserPasswordCommand{ - UserId: cmd.UserID, - NewPassword: cmd.NewPassword, - OldPassword: cmd.OldPassword, - } - return s.sqlStore.ChangeUserPassword(ctx, q) + return s.store.ChangePassword(ctx, cmd) } -// TODO: remove wrapper around sqlstore func (s *Service) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLastSeenAtCommand) error { - q := &models.UpdateUserLastSeenAtCommand{ - UserId: cmd.UserID, - } - return s.sqlStore.UpdateUserLastSeenAt(ctx, q) + return s.store.UpdateLastSeenAt(ctx, cmd) } // TODO: remove wrapper around sqlstore diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index d69329a0d64..4bf87408b3e 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -129,3 +129,15 @@ func (f *FakeUserStore) GetByLogin(ctx context.Context, query *user.GetUserByLog func (f *FakeUserStore) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuery) (*user.User, error) { return f.ExpectedUser, f.ExpectedError } + +func (f *FakeUserStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) error { + return f.ExpectedError +} + +func (f *FakeUserStore) ChangePassword(ctx context.Context, cmd *user.ChangeUserPasswordCommand) error { + return f.ExpectedError +} + +func (f *FakeUserStore) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLastSeenAtCommand) error { + return f.ExpectedError +}