From 1e8f8dff4b3f3974aa5d97d42767235ac3153833 Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Thu, 29 Sep 2022 14:26:24 +0200 Subject: [PATCH] Auth: Adds validation and ingestion of conflict file (#53014) * add users-manager command * add users-manager command * rename files * refactor: imports and renaming * Command: add conflict merge user command - MergeUser will - replace all user_ids from conflicting users to the chosen userId - delete users whose user_ids are not the chosen user - SameIdentification will - update chosen user with chosen email,login details - delete users whose user_ids are not the chosen user * refactor: clean up * refactor: create structure for read, validate, ingest * feat: ls and generate-file for conflicting users * remove usagestats * added back pkg/services/login/authinfoservice/database/stats.go * Revert "added back pkg/services/login/authinfoservice/database/stats.go" This reverts commit 2ba6e3c4d602122bda86911c56934407904eb268. * Revert "remove usagestats" This reverts commit 1e3fa978100eed48f4bead0f631b8bd03e01588b. * cherry pick * Revert "cherry pick" This reverts commit 461626c306b9501e3e4eed05a5919caa7a3de884. * validation of picked merge user * fix test * make lint * make test run * tests for ingest working * clean up and refactored to align with downstream refactoring * formatting * refactor: name list instead of ls * fix: static lint error use trimprefix * WIP: permissions for validation * fix: remove unused functions in sqlstore * fix: remove unused function * handling of multiple users and resolve discarded users * fix tests * fix: bug that did not exclude the blocks * ioutil is blacklisted * WIP: validation * tests for merging a user working * add latest changes to output print * refactor: removed conflictEmail and conflictLogin that was not used * refactor: code clean up, showChanges working * test and linting fixes * test and linting fixes * refactor: removed logging of config and added more info for vlidation command * refactor: fix order of code * fix time now * refactor: no longer need for check casesensitive login/email * removed unnessecary loop * refactor: move functions around * test: working * docs: add docuemntationf for file * Add failing test for generating the conflict login block * Fix regex * Fix some stuff/tests Co-authored-by: eleijonmarck * add: docs for conflict file * add: conflict_email, conflict_login fields * add: conflict_email, conflict_login fields * WIP * fix: tests working as intended * Update pkg/cmd/grafana-cli/commands/conflict_user_command.go Co-authored-by: linoman <2051016+linoman@users.noreply.github.com> * review comments * Update pkg/cmd/grafana-cli/commands/conflict_user_command.go Co-authored-by: Misi * Update pkg/cmd/grafana-cli/commands/conflict_user_command.go Co-authored-by: Misi * missspelling * trailing new line * update to use userimpl store * remove newline * remove newline * refactor: initializing of resolver for conflicts * fix: test sqlStore * refactor: removed lines * refactor: remove TODOs Co-authored-by: Mihaly Gyongyosi Co-authored-by: linoman <2051016+linoman@users.noreply.github.com> --- pkg/cmd/grafana-cli/commands/commands.go | 12 +- .../commands/conflict_user_command.go | 478 +++++++++++--- .../commands/conflict_user_command_test.go | 616 +++++++++++++++--- .../accesscontrol/database/database.go | 4 +- pkg/services/sqlstore/org_users.go | 2 +- pkg/services/sqlstore/user.go | 23 +- 6 files changed, 937 insertions(+), 198 deletions(-) diff --git a/pkg/cmd/grafana-cli/commands/commands.go b/pkg/cmd/grafana-cli/commands/commands.go index 2765940a67b..6436b0ce4d2 100644 --- a/pkg/cmd/grafana-cli/commands/commands.go +++ b/pkg/cmd/grafana-cli/commands/commands.go @@ -208,9 +208,19 @@ var adminCommands = []*cli.Command{ }, { Name: "generate-file", - Usage: "creates a conflict users file.. Safe to execute multiple times.", + Usage: "creates a conflict users file. Safe to execute multiple times.", Action: runGenerateConflictUsersFile(), }, + { + Name: "validate-file", + Usage: "validates the conflict users file. Safe to execute multiple times.", + Action: runValidateConflictUsersFile(), + }, + { + Name: "ingest-file", + Usage: "ingests the conflict users file", + Action: runIngestConflictUsersFile(), + }, }, }, }, diff --git a/pkg/cmd/grafana-cli/commands/conflict_user_command.go b/pkg/cmd/grafana-cli/commands/conflict_user_command.go index 53cf732fd20..1826ba0329f 100644 --- a/pkg/cmd/grafana-cli/commands/conflict_user_command.go +++ b/pkg/cmd/grafana-cli/commands/conflict_user_command.go @@ -1,8 +1,13 @@ package commands import ( + "context" + "errors" "fmt" "os" + "path/filepath" + "regexp" + "strconv" "strings" "github.com/fatih/color" @@ -10,19 +15,49 @@ import ( "github.com/grafana/grafana/pkg/cmd/grafana-cli/logger" "github.com/grafana/grafana/pkg/cmd/grafana-cli/utils" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/db" "github.com/grafana/grafana/pkg/services/sqlstore/migrations" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/user/userimpl" + "github.com/grafana/grafana/pkg/setting" "github.com/urfave/cli/v2" ) -func getSqlStore(context *cli.Context) (*sqlstore.SQLStore, error) { - cmd := &utils.ContextCommandLine{Context: context} - cfg, err := initCfg(cmd) - cfg.Logger = nil +func initConflictCfg(cmd *utils.ContextCommandLine) (*setting.Cfg, error) { + configOptions := strings.Split(cmd.String("configOverrides"), " ") + configOptions = append(configOptions, cmd.Args().Slice()...) + cfg, err := setting.NewCfgFromArgs(setting.CommandLineArgs{ + Config: cmd.ConfigFile(), + HomePath: cmd.HomePath(), + Args: append(configOptions, "cfg:log.level=error"), // tailing arguments have precedence over the options string + }) + if err != nil { + return nil, err + } + return cfg, nil +} + +func initializeConflictResolver(cmd *utils.ContextCommandLine, f Formatter, ctx *cli.Context) (*ConflictResolver, error) { + cfg, err := initConflictCfg(cmd) if err != nil { return nil, fmt.Errorf("%v: %w", "failed to load configuration", err) } + s, err := getSqlStore(cfg) + if err != nil { + return nil, fmt.Errorf("%v: %w", "failed to get to sql", err) + } + conflicts, err := GetUsersWithConflictingEmailsOrLogins(ctx, s) + if err != nil { + return nil, fmt.Errorf("%v: %w", "failed to get users with conflicting logins", err) + } + resolver := ConflictResolver{Users: conflicts} + resolver.BuildConflictBlocks(conflicts, f) + return &resolver, nil +} + +func getSqlStore(cfg *setting.Cfg) (*sqlstore.SQLStore, error) { tracer, err := tracing.ProvideService(cfg) if err != nil { return nil, fmt.Errorf("%v: %w", "failed to initialize tracer service", err) @@ -33,28 +68,21 @@ func getSqlStore(context *cli.Context) (*sqlstore.SQLStore, error) { func runListConflictUsers() func(context *cli.Context) error { return func(context *cli.Context) error { - s, err := getSqlStore(context) + cmd := &utils.ContextCommandLine{Context: context} + whiteBold := color.New(color.FgWhite).Add(color.Bold) + r, err := initializeConflictResolver(cmd, whiteBold.Sprintf, context) if err != nil { - return fmt.Errorf("%v: %w", "failed to get to sql", err) + return fmt.Errorf("%v: %w", "failed to initialize conflict resolver", err) } - conflicts, err := GetUsersWithConflictingEmailsOrLogins(context, s) - if err != nil { - return fmt.Errorf("%v: %w", "failed to get users with conflicting logins", err) - } - if len(conflicts) < 1 { + if len(r.Users) < 1 { logger.Info(color.GreenString("No Conflicting users found.\n\n")) return nil } - whiteBold := color.New(color.FgWhite).Add(color.Bold) - resolver := ConflictResolver{Users: conflicts} - resolver.BuildConflictBlocks(whiteBold.Sprintf) - logger.Infof("\n\nShowing Conflicts\n\n") - logger.Infof(resolver.ToStringPresentation()) + logger.Infof("\n\nShowing conflicts\n\n") + logger.Infof(r.ToStringPresentation()) logger.Infof("\n") - // TODO: remove line when finished - // this is only for debugging - if len(resolver.DiscardedBlocks) != 0 { - resolver.logDiscardedUsers() + if len(r.DiscardedBlocks) != 0 { + r.logDiscardedUsers() } return nil } @@ -62,74 +90,324 @@ func runListConflictUsers() func(context *cli.Context) error { func runGenerateConflictUsersFile() func(context *cli.Context) error { return func(context *cli.Context) error { - s, err := getSqlStore(context) + cmd := &utils.ContextCommandLine{Context: context} + r, err := initializeConflictResolver(cmd, fmt.Sprintf, context) if err != nil { - return fmt.Errorf("%v: %w", "failed to get to sql", err) + return fmt.Errorf("%v: %w", "failed to initialize conflict resolver", err) } - conflicts, err := GetUsersWithConflictingEmailsOrLogins(context, s) - if err != nil { - return fmt.Errorf("%v: %w", "failed to get users with conflicting logins", err) - } - if len(conflicts) < 1 { + if len(r.Users) < 1 { logger.Info(color.GreenString("No Conflicting users found.\n\n")) return nil } - resolver := ConflictResolver{Users: conflicts} - resolver.BuildConflictBlocks(fmt.Sprintf) - tmpFile, err := generateConflictUsersFile(&resolver) + tmpFile, err := generateConflictUsersFile(r) if err != nil { return fmt.Errorf("generating file return error: %w", err) } logger.Infof("\n\ngenerated file\n") logger.Infof("%s\n\n", tmpFile.Name()) logger.Infof("once the file is edited and resolved conflicts, you can either validate or ingest the file\n\n") - if len(resolver.DiscardedBlocks) != 0 { - resolver.logDiscardedUsers() + if len(r.DiscardedBlocks) != 0 { + r.logDiscardedUsers() } return nil } } +func runValidateConflictUsersFile() func(context *cli.Context) error { + return func(context *cli.Context) error { + cmd := &utils.ContextCommandLine{Context: context} + r, err := initializeConflictResolver(cmd, fmt.Sprintf, context) + if err != nil { + return fmt.Errorf("%v: %w", "failed to initialize conflict resolver", err) + } + + // read in the file to validate + // read in the file to ingest + arg := cmd.Args().First() + if arg == "" { + return errors.New("please specify a absolute path to file to read from") + } + b, err := os.ReadFile(filepath.Clean(arg)) + if err != nil { + return fmt.Errorf("could not read file with error %e", err) + } + validErr := getValidConflictUsers(r, b) + if validErr != nil { + return fmt.Errorf("could not validate file with error %s", err) + } + logger.Info("File validation complete without errors.\n\n File can be used with ingesting command `ingest-file`.\n\n") + return nil + } +} + +func runIngestConflictUsersFile() func(context *cli.Context) error { + return func(context *cli.Context) error { + cmd := &utils.ContextCommandLine{Context: context} + r, err := initializeConflictResolver(cmd, fmt.Sprintf, context) + if err != nil { + return fmt.Errorf("%v: %w", "failed to initialize conflict resolver", err) + } + + // read in the file to ingest + arg := cmd.Args().First() + if arg == "" { + return errors.New("please specify a absolute path to file to read from") + } + b, err := os.ReadFile(filepath.Clean(arg)) + if err != nil { + return fmt.Errorf("could not read file with error %e", err) + } + validErr := getValidConflictUsers(r, b) + if validErr != nil { + return fmt.Errorf("could not validate file with error %s", validErr) + } + // should we rebuild blocks here? + // kind of a weird thing maybe? + if len(r.ValidUsers) == 0 { + return fmt.Errorf("no users") + } + r.showChanges() + if !confirm("\n\nWe encourage users to create a db backup before running this command. \n Proceed with operation?") { + return fmt.Errorf("user cancelled") + } + err = r.MergeConflictingUsers(context.Context) + if err != nil { + return fmt.Errorf("not able to merge with %e", err) + } + logger.Info("\n\nconflicts resolved.\n") + return nil + } +} + +func getDocumentationForFile() string { + return `# Conflicts File +# This file is generated by the grafana-cli command ` + color.CyanString("grafana-cli admin user-manager conflicts generate-file") + `. +# +# Commands: +# +, keep = keep user +# -, delete = delete user +# +# The fields conflict_email and conflict_login +# indicate that we see a conflict in email and/or login with another user. +# Both these fields can be true. +# +# There needs to be exactly one picked user per conflict block. +# +# The lines can be re-ordered. +# +# If you feel like you want to wait with a specific block, +# delete all lines regarding that conflict block. +# +` +} + func generateConflictUsersFile(r *ConflictResolver) (*os.File, error) { tmpFile, err := os.CreateTemp(os.TempDir(), "conflicting_user_*.diff") if err != nil { return nil, err } + if _, err := tmpFile.Write([]byte(getDocumentationForFile())); err != nil { + return nil, err + } if _, err := tmpFile.Write([]byte(r.ToStringPresentation())); err != nil { return nil, err } return tmpFile, nil } +func getValidConflictUsers(r *ConflictResolver, b []byte) error { + newConflicts := make(ConflictingUsers, 0) + // need to verify that id or email exists + previouslySeenIds := map[string]bool{} + previouslySeenEmails := map[string]bool{} + for _, users := range r.Blocks { + for _, u := range users { + previouslySeenIds[strings.ToLower(u.ID)] = true + previouslySeenEmails[strings.ToLower(u.Email)] = true + } + } + + // tested in https://regex101.com/r/una3zC/1 + diffPattern := `^[+-]` + // compiling since in a loop + matchingExpression, err := regexp.Compile(diffPattern) + if err != nil { + return fmt.Errorf("unable to compile regex %s: %w", diffPattern, err) + } + for _, row := range strings.Split(string(b), "\n") { + if row == "" { + // end of file + break + } + // if the row starts with a #, it is a comment + if row[0] == '#' { + // comment + continue + } + entryRow := matchingExpression.Match([]byte(row)) + if !entryRow { + // block row + // conflict: hej + continue + } + + newUser := &ConflictingUser{} + err := newUser.Marshal(row) + if err != nil { + return fmt.Errorf("could not parse the content of the file with error %e", err) + } + if !previouslySeenEmails[strings.ToLower(newUser.Email)] { + return fmt.Errorf("not valid email: %s, email not in previous conflicts seen", newUser.Email) + } + // valid entry + newConflicts = append(newConflicts, *newUser) + } + r.ValidUsers = newConflicts + r.BuildConflictBlocks(newConflicts, fmt.Sprintf) + return nil +} + +func (r *ConflictResolver) MergeConflictingUsers(ctx context.Context) error { + for block, users := range r.Blocks { + if len(users) < 2 { + return fmt.Errorf("not enough users to perform merge, found %d for id %s, should be at least 2", len(users), block) + } + var intoUser user.User + var intoUserId int64 + var fromUserIds []int64 + + // creating a session for each block of users + // we want to rollback incase something happens during update / delete + sess := r.Store.NewSession(ctx) + defer sess.Close() + err := sess.Begin() + if err != nil { + return fmt.Errorf("could not open a db session: %w", err) + } + for _, u := range users { + if u.Direction == "+" { + id, err := strconv.ParseInt(u.ID, 10, 64) + if err != nil { + return fmt.Errorf("could not convert id in +") + } + intoUserId = id + } else if u.Direction == "-" { + id, err := strconv.ParseInt(u.ID, 10, 64) + if err != nil { + return fmt.Errorf("could not convert id in -") + } + fromUserIds = append(fromUserIds, id) + } + } + if _, err := sess.ID(intoUserId).Where(sqlstore.NotServiceAccountFilter(r.Store)).Get(&intoUser); 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) + 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}) + if delErr != nil { + return fmt.Errorf("error during deletion of user: %w", delErr) + } + } + 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) + if updateErr != nil { + return fmt.Errorf("could not update user: %w", updateErr) + } + } + return nil +} + +/* +hej@test.com+hej@test.com +all of the permissions, roles and ownership will be transferred to the user. ++ id: 1, email: hej@test.com, login: hej@test.com +these user(s) will be deleted and their permissions transferred. +- id: 2, email: HEJ@TEST.COM, login: HEJ@TEST.COM +- id: 3, email: hej@TEST.com, login: hej@TEST.com +*/ +func (r *ConflictResolver) showChanges() { + if len(r.ValidUsers) == 0 { + fmt.Println("no changes will take place as we have no valid users.") + return + } + + var b strings.Builder + for block, users := range r.Blocks { + if _, ok := r.DiscardedBlocks[block]; ok { + // skip block + continue + } + + // looping as we want to can get these out of order (meaning the + and -) + var mainUser ConflictingUser + for _, u := range users { + if u.Direction == "+" { + mainUser = u + break + } + } + b.WriteString("Keep the following user.\n") + b.WriteString(fmt.Sprintf("%s\n", block)) + b.WriteString(fmt.Sprintf("id: %s, email: %s, login: %s\n", mainUser.ID, mainUser.Email, mainUser.Login)) + b.WriteString("\n\n") + b.WriteString("The following user(s) will be deleted.\n") + for _, user := range users { + if user.ID == mainUser.ID { + continue + } + // mergeable users + b.WriteString(fmt.Sprintf("id: %s, email: %s, login: %s\n", user.ID, user.Email, user.Login)) + } + b.WriteString("\n\n") + } + logger.Info("\n\nChanges that will take place\n\n") + logger.Infof(b.String()) +} + // Formatter make it possible for us to write to terminal and to a file // with different formats depending on the usecase type Formatter func(format string, a ...interface{}) string -func BoldFormatter(format string, a ...interface{}) string { - white := color.New(color.FgWhite) - whiteBold := white.Add(color.Bold) - return whiteBold.Sprintf(format, a...) -} - func shouldDiscardBlock(seenUsersInBlock map[string]string, block string, user ConflictingUser) bool { // loop through users to see if we should skip this block // we have some more tricky scenarios where we have more than two users that can have conflicts with each other // we have made the approach to discard any users that we have seen - if _, ok := seenUsersInBlock[user.Id]; ok { + if _, ok := seenUsersInBlock[user.ID]; ok { // we have seen the user in different block than the current block - if seenUsersInBlock[user.Id] != block { + if seenUsersInBlock[user.ID] != block { return true } } - seenUsersInBlock[user.Id] = block + seenUsersInBlock[user.ID] = block return false } -func (r *ConflictResolver) BuildConflictBlocks(f Formatter) { +// BuildConflictBlocks builds blocks of users where each block is a unique email/login +// NOTE: currently this function assumes that the users are in order of grouping already +func (r *ConflictResolver) BuildConflictBlocks(users ConflictingUsers, f Formatter) { discardedBlocks := make(map[string]bool) seenUsersToBlock := make(map[string]string) blocks := make(map[string]ConflictingUsers) - for _, user := range r.Users { + for _, user := range users { // conflict blocks is how we identify a conflict in the user base. var conflictBlock string if user.ConflictEmail != "" { @@ -165,7 +443,7 @@ func (r *ConflictResolver) BuildConflictBlocks(f Formatter) { func contains(cu ConflictingUsers, target ConflictingUser) bool { for _, u := range cu { - if u.Id == target.Id { + if u.ID == target.ID { return true } } @@ -176,7 +454,7 @@ func (r *ConflictResolver) logDiscardedUsers() { keys := make([]string, 0, len(r.DiscardedBlocks)) for block := range r.DiscardedBlocks { for _, u := range r.Blocks[block] { - keys = append(keys, u.Id) + keys = append(keys, u.ID) } } warn := color.YellowString("Note: We discarded some conflicts that have multiple conflicting types involved.") @@ -208,7 +486,7 @@ func (r *ConflictResolver) ToStringPresentation() string { - id: 3, email: hej@TEST.com, login: hej@TEST.com */ startOfBlock := make(map[string]bool) - fileString := "" + var b strings.Builder for block, users := range r.Blocks { if _, ok := r.DiscardedBlocks[block]; ok { // skip block @@ -216,76 +494,105 @@ func (r *ConflictResolver) ToStringPresentation() string { } for _, user := range users { if !startOfBlock[block] { - fileString += fmt.Sprintf("%s\n", block) + b.WriteString(fmt.Sprintf("%s\n", block)) startOfBlock[block] = true - fileString += fmt.Sprintf("+ id: %s, email: %s, login: %s\n", user.Id, user.Email, user.Login) + b.WriteString(fmt.Sprintf("+ id: %s, email: %s, login: %s, last_seen_at: %s, auth_module: %s, conflict_email: %s, conflict_login: %s\n", + user.ID, + user.Email, + user.Login, + user.LastSeenAt, + user.AuthModule, + user.ConflictEmail, + user.ConflictLogin, + )) continue } - // mergable users - fileString += fmt.Sprintf("- id: %s, email: %s, login: %s\n", user.Id, user.Email, user.Login) + // mergeable users + b.WriteString(fmt.Sprintf("- id: %s, email: %s, login: %s, last_seen_at: %s, auth_module: %s, conflict_email: %s, conflict_login: %s\n", + user.ID, + user.Email, + user.Login, + user.LastSeenAt, + user.AuthModule, + user.ConflictEmail, + user.ConflictLogin, + )) } } - return fileString + return b.String() } type ConflictResolver struct { + Store *sqlstore.SQLStore + Config *setting.Cfg Users ConflictingUsers + ValidUsers ConflictingUsers Blocks map[string]ConflictingUsers DiscardedBlocks map[string]bool } type ConflictingUser struct { - // IDENTIFIER - // TODO: should have conflict block in sql for performance and stability - Direction string `xorm:"direction"` - // FIXME: refactor change to correct type int64 - Id string `xorm:"id"` - Email string `xorm:"email"` - Login string `xorm:"login"` - // FIXME: refactor change to correct type <> - LastSeenAt string `xorm:"last_seen_at"` - AuthModule string `xorm:"auth_module"` - // currently not really used for anything + // direction is the +/- which indicates if we should keep or delete the user + Direction string `xorm:"direction"` + ID string `xorm:"id"` + Email string `xorm:"email"` + Login string `xorm:"login"` + LastSeenAt string `xorm:"last_seen_at"` + AuthModule string `xorm:"auth_module"` ConflictEmail string `xorm:"conflict_email"` ConflictLogin string `xorm:"conflict_login"` } -// always better to have a slice of the object -// not a pointer for slice type ConflictingUsers []*ConflictingUser type ConflictingUsers []ConflictingUser func (c *ConflictingUser) Marshal(filerow string) error { - // +/- id: 1, email: hej, + // example view of the file to ingest + // +/- id: 1, email: hej, auth_module: LDAP trimmedSpaces := strings.ReplaceAll(filerow, " ", "") if trimmedSpaces[0] == '+' { c.Direction = "+" } else if trimmedSpaces[0] == '-' { c.Direction = "-" } else { - return fmt.Errorf("unable to get which operation the user would receive") + return fmt.Errorf("unable to get which operation was chosen") } trimmed := strings.TrimLeft(trimmedSpaces, "+-") values := strings.Split(trimmed, ",") - if len(values) != 5 { - // fmt errror - return fmt.Errorf("expected 5 values in entryrow") + + if len(values) < 3 { + return fmt.Errorf("expected at least 3 values in entry row") } + // expected fields id := strings.Split(values[0], ":") email := strings.Split(values[1], ":") login := strings.Split(values[2], ":") + c.ID = id[1] + c.Email = email[1] + c.Login = login[1] + + // why trim values, 2022-08-20:19:17:12 lastSeenAt := strings.TrimPrefix(values[3], "last_seen_at:") authModule := strings.Split(values[4], ":") - // optional field if len(authModule) < 2 { c.AuthModule = "" } else { c.AuthModule = authModule[1] } - // expected fields - c.Id = id[1] - c.Email = email[1] - c.Login = login[1] c.LastSeenAt = lastSeenAt + + // which conflict + conflictEmail := strings.Split(values[5], ":") + conflictLogin := strings.Split(values[6], ":") + if len(conflictEmail) < 2 { + c.ConflictEmail = "" + } else { + c.ConflictEmail = conflictEmail[1] + } + if len(conflictLogin) < 2 { + c.ConflictLogin = "" + } else { + c.ConflictLogin = conflictLogin[1] + } return nil } @@ -306,6 +613,7 @@ func GetUsersWithConflictingEmailsOrLogins(ctx *cli.Context, s *sqlstore.SQLStor // sorts the users by their useridentification and ids func conflictingUserEntriesSQL(s *sqlstore.SQLStore) string { userDialect := db.DB.GetDialect(s).Quote("user") + sqlQuery := ` SELECT DISTINCT u1.id, @@ -314,12 +622,12 @@ func conflictingUserEntriesSQL(s *sqlstore.SQLStore) string { u1.last_seen_at, user_auth.auth_module, ( SELECT - 'conflict_email' + 'true' FROM ` + userDialect + ` WHERE (LOWER(u1.email) = LOWER(u2.email)) AND(u1.email != u2.email)) AS conflict_email, ( SELECT - 'conflict_login' + 'true' FROM ` + userDialect + ` WHERE (LOWER(u1.login) = LOWER(u2.login) AND(u1.login != u2.login))) AS conflict_login @@ -337,3 +645,21 @@ func notServiceAccount(ss *sqlstore.SQLStore) string { return fmt.Sprintf("is_service_account = %s", ss.Dialect.BooleanStr(false)) } + +// confirm function asks for user input +// returns bool +func confirm(confirmPrompt string) bool { + var input string + logger.Infof("%s? [y|n]: ", confirmPrompt) + + _, err := fmt.Scanln(&input) + if err != nil { + logger.Infof("could not parse input from user for confirmation") + return false + } + input = strings.ToLower(input) + if input == "y" || input == "yes" { + return true + } + return false +} diff --git a/pkg/cmd/grafana-cli/commands/conflict_user_command_test.go b/pkg/cmd/grafana-cli/commands/conflict_user_command_test.go index d59501717b2..6e1883d9da5 100644 --- a/pkg/cmd/grafana-cli/commands/conflict_user_command_test.go +++ b/pkg/cmd/grafana-cli/commands/conflict_user_command_test.go @@ -3,14 +3,250 @@ package commands import ( "context" "fmt" + "os" + "sort" + "testing" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/team/teamimpl" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" "github.com/stretchr/testify/require" "github.com/urfave/cli/v2" ) +// "Skipping conflicting users test for mysql as it does make unique constraint case insensitive by default +const ignoredDatabase = "mysql" + +func TestBuildConflictBlock(t *testing.T) { + type testBuildConflictBlock struct { + desc string + users []user.User + expectedBlock string + wantDiscardedBlock string + wantConflictUser *ConflictingUser + wantedNumberOfUsers int + } + testOrgID := 1 + testCases := []testBuildConflictBlock{ + { + desc: "should get one block with only 3 users", + users: []user.User{ + { + Email: "ldap-editor", + Login: "ldap-editor", + OrgID: int64(testOrgID), + }, + { + Email: "LDAP-EDITOR", + Login: "LDAP-EDITOR", + OrgID: int64(testOrgID), + }, + { + Email: "overlapping conflict", + Login: "LDAP-editor", + OrgID: int64(testOrgID), + }, + { + Email: "OVERLAPPING conflict", + Login: "no conflict", + OrgID: int64(testOrgID), + }, + }, + expectedBlock: "conflict: ldap-editor", + wantDiscardedBlock: "conflict: overlapping conflict", + wantedNumberOfUsers: 3, + }, + { + desc: "should get conflict_email true and conflict_login empty string", + users: []user.User{ + { + Email: "conflict@email", + Login: "login", + OrgID: int64(testOrgID), + }, + { + Email: "conflict@EMAIL", + Login: "plainlogin", + OrgID: int64(testOrgID), + }, + }, + expectedBlock: "conflict: conflict@email", + wantedNumberOfUsers: 2, + wantConflictUser: &ConflictingUser{ConflictEmail: "true", ConflictLogin: ""}, + }, + { + desc: "should get conflict_email empty string and conflict_login true", + users: []user.User{ + { + Email: "regular@email", + Login: "CONFLICTLOGIN", + OrgID: int64(testOrgID), + }, + { + Email: "regular-no-conflict@email", + Login: "conflictlogin", + OrgID: int64(testOrgID), + }, + }, + expectedBlock: "conflict: conflictlogin", + wantedNumberOfUsers: 2, + wantConflictUser: &ConflictingUser{ConflictEmail: "", ConflictLogin: "true"}, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + // Restore after destructive operation + sqlStore := sqlstore.InitTestDB(t) + + if sqlStore.GetDialect().DriverName() != ignoredDatabase { + for _, u := range tc.users { + cmd := user.CreateUserCommand{ + Email: u.Email, + Name: u.Name, + Login: u.Login, + OrgID: int64(testOrgID), + } + _, err := sqlStore.CreateUser(context.Background(), cmd) + require.NoError(t, err) + } + m, err := GetUsersWithConflictingEmailsOrLogins(&cli.Context{Context: context.Background()}, sqlStore) + require.NoError(t, err) + r := ConflictResolver{Store: sqlStore} + r.BuildConflictBlocks(m, fmt.Sprintf) + require.Equal(t, tc.wantedNumberOfUsers, len(r.Blocks[tc.expectedBlock])) + if tc.wantDiscardedBlock != "" { + require.Equal(t, true, r.DiscardedBlocks[tc.wantDiscardedBlock]) + } + if tc.wantConflictUser != nil { + for _, u := range m { + require.Equal(t, tc.wantConflictUser.ConflictEmail, u.ConflictEmail) + require.Equal(t, tc.wantConflictUser.ConflictLogin, u.ConflictLogin) + } + } + } + }) + } +} + +func TestBuildConflictBlockFromFileRepresentation(t *testing.T) { + type testBuildConflictBlock struct { + desc string + users []user.User + fileString string + expectedBlocks []string + expectedIdsInBlocks map[string][]string + } + testOrgID := 1 + testCases := []testBuildConflictBlock{ + { + desc: "should be able to parse the fileString containing the conflicts", + users: []user.User{ + { + Email: "test", + Login: "test", + OrgID: int64(testOrgID), + }, + { + Email: "TEST", + Login: "TEST", + OrgID: int64(testOrgID), + }, + { + Email: "test2", + Login: "test2", + OrgID: int64(testOrgID), + }, + { + Email: "TEST2", + Login: "TEST2", + OrgID: int64(testOrgID), + }, + { + Email: "Test2", + Login: "Test2", + OrgID: int64(testOrgID), + }, + }, + fileString: `conflict: test +- id: 2, email: test, login: test, last_seen_at: 2012-09-19T08:31:20Z, auth_module: , conflict_email: true, conflict_login: true ++ id: 3, email: TEST, login: TEST, last_seen_at: 2012-09-19T08:31:29Z, auth_module: , conflict_email: true, conflict_login: true +conflict: test2 +- id: 4, email: test2, login: test2, last_seen_at: 2012-09-19T08:31:41Z, auth_module: , conflict_email: true, conflict_login: true ++ id: 5, email: TEST2, login: TEST2, last_seen_at: 2012-09-19T08:31:51Z, auth_module: , conflict_email: true, conflict_login: true +- id: 6, email: Test2, login: Test2, last_seen_at: 2012-09-19T08:32:03Z, auth_module: , conflict_email: true, conflict_login: true`, + expectedBlocks: []string{"conflict: test", "conflict: test2"}, + expectedIdsInBlocks: map[string][]string{"conflict: test": {"2", "3"}, "conflict: test2": {"4", "5", "6"}}, + }, + { + desc: "should be able to parse the fileString containing the conflicts 123", + users: []user.User{ + { + Email: "saml-misi@example.org", + Login: "saml-misi", + OrgID: int64(testOrgID), + }, + { + Email: "saml-misi@example", + Login: "saml-Misi", + OrgID: int64(testOrgID), + }, + }, + fileString: `conflict: saml-misi ++ id: 5, email: saml-misi@example.org, login: saml-misi, last_seen_at: 2022-09-22T12:00:49Z, auth_module: auth.saml, conflict_email: , conflict_login: true +- id: 15, email: saml-misi@example, login: saml-Misi, last_seen_at: 2012-09-26T11:31:32Z, auth_module: , conflict_email: , conflict_login: true`, + expectedBlocks: []string{"conflict: saml-misi"}, + expectedIdsInBlocks: map[string][]string{"conflict: saml-misi": {"5", "15"}}, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + // Restore after destructive operation + sqlStore := sqlstore.InitTestDB(t) + + if sqlStore.GetDialect().DriverName() != ignoredDatabase { + for _, u := range tc.users { + cmd := user.CreateUserCommand{ + Email: u.Email, + Name: u.Name, + Login: u.Login, + OrgID: int64(testOrgID), + } + _, err := sqlStore.CreateUser(context.Background(), cmd) + require.NoError(t, err) + } + + conflicts, err := GetUsersWithConflictingEmailsOrLogins(&cli.Context{Context: context.Background()}, sqlStore) + r := ConflictResolver{Users: conflicts, Store: sqlStore} + r.BuildConflictBlocks(conflicts, fmt.Sprintf) + require.NoError(t, err) + validErr := getValidConflictUsers(&r, []byte(tc.fileString)) + require.NoError(t, validErr) + + // test starts here + keys := make([]string, 0) + for k := range r.Blocks { + keys = append(keys, k) + } + sort.Strings(keys) + require.Equal(t, tc.expectedBlocks, keys) + + // we want to validate the ids in the blocks + for _, block := range tc.expectedBlocks { + // checking for parsing of ids + conflictIds := []string{} + for _, u := range r.Blocks[block] { + conflictIds = append(conflictIds, u.ID) + } + require.Equal(t, tc.expectedIdsInBlocks[block], conflictIds) + } + } + }) + } +} func TestGetConflictingUsers(t *testing.T) { type testListConflictingUsers struct { desc string @@ -52,9 +288,6 @@ func TestGetConflictingUsers(t *testing.T) { }, want: 2, }, - // TODO: - // refactor the sql to get 3 users from this test - // if this is changed, one needs to correct the filerepresentation { desc: "should be 5 conflicting users, each conflict gets 2 users", users: []user.User{ @@ -151,8 +384,7 @@ func TestGetConflictingUsers(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { // Restore after destructive operation sqlStore := sqlstore.InitTestDB(t) - // "Skipping conflicting users test for mysql as it does make unique constraint case insensitive by default - if sqlStore.GetDialect().DriverName() != "mysql" { + if sqlStore.GetDialect().DriverName() != ignoredDatabase { for _, u := range tc.users { cmd := user.CreateUserCommand{ Email: u.Email, @@ -175,82 +407,16 @@ func TestGetConflictingUsers(t *testing.T) { } } -func TestBuildConflictBlock(t *testing.T) { - type testBuildConflictBlock struct { - desc string - users []user.User - expectedBlock string - wantDiscardedBlock string - wantedNumberOfUsers int - } - testOrgID := 1 - testCases := []testBuildConflictBlock{ - { - desc: "should get one block with only 3 users", - users: []user.User{ - { - Email: "ldap-editor", - Login: "ldap-editor", - OrgID: int64(testOrgID), - }, - { - Email: "LDAP-EDITOR", - Login: "LDAP-EDITOR", - OrgID: int64(testOrgID), - }, - { - Email: "overlapping conflict", - Login: "LDAP-editor", - OrgID: int64(testOrgID), - }, - { - Email: "OVERLAPPING conflict", - Login: "no conflict", - OrgID: int64(testOrgID), - }, - }, - expectedBlock: "conflict: ldap-editor", - wantDiscardedBlock: "conflict: overlapping conflict", - wantedNumberOfUsers: 3, - }, - } - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - // Restore after destructive operation - sqlStore := sqlstore.InitTestDB(t) - - // "Skipping conflicting users test for mysql as it does make unique constraint case insensitive by default - if sqlStore.GetDialect().DriverName() != "mysql" { - for _, u := range tc.users { - cmd := user.CreateUserCommand{ - Email: u.Email, - Name: u.Name, - Login: u.Login, - OrgID: int64(testOrgID), - } - _, err := sqlStore.CreateUser(context.Background(), cmd) - require.NoError(t, err) - } - m, err := GetUsersWithConflictingEmailsOrLogins(&cli.Context{Context: context.Background()}, sqlStore) - require.NoError(t, err) - r := ConflictResolver{Users: m} - r.BuildConflictBlocks(fmt.Sprintf) - require.Equal(t, tc.wantedNumberOfUsers, len(r.Blocks[tc.expectedBlock])) - require.Equal(t, true, r.DiscardedBlocks[tc.wantDiscardedBlock]) - } - }) - } -} - func TestGenerateConflictingUsersFile(t *testing.T) { - type testListConflictingUsers struct { - desc string - users []user.User - wantDiscardedBlock string - want string + type testGenerateConflictUsers struct { + desc string + users []user.User + expectedDiscardedBlock string + expectedBlocks []string + expectedEmailInBlocks map[string][]string } testOrgID := 1 - testCases := []testListConflictingUsers{ + testCases := []testGenerateConflictUsers{ { desc: "should get conflicting users", users: []user.User{ @@ -290,10 +456,17 @@ func TestGenerateConflictingUsersFile(t *testing.T) { OrgID: int64(testOrgID), }, }, - wantDiscardedBlock: "conflict: user2", + expectedBlocks: []string{"conflict: ldap-admin", "conflict: user_duplicate_test_login", "conflict: oauth-admin@example.org", "conflict: user2"}, + expectedEmailInBlocks: map[string][]string{ + "conflict: ldap-admin": {"ldap-admin", "xo"}, + "conflict: user_duplicate_test_login": {"user1", "user2"}, + "conflict: oauth-admin@example.org": {"oauth-admin@EXAMPLE.ORG", "oauth-admin@example.org"}, + "conflict: user2": {"USER2", "user2"}, + }, + expectedDiscardedBlock: "conflict: user2", }, { - desc: "should get one block with only 3 users", + desc: "should get only one block with 3 users", users: []user.User{ { Email: "ldap-editor", @@ -311,19 +484,15 @@ func TestGenerateConflictingUsersFile(t *testing.T) { OrgID: int64(testOrgID), }, }, - want: `conflict: ldap-editor -+ id: 1, email: ldap-editor, login: ldap-editor -- id: 2, email: LDAP-EDITOR, login: LDAP-EDITOR -- id: 3, email: No confli, login: LDAP-editor -`, + expectedBlocks: []string{"conflict: ldap-editor"}, + expectedEmailInBlocks: map[string][]string{"conflict: ldap-editor": {"ldap-editor", "LDAP-EDITOR", "No confli"}}, }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { // Restore after destructive operation sqlStore := sqlstore.InitTestDB(t) - // "Skipping conflicting users test for mysql as it does make unique constraint case insensitive by default - if sqlStore.GetDialect().DriverName() != "mysql" { + if sqlStore.GetDialect().DriverName() != ignoredDatabase { for _, u := range tc.users { cmd := user.CreateUserCommand{ Email: u.Email, @@ -336,49 +505,280 @@ func TestGenerateConflictingUsersFile(t *testing.T) { } m, err := GetUsersWithConflictingEmailsOrLogins(&cli.Context{Context: context.Background()}, sqlStore) require.NoError(t, err) - r := ConflictResolver{Users: m} - r.BuildConflictBlocks(fmt.Sprintf) - if tc.wantDiscardedBlock != "" { - require.Equal(t, true, r.DiscardedBlocks[tc.wantDiscardedBlock]) + r := ConflictResolver{Store: sqlStore} + r.BuildConflictBlocks(m, fmt.Sprintf) + if tc.expectedDiscardedBlock != "" { + require.Equal(t, true, r.DiscardedBlocks[tc.expectedDiscardedBlock]) } - if tc.want != "" { - fileString := r.ToStringPresentation() - require.Equal(t, tc.want, fileString) + + // test starts here + keys := make([]string, 0) + for k := range r.Blocks { + keys = append(keys, k) + } + expectedBlocks := tc.expectedBlocks + sort.Strings(keys) + sort.Strings(expectedBlocks) + require.Equal(t, expectedBlocks, keys) + + // we want to validate the ids in the blocks + for _, block := range tc.expectedBlocks { + // checking for parsing of ids + conflictEmails := []string{} + for _, u := range r.Blocks[block] { + conflictEmails = append(conflictEmails, u.Email) + } + expectedEmailsInBlock := tc.expectedEmailInBlocks[block] + sort.Strings(conflictEmails) + sort.Strings(expectedEmailsInBlock) + require.Equal(t, expectedEmailsInBlock, conflictEmails) } } }) } } +func TestRunValidateConflictUserFile(t *testing.T) { + t.Run("should validate file thats gets created", func(t *testing.T) { + // Restore after destructive operation + sqlStore := sqlstore.InitTestDB(t) + const testOrgID int64 = 1 + if sqlStore.GetDialect().DriverName() != ignoredDatabase { + // add additional user with conflicting login where DOMAIN is upper case + dupUserLogincmd := user.CreateUserCommand{ + Email: "userduplicatetest1@test.com", + Login: "user_duplicate_test_1_login", + OrgID: testOrgID, + } + _, err := sqlStore.CreateUser(context.Background(), dupUserLogincmd) + require.NoError(t, err) + dupUserEmailcmd := user.CreateUserCommand{ + Email: "USERDUPLICATETEST1@TEST.COM", + Login: "USER_DUPLICATE_TEST_1_LOGIN", + OrgID: testOrgID, + } + _, err = sqlStore.CreateUser(context.Background(), dupUserEmailcmd) + require.NoError(t, err) + + // get users + conflictUsers, err := GetUsersWithConflictingEmailsOrLogins(&cli.Context{Context: context.Background()}, sqlStore) + require.NoError(t, err) + r := ConflictResolver{Store: sqlStore} + r.BuildConflictBlocks(conflictUsers, fmt.Sprintf) + tmpFile, err := generateConflictUsersFile(&r) + require.NoError(t, err) + + b, err := os.ReadFile(tmpFile.Name()) + require.NoError(t, err) + + validErr := getValidConflictUsers(&r, b) + require.NoError(t, validErr) + require.Equal(t, 2, len(r.ValidUsers)) + } + }) +} + +func TestMergeUser(t *testing.T) { + t.Run("should be able to merge user", func(t *testing.T) { + // Restore after destructive operation + sqlStore := sqlstore.InitTestDB(t) + teamSvc := teamimpl.ProvideService(sqlStore, setting.NewCfg()) + team1, err := teamSvc.CreateTeam("team1 name", "", 1) + require.Nil(t, err) + const testOrgID int64 = 1 + + if sqlStore.GetDialect().DriverName() != ignoredDatabase { + // add additional user with conflicting login where DOMAIN is upper case + + // the order of adding the conflict matters + dupUserLogincmd := user.CreateUserCommand{ + Email: "userduplicatetest1@test.com", + Name: "user name 1", + Login: "user_duplicate_test_1_login", + OrgID: testOrgID, + } + _, err := sqlStore.CreateUser(context.Background(), dupUserLogincmd) + require.NoError(t, err) + dupUserEmailcmd := user.CreateUserCommand{ + Email: "USERDUPLICATETEST1@TEST.COM", + Name: "user name 1", + Login: "USER_DUPLICATE_TEST_1_LOGIN", + OrgID: testOrgID, + } + userWithUpperCase, err := sqlStore.CreateUser(context.Background(), dupUserEmailcmd) + require.NoError(t, err) + // this is the user we want to update to another team + err = teamSvc.AddTeamMember(userWithUpperCase.ID, testOrgID, team1.Id, false, 0) + require.NoError(t, err) + + // get users + conflictUsers, err := GetUsersWithConflictingEmailsOrLogins(&cli.Context{Context: context.Background()}, sqlStore) + require.NoError(t, err) + r := ConflictResolver{Store: sqlStore} + r.BuildConflictBlocks(conflictUsers, fmt.Sprintf) + tmpFile, err := generateConflictUsersFile(&r) + require.NoError(t, err) + // validation to get newConflicts + // edited file + b, err := os.ReadFile(tmpFile.Name()) + require.NoError(t, err) + validErr := getValidConflictUsers(&r, b) + require.NoError(t, validErr) + require.Equal(t, 2, len(r.ValidUsers)) + + // test starts here + err = r.MergeConflictingUsers(context.Background()) + require.NoError(t, err) + + // user with uppercaseemail should not exist + query := &models.GetUserByIdQuery{Id: userWithUpperCase.ID} + err = sqlStore.GetUserById(context.Background(), query) + require.Error(t, user.ErrUserNotFound, err) + } + }) +} + +func TestMergeUserFromNewFileInput(t *testing.T) { + t.Run("should be able to merge users after choosing a different user to keep", func(t *testing.T) { + // Restore after destructive operation + sqlStore := sqlstore.InitTestDB(t) + + type testBuildConflictBlock struct { + desc string + users []user.User + fileString string + expectedBlocks []string + expectedIdsInBlocks map[string][]string + } + testOrgID := 1 + m := make(map[string][]string) + conflict1 := "conflict: test" + conflict2 := "conflict: test2" + m[conflict1] = []string{"2", "3"} + m[conflict2] = []string{"4", "5", "6"} + testCases := []testBuildConflictBlock{ + { + desc: "should be able to parse the fileString containing the conflicts", + users: []user.User{ + { + Email: "TEST", + Login: "TEST", + OrgID: int64(testOrgID), + }, + { + Email: "test", + Login: "test", + OrgID: int64(testOrgID), + }, + { + Email: "test2", + Login: "test2", + OrgID: int64(testOrgID), + }, + { + Email: "TEST2", + Login: "TEST2", + OrgID: int64(testOrgID), + }, + { + Email: "Test2", + Login: "Test2", + OrgID: int64(testOrgID), + }, + }, + fileString: `conflict: test +- id: 1, email: test, login: test, last_seen_at: 2012-09-19T08:31:20Z, auth_module:, conflict_email: true, conflict_login: true ++ id: 2, email: TEST, login: TEST, last_seen_at: 2012-09-19T08:31:29Z, auth_module:, conflict_email: true, conflict_login: true +conflict: test2 +- id: 3, email: test2, login: test2, last_seen_at: 2012-09-19T08:31:41Z, auth_module: , conflict_email: true, conflict_login: true ++ id: 4, email: TEST2, login: TEST2, last_seen_at: 2012-09-19T08:31:51Z, auth_module: , conflict_email: true, conflict_login: true +- id: 5, email: Test2, login: Test2, last_seen_at: 2012-09-19T08:32:03Z, auth_module: , conflict_email: true, conflict_login: true`, + expectedBlocks: []string{"conflict: test", "conflict: test2"}, + expectedIdsInBlocks: m, + }, + } + for _, tc := range testCases { + if sqlStore.GetDialect().DriverName() != ignoredDatabase { + for _, u := range tc.users { + cmd := user.CreateUserCommand{ + Email: u.Email, + Name: u.Name, + Login: u.Login, + OrgID: int64(testOrgID), + } + _, err := sqlStore.CreateUser(context.Background(), cmd) + require.NoError(t, err) + } + // 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} + r.BuildConflictBlocks(conflictUsers, fmt.Sprintf) + require.NoError(t, err) + // validation to get newConflicts + // edited file + // b, err := os.ReadFile(tmpFile.Name()) + // mocked file input + b := tc.fileString + require.NoError(t, err) + validErr := getValidConflictUsers(&r, []byte(b)) + require.NoError(t, validErr) + + // test starts here + err = r.MergeConflictingUsers(context.Background()) + require.NoError(t, err) + } + } + }) +} + func TestMarshalConflictUser(t *testing.T) { - // TODO: add more testcases testCases := []struct { name string inputRow string expectedUser ConflictingUser - }{{ - name: "should be able to marshal expected input row", - inputRow: "+ id: 4, email: userduplicatetest1@test.com, login: userduplicatetest1@test.com, last_seen_at: 2012-07-26T16:08:11Z, auth_module:", - expectedUser: ConflictingUser{ - Direction: "+", - Id: "4", - Email: "userduplicatetest1@test.com", - Login: "userduplicatetest1@test.com", - LastSeenAt: "2012-07-26T16:08:11Z", - AuthModule: "", + }{ + { + name: "should be able to marshal expected input row", + inputRow: "+ id: 4, email: userduplicatetest1@test.com, login: userduplicatetest1, last_seen_at: 2012-07-26T16:08:11Z, auth_module: auth.saml, conflict_email: true, conflict_login: ", + expectedUser: ConflictingUser{ + Direction: "+", + ID: "4", + Email: "userduplicatetest1@test.com", + Login: "userduplicatetest1", + LastSeenAt: "2012-07-26T16:08:11Z", + AuthModule: "auth.saml", + ConflictEmail: "true", + ConflictLogin: "", + }, }, - }} - + { + name: "should be able to marshal expected input row", + inputRow: "+ id: 1, email: userduplicatetest1@test.com, login: user_duplicate_test_1_login, last_seen_at: 2012-07-26T16:08:11Z, auth_module: , conflict_email: , conflict_login: true", + expectedUser: ConflictingUser{ + Direction: "+", + ID: "1", + Email: "userduplicatetest1@test.com", + Login: "user_duplicate_test_1_login", + LastSeenAt: "2012-07-26T16:08:11Z", + AuthModule: "", + ConflictEmail: "", + ConflictLogin: "true", + }, + }, + } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { user := ConflictingUser{} err := user.Marshal(tc.inputRow) require.NoError(t, err) require.Equal(t, tc.expectedUser.Direction, user.Direction) - require.Equal(t, tc.expectedUser.Id, user.Id) + require.Equal(t, tc.expectedUser.ID, user.ID) require.Equal(t, tc.expectedUser.Email, user.Email) require.Equal(t, tc.expectedUser.Login, user.Login) require.Equal(t, tc.expectedUser.LastSeenAt, user.LastSeenAt) + require.Equal(t, tc.expectedUser.ConflictEmail, user.ConflictEmail) + require.Equal(t, tc.expectedUser.ConflictLogin, user.ConflictLogin) }) } } diff --git a/pkg/services/accesscontrol/database/database.go b/pkg/services/accesscontrol/database/database.go index 7856232ed33..dcf967751ad 100644 --- a/pkg/services/accesscontrol/database/database.go +++ b/pkg/services/accesscontrol/database/database.go @@ -49,7 +49,9 @@ func (s *AccessControlStore) GetUserPermissions(ctx context.Context, query acces params = append(params, a) } } - + q += ` + ORDER BY permission.scope + ` if err := sess.SQL(q, params...).Find(&result); err != nil { return err } diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index 1e154b07610..34de541d591 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -14,7 +14,7 @@ func (ss *SQLStore) AddOrgUser(ctx context.Context, cmd *models.AddOrgUserComman var usr user.User session := sess.ID(cmd.UserId) if !cmd.AllowAddingServiceAccount { - session = session.Where(notServiceAccountFilter(ss)) + session = session.Where(NotServiceAccountFilter(ss)) } if exists, err := session.Get(&usr); err != nil { diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index a683e2a13ce..71090e630ee 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -169,7 +169,7 @@ func (ss *SQLStore) CreateUser(ctx context.Context, cmd user.CreateUserCommand) return &user, createErr } -func notServiceAccountFilter(ss *SQLStore) string { +func NotServiceAccountFilter(ss *SQLStore) string { return fmt.Sprintf("%s.is_service_account = %s", ss.Dialect.Quote("user"), ss.Dialect.BooleanStr(false)) @@ -180,7 +180,7 @@ func (ss *SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQu usr := new(user.User) has, err := sess.ID(query.Id). - Where(notServiceAccountFilter(ss)). + Where(NotServiceAccountFilter(ss)). Get(usr) if err != nil { @@ -235,7 +235,7 @@ func setUsingOrgInTransaction(sess *DBSession, userID int64, orgID int64) error func (ss *SQLStore) GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error { return ss.WithDbSession(ctx, func(sess *DBSession) error { var usr user.User - has, err := sess.ID(query.UserId).Where(notServiceAccountFilter(ss)).Get(&usr) + has, err := sess.ID(query.UserId).Where(NotServiceAccountFilter(ss)).Get(&usr) if err != nil { return err @@ -288,7 +288,7 @@ func (ss *SQLStore) GetUserOrgList(ctx context.Context, query *models.GetUserOrg sess.Join("INNER", "org", "org_user.org_id=org.id") sess.Join("INNER", ss.Dialect.Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", ss.Dialect.Quote("user"))) sess.Where("org_user.user_id=?", query.UserId) - sess.Where(notServiceAccountFilter(ss)) + sess.Where(NotServiceAccountFilter(ss)) sess.Cols("org.name", "org_user.role", "org_user.org_id") sess.OrderBy("org.name") err := sess.Find(&query.Result) @@ -581,7 +581,7 @@ func (ss *SQLStore) DisableUser(ctx context.Context, cmd *models.DisableUserComm usr := user.User{} sess := dbSess.Table("user") - if has, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&usr); err != nil { + if has, err := sess.ID(cmd.UserId).Where(NotServiceAccountFilter(ss)).Get(&usr); err != nil { return err } else if !has { return user.ErrUserNotFound @@ -611,7 +611,7 @@ func (ss *SQLStore) BatchDisableUsers(ctx context.Context, cmd *models.BatchDisa disableParams = append(disableParams, v) } - _, err := sess.Where(notServiceAccountFilter(ss)).Exec(disableParams...) + _, err := sess.Where(NotServiceAccountFilter(ss)).Exec(disableParams...) return err }) } @@ -622,10 +622,14 @@ func (ss *SQLStore) DeleteUser(ctx context.Context, cmd *models.DeleteUserComman }) } +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) + has, err := sess.Where(NotServiceAccountFilter(ss)).Get(&usr) if err != nil { return err } @@ -701,23 +705,20 @@ func UserDeletions() []string { func (ss *SQLStore) UpdateUserPermissions(userID int64, isAdmin bool) error { return ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error { var user user.User - if _, err := sess.ID(userID).Where(notServiceAccountFilter(ss)).Get(&user); err != nil { + if _, err := sess.ID(userID).Where(NotServiceAccountFilter(ss)).Get(&user); err != nil { return err } user.IsAdmin = isAdmin sess.UseBool("is_admin") - _, err := sess.ID(user.ID).Update(&user) if err != nil { return err } - // validate that after update there is at least one server admin if err := validateOneAdminLeft(sess); err != nil { return err } - return nil }) }