diff --git a/app/login.go b/app/login.go index 0addd6225f..a05c938696 100644 --- a/app/login.go +++ b/app/login.go @@ -93,8 +93,8 @@ func (a *App) GetUserForLogin(id, loginId string) (*model.User, *model.AppError) } // Try to get the user by username/email - if result := <-a.Srv.Store.User().GetForLogin(loginId, enableUsername, enableEmail); result.Err == nil { - return result.Data.(*model.User), nil + if user, err := a.Srv.Store.User().GetForLogin(loginId, enableUsername, enableEmail); err == nil { + return user, nil } // Try to get the user with LDAP if enabled diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 51c1edd5a0..af704ba25a 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -1074,45 +1074,38 @@ func (us SqlUserStore) GetByUsername(username string) store.StoreChannel { }) } -func (us SqlUserStore) GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail bool) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - query := us.usersQuery +func (us SqlUserStore) GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail bool) (*model.User, *model.AppError) { + query := us.usersQuery + if allowSignInWithUsername && allowSignInWithEmail { + query = query.Where("Username = ? OR Email = ?", loginId, loginId) + } else if allowSignInWithUsername { + query = query.Where("Username = ?", loginId) + } else if allowSignInWithEmail { + query = query.Where("Email = ?", loginId) + } else { + return nil, model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.app_error", nil, "", http.StatusInternalServerError) + } - if allowSignInWithUsername && allowSignInWithEmail { - query = query.Where("Username = ? OR Email = ?", loginId, loginId) - } else if allowSignInWithUsername { - query = query.Where("Username = ?", loginId) - } else if allowSignInWithEmail { - query = query.Where("Email = ?", loginId) - } else { - result.Err = model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.app_error", nil, "", http.StatusInternalServerError) - return - } + queryString, args, err := query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) + } - queryString, args, err := query.ToSql() - if err != nil { - result.Err = model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + users := []*model.User{} + if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil { + return nil, model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.app_error", nil, err.Error(), http.StatusInternalServerError) + } - users := []*model.User{} - if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil { - result.Err = model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.app_error", nil, err.Error(), http.StatusInternalServerError) - return - } + if len(users) == 0 { + return nil, model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.app_error", nil, "", http.StatusInternalServerError) + } - if len(users) == 0 { - result.Err = model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.app_error", nil, "", http.StatusInternalServerError) - return - } + if len(users) > 1 { + return nil, model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.multiple_users", nil, "", http.StatusInternalServerError) + } - if len(users) > 1 { - result.Err = model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.multiple_users", nil, "", http.StatusInternalServerError) - return - } + return users[0], nil - result.Data = users[0] - }) } func (us SqlUserStore) VerifyEmail(userId, email string) (string, *model.AppError) { diff --git a/store/store.go b/store/store.go index 5d97dbc852..cc176e354c 100644 --- a/store/store.go +++ b/store/store.go @@ -275,7 +275,7 @@ type UserStore interface { GetByAuth(authData *string, authService string) (*model.User, *model.AppError) GetAllUsingAuthService(authService string) ([]*model.User, *model.AppError) GetByUsername(username string) StoreChannel - GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail bool) StoreChannel + GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail bool) (*model.User, *model.AppError) VerifyEmail(userId, email string) (string, *model.AppError) GetEtagForAllProfiles() StoreChannel GetEtagForProfiles(teamId string) StoreChannel diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index ba2cedc077..b8f631a171 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -412,19 +412,28 @@ func (_m *UserStore) GetEtagForProfilesNotInTeam(teamId string) store.StoreChann } // GetForLogin provides a mock function with given fields: loginId, allowSignInWithUsername, allowSignInWithEmail -func (_m *UserStore) GetForLogin(loginId string, allowSignInWithUsername bool, allowSignInWithEmail bool) store.StoreChannel { +func (_m *UserStore) GetForLogin(loginId string, allowSignInWithUsername bool, allowSignInWithEmail bool) (*model.User, *model.AppError) { ret := _m.Called(loginId, allowSignInWithUsername, allowSignInWithEmail) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, bool, bool) store.StoreChannel); ok { + var r0 *model.User + if rf, ok := ret.Get(0).(func(string, bool, bool) *model.User); ok { r0 = rf(loginId, allowSignInWithUsername, allowSignInWithEmail) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.User) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, bool, bool) *model.AppError); ok { + r1 = rf(loginId, allowSignInWithUsername, allowSignInWithEmail) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetNewUsersForTeam provides a mock function with given fields: teamId, offset, limit, viewRestrictions diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 77d7680295..fab9213dfe 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -1698,45 +1698,45 @@ func testUserStoreGetForLogin(t *testing.T, ss store.Store) { defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() t.Run("get u1 by username, allow both", func(t *testing.T) { - result := <-ss.User().GetForLogin(u1.Username, true, true) - require.Nil(t, result.Err) - assert.Equal(t, u1, result.Data.(*model.User)) + user, err := ss.User().GetForLogin(u1.Username, true, true) + require.Nil(t, err) + assert.Equal(t, u1, user) }) t.Run("get u1 by username, allow only email", func(t *testing.T) { - result := <-ss.User().GetForLogin(u1.Username, false, true) - require.NotNil(t, result.Err) - require.Equal(t, result.Err.Id, "store.sql_user.get_for_login.app_error") + _, err := ss.User().GetForLogin(u1.Username, false, true) + require.NotNil(t, err) + require.Equal(t, err.Id, "store.sql_user.get_for_login.app_error") }) t.Run("get u1 by email, allow both", func(t *testing.T) { - result := <-ss.User().GetForLogin(u1.Email, true, true) - require.Nil(t, result.Err) - assert.Equal(t, u1, result.Data.(*model.User)) + user, err := ss.User().GetForLogin(u1.Email, true, true) + require.Nil(t, err) + assert.Equal(t, u1, user) }) t.Run("get u1 by email, allow only username", func(t *testing.T) { - result := <-ss.User().GetForLogin(u1.Email, true, false) - require.NotNil(t, result.Err) - require.Equal(t, result.Err.Id, "store.sql_user.get_for_login.app_error") + _, err := ss.User().GetForLogin(u1.Email, true, false) + require.NotNil(t, err) + require.Equal(t, err.Id, "store.sql_user.get_for_login.app_error") }) t.Run("get u2 by username, allow both", func(t *testing.T) { - result := <-ss.User().GetForLogin(u2.Username, true, true) - require.Nil(t, result.Err) - assert.Equal(t, u2, result.Data.(*model.User)) + user, err := ss.User().GetForLogin(u2.Username, true, true) + require.Nil(t, err) + assert.Equal(t, u2, user) }) t.Run("get u2 by email, allow both", func(t *testing.T) { - result := <-ss.User().GetForLogin(u2.Email, true, true) - require.Nil(t, result.Err) - assert.Equal(t, u2, result.Data.(*model.User)) + user, err := ss.User().GetForLogin(u2.Email, true, true) + require.Nil(t, err) + assert.Equal(t, u2, user) }) t.Run("get u2 by username, allow neither", func(t *testing.T) { - result := <-ss.User().GetForLogin(u2.Username, false, false) - require.NotNil(t, result.Err) - require.Equal(t, result.Err.Id, "store.sql_user.get_for_login.app_error") + _, err := ss.User().GetForLogin(u2.Username, false, false) + require.NotNil(t, err) + require.Equal(t, err.Id, "store.sql_user.get_for_login.app_error") }) }