diff --git a/internal/clientcache/cmd/search/search.go b/internal/clientcache/cmd/search/search.go index e159b6f95..c3cf65b8d 100644 --- a/internal/clientcache/cmd/search/search.go +++ b/internal/clientcache/cmd/search/search.go @@ -10,6 +10,7 @@ import ( "math" "net/url" "os" + "slices" "strings" "time" @@ -19,12 +20,12 @@ import ( "github.com/hashicorp/boundary/api/sessions" "github.com/hashicorp/boundary/api/targets" cachecmd "github.com/hashicorp/boundary/internal/clientcache/cmd/cache" + "github.com/hashicorp/boundary/internal/clientcache/internal/cache" "github.com/hashicorp/boundary/internal/clientcache/internal/client" "github.com/hashicorp/boundary/internal/clientcache/internal/daemon" "github.com/hashicorp/boundary/internal/cmd/base" "github.com/mitchellh/cli" "github.com/posener/complete" - "golang.org/x/exp/slices" ) var ( @@ -39,6 +40,7 @@ var ( } errCacheNotRunning = stderrors.New("The cache process is not running.") + sortDirections = []string{"asc", "desc"} ) type SearchCommand struct { @@ -47,6 +49,8 @@ type SearchCommand struct { flagResource string flagForceRefresh bool flagMaxResultSetSize int64 + flagSortBy string + flagSortDirection string } func (c *SearchCommand) Synopsis() string { @@ -128,6 +132,18 @@ func (c *SearchCommand) Flags() *base.FlagSets { Usage: `Forces a refresh to be attempted prior to performing the search`, Hidden: true, }) + f.StringVar(&base.StringVar{ + Name: "sort-by", + Target: &c.flagSortBy, + Usage: `Specifies which column to sort resources by. Targets may be sorted by 'name'. Sessions may be sorted by 'created_time'. Use sort-direction to control which direction to sort results by.`, + Completion: complete.PredictSet(getSortableResources()...), + }) + f.StringVar(&base.StringVar{ + Name: "sort-direction", + Target: &c.flagSortDirection, + Usage: `Specifies which direction, 'asc' or 'desc', to sort results by. Requires sort-by and defaults to 'asc'.`, + Completion: complete.PredictSet(sortDirections...), + }) return set } @@ -163,10 +179,33 @@ func (c *SearchCommand) Run(args []string) int { c.PrintCliError(stderrors.New("Max result set size must be greater than or equal to -1")) return base.CommandUserError case c.flagMaxResultSetSize > math.MaxInt: - c.PrintCliError(stderrors.New(fmt.Sprintf("Max result set size must be less than or equal to the %v", math.MaxInt))) + c.PrintCliError(fmt.Errorf("Max result set size must be less than or equal to the %v", math.MaxInt)) return base.CommandUserError } + if c.flagSortDirection != "" { + if c.flagSortBy == "" { + c.PrintCliError(stderrors.New("sort-direction requires sort-by")) + return base.CommandUserError + } + if c.flagSortDirection != "asc" && c.flagSortDirection != "desc" { + c.PrintCliError(stderrors.New("sort-direction must be one of 'asc' or 'desc'")) + return base.CommandUserError + } + } + + if c.flagSortBy != "" { + sortableBy, ok := daemon.SortableColumnsForResource[cache.ToSearchableResource(c.flagResource)] + if !ok { + c.PrintCliError(fmt.Errorf("resource %q is not sortable", c.flagResource)) + return base.CommandUserError + } + if !slices.Contains(sortableBy, cache.SortBy(c.flagSortBy)) { + c.PrintCliError(fmt.Errorf("resource %q is not sortable by %q. %q is sortable by %v", c.flagResource, c.flagSortBy, c.flagResource, sortableBy)) + return base.CommandUserError + } + } + resp, result, apiErr, err := c.Search(ctx) if err != nil { c.PrintCliError(err) @@ -238,10 +277,10 @@ func (c *SearchCommand) Search(ctx context.Context) (*api.Response, *daemon.Sear if err != nil { return nil, nil, nil, err } - return search(ctx, dotPath, tf, opts...) + return search(ctx, dotPath, tf, c.flagSortBy, c.flagSortDirection, opts...) } -func search(ctx context.Context, daemonPath string, fb filterBy, opt ...client.Option) (*api.Response, *daemon.SearchResult, *api.Error, error) { +func search(ctx context.Context, daemonPath string, fb filterBy, sortBy string, sortDirection string, opt ...client.Option) (*api.Response, *daemon.SearchResult, *api.Error, error) { addr := daemon.SocketAddress(daemonPath) _, err := os.Stat(addr.Path) if addr.Scheme == "unix" && err != nil { @@ -263,6 +302,14 @@ func search(ctx context.Context, daemonPath string, fb filterBy, opt ...client.O if fb.maxResultSetSize != 0 { q.Add("max_result_set_size", fmt.Sprintf("%d", fb.maxResultSetSize)) } + + if sortBy != "" { + q.Add(daemon.SortByKey, sortBy) + if sortDirection != "" { + q.Add(daemon.SortDirectionKey, sortDirection) + } + } + resp, err := c.Get(ctx, "/v1/search", q, opt...) if err != nil { return nil, nil, nil, fmt.Errorf("Error when sending request to the cache: %w.", err) @@ -491,3 +538,11 @@ type filterBy struct { forceRefresh bool maxResultSetSize int } + +func getSortableResources() []string { + ret := []string{} + for sr := range daemon.SortableColumnsForResource { + ret = append(ret, string(sr)) + } + return ret +} diff --git a/internal/clientcache/cmd/search/search_test.go b/internal/clientcache/cmd/search/search_test.go index 4e0a7fcde..2acf0ae36 100644 --- a/internal/clientcache/cmd/search/search_test.go +++ b/internal/clientcache/cmd/search/search_test.go @@ -128,7 +128,7 @@ func TestSearch(t *testing.T) { for _, tc := range errorCases { t.Run(tc.name, func(t *testing.T) { - resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), tc.fb) + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), tc.fb, "", "") require.NoError(t, err) assert.NotNil(t, apiErr) assert.Contains(t, apiErr.Message, tc.apiErrContains) @@ -141,7 +141,7 @@ func TestSearch(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: at.Id, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -157,7 +157,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "name='name'", resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -173,7 +173,7 @@ func TestSearch(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: unsupportedAt.Id, resource: "targets", - }) + }, "", "") assert.NoError(t, err) require.NotNil(t, apiErr) assert.NotNil(t, resp) @@ -182,22 +182,27 @@ func TestSearch(t *testing.T) { assert.Contains(t, apiErr.Message, "doesn't support search") }) - srv.AddResources(t, at, []*aliases.Alias{ + expectedAliases := []*aliases.Alias{ {Id: "alt_1234567890", Value: "value1", DestinationId: "ttcp_1234567890"}, {Id: "alt_0987654321", Name: "value2", DestinationId: "ttcp_0987654321"}, - }, []*targets.Target{ + } + // These need to be sorted by name to preserve the sorting tests + expectedTargets := []*targets.Target{ {Id: "ttcp_1234567890", Name: "name1", Description: "description1"}, {Id: "ttcp_0987654321", Name: "name2", Description: "description2"}, - }, []*sessions.Session{ - {Id: "sess_1234567890", TargetId: "ttcp_1234567890", Status: "pending"}, - {Id: "sess_0987654321", TargetId: "ttcp_0987654321", Status: "pending"}, - }, boundaryTokenReaderFn) + } + expectedSessions := []*sessions.Session{ + {Id: "sess_0987654321", TargetId: "ttcp_0987654321", Status: "pending", CreatedTime: time.Now()}, + {Id: "sess_1234567890", TargetId: "ttcp_1234567890", Status: "pending", CreatedTime: time.Now().Add(-100 * time.Second)}, + } + + srv.AddResources(t, at, expectedAliases, expectedTargets, expectedSessions, boundaryTokenReaderFn) t.Run("target response from list", func(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: at.Id, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.Nil(t, apiErr) assert.NotNil(t, resp) @@ -208,7 +213,7 @@ func TestSearch(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: at.Id, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -221,7 +226,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "id % 'ttcp'", resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -234,7 +239,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "id % 'ttcp_1234567890'", resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -249,7 +254,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagFilter: `"/item/id" matches "ttcp"`, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -262,7 +267,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagFilter: `"/item/id" matches "ttcp_1234567890"`, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -276,7 +281,7 @@ func TestSearch(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: at.Id, resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -290,7 +295,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "id % 'sess'", resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -303,7 +308,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "id % 'sess_1234567890'", resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -316,7 +321,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagFilter: `"/item/id" matches "sess"`, resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -329,7 +334,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagFilter: `"/item/id" matches "sess_1234567890"`, resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -337,4 +342,73 @@ func TestSearch(t *testing.T) { assert.NotNil(t, r) assert.Len(t, r.Sessions, 1) }) + + t.Run("sorted targets from list", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + resource: "targets", + }, "name", "desc") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Equal(t, expectedTargets[1].Name, r.Targets[0].Name) + assert.Equal(t, expectedTargets[0].Name, r.Targets[1].Name) + }) + + t.Run("sorted targets ascending by default", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + resource: "targets", + }, "name", "") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Equal(t, expectedTargets[0].Name, r.Targets[0].Name) + assert.Equal(t, expectedTargets[1].Name, r.Targets[1].Name) + }) + + t.Run("sorted sessions from list", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + resource: "sessions", + }, "created_time", "desc") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Less(t, r.Sessions[1].CreatedTime, r.Sessions[0].CreatedTime) + }) + + t.Run("sorted sessions descending by default", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + resource: "sessions", + }, "created_time", "") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Less(t, r.Sessions[1].CreatedTime, r.Sessions[0].CreatedTime) + }) + + t.Run("sorted and filtered", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + flagFilter: `"/item/id" matches "ttcp"`, + resource: "targets", + }, "name", "desc") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Equal(t, expectedTargets[1].Name, r.Targets[0].Name) + assert.Equal(t, expectedTargets[0].Name, r.Targets[1].Name) + }) } diff --git a/internal/clientcache/internal/cache/options.go b/internal/clientcache/internal/cache/options.go index 41554ca3b..61455fc01 100644 --- a/internal/clientcache/internal/cache/options.go +++ b/internal/clientcache/internal/cache/options.go @@ -5,10 +5,21 @@ package cache import ( stderrors "errors" + "fmt" + "regexp" + "slices" "github.com/hashicorp/go-dbw" ) +// safeSortColumnRegex contains characters that could break SQL ORDER BY clauses +var safeSortColumnRegex = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`) + +var ( + errInvalidSortColumn = stderrors.New("not allowed for this resource type") + errUnsafeSortColumn = stderrors.New("contains unsafe characters") +) + type testRefreshWaitChs struct { firstSempahore chan struct{} secondSemaphore chan struct{} @@ -26,6 +37,8 @@ type options struct { withMaxResultSetSize int withTestRefreshWaitChs *testRefreshWaitChs withUseNonPagedListing bool + withSortBy SortBy // validated DB column name + withSortDirection SortDirection // "asc" or "desc" } // Option - how options are passed as args @@ -139,3 +152,26 @@ func WithUseNonPagedListing(b bool) Option { return nil } } + +// WithSort configures sorting for query results. +// Empty sortBy is silently ignored. Empty direction defaults to ascending in the repository layer. +// Validates column against sortableColumns and rejects SQL-unsafe characters. +func WithSort(sortBy SortBy, direction SortDirection, sortableColumns []SortBy) Option { + return func(o *options) error { + // ignore empty sortBy + if sortBy == SortByDefault { + return nil + } + + switch { + case !slices.Contains(sortableColumns, sortBy): + return fmt.Errorf("invalid sort column %q: %w", sortBy, errInvalidSortColumn) + case !safeSortColumnRegex.MatchString(string(sortBy)): + return fmt.Errorf("invalid sort column %q: %w", sortBy, errUnsafeSortColumn) + } + + o.withSortBy = sortBy + o.withSortDirection = direction + return nil + } +} diff --git a/internal/clientcache/internal/cache/options_test.go b/internal/clientcache/internal/cache/options_test.go index 7a511b927..eafd786d4 100644 --- a/internal/clientcache/internal/cache/options_test.go +++ b/internal/clientcache/internal/cache/options_test.go @@ -126,4 +126,105 @@ func Test_GetOpts(t *testing.T) { testOpts.withUseNonPagedListing = true assert.Equal(t, opts, testOpts) }) + t.Run("WithSort-default-sortby-ignored", func(t *testing.T) { + opts, err := getOpts(WithSort(SortByDefault, Ascending, []SortBy{SortByName})) + require.NoError(t, err) + testOpts := getDefaultOptions() + assert.Equal(t, opts, testOpts) + }) + t.Run("WithSort-empty-sortby-ignored", func(t *testing.T) { + opts, err := getOpts(WithSort("", Ascending, []SortBy{SortByName})) + require.NoError(t, err) + testOpts := getDefaultOptions() + assert.Equal(t, opts, testOpts) + }) + t.Run("WithSort-valid-name-ascending", func(t *testing.T) { + opts, err := getOpts(WithSort(SortByName, Ascending, []SortBy{SortByName, SortByCreatedTime})) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.withSortBy = SortByName + testOpts.withSortDirection = Ascending + assert.Equal(t, opts, testOpts) + }) + t.Run("WithSort-valid-created_time-descending", func(t *testing.T) { + opts, err := getOpts(WithSort(SortByCreatedTime, Descending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.withSortBy = SortByCreatedTime + testOpts.withSortDirection = Descending + assert.Equal(t, opts, testOpts) + }) + t.Run("WithSort-column-not-in-sortable-list", func(t *testing.T) { + _, err := getOpts(WithSort(SortByName, Ascending, []SortBy{SortByCreatedTime})) + require.Error(t, err) + assert.ErrorContains(t, err, errInvalidSortColumn.Error()) + }) + t.Run("WithSort-empty-sortable-columns", func(t *testing.T) { + _, err := getOpts(WithSort(SortByName, Ascending, []SortBy{})) + require.Error(t, err) + assert.ErrorContains(t, err, errInvalidSortColumn.Error()) + }) + t.Run("WithSort-nil-sortable-columns", func(t *testing.T) { + _, err := getOpts(WithSort(SortByName, Ascending, nil)) + require.Error(t, err) + assert.ErrorContains(t, err, errInvalidSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-semicolon", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name; DROP TABLE"), Ascending, []SortBy{SortBy("name; DROP TABLE")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-quote", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name'--"), Ascending, []SortBy{SortBy("name'--")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-double-quote", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name\"--"), Ascending, []SortBy{SortBy("name\"--")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-backslash", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name\\x00"), Ascending, []SortBy{SortBy("name\\x00")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-comma", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name,other"), Ascending, []SortBy{SortBy("name,other")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-parenthesis", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name("), Ascending, []SortBy{SortBy("name(")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-space", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name "), Ascending, []SortBy{SortBy("name ")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-tab", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name\t"), Ascending, []SortBy{SortBy("name\t")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-newline", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name\n"), Ascending, []SortBy{SortBy("name\n")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-unsafe-chars-dash", func(t *testing.T) { + _, err := getOpts(WithSort(SortBy("name-col"), Ascending, []SortBy{SortBy("name-col")})) + require.Error(t, err) + assert.ErrorContains(t, err, errUnsafeSortColumn.Error()) + }) + t.Run("WithSort-default-direction", func(t *testing.T) { + opts, err := getOpts(WithSort(SortByName, SortDirectionDefault, []SortBy{SortByName})) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.withSortBy = SortByName + testOpts.withSortDirection = SortDirectionDefault + assert.Equal(t, opts, testOpts) + }) } diff --git a/internal/clientcache/internal/cache/repository_sessions.go b/internal/clientcache/internal/cache/repository_sessions.go index bc053caa0..7a7d42019 100644 --- a/internal/clientcache/internal/cache/repository_sessions.go +++ b/internal/clientcache/internal/cache/repository_sessions.go @@ -19,6 +19,13 @@ import ( "github.com/hashicorp/mql" ) +// sessionSortFieldMap maps SortBy values to their corresponding JSON field paths. +// Sessions store data in a JSON blob, requiring json_extract for sorting. +var sessionSortFieldMap = map[SortBy]string{ + SortByCreatedTime: "created_time", + // Add future sortable fields here +} + // SessionRetrievalFunc is a function that retrieves sessions // from the provided boundary addr using the provided token. type SessionRetrievalFunc func(ctx context.Context, addr, authTok string, refreshTok RefreshTokenValue, inPage *sessions.SessionListResult, opt ...Option) (ret *sessions.SessionListResult, refreshToken RefreshTokenValue, err error) @@ -373,8 +380,32 @@ func (r *Repository) searchSessions(ctx context.Context, condition string, searc searchArgs = append(searchArgs, opts.withUserId) } + dbOpts := []db.Option{db.WithLimit(opts.withMaxResultSetSize + 1)} + + if opts.withSortBy != SortByDefault { + var direction string + switch opts.withSortDirection { + // default to descending + case Descending, SortDirectionDefault: + direction = "desc" + case Ascending: + direction = "asc" + default: + return nil, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("unsupported sort direction: %v", opts.withSortDirection)) + } + + jsonField, ok := sessionSortFieldMap[opts.withSortBy] + if !ok { + return nil, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("unsupported sort field for sessions: %v", opts.withSortBy)) + } + + // Sessions store data in a JSON blob (i.e. created_time), so we need to use json_extract for sorting + orderClause := fmt.Sprintf("json_extract(item, '$.%s') %s", jsonField, direction) + dbOpts = append(dbOpts, db.WithOrder(orderClause)) + } + var cachedSessions []*Session - if err := r.rw.SearchWhere(ctx, &cachedSessions, condition, searchArgs, db.WithLimit(opts.withMaxResultSetSize+1)); err != nil { + if err := r.rw.SearchWhere(ctx, &cachedSessions, condition, searchArgs, dbOpts...); err != nil { return nil, errors.Wrap(ctx, err, op) } diff --git a/internal/clientcache/internal/cache/repository_sessions_test.go b/internal/clientcache/internal/cache/repository_sessions_test.go index f2e79f955..2a11e720e 100644 --- a/internal/clientcache/internal/cache/repository_sessions_test.go +++ b/internal/clientcache/internal/cache/repository_sessions_test.go @@ -6,6 +6,7 @@ package cache import ( "context" "encoding/json" + "fmt" "strconv" "sync" "testing" @@ -643,3 +644,991 @@ func TestDefaultSessionRetrievalFunc(t *testing.T) { assert.Empty(t, got2.RemovedIds) assert.Empty(t, got2.Items) } + +func TestRepository_SearchSessionsSorting(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create sessions with different created_time times + ss := []*sessions.Session{ + { + Id: "ttcp_1", + Status: "status1", + Endpoint: "address1", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: time.Now().Add(-2 * time.Hour), + }, + { + Id: "ttcp_2", + Status: "status2", + Endpoint: "address2", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: time.Now().Add(-1 * time.Hour), + }, + { + Id: "ttcp_3", + Status: "status3", + Endpoint: "address3", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: time.Now(), + }, + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + searchService, err := NewSearchService(ctx, r) + require.NoError(t, err) + + t.Run("sort by created_time ascending", func(t *testing.T) { + params := SearchParams{ + Resource: Sessions, + AuthTokenId: kt.AuthTokenId, + SortBy: SortByCreatedTime, + SortDirection: Ascending, + } + result, err := searchService.Search(ctx, params) + require.NoError(t, err) + require.Len(t, result.Sessions, 3) + assert.Equal(t, "ttcp_1", result.Sessions[0].Id) + assert.Equal(t, "ttcp_2", result.Sessions[1].Id) + assert.Equal(t, "ttcp_3", result.Sessions[2].Id) + }) + + t.Run("sort by created_time descending", func(t *testing.T) { + params := SearchParams{ + Resource: Sessions, + AuthTokenId: kt.AuthTokenId, + SortBy: SortByCreatedTime, + SortDirection: Descending, + } + result, err := searchService.Search(ctx, params) + require.NoError(t, err) + require.Len(t, result.Sessions, 3) + assert.Equal(t, "ttcp_3", result.Sessions[0].Id) + assert.Equal(t, "ttcp_2", result.Sessions[1].Id) + assert.Equal(t, "ttcp_1", result.Sessions[2].Id) + }) + + t.Run("sort by created_time with default direction defaults to desc", func(t *testing.T) { + params := SearchParams{ + Resource: Sessions, + AuthTokenId: kt.AuthTokenId, + SortBy: SortByCreatedTime, + SortDirection: SortDirectionDefault, + } + result, err := searchService.Search(ctx, params) + require.NoError(t, err) + require.Len(t, result.Sessions, 3) + assert.Equal(t, "ttcp_3", result.Sessions[0].Id) + assert.Equal(t, "ttcp_2", result.Sessions[1].Id) + assert.Equal(t, "ttcp_1", result.Sessions[2].Id) + }) + + t.Run("no sort specified returns results", func(t *testing.T) { + params := SearchParams{ + Resource: Sessions, + AuthTokenId: kt.AuthTokenId, + } + result, err := searchService.Search(ctx, params) + require.NoError(t, err) + assert.Len(t, result.Sessions, 3) + }) + + t.Run("invalid sort column for sessions returns error", func(t *testing.T) { + params := SearchParams{ + Resource: Sessions, + AuthTokenId: kt.AuthTokenId, + SortBy: SortByName, // name is not valid for sessions + SortDirection: Ascending, + } + _, err := searchService.Search(ctx, params) + require.Error(t, err) + assert.ErrorContains(t, err, errInvalidSortColumn.Error()) + }) + + t.Run("sort with query", func(t *testing.T) { + params := SearchParams{ + Resource: Sessions, + AuthTokenId: kt.AuthTokenId, + Query: `(status % "status")`, + SortBy: SortByCreatedTime, + SortDirection: Descending, + } + result, err := searchService.Search(ctx, params) + require.NoError(t, err) + require.Len(t, result.Sessions, 3) + assert.Equal(t, "ttcp_3", result.Sessions[0].Id) + assert.Equal(t, "ttcp_2", result.Sessions[1].Id) + assert.Equal(t, "ttcp_1", result.Sessions[2].Id) + }) + + t.Run("sort with limit", func(t *testing.T) { + params := SearchParams{ + Resource: Sessions, + AuthTokenId: kt.AuthTokenId, + SortBy: SortByCreatedTime, + SortDirection: Descending, + MaxResultSetSize: 2, + } + result, err := searchService.Search(ctx, params) + require.NoError(t, err) + assert.Len(t, result.Sessions, 2) + assert.True(t, result.Incomplete) + assert.Equal(t, "ttcp_3", result.Sessions[0].Id) + assert.Equal(t, "ttcp_2", result.Sessions[1].Id) + }) +} + +func TestRepository_searchSessions_dbOptions(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create sessions with different created_time values for sorting tests + // These timestamps are intentionally different to verify sorting works correctly + now := time.Now() + ss := []*sessions.Session{ + { + Id: "ttcp_1", + Status: "status1", + Endpoint: "address1", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now.Add(-2 * time.Hour), // Oldest + }, + { + Id: "ttcp_2", + Status: "status2", + Endpoint: "address2", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now.Add(-1 * time.Hour), // Middle + }, + { + Id: "ttcp_3", + Status: "status3", + Endpoint: "address3", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now, // Newest + }, + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + tests := []struct { + name string + sortBy SortBy + sortDirection SortDirection + maxResultSetSize int + expectedOrder []string // expected session IDs in order - verifies json_extract sorting works + expectIncomplete bool + }{ + { + name: "default - no sorting", + sortBy: SortByDefault, + sortDirection: SortDirectionDefault, + maxResultSetSize: 10, + expectIncomplete: false, + }, + { + name: "sort by created_time ascending - verifies json_extract asc works", + sortBy: SortByCreatedTime, + sortDirection: Ascending, + maxResultSetSize: 10, + expectedOrder: []string{"ttcp_1", "ttcp_2", "ttcp_3"}, // oldest to newest + expectIncomplete: false, + }, + { + name: "sort by created_time descending - verifies json_extract desc works", + sortBy: SortByCreatedTime, + sortDirection: Descending, + maxResultSetSize: 10, + expectedOrder: []string{"ttcp_3", "ttcp_2", "ttcp_1"}, // newest to oldest + expectIncomplete: false, + }, + { + name: "sort ascending with limit - verifies LIMIT clause", + sortBy: SortByCreatedTime, + sortDirection: Ascending, + maxResultSetSize: 2, + expectedOrder: []string{"ttcp_1", "ttcp_2"}, + expectIncomplete: true, + }, + { + name: "sort descending with limit - verifies LIMIT clause", + sortBy: SortByCreatedTime, + sortDirection: Descending, + maxResultSetSize: 2, + expectedOrder: []string{"ttcp_3", "ttcp_2"}, + expectIncomplete: true, + }, + { + name: "no limit (-1)", + sortBy: SortByCreatedTime, + sortDirection: Ascending, + maxResultSetSize: -1, + expectedOrder: []string{"ttcp_1", "ttcp_2", "ttcp_3"}, + expectIncomplete: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Build options + opts := []Option{ + withAuthTokenId(at.Id), + WithMaxResultSetSize(tt.maxResultSetSize), + } + if tt.sortBy != SortByDefault { + opts = append(opts, WithSort(tt.sortBy, tt.sortDirection, []SortBy{SortByCreatedTime})) + } + + // Execute the search + result, err := r.searchSessions(ctx, "true", nil, opts...) + require.NoError(t, err) + require.NotNil(t, result) + + // Verify the result + if tt.expectIncomplete { + assert.True(t, result.Incomplete, "expected incomplete result") + assert.Len(t, result.Sessions, tt.maxResultSetSize, "expected limited results") + } else { + assert.False(t, result.Incomplete, "expected complete result") + } + + // Verify the order if sorting was applied + // This proves json_extract(item, '$.created_time') is working correctly + if tt.sortBy != SortByDefault && len(tt.expectedOrder) > 0 { + var actualOrder []string + for _, sess := range result.Sessions { + actualOrder = append(actualOrder, sess.Id) + } + assert.Equal(t, tt.expectedOrder[:len(actualOrder)], actualOrder, "sessions not in expected order") + + // Additionally verify the timestamps are actually in the correct order + if len(result.Sessions) > 1 { + for i := 0; i < len(result.Sessions)-1; i++ { + curr := result.Sessions[i].CreatedTime + next := result.Sessions[i+1].CreatedTime + if tt.sortDirection == Ascending { + assert.True(t, !curr.After(next), "timestamps should be in ascending order") + } else { + assert.True(t, !curr.Before(next), "timestamps should be in descending order") + } + } + } + } + }) + } +} + +func TestRepository_searchSessions_dbOptionsWithQuery(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create sessions with different created_time values + now := time.Now() + ss := []*sessions.Session{ + { + Id: "ttcp_1", + Status: "pending", + Endpoint: "address1", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now.Add(-2 * time.Hour), + }, + { + Id: "ttcp_2", + Status: "active", + Endpoint: "address2", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now.Add(-1 * time.Hour), + }, + { + Id: "ttcp_3", + Status: "active", + Endpoint: "address3", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now, + }, + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + tests := []struct { + name string + query string + sortBy SortBy + sortDirection SortDirection + maxResultSetSize int + expectedCount int + expectedOrder []string + expectIncomplete bool + }{ + { + name: "query with sort ascending", + query: `status % "active"`, + sortBy: SortByCreatedTime, + sortDirection: Ascending, + maxResultSetSize: 10, + expectedCount: 2, + expectedOrder: []string{"ttcp_2", "ttcp_3"}, + expectIncomplete: false, + }, + { + name: "query with sort descending", + query: `status % "active"`, + sortBy: SortByCreatedTime, + sortDirection: Descending, + maxResultSetSize: 10, + expectedCount: 2, + expectedOrder: []string{"ttcp_3", "ttcp_2"}, + expectIncomplete: false, + }, + { + name: "query with sort and limit", + query: `status % "active"`, + sortBy: SortByCreatedTime, + sortDirection: Descending, + maxResultSetSize: 1, + expectedCount: 1, + expectedOrder: []string{"ttcp_3"}, + expectIncomplete: true, + }, + { + name: "query without sort", + query: `status % "active"`, + sortBy: SortByDefault, + sortDirection: SortDirectionDefault, + maxResultSetSize: 10, + expectedCount: 2, + expectIncomplete: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Build options + opts := []Option{ + WithMaxResultSetSize(tt.maxResultSetSize), + } + if tt.sortBy != SortByDefault { + opts = append(opts, WithSort(tt.sortBy, tt.sortDirection, []SortBy{SortByCreatedTime})) + } + + // Execute the query + result, err := r.QuerySessions(ctx, at.Id, tt.query, opts...) + require.NoError(t, err) + require.NotNil(t, result) + + // Verify results + assert.Len(t, result.Sessions, tt.expectedCount) + assert.Equal(t, tt.expectIncomplete, result.Incomplete) + + // Verify order if expected + if len(tt.expectedOrder) > 0 { + var actualOrder []string + for _, sess := range result.Sessions { + actualOrder = append(actualOrder, sess.Id) + } + assert.Equal(t, tt.expectedOrder, actualOrder) + } + }) + } +} + +func TestRepository_searchSessions_limitBehavior(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create 10 sessions + var ss []*sessions.Session + for i := 0; i < 10; i++ { + ss = append(ss, &sessions.Session{ + Id: fmt.Sprintf("ttcp_%d", i), + Status: "active", + Endpoint: fmt.Sprintf("address%d", i), + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + }) + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + tests := []struct { + name string + maxResultSetSize int + expectedCount int + expectIncomplete bool + }{ + { + name: "limit of 5", + maxResultSetSize: 5, + expectedCount: 5, + expectIncomplete: true, + }, + { + name: "limit of 10 (exact match)", + maxResultSetSize: 10, + expectedCount: 10, + expectIncomplete: false, + }, + { + name: "limit of 15 (more than available)", + maxResultSetSize: 15, + expectedCount: 10, + expectIncomplete: false, + }, + { + name: "no limit (-1)", + maxResultSetSize: -1, + expectedCount: 10, + expectIncomplete: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Execute the list + result, err := r.ListSessions(ctx, at.Id, WithMaxResultSetSize(tt.maxResultSetSize)) + require.NoError(t, err) + require.NotNil(t, result) + + assert.Len(t, result.Sessions, tt.expectedCount) + assert.Equal(t, tt.expectIncomplete, result.Incomplete) + }) + } +} + +func TestRepository_searchSessions_dbOptionsVerifyOrderClause(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create a single session + ss := []*sessions.Session{ + { + Id: "ttcp_1", + Status: "active", + Endpoint: "address1", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + }, + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + t.Run("verify order clause format for ascending", func(t *testing.T) { + result, err := r.ListSessions(ctx, at.Id, + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, Ascending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 1) + }) + + t.Run("verify order clause format for descending", func(t *testing.T) { + result, err := r.ListSessions(ctx, at.Id, + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, Descending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 1) + }) + + t.Run("verify no order clause when sortBy is default", func(t *testing.T) { + result, err := r.ListSessions(ctx, at.Id, WithMaxResultSetSize(10)) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 1) + }) +} + +func TestRepository_searchSessions_sortingEdgeCases(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create test sessions + now := time.Now() + ss := []*sessions.Session{ + { + Id: "ttcp_1", + Status: "active", + Endpoint: "address1", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now.Add(-1 * time.Hour), + }, + { + Id: "ttcp_2", + Status: "pending", + Endpoint: "address2", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now, + }, + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + t.Run("unsupported sort field - SortByName not in sessionSortFieldMap", func(t *testing.T) { + // SortByName is not in sessionSortFieldMap, should return error + result, err := r.searchSessions(ctx, "true", nil, + withAuthTokenId(at.Id), + WithMaxResultSetSize(10), + WithSort(SortByName, Ascending, []SortBy{SortByName})) + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "unsupported sort field for sessions") + }) + + t.Run("invalid sort direction - unknown value", func(t *testing.T) { + // Using an invalid sort direction that's not in the switch statement + result, err := r.searchSessions(ctx, "true", nil, + withAuthTokenId(at.Id), + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, SortDirection("invalid"), []SortBy{SortByCreatedTime})) + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "unsupported sort direction") + }) + + t.Run("valid sort with SortDirectionDefault - should default to descending", func(t *testing.T) { + // SortDirectionDefault should be treated as ascending + result, err := r.searchSessions(ctx, "true", nil, + withAuthTokenId(at.Id), + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, SortDirectionDefault, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 2) + // Should be in descending order (newest first) + assert.Equal(t, "ttcp_2", result.Sessions[0].Id) + assert.Equal(t, "ttcp_1", result.Sessions[1].Id) + }) + + t.Run("sort with limit 1 - verify incomplete flag", func(t *testing.T) { + result, err := r.searchSessions(ctx, "true", nil, + withAuthTokenId(at.Id), + WithMaxResultSetSize(1), + WithSort(SortByCreatedTime, Descending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 1) + assert.True(t, result.Incomplete) + assert.Equal(t, "ttcp_2", result.Sessions[0].Id) // newest first + }) + + t.Run("sort with no results - empty result set", func(t *testing.T) { + // Query that matches nothing + result, err := r.QuerySessions(ctx, at.Id, `status % "nonexistent"`, + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, Ascending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 0) + assert.False(t, result.Incomplete) + }) +} + +func TestRepository_searchSessions_jsonExtractSafety(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create test session + ss := []*sessions.Session{ + { + Id: "ttcp_1", + Status: "active", + Endpoint: "address1", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: time.Now(), + }, + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + t.Run("verify json_extract uses safe field from sessionSortFieldMap", func(t *testing.T) { + // This test verifies that the json_extract uses the mapped field name + // from sessionSortFieldMap, which should be safe + result, err := r.searchSessions(ctx, "true", nil, + withAuthTokenId(at.Id), + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, Ascending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 1) + // If we got here without SQL errors, json_extract is working correctly + }) + + t.Run("verify sorting works with actual JSON data", func(t *testing.T) { + // Verify that the JSON item field contains valid data and sorting works + result, err := r.ListSessions(ctx, at.Id, + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, Descending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 1) + assert.Equal(t, "ttcp_1", result.Sessions[0].Id) + // Verify the session has all expected fields from JSON unmarshaling + assert.Equal(t, "active", result.Sessions[0].Status) + assert.Equal(t, "address1", result.Sessions[0].Endpoint) + }) +} + +func TestRepository_ListSessions_sortingValidation(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create test sessions + now := time.Now() + ss := []*sessions.Session{ + { + Id: "ttcp_1", + Status: "active", + Endpoint: "address1", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now.Add(-1 * time.Hour), + }, + { + Id: "ttcp_2", + Status: "pending", + Endpoint: "address2", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now, + }, + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + t.Run("ListSessions with unsupported sort field", func(t *testing.T) { + // Trying to sort by a field not in sessionSortFieldMap + result, err := r.ListSessions(ctx, at.Id, + WithMaxResultSetSize(10), + WithSort(SortByName, Ascending, []SortBy{SortByName})) + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "unsupported sort field for sessions") + }) + + t.Run("ListSessions with valid sort field", func(t *testing.T) { + result, err := r.ListSessions(ctx, at.Id, + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, Ascending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 2) + assert.Equal(t, "ttcp_1", result.Sessions[0].Id) + assert.Equal(t, "ttcp_2", result.Sessions[1].Id) + }) +} + +func TestRepository_QuerySessions_sortingValidation(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + addr := "address" + u := &user{ + Id: "u1", + Address: addr, + } + at := &authtokens.AuthToken{ + Id: "at_1", + Token: "at_1_token", + UserId: u.Id, + } + kt := KeyringToken{ + KeyringType: "k1", + TokenName: "t1", + AuthTokenId: at.Id, + } + atMap := map[ringToken]*authtokens.AuthToken{ + {"k1", "t1"}: at, + } + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(maps.Values(atMap))) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, addr, kt)) + + // Create test sessions + now := time.Now() + ss := []*sessions.Session{ + { + Id: "ttcp_1", + Status: "active", + Endpoint: "address1", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now.Add(-1 * time.Hour), + }, + { + Id: "ttcp_2", + Status: "active", + Endpoint: "address2", + ScopeId: "p_123", + TargetId: "ttcp_123", + UserId: "u_123", + Type: "tcp", + CreatedTime: now, + }, + } + require.NoError(t, r.refreshSessions(ctx, u, map[AuthToken]string{{Id: "id"}: "something"}, + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc(t, [][]*sessions.Session{ss}, [][]string{nil}))))) + + t.Run("QuerySessions with unsupported sort field", func(t *testing.T) { + result, err := r.QuerySessions(ctx, at.Id, `status % "active"`, + WithMaxResultSetSize(10), + WithSort(SortByName, Ascending, []SortBy{SortByName})) + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "unsupported sort field for sessions") + }) + + t.Run("QuerySessions with valid sort field and query", func(t *testing.T) { + result, err := r.QuerySessions(ctx, at.Id, `status % "active"`, + WithMaxResultSetSize(10), + WithSort(SortByCreatedTime, Descending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 2) + // Descending order - newest first + assert.Equal(t, "ttcp_2", result.Sessions[0].Id) + assert.Equal(t, "ttcp_1", result.Sessions[1].Id) + }) + + t.Run("QuerySessions with sort and limit causing incomplete", func(t *testing.T) { + result, err := r.QuerySessions(ctx, at.Id, `status % "active"`, + WithMaxResultSetSize(1), + WithSort(SortByCreatedTime, Ascending, []SortBy{SortByCreatedTime})) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Sessions, 1) + assert.True(t, result.Incomplete) + assert.Equal(t, "ttcp_1", result.Sessions[0].Id) // oldest first + }) +} diff --git a/internal/clientcache/internal/cache/repository_targets.go b/internal/clientcache/internal/cache/repository_targets.go index ce4805f36..f85be97bb 100644 --- a/internal/clientcache/internal/cache/repository_targets.go +++ b/internal/clientcache/internal/cache/repository_targets.go @@ -373,8 +373,18 @@ func (r *Repository) searchTargets(ctx context.Context, condition string, search searchArgs = append(searchArgs, opts.withUserId) } + dbOpts := []db.Option{db.WithLimit(opts.withMaxResultSetSize + 1)} + if opts.withSortBy != "" { + sd := opts.withSortDirection + if sd != Ascending && sd != Descending { + sd = Ascending + } + orderClause := fmt.Sprintf("%s %s", opts.withSortBy, sd) + dbOpts = append(dbOpts, db.WithOrder(orderClause)) + } + var cachedTargets []*Target - if err := r.rw.SearchWhere(ctx, &cachedTargets, condition, searchArgs, db.WithLimit(opts.withMaxResultSetSize+1)); err != nil { + if err := r.rw.SearchWhere(ctx, &cachedTargets, condition, searchArgs, dbOpts...); err != nil { return nil, errors.Wrap(ctx, err, op) } diff --git a/internal/clientcache/internal/cache/repository_targets_test.go b/internal/clientcache/internal/cache/repository_targets_test.go index 11a96631f..6bb968289 100644 --- a/internal/clientcache/internal/cache/repository_targets_test.go +++ b/internal/clientcache/internal/cache/repository_targets_test.go @@ -358,6 +358,27 @@ func TestRepository_ListTargets(t *testing.T) { assert.Len(t, l.Targets, len(ts)) assert.ElementsMatch(t, l.Targets, ts) }) + + t.Run("withSortBy sorts targets", func(t *testing.T) { + l, err := r.ListTargets(ctx, kt1.AuthTokenId, WithSort(SortByName, Descending, []SortBy{SortByName})) + assert.NoError(t, err) + assert.Equal(t, ts[2].Name, l.Targets[0].Name) + assert.Equal(t, ts[1].Name, l.Targets[1].Name) + assert.Equal(t, ts[0].Name, l.Targets[2].Name) + }) + + t.Run("withSortBy bad SortBy errors", func(t *testing.T) { + _, err := r.ListTargets(ctx, kt1.AuthTokenId, WithSort(SortByCreatedTime, Descending, []SortBy{SortByName})) + assert.Error(t, err) + }) + + t.Run("withSortBy bad SortDirection defaults to Ascending", func(t *testing.T) { + l, err := r.ListTargets(ctx, kt1.AuthTokenId, WithSort(SortByName, "Something else", []SortBy{SortByName})) + assert.NoError(t, err) + assert.Equal(t, ts[0].Name, l.Targets[0].Name) + assert.Equal(t, ts[1].Name, l.Targets[1].Name) + assert.Equal(t, ts[2].Name, l.Targets[2].Name) + }) } func TestRepository_ListTargetsLimiting(t *testing.T) { @@ -525,6 +546,25 @@ func TestRepository_QueryTargets(t *testing.T) { assert.Len(t, l.Targets, 2) assert.ElementsMatch(t, l.Targets, ts[0:2]) }) + + t.Run("withSortBy sorts targets", func(t *testing.T) { + l, err := r.QueryTargets(ctx, kt1.AuthTokenId, query, WithSort(SortByName, Descending, []SortBy{SortByName})) + assert.NoError(t, err) + assert.Equal(t, ts[1].Name, l.Targets[0].Name) + assert.Equal(t, ts[0].Name, l.Targets[1].Name) + }) + + t.Run("withSortBy bad SortBy errors", func(t *testing.T) { + _, err := r.QueryTargets(ctx, kt1.AuthTokenId, query, WithSort(SortByCreatedTime, Descending, []SortBy{SortByName})) + assert.Error(t, err) + }) + + t.Run("withSortBy bad SortDirection defaults to Ascending", func(t *testing.T) { + l, err := r.QueryTargets(ctx, kt1.AuthTokenId, query, WithSort(SortByName, "Something else", []SortBy{SortByName})) + assert.NoError(t, err) + assert.Equal(t, ts[0].Name, l.Targets[0].Name) + assert.Equal(t, ts[1].Name, l.Targets[1].Name) + }) } func TestRepository_QueryTargetsLimiting(t *testing.T) { diff --git a/internal/clientcache/internal/cache/search.go b/internal/clientcache/internal/cache/search.go index 2e3bcaa13..8742ca131 100644 --- a/internal/clientcache/internal/cache/search.go +++ b/internal/clientcache/internal/cache/search.go @@ -17,6 +17,40 @@ import ( "github.com/hashicorp/go-bexpr" ) +type SortBy string + +const ( + SortByDefault SortBy = "" + SortByName SortBy = "name" + SortByCreatedTime SortBy = "created_time" +) + +// Valid returns true if the SortBy value is a known good value +func (s SortBy) Valid() bool { + switch s { + case SortByDefault, SortByName, SortByCreatedTime: + return true + } + return false +} + +type SortDirection string + +const ( + SortDirectionDefault SortDirection = "" + Ascending SortDirection = "asc" + Descending SortDirection = "desc" +) + +// Valid returns true if the SortDirection value is a known good value +func (d SortDirection) Valid() bool { + switch d { + case SortDirectionDefault, Ascending, Descending: + return true + } + return false +} + type SearchableResource string const ( @@ -61,6 +95,10 @@ type SearchParams struct { Filter string // Max result set size is an override to the default max result set size MaxResultSetSize int + // Which column to sort results by, default is resource specific + SortBy SortBy + // Which direction to sort results by (asc, desc), default is resource specific + SortDirection SortDirection } // SearchResult returns the results from searching the cache. @@ -116,6 +154,7 @@ func NewSearchService(ctx context.Context, repo *Repository) (*SearchService, er } in.Targets = finalResults }, + sortableColumns: []SortBy{SortByName}, }, Sessions: &resourceSearchFns[*sessions.Session]{ list: repo.ListSessions, @@ -129,6 +168,7 @@ func NewSearchService(ctx context.Context, repo *Repository) (*SearchService, er } in.Sessions = finalResults }, + sortableColumns: []SortBy{SortByCreatedTime}, }, ImplicitScopes: &resourceSearchFns[*scopes.Scope]{ list: repo.ListImplicitScopes, @@ -184,6 +224,10 @@ func (s *SearchService) Search(ctx context.Context, params SearchParams) (*Searc return nil, errors.New(ctx, errors.InvalidParameter, op, "invalid resource") case params.AuthTokenId == "": return nil, errors.New(ctx, errors.InvalidParameter, op, "missing auth token id") + case !params.SortBy.Valid(): + return nil, errors.New(ctx, errors.InvalidParameter, op, "invalid sort by value") + case !params.SortDirection.Valid(): + return nil, errors.New(ctx, errors.InvalidParameter, op, "invalid sort direction value") } rSearcher, ok := s.searchableResources[params.Resource] if !ok { @@ -211,6 +255,8 @@ type resourceSearchFns[T any] struct { // filter takes results and a ready-to-use evaluator and filters the items // in the result filter func(*SearchResult, *bexpr.Evaluator) + // sortableColumns is a list of columns that can be used for sorting + sortableColumns []SortBy } // resourceSearcher is an interface that only resourceSearchFns[T] is expected @@ -229,13 +275,18 @@ type resourceSearcher interface { func (l *resourceSearchFns[T]) search(ctx context.Context, p SearchParams) (*SearchResult, error) { const op = "cache.(resourceSearchFns).search" + opts := []Option{WithMaxResultSetSize(p.MaxResultSetSize)} + if p.SortBy != SortByDefault { + opts = append(opts, WithSort(p.SortBy, p.SortDirection, l.sortableColumns)) + } + var found *SearchResult var err error switch p.Query { case "": - found, err = l.list(ctx, p.AuthTokenId, WithMaxResultSetSize(p.MaxResultSetSize)) + found, err = l.list(ctx, p.AuthTokenId, opts...) default: - found, err = l.query(ctx, p.AuthTokenId, p.Query, WithMaxResultSetSize(p.MaxResultSetSize)) + found, err = l.query(ctx, p.AuthTokenId, p.Query, opts...) } if err != nil { return nil, errors.Wrap(ctx, err, op) diff --git a/internal/clientcache/internal/cache/search_test.go b/internal/clientcache/internal/cache/search_test.go index 1440ae339..578e133f4 100644 --- a/internal/clientcache/internal/cache/search_test.go +++ b/internal/clientcache/internal/cache/search_test.go @@ -442,3 +442,166 @@ func TestSearch(t *testing.T) { assert.Equal(t, &SearchResult{Targets: []*targets.Target{}}, got) }) } + +func TestSortByValid(t *testing.T) { + cases := []struct { + sortBy SortBy + valid bool + }{ + {SortByDefault, true}, + {SortBy(""), true}, + {SortByName, true}, + {SortByCreatedTime, true}, + {SortBy("unknown"), false}, + {SortBy("id"), false}, + {SortBy("invalid_column"), false}, + {SortBy("name; DROP TABLE"), false}, + {SortBy("name'--"), false}, + {SortBy("name\"--"), false}, + {SortBy("name\\x00"), false}, + {SortBy("name,other"), false}, + {SortBy("name ("), false}, + {SortBy("name)"), false}, + {SortBy("name\t"), false}, + {SortBy("name\n"), false}, + {SortBy("name\r"), false}, + {SortBy("name "), false}, + } + + for _, tc := range cases { + t.Run(string(tc.sortBy), func(t *testing.T) { + assert.Equal(t, tc.valid, tc.sortBy.Valid()) + }) + } +} + +func TestSortDirectionValid(t *testing.T) { + cases := []struct { + direction SortDirection + valid bool + }{ + {SortDirectionDefault, true}, + {SortDirection(""), true}, + {Ascending, true}, + {Descending, true}, + {SortDirection("ASC"), false}, + {SortDirection("DESC"), false}, + {SortDirection("invalid"), false}, + } + + for _, tc := range cases { + t.Run(string(tc.direction), func(t *testing.T) { + assert.Equal(t, tc.valid, tc.direction.Valid()) + }) + } +} + +func TestSearch_Sorting(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + + at := &AuthToken{ + Id: "at_sort", + UserId: "u_sort", + } + { + u := &user{Id: at.UserId, Address: "address"} + rw := db.New(s) + require.NoError(t, rw.Create(ctx, u)) + require.NoError(t, rw.Create(ctx, at)) + + targets := []*Target{ + {FkUserId: u.Id, Id: "t_1", Name: "alpha", Type: "tcp", Item: `{"id": "t_1", "name": "alpha", "type": "tcp"}`}, + {FkUserId: u.Id, Id: "t_2", Name: "charlie", Type: "tcp", Item: `{"id": "t_2", "name": "charlie", "type": "tcp"}`}, + {FkUserId: u.Id, Id: "t_3", Name: "bravo", Type: "tcp", Item: `{"id": "t_3", "name": "bravo", "type": "tcp"}`}, + } + require.NoError(t, rw.CreateItems(ctx, targets)) + + sessions := []*Session{ + {FkUserId: u.Id, Id: "s_1", Endpoint: "one", Type: "tcp", UserId: "u123", Item: `{"id": "s_1", "endpoint": "one", "type": "tcp", "user_id": "u123"}`}, + {FkUserId: u.Id, Id: "s_2", Endpoint: "two", Type: "ssh", UserId: "u321", Item: `{"id": "s_2", "endpoint": "two", "type": "ssh", "user_id": "u321"}`}, + } + require.NoError(t, rw.CreateItems(ctx, sessions)) + + aliases := []*ResolvableAlias{ + {FkUserId: u.Id, Id: "alt_1", Value: "one", Type: "target", Item: `{"id": "alt_1", "value": "one", "type": "target"}`}, + } + require.NoError(t, rw.CreateItems(ctx, aliases)) + } + + r, err := NewRepository(ctx, s, &sync.Map{}, + mapBasedAuthTokenKeyringLookup(nil), + sliceBasedAuthTokenBoundaryReader(nil)) + require.NoError(t, err) + + ss, err := NewSearchService(ctx, r) + require.NoError(t, err) + + t.Run("no sort specified returns results", func(t *testing.T) { + got, err := ss.Search(ctx, SearchParams{ + Resource: Targets, + AuthTokenId: at.Id, + }) + require.NoError(t, err) + require.Len(t, got.Targets, 3) + }) + + t.Run("invalid sort by value", func(t *testing.T) { + got, err := ss.Search(ctx, SearchParams{ + Resource: Targets, + AuthTokenId: at.Id, + SortBy: SortBy("invalid_column"), + }) + assert.Error(t, err) + assert.ErrorContains(t, err, "invalid sort by value") + assert.Nil(t, got) + }) + + t.Run("invalid sort direction value", func(t *testing.T) { + got, err := ss.Search(ctx, SearchParams{ + Resource: Targets, + AuthTokenId: at.Id, + SortBy: SortByName, + SortDirection: SortDirection("invalid"), + }) + assert.Error(t, err) + assert.ErrorContains(t, err, "invalid sort direction value") + assert.Nil(t, got) + }) + + t.Run("sort column not allowed for resolvable aliases", func(t *testing.T) { + got, err := ss.Search(ctx, SearchParams{ + Resource: ResolvableAliases, + AuthTokenId: at.Id, + SortBy: SortByName, + SortDirection: Ascending, + }) + assert.Error(t, err) + assert.ErrorContains(t, err, "not allowed for this resource type") + assert.Nil(t, got) + }) + + t.Run("sessions reject name sort", func(t *testing.T) { + got, err := ss.Search(ctx, SearchParams{ + Resource: Sessions, + AuthTokenId: at.Id, + SortBy: SortByName, + SortDirection: Ascending, + }) + assert.Error(t, err) + assert.ErrorContains(t, err, "not allowed for this resource type") + assert.Nil(t, got) + }) + + t.Run("sessions accept created time sort", func(t *testing.T) { + got, err := ss.Search(ctx, SearchParams{ + Resource: Sessions, + AuthTokenId: at.Id, + SortBy: SortByCreatedTime, + SortDirection: Descending, + }) + require.NoError(t, err) + require.Len(t, got.Sessions, 2) + }) +} diff --git a/internal/clientcache/internal/daemon/search_handler.go b/internal/clientcache/internal/daemon/search_handler.go index 2d7e706f1..a54babd2c 100644 --- a/internal/clientcache/internal/daemon/search_handler.go +++ b/internal/clientcache/internal/daemon/search_handler.go @@ -9,7 +9,9 @@ import ( stderrors "errors" "fmt" "net/http" + "slices" "strconv" + "strings" "github.com/hashicorp/boundary/api" "github.com/hashicorp/boundary/api/aliases" @@ -57,8 +59,18 @@ const ( forceRefreshKey = "force_refresh" authTokenIdKey = "auth_token_id" maxResultSetSizeKey = "max_result_set_size" + // SortByKey The key used as the query parameter for which column to sort resources by + SortByKey = "sort_by" + // SortDirectionKey The key used as the query parameter for which direction, asc or desc, to sort resources by. + SortDirectionKey = "sort_direction" ) +// SortableColumnsForResource Defines which columns a given resource can be sorted by +var SortableColumnsForResource = map[cache.SearchableResource][]cache.SortBy{ + cache.Targets: {cache.SortByName}, + cache.Sessions: {cache.SortByCreatedTime}, +} + func newSearchHandlerFunc(ctx context.Context, repo *cache.Repository, refreshService *cache.RefreshService, logger hclog.Logger) (http.HandlerFunc, error) { const op = "daemon.newSearchHandlerFunc" switch { @@ -84,6 +96,8 @@ func newSearchHandlerFunc(ctx context.Context, repo *cache.Repository, refreshSe maxResultSetSizeInt, maxResultSetSizeIntErr := strconv.Atoi(maxResultSetSizeStr) query := q.Get(queryKey) filter := q.Get(filterKey) + sb := q.Get(SortByKey) + sd := q.Get(SortDirectionKey) searchableResource := cache.ToSearchableResource(resource) switch { @@ -121,6 +135,20 @@ func newSearchHandlerFunc(ctx context.Context, repo *cache.Repository, refreshSe return } + sortBy, valid := parseSortBy(sb, searchableResource) + if !valid { + event.WriteError(ctx, op, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("sort_by parameter %q not valid for resource %q", sb, searchableResource))) + writeError(w, fmt.Sprintf("sort_by parameter %q not valid for resource %q", sb, searchableResource), http.StatusBadRequest) + return + } + + sortDirection, valid := parseSortDirection(sd) + if !valid { + event.WriteError(ctx, op, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("sort_direction parameter %q not valid", sd))) + writeError(w, fmt.Sprintf("sort_direction parameter %q not valid ", sd), http.StatusBadRequest) + return + } + t, err := repo.LookupToken(reqCtx, authTokenId, cache.WithUpdateLastAccessedTime(true)) if err != nil || t == nil { if err != nil { @@ -175,6 +203,8 @@ func newSearchHandlerFunc(ctx context.Context, repo *cache.Repository, refreshSe Query: query, Filter: filter, MaxResultSetSize: maxResultSetSizeInt, + SortBy: sortBy, + SortDirection: sortDirection, }) if err != nil { event.WriteError(ctx, op, err, event.WithInfoMsg("when performing search", "auth_token_id", authTokenId, "resource", searchableResource, "query", query, "filter", filter)) @@ -237,3 +267,36 @@ func writeUnsupportedError(w http.ResponseWriter) { } http.Error(w, string(b), http.StatusBadRequest) } + +// Parses a raw sort direction string into a cache.SortDirection +// Returns the sort direction and whether the provided direction was valid or not +func parseSortDirection(sd string) (cache.SortDirection, bool) { + sd = strings.ToLower(sd) + switch sd { + case "asc", "ascending": + return cache.Ascending, true + case "desc", "descending": + return cache.Descending, true + case "": + return cache.SortDirectionDefault, true + default: + return cache.SortDirectionDefault, false + } +} + +// Parses a raw column name to sort by into a cache.SortBy +// Returns the column to sort by and whether the provided column was valid or not +func parseSortBy(sb string, sr cache.SearchableResource) (cache.SortBy, bool) { + sb = strings.ToLower(sb) + by := cache.SortBy(sb) + + if by == cache.SortByDefault { + return cache.SortByDefault, true + } + + sortableBys, ok := SortableColumnsForResource[sr] + if !ok || !slices.Contains(sortableBys, by) { + return cache.SortByDefault, false + } + return by, true +} diff --git a/internal/clientcache/internal/daemon/search_handler_test.go b/internal/clientcache/internal/daemon/search_handler_test.go new file mode 100644 index 000000000..57a49e9d1 --- /dev/null +++ b/internal/clientcache/internal/daemon/search_handler_test.go @@ -0,0 +1,60 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package daemon + +import ( + "testing" + + "github.com/hashicorp/boundary/internal/clientcache/internal/cache" + "github.com/stretchr/testify/assert" +) + +func TestParseSortBy(t *testing.T) { + testCases := []struct { + inputSb string + inputSr cache.SearchableResource + expectedValid bool + expectedSortBy cache.SortBy + }{ + {"name", cache.Targets, true, cache.SortByName}, + {"name", cache.Sessions, false, cache.SortByDefault}, + {"created_time", cache.Targets, false, cache.SortByDefault}, + {"created_time", cache.Sessions, true, cache.SortByCreatedTime}, + {"", cache.Targets, true, cache.SortByDefault}, + {"", cache.Sessions, true, cache.SortByDefault}, + {"ljkdhnsfg", cache.Targets, false, cache.SortByDefault}, + {"xcvbxcvb", cache.Sessions, false, cache.SortByDefault}, + {"name ", cache.Targets, false, cache.SortByDefault}, // Unicode no break space + {"name\u202e", cache.Targets, false, cache.SortByDefault}, // Unicode RtL override + {"\u202ename", cache.Targets, false, cache.SortByDefault}, // Unicode RtL override + } + for _, tc := range testCases { + actualSortBy, actualValid := parseSortBy(tc.inputSb, tc.inputSr) + assert.Equal(t, tc.expectedSortBy, actualSortBy) + assert.Equal(t, tc.expectedValid, actualValid) + } +} + +func TestParseSortDirection(t *testing.T) { + testCases := []struct { + inputSd string + expectedValid bool + expectedSortDirection cache.SortDirection + }{ + {"asc", true, cache.Ascending}, + {"ascending", true, cache.Ascending}, + {"desc", true, cache.Descending}, + {"descending", true, cache.Descending}, + {"", true, cache.SortDirectionDefault}, + {"asdasd", false, cache.SortDirectionDefault}, + {"asc ", false, cache.SortDirectionDefault}, + {"name\u202e", false, cache.SortDirectionDefault}, + {"\u202ename", false, cache.SortDirectionDefault}, + } + for _, tc := range testCases { + actualSortDirection, actualValid := parseSortDirection(tc.inputSd) + assert.Equal(t, tc.expectedSortDirection, actualSortDirection) + assert.Equal(t, tc.expectedValid, actualValid) + } +}