feat: improve Users filter API (#2645)

coderd/database/databasefake/databasefake.go

@@ -12,6 +12,7 @@ import (
 	"golang.org/x/exp/slices"
 
 	"github.com/coder/coder/coderd/database"
+	"github.com/coder/coder/coderd/rbac"
 	"github.com/coder/coder/coderd/util/slice"
 )
 
@@ -276,9 +277,9 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
 	if params.Search != "" {
 		tmp := make([]database.User, 0, len(users))
 		for i, user := range users {
-			if strings.Contains(user.Email, params.Search) {
+			if strings.Contains(strings.ToLower(user.Email), strings.ToLower(params.Search)) {
 				tmp = append(tmp, users[i])
-			} else if strings.Contains(user.Username, params.Search) {
+			} else if strings.Contains(strings.ToLower(user.Username), strings.ToLower(params.Search)) {
 				tmp = append(tmp, users[i])
 			}
 		}
@@ -295,7 +296,7 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
 		users = usersFilteredByStatus
 	}
 
-	if len(params.RbacRole) > 0 {
+	if len(params.RbacRole) > 0 && !slice.Contains(params.RbacRole, rbac.RoleMember()) {
 		usersFilteredByRole := make([]database.User, 0, len(users))
 		for i, user := range users {
 			if slice.Overlap(params.RbacRole, user.RBACRoles) {

coderd/database/queries.sql.go

@@ -2563,8 +2563,8 @@ WHERE
 	-- Filter by name, email or username
 	AND CASE
 		WHEN $2 :: text != '' THEN (
-			email LIKE concat('%', $2, '%')
-			OR username LIKE concat('%', $2, '%')
+			email ILIKE concat('%', $2, '%')
+			OR username ILIKE concat('%', $2, '%')
 		)
 		ELSE true
 	END

coderd/database/queries/users.sql

@@ -99,8 +99,8 @@ WHERE
 	-- Filter by name, email or username
 	AND CASE
 		WHEN @search :: text != '' THEN (
-			email LIKE concat('%', @search, '%')
-			OR username LIKE concat('%', @search, '%')
+			email ILIKE concat('%', @search, '%')
+			OR username ILIKE concat('%', @search, '%')
 		)
 		ELSE true
 	END

coderd/httpapi/queryparams.go

@@ -107,7 +107,7 @@ func ParseCustom[T any](parser *QueryParamParser, vals url.Values, def T, queryP
 	if err != nil {
 		parser.Errors = append(parser.Errors, Error{
 			Field:  queryParam,
-			Detail: fmt.Sprintf("Query param %q has invalid uuids: %q", queryParam, err.Error()),
+			Detail: fmt.Sprintf("Query param %q has invalid value: %s", queryParam, err.Error()),
 		})
 	}
 	return v

coderd/users.go

@@ -127,6 +127,7 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) {
 			Message:     "Invalid user search query.",
 			Validations: errs,
 		})
+		return
 	}
 
 	paginationParams, ok := parsePagination(rw, r)
@@ -959,6 +960,7 @@ func userSearchQuery(query string) (database.GetUsersParams, []httpapi.Error) {
 		// No filter
 		return database.GetUsersParams{}, nil
 	}
+	query = strings.ToLower(query)
 	// Because we do this in 2 passes, we want to maintain quotes on the first
 	// pass.Further splitting occurs on the second pass and quotes will be
 	// dropped.

coderd/users_internal_test.go

@@ -0,0 +1,124 @@
+package coderd
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/coder/coder/coderd/database"
+	"github.com/coder/coder/coderd/rbac"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSearchUsers(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		Name                  string
+		Query                 string
+		Expected              database.GetUsersParams
+		ExpectedErrorContains string
+	}{
+		{
+			Name:     "Empty",
+			Query:    "",
+			Expected: database.GetUsersParams{},
+		},
+		{
+			Name:  "Username",
+			Query: "user-name",
+			Expected: database.GetUsersParams{
+				Search:   "user-name",
+				Status:   []database.UserStatus{},
+				RbacRole: []string{},
+			},
+		},
+		{
+			Name:  "Username+Param",
+			Query: "usEr-name stAtus:actiVe",
+			Expected: database.GetUsersParams{
+				Search:   "user-name",
+				Status:   []database.UserStatus{database.UserStatusActive},
+				RbacRole: []string{},
+			},
+		},
+		{
+			Name:  "OnlyParams",
+			Query: "status:acTIve sEArch:User-Name role:Admin",
+			Expected: database.GetUsersParams{
+				Search:   "user-name",
+				Status:   []database.UserStatus{database.UserStatusActive},
+				RbacRole: []string{rbac.RoleAdmin()},
+			},
+		},
+		{
+			Name:  "QuotedParam",
+			Query: `status:SuSpenDeD sEArch:"User Name" role:meMber`,
+			Expected: database.GetUsersParams{
+				Search:   "user name",
+				Status:   []database.UserStatus{database.UserStatusSuspended},
+				RbacRole: []string{rbac.RoleMember()},
+			},
+		},
+		{
+			Name:  "QuotedKey",
+			Query: `"status":acTIve "sEArch":User-Name "role":Admin`,
+			Expected: database.GetUsersParams{
+				Search:   "user-name",
+				Status:   []database.UserStatus{database.UserStatusActive},
+				RbacRole: []string{rbac.RoleAdmin()},
+			},
+		},
+		{
+			// This will not return an error
+			Name:  "ExtraKeys",
+			Query: `foo:bar`,
+			Expected: database.GetUsersParams{
+				Search:   "",
+				Status:   []database.UserStatus{},
+				RbacRole: []string{},
+			},
+		},
+		{
+			// Quotes keep elements together
+			Name:  "QuotedSpecial",
+			Query: `search:"user:name"`,
+			Expected: database.GetUsersParams{
+				Search:   "user:name",
+				Status:   []database.UserStatus{},
+				RbacRole: []string{},
+			},
+		},
+
+		// Failures
+		{
+			Name:                  "ExtraColon",
+			Query:                 `search:name:extra`,
+			ExpectedErrorContains: "can only contain 1 ':'",
+		},
+		{
+			Name:                  "InvalidStatus",
+			Query:                 "status:inActive",
+			ExpectedErrorContains: "status: Query param \"status\" has invalid value: \"inactive\" is not a valid user status\n",
+		},
+	}
+
+	for _, c := range testCases {
+		c := c
+		t.Run(c.Name, func(t *testing.T) {
+			t.Parallel()
+			values, errs := userSearchQuery(c.Query)
+			if c.ExpectedErrorContains != "" {
+				require.True(t, len(errs) > 0, "expect some errors")
+				var s strings.Builder
+				for _, err := range errs {
+					_, _ = s.WriteString(fmt.Sprintf("%s: %s\n", err.Field, err.Detail))
+				}
+				require.Contains(t, s.String(), c.ExpectedErrorContains)
+			} else {
+				require.Len(t, errs, 0, "expected no error")
+				require.Equal(t, c.Expected, values, "expected values")
+			}
+		})
+	}
+}

coderd/users_test.go

@@ -732,6 +732,13 @@ func TestUsersFilter(t *testing.T) {
 			require.NoError(t, err, "suspend user")
 		}
 
+		if i%5 == 0 {
+			user, err = client.UpdateUserProfile(context.Background(), user.ID.String(), codersdk.UpdateUserProfileRequest{
+				Username: strings.ToUpper(user.Username),
+			})
+			require.NoError(t, err, "update username to uppercase")
+		}
+
 		users = append(users, user)
 	}
 
@@ -760,6 +767,15 @@ func TestUsersFilter(t *testing.T) {
 				return u.Status == codersdk.UserStatusActive
 			},
 		},
+		{
+			Name: "ActiveUppercase",
+			Filter: codersdk.UsersRequest{
+				Status: "ACTIVE",
+			},
+			FilterF: func(_ codersdk.UsersRequest, u codersdk.User) bool {
+				return u.Status == codersdk.UserStatusActive
+			},
+		},
 		{
 			Name: "Suspended",
 			Filter: codersdk.UsersRequest{
@@ -775,7 +791,7 @@ func TestUsersFilter(t *testing.T) {
 				Search: "a",
 			},
 			FilterF: func(_ codersdk.UsersRequest, u codersdk.User) bool {
-				return (strings.Contains(u.Username, "a") || strings.Contains(u.Email, "a"))
+				return (strings.ContainsAny(u.Username, "aA") || strings.ContainsAny(u.Email, "aA"))
 			},
 		},
 		{
@@ -793,6 +809,31 @@ func TestUsersFilter(t *testing.T) {
 				return false
 			},
 		},
+		{
+			Name: "AdminsUppercase",
+			Filter: codersdk.UsersRequest{
+				Role:   "ADMIN",
+				Status: codersdk.UserStatusSuspended + "," + codersdk.UserStatusActive,
+			},
+			FilterF: func(_ codersdk.UsersRequest, u codersdk.User) bool {
+				for _, r := range u.Roles {
+					if r.Name == rbac.RoleAdmin() {
+						return true
+					}
+				}
+				return false
+			},
+		},
+		{
+			Name: "Members",
+			Filter: codersdk.UsersRequest{
+				Role:   rbac.RoleMember(),
+				Status: codersdk.UserStatusSuspended + "," + codersdk.UserStatusActive,
+			},
+			FilterF: func(_ codersdk.UsersRequest, u codersdk.User) bool {
+				return true
+			},
+		},
 		{
 			Name: "SearchQuery",
 			Filter: codersdk.UsersRequest{
@@ -801,7 +842,22 @@ func TestUsersFilter(t *testing.T) {
 			FilterF: func(_ codersdk.UsersRequest, u codersdk.User) bool {
 				for _, r := range u.Roles {
 					if r.Name == rbac.RoleAdmin() {
-						return (strings.Contains(u.Username, "i") || strings.Contains(u.Email, "i")) &&
+						return (strings.ContainsAny(u.Username, "iI") || strings.ContainsAny(u.Email, "iI")) &&
+							u.Status == codersdk.UserStatusActive
+					}
+				}
+				return false
+			},
+		},
+		{
+			Name: "SearchQueryInsensitive",
+			Filter: codersdk.UsersRequest{
+				SearchQuery: "i Role:Admin STATUS:Active",
+			},
+			FilterF: func(_ codersdk.UsersRequest, u codersdk.User) bool {
+				for _, r := range u.Roles {
+					if r.Name == rbac.RoleAdmin() {
+						return (strings.ContainsAny(u.Username, "iI") || strings.ContainsAny(u.Email, "iI")) &&
 							u.Status == codersdk.UserStatusActive
 					}
 				}