diff --git a/pkg/services/temp_user/tempuserimpl/store.go b/pkg/services/temp_user/tempuserimpl/store.go index c66f702a8a9..d362b4ba897 100644 --- a/pkg/services/temp_user/tempuserimpl/store.go +++ b/pkg/services/temp_user/tempuserimpl/store.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" tempuser "github.com/grafana/grafana/pkg/services/temp_user" + "github.com/grafana/grafana/pkg/setting" ) type store interface { @@ -18,7 +19,8 @@ type store interface { } type xormStore struct { - db db.DB + db db.DB + cfg *setting.Cfg } func (ss *xormStore) UpdateTempUserStatus(ctx context.Context, cmd *tempuser.UpdateTempUserStatusCommand) error { @@ -102,7 +104,11 @@ func (ss *xormStore) GetTempUsersQuery(ctx context.Context, query *tempuser.GetT } if query.Email != "" { - rawSQL += ` AND tu.email=?` + if ss.cfg.CaseInsensitiveLogin { + rawSQL += ` AND LOWER(tu.email)=LOWER(?)` + } else { + rawSQL += ` AND tu.email=?` + } 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 507b5ef179b..7fa636110e5 100644 --- a/pkg/services/temp_user/tempuserimpl/store_test.go +++ b/pkg/services/temp_user/tempuserimpl/store_test.go @@ -15,7 +15,7 @@ func TestIntegrationTempUserCommandsAndQueries(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } - var store store + var store *xormStore var tempUser *tempuser.TempUser var err error cmd := tempuser.CreateTempUserCommand{ @@ -27,7 +27,7 @@ func TestIntegrationTempUserCommandsAndQueries(t *testing.T) { } setup := func(t *testing.T) { db := db.InitTestDB(t) - store = &xormStore{db: db} + store = &xormStore{db: db, cfg: db.Cfg} tempUser, err = store.CreateTempUser(context.Background(), &cmd) require.Nil(t, err) } @@ -56,6 +56,31 @@ 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) { setup(t) diff --git a/pkg/services/temp_user/tempuserimpl/temp_user.go b/pkg/services/temp_user/tempuserimpl/temp_user.go index b46b3281ce8..e6dd24fe452 100644 --- a/pkg/services/temp_user/tempuserimpl/temp_user.go +++ b/pkg/services/temp_user/tempuserimpl/temp_user.go @@ -5,6 +5,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" tempuser "github.com/grafana/grafana/pkg/services/temp_user" + "github.com/grafana/grafana/pkg/setting" ) type Service struct { @@ -13,9 +14,10 @@ type Service struct { func ProvideService( db db.DB, + cfg *setting.Cfg, ) tempuser.Service { return &Service{ - store: &xormStore{db: db}, + store: &xormStore{db: db, cfg: cfg}, } } diff --git a/pkg/services/user/userimpl/store.go b/pkg/services/user/userimpl/store.go index f2f9355ddfe..f59c6e40302 100644 --- a/pkg/services/user/userimpl/store.go +++ b/pkg/services/user/userimpl/store.go @@ -88,8 +88,16 @@ 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 { - exists, err := sess.Where("email=? OR login=?", usr.Email, usr.Login).Get(usr) + where := "email=? OR login=?" + login := usr.Login + email := usr.Email + if ss.cfg.CaseInsensitiveLogin { + where = "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)" + } + + exists, err := sess.Where(where, email, login).Get(ret) if !exists { return user.ErrUserNotFound } @@ -101,7 +109,8 @@ func (ss *sqlStore) Get(ctx context.Context, usr *user.User) (*user.User, error) if err != nil { return nil, err } - return usr, nil + + return ret, nil } func (ss *sqlStore) Delete(ctx context.Context, userID int64) error { diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index e3b43a801b0..e617478d161 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -21,6 +21,94 @@ import ( "github.com/grafana/grafana/pkg/setting" ) +func TestIntegrationUserGet(t *testing.T) { + testCases := []struct { + name string + wantErr error + searchLogin string + searchEmail string + caseInsensitive bool + }{ + { + name: "user not found non exact - not case insensitive", + wantErr: user.ErrUserNotFound, + searchLogin: "Test", + searchEmail: "Test@email.com", + caseInsensitive: false, + }, + { + name: "user found exact - not case insensitive", + wantErr: nil, + searchLogin: "test", + searchEmail: "test@email.com", + caseInsensitive: false, + }, + { + 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", + caseInsensitive: true, + }, + { + name: "user not found - case insensitive", + wantErr: user.ErrUserNotFound, + searchLogin: "Test_login", + searchEmail: "Test*@email.com", + caseInsensitive: true, + }, + } + + if testing.Short() { + t.Skip("skipping integration test") + } + + ss := db.InitTestDB(t) + cfg := ss.Cfg + userStore := ProvideStore(ss, cfg) + + _, errUser := userStore.Insert(context.Background(), + &user.User{ + Email: "test@email.com", + Name: "test", + Login: "test", + Created: time.Now(), + Updated: time.Now(), + }, + ) + require.NoError(t, errUser) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if !tc.caseInsensitive && db.IsTestDbMySQL() { + t.Skip("mysql is always case insensitive") + } + cfg.CaseInsensitiveLogin = tc.caseInsensitive + usr, err := userStore.Get(context.Background(), + &user.User{ + Email: tc.searchEmail, + Login: tc.searchLogin, + }, + ) + + if tc.wantErr != nil { + require.Error(t, err) + require.Nil(t, usr) + } else { + require.NoError(t, err) + require.NotNil(t, usr) + } + }) + } +} + func TestIntegrationUserDataAccess(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 97f7a39e78b..3f0e65a1711 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -1590,7 +1590,7 @@ func readUserSettings(iniFile *ini.File, cfg *Cfg) error { cfg.AutoAssignOrgRole = users.Key("auto_assign_org_role").In("Editor", []string{"Editor", "Admin", "Viewer"}) VerifyEmailEnabled = users.Key("verify_email_enabled").MustBool(false) - cfg.CaseInsensitiveLogin = users.Key("case_insensitive_login").MustBool(false) + cfg.CaseInsensitiveLogin = users.Key("case_insensitive_login").MustBool(true) LoginHint = valueAsString(users, "login_hint", "") PasswordHint = valueAsString(users, "password_hint", "")