diff --git a/pkg/api/query/normalize_test.go b/pkg/api/query/normalize_test.go new file mode 100644 index 00000000000..e8386cee375 --- /dev/null +++ b/pkg/api/query/normalize_test.go @@ -0,0 +1,31 @@ +package query_test + +import ( + "testing" + + "github.com/shellhub-io/shellhub/pkg/api/query" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type listRequest struct { + query.Paginator + query.Sorter +} + +func TestEmbeddedAccessorsReachBothValues(t *testing.T) { + req := &listRequest{} + + paginated, ok := any(req).(query.Paginated) + require.True(t, ok, "a request embedding query.Paginator must satisfy query.Paginated") + + sorted, ok := any(req).(query.Sorted) + require.True(t, ok, "a request embedding query.Sorter must satisfy query.Sorted") + + paginated.GetPaginator().Normalize() + sorted.GetSorter().Normalize() + + assert.Equal(t, query.MinPage, req.Paginator.Page) + assert.Equal(t, query.DefaultPerPage, req.Paginator.PerPage) + assert.Equal(t, query.OrderDesc, req.Sorter.Order) +} diff --git a/pkg/api/query/paginator.go b/pkg/api/query/paginator.go index 8fdc242b2d7..851663c1a41 100644 --- a/pkg/api/query/paginator.go +++ b/pkg/api/query/paginator.go @@ -41,3 +41,18 @@ func (p *Paginator) Normalize() { p.PerPage = int(math.Max(math.Min(float64(p.PerPage), float64(MaxPerPage)), float64(MinPerPage))) } } + +// Paginated is a request that carries a [Paginator]. Every request type embedding one satisfies it, +// so a caller holding only the request can normalize the page without knowing its concrete type. +// +// The accessor exists because a request embedding both a [Paginator] and a [Sorter] promotes two +// Normalize methods at the same depth, which cancel each other out: neither is reachable through +// the outer type, and no interface over that name can be satisfied. +type Paginated interface { + GetPaginator() *Paginator +} + +// GetPaginator returns the paginator itself, satisfying [Paginated] for every type that embeds it. +func (p *Paginator) GetPaginator() *Paginator { + return p +} diff --git a/pkg/api/query/sorter.go b/pkg/api/query/sorter.go index 6e3688d4b00..cb9755d6d44 100644 --- a/pkg/api/query/sorter.go +++ b/pkg/api/query/sorter.go @@ -29,3 +29,14 @@ func (s *Sorter) Normalize() { s.Order = OrderDesc } } + +// Sorted is a request that carries a [Sorter]. It is the sorting half of [Paginated], and exists +// for the same reason: the promoted Normalize methods cancel out when both are embedded. +type Sorted interface { + GetSorter() *Sorter +} + +// GetSorter returns the sorter itself, satisfying [Sorted] for every type that embeds it. +func (s *Sorter) GetSorter() *Sorter { + return s +} diff --git a/pkg/api/requests/device.go b/pkg/api/requests/device.go index 382859c559d..7d46cb30b0b 100644 --- a/pkg/api/requests/device.go +++ b/pkg/api/requests/device.go @@ -10,6 +10,11 @@ import ( type DeviceList struct { TenantID string `header:"X-Tenant-ID"` DeviceStatus models.DeviceStatus `query:"status"` // TODO: validate + + // Connector asks for connector devices only. It is the caller's intent, not a filter: the + // service decides how to express it, and the default excludes them. + Connector bool `query:"connector"` + query.Paginator query.Sorter query.Filters diff --git a/pkg/api/requests/empty.go b/pkg/api/requests/empty.go new file mode 100644 index 00000000000..d47c5d3c6e6 --- /dev/null +++ b/pkg/api/requests/empty.go @@ -0,0 +1,5 @@ +package requests + +// Empty is the request of a route that takes no input. A handler is a function of its request, so +// a route with nothing to read still names the shape of what it reads. +type Empty struct{} diff --git a/server/api/pkg/gateway/actor.go b/server/api/pkg/gateway/actor.go new file mode 100644 index 00000000000..fdd663e7123 --- /dev/null +++ b/server/api/pkg/gateway/actor.go @@ -0,0 +1,29 @@ +package gateway + +// Actor is the authenticated identity a request carries: who is performing it, established before +// any namespace is considered. An actor is not yet a member of anything — resolving it within a +// namespace scope is what produces the acting member. +// +// Which fields are set follows the credential the request authenticated with. A user token names +// the acting person, filling ID and Username; an API key and a device token name a namespace +// principal with no person behind it, so both leave ID and Username empty. +type Actor struct { + // ID is the acting user's ID, empty when the credential names no person. + ID string + + // Username is the acting user's username. It is the only identifier an admin-console request + // carries, because that surface deliberately strips the user's ID. + Username string + + // APIKey is the key the request authenticated with, empty otherwise. + APIKey string + + // DeviceUID is the device the request authenticated as, empty otherwise. + DeviceUID string +} + +// IsZero reports whether the request carried no authenticated identity at all. A route that +// requires an actor refuses such a request; an anonymous route is the one place it is expected. +func (a Actor) IsZero() bool { + return a.ID == "" && a.Username == "" && a.APIKey == "" && a.DeviceUID == "" +} diff --git a/server/api/pkg/gateway/identity.go b/server/api/pkg/gateway/identity.go index b25372a4936..a0c69782c3b 100644 --- a/server/api/pkg/gateway/identity.go +++ b/server/api/pkg/gateway/identity.go @@ -2,6 +2,7 @@ package gateway import ( "net/http" + "slices" "github.com/shellhub-io/shellhub/pkg/api/authorizer" ) @@ -29,6 +30,13 @@ var identityHeaders = []string{ "X-Admin", } +// IdentityHeaders returns the headers [Identity.WriteTo] stamps. A request dispatched internally +// must carry them forward for the caller's identity to survive the hop, and reading them from here +// is what keeps that set from drifting out of step with the write. +func IdentityHeaders() []string { + return slices.Clone(identityHeaders) +} + // WriteTo stamps the identity onto header, clearing every identity header // first — including the ones this identity leaves empty. // @@ -63,6 +71,29 @@ func (i *Identity) WriteTo(header http.Header) { } } +// IdentityFrom reads back the identity [Identity.WriteTo] stamped onto header. +// +// It is the read side of that write, and the two must name the same headers. TestIdentityRoundTrip +// is what holds them together; a comment cannot. +func IdentityFrom(header http.Header) Identity { + return Identity{ + ID: header.Get("X-ID"), + Username: header.Get("X-Username"), + TenantID: header.Get("X-Tenant-ID"), + DeviceUID: header.Get("X-Device-UID"), + APIKey: header.Get("X-API-Key"), + Role: authorizer.RoleFromString(header.Get("X-Role")), + Admin: header.Get("X-Admin") == "true", + } +} + +// Actor returns the identity as the [Actor] a handler receives: who is performing the request, +// without the role and admin flag. Those decide what the caller may do, which the middleware +// answers before a handler runs. +func (i *Identity) Actor() Actor { + return Actor{ID: i.ID, Username: i.Username, APIKey: i.APIKey, DeviceUID: i.DeviceUID} +} + // WithoutUserScope returns the identity stripped of the acting user's ID and // namespace scope, keeping the admin flag. // diff --git a/server/api/pkg/gateway/identity_test.go b/server/api/pkg/gateway/identity_test.go new file mode 100644 index 00000000000..6bfeff84327 --- /dev/null +++ b/server/api/pkg/gateway/identity_test.go @@ -0,0 +1,84 @@ +package gateway + +import ( + "net/http" + "testing" + + "github.com/shellhub-io/shellhub/pkg/api/authorizer" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestIdentityRoundTrip is what keeps [Identity.WriteTo] and [IdentityFrom] naming the same headers. +// A field added to one and forgotten in the other survives review and the compiler, and shows up +// only as an identity that silently loses part of itself between the authenticator and the handler. +func TestIdentityRoundTrip(t *testing.T) { + cases := []struct { + description string + identity Identity + }{ + { + description: "a user token", + identity: Identity{ + ID: "user-id", + Username: "username", + TenantID: "00000000-0000-4000-0000-000000000000", + Role: authorizer.RoleOwner, + }, + }, + { + description: "an api key", + identity: Identity{ + TenantID: "00000000-0000-4000-0000-000000000000", + APIKey: "key", + Role: authorizer.RoleObserver, + }, + }, + { + description: "a device token", + identity: Identity{ + DeviceUID: "device-uid", + TenantID: "00000000-0000-4000-0000-000000000000", + }, + }, + { + description: "an admin browsing the admin console", + identity: Identity{ + Username: "username", + Role: authorizer.RoleOwner, + Admin: true, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(tt *testing.T) { + header := http.Header{} + tc.identity.WriteTo(header) + + require.Equal(tt, tc.identity, IdentityFrom(header)) + }) + } +} + +// TestIdentityActorDropsAuthorization pins what an actor deliberately is not. Role and admin decide +// what the caller may do, which the middleware owns; handing them to a handler invites it to make +// that decision a second time. +func TestIdentityActorDropsAuthorization(t *testing.T) { + identity := Identity{ + ID: "user-id", + Username: "username", + TenantID: "00000000-0000-4000-0000-000000000000", + DeviceUID: "device-uid", + APIKey: "key", + Role: authorizer.RoleOwner, + Admin: true, + } + + assert.Equal(t, Actor{ + ID: "user-id", + Username: "username", + APIKey: "key", + DeviceUID: "device-uid", + }, identity.Actor()) +} diff --git a/server/api/pkg/gateway/route.go b/server/api/pkg/gateway/route.go new file mode 100644 index 00000000000..af1296a44fd --- /dev/null +++ b/server/api/pkg/gateway/route.go @@ -0,0 +1,253 @@ +package gateway + +import ( + "context" + "net/http" + "reflect" + "runtime" + "sort" + "strconv" + "sync" + + "github.com/labstack/echo/v5" + "github.com/shellhub-io/shellhub/pkg/api/query" + "github.com/shellhub-io/shellhub/pkg/api/scope" + routes "github.com/shellhub-io/shellhub/server/api/routes/errors" +) + +const totalCountHeader = "X-Total-Count" + +// Shape names the response a wrapped handler produces. Every API resource operation answers with +// one of the three; a route that fits none of them is registered directly and named in the route +// table's exempt set. +type Shape string + +const ( + // ShapeOne answers with a JSON body. + ShapeOne Shape = "one" + // ShapeList answers with a JSON body and the total count of the collection. + ShapeList Shape = "list" + // ShapeNone answers with 200 and no body. + ShapeNone Shape = "none" +) + +// OneHandler answers with a single value. It is a function of its inputs: it does not know that +// HTTP exists, and cannot be called without the namespace it is bounded to and the actor +// performing it. +type OneHandler[T, R any] func(ctx context.Context, sc scope.Scope, actor Actor, req *T) (R, error) + +// ListHandler answers with a page of values and the size of the whole collection. The wrapper +// writes that count to the response, so no handler decides where the header goes. +type ListHandler[T, R any] func(ctx context.Context, sc scope.Scope, actor Actor, req *T) (R, int, error) + +// NoneHandler answers with success alone. +type NoneHandler[T any] func(ctx context.Context, sc scope.Scope, actor Actor, req *T) error + +// One registers handler as a route answering with a JSON body. +func One[T, R any](handler OneHandler[T, R], options ...RouteOption) echo.HandlerFunc { + declaration := declare(handler, ShapeOne, options) + + return func(c *echo.Context) error { + in, err := prepare[T](c, declaration) + if err != nil { + return err + } + + res, err := handler(in.ctx, in.scope, in.actor, in.req) + if err != nil { + return err + } + + return c.JSON(http.StatusOK, res) + } +} + +// List registers handler as a route answering with a JSON body and the total-count header. +func List[T, R any](handler ListHandler[T, R], options ...RouteOption) echo.HandlerFunc { + declaration := declare(handler, ShapeList, options) + + return func(c *echo.Context) error { + in, err := prepare[T](c, declaration) + if err != nil { + return err + } + + res, count, err := handler(in.ctx, in.scope, in.actor, in.req) + if err != nil { + return err + } + + c.Response().Header().Set(totalCountHeader, strconv.Itoa(count)) + + return c.JSON(http.StatusOK, res) + } +} + +// None registers handler as a route answering with 200 and no body. +func None[T any](handler NoneHandler[T], options ...RouteOption) echo.HandlerFunc { + declaration := declare(handler, ShapeNone, options) + + return func(c *echo.Context) error { + in, err := prepare[T](c, declaration) + if err != nil { + return err + } + + if err := handler(in.ctx, in.scope, in.actor, in.req); err != nil { + return err + } + + return c.NoContent(http.StatusOK) + } +} + +type inputs[T any] struct { + ctx context.Context + scope scope.Scope + actor Actor + req *T +} + +func prepare[T any](c *echo.Context, declaration Declaration) (inputs[T], error) { + gCtx, ok := From(c) + if !ok { + return inputs[T]{}, echo.ErrInternalServerError + } + + stash(c, gCtx) + + req := new(T) + if err := c.Bind(req); err != nil { + return inputs[T]{}, err + } + + if paginated, ok := any(req).(query.Paginated); ok { + paginated.GetPaginator().Normalize() + } + + if sorted, ok := any(req).(query.Sorted); ok { + sorted.GetSorter().Normalize() + } + + if err := c.Validate(req); err != nil { + return inputs[T]{}, err + } + + sc, err := declaration.resolveScope(gCtx) + if err != nil { + return inputs[T]{}, err + } + + actor, err := declaration.resolveActor(gCtx) + if err != nil { + return inputs[T]{}, err + } + + return inputs[T]{ctx: gCtx.Ctx(), scope: sc, actor: actor, req: req}, nil +} + +// RouteOption declares an exception to the two rules every route follows: it is bounded to a +// namespace, and it is performed by an actor. +type RouteOption func(*Declaration) + +// Unbounded declares that the route deliberately reads across namespaces, and records why that is +// safe. The reason is a required argument, so breadth cannot arrive by omission — only by someone +// typing why. +func Unbounded(reason string) RouteOption { + return func(d *Declaration) { + d.Unbounded, d.UnboundedReason = true, reason + } +} + +// Anonymous declares that the route deliberately carries no actor, and records why that is safe. +// It is independent of [Unbounded]: a device authenticating with its own token is bounded to a +// namespace and still carries no actor. +func Anonymous(reason string) RouteOption { + return func(d *Declaration) { + d.Anonymous, d.AnonymousReason = true, reason + } +} + +// Declaration is what one route registration claims about itself: the shape it answers with, and +// any exception it takes to the default rules. +type Declaration struct { + // Handler is the fully qualified name of the wrapped function, which is what ties a claim back + // to the code it is about. + Handler string + Shape Shape + + Unbounded bool + UnboundedReason string + + Anonymous bool + AnonymousReason string +} + +func (d Declaration) resolveScope(c *Context) (scope.Scope, error) { + if d.Unbounded { + return scope.NewUnbounded(d.UnboundedReason), nil + } + + return c.AdminOrScope() +} + +func (d Declaration) resolveActor(c *Context) (Actor, error) { + identity := IdentityFrom(c.Request().Header) + + actor := identity.Actor() + + if d.Anonymous || !actor.IsZero() { + return actor, nil + } + + return Actor{}, routes.NewErrUnauthorized(nil) +} + +var declarations = struct { + sync.Mutex + set map[Declaration]struct{} +}{set: make(map[Declaration]struct{})} + +func declare(handler any, shape Shape, options []RouteOption) Declaration { + d := Declaration{Handler: handlerName(handler), Shape: shape} + for _, option := range options { + option(&d) + } + + declarations.Lock() + defer declarations.Unlock() + + declarations.set[d] = struct{}{} + + return d +} + +// Declarations returns every claim the route tables built in this process have made, ordered by +// handler name. +func Declarations() []Declaration { + declarations.Lock() + defer declarations.Unlock() + + all := make([]Declaration, 0, len(declarations.set)) + for d := range declarations.set { + all = append(all, d) + } + + sort.Slice(all, func(i, j int) bool { return all[i].Handler < all[j].Handler }) + + return all +} + +func handlerName(handler any) string { + value := reflect.ValueOf(handler) + if value.Kind() != reflect.Func { + return "" + } + + fn := runtime.FuncForPC(value.Pointer()) + if fn == nil { + return "" + } + + return fn.Name() +} diff --git a/server/api/pkg/gateway/route_test.go b/server/api/pkg/gateway/route_test.go new file mode 100644 index 00000000000..3fed24258a6 --- /dev/null +++ b/server/api/pkg/gateway/route_test.go @@ -0,0 +1,374 @@ +package gateway_test + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/labstack/echo/v5" + "github.com/shellhub-io/shellhub/pkg/api/query" + "github.com/shellhub-io/shellhub/pkg/api/scope" + "github.com/shellhub-io/shellhub/pkg/errors" + "github.com/shellhub-io/shellhub/server/api/pkg/echo/handlers" + "github.com/shellhub-io/shellhub/server/api/pkg/gateway" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const probeTenant = "00000000-0000-4000-0000-000000000000" + +type probeRequest struct { + Count int `query:"count"` + Name string `query:"name" validate:"omitempty,min=3"` + query.Paginator + query.Sorter +} + +type probeCall struct { + called bool + scope scope.Scope + actor gateway.Actor + req *probeRequest + tenant string +} + +func probeRouter(t *testing.T, withGatewayContext bool) *echo.Echo { + t.Helper() + + e := echo.New() + e.Binder = handlers.NewBinder() + e.Validator = handlers.NewValidator() + e.HTTPErrorHandler = handlers.NewErrors(nil) + + if withGatewayContext { + e.Use(gateway.WithContext(nil)) + } + + return e +} + +func probeHandler(call *probeCall, res []string, count int, err error) gateway.ListHandler[probeRequest, []string] { + return func(ctx context.Context, sc scope.Scope, actor gateway.Actor, req *probeRequest) ([]string, int, error) { + call.called = true + call.scope = sc + call.actor = actor + call.req = req + + if tenant := gateway.TenantFromContext(ctx); tenant != nil { + call.tenant = tenant.ID + } + + return res, count, err + } +} + +func TestWrapperCeremony(t *testing.T) { + cases := []struct { + description string + withGatewayContext bool + headers map[string]string + target string + options []gateway.RouteOption + expectedStatus int + expectedCall bool + assert func(*testing.T, *probeCall) + }{ + { + description: "normalizes the paginator and the sorter before the handler sees them", + withGatewayContext: true, + headers: map[string]string{"X-Tenant-ID": probeTenant, "X-ID": "user-id"}, + target: "/probe?page=0&per_page=999&order_by=sideways", + expectedStatus: http.StatusOK, + expectedCall: true, + assert: func(t *testing.T, call *probeCall) { + t.Helper() + + assert.Equal(t, query.MinPage, call.req.Paginator.Page) + assert.Equal(t, query.MaxPerPage, call.req.Paginator.PerPage) + assert.Equal(t, query.OrderDesc, call.req.Sorter.Order) + }, + }, + { + description: "refuses a request whose query cannot bind", + withGatewayContext: true, + headers: map[string]string{"X-Tenant-ID": probeTenant, "X-ID": "user-id"}, + target: "/probe?count=not-a-number", + expectedStatus: http.StatusUnprocessableEntity, + }, + { + description: "refuses a request that fails validation", + withGatewayContext: true, + headers: map[string]string{"X-Tenant-ID": probeTenant, "X-ID": "user-id"}, + target: "/probe?name=ab", + expectedStatus: http.StatusBadRequest, + }, + { + description: "bounds the handler to the namespace the caller carries", + withGatewayContext: true, + headers: map[string]string{"X-Tenant-ID": probeTenant, "X-ID": "user-id"}, + target: "/probe", + expectedStatus: http.StatusOK, + expectedCall: true, + assert: func(t *testing.T, call *probeCall) { + t.Helper() + + assert.Equal(t, scope.MustBounded(probeTenant), call.scope) + }, + }, + { + description: "refuses a bounded route when the caller carries no namespace", + withGatewayContext: true, + headers: map[string]string{"X-ID": "user-id"}, + target: "/probe", + expectedStatus: http.StatusForbidden, + }, + { + description: "hands an unbounded route the reason its registration stated", + withGatewayContext: true, + headers: map[string]string{"X-ID": "user-id"}, + target: "/probe", + options: []gateway.RouteOption{gateway.Unbounded("the probe reads every namespace")}, + expectedStatus: http.StatusOK, + expectedCall: true, + assert: func(t *testing.T, call *probeCall) { + t.Helper() + + assert.False(t, call.scope.IsBounded()) + assert.Equal(t, "the probe reads every namespace", call.scope.Reason()) + }, + }, + { + description: "hands the handler the identity the request authenticated as", + withGatewayContext: true, + headers: map[string]string{ + "X-Tenant-ID": probeTenant, + "X-ID": "user-id", + "X-Username": "username", + }, + target: "/probe", + expectedStatus: http.StatusOK, + expectedCall: true, + assert: func(t *testing.T, call *probeCall) { + t.Helper() + + assert.Equal(t, gateway.Actor{ID: "user-id", Username: "username"}, call.actor) + }, + }, + { + description: "accepts an api key as the acting identity", + withGatewayContext: true, + headers: map[string]string{"X-Tenant-ID": probeTenant, "X-API-Key": "key"}, + target: "/probe", + expectedStatus: http.StatusOK, + expectedCall: true, + assert: func(t *testing.T, call *probeCall) { + t.Helper() + + assert.Equal(t, gateway.Actor{APIKey: "key"}, call.actor) + }, + }, + { + description: "refuses a route that requires an actor when the request carries none", + withGatewayContext: true, + headers: map[string]string{"X-Tenant-ID": probeTenant}, + target: "/probe", + expectedStatus: http.StatusUnauthorized, + }, + { + description: "runs an anonymous route with no actor at all", + withGatewayContext: true, + headers: map[string]string{"X-Tenant-ID": probeTenant}, + target: "/probe", + options: []gateway.RouteOption{gateway.Anonymous("the probe establishes the actor")}, + expectedStatus: http.StatusOK, + expectedCall: true, + assert: func(t *testing.T, call *probeCall) { + t.Helper() + + assert.True(t, call.actor.IsZero()) + }, + }, + { + description: "refuses the request when the gateway context is not installed", + withGatewayContext: false, + headers: map[string]string{"X-Tenant-ID": probeTenant, "X-ID": "user-id"}, + target: "/probe", + expectedStatus: http.StatusInternalServerError, + }, + { + description: "keeps the gateway context reachable from the request context", + withGatewayContext: true, + headers: map[string]string{"X-Tenant-ID": probeTenant, "X-ID": "user-id"}, + target: "/probe", + expectedStatus: http.StatusOK, + expectedCall: true, + assert: func(t *testing.T, call *probeCall) { + t.Helper() + + assert.Equal(t, probeTenant, call.tenant) + }, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + call := new(probeCall) + + e := probeRouter(t, tc.withGatewayContext) + e.GET("/probe", gateway.List(probeHandler(call, []string{"item"}, 1, nil), tc.options...)) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, tc.target, nil) + for name, value := range tc.headers { + req.Header.Set(name, value) + } + + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + require.Equal(t, tc.expectedStatus, rec.Code, rec.Body.String()) + require.Equal(t, tc.expectedCall, call.called) + + if tc.assert != nil { + tc.assert(t, call) + } + }) + } +} + +// TestListWritesTheTotalCountAfterTheErrorCheck pins the answer the twelve hand-written copies of +// this header disagreed on: the count the handler returned, and only once the call succeeded. +func TestListWritesTheTotalCountAfterTheErrorCheck(t *testing.T) { + cases := []struct { + description string + count int + err error + expectedStatus int + expectedCount string + }{ + { + description: "writes the count the handler returned", + count: 42, + expectedStatus: http.StatusOK, + expectedCount: "42", + }, + { + description: "writes no count when the handler failed", + count: 42, + err: errors.New("boom", "route", 3), + expectedStatus: http.StatusUnauthorized, + expectedCount: "", + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + call := new(probeCall) + + e := probeRouter(t, true) + e.GET("/probe", gateway.List(probeHandler(call, []string{"item"}, tc.count, tc.err))) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/probe", nil) + req.Header.Set("X-Tenant-ID", probeTenant) + req.Header.Set("X-ID", "user-id") + + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + require.Equal(t, tc.expectedStatus, rec.Code, rec.Body.String()) + assert.Equal(t, tc.expectedCount, rec.Header().Get("X-Total-Count")) + }) + } +} + +// TestWrapperNormalizesNothingForARequestCarryingNeither drives the case the accessors exist to +// distinguish: a request embedding no paginator and no sorter is passed through untouched, rather +// than normalized against values the wrapper invented. +func TestWrapperNormalizesNothingForARequestCarryingNeither(t *testing.T) { + type plainRequest struct { + UID string `query:"uid"` + } + + var got *plainRequest + + e := probeRouter(t, true) + e.GET("/probe", gateway.One(func(_ context.Context, _ scope.Scope, _ gateway.Actor, req *plainRequest) (string, error) { + got = req + + return "ok", nil + })) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/probe?uid=device&page=0&per_page=999", nil) + req.Header.Set("X-Tenant-ID", probeTenant) + req.Header.Set("X-ID", "user-id") + + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + require.NotNil(t, got) + assert.Equal(t, "device", got.UID) +} + +func TestOneEncodesTheHandlerResult(t *testing.T) { + e := probeRouter(t, true) + e.GET("/probe", gateway.One(func(_ context.Context, _ scope.Scope, _ gateway.Actor, _ *probeRequest) (map[string]string, error) { + return map[string]string{"name": "value"}, nil + })) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/probe", nil) + req.Header.Set("X-Tenant-ID", probeTenant) + req.Header.Set("X-ID", "user-id") + + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + assert.JSONEq(t, `{"name":"value"}`, rec.Body.String()) + assert.Empty(t, rec.Header().Get("X-Total-Count")) +} + +func TestNoneAnswersWithoutABody(t *testing.T) { + e := probeRouter(t, true) + e.GET("/probe", gateway.None(func(_ context.Context, _ scope.Scope, _ gateway.Actor, _ *probeRequest) error { + return nil + })) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/probe", nil) + req.Header.Set("X-Tenant-ID", probeTenant) + req.Header.Set("X-ID", "user-id") + + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + assert.Empty(t, rec.Body.String()) +} + +// TestDeclarationsRecordEveryClaim makes the claims a route table makes readable by a test: a route +// declaring breadth or anonymity must be able to show the reason it typed. +func TestDeclarationsRecordEveryClaim(t *testing.T) { + const unboundedReason = "the declared probe reads every namespace" + + e := probeRouter(t, true) + e.GET("/declared", gateway.List(probeHandler(new(probeCall), nil, 0, nil), + gateway.Unbounded(unboundedReason), + gateway.Anonymous("the declared probe establishes the actor"))) + + var found bool + + for _, declaration := range gateway.Declarations() { + if declaration.UnboundedReason != unboundedReason { + continue + } + + found = true + + assert.Equal(t, gateway.ShapeList, declaration.Shape) + assert.True(t, declaration.Anonymous) + assert.Equal(t, "the declared probe establishes the actor", declaration.AnonymousReason) + assert.NotEmpty(t, declaration.Handler) + } + + assert.True(t, found, "the wrapper recorded no declaration for the probe route") +} diff --git a/server/api/pkg/gateway/utils.go b/server/api/pkg/gateway/utils.go index 0fa9ec73574..90b0f26ff31 100644 --- a/server/api/pkg/gateway/utils.go +++ b/server/api/pkg/gateway/utils.go @@ -15,14 +15,16 @@ func Handler(next func(*Context) error) echo.HandlerFunc { return echo.ErrInternalServerError } - ctx := context.WithValue(c.Request().Context(), "ctx", gCtx) - - c.SetRequest(c.Request().WithContext(ctx)) + stash(c, gCtx) return next(gCtx) } } +func stash(c *echo.Context, gCtx *Context) { + c.SetRequest(c.Request().WithContext(context.WithValue(c.Request().Context(), "ctx", gCtx))) +} + // Middleware adapts echo middleware so it runs with a gateway [Context] in place. func Middleware(m echo.MiddlewareFunc) echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { diff --git a/server/api/routes/device.go b/server/api/routes/device.go index 684c4f56057..d212507192a 100644 --- a/server/api/routes/device.go +++ b/server/api/routes/device.go @@ -1,13 +1,15 @@ package routes import ( + "context" "net/http" - "strconv" "github.com/shellhub-io/shellhub/pkg/api/query" "github.com/shellhub-io/shellhub/pkg/api/requests" + "github.com/shellhub-io/shellhub/pkg/api/scope" "github.com/shellhub-io/shellhub/pkg/models" "github.com/shellhub-io/shellhub/server/api/pkg/gateway" + errs "github.com/shellhub-io/shellhub/server/api/routes/errors" "github.com/shellhub-io/shellhub/server/api/services" log "github.com/sirupsen/logrus" ) @@ -33,111 +35,31 @@ const ( ) // GetDeviceList serves the namespace's devices, filtered, sorted and paginated as requested. -func (h *Handler) GetDeviceList(c *gateway.Context) error { - req := new(requests.DeviceList) - - if err := c.Bind(req); err != nil { - return err - } - - req.Paginator.Normalize() - req.Sorter.Normalize() - +func (h *Handler) GetDeviceList(ctx context.Context, sc scope.Scope, _ gateway.Actor, req *requests.DeviceList) ([]models.Device, int, error) { if err := query.ValidateSorter(&req.Sorter, services.DeviceSortFields); err != nil { - return c.NoContent(http.StatusBadRequest) + log.WithError(err).WithField("sort_by", req.Sorter.By).Warn("failed to validate device list sorter") + + return nil, 0, errs.NewErrInvalidEntity(map[string]string{"sort_by": req.Sorter.By}) } if err := req.Filters.Unmarshal(); err != nil { log.WithError(err).WithField("filter", req.Filters.Raw).Warn("failed to decode device list filter") - return c.NoContent(http.StatusBadRequest) + return nil, 0, errs.NewErrInvalidEntity(map[string]string{"filter": "cannot be decoded"}) } if err := query.ValidateFilters(&req.Filters, services.DeviceFilterFields); err != nil { - return c.NoContent(http.StatusBadRequest) - } - - if c.QueryParam("connector") != "" { - filter := []query.Filter{ - { - Type: query.FilterTypeOperator, - Params: &query.FilterOperator{ - Name: "and", - }, - }, - { - Type: query.FilterTypeProperty, - Params: &query.FilterProperty{ - Name: "platform", - Operator: "eq", - Value: "connector", - }, - }, - } - - req.Filters.Data = append(req.Filters.Data, filter...) - } else { - filter := []query.Filter{ - { - Type: query.FilterTypeOperator, - Params: &query.FilterOperator{ - Name: "and", - }, - }, - { - Type: query.FilterTypeProperty, - Params: &query.FilterProperty{ - Name: "platform", - Operator: "ne", - Value: "connector", - }, - }, - } - - req.Filters.Data = append(req.Filters.Data, filter...) - } - - if err := c.Validate(req); err != nil { - return err - } - - sc, err := c.AdminOrScope() - if err != nil { - return err - } - - res, count, err := h.service.ListDevices(c.Ctx(), sc, req) - c.Response().Header().Set("X-Total-Count", strconv.Itoa(count)) + log.WithError(err).WithField("filter", req.Filters.Raw).Warn("failed to validate device list filter") - if err != nil { - return err + return nil, 0, errs.NewErrInvalidEntity(map[string]string{"filter": "is not valid"}) } - return c.JSON(http.StatusOK, res) + return h.service.ListDevices(ctx, sc, req) } // GetDevice serves a single device by UID. -func (h *Handler) GetDevice(c *gateway.Context) error { - var req requests.DeviceGet - if err := c.Bind(&req); err != nil { - return err - } - - if err := c.Validate(&req); err != nil { - return err - } - - sc, err := c.AdminOrScope() - if err != nil { - return err - } - - device, err := h.service.GetDevice(c.Ctx(), sc, models.UID(req.UID)) - if err != nil { - return err - } - - return c.JSON(http.StatusOK, device) +func (h *Handler) GetDevice(ctx context.Context, sc scope.Scope, _ gateway.Actor, req *requests.DeviceGet) (*models.Device, error) { + return h.service.GetDevice(ctx, sc, models.UID(req.UID)) } // ResolveDevice serves the device matching a name or SSHID, for callers that have a name diff --git a/server/api/routes/device_handler_test.go b/server/api/routes/device_handler_test.go new file mode 100644 index 00000000000..2dc6651043c --- /dev/null +++ b/server/api/routes/device_handler_test.go @@ -0,0 +1,180 @@ +package routes + +import ( + "encoding/base64" + "encoding/json" + "testing" + + "github.com/shellhub-io/shellhub/pkg/api/query" + "github.com/shellhub-io/shellhub/pkg/api/requests" + "github.com/shellhub-io/shellhub/pkg/api/scope" + "github.com/shellhub-io/shellhub/pkg/errors" + "github.com/shellhub-io/shellhub/pkg/models" + "github.com/shellhub-io/shellhub/server/api/pkg/gateway" + errs "github.com/shellhub-io/shellhub/server/api/routes/errors" + svc "github.com/shellhub-io/shellhub/server/api/services" + "github.com/shellhub-io/shellhub/server/api/services/mocks" + "github.com/stretchr/testify/assert" + gomock "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// TestGetDeviceHandler drives the handler as the pure function it is: no HTTP server, no status +// code, just the inputs the route table resolves and the service behind a mock. +func TestGetDeviceHandler(t *testing.T) { + cases := []struct { + description string + sc scope.Scope + uid string + requiredMocks func(*mocks.MockService) + expectedDevice *models.Device + expectedErr error + }{ + { + description: "passes the namespace scope it was given through to the service", + sc: scope.MustBounded("00000000-0000-4000-0000-000000000000"), + uid: "uid", + requiredMocks: func(service *mocks.MockService) { + service. + On("GetDevice", gomock.Anything, scope.MustBounded("00000000-0000-4000-0000-000000000000"), models.UID("uid")). + Return(&models.Device{UID: "uid"}, nil). + Once() + }, + expectedDevice: &models.Device{UID: "uid"}, + }, + { + description: "reports the service's failure unchanged", + sc: scope.NewUnbounded("the admin console reads every namespace"), + uid: "missing", + requiredMocks: func(service *mocks.MockService) { + service. + On("GetDevice", gomock.Anything, scope.NewUnbounded("the admin console reads every namespace"), models.UID("missing")). + Return(nil, svc.ErrDeviceNotFound). + Once() + }, + expectedErr: svc.ErrDeviceNotFound, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + service := mocks.NewMockService(t) + tc.requiredMocks(service) + + handler := NewHandler(service, nil) + + device, err := handler.GetDevice(t.Context(), tc.sc, gateway.Actor{ID: "user-id"}, + &requests.DeviceGet{DeviceParam: requests.DeviceParam{UID: tc.uid}}) + + require.Equal(t, tc.expectedErr, err) + require.Equal(t, tc.expectedDevice, device) + }) + } +} + +// TestGetDeviceListHandler covers what the device list handler still decides now that the wrapper +// owns the ceremony: the sort field, the caller's encoded filter, and nothing else. +func TestGetDeviceListHandler(t *testing.T) { + const tenantID = "00000000-0000-4000-0000-000000000000" + + encode := func(t *testing.T, filters []query.Filter) string { + t.Helper() + + raw, err := json.Marshal(filters) + require.NoError(t, err) + + return base64.StdEncoding.EncodeToString(raw) + } + + cases := []struct { + description string + req func(*testing.T) *requests.DeviceList + requiredMocks func(*mocks.MockService) + expectedFields map[string]string + }{ + { + description: "refuses a sort field the device list does not accept", + req: func(*testing.T) *requests.DeviceList { + return &requests.DeviceList{Sorter: query.Sorter{By: "not_a_column", Order: query.OrderAsc}} + }, + requiredMocks: func(*mocks.MockService) {}, + expectedFields: map[string]string{"sort_by": "not_a_column"}, + }, + { + description: "refuses a filter that is not valid base64", + req: func(*testing.T) *requests.DeviceList { + return &requests.DeviceList{Filters: query.Filters{Raw: "!!!not-base64!!!"}} + }, + requiredMocks: func(*mocks.MockService) {}, + expectedFields: map[string]string{"filter": "cannot be decoded"}, + }, + { + description: "refuses a filter naming a field the device list does not know", + req: func(t *testing.T) *requests.DeviceList { + t.Helper() + + raw := encode(t, []query.Filter{{ + Type: query.FilterTypeProperty, + Params: &query.FilterProperty{Name: "nonexistent_field", Operator: "eq", Value: "foo"}, + }}) + + return &requests.DeviceList{Filters: query.Filters{Raw: raw}} + }, + requiredMocks: func(*mocks.MockService) {}, + expectedFields: map[string]string{"filter": "is not valid"}, + }, + { + description: "hands the service the decoded filter and the caller's connector intent", + req: func(t *testing.T) *requests.DeviceList { + t.Helper() + + raw := encode(t, []query.Filter{{ + Type: query.FilterTypeProperty, + Params: &query.FilterProperty{Name: "name", Operator: "contains", Value: "foo"}, + }}) + + return &requests.DeviceList{TenantID: tenantID, Connector: true, Filters: query.Filters{Raw: raw}} + }, + requiredMocks: func(service *mocks.MockService) { + service. + On("ListDevices", gomock.Anything, scope.MustBounded(tenantID), gomock.MatchedBy(func(req *requests.DeviceList) bool { + if !req.Connector || len(req.Filters.Data) != 1 { + return false + } + + property, ok := req.Filters.Data[0].Params.(*query.FilterProperty) + + return ok && property.Name == "name" && property.Value == "foo" + })). + Return([]models.Device{}, 0, nil). + Once() + }, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + service := mocks.NewMockService(t) + tc.requiredMocks(service) + + handler := NewHandler(service, nil) + + _, _, err := handler.GetDeviceList(t.Context(), scope.MustBounded(tenantID), gateway.Actor{ID: "user-id"}, tc.req(t)) + + if tc.expectedFields != nil { + require.Error(t, err) + + var wrapped errors.Error + require.ErrorAs(t, err, &wrapped, "a refusal must be a ShellHub error") + + data, ok := wrapped.Data.(errs.ErrDataInvalidEntity) + require.True(t, ok, "a refusal must carry the fields the caller can act on, got %v", wrapped.Data) + assert.Equal(t, tc.expectedFields, data.Fields) + + return + } + + require.NoError(t, err) + }) + } +} diff --git a/server/api/routes/device_test.go b/server/api/routes/device_test.go index b0c6da0a2d1..62605542a1d 100644 --- a/server/api/routes/device_test.go +++ b/server/api/routes/device_test.go @@ -36,9 +36,21 @@ func TestGetDevice(t *testing.T) { uid string tenant string admin bool + noIdentity bool requiredMocks func() expected Expected }{ + { + title: "refuses the request when the caller carries no identity", + uid: "1234", + tenant: "00000000-0000-4000-0000-000000000000", + noIdentity: true, + requiredMocks: func() {}, + expected: Expected{ + expectedSession: nil, + expectedStatus: http.StatusUnauthorized, + }, + }, { title: "fails when bind fails to validate uid", uid: "", @@ -105,6 +117,9 @@ func TestGetDevice(t *testing.T) { req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/devices/"+tc.uid, nil) req.Header.Set("Content-Type", "application/json") req.Header.Set("X-Role", authorizer.RoleOwner.String()) + if !tc.noIdentity { + req.Header.Set("X-ID", "000000000000000000000000") + } if tc.tenant != "" { req.Header.Set("X-Tenant-ID", tc.tenant) } @@ -400,6 +415,7 @@ func TestGetDeviceList(t *testing.T) { req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/devices?"+urlVal.Encode(), nil) req.Header.Set("X-Role", authorizer.RoleOwner.String()) + req.Header.Set("X-ID", "000000000000000000000000") req.Header.Set("X-Tenant-ID", tc.req.TenantID) rec := httptest.NewRecorder() @@ -485,6 +501,7 @@ func TestGetDeviceListBadFilter(t *testing.T) { req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/devices?"+urlVal.Encode(), nil) req.Header.Set("X-Role", authorizer.RoleOwner.String()) + req.Header.Set("X-ID", "000000000000000000000000") req.Header.Set("X-Tenant-ID", "00000000-0000-4000-0000-000000000000") rec := httptest.NewRecorder() @@ -497,88 +514,71 @@ func TestGetDeviceListBadFilter(t *testing.T) { } } -func TestGetDeviceListConnectorFilterOrder(t *testing.T) { - cases := []struct { - description string - connector string - userFilter []query.Filter - }{ - { - description: "connector filter has AND before property when user filter is present", - connector: "", - userFilter: []query.Filter{ - { - Type: query.FilterTypeProperty, - Params: &query.FilterProperty{Name: "name", Operator: "contains", Value: "foo"}, - }, - }, - }, - { - description: "connector=true filter has AND before property when user filter is present", - connector: "true", - userFilter: []query.Filter{ - { - Type: query.FilterTypeProperty, - Params: &query.FilterProperty{Name: "name", Operator: "contains", Value: "foo"}, - }, - }, - }, - } +// TestContainerAliasCarriesTheConnectorIntent pins the two behaviours the /api/containers rewrite +// depends on: the container list is the device list carrying the connector intent, and a single +// container is the plain device route carrying none. +// +// Which comparison that intent becomes is the service's decision, and is asserted there. +func TestContainerAliasCarriesTheConnectorIntent(t *testing.T) { + const tenantID = "00000000-0000-4000-0000-000000000000" - for _, tc := range cases { - t.Run(tc.description, func(t *testing.T) { - mock := mocks.NewMockService(t) - - var captured *requests.DeviceList - mock. - On("ListDevices", gomock.Anything, gomock.Anything, gomock.AnythingOfType("*requests.DeviceList")). - Run(func(args gomock.Arguments) { - list, ok := args.Get(2).(*requests.DeviceList) - require.True(t, ok) - captured = list - }). - Return([]models.Device{}, 0, nil). - Once() - - filterJSON, err := json.Marshal(tc.userFilter) - require.NoError(t, err) - - filterB64 := base64.StdEncoding.EncodeToString(filterJSON) - - urlVal := &url.Values{} - urlVal.Set("page", "1") - urlVal.Set("per_page", "10") - urlVal.Set("sort_by", "name") - urlVal.Set("order_by", "asc") - urlVal.Set("status", "accepted") - urlVal.Set("filter", filterB64) - if tc.connector != "" { - urlVal.Set("connector", tc.connector) - } + get := func(t *testing.T, mock *mocks.MockService, target string) int { + t.Helper() - req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api/devices?"+urlVal.Encode(), nil) - req.Header.Set("X-Role", authorizer.RoleOwner.String()) - req.Header.Set("X-Tenant-ID", "00000000-0000-4000-0000-000000000000") + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil) + req.Header.Set("X-Role", authorizer.RoleOwner.String()) + req.Header.Set("X-ID", "000000000000000000000000") + req.Header.Set("X-Tenant-ID", tenantID) - rec := httptest.NewRecorder() - e := NewRouter(mock) - e.ServeHTTP(rec, req) + rec := httptest.NewRecorder() + NewRouter(mock).ServeHTTP(rec, req) - require.Equal(t, http.StatusOK, rec.Result().StatusCode) - require.NotNil(t, captured) + return rec.Result().StatusCode + } - data := captured.Data - require.GreaterOrEqual(t, len(data), 3) + listExpecting := func(t *testing.T, connector bool) *mocks.MockService { + t.Helper() - lastTwo := data[len(data)-2:] - require.Equal(t, query.FilterTypeOperator, lastTwo[0].Type, "AND operator must precede the platform property filter") - require.Equal(t, query.FilterTypeProperty, lastTwo[1].Type, "platform property filter must follow the AND operator") + mock := mocks.NewMockService(t) + mock. + On("ListDevices", gomock.Anything, scope.MustBounded(tenantID), gomock.MatchedBy(func(req *requests.DeviceList) bool { + return req.Connector == connector + })). + Return([]models.Device{}, 0, nil). + Once() - op, ok := lastTwo[0].Params.(*query.FilterOperator) - require.True(t, ok) - require.Equal(t, "and", op.Name) - }) + return mock } + + t.Run("the container list asks for connector devices", func(t *testing.T) { + mock := listExpecting(t, true) + require.Equal(t, http.StatusOK, get(t, mock, "/api/containers")) + }) + + t.Run("the container list keeps the intent alongside a query string", func(t *testing.T) { + mock := listExpecting(t, true) + require.Equal(t, http.StatusOK, get(t, mock, "/api/containers?status=accepted")) + }) + + t.Run("the container list keeps its intent against a connector the caller sent", func(t *testing.T) { + mock := listExpecting(t, true) + require.Equal(t, http.StatusOK, get(t, mock, "/api/containers?connector=false")) + }) + + t.Run("the device list asks for none", func(t *testing.T) { + mock := listExpecting(t, false) + require.Equal(t, http.StatusOK, get(t, mock, "/api/devices")) + }) + + t.Run("a single container resolves to the plain device route", func(t *testing.T) { + mock := mocks.NewMockService(t) + mock. + On("GetDevice", gomock.Anything, scope.MustBounded(tenantID), models.UID("uid1")). + Return(&models.Device{UID: "uid1"}, nil). + Once() + + require.Equal(t, http.StatusOK, get(t, mock, "/api/containers/uid1")) + }) } func TestUpdateDevice(t *testing.T) { diff --git a/server/api/routes/healthcheck.go b/server/api/routes/healthcheck.go index c885b14fae5..a5e196949fd 100644 --- a/server/api/routes/healthcheck.go +++ b/server/api/routes/healthcheck.go @@ -1,8 +1,10 @@ package routes import ( - "net/http" + "context" + "github.com/shellhub-io/shellhub/pkg/api/requests" + "github.com/shellhub-io/shellhub/pkg/api/scope" "github.com/shellhub-io/shellhub/server/api/pkg/gateway" ) @@ -13,6 +15,6 @@ const ( // EvaluateHealth answers that the API is serving. It checks nothing behind the API, so it // reports reachability rather than readiness. -func (h *Handler) EvaluateHealth(c *gateway.Context) error { - return c.NoContent(http.StatusOK) +func (h *Handler) EvaluateHealth(_ context.Context, _ scope.Scope, _ gateway.Actor, _ *requests.Empty) error { + return nil } diff --git a/server/api/routes/healthcheck_test.go b/server/api/routes/healthcheck_test.go index 6749e2a1174..e1d5214c77b 100644 --- a/server/api/routes/healthcheck_test.go +++ b/server/api/routes/healthcheck_test.go @@ -5,41 +5,33 @@ import ( "net/http/httptest" "testing" - "github.com/labstack/echo/v5" + "github.com/shellhub-io/shellhub/pkg/api/requests" + "github.com/shellhub-io/shellhub/pkg/api/scope" "github.com/shellhub-io/shellhub/server/api/pkg/gateway" "github.com/shellhub-io/shellhub/server/api/services/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestEvaluateHealth(t *testing.T) { - e := echo.New() mock := mocks.NewMockService(t) h := NewHandler(mock, nil) - cases := []struct { - title string - requiredMocks func() - expectedErr error - }{ - { - title: "success when try to make a evaluate health", - expectedErr: nil, - }, - } - - for _, tc := range cases { - t.Run(tc.title, func(t *testing.T) { - req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, HealthCheckURL, nil) - rec := httptest.NewRecorder() - echoContext := e.NewContext(req, rec) - - apictx := gateway.NewContext(mock, echoContext) - err := h.EvaluateHealth(apictx) - - assert.Equal(t, tc.expectedErr, err) - assert.Equal(t, http.StatusOK, rec.Code) - }) - } + require.NoError(t, h.EvaluateHealth(t.Context(), scope.NewUnbounded("test"), gateway.Actor{}, &requests.Empty{})) mock.AssertExpectations(t) } + +// TestHealthCheckAnswersWithoutACredential drives the registration rather than the handler: the +// health check is the route that declares both an unbounded scope and an anonymous actor, so it is +// where those two claims are proven to reach the wire. +func TestHealthCheckAnswersWithoutACredential(t *testing.T) { + router, _, _ := authenticatedRouter(t) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/api"+HealthCheckURL, nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Empty(t, rec.Body.String()) +} diff --git a/server/api/routes/mcp.go b/server/api/routes/mcp.go index deaf3024f4f..e735e60bd80 100644 --- a/server/api/routes/mcp.go +++ b/server/api/routes/mcp.go @@ -15,6 +15,7 @@ import ( "github.com/mark3labs/mcp-go/mcp" mcpserver "github.com/mark3labs/mcp-go/server" "github.com/shellhub-io/shellhub/pkg/api/authorizer" + "github.com/shellhub-io/shellhub/server/api/pkg/gateway" ) type mcpContextKey string @@ -24,12 +25,6 @@ const ( mcpKeyHeaders mcpContextKey = "mcp_headers" ) -var mcpAuthHeaders = []string{ - "X-Tenant-ID", - "X-Role", - "X-Api-Key", -} - // SetupMCPRoutes mounts the MCP Streamable HTTP server at /mcp. func SetupMCPRoutes(router *echo.Echo) { s := buildMCPServer(router) @@ -45,7 +40,7 @@ func SetupMCPRoutes(router *echo.Echo) { ctx = context.WithValue(ctx, mcpKeyTenantID, tenantID) headers := http.Header{} - for _, key := range mcpAuthHeaders { + for _, key := range gateway.IdentityHeaders() { if value := r.Header.Get(key); value != "" { headers.Set(key, value) } diff --git a/server/api/routes/mcp_test.go b/server/api/routes/mcp_test.go index 45f5907b5b3..1b3188586d0 100644 --- a/server/api/routes/mcp_test.go +++ b/server/api/routes/mcp_test.go @@ -2,7 +2,6 @@ package routes import ( "bytes" - "context" "encoding/json" "net/http" "net/http/httptest" @@ -27,7 +26,13 @@ const mcpCallerTenant = "00000000-0000-4000-0000-000000000000" func mcpCall(t *testing.T, router http.Handler, tenant, role, body string) *httptest.ResponseRecorder { t.Helper() - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/mcp", bytes.NewBufferString(body)) + return mcpCallAs(t, router, tenant, role, http.Header{"X-API-Key": []string{"mcp-api-key"}}, body) +} + +func mcpCallAs(t *testing.T, router http.Handler, tenant, role string, credential http.Header, body string) *httptest.ResponseRecorder { + t.Helper() + + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/mcp", bytes.NewBufferString(body)) req.Header.Set("Content-Type", "application/json") req.Header.Set("Mcp-Session-Id", "mcp-session-00000000-0000-4000-8000-000000000000") if tenant != "" { @@ -36,6 +41,11 @@ func mcpCall(t *testing.T, router http.Handler, tenant, role, body string) *http if role != "" { req.Header.Set("X-Role", role) } + for key, values := range credential { + for _, value := range values { + req.Header.Add(key, value) + } + } rec := httptest.NewRecorder() router.ServeHTTP(rec, req) @@ -145,6 +155,47 @@ func TestMCPListDevices(t *testing.T) { mock.AssertExpectations(t) } +func TestMCPForwardsEveryCredentialsActor(t *testing.T) { + credentials := map[string]http.Header{ + "a user token names the acting person": {"X-ID": []string{"000000000000000000000000"}}, + "an API key names no person": {"X-API-Key": []string{"mcp-api-key"}}, + "an admin request carries only a username": { + "X-Username": []string{"admin"}, + "X-Admin": []string{"true"}, + }, + } + + for description, credential := range credentials { + t.Run(description, func(t *testing.T) { + mock := mocks.NewMockService(t) + mock. + On("ListDevices", gomock.Anything, gomock.Anything, gomock.AnythingOfType("*requests.DeviceList")). + Return([]models.Device{{UID: "uid1"}}, 7, nil). + Once() + + rec := mcpCallAs(t, NewRouter(mock), mcpCallerTenant, authorizer.RoleOwner.String(), credential, + mcpToolCall("shellhub_list_devices", `{}`)) + + text, isErr := mcpToolResult(t, rec) + require.False(t, isErr, text) + assert.Contains(t, text, `"total": 7`) + mock.AssertExpectations(t) + }) + } +} + +func TestMCPRefusesACallCarryingNoActor(t *testing.T) { + mock := mocks.NewMockService(t) + + rec := mcpCallAs(t, NewRouter(mock), mcpCallerTenant, authorizer.RoleOwner.String(), http.Header{}, + mcpToolCall("shellhub_list_devices", `{}`)) + + text, isErr := mcpToolResult(t, rec) + assert.True(t, isErr) + assert.Contains(t, text, "unauthorized") + mock.AssertNotCalled(t, "ListDevices") +} + // TestMCPGetDevice ensures the uid arg becomes the path parameter. func TestMCPGetDevice(t *testing.T) { mock := mocks.NewMockService(t) diff --git a/server/api/routes/route_table_test.go b/server/api/routes/route_table_test.go new file mode 100644 index 00000000000..6260dafbd4a --- /dev/null +++ b/server/api/routes/route_table_test.go @@ -0,0 +1,147 @@ +package routes + +import ( + "strings" + "testing" + + "github.com/shellhub-io/shellhub/server/api/pkg/gateway" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func unstatedClaims(declarations []gateway.Declaration) []string { + unstated := make([]string, 0) + + for _, declaration := range declarations { + if declaration.Unbounded && strings.TrimSpace(declaration.UnboundedReason) == "" { + unstated = append(unstated, declaration.Handler+" reads across namespaces and states no reason") + } + + if declaration.Anonymous && strings.TrimSpace(declaration.AnonymousReason) == "" { + unstated = append(unstated, declaration.Handler+" requires no actor and states no reason") + } + } + + return unstated +} + +// TestRouteTableStatesEveryClaim reads the claims the route table made while it was built. A route +// that reads across namespaces, or that needs no actor, has to say why — otherwise breadth and +// anonymity arrive by omission, which is what the two claims exist to prevent. +func TestRouteTableStatesEveryClaim(t *testing.T) { + authenticatedRouter(t) + + declarations := gateway.Declarations() + require.NotEmpty(t, declarations, "the route table registered no wrapped route") + + assert.Empty(t, unstatedClaims(declarations)) +} + +// TestUnstatedClaimsRefusesAnEmptyReason proves the check above bites, rather than passing because +// it looks at nothing. +func TestUnstatedClaimsRefusesAnEmptyReason(t *testing.T) { + unstated := unstatedClaims([]gateway.Declaration{ + {Handler: "silent", Unbounded: true, Anonymous: true}, + {Handler: "stated", Unbounded: true, UnboundedReason: "because"}, + }) + + require.Len(t, unstated, 2) + for _, complaint := range unstated { + assert.Contains(t, complaint, "silent") + } +} + +var wrapperExemptRoutes = []string{ + "GET /api/install", + "POST /api/login", + "POST /api/auth/user", + "POST /api/tags", + "POST /api/namespaces/:tenant/tags", +} + +// TestWrapperExemptRoutesAreRegistered catches a stale member: an exempt route that no longer +// exists, or was renamed, leaves the set claiming an exemption for nothing. +func TestWrapperExemptRoutesAreRegistered(t *testing.T) { + router, _, _ := authenticatedRouter(t) + + registered := make(map[string]struct{}) + for _, route := range router.Router().Routes() { + registered[route.Method+" "+route.Path] = struct{}{} + } + + for _, exempt := range wrapperExemptRoutes { + assert.Contains(t, registered, exempt, "the exempt set names %q but no such route is registered", exempt) + } +} + +var convertedRoutes = []struct { + handler string + shape gateway.Shape + route string +}{ + {handler: "EvaluateHealth", shape: gateway.ShapeNone, route: "GET /api" + HealthCheckURL}, + {handler: "GetDevice", shape: gateway.ShapeOne, route: "GET /api" + GetDeviceURL}, + {handler: "GetDeviceList", shape: gateway.ShapeList, route: "GET /api" + GetDeviceListURL}, +} + +func methodName(qualified string) string { + return strings.TrimSuffix(qualified[strings.LastIndex(qualified, ".")+1:], "-fm") +} + +// TestAnonymousClaimsMatchTheAllowlist joins the two places a route's anonymity is stated. The +// gateway claim frees the handler from needing an actor; the authenticator's allowlist is what lets +// the request past the credential check. Nothing but this holds them to the same answer, and a +// route carrying one without the other is either unreachable or reachable without a credential. +func TestAnonymousClaimsMatchTheAllowlist(t *testing.T) { + _, authn, _ := authenticatedRouter(t) + + allowed := make(map[string]struct{}) + for _, route := range authn.AnonymousRoutes() { + allowed[route] = struct{}{} + } + + claimed := make(map[string]bool) + for _, declaration := range gateway.Declarations() { + claimed[methodName(declaration.Handler)] = declaration.Anonymous + } + + for _, tc := range convertedRoutes { + t.Run(tc.handler, func(t *testing.T) { + _, inAllowlist := allowed[tc.route] + + assert.Equal(t, claimed[tc.handler], inAllowlist, + "%s declares Anonymous=%v but the authenticator's allowlist says %v", + tc.handler, claimed[tc.handler], inAllowlist) + }) + } +} + +// TestConvertedRoutesDeclareTheirShape pins the three routes this change converted: each answers +// with the shape it was registered under, and each is mounted at the address it claims. +// +// The second half is what keeps the declaration honest. Echo does not expose a route's handler, so +// a declaration cannot be matched to its route in general — but for a named handler at a known +// address, asserting both is enough to rule out a claim recorded by a wrapper nothing mounted. +func TestConvertedRoutesDeclareTheirShape(t *testing.T) { + router, _, _ := authenticatedRouter(t) + + registered := make(map[string]struct{}) + for _, route := range router.Router().Routes() { + registered[route.Method+" "+route.Path] = struct{}{} + } + + shapes := make(map[string]gateway.Shape) + for _, declaration := range gateway.Declarations() { + shapes[methodName(declaration.Handler)] = declaration.Shape + } + + for _, tc := range convertedRoutes { + t.Run(tc.handler, func(tt *testing.T) { + declared, found := shapes[tc.handler] + + require.True(tt, found, "%s is not registered through a gateway shape", tc.handler) + assert.Equal(tt, tc.shape, declared, "%s answers with the wrong shape", tc.handler) + assert.Contains(tt, registered, tc.route, "%s declares a shape but is not mounted", tc.handler) + }) + } +} diff --git a/server/api/routes/routes.go b/server/api/routes/routes.go index 9a45aeae724..7d63553659f 100644 --- a/server/api/routes/routes.go +++ b/server/api/routes/routes.go @@ -118,7 +118,10 @@ func NewRouter(service services.Service, opts ...Option) *echo.Echo { } publicAPI := router.Group("/api") - publicAPI.GET(HealthCheckURL, gateway.Handler(handler.EvaluateHealth)) + publicAPI.GET(HealthCheckURL, + gateway.None(handler.EvaluateHealth, + gateway.Unbounded("the health check reports on the instance, which belongs to no namespace"), + gateway.Anonymous("the health check is what a load balancer asks before any credential exists"))) publicAPI.GET(AuthLocalUserURLV2, gateway.Handler(handler.CreateUserToken)) // TODO: method POST publicAPI.GET(AuthUserTokenPublicURL, gateway.Handler(handler.CreateUserToken), routesmiddleware.BlockAPIKey) // TODO: method POST @@ -152,8 +155,8 @@ func NewRouter(service services.Service, opts ...Option) *echo.Echo { publicAPI.GET(URLNamespaceMembershipInvitationList, gateway.Handler(handler.GetNamespaceMembershipInvitationList), routesmiddleware.RequiresPermission(authorizer.NamespaceEditMember)) publicAPI.DELETE(URLCancelMembershipInvitation, gateway.Handler(handler.CancelMembershipInvitation), routesmiddleware.RequiresPermission(authorizer.NamespaceRemoveMember)) - publicAPI.GET(GetDeviceListURL, routesmiddleware.Authorize(gateway.Handler(handler.GetDeviceList))) - publicAPI.GET(GetDeviceURL, routesmiddleware.Authorize(gateway.Handler(handler.GetDevice))) + publicAPI.GET(GetDeviceListURL, routesmiddleware.Authorize(gateway.List(handler.GetDeviceList))) + publicAPI.GET(GetDeviceURL, routesmiddleware.Authorize(gateway.One(handler.GetDevice))) publicAPI.GET(ResolveDeviceURL, routesmiddleware.Authorize(gateway.Handler(handler.ResolveDevice))) publicAPI.PUT(UpdateDevice, gateway.Handler(handler.UpdateDevice), routesmiddleware.RequiresPermission(authorizer.DeviceUpdate)) publicAPI.PATCH(RenameDeviceURL, gateway.Handler(handler.RenameDevice), routesmiddleware.RequiresPermission(authorizer.DeviceRename)) @@ -253,7 +256,7 @@ func NewRouter(service services.Service, opts ...Option) *echo.Echo { router.Pre(echoMiddleware.Rewrite(map[string]string{ "/api/containers": "/api/devices?connector=true", - "/api/containers?*": "/api/devices?$1&connector=true", + "/api/containers?*": "/api/devices?connector=true&$1", "/api/containers/*": "/api/devices/$1", })) diff --git a/server/api/services/device.go b/server/api/services/device.go index 0a16b13d8ba..7a7e2bd1396 100644 --- a/server/api/services/device.go +++ b/server/api/services/device.go @@ -111,7 +111,44 @@ func (s *service) deviceLimit(ctx context.Context, tenantID string) (models.Name return s.store.NamespaceGetDeviceLimit(ctx, tenantID) } +const connectorPlatform = "connector" + +func connectorFilters(filters query.Filters, connector bool) (query.Filters, error) { + operator := "ne" + if connector { + operator = "eq" + } + + narrowed := query.Filters{ + Raw: filters.Raw, + Data: make([]query.Filter, 0, len(filters.Data)+2), + } + + narrowed.Data = append(narrowed.Data, filters.Data...) + narrowed.Data = append(narrowed.Data, + query.Filter{ + Type: query.FilterTypeOperator, + Params: &query.FilterOperator{Name: "and"}, + }, + query.Filter{ + Type: query.FilterTypeProperty, + Params: &query.FilterProperty{Name: "platform", Operator: operator, Value: connectorPlatform}, + }, + ) + + if len(narrowed.Data) > query.MaxFilterItems { + return query.Filters{}, NewErrDeviceFilterInvalid(query.ErrFilterPropertyInvalid) + } + + return narrowed, nil +} + func (s *service) ListDevices(ctx context.Context, sc scope.Scope, req *requests.DeviceList) ([]models.Device, int, error) { + filters, err := connectorFilters(req.Filters, req.Connector) + if err != nil { + return nil, 0, err + } + opts := []store.QueryOption{} if req.DeviceStatus != "" { @@ -124,7 +161,7 @@ func (s *service) ListDevices(ctx context.Context, sc scope.Scope, req *requests req.Sorter.Tiebreak = "id" - opts = append(opts, s.store.Options().Match(&req.Filters), s.store.Options().Sort(&req.Sorter), s.store.Options().Paginate(&req.Paginator)) + opts = append(opts, s.store.Options().Match(&filters), s.store.Options().Sort(&req.Sorter), s.store.Options().Paginate(&req.Paginator)) if req.DeviceStatus == models.DeviceStatusRemoved { return s.store.DeviceList(ctx, sc, store.DeviceAcceptableFromRemoved, opts...) diff --git a/server/api/services/device_connector_test.go b/server/api/services/device_connector_test.go new file mode 100644 index 00000000000..a6fbc64052a --- /dev/null +++ b/server/api/services/device_connector_test.go @@ -0,0 +1,130 @@ +package services + +import ( + "testing" + + "github.com/shellhub-io/shellhub/pkg/api/query" + "github.com/shellhub-io/shellhub/pkg/api/requests" + "github.com/shellhub-io/shellhub/pkg/api/scope" + storecache "github.com/shellhub-io/shellhub/pkg/cache" + "github.com/shellhub-io/shellhub/pkg/models" + "github.com/shellhub-io/shellhub/server/api/store" + storemock "github.com/shellhub-io/shellhub/server/api/store/mocks" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func connectorFilter(operator string) query.Filter { + return query.Filter{ + Type: query.FilterTypeProperty, + Params: &query.FilterProperty{Name: "platform", Operator: operator, Value: "connector"}, + } +} + +func andFilter() query.Filter { + return query.Filter{ + Type: query.FilterTypeOperator, + Params: &query.FilterOperator{Name: "and"}, + } +} + +func withoutConnectors(filters ...query.Filter) *query.Filters { + return &query.Filters{Data: append(filters, andFilter(), connectorFilter("ne"))} +} + +// TestListDevicesConnectorIntent asserts what the caller's intent means, not how it is spelled: the +// route test it replaces only checked that an operator preceded a property, and never which of +// them included or excluded connector devices. +func TestListDevicesConnectorIntent(t *testing.T) { + const tenantID = "00000000-0000-4000-0000-000000000000" + + userFilter := query.Filter{ + Type: query.FilterTypeProperty, + Params: &query.FilterProperty{Name: "name", Operator: "contains", Value: "foo"}, + } + + cases := []struct { + description string + connector bool + filters []query.Filter + expectedFilters []query.Filter + expectedErr error + }{ + { + description: "excludes connector devices when the caller asked for none", + connector: false, + filters: nil, + expectedFilters: []query.Filter{andFilter(), connectorFilter("ne")}, + }, + { + description: "narrows to connector devices when the caller asked for them", + connector: true, + filters: nil, + expectedFilters: []query.Filter{andFilter(), connectorFilter("eq")}, + }, + { + description: "applies the intent after the caller's own filters", + connector: true, + filters: []query.Filter{userFilter}, + expectedFilters: []query.Filter{userFilter, andFilter(), connectorFilter("eq")}, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(tt *testing.T) { + storeMock := storemock.NewMockStore(tt) + queryOptionsMock := storemock.NewMockQueryOptions(tt) + storeMock.On("Options").Return(queryOptionsMock).Maybe() + + queryOptionsMock.On("Match", &query.Filters{Data: tc.expectedFilters}).Return(nil).Once() + queryOptionsMock.On("Sort", mock.Anything).Return(nil).Once() + queryOptionsMock.On("Paginate", mock.Anything).Return(nil).Once() + storeMock.On("NamespaceGetDeviceLimit", mock.Anything, tenantID).Return(models.NamespaceDeviceLimit{}, nil).Once() + storeMock. + On("DeviceList", mock.Anything, scope.MustBounded(tenantID), store.DeviceAcceptableIfNotAccepted, mock.Anything). + Return([]models.Device{}, 0, nil). + Once() + + service := NewService(storeMock, privateKey, publicKey, storecache.NewNullCache()) + + req := &requests.DeviceList{ + TenantID: tenantID, + Connector: tc.connector, + Paginator: query.Paginator{Page: 1, PerPage: 10}, + Sorter: query.Sorter{By: "created_at", Order: query.OrderAsc}, + Filters: query.Filters{Data: tc.filters}, + } + + _, _, err := service.ListDevices(t.Context(), scope.MustBounded(tenantID), req) + require.NoError(tt, err) + }) + } +} + +// TestListDevicesConnectorIntentRespectsTheFilterLimit pins that the pair the service appends is +// counted like any other filter. Appending it around the limit would let a caller buy two extra +// filter entries by asking for containers. +func TestListDevicesConnectorIntentRespectsTheFilterLimit(t *testing.T) { + const tenantID = "00000000-0000-4000-0000-000000000000" + + filters := make([]query.Filter, 0, query.MaxFilterItems) + for range query.MaxFilterItems { + filters = append(filters, query.Filter{ + Type: query.FilterTypeProperty, + Params: &query.FilterProperty{Name: "name", Operator: "contains", Value: "foo"}, + }) + } + + storeMock := storemock.NewMockStore(t) + service := NewService(storeMock, privateKey, publicKey, storecache.NewNullCache()) + + req := &requests.DeviceList{ + TenantID: tenantID, + Paginator: query.Paginator{Page: 1, PerPage: 10}, + Sorter: query.Sorter{By: "created_at", Order: query.OrderAsc}, + Filters: query.Filters{Data: filters}, + } + + _, _, err := service.ListDevices(t.Context(), scope.MustBounded(tenantID), req) + require.ErrorIs(t, err, ErrDeviceFilterInvalid) +} diff --git a/server/api/services/device_test.go b/server/api/services/device_test.go index 47af1041cb1..15d58ad72c3 100644 --- a/server/api/services/device_test.go +++ b/server/api/services/device_test.go @@ -57,7 +57,7 @@ func TestListDevices(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -99,7 +99,7 @@ func TestListDevices(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -140,7 +140,7 @@ func TestListDevices(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -194,7 +194,7 @@ func TestListDevices_namespaceFromRequestContext(t *testing.T) { expectQueryOptions := func(queryOptionsMock *storemock.MockQueryOptions) { queryOptionsMock.On("WithDeviceStatus", models.DeviceStatusAccepted).Return(nil).Once() - queryOptionsMock.On("Match", &query.Filters{}).Return(nil).Once() + queryOptionsMock.On("Match", withoutConnectors()).Return(nil).Once() queryOptionsMock.On("Sort", &query.Sorter{By: "created_at", Order: query.OrderAsc, Tiebreak: "id"}).Return(nil).Once() queryOptionsMock.On("Paginate", &query.Paginator{Page: 1, PerPage: 10}).Return(nil).Once() } @@ -276,7 +276,7 @@ func TestListDevices_status_removed(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -314,7 +314,7 @@ func TestListDevices_status_removed(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -392,7 +392,7 @@ func TestListDevices_tenant_not_empty(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -431,7 +431,7 @@ func TestListDevices_tenant_not_empty(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -474,7 +474,7 @@ func TestListDevices_tenant_not_empty(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -517,7 +517,7 @@ func TestListDevices_tenant_not_empty(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. @@ -560,7 +560,7 @@ func TestListDevices_tenant_not_empty(t *testing.T) { Return(nil). Once() queryOptionsMock. - On("Match", &query.Filters{}). + On("Match", withoutConnectors()). Return(nil). Once() queryOptionsMock. diff --git a/server/api/services/errors.go b/server/api/services/errors.go index b9271673b22..d3ceb721162 100644 --- a/server/api/services/errors.go +++ b/server/api/services/errors.go @@ -109,6 +109,7 @@ var ( ErrNoTags = errors.New("no tags has found", ErrLayer, ErrCodeNotFound) ErrConflictName = errors.New("name duplicated", ErrLayer, ErrCodeDuplicated) ErrInvalidFormat = errors.New("invalid format", ErrLayer, ErrCodeInvalid) + ErrDeviceFilterInvalid = errors.New("device filter invalid", ErrLayer, ErrCodeInvalid) ErrDeviceNotFound = errors.New("device not found", ErrLayer, ErrCodeNotFound) ErrDeviceLoginCodeNotFound = errors.New("device login code not found", ErrLayer, ErrCodeNotFound) ErrDevicePairingCodeNotFound = errors.New("device pairing code not found", ErrLayer, ErrCodeNotFound) @@ -433,6 +434,12 @@ func NewErrPublicKeyFilter(next error) error { return NewErrInvalid(ErrPublicKeyFilter, nil, next) } +// NewErrDeviceFilterInvalid returns an error when the device list filter cannot be honoured, such +// as when it exceeds the filter limits. +func NewErrDeviceFilterInvalid(next error) error { + return NewErrInvalid(ErrDeviceFilterInvalid, nil, next) +} + // NewErrDeviceNotFound returns an error when the device is not found. func NewErrDeviceNotFound(id models.UID, next error) error { return NewErrNotFound(ErrDeviceNotFound, string(id), next)