diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 960d592136c..e10f15c5150 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -49,7 +49,7 @@ import ( "github.com/grafana/grafana/pkg/setting" ) -const newEmail = "newEmail@localhost" +const newEmail = "newemail@localhost" func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { settings := setting.NewCfg() @@ -650,7 +650,7 @@ func TestUser_UpdateEmail(t *testing.T) { require.False(t, nsMock.EmailVerified) // Start email update - newName := "newName" + newName := "newname" body := fmt.Sprintf(`{"email": "%s", "name": "%s"}`, newEmail, newName) sendUpdateReq(server, originalUsr, body) @@ -752,8 +752,8 @@ func TestUser_UpdateEmail(t *testing.T) { require.False(t, nsMock.EmailVerified) // Start email update - newLogin := "newLogin" - newName := "newName" + newLogin := "newlogin" + newName := "newname" body := fmt.Sprintf(`{"login": "%s", "name": "%s"}`, newLogin, newName) sendUpdateReq(server, originalUsr, body) @@ -805,7 +805,7 @@ func TestUser_UpdateEmail(t *testing.T) { require.False(t, nsMock.EmailVerified) // Start email update - newLogin := "newEmail2@localhost" + newLogin := "newemail2@localhost" body := fmt.Sprintf(`{"email": "%s", "login": "%s"}`, newEmail, newLogin) sendUpdateReq(server, originalUsr, body) @@ -840,7 +840,7 @@ func TestUser_UpdateEmail(t *testing.T) { require.False(t, nsMock.EmailVerified) // Start email update - newLogin := "newEmail2@localhost" + newLogin := "newemail2@localhost" body := fmt.Sprintf(`{"email": "%s", "login": "%s"}`, newEmail, newLogin) sendUpdateReq(server, originalUsr, body) @@ -912,14 +912,14 @@ func TestUser_UpdateEmail(t *testing.T) { require.False(t, nsMock.EmailVerified) // First email verification - firstNewEmail := "newEmail1@localhost" + firstNewEmail := "newemail1@localhost" body := fmt.Sprintf(`{"email": "%s"}`, firstNewEmail) sendUpdateReq(server, originalUsr, body) verifyEmailData(tempUserSvc, nsMock, originalUsr, firstNewEmail) firstCode := nsMock.EmailVerification.Code // Second email verification - secondNewEmail := "newEmail2@localhost" + secondNewEmail := "newemail2@localhost" body = fmt.Sprintf(`{"email": "%s"}`, secondNewEmail) sendUpdateReq(server, originalUsr, body) verifyEmailData(tempUserSvc, nsMock, originalUsr, secondNewEmail) 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 0da42a55cbf..3d0f600dc88 100644 --- a/pkg/cmd/grafana-cli/commands/conflict_user_command_test.go +++ b/pkg/cmd/grafana-cli/commands/conflict_user_command_test.go @@ -13,11 +13,8 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" - "github.com/grafana/grafana/pkg/services/org/orgimpl" - "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" - "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" "github.com/grafana/grafana/pkg/services/team/teamimpl" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/userimpl" @@ -568,24 +565,50 @@ func TestRunValidateConflictUserFile(t *testing.T) { t.Run("should validate file thats gets created", func(t *testing.T) { // Restore after destructive operation sqlStore := db.InitTestDB(t) - usrSvc := setupTestUserService(t, sqlStore) 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 := usrSvc.Create(context.Background(), &dupUserLogincmd) - require.NoError(t, err) - dupUserEmailcmd := user.CreateUserCommand{ - Email: "USERDUPLICATETEST1@TEST.COM", - Login: "USER_DUPLICATE_TEST_1_LOGIN", - OrgID: testOrgID, - } - _, err = usrSvc.Create(context.Background(), &dupUserEmailcmd) + err := sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + // create a user + // 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, + } + rawSQL := fmt.Sprintf( + "INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,\"2024-03-18T15:25:32\",\"2024-03-18T15:25:32\")", + sqlStore.Quote("user"), + sqlStore.Dialect.BooleanStr(false), + ) + result, err := sess.Exec(rawSQL, dupUserLogincmd.Email, dupUserLogincmd.Login, dupUserLogincmd.OrgID) + if err != nil { + return err + } + n, err := result.RowsAffected() + if err != nil { + return err + } else if n == 0 { + return user.ErrUserNotFound + } + dupUserEmailcmd := user.CreateUserCommand{ + Email: "USERDUPLICATETEST1@TEST.COM", + Login: "USER_DUPLICATE_TEST_1_LOGIN", + OrgID: testOrgID, + } + result, err = sess.Exec(rawSQL, dupUserEmailcmd.Email, dupUserEmailcmd.Login, dupUserEmailcmd.OrgID) + if err != nil { + return err + } + n, err = result.RowsAffected() + if err != nil { + return err + } else if n == 0 { + return user.ErrUserNotFound + } + return nil + }) require.NoError(t, err) // get users @@ -617,31 +640,78 @@ func TestIntegrationMergeUser(t *testing.T) { require.NoError(t, err) team1, err := teamSvc.CreateTeam("team1 name", "", 1) require.NoError(t, err) - usrSvc := setupTestUserService(t, sqlStore) 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.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + // the order of adding the conflict matters + cmd := user.User{ + Email: "userduplicatetest1@test.com", + Name: "user name 1", + Login: "user_duplicate_test_1_login", + OrgID: testOrgID, + Created: time.Now(), + Updated: time.Now(), + } + + // call user store instead of user service so as not to prevent conflicting users + rawSQL := fmt.Sprintf( + "INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)", + sqlStore.Quote("user"), + sqlStore.Dialect.BooleanStr(false), + ) + result, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated) + if err != nil { + return err + } + n, err := result.RowsAffected() + if err != nil { + return err + } else if n == 0 { + return user.ErrUserNotFound + } + require.NoError(t, err) + return nil + }) + if err != nil { + t.Error(err) } - _, err := usrSvc.Create(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, + + err = sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + cmd := user.User{ + Email: "USERDUPLICATETEST1@TEST.COM", + Name: "user name 1", + Login: "USER_DUPLICATE_TEST_1_LOGIN", + OrgID: testOrgID, + Created: time.Now(), + Updated: time.Now(), + } + // call user store instead of user service so as not to prevent conflicting users + rawSQL := fmt.Sprintf( + "INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)", + sqlStore.Quote("user"), + sqlStore.Dialect.BooleanStr(false), + ) + result, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated) + if err != nil { + return err + } + n, err := result.RowsAffected() + if err != nil { + return err + } else if n == 0 { + return user.ErrUserNotFound + } + require.NoError(t, err) + return nil + }) + if err != nil { + t.Error(err) } - userWithUpperCase, err := usrSvc.Create(context.Background(), &dupUserEmailcmd) - require.NoError(t, err) // this is the user we want to update to another team - err = teamSvc.AddTeamMember(context.Background(), userWithUpperCase.ID, testOrgID, team1.ID, false, 0) + err = teamSvc.AddTeamMember(context.Background(), 1, testOrgID, team1.ID, false, 0) require.NoError(t, err) // get users @@ -776,20 +846,40 @@ conflict: test2 // Restore after destructive operation sqlStore := db.InitTestDB(t) if sqlStore.GetDialect().DriverName() != ignoredDatabase { - userStore := userimpl.ProvideStore(sqlStore, sqlStore.Cfg) - for _, u := range tc.users { - cmd := user.User{ - Email: u.Email, - Name: u.Name, - Login: u.Login, - OrgID: int64(testOrgID), - Created: time.Now(), - Updated: time.Now(), + err := sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + for _, u := range tc.users { + cmd := user.User{ + Email: u.Email, + Name: u.Name, + Login: u.Login, + OrgID: int64(testOrgID), + Created: time.Now(), + Updated: time.Now(), + } + // call user store instead of user service so as not to prevent conflicting users + rawSQL := fmt.Sprintf( + "INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)", + sqlStore.Quote("user"), + sqlStore.Dialect.BooleanStr(false), + ) + result, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated) + if err != nil { + return err + } + n, err := result.RowsAffected() + if err != nil { + return err + } else if n == 0 { + return user.ErrUserNotFound + } + require.NoError(t, err) } - // call user store instead of user service so as not to prevent conflicting users - _, err := userStore.Insert(context.Background(), &cmd) - require.NoError(t, err) + return nil + }) + if err != nil { + t.Fatal(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) @@ -873,13 +963,3 @@ func TestMarshalConflictUser(t *testing.T) { }) } } - -func setupTestUserService(t *testing.T, sqlStore *sqlstore.SQLStore) user.Service { - t.Helper() - orgSvc, err := orgimpl.ProvideService(sqlStore, sqlStore.Cfg, "atest.FakeQuotaService{}) - require.NoError(t, err) - usrSvc, err := userimpl.ProvideService(sqlStore, orgSvc, sqlStore.Cfg, nil, nil, "atest.FakeQuotaService{}, supportbundlestest.NewFakeBundleService()) - require.NoError(t, err) - - return usrSvc -} diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 169bbe7ee3f..1b2a9249bd3 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -50,12 +50,9 @@ func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args user.C args.Email = args.Login } - where := "email=? OR login=?" - if ss.Cfg.CaseInsensitiveLogin { - where = "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)" - args.Login = strings.ToLower(args.Login) - args.Email = strings.ToLower(args.Email) - } + where := "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)" + args.Login = strings.ToLower(args.Login) + args.Email = strings.ToLower(args.Email) exists, err := sess.Where(where, args.Email, args.Login).Get(&user.User{}) if err != nil { diff --git a/pkg/services/temp_user/tempuserimpl/store.go b/pkg/services/temp_user/tempuserimpl/store.go index 9affd894083..9fbef536b50 100644 --- a/pkg/services/temp_user/tempuserimpl/store.go +++ b/pkg/services/temp_user/tempuserimpl/store.go @@ -106,11 +106,7 @@ func (ss *xormStore) GetTempUsersQuery(ctx context.Context, query *tempuser.GetT } if query.Email != "" { - if ss.cfg.CaseInsensitiveLogin { - rawSQL += ` AND LOWER(tu.email)=LOWER(?)` - } else { - rawSQL += ` AND tu.email=?` - } + rawSQL += ` AND LOWER(tu.email)=LOWER(?)` params = append(params, query.Email) } diff --git a/pkg/services/temp_user/tempuserimpl/store_test.go b/pkg/services/temp_user/tempuserimpl/store_test.go index 6bcd65acd0e..491850afe51 100644 --- a/pkg/services/temp_user/tempuserimpl/store_test.go +++ b/pkg/services/temp_user/tempuserimpl/store_test.go @@ -61,30 +61,14 @@ func TestIntegrationTempUserCommandsAndQueries(t *testing.T) { require.Nil(t, err) require.Equal(t, 1, len(queryResult)) }) - t.Run("Should not be able to get temp users by case-insentive email - case sensitive", func(t *testing.T) { - if db.IsTestDbMySQL() { - t.Skip("MySQL is case insensitive by default") - } - setup(t) - store.cfg.CaseInsensitiveLogin = false - query := tempuser.GetTempUsersQuery{Email: "E@as.co", Status: tempuser.TmpUserInvitePending} - queryResult, err := store.GetTempUsersQuery(context.Background(), &query) - - require.Nil(t, err) - require.Equal(t, 0, len(queryResult)) - }) t.Run("Should be able to get temp users by email - case insensitive", func(t *testing.T) { setup(t) - store.cfg.CaseInsensitiveLogin = true query := tempuser.GetTempUsersQuery{Email: "E@as.co", Status: tempuser.TmpUserInvitePending} queryResult, err := store.GetTempUsersQuery(context.Background(), &query) require.Nil(t, err) require.Equal(t, 1, len(queryResult)) - t.Cleanup(func() { - store.cfg.CaseInsensitiveLogin = false - }) }) t.Run("Should be able to get temp users by code", func(t *testing.T) { diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index 8923e446348..c91195c203b 100644 --- a/pkg/services/user/userimpl/store.go +++ b/pkg/services/user/userimpl/store.go @@ -23,7 +23,7 @@ type store interface { GetByID(context.Context, int64) (*user.User, error) GetNotServiceAccount(context.Context, int64) (*user.User, error) Delete(context.Context, int64) error - LoginConflict(ctx context.Context, login, email string, caseInsensitive bool) error + LoginConflict(ctx context.Context, login, email string) error CaseInsensitiveLoginConflict(context.Context, string, string) error GetByLogin(context.Context, *user.GetUserByLoginQuery) (*user.User, error) GetByEmail(context.Context, *user.GetUserByEmailQuery) (*user.User, error) @@ -94,12 +94,9 @@ func (ss *sqlStore) Insert(ctx context.Context, cmd *user.User) (int64, error) { func (ss *sqlStore) Get(ctx context.Context, usr *user.User) (*user.User, error) { ret := &user.User{} err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { - where := "email=? OR login=?" login := usr.Login email := usr.Email - if ss.cfg.CaseInsensitiveLogin { - where = "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)" - } + where := "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)" exists, err := sess.Where(where, email, login).Get(ret) if !exists { @@ -198,10 +195,7 @@ func (ss *sqlStore) GetByLogin(ctx context.Context, query *user.GetUserByLoginQu // Since username can be an email address, attempt login with email address // first if the login field has the "@" symbol. if strings.Contains(query.LoginOrEmail, "@") { - where = "email=?" - if ss.cfg.CaseInsensitiveLogin { - where = "LOWER(email)=LOWER(?)" - } + where = "LOWER(email)=LOWER(?)" has, err = sess.Where(ss.notServiceAccountFilter()).Where(where, query.LoginOrEmail).Get(usr) if err != nil { return err @@ -210,10 +204,7 @@ func (ss *sqlStore) GetByLogin(ctx context.Context, query *user.GetUserByLoginQu // Look for the login field instead of email if !has { - where = "login=?" - if ss.cfg.CaseInsensitiveLogin { - where = "LOWER(login)=LOWER(?)" - } + where = "LOWER(login)=LOWER(?)" has, err = sess.Where(ss.notServiceAccountFilter()).Where(where, query.LoginOrEmail).Get(usr) } @@ -222,10 +213,8 @@ func (ss *sqlStore) GetByLogin(ctx context.Context, query *user.GetUserByLoginQu } else if !has { return user.ErrUserNotFound } - if ss.cfg.CaseInsensitiveLogin { - if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil { - return err - } + if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil { + return err } return nil }) @@ -244,11 +233,7 @@ func (ss *sqlStore) GetByEmail(ctx context.Context, query *user.GetUserByEmailQu return user.ErrUserNotFound } - where := "email=?" - if ss.cfg.CaseInsensitiveLogin { - where = "LOWER(email)=LOWER(?)" - } - + where := "LOWER(email)=LOWER(?)" has, err := sess.Where(ss.notServiceAccountFilter()).Where(where, query.Email).Get(usr) if err != nil { @@ -257,10 +242,8 @@ func (ss *sqlStore) GetByEmail(ctx context.Context, query *user.GetUserByEmailQu return user.ErrUserNotFound } - if ss.cfg.CaseInsensitiveLogin { - if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil { - return err - } + if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil { + return err } return nil }) @@ -288,21 +271,18 @@ func (ss *sqlStore) userCaseInsensitiveLoginConflict(ctx context.Context, sess * // LoginConflict returns an error if the provided email or login are already // associated with a user. If caseInsensitive is true the search is not case // sensitive. -func (ss *sqlStore) LoginConflict(ctx context.Context, login, email string, caseInsensitive bool) error { +func (ss *sqlStore) LoginConflict(ctx context.Context, login, email string) error { err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { - return ss.loginConflict(ctx, sess, login, email, caseInsensitive) + return ss.loginConflict(ctx, sess, login, email) }) return err } -func (ss *sqlStore) loginConflict(ctx context.Context, sess *db.Session, login, email string, caseInsensitive bool) error { +func (ss *sqlStore) loginConflict(ctx context.Context, sess *db.Session, login, email string) error { users := make([]user.User, 0) - where := "email=? OR login=?" - if caseInsensitive { - where = "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)" - login = strings.ToLower(login) - email = strings.ToLower(email) - } + where := "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)" + login = strings.ToLower(login) + email = strings.ToLower(email) exists, err := sess.Where(where, email, login).Get(&user.User{}) if err != nil { @@ -323,10 +303,8 @@ func (ss *sqlStore) loginConflict(ctx context.Context, sess *db.Session, login, } 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) - } + cmd.Login = strings.ToLower(cmd.Login) + cmd.Email = strings.ToLower(cmd.Email) return ss.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { user := user.User{ @@ -341,10 +319,8 @@ func (ss *sqlStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) err return err } - if ss.cfg.CaseInsensitiveLogin { - if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil { - return err - } + if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil { + return err } sess.PublishAfterCommit(&events.UserUpdated{ @@ -418,17 +394,9 @@ func (ss *sqlStore) GetSignedInUser(ctx context.Context, query *user.GetSignedIn case query.UserID > 0: sess.SQL(rawSQL+"WHERE u.id=?", query.UserID) case query.Login != "": - if ss.cfg.CaseInsensitiveLogin { - sess.SQL(rawSQL+"WHERE LOWER(u.login)=LOWER(?)", query.Login) - } else { - sess.SQL(rawSQL+"WHERE u.login=?", query.Login) - } + sess.SQL(rawSQL+"WHERE LOWER(u.login)=LOWER(?)", query.Login) case query.Email != "": - if ss.cfg.CaseInsensitiveLogin { - sess.SQL(rawSQL+"WHERE LOWER(u.email)=LOWER(?)", query.Email) - } else { - sess.SQL(rawSQL+"WHERE u.email=?", query.Email) - } + sess.SQL(rawSQL+"WHERE LOWER(u.email)=LOWER(?)", query.Email) default: return user.ErrNoUniqueID } diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index 9b6cd8391a4..13e399493f5 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/services/org/orgimpl" "github.com/grafana/grafana/pkg/services/quota/quotaimpl" "github.com/grafana/grafana/pkg/services/searchusers/sortopts" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" "github.com/grafana/grafana/pkg/services/user" @@ -28,46 +29,34 @@ func TestMain(m *testing.M) { func TestIntegrationUserGet(t *testing.T) { testCases := []struct { - name string - wantErr error - searchLogin string - searchEmail string - caseInsensitive bool + name string + wantErr error + searchLogin string + searchEmail string }{ { - name: "user not found non exact - not case insensitive", - wantErr: user.ErrUserNotFound, - searchLogin: "Test", - searchEmail: "Test@email.com", - caseInsensitive: false, + name: "user found non exact", + wantErr: nil, + searchLogin: "test", + searchEmail: "Test@email.com", }, { - name: "user found exact - not case insensitive", - wantErr: nil, - searchLogin: "test", - searchEmail: "test@email.com", - caseInsensitive: false, + name: "user found exact", + wantErr: nil, + searchLogin: "test", + searchEmail: "test@email.com", }, { - name: "user found non exact - case insensitive", - wantErr: nil, - searchLogin: "Test", - searchEmail: "Test@email.com", - caseInsensitive: true, + name: "user found exact - case insensitive", + wantErr: nil, + searchLogin: "Test", + searchEmail: "Test@email.com", }, { - name: "user found exact - case insensitive", - wantErr: nil, - searchLogin: "Test", - searchEmail: "Test@email.com", - caseInsensitive: true, - }, - { - name: "user not found - case insensitive", - wantErr: user.ErrUserNotFound, - searchLogin: "Test_login", - searchEmail: "Test*@email.com", - caseInsensitive: true, + name: "user not found - case insensitive", + wantErr: user.ErrUserNotFound, + searchLogin: "Test_login", + searchEmail: "Test*@email.com", }, } @@ -92,10 +81,9 @@ func TestIntegrationUserGet(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - if !tc.caseInsensitive && db.IsTestDbMySQL() { + if db.IsTestDbMySQL() { t.Skip("mysql is always case insensitive") } - cfg.CaseInsensitiveLogin = tc.caseInsensitive usr, err := userStore.Get(context.Background(), &user.User{ Email: tc.searchEmail, @@ -224,7 +212,6 @@ func TestIntegrationUserDataAccess(t *testing.T) { require.False(t, result.IsDisabled) t.Run("Get User by email case insensitive", func(t *testing.T) { - userStore.cfg.CaseInsensitiveLogin = true query := user.GetUserByEmailQuery{Email: "USERtest@TEST.COM"} result, err := userStore.GetByEmail(context.Background(), &query) require.Nil(t, err) @@ -234,8 +221,6 @@ func TestIntegrationUserDataAccess(t *testing.T) { require.Len(t, result.Rands, 10) require.Len(t, result.Salt, 10) require.False(t, result.IsDisabled) - - userStore.cfg.CaseInsensitiveLogin = false }) t.Run("Testing DB - creates and loads user", func(t *testing.T) { @@ -256,7 +241,6 @@ func TestIntegrationUserDataAccess(t *testing.T) { require.Len(t, result.Rands, 10) require.Len(t, result.Salt, 10) require.False(t, result.IsDisabled) - ss.Cfg.CaseInsensitiveLogin = false }) }) @@ -264,43 +248,104 @@ func TestIntegrationUserDataAccess(t *testing.T) { if ss.GetDBType() == migrator.MySQL { t.Skip("Skipping on MySQL due to case insensitive indexes") } - userStore.cfg.CaseInsensitiveLogin = true - cmd := user.CreateUserCommand{ - Email: "confusertest@test.com", - Name: "user name", - Login: "user_email_conflict", - } - // userEmailConflict - _, err = usrSvc.Create(context.Background(), &cmd) + testOrgID := int64(1) + err := ss.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + // create a user + // add additional user with conflicting login where DOMAIN is upper case + cmd := user.User{ + Email: "confusertest@test.com", + Name: "user name", + Login: "user_email_conflict", + OrgID: testOrgID, + Created: time.Now(), + Updated: time.Now(), + } + rawSQL := fmt.Sprintf( + "INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)", + ss.Quote("user"), + ss.GetDialect().BooleanStr(false), + ) + _, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated) + if err != nil { + return err + } + return nil + }) + require.NoError(t, err) + err = ss.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + // create a user + // add additional user with conflicting login where DOMAIN is upper case + cmd := user.User{ + Email: "confusertest@TEST.COM", + Name: "user name", + Login: "user_email_conflict_two", + OrgID: testOrgID, + Created: time.Now(), + Updated: time.Now(), + } + rawSQL := fmt.Sprintf( + "INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)", + ss.Quote("user"), + ss.GetDialect().BooleanStr(false), + ) + _, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated) + if err != nil { + return err + } + return nil + }) require.NoError(t, err) - cmd = user.CreateUserCommand{ - Email: "confusertest@TEST.COM", - Name: "user name", - Login: "user_email_conflict_two", - } - _, err := usrSvc.Create(context.Background(), &cmd) + err = ss.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + // create a user + // add additional user with conflicting login where DOMAIN is upper case + // userLoginConflict + cmd := user.User{ + Email: "user_test_login_conflict@test.com", + Name: "user name", + Login: "user_test_login_conflict", + OrgID: testOrgID, + Created: time.Now(), + Updated: time.Now(), + } + rawSQL := fmt.Sprintf( + "INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)", + ss.Quote("user"), + ss.GetDialect().BooleanStr(false), + ) + _, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated) + if err != nil { + return err + } + return nil + }) require.NoError(t, err) - cmd = user.CreateUserCommand{ - Email: "user_test_login_conflict@test.com", - Name: "user name", - Login: "user_test_login_conflict", - } - // userLoginConflict - _, err = usrSvc.Create(context.Background(), &cmd) + err = ss.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + // create a user + // add additional user with conflicting login where DOMAIN is upper case + // userLoginConflict + cmd := user.User{ + Email: "user_test_login_conflict_two@test.com", + Name: "user name", + Login: "user_test_login_CONFLICT", + OrgID: testOrgID, + Created: time.Now(), + Updated: time.Now(), + } + rawSQL := fmt.Sprintf( + "INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)", + ss.Quote("user"), + ss.GetDialect().BooleanStr(false), + ) + _, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated) + if err != nil { + return err + } + return nil + }) require.NoError(t, err) - cmd = user.CreateUserCommand{ - Email: "user_test_login_conflict_two@test.com", - Name: "user name", - Login: "user_test_login_CONFLICT", - } - _, err = usrSvc.Create(context.Background(), &cmd) - require.NoError(t, err) - - ss.Cfg.CaseInsensitiveLogin = true - t.Run("GetByEmail - email conflict", func(t *testing.T) { query := user.GetUserByEmailQuery{Email: "confusertest@test.com"} _, err = userStore.GetByEmail(context.Background(), &query) @@ -371,8 +416,6 @@ func TestIntegrationUserDataAccess(t *testing.T) { require.NotEqual(t, user2.Login, result.Login) require.NotEqual(t, user2.Name, result.Name) }) - - ss.Cfg.CaseInsensitiveLogin = false }) t.Run("Change user password", func(t *testing.T) { @@ -880,8 +923,6 @@ func TestIntegrationUserUpdate(t *testing.T) { } }) - 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", @@ -926,8 +967,6 @@ func TestIntegrationUserUpdate(t *testing.T) { require.Equal(t, "loginUSER3", result.Login) require.Equal(t, "USER3@test.com", result.Email) }) - - ss.Cfg.CaseInsensitiveLogin = false } func createFiveTestUsers(t *testing.T, svc user.Service, fn func(i int) *user.CreateUserCommand) []user.User { diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index cdcd7be9a26..4dbe3e4114d 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -71,16 +71,11 @@ func ProvideService( func (s *Service) GetUsageStats(ctx context.Context) map[string]any { stats := map[string]any{} - caseInsensitiveLoginVal := 0 basicAuthStrongPasswordPolicyVal := 0 - if s.cfg.CaseInsensitiveLogin { - caseInsensitiveLoginVal = 1 - } if s.cfg.BasicAuthStrongPasswordPolicy { basicAuthStrongPasswordPolicyVal = 1 } - stats["stats.case_insensitive_login.count"] = caseInsensitiveLoginVal stats["stats.password_policy.count"] = basicAuthStrongPasswordPolicyVal count, err := s.store.CountUserAccountsWithEmptyRole(ctx) @@ -132,7 +127,7 @@ func (s *Service) Create(ctx context.Context, cmd *user.CreateUserCommand) (*use cmd.Email = cmd.Login } - err = s.store.LoginConflict(ctx, cmd.Login, cmd.Email, s.cfg.CaseInsensitiveLogin) + err = s.store.LoginConflict(ctx, cmd.Login, cmd.Email) if err != nil { return nil, user.ErrUserAlreadyExists } @@ -218,10 +213,8 @@ func (s *Service) GetByID(ctx context.Context, query *user.GetUserByIDQuery) (*u if err != nil { return nil, err } - if s.cfg.CaseInsensitiveLogin { - if err := s.store.CaseInsensitiveLoginConflict(ctx, user.Login, user.Email); err != nil { - return nil, err - } + if err := s.store.CaseInsensitiveLoginConflict(ctx, user.Login, user.Email); err != nil { + return nil, err } return user, nil } @@ -235,10 +228,8 @@ func (s *Service) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuer } func (s *Service) Update(ctx context.Context, cmd *user.UpdateUserCommand) error { - if s.cfg.CaseInsensitiveLogin { - cmd.Login = strings.ToLower(cmd.Login) - cmd.Email = strings.ToLower(cmd.Email) - } + cmd.Login = strings.ToLower(cmd.Login) + cmd.Email = strings.ToLower(cmd.Email) return s.store.Update(ctx, cmd) } @@ -404,7 +395,7 @@ func readQuotaConfig(cfg *setting.Cfg) (*quota.Map, error) { // CreateServiceAccount creates a service account in the user table and adds service account to an organisation in the org_user table func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUserCommand) (*user.User, error) { cmd.Email = cmd.Login - err := s.store.LoginConflict(ctx, cmd.Login, cmd.Email, s.cfg.CaseInsensitiveLogin) + err := s.store.LoginConflict(ctx, cmd.Login, cmd.Email) if err != nil { return nil, serviceaccounts.ErrServiceAccountAlreadyExists.Errorf("service account with login %s already exists", cmd.Login) } diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index b33aa2eb4f7..7d19e5b6696 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -50,18 +50,6 @@ func TestUserService(t *testing.T) { t.Run("get user by ID", func(t *testing.T) { userService.cfg = setting.NewCfg() - userService.cfg.CaseInsensitiveLogin = false - userStore.ExpectedUser = &user.User{ID: 1, Email: "email", Login: "login", Name: "name"} - u, err := userService.GetByID(context.Background(), &user.GetUserByIDQuery{ID: 1}) - require.NoError(t, err) - require.Equal(t, "login", u.Login) - require.Equal(t, "name", u.Name) - require.Equal(t, "email", u.Email) - }) - - t.Run("get user by ID with case insensitive login", func(t *testing.T) { - userService.cfg = setting.NewCfg() - userService.cfg.CaseInsensitiveLogin = true userStore.ExpectedUser = &user.User{ID: 1, Email: "email", Login: "login", Name: "name"} u, err := userService.GetByID(context.Background(), &user.GetUserByIDQuery{ID: 1}) require.NoError(t, err) @@ -99,7 +87,6 @@ func TestUserService(t *testing.T) { }) t.Run("GetByID - email conflict", func(t *testing.T) { - userService.cfg.CaseInsensitiveLogin = true userStore.ExpectedError = errors.New("email conflict") query := user.GetUserByIDQuery{} _, err := userService.GetByID(context.Background(), &query) @@ -218,14 +205,12 @@ func TestMetrics(t *testing.T) { userStore.ExpectedCountUserAccountsWithEmptyRoles = int64(1) userService.cfg = setting.NewCfg() - userService.cfg.CaseInsensitiveLogin = true userService.cfg.BasicAuthStrongPasswordPolicy = true stats := userService.GetUsageStats(context.Background()) assert.NotEmpty(t, stats) - assert.Len(t, stats, 3, stats) - assert.Equal(t, 1, stats["stats.case_insensitive_login.count"]) + assert.Len(t, stats, 2, stats) assert.Equal(t, int64(1), stats["stats.user.role_none.count"]) assert.Equal(t, 1, stats["stats.password_policy.count"]) }) @@ -269,7 +254,7 @@ func (f *FakeUserStore) CaseInsensitiveLoginConflict(context.Context, string, st return f.ExpectedError } -func (f *FakeUserStore) LoginConflict(context.Context, string, string, bool) error { +func (f *FakeUserStore) LoginConflict(context.Context, string, string) error { return f.ExpectedError } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 2a142630b14..959cac19af4 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -1656,7 +1656,9 @@ func readUserSettings(iniFile *ini.File, cfg *Cfg) error { string(roletype.RoleAdmin)}) cfg.VerifyEmailEnabled = users.Key("verify_email_enabled").MustBool(false) - cfg.CaseInsensitiveLogin = users.Key("case_insensitive_login").MustBool(true) + // Deprecated + // cfg.CaseInsensitiveLogin = users.Key("case_insensitive_login").MustBool(true) + cfg.CaseInsensitiveLogin = true cfg.LoginHint = valueAsString(users, "login_hint", "") cfg.PasswordHint = valueAsString(users, "password_hint", "")