diff --git a/coderd/audit.go b/coderd/audit.go index 8fc144ae0d148..493b5bec795f4 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -8,8 +8,6 @@ import ( "net" "net/http" "net/netip" - "net/url" - "strings" "time" "github.com/google/uuid" @@ -22,6 +20,7 @@ import ( "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/coderd/searchquery" "github.com/coder/coder/codersdk" ) @@ -49,7 +48,7 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) { } queryStr := r.URL.Query().Get("q") - filter, errs := auditSearchQuery(queryStr) + filter, errs := searchquery.AuditLogs(queryStr) if len(errs) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid audit search query.", @@ -373,125 +372,3 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit return "" } } - -// auditSearchQuery takes a query string and returns the auditLog filter. -// It also can return the list of validation errors to return to the api. -func auditSearchQuery(query string) (database.GetAuditLogsOffsetParams, []codersdk.ValidationError) { - searchParams := make(url.Values) - if query == "" { - // No filter - return database.GetAuditLogsOffsetParams{}, 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. - elements := splitQueryParameterByDelimiter(query, ' ', true) - for _, element := range elements { - parts := splitQueryParameterByDelimiter(element, ':', false) - switch len(parts) { - case 1: - // No key:value pair. - searchParams.Set("resource_type", parts[0]) - case 2: - searchParams.Set(parts[0], parts[1]) - default: - return database.GetAuditLogsOffsetParams{}, []codersdk.ValidationError{ - {Field: "q", Detail: fmt.Sprintf("Query element %q can only contain 1 ':'", element)}, - } - } - } - - // Using the query param parser here just returns consistent errors with - // other parsing. - parser := httpapi.NewQueryParamParser() - const layout = "2006-01-02" - - var ( - dateFromString = parser.String(searchParams, "", "date_from") - dateToString = parser.String(searchParams, "", "date_to") - parsedDateFrom, _ = time.Parse(layout, dateFromString) - parsedDateTo, _ = time.Parse(layout, dateToString) - ) - - if dateToString != "" { - parsedDateTo = parsedDateTo.Add(23*time.Hour + 59*time.Minute + 59*time.Second) // parsedDateTo goes to 23:59 - } - - if dateToString != "" && parsedDateTo.Before(parsedDateFrom) { - return database.GetAuditLogsOffsetParams{}, []codersdk.ValidationError{ - {Field: "q", Detail: fmt.Sprintf("DateTo value %q cannot be before than DateFrom", parsedDateTo)}, - } - } - - filter := database.GetAuditLogsOffsetParams{ - ResourceType: resourceTypeFromString(parser.String(searchParams, "", "resource_type")), - ResourceID: parser.UUID(searchParams, uuid.Nil, "resource_id"), - Action: actionFromString(parser.String(searchParams, "", "action")), - Username: parser.String(searchParams, "", "username"), - Email: parser.String(searchParams, "", "email"), - DateFrom: parsedDateFrom, - DateTo: parsedDateTo, - BuildReason: buildReasonFromString(parser.String(searchParams, "", "build_reason")), - } - - return filter, parser.Errors -} - -func resourceTypeFromString(resourceTypeString string) string { - switch codersdk.ResourceType(resourceTypeString) { - case codersdk.ResourceTypeTemplate: - return resourceTypeString - case codersdk.ResourceTypeTemplateVersion: - return resourceTypeString - case codersdk.ResourceTypeUser: - return resourceTypeString - case codersdk.ResourceTypeWorkspace: - return resourceTypeString - case codersdk.ResourceTypeWorkspaceBuild: - return resourceTypeString - case codersdk.ResourceTypeGitSSHKey: - return resourceTypeString - case codersdk.ResourceTypeAPIKey: - return resourceTypeString - case codersdk.ResourceTypeGroup: - return resourceTypeString - case codersdk.ResourceTypeLicense: - return resourceTypeString - } - return "" -} - -func actionFromString(actionString string) string { - switch codersdk.AuditAction(actionString) { - case codersdk.AuditActionCreate: - return actionString - case codersdk.AuditActionWrite: - return actionString - case codersdk.AuditActionDelete: - return actionString - case codersdk.AuditActionStart: - return actionString - case codersdk.AuditActionStop: - return actionString - case codersdk.AuditActionLogin: - return actionString - case codersdk.AuditActionLogout: - return actionString - default: - } - return "" -} - -func buildReasonFromString(buildReasonString string) string { - switch codersdk.BuildReason(buildReasonString) { - case codersdk.BuildReasonInitiator: - return buildReasonString - case codersdk.BuildReasonAutostart: - return buildReasonString - case codersdk.BuildReasonAutostop: - return buildReasonString - default: - } - return "" -} diff --git a/coderd/audit_test.go b/coderd/audit_test.go index 92a6f35a48e9d..cc8698775bd22 100644 --- a/coderd/audit_test.go +++ b/coderd/audit_test.go @@ -151,6 +151,7 @@ func TestAuditLogsFilter(t *testing.T) { Name string SearchQuery string ExpectedResult int + ExpectedError bool }{ { Name: "FilterByCreateAction", @@ -188,19 +189,19 @@ func TestAuditLogsFilter(t *testing.T) { ExpectedResult: 2, }, { - Name: "FilterInvalidSingleValue", - SearchQuery: "invalid", - ExpectedResult: 5, + Name: "FilterInvalidSingleValue", + SearchQuery: "invalid", + ExpectedError: true, }, { - Name: "FilterWithInvalidResourceType", - SearchQuery: "resource_type:invalid", - ExpectedResult: 5, + Name: "FilterWithInvalidResourceType", + SearchQuery: "resource_type:invalid", + ExpectedError: true, }, { - Name: "FilterWithInvalidAction", - SearchQuery: "action:invalid", - ExpectedResult: 5, + Name: "FilterWithInvalidAction", + SearchQuery: "action:invalid", + ExpectedError: true, }, { Name: "FilterOnCreateSingleDay", @@ -229,7 +230,7 @@ func TestAuditLogsFilter(t *testing.T) { }, { Name: "FilterOnWorkspaceBuildStartByInitiator", - SearchQuery: "resource_type:workspace_build action:start build_reason:start", + SearchQuery: "resource_type:workspace_build action:start build_reason:initiator", ExpectedResult: 1, }, } @@ -245,9 +246,13 @@ func TestAuditLogsFilter(t *testing.T) { Limit: 25, }, }) - require.NoError(t, err, "fetch audit logs") - require.Len(t, auditLogs.AuditLogs, testCase.ExpectedResult, "expected audit logs returned") - require.Equal(t, testCase.ExpectedResult, int(auditLogs.Count), "expected audit log count returned") + if testCase.ExpectedError { + require.Error(t, err, "expected error") + } else { + require.NoError(t, err, "fetch audit logs") + require.Len(t, auditLogs.AuditLogs, testCase.ExpectedResult, "expected audit logs returned") + require.Equal(t, testCase.ExpectedResult, int(auditLogs.Count), "expected audit log count returned") + } }) } }) diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index a4693643bd433..7fdbfde7ee493 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -966,13 +966,13 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. return nil, xerrors.Errorf("get provisioner job: %w", err) } - switch arg.Status { - case "pending": + switch database.WorkspaceStatus(arg.Status) { + case database.WorkspaceStatusPending: if !job.StartedAt.Valid { continue } - case "starting": + case database.WorkspaceStatusStarting: if !job.StartedAt.Valid && !job.CanceledAt.Valid && job.CompletedAt.Valid && @@ -981,7 +981,7 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. continue } - case "running": + case database.WorkspaceStatusRunning: if !job.CompletedAt.Valid && job.CanceledAt.Valid && job.Error.Valid || @@ -989,7 +989,7 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. continue } - case "stopping": + case database.WorkspaceStatusStopping: if !job.StartedAt.Valid && !job.CanceledAt.Valid && job.CompletedAt.Valid && @@ -998,7 +998,7 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. continue } - case "stopped": + case database.WorkspaceStatusStopped: if !job.CompletedAt.Valid && job.CanceledAt.Valid && job.Error.Valid || @@ -1006,23 +1006,23 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. continue } - case "failed": + case database.WorkspaceStatusFailed: if (!job.CanceledAt.Valid && !job.Error.Valid) || (!job.CompletedAt.Valid && !job.Error.Valid) { continue } - case "canceling": + case database.WorkspaceStatusCanceling: if !job.CanceledAt.Valid && job.CompletedAt.Valid { continue } - case "canceled": + case database.WorkspaceStatusCanceled: if !job.CanceledAt.Valid && !job.CompletedAt.Valid { continue } - case "deleted": + case database.WorkspaceStatusDeleted: if !job.StartedAt.Valid && job.CanceledAt.Valid && !job.CompletedAt.Valid && @@ -1031,7 +1031,7 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. continue } - case "deleting": + case database.WorkspaceStatusDeleting: if !job.CompletedAt.Valid && job.CanceledAt.Valid && job.Error.Valid && diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 44c598697ef8b..9a5051bd7d5ac 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -7,6 +7,33 @@ import ( "github.com/coder/coder/coderd/rbac" ) +type WorkspaceStatus string + +const ( + WorkspaceStatusPending WorkspaceStatus = "pending" + WorkspaceStatusStarting WorkspaceStatus = "starting" + WorkspaceStatusRunning WorkspaceStatus = "running" + WorkspaceStatusStopping WorkspaceStatus = "stopping" + WorkspaceStatusStopped WorkspaceStatus = "stopped" + WorkspaceStatusFailed WorkspaceStatus = "failed" + WorkspaceStatusCanceling WorkspaceStatus = "canceling" + WorkspaceStatusCanceled WorkspaceStatus = "canceled" + WorkspaceStatusDeleting WorkspaceStatus = "deleting" + WorkspaceStatusDeleted WorkspaceStatus = "deleted" +) + +func (s WorkspaceStatus) Valid() bool { + switch s { + case WorkspaceStatusPending, WorkspaceStatusStarting, WorkspaceStatusRunning, + WorkspaceStatusStopping, WorkspaceStatusStopped, WorkspaceStatusFailed, + WorkspaceStatusCanceling, WorkspaceStatusCanceled, WorkspaceStatusDeleting, + WorkspaceStatusDeleted: + return true + default: + return false + } +} + type AuditableGroup struct { Group Members []GroupMember `json:"members"` diff --git a/coderd/httpapi/queryparams.go b/coderd/httpapi/queryparams.go index e1fe2c38f0b55..b1a66d74184fa 100644 --- a/coderd/httpapi/queryparams.go +++ b/coderd/httpapi/queryparams.go @@ -5,12 +5,13 @@ import ( "net/url" "strconv" "strings" + "time" "github.com/google/uuid" + "golang.org/x/xerrors" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" - - "golang.org/x/xerrors" ) // QueryParamParser is a helper for parsing all query params and gathering all @@ -20,16 +21,38 @@ type QueryParamParser struct { // Errors is the set of errors to return via the API. If the length // of this set is 0, there are no errors!. Errors []codersdk.ValidationError + // Parsed is a map of all query params that were parsed. This is useful + // for checking if extra query params were passed in. + Parsed map[string]bool } func NewQueryParamParser() *QueryParamParser { return &QueryParamParser{ Errors: []codersdk.ValidationError{}, + Parsed: map[string]bool{}, + } +} + +// ErrorExcessParams checks if any query params were passed in that were not +// parsed. If so, it adds an error to the parser as these values are not valid +// query parameters. +func (p *QueryParamParser) ErrorExcessParams(values url.Values) { + for k := range values { + if _, ok := p.Parsed[k]; !ok { + p.Errors = append(p.Errors, codersdk.ValidationError{ + Field: k, + Detail: fmt.Sprintf("Query param %q is not a valid query param", k), + }) + } } } +func (p *QueryParamParser) addParsed(key string) { + p.Parsed[key] = true +} + func (p *QueryParamParser) Int(vals url.Values, def int, queryParam string) int { - v, err := parseQueryParam(vals, strconv.Atoi, def, queryParam) + v, err := parseQueryParam(p, vals, strconv.Atoi, def, queryParam) if err != nil { p.Errors = append(p.Errors, codersdk.ValidationError{ Field: queryParam, @@ -40,14 +63,16 @@ func (p *QueryParamParser) Int(vals url.Values, def int, queryParam string) int } func (p *QueryParamParser) UUIDorMe(vals url.Values, def uuid.UUID, me uuid.UUID, queryParam string) uuid.UUID { - if vals.Get(queryParam) == "me" { - return me - } - return p.UUID(vals, def, queryParam) + return ParseCustom(p, vals, def, queryParam, func(v string) (uuid.UUID, error) { + if v == "me" { + return me, nil + } + return uuid.Parse(v) + }) } func (p *QueryParamParser) UUID(vals url.Values, def uuid.UUID, queryParam string) uuid.UUID { - v, err := parseQueryParam(vals, uuid.Parse, def, queryParam) + v, err := parseQueryParam(p, vals, uuid.Parse, def, queryParam) if err != nil { p.Errors = append(p.Errors, codersdk.ValidationError{ Field: queryParam, @@ -58,54 +83,61 @@ func (p *QueryParamParser) UUID(vals url.Values, def uuid.UUID, queryParam strin } func (p *QueryParamParser) UUIDs(vals url.Values, def []uuid.UUID, queryParam string) []uuid.UUID { - v, err := parseQueryParam(vals, func(v string) ([]uuid.UUID, error) { - var badValues []string - strs := strings.Split(v, ",") - ids := make([]uuid.UUID, 0, len(strs)) - for _, s := range strs { - id, err := uuid.Parse(strings.TrimSpace(s)) - if err != nil { - badValues = append(badValues, v) - continue - } - ids = append(ids, id) - } + return ParseCustomList(p, vals, def, queryParam, func(v string) (uuid.UUID, error) { + return uuid.Parse(strings.TrimSpace(v)) + }) +} - if len(badValues) > 0 { - return []uuid.UUID{}, xerrors.Errorf("%s", strings.Join(badValues, ",")) - } - return ids, nil +func (p *QueryParamParser) Time(vals url.Values, def time.Time, queryParam string, format string) time.Time { + v, err := parseQueryParam(p, vals, func(term string) (time.Time, error) { + return time.Parse(format, term) }, def, queryParam) if err != nil { p.Errors = append(p.Errors, codersdk.ValidationError{ Field: queryParam, - Detail: fmt.Sprintf("Query param %q has invalid uuids: %q", queryParam, err.Error()), + Detail: fmt.Sprintf("Query param %q must be a valid date format (%s): %s", queryParam, format, err.Error()), }) } return v } -func (*QueryParamParser) String(vals url.Values, def string, queryParam string) string { - v, _ := parseQueryParam(vals, func(v string) (string, error) { +func (p *QueryParamParser) String(vals url.Values, def string, queryParam string) string { + v, _ := parseQueryParam(p, vals, func(v string) (string, error) { return v, nil }, def, queryParam) return v } -func (*QueryParamParser) Strings(vals url.Values, def []string, queryParam string) []string { - v, _ := parseQueryParam(vals, func(v string) ([]string, error) { - if v == "" { - return []string{}, nil - } - return strings.Split(v, ","), nil - }, def, queryParam) - return v +func (p *QueryParamParser) Strings(vals url.Values, def []string, queryParam string) []string { + return ParseCustomList(p, vals, def, queryParam, func(v string) (string, error) { + return v, nil + }) +} + +// ValidEnum parses enum query params. Add more to the list as needed. +type ValidEnum interface { + database.ResourceType | database.AuditAction | database.BuildReason | database.UserStatus | + database.WorkspaceStatus + + // Valid is required on the enum type to be used with ParseEnum. + Valid() bool +} + +// ParseEnum is a function that can be passed into ParseCustom that handles enum +// validation. +func ParseEnum[T ValidEnum](term string) (T, error) { + enum := T(term) + if enum.Valid() { + return enum, nil + } + var empty T + return empty, xerrors.Errorf("%q is not a valid value", term) } // ParseCustom has to be a function, not a method on QueryParamParser because generics // cannot be used on struct methods. func ParseCustom[T any](parser *QueryParamParser, vals url.Values, def T, queryParam string, parseFunc func(v string) (T, error)) T { - v, err := parseQueryParam(vals, parseFunc, def, queryParam) + v, err := parseQueryParam(parser, vals, parseFunc, def, queryParam) if err != nil { parser.Errors = append(parser.Errors, codersdk.ValidationError{ Field: queryParam, @@ -115,10 +147,41 @@ func ParseCustom[T any](parser *QueryParamParser, vals url.Values, def T, queryP return v } -func parseQueryParam[T any](vals url.Values, parse func(v string) (T, error), def T, queryParam string) (T, error) { +// ParseCustomList is a function that handles csv query params. +func ParseCustomList[T any](parser *QueryParamParser, vals url.Values, def []T, queryParam string, parseFunc func(v string) (T, error)) []T { + v, err := parseQueryParam(parser, vals, func(v string) ([]T, error) { + terms := strings.Split(v, ",") + var badValues []string + var output []T + for _, s := range terms { + good, err := parseFunc(s) + if err != nil { + badValues = append(badValues, s) + continue + } + output = append(output, good) + } + if len(badValues) > 0 { + return []T{}, xerrors.Errorf("%s", strings.Join(badValues, ",")) + } + + return output, nil + }, def, queryParam) + if err != nil { + parser.Errors = append(parser.Errors, codersdk.ValidationError{ + Field: queryParam, + Detail: fmt.Sprintf("Query param %q has invalid values: %s", queryParam, err.Error()), + }) + } + return v +} + +func parseQueryParam[T any](parser *QueryParamParser, vals url.Values, parse func(v string) (T, error), def T, queryParam string) (T, error) { + parser.addParsed(queryParam) if !vals.Has(queryParam) || vals.Get(queryParam) == "" { return def, nil } + str := vals.Get(queryParam) return parse(str) } diff --git a/coderd/httpapi/queryparams_test.go b/coderd/httpapi/queryparams_test.go index f4ff580b4dc22..6232bef22862d 100644 --- a/coderd/httpapi/queryparams_test.go +++ b/coderd/httpapi/queryparams_test.go @@ -5,10 +5,12 @@ import ( "net/http" "net/url" "testing" + "time" "github.com/google/uuid" "github.com/stretchr/testify/require" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" ) @@ -26,6 +28,68 @@ type queryParamTestCase[T any] struct { func TestParseQueryParams(t *testing.T) { t.Parallel() + t.Run("Enum", func(t *testing.T) { + t.Parallel() + + expParams := []queryParamTestCase[database.ResourceType]{ + { + QueryParam: "resource_type", + Value: string(database.ResourceTypeWorkspace), + Expected: database.ResourceTypeWorkspace, + }, + { + QueryParam: "bad_type", + Value: "foo", + ExpectedErrorContains: "not a valid value", + }, + } + + parser := httpapi.NewQueryParamParser() + testQueryParams(t, expParams, parser, func(vals url.Values, def database.ResourceType, queryParam string) database.ResourceType { + return httpapi.ParseCustom(parser, vals, def, queryParam, httpapi.ParseEnum[database.ResourceType]) + }) + }) + + t.Run("EnumList", func(t *testing.T) { + t.Parallel() + + expParams := []queryParamTestCase[[]database.ResourceType]{ + { + QueryParam: "resource_type", + Value: fmt.Sprintf("%s,%s", database.ResourceTypeWorkspace, database.ResourceTypeApiKey), + Expected: []database.ResourceType{database.ResourceTypeWorkspace, database.ResourceTypeApiKey}, + }, + } + + parser := httpapi.NewQueryParamParser() + testQueryParams(t, expParams, parser, func(vals url.Values, def []database.ResourceType, queryParam string) []database.ResourceType { + return httpapi.ParseCustomList(parser, vals, def, queryParam, httpapi.ParseEnum[database.ResourceType]) + }) + }) + + t.Run("Time", func(t *testing.T) { + t.Parallel() + const layout = "2006-01-02" + + expParams := []queryParamTestCase[time.Time]{ + { + QueryParam: "date", + Value: "2010-01-01", + Expected: must(time.Parse(layout, "2010-01-01")), + }, + { + QueryParam: "bad_date", + Value: "2010", + ExpectedErrorContains: "must be a valid date format", + }, + } + + parser := httpapi.NewQueryParamParser() + testQueryParams(t, expParams, parser, func(vals url.Values, def time.Time, queryParam string) time.Time { + return parser.Time(vals, time.Time{}, queryParam, layout) + }) + }) + t.Run("UUID", func(t *testing.T) { t.Parallel() me := uuid.New() @@ -43,12 +107,12 @@ func TestParseQueryParams(t *testing.T) { { QueryParam: "invalid_id", Value: "bogus", - ExpectedErrorContains: "must be a valid uuid", + ExpectedErrorContains: "invalid UUID length", }, { QueryParam: "long_id", Value: "afe39fbf-0f52-4a62-b0cc-58670145d773-123", - ExpectedErrorContains: "must be a valid uuid", + ExpectedErrorContains: "invalid UUID length", }, { QueryParam: "no_value", @@ -187,8 +251,8 @@ func testQueryParams[T any](t *testing.T, testCases []queryParamTestCase[T], par for _, c := range testCases { // !! Do not run these in parallel !! t.Run(c.QueryParam, func(t *testing.T) { - v := parse(v, c.Default, c.QueryParam) - require.Equal(t, c.Expected, v, fmt.Sprintf("param=%q value=%q", c.QueryParam, c.Value)) + value := parse(v, c.Default, c.QueryParam) + require.Equal(t, c.Expected, value, fmt.Sprintf("param=%q value=%q", c.QueryParam, c.Value)) if c.ExpectedErrorContains != "" { errors := parser.Errors require.True(t, len(errors) > 0, "error exist") @@ -199,3 +263,10 @@ func testQueryParams[T any](t *testing.T, testCases []queryParamTestCase[T], par }) } } + +func must[T any](value T, err error) T { + if err != nil { + panic(err) + } + return value +} diff --git a/coderd/searchquery/search.go b/coderd/searchquery/search.go new file mode 100644 index 0000000000000..419b73117598a --- /dev/null +++ b/coderd/searchquery/search.go @@ -0,0 +1,188 @@ +package searchquery + +import ( + "fmt" + "net/url" + "strings" + "time" + + "github.com/google/uuid" + + "golang.org/x/xerrors" + + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/codersdk" +) + +func AuditLogs(query string) (database.GetAuditLogsOffsetParams, []codersdk.ValidationError) { + // Always lowercase for all searches. + query = strings.ToLower(query) + values, errors := searchTerms(query, func(term string, values url.Values) error { + values.Add("resource_type", term) + return nil + }) + if len(errors) > 0 { + return database.GetAuditLogsOffsetParams{}, errors + } + + const dateLayout = "2006-01-02" + parser := httpapi.NewQueryParamParser() + filter := database.GetAuditLogsOffsetParams{ + ResourceID: parser.UUID(values, uuid.Nil, "resource_id"), + Username: parser.String(values, "", "username"), + Email: parser.String(values, "", "email"), + DateFrom: parser.Time(values, time.Time{}, "date_from", dateLayout), + DateTo: parser.Time(values, time.Time{}, "date_to", dateLayout), + ResourceType: string(httpapi.ParseCustom(parser, values, "", "resource_type", httpapi.ParseEnum[database.ResourceType])), + Action: string(httpapi.ParseCustom(parser, values, "", "action", httpapi.ParseEnum[database.AuditAction])), + BuildReason: string(httpapi.ParseCustom(parser, values, "", "build_reason", httpapi.ParseEnum[database.BuildReason])), + } + if !filter.DateTo.IsZero() { + filter.DateTo = filter.DateTo.Add(23*time.Hour + 59*time.Minute + 59*time.Second) + } + parser.ErrorExcessParams(values) + return filter, parser.Errors +} + +func Users(query string) (database.GetUsersParams, []codersdk.ValidationError) { + // Always lowercase for all searches. + query = strings.ToLower(query) + values, errors := searchTerms(query, func(term string, values url.Values) error { + values.Add("search", term) + return nil + }) + if len(errors) > 0 { + return database.GetUsersParams{}, errors + } + + parser := httpapi.NewQueryParamParser() + filter := database.GetUsersParams{ + Search: parser.String(values, "", "search"), + Status: httpapi.ParseCustomList(parser, values, []database.UserStatus{}, "status", httpapi.ParseEnum[database.UserStatus]), + RbacRole: parser.Strings(values, []string{}, "role"), + } + parser.ErrorExcessParams(values) + return filter, parser.Errors +} + +func Workspaces(query string, page codersdk.Pagination, agentInactiveDisconnectTimeout time.Duration) (database.GetWorkspacesParams, []codersdk.ValidationError) { + filter := database.GetWorkspacesParams{ + AgentInactiveDisconnectTimeoutSeconds: int64(agentInactiveDisconnectTimeout.Seconds()), + + Offset: int32(page.Offset), + Limit: int32(page.Limit), + } + + if query == "" { + return filter, nil + } + + // Always lowercase for all searches. + query = strings.ToLower(query) + values, errors := searchTerms(query, func(term string, values url.Values) error { + // It is a workspace name, and maybe includes an owner + parts := splitQueryParameterByDelimiter(term, '/', false) + switch len(parts) { + case 1: + values.Add("name", parts[0]) + case 2: + values.Add("owner", parts[0]) + values.Add("name", parts[1]) + default: + return xerrors.Errorf("Query element %q can only contain 1 '/'", term) + } + return nil + }) + if len(errors) > 0 { + return filter, errors + } + + parser := httpapi.NewQueryParamParser() + filter.OwnerUsername = parser.String(values, "", "owner") + filter.TemplateName = parser.String(values, "", "template") + filter.Name = parser.String(values, "", "name") + filter.Status = string(httpapi.ParseCustom(parser, values, "", "status", httpapi.ParseEnum[database.WorkspaceStatus])) + filter.HasAgent = parser.String(values, "", "has-agent") + parser.ErrorExcessParams(values) + return filter, parser.Errors +} + +func searchTerms(query string, defaultKey func(term string, values url.Values) error) (url.Values, []codersdk.ValidationError) { + searchValues := make(url.Values) + + // 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. + elements := splitQueryParameterByDelimiter(query, ' ', true) + for _, element := range elements { + if strings.HasPrefix(element, ":") || strings.HasSuffix(element, ":") { + return nil, []codersdk.ValidationError{ + { + Field: "q", + Detail: fmt.Sprintf("Query element %q cannot start or end with ':'", element), + }, + } + } + parts := splitQueryParameterByDelimiter(element, ':', false) + switch len(parts) { + case 1: + // No key:value pair. Use default behavior. + err := defaultKey(element, searchValues) + if err != nil { + return nil, []codersdk.ValidationError{ + {Field: "q", Detail: err.Error()}, + } + } + case 2: + searchValues.Add(strings.ToLower(parts[0]), parts[1]) + default: + return nil, []codersdk.ValidationError{ + { + Field: "q", + Detail: fmt.Sprintf("Query element %q can only contain 1 ':'", element), + }, + } + } + } + + for k := range searchValues { + if len(searchValues[k]) > 1 { + return nil, []codersdk.ValidationError{ + { + Field: "q", + Detail: fmt.Sprintf("Query parameter %q provided more than once, found %d times", k, len(searchValues[k])), + }, + } + } + } + + return searchValues, nil +} + +// splitQueryParameterByDelimiter takes a query string and splits it into the individual elements +// of the query. Each element is separated by a delimiter. All quoted strings are +// kept as a single element. +// +// Although all our names cannot have spaces, that is a validation error. +// We should still parse the quoted string as a single value so that validation +// can properly fail on the space. If we do not, a value of `template:"my name"` +// will search `template:"my name:name"`, which produces an empty list instead of +// an error. +// nolint:revive +func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes bool) []string { + quoted := false + parts := strings.FieldsFunc(query, func(r rune) bool { + if r == '"' { + quoted = !quoted + } + return !quoted && r == delimiter + }) + if !maintainQuotes { + for i, part := range parts { + parts[i] = strings.Trim(part, "\"") + } + } + + return parts +} diff --git a/coderd/searchquery/search_test.go b/coderd/searchquery/search_test.go new file mode 100644 index 0000000000000..bb7d135c055db --- /dev/null +++ b/coderd/searchquery/search_test.go @@ -0,0 +1,344 @@ +package searchquery_test + +import ( + "fmt" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/coderd/searchquery" + "github.com/coder/coder/codersdk" +) + +func TestSearchWorkspace(t *testing.T) { + t.Parallel() + testCases := []struct { + Name string + Query string + Expected database.GetWorkspacesParams + ExpectedErrorContains string + }{ + { + Name: "Empty", + Query: "", + Expected: database.GetWorkspacesParams{}, + }, + { + Name: "Owner/Name", + Query: "Foo/Bar", + Expected: database.GetWorkspacesParams{ + OwnerUsername: "foo", + Name: "bar", + }, + }, + { + Name: "Owner/NameWithSpaces", + Query: " Foo/Bar ", + Expected: database.GetWorkspacesParams{ + OwnerUsername: "foo", + Name: "bar", + }, + }, + { + Name: "Name", + Query: "workspace-name", + Expected: database.GetWorkspacesParams{ + Name: "workspace-name", + }, + }, + { + Name: "Name+Param", + Query: "workspace-name TEMPLATE:docker", + Expected: database.GetWorkspacesParams{ + Name: "workspace-name", + TemplateName: "docker", + }, + }, + { + Name: "OnlyParams", + Query: "name:workspace-name template:docker OWNER:Alice", + Expected: database.GetWorkspacesParams{ + Name: "workspace-name", + TemplateName: "docker", + OwnerUsername: "alice", + }, + }, + { + Name: "QuotedParam", + Query: `name:workspace-name template:"docker template" owner:alice`, + Expected: database.GetWorkspacesParams{ + Name: "workspace-name", + TemplateName: "docker template", + OwnerUsername: "alice", + }, + }, + { + Name: "QuotedKey", + Query: `"name":baz "template":foo "owner":bar`, + Expected: database.GetWorkspacesParams{ + Name: "baz", + TemplateName: "foo", + OwnerUsername: "bar", + }, + }, + { + // Quotes keep elements together + Name: "QuotedSpecial", + Query: `name:"workspace:name"`, + Expected: database.GetWorkspacesParams{ + Name: "workspace:name", + }, + }, + { + Name: "QuotedMadness", + Query: `"name":"foo:bar:baz/baz/zoo:zonk"`, + Expected: database.GetWorkspacesParams{ + Name: "foo:bar:baz/baz/zoo:zonk", + }, + }, + { + Name: "QuotedName", + Query: `"foo/bar"`, + Expected: database.GetWorkspacesParams{ + Name: "foo/bar", + }, + }, + { + Name: "QuotedOwner/Name", + Query: `"foo"/"bar"`, + Expected: database.GetWorkspacesParams{ + Name: "bar", + OwnerUsername: "foo", + }, + }, + + // Failures + { + Name: "NoPrefix", + Query: `:foo`, + ExpectedErrorContains: "cannot start or end", + }, + { + Name: "Double", + Query: `name:foo name:bar`, + ExpectedErrorContains: "provided more than once", + }, + { + Name: "ExtraSlashes", + Query: `foo/bar/baz`, + ExpectedErrorContains: "can only contain 1 '/'", + }, + { + Name: "ExtraColon", + Query: `owner:name:extra`, + ExpectedErrorContains: "can only contain 1 ':'", + }, + { + Name: "ExtraKeys", + Query: `foo:bar`, + ExpectedErrorContains: `Query param "foo" is not a valid query param`, + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + values, errs := searchquery.Workspaces(c.Query, codersdk.Pagination{}, 0) + 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") + } + }) + } + t.Run("AgentInactiveDisconnectTimeout", func(t *testing.T) { + t.Parallel() + + query := `` + timeout := 1337 * time.Second + values, errs := searchquery.Workspaces(query, codersdk.Pagination{}, timeout) + require.Empty(t, errs) + require.Equal(t, int64(timeout.Seconds()), values.AgentInactiveDisconnectTimeoutSeconds) + }) +} + +func TestSearchAudit(t *testing.T) { + t.Parallel() + testCases := []struct { + Name string + Query string + Expected database.GetAuditLogsOffsetParams + ExpectedErrorContains string + }{ + { + Name: "Empty", + Query: "", + Expected: database.GetAuditLogsOffsetParams{}, + }, + // Failures + { + Name: "ExtraColon", + Query: `search:name:extra`, + ExpectedErrorContains: "can only contain 1 ':'", + }, + { + Name: "ExtraKeys", + Query: `foo:bar`, + ExpectedErrorContains: `Query param "foo" is not a valid query param`, + }, + { + Name: "Dates", + Query: "date_from:2006", + ExpectedErrorContains: "valid date format", + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + values, errs := searchquery.AuditLogs(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") + } + }) + } +} + +func TestSearchUsers(t *testing.T) { + t.Parallel() + testCases := []struct { + Name string + Query string + Expected database.GetUsersParams + ExpectedErrorContains string + }{ + { + Name: "Empty", + Query: "", + Expected: database.GetUsersParams{ + Status: []database.UserStatus{}, + RbacRole: []string{}, + }, + }, + { + Name: "Username", + Query: "user-name", + Expected: database.GetUsersParams{ + Search: "user-name", + Status: []database.UserStatus{}, + RbacRole: []string{}, + }, + }, + { + Name: "UsernameWithSpaces", + 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:Owner", + Expected: database.GetUsersParams{ + Search: "user-name", + Status: []database.UserStatus{database.UserStatusActive}, + RbacRole: []string{rbac.RoleOwner()}, + }, + }, + { + 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":Owner`, + Expected: database.GetUsersParams{ + Search: "user-name", + Status: []database.UserStatus{database.UserStatusActive}, + RbacRole: []string{rbac.RoleOwner()}, + }, + }, + { + // 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: "has invalid values", + }, + { + Name: "ExtraKeys", + Query: `foo:bar`, + ExpectedErrorContains: `Query param "foo" is not a valid query param`, + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + values, errs := searchquery.Users(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") + } + }) + } +} diff --git a/coderd/users.go b/coderd/users.go index 10cb90d6a1a4b..8e080039db02b 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -6,8 +6,6 @@ import ( "errors" "fmt" "net/http" - "net/url" - "strings" "github.com/go-chi/chi/v5" "github.com/go-chi/render" @@ -21,6 +19,7 @@ import ( "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/coderd/searchquery" "github.com/coder/coder/coderd/telemetry" "github.com/coder/coder/coderd/userpassword" "github.com/coder/coder/coderd/util/slice" @@ -182,7 +181,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { func (api *API) users(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() query := r.URL.Query().Get("q") - params, errs := userSearchQuery(query) + params, errs := searchquery.Users(query) if len(errs) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid user search query.", @@ -1166,60 +1165,6 @@ func findUser(id uuid.UUID, users []database.User) *database.User { return nil } -func userSearchQuery(query string) (database.GetUsersParams, []codersdk.ValidationError) { - searchParams := make(url.Values) - if query == "" { - // 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. - elements := splitQueryParameterByDelimiter(query, ' ', true) - for _, element := range elements { - parts := splitQueryParameterByDelimiter(element, ':', false) - switch len(parts) { - case 1: - // No key:value pair. - searchParams.Set("search", parts[0]) - case 2: - searchParams.Set(parts[0], parts[1]) - default: - return database.GetUsersParams{}, []codersdk.ValidationError{ - {Field: "q", Detail: fmt.Sprintf("Query element %q can only contain 1 ':'", element)}, - } - } - } - - parser := httpapi.NewQueryParamParser() - filter := database.GetUsersParams{ - Search: parser.String(searchParams, "", "search"), - Status: httpapi.ParseCustom(parser, searchParams, []database.UserStatus{}, "status", parseUserStatus), - RbacRole: parser.Strings(searchParams, []string{}, "role"), - } - - return filter, parser.Errors -} - -// parseUserStatus ensures proper enums are used for user statuses -func parseUserStatus(v string) ([]database.UserStatus, error) { - var statuses []database.UserStatus - if v == "" { - return statuses, nil - } - parts := strings.Split(v, ",") - for _, part := range parts { - switch database.UserStatus(part) { - case database.UserStatusActive, database.UserStatusSuspended: - statuses = append(statuses, database.UserStatus(part)) - default: - return []database.UserStatus{}, xerrors.Errorf("%q is not a valid user status", part) - } - } - return statuses, nil -} - func convertAPIKey(k database.APIKey) codersdk.APIKey { return codersdk.APIKey{ ID: k.ID, diff --git a/coderd/users_internal_test.go b/coderd/users_internal_test.go deleted file mode 100644 index c44f499d4ca94..0000000000000 --- a/coderd/users_internal_test.go +++ /dev/null @@ -1,133 +0,0 @@ -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: "UsernameWithSpaces", - 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:Owner", - Expected: database.GetUsersParams{ - Search: "user-name", - Status: []database.UserStatus{database.UserStatusActive}, - RbacRole: []string{rbac.RoleOwner()}, - }, - }, - { - 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":Owner`, - Expected: database.GetUsersParams{ - Search: "user-name", - Status: []database.UserStatus{database.UserStatusActive}, - RbacRole: []string{rbac.RoleOwner()}, - }, - }, - { - // 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") - } - }) - } -} diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 4bafa506b42aa..f6aabd55ffe2c 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -7,10 +7,8 @@ import ( "errors" "fmt" "net/http" - "net/url" "sort" "strconv" - "strings" "time" "github.com/go-chi/chi/v5" @@ -19,7 +17,6 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" - "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/autobuild/schedule" "github.com/coder/coder/coderd/database" @@ -27,6 +24,7 @@ import ( "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/provisionerdserver" "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/coderd/searchquery" "github.com/coder/coder/coderd/telemetry" "github.com/coder/coder/coderd/tracing" "github.com/coder/coder/coderd/util/ptr" @@ -126,7 +124,7 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) { } queryStr := r.URL.Query().Get("q") - filter, errs := workspaceSearchQuery(queryStr, page, api.AgentInactiveDisconnectTimeout) + filter, errs := searchquery.Workspaces(queryStr, page, api.AgentInactiveDisconnectTimeout) if len(errs) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid workspace search query.", @@ -1240,89 +1238,6 @@ func validWorkspaceSchedule(s *string) (sql.NullString, error) { }, nil } -// workspaceSearchQuery takes a query string and returns the workspace filter. -// It also can return the list of validation errors to return to the api. -func workspaceSearchQuery(query string, page codersdk.Pagination, agentInactiveDisconnectTimeout time.Duration) (database.GetWorkspacesParams, []codersdk.ValidationError) { - filter := database.GetWorkspacesParams{ - AgentInactiveDisconnectTimeoutSeconds: int64(agentInactiveDisconnectTimeout.Seconds()), - - Offset: int32(page.Offset), - Limit: int32(page.Limit), - } - searchParams := make(url.Values) - if query == "" { - // No filter - return filter, 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. - elements := splitQueryParameterByDelimiter(query, ' ', true) - for _, element := range elements { - parts := splitQueryParameterByDelimiter(element, ':', false) - switch len(parts) { - case 1: - // No key:value pair. It is a workspace name, and maybe includes an owner - parts = splitQueryParameterByDelimiter(element, '/', false) - switch len(parts) { - case 1: - searchParams.Set("name", parts[0]) - case 2: - searchParams.Set("owner", parts[0]) - searchParams.Set("name", parts[1]) - default: - return database.GetWorkspacesParams{}, []codersdk.ValidationError{ - {Field: "q", Detail: fmt.Sprintf("Query element %q can only contain 1 '/'", element)}, - } - } - case 2: - searchParams.Set(parts[0], parts[1]) - default: - return database.GetWorkspacesParams{}, []codersdk.ValidationError{ - {Field: "q", Detail: fmt.Sprintf("Query element %q can only contain 1 ':'", element)}, - } - } - } - - // Using the query param parser here just returns consistent errors with - // other parsing. - parser := httpapi.NewQueryParamParser() - filter.OwnerUsername = parser.String(searchParams, "", "owner") - filter.TemplateName = parser.String(searchParams, "", "template") - filter.Name = parser.String(searchParams, "", "name") - filter.Status = parser.String(searchParams, "", "status") - filter.HasAgent = parser.String(searchParams, "", "has-agent") - return filter, parser.Errors -} - -// splitQueryParameterByDelimiter takes a query string and splits it into the individual elements -// of the query. Each element is separated by a delimiter. All quoted strings are -// kept as a single element. -// -// Although all our names cannot have spaces, that is a validation error. -// We should still parse the quoted string as a single value so that validation -// can properly fail on the space. If we do not, a value of `template:"my name"` -// will search `template:"my name:name"`, which produces an empty list instead of -// an error. -// nolint:revive -func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes bool) []string { - quoted := false - parts := strings.FieldsFunc(query, func(r rune) bool { - if r == '"' { - quoted = !quoted - } - return !quoted && r == delimiter - }) - if !maintainQuotes { - for i, part := range parts { - parts[i] = strings.Trim(part, "\"") - } - } - - return parts -} - func watchWorkspaceChannel(id uuid.UUID) string { return fmt.Sprintf("workspace:%s", id) } diff --git a/coderd/workspaces_internal_test.go b/coderd/workspaces_internal_test.go deleted file mode 100644 index 3cfa8ead5665d..0000000000000 --- a/coderd/workspaces_internal_test.go +++ /dev/null @@ -1,163 +0,0 @@ -package coderd - -import ( - "fmt" - "strings" - "testing" - "time" - - "github.com/coder/coder/coderd/database" - "github.com/coder/coder/codersdk" - - "github.com/stretchr/testify/require" -) - -func TestSearchWorkspace(t *testing.T) { - t.Parallel() - testCases := []struct { - Name string - Query string - Expected database.GetWorkspacesParams - ExpectedErrorContains string - }{ - { - Name: "Empty", - Query: "", - Expected: database.GetWorkspacesParams{}, - }, - { - Name: "Owner/Name", - Query: "Foo/Bar", - Expected: database.GetWorkspacesParams{ - OwnerUsername: "foo", - Name: "bar", - }, - }, - { - Name: "Owner/NameWithSpaces", - Query: " Foo/Bar ", - Expected: database.GetWorkspacesParams{ - OwnerUsername: "foo", - Name: "bar", - }, - }, - { - Name: "Name", - Query: "workspace-name", - Expected: database.GetWorkspacesParams{ - Name: "workspace-name", - }, - }, - { - Name: "Name+Param", - Query: "workspace-name TEMPLATE:docker", - Expected: database.GetWorkspacesParams{ - Name: "workspace-name", - TemplateName: "docker", - }, - }, - { - Name: "OnlyParams", - Query: "name:workspace-name template:docker OWNER:Alice", - Expected: database.GetWorkspacesParams{ - Name: "workspace-name", - TemplateName: "docker", - OwnerUsername: "alice", - }, - }, - { - Name: "QuotedParam", - Query: `name:workspace-name template:"docker template" owner:alice`, - Expected: database.GetWorkspacesParams{ - Name: "workspace-name", - TemplateName: "docker template", - OwnerUsername: "alice", - }, - }, - { - Name: "QuotedKey", - Query: `"name":baz "template":foo "owner":bar`, - Expected: database.GetWorkspacesParams{ - Name: "baz", - TemplateName: "foo", - OwnerUsername: "bar", - }, - }, - { - // This will not return an error - Name: "ExtraKeys", - Query: `foo:bar`, - Expected: database.GetWorkspacesParams{}, - }, - { - // Quotes keep elements together - Name: "QuotedSpecial", - Query: `name:"workspace:name"`, - Expected: database.GetWorkspacesParams{ - Name: "workspace:name", - }, - }, - { - Name: "QuotedMadness", - Query: `"name":"foo:bar:baz/baz/zoo:zonk"`, - Expected: database.GetWorkspacesParams{ - Name: "foo:bar:baz/baz/zoo:zonk", - }, - }, - { - Name: "QuotedName", - Query: `"foo/bar"`, - Expected: database.GetWorkspacesParams{ - Name: "foo/bar", - }, - }, - { - Name: "QuotedOwner/Name", - Query: `"foo"/"bar"`, - Expected: database.GetWorkspacesParams{ - Name: "bar", - OwnerUsername: "foo", - }, - }, - - // Failures - { - Name: "ExtraSlashes", - Query: `foo/bar/baz`, - ExpectedErrorContains: "can only contain 1 '/'", - }, - { - Name: "ExtraColon", - Query: `owner:name:extra`, - ExpectedErrorContains: "can only contain 1 ':'", - }, - } - - for _, c := range testCases { - c := c - t.Run(c.Name, func(t *testing.T) { - t.Parallel() - values, errs := workspaceSearchQuery(c.Query, codersdk.Pagination{}, 0) - 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") - } - }) - } - t.Run("AgentInactiveDisconnectTimeout", func(t *testing.T) { - t.Parallel() - - query := `foo:bar` - timeout := 1337 * time.Second - values, errs := workspaceSearchQuery(query, codersdk.Pagination{}, timeout) - require.Empty(t, errs) - require.Equal(t, int64(timeout.Seconds()), values.AgentInactiveDisconnectTimeoutSeconds) - }) -} diff --git a/site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx b/site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx index 4d73496ecf6c2..43fd72b2140af 100644 --- a/site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx +++ b/site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx @@ -176,6 +176,7 @@ const useStyles = makeStyles((theme) => ({ }, errorRoot: { color: theme.palette.error.main, + whiteSpace: "pre-wrap", }, inputStyles: { height: "100%", diff --git a/site/src/components/WorkspacesTable/WorkspacesTable.tsx b/site/src/components/WorkspacesTable/WorkspacesTable.tsx index 48ea5c8bbcf19..94eb6e5c7568f 100644 --- a/site/src/components/WorkspacesTable/WorkspacesTable.tsx +++ b/site/src/components/WorkspacesTable/WorkspacesTable.tsx @@ -20,12 +20,14 @@ export interface WorkspacesTableProps { workspaces?: Workspace[] isUsingFilter: boolean onUpdateWorkspace: (workspace: Workspace) => void + error?: Error | unknown } export const WorkspacesTable: FC = ({ workspaces, isUsingFilter, onUpdateWorkspace, + error, }) => { return ( @@ -44,6 +46,7 @@ export const WorkspacesTable: FC = ({ workspaces={workspaces} isUsingFilter={isUsingFilter} onUpdateWorkspace={onUpdateWorkspace} + error={error} /> diff --git a/site/src/components/WorkspacesTable/WorkspacesTableBody.tsx b/site/src/components/WorkspacesTable/WorkspacesTableBody.tsx index ed93e439c4551..3d58ff8395720 100644 --- a/site/src/components/WorkspacesTable/WorkspacesTableBody.tsx +++ b/site/src/components/WorkspacesTable/WorkspacesTableBody.tsx @@ -15,14 +15,19 @@ interface TableBodyProps { workspaces?: Workspace[] isUsingFilter: boolean onUpdateWorkspace: (workspace: Workspace) => void + error?: Error | unknown } export const WorkspacesTableBody: FC< React.PropsWithChildren -> = ({ workspaces, isUsingFilter, onUpdateWorkspace }) => { +> = ({ workspaces, isUsingFilter, onUpdateWorkspace, error }) => { const { t } = useTranslation("workspacesPage") const styles = useStyles() + if (error) { + return + } + if (!workspaces) { return } diff --git a/site/src/pages/AuditPage/AuditPage.tsx b/site/src/pages/AuditPage/AuditPage.tsx index cdc6cd0588127..d06808cb630ca 100644 --- a/site/src/pages/AuditPage/AuditPage.tsx +++ b/site/src/pages/AuditPage/AuditPage.tsx @@ -26,7 +26,7 @@ const AuditPage: FC = () => { }, }) - const { auditLogs, count } = auditState.context + const { auditLogs, count, apiError } = auditState.context const paginationRef = auditState.context.paginationRef as PaginationMachineRef const { audit_log: isAuditLogVisible } = useFeatureVisibility() @@ -45,6 +45,7 @@ const AuditPage: FC = () => { paginationRef={paginationRef} isNonInitialPage={nonInitialPage(searchParams)} isAuditLogVisible={isAuditLogVisible} + error={apiError} /> ) diff --git a/site/src/pages/AuditPage/AuditPageView.tsx b/site/src/pages/AuditPage/AuditPageView.tsx index ea2202ee7baa8..312cebfb79288 100644 --- a/site/src/pages/AuditPage/AuditPageView.tsx +++ b/site/src/pages/AuditPage/AuditPageView.tsx @@ -54,6 +54,7 @@ export interface AuditPageViewProps { paginationRef: PaginationMachineRef isNonInitialPage: boolean isAuditLogVisible: boolean + error?: Error | unknown } export const AuditPageView: FC = ({ @@ -64,8 +65,10 @@ export const AuditPageView: FC = ({ paginationRef, isNonInitialPage, isAuditLogVisible, + error, }) => { const { t } = useTranslation("auditLog") + const isLoading = auditLogs === undefined || count === undefined const isEmpty = !isLoading && auditLogs.length === 0 @@ -88,12 +91,21 @@ export const AuditPageView: FC = ({ filter={filter} onFilter={onFilter} presetFilters={presetFilters} + error={error} /> + {/* Error condition should just show an empty table. */} + + + + + + + diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx index a7d25e263b6f9..f112c7c1fa7d9 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx @@ -95,12 +95,14 @@ export const WorkspacesPageView: FC< filter={filter} onFilter={onFilter} presetFilters={presetFilters} + error={error} /> {count !== undefined && ( filter, }), + assignError: assign({ + apiError: (_, event) => event.data, + }), + clearError: assign({ + apiError: (_) => undefined, + }), displayApiError: (_, event) => { const message = getErrorMessage( event.data,