mirror of
https://github.com/grafana/grafana.git
synced 2024-11-24 09:50:29 -06:00
Chore: Delete DeleteUserInSession from sqlstore (#59275)
* Remove DeleteUser from sqlstore * Use Delete instead of DeleteUser * Remove unused method * Remove DeleteUserSession from sqlstore * Add fake for DeleteUseraccessControl * Use user service, add fakes to tests * Add comments to sqlstore * Correct typo * Add quota service initialisation * Add config to acimpl initialisation * Add routing to initialisation * Making user provideStore private * Use InTransaction instead of session * Add cfg to userimpl * Fix lint * Make ProvideStore public again - enterprise tests * Fix back ProvideStore to public * Wrap merge user in transaction * Delete DeleteUserAccessControl use DeleteUSerPermissions instead * Add feature mgmt into acimpl * DeleteUserAccessControl from ac database * Remove case insensitive clause
This commit is contained in:
parent
5b71a16acf
commit
4ec08c4317
@ -14,12 +14,16 @@ import (
|
||||
"github.com/fatih/color"
|
||||
"github.com/urfave/cli/v2"
|
||||
|
||||
"github.com/grafana/grafana/pkg/api/routing"
|
||||
"github.com/grafana/grafana/pkg/bus"
|
||||
"github.com/grafana/grafana/pkg/cmd/grafana-cli/logger"
|
||||
"github.com/grafana/grafana/pkg/cmd/grafana-cli/utils"
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/infra/tracing"
|
||||
"github.com/grafana/grafana/pkg/models"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore/migrations"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
|
||||
@ -36,10 +40,7 @@ func initConflictCfg(cmd *utils.ContextCommandLine) (*setting.Cfg, error) {
|
||||
HomePath: cmd.HomePath(),
|
||||
Args: append(configOptions, "cfg:log.level=error"), // tailing arguments have precedence over the options string
|
||||
})
|
||||
if !cfg.CaseInsensitiveLogin {
|
||||
logger.Info("Case Insensitive Login is not enabled, setting to true to not introduce any more conflicts")
|
||||
cfg.CaseInsensitiveLogin = true
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@ -59,7 +60,21 @@ func initializeConflictResolver(cmd *utils.ContextCommandLine, f Formatter, ctx
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("%v: %w", "failed to get users with conflicting logins", err)
|
||||
}
|
||||
resolver := ConflictResolver{Users: conflicts, Store: s}
|
||||
quotaService := quotaimpl.ProvideService(s, cfg)
|
||||
userService, err := userimpl.ProvideService(s, nil, cfg, nil, nil, quotaService)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("%v: %w", "failed to get user service", err)
|
||||
}
|
||||
routing := routing.ProvideRegister()
|
||||
featMgmt, err := featuremgmt.ProvideManagerService(cfg, nil)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("%v: %w", "failed to get feature management service", err)
|
||||
}
|
||||
acService, err := acimpl.ProvideService(cfg, s, routing, nil, nil, featMgmt)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("%v: %w", "failed to get access control", err)
|
||||
}
|
||||
resolver := ConflictResolver{Users: conflicts, Store: s, userService: userService, ac: acService}
|
||||
resolver.BuildConflictBlocks(conflicts, f)
|
||||
return &resolver, nil
|
||||
}
|
||||
@ -337,11 +352,7 @@ func (r *ConflictResolver) MergeConflictingUsers(ctx context.Context) error {
|
||||
|
||||
// creating a session for each block of users
|
||||
// we want to rollback incase something happens during update / delete
|
||||
if err := r.Store.WithDbSession(ctx, func(sess *db.Session) error {
|
||||
err := sess.Begin()
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not open a db session: %w", err)
|
||||
}
|
||||
if err := r.Store.InTransaction(ctx, func(ctx context.Context) error {
|
||||
for _, u := range users {
|
||||
if u.Direction == "+" {
|
||||
id, err := strconv.ParseInt(u.ID, 10, 64)
|
||||
@ -357,36 +368,34 @@ func (r *ConflictResolver) MergeConflictingUsers(ctx context.Context) error {
|
||||
fromUserIds = append(fromUserIds, id)
|
||||
}
|
||||
}
|
||||
if _, err := sess.ID(intoUserId).Where(sqlstore.NotServiceAccountFilter(r.Store)).Get(&intoUser); err != nil {
|
||||
if _, err := r.userService.GetByID(ctx, &user.GetUserByIDQuery{ID: intoUserId}); err != nil {
|
||||
return fmt.Errorf("could not find intoUser: %w", err)
|
||||
}
|
||||
|
||||
for _, fromUserId := range fromUserIds {
|
||||
var fromUser user.User
|
||||
exists, err := sess.ID(fromUserId).Where(sqlstore.NotServiceAccountFilter(r.Store)).Get(&fromUser)
|
||||
_, err := r.userService.GetByID(ctx, &user.GetUserByIDQuery{ID: fromUserId})
|
||||
if err != nil && errors.Is(err, user.ErrUserNotFound) {
|
||||
fmt.Printf("user with id %d does not exist, skipping\n", fromUserId)
|
||||
}
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not find fromUser: %w", err)
|
||||
}
|
||||
if !exists {
|
||||
fmt.Printf("user with id %d does not exist, skipping\n", fromUserId)
|
||||
}
|
||||
// // delete the user
|
||||
delErr := r.Store.DeleteUserInSession(ctx, sess, &models.DeleteUserCommand{UserId: fromUserId})
|
||||
// delete the user
|
||||
delErr := r.userService.Delete(ctx, &user.DeleteUserCommand{UserID: fromUserId})
|
||||
if delErr != nil {
|
||||
return fmt.Errorf("error during deletion of user: %w", delErr)
|
||||
}
|
||||
delACErr := r.ac.DeleteUserPermissions(ctx, 0, fromUserId)
|
||||
if delACErr != nil {
|
||||
return fmt.Errorf("error during deletion of user access control: %w", delACErr)
|
||||
}
|
||||
}
|
||||
commitErr := sess.Commit()
|
||||
if commitErr != nil {
|
||||
return fmt.Errorf("could not commit operation for useridentification %s: %w", block, commitErr)
|
||||
}
|
||||
userStore := userimpl.ProvideStore(r.Store, setting.NewCfg())
|
||||
|
||||
updateMainCommand := &user.UpdateUserCommand{
|
||||
UserID: intoUser.ID,
|
||||
Login: strings.ToLower(intoUser.Login),
|
||||
Email: strings.ToLower(intoUser.Email),
|
||||
}
|
||||
updateErr := userStore.Update(ctx, updateMainCommand)
|
||||
updateErr := r.userService.Update(ctx, updateMainCommand)
|
||||
if updateErr != nil {
|
||||
return fmt.Errorf("could not update user: %w", updateErr)
|
||||
}
|
||||
@ -602,6 +611,8 @@ func (r *ConflictResolver) ToStringPresentation() string {
|
||||
|
||||
type ConflictResolver struct {
|
||||
Store *sqlstore.SQLStore
|
||||
userService user.Service
|
||||
ac accesscontrol.Service
|
||||
Config *setting.Cfg
|
||||
Users ConflictingUsers
|
||||
ValidUsers ConflictingUsers
|
||||
|
@ -11,9 +11,11 @@ import (
|
||||
"github.com/urfave/cli/v2"
|
||||
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
|
||||
"github.com/grafana/grafana/pkg/services/team/teamimpl"
|
||||
"github.com/grafana/grafana/pkg/services/user"
|
||||
"github.com/grafana/grafana/pkg/services/user/usertest"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
|
||||
@ -578,6 +580,9 @@ func TestRunValidateConflictUserFile(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestIntegrationMergeUser(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("skipping integration test")
|
||||
}
|
||||
t.Run("should be able to merge user", func(t *testing.T) {
|
||||
// Restore after destructive operation
|
||||
sqlStore := db.InitTestDB(t)
|
||||
@ -613,7 +618,11 @@ func TestIntegrationMergeUser(t *testing.T) {
|
||||
// get users
|
||||
conflictUsers, err := GetUsersWithConflictingEmailsOrLogins(&cli.Context{Context: context.Background()}, sqlStore)
|
||||
require.NoError(t, err)
|
||||
r := ConflictResolver{Store: sqlStore}
|
||||
r := ConflictResolver{
|
||||
Store: sqlStore,
|
||||
userService: usertest.NewUserServiceFake(),
|
||||
ac: actest.FakeService{},
|
||||
}
|
||||
r.BuildConflictBlocks(conflictUsers, fmt.Sprintf)
|
||||
tmpFile, err := generateConflictUsersFile(&r)
|
||||
require.NoError(t, err)
|
||||
@ -633,6 +642,9 @@ func TestIntegrationMergeUser(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestIntegrationMergeUserFromNewFileInput(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("skipping integration test")
|
||||
}
|
||||
t.Run("should be able to merge users after choosing a different user to keep", func(t *testing.T) {
|
||||
type testBuildConflictBlock struct {
|
||||
desc string
|
||||
@ -748,7 +760,13 @@ conflict: test2
|
||||
// add additional user with conflicting login where DOMAIN is upper case
|
||||
conflictUsers, err := GetUsersWithConflictingEmailsOrLogins(&cli.Context{Context: context.Background()}, sqlStore)
|
||||
require.NoError(t, err)
|
||||
r := ConflictResolver{Store: sqlStore}
|
||||
userFake := usertest.NewUserServiceFake()
|
||||
userFake.ExpectedUser = &user.User{Email: "test", Login: "test", OrgID: int64(testOrgID)}
|
||||
r := ConflictResolver{
|
||||
Store: sqlStore,
|
||||
userService: userFake,
|
||||
ac: actest.FakeService{},
|
||||
}
|
||||
r.BuildConflictBlocks(conflictUsers, fmt.Sprintf)
|
||||
require.NoError(t, err)
|
||||
// validation to get newConflicts
|
||||
|
@ -1,3 +1,4 @@
|
||||
// DO NOT ADD METHODS TO THIS FILES. SQLSTORE IS DEPRECATED AND WILL BE REMOVED.
|
||||
package sqlstore
|
||||
|
||||
import (
|
||||
|
@ -1,3 +1,4 @@
|
||||
// DO NOT ADD METHODS TO THIS FILES. SQLSTORE IS DEPRECATED AND WILL BE REMOVED.
|
||||
package sqlstore
|
||||
|
||||
import (
|
||||
@ -5,7 +6,6 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/grafana/grafana/pkg/events"
|
||||
@ -256,70 +256,6 @@ func getTeamSelectSQLBase(filteredUsers []string) string {
|
||||
` FROM team as team `
|
||||
}
|
||||
|
||||
func (ss *SQLStore) DeleteUserInSession(ctx context.Context, sess *DBSession, cmd *models.DeleteUserCommand) error {
|
||||
return deleteUserInTransaction(ss, sess, cmd)
|
||||
}
|
||||
|
||||
func deleteUserInTransaction(ss *SQLStore, sess *DBSession, cmd *models.DeleteUserCommand) error {
|
||||
// Check if user exists
|
||||
usr := user.User{ID: cmd.UserId}
|
||||
has, err := sess.Where(NotServiceAccountFilter(ss)).Get(&usr)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !has {
|
||||
return user.ErrUserNotFound
|
||||
}
|
||||
for _, sql := range UserDeletions() {
|
||||
_, err := sess.Exec(sql, cmd.UserId)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
return deleteUserAccessControl(sess, cmd.UserId)
|
||||
}
|
||||
|
||||
func deleteUserAccessControl(sess *DBSession, userID int64) error {
|
||||
// Delete user role assignments
|
||||
if _, err := sess.Exec("DELETE FROM user_role WHERE user_id = ?", userID); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Delete permissions that are scoped to user
|
||||
if _, err := sess.Exec("DELETE FROM permission WHERE scope = ?", ac.Scope("users", "id", strconv.FormatInt(userID, 10))); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
var roleIDs []int64
|
||||
if err := sess.SQL("SELECT id FROM role WHERE name = ?", ac.ManagedUserRoleName(userID)).Find(&roleIDs); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if len(roleIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
query := "DELETE FROM permission WHERE role_id IN(? " + strings.Repeat(",?", len(roleIDs)-1) + ")"
|
||||
args := make([]interface{}, 0, len(roleIDs)+1)
|
||||
args = append(args, query)
|
||||
for _, id := range roleIDs {
|
||||
args = append(args, id)
|
||||
}
|
||||
|
||||
// Delete managed user permissions
|
||||
if _, err := sess.Exec(args...); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Delete managed user roles
|
||||
if _, err := sess.Exec("DELETE FROM role WHERE name = ?", ac.ManagedUserRoleName(userID)); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func UserDeletions() []string {
|
||||
deletes := []string{
|
||||
"DELETE FROM star WHERE user_id = ?",
|
||||
|
Loading…
Reference in New Issue
Block a user