Browse Source

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
Oleg Gaidarenko 6 years ago
parent
commit
8e0f091f14
3 changed files with 192 additions and 31 deletions
  1. 5 0
      pkg/models/user.go
  2. 11 0
      pkg/services/sqlstore/user.go
  3. 176 31
      pkg/services/sqlstore/user_test.go

+ 5 - 0
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
 }
 

+ 11 - 0
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,

+ 176 - 31
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
+}