From b6a91429c9d03d69f341d8afa6565df5c9964045 Mon Sep 17 00:00:00 2001 From: Jo Date: Fri, 21 Oct 2022 12:15:59 +0000 Subject: [PATCH] Swap order of login fields (#57426) Co-authored-by: linoman <2051016+linoman@users.noreply.github.com> --- pkg/services/sqlstore/user.go | 35 +++++++++++++++------------ pkg/services/sqlstore/user_test.go | 39 ++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index a0fb2b21376..6b965fe6252 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -228,28 +228,33 @@ func (ss *SQLStore) GetUserByLogin(ctx context.Context, query *models.GetUserByL return user.ErrUserNotFound } - // Try and find the user by login first. - // It's not sufficient to assume that a LoginOrEmail with an "@" is an email. + var where string + var has bool + var err error + + // Since username can be an email address, attempt login with email address + // first if the login field has the "@" symbol. usr := &user.User{} - where := "login=?" - if ss.Cfg.CaseInsensitiveLogin { - where = "LOWER(login)=LOWER(?)" - } - - has, err := sess.Where(notServiceAccountFilter(ss)).Where(where, query.LoginOrEmail).Get(usr) - if err != nil { - return err - } - - if !has && strings.Contains(query.LoginOrEmail, "@") { - // If the user wasn't found, and it contains an "@" fallback to finding the - // user by email. + if strings.Contains(query.LoginOrEmail, "@") { where = "email=?" if ss.Cfg.CaseInsensitiveLogin { where = "LOWER(email)=LOWER(?)" } + has, err = sess.Where(notServiceAccountFilter(ss)).Where(where, query.LoginOrEmail).Get(usr) + + if err != nil { + return err + } + } + + // Look for the login field instead of email + if !has { usr = &user.User{} + where = "login=?" + if ss.Cfg.CaseInsensitiveLogin { + where = "LOWER(login)=LOWER(?)" + } has, err = sess.Where(notServiceAccountFilter(ss)).Where(where, query.LoginOrEmail).Get(usr) } diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index bc80e36dac9..07d2e53d940 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -166,6 +166,45 @@ func TestIntegrationUserDataAccess(t *testing.T) { }) }) + t.Run("Get User by login - user_2 uses user_1.email as login", func(t *testing.T) { + ss = InitTestDB(t) + + // create user_1 + cmd := user.CreateUserCommand{ + Email: "user_1@mail.com", + Name: "user_1", + Login: "user_1", + Password: "user_1_password", + IsDisabled: true, + } + user_1, err := ss.CreateUser(context.Background(), cmd) + require.Nil(t, err) + + // create user_2 + cmd = user.CreateUserCommand{ + Email: "user_2@mail.com", + Name: "user_2", + Login: "user_1@mail.com", + Password: "user_2_password", + IsDisabled: true, + } + user_2, err := ss.CreateUser(context.Background(), cmd) + require.Nil(t, err) + + // query user database for user_1 email + query := models.GetUserByLoginQuery{LoginOrEmail: "user_1@mail.com"} + err = ss.GetUserByLogin(context.Background(), &query) + require.Nil(t, err) + + // expect user_1 as result + require.Equal(t, user_1.Email, query.Result.Email) + require.Equal(t, user_1.Login, query.Result.Login) + require.Equal(t, user_1.Name, query.Result.Name) + require.NotEqual(t, user_2.Email, query.Result.Email) + require.NotEqual(t, user_2.Login, query.Result.Login) + require.NotEqual(t, user_2.Name, query.Result.Name) + }) + t.Run("Testing DB - creates and loads disabled user", func(t *testing.T) { ss = InitTestDB(t) cmd := user.CreateUserCommand{