From 8e0f091f1481adc70fba4dda2570b77ca45b3d01 Mon Sep 17 00:00:00 2001 From: Oleg Gaidarenko Date: Mon, 15 Jul 2019 09:14:32 +0300 Subject: [PATCH] SQLStore: allow to look for `is_disabled` flag (#18032) * Add support for `is_disabled` to `CreateUser()` * Add support for `is_disabled` to `SearchUsers()` Had to add it as a `string` type not as `bool`, since if that's property is omitted, we would have add it to SQL request, which might be dangerous * Restructure desctructive tests and add more --- pkg/models/user.go | 5 + pkg/services/sqlstore/user.go | 11 ++ pkg/services/sqlstore/user_test.go | 207 ++++++++++++++++++++++++----- 3 files changed, 192 insertions(+), 31 deletions(-) diff --git a/pkg/models/user.go b/pkg/models/user.go index bf52aed6160..942fd8e1035 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -62,6 +62,7 @@ type CreateUserCommand struct { Password string EmailVerified bool IsAdmin bool + IsDisabled bool SkipOrgSetup bool DefaultOrgRole string @@ -146,6 +147,10 @@ type SearchUsersQuery struct { Limit int AuthModule string + // We have to use string not bool, since there is cases when + // we don't care if user is disabled or not + IsDisabled string + Result SearchUserQueryResult } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index e571c002f8b..42dc2177f91 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -106,6 +106,7 @@ func CreateUser(ctx context.Context, cmd *models.CreateUserCommand) error { Login: cmd.Login, Company: cmd.Company, IsAdmin: cmd.IsAdmin, + IsDisabled: cmd.IsDisabled, OrgId: orgId, EmailVerified: cmd.EmailVerified, Created: time.Now(), @@ -455,6 +456,16 @@ func SearchUsers(query *models.SearchUsersQuery) error { whereParams = append(whereParams, queryWithWildcards, queryWithWildcards, queryWithWildcards) } + if query.IsDisabled != "" { + param, err := strconv.ParseBool(query.IsDisabled) + if err != nil { + return err + } + + whereConditions = append(whereConditions, "is_disabled = ?") + whereParams = append(whereParams, param) + } + if query.AuthModule != "" { whereConditions = append( whereConditions, diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 227e90f86c2..38e59a53002 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -16,7 +16,7 @@ func TestUserDataAccess(t *testing.T) { Convey("Testing DB", t, func() { ss := InitTestDB(t) - Convey("Creating a user", func() { + Convey("Creates a user", func() { cmd := &models.CreateUserCommand{ Email: "usertest@test.com", Name: "user name", @@ -35,27 +35,47 @@ func TestUserDataAccess(t *testing.T) { So(query.Result.Password, ShouldEqual, "") So(query.Result.Rands, ShouldHaveLength, 10) So(query.Result.Salt, ShouldHaveLength, 10) + So(query.Result.IsDisabled, ShouldBeFalse) + }) + }) + + Convey("Creates disabled user", func() { + cmd := &models.CreateUserCommand{ + Email: "usertest@test.com", + Name: "user name", + Login: "user_test_login", + IsDisabled: true, + } + + err := CreateUser(context.Background(), cmd) + So(err, ShouldBeNil) + + Convey("Loading a user", func() { + query := models.GetUserByIdQuery{Id: cmd.Result.Id} + err := GetUserById(&query) + So(err, ShouldBeNil) + + So(query.Result.Email, ShouldEqual, "usertest@test.com") + So(query.Result.Password, ShouldEqual, "") + So(query.Result.Rands, ShouldHaveLength, 10) + So(query.Result.Salt, ShouldHaveLength, 10) + So(query.Result.IsDisabled, ShouldBeTrue) }) }) Convey("Given 5 users", func() { - var err error - var cmd *models.CreateUserCommand - users := []models.User{} - for i := 0; i < 5; i++ { - cmd = &models.CreateUserCommand{ - Email: fmt.Sprint("user", i, "@test.com"), - Name: fmt.Sprint("user", i), - Login: fmt.Sprint("loginuser", i), + users := createFiveTestUsers(func(i int) *models.CreateUserCommand { + return &models.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + IsDisabled: false, } - err = CreateUser(context.Background(), cmd) - So(err, ShouldBeNil) - users = append(users, cmd.Result) - } + }) Convey("Can return the first page of users and a total count", func() { query := models.SearchUsersQuery{Query: "", Page: 1, Limit: 3} - err = SearchUsers(&query) + err := SearchUsers(&query) So(err, ShouldBeNil) So(len(query.Result.Users), ShouldEqual, 3) @@ -64,7 +84,7 @@ func TestUserDataAccess(t *testing.T) { Convey("Can return the second page of users and a total count", func() { query := models.SearchUsersQuery{Query: "", Page: 2, Limit: 3} - err = SearchUsers(&query) + err := SearchUsers(&query) So(err, ShouldBeNil) So(len(query.Result.Users), ShouldEqual, 2) @@ -73,7 +93,7 @@ func TestUserDataAccess(t *testing.T) { Convey("Can return list of users matching query on user name", func() { query := models.SearchUsersQuery{Query: "use", Page: 1, Limit: 3} - err = SearchUsers(&query) + err := SearchUsers(&query) So(err, ShouldBeNil) So(len(query.Result.Users), ShouldEqual, 3) @@ -103,7 +123,7 @@ func TestUserDataAccess(t *testing.T) { Convey("Can return list of users matching query on email", func() { query := models.SearchUsersQuery{Query: "ser1@test.com", Page: 1, Limit: 3} - err = SearchUsers(&query) + err := SearchUsers(&query) So(err, ShouldBeNil) So(len(query.Result.Users), ShouldEqual, 1) @@ -112,14 +132,14 @@ func TestUserDataAccess(t *testing.T) { Convey("Can return list of users matching query on login name", func() { query := models.SearchUsersQuery{Query: "loginuser1", Page: 1, Limit: 3} - err = SearchUsers(&query) + err := SearchUsers(&query) So(err, ShouldBeNil) So(len(query.Result.Users), ShouldEqual, 1) So(query.Result.TotalCount, ShouldEqual, 1) }) - Convey("can return list users based on their auth type", func() { + Convey("Can return list users based on their auth type", func() { // add users to auth table for index, user := range users { authModule := "killa" @@ -134,11 +154,11 @@ func TestUserDataAccess(t *testing.T) { AuthModule: authModule, AuthId: "gorilla", } - err = SetAuthInfo(cmd2) + err := SetAuthInfo(cmd2) So(err, ShouldBeNil) } query := models.SearchUsersQuery{AuthModule: "ldap"} - err = SearchUsers(&query) + err := SearchUsers(&query) So(err, ShouldBeNil) So(query.Result.Users, ShouldHaveLength, 3) @@ -163,8 +183,50 @@ func TestUserDataAccess(t *testing.T) { So(fourth, ShouldBeTrue) }) + Convey("Can return list users based on their is_disabled flag", func() { + ss = InitTestDB(t) + createFiveTestUsers(func(i int) *models.CreateUserCommand { + return &models.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + IsDisabled: i%2 == 0, + } + }) + + query := models.SearchUsersQuery{IsDisabled: "false"} + err := SearchUsers(&query) + So(err, ShouldBeNil) + + So(query.Result.Users, ShouldHaveLength, 2) + + first, third := false, false + for _, user := range query.Result.Users { + if user.Name == "user1" { + first = true + } + + if user.Name == "user3" { + third = true + } + } + + So(first, ShouldBeTrue) + So(third, ShouldBeTrue) + + ss = InitTestDB(t) + users = createFiveTestUsers(func(i int) *models.CreateUserCommand { + return &models.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + IsDisabled: false, + } + }) + }) + Convey("when a user is an org member and has been assigned permissions", func() { - err = AddOrgUser(&models.AddOrgUserCommand{LoginOrEmail: users[1].Login, Role: models.ROLE_VIEWER, OrgId: users[0].OrgId, UserId: users[1].Id}) + err := AddOrgUser(&models.AddOrgUserCommand{LoginOrEmail: users[1].Login, Role: models.ROLE_VIEWER, OrgId: users[0].OrgId, UserId: users[1].Id}) So(err, ShouldBeNil) testHelperUpdateDashboardAcl(1, models.DashboardAcl{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[1].Id, Permission: models.PERMISSION_EDIT}) @@ -222,19 +284,75 @@ func TestUserDataAccess(t *testing.T) { }) Convey("When batch disabling users", func() { - userIdsToDisable := []int64{} - for i := 0; i < 3; i++ { - userIdsToDisable = append(userIdsToDisable, users[i].Id) - } - disableCmd := models.BatchDisableUsersCommand{UserIds: userIdsToDisable, IsDisabled: true} + Convey("Should disable all users", func() { + disableCmd := models.BatchDisableUsersCommand{ + UserIds: []int64{1, 2, 3, 4, 5}, + IsDisabled: true, + } - err = BatchDisableUsers(&disableCmd) - So(err, ShouldBeNil) + err := BatchDisableUsers(&disableCmd) + So(err, ShouldBeNil) + + query := &models.SearchUsersQuery{IsDisabled: "true"} + err = SearchUsers(query) + + So(err, ShouldBeNil) + So(query.Result.TotalCount, ShouldEqual, 5) + }) + + Convey("Should enable all users", func() { + ss = InitTestDB(t) + createFiveTestUsers(func(i int) *models.CreateUserCommand { + return &models.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + IsDisabled: true, + } + }) + + disableCmd := models.BatchDisableUsersCommand{ + UserIds: []int64{1, 2, 3, 4, 5}, + IsDisabled: false, + } + + err := BatchDisableUsers(&disableCmd) + So(err, ShouldBeNil) + + query := &models.SearchUsersQuery{IsDisabled: "false"} + err = SearchUsers(query) + + So(err, ShouldBeNil) + So(query.Result.TotalCount, ShouldEqual, 5) + }) + + Convey("Should disable only specific users", func() { + ss = InitTestDB(t) + users = createFiveTestUsers(func(i int) *models.CreateUserCommand { + return &models.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + IsDisabled: false, + } + }) + + userIdsToDisable := []int64{} + for i := 0; i < 3; i++ { + userIdsToDisable = append(userIdsToDisable, users[i].Id) + } + disableCmd := models.BatchDisableUsersCommand{ + UserIds: userIdsToDisable, + IsDisabled: true, + } + + err := BatchDisableUsers(&disableCmd) + So(err, ShouldBeNil) - Convey("Should disable all provided users", func() { query := models.SearchUsersQuery{} err = SearchUsers(&query) + So(err, ShouldBeNil) So(query.Result.TotalCount, ShouldEqual, 5) for _, user := range query.Result.Users { shouldBeDisabled := false @@ -253,6 +371,17 @@ func TestUserDataAccess(t *testing.T) { } } }) + + // Since previous tests were destructive + ss = InitTestDB(t) + users = createFiveTestUsers(func(i int) *models.CreateUserCommand { + return &models.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + IsDisabled: false, + } + }) }) Convey("When searching users", func() { @@ -263,7 +392,7 @@ func TestUserDataAccess(t *testing.T) { // Make the first log-in during the past getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } query := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "test1", AuthId: "test1"} - err = GetUserByAuthInfo(query) + err := GetUserByAuthInfo(query) getTime = time.Now So(err, ShouldBeNil) @@ -349,3 +478,19 @@ func GetOrgUsersForTest(query *models.GetOrgUsersQuery) error { err := sess.Find(&query.Result) return err } + +func createFiveTestUsers(fn func(i int) *models.CreateUserCommand) []models.User { + var err error + var cmd *models.CreateUserCommand + users := []models.User{} + for i := 0; i < 5; i++ { + cmd = fn(i) + + err = CreateUser(context.Background(), cmd) + users = append(users, cmd.Result) + + So(err, ShouldBeNil) + } + + return users +}