refactor(server): make route registration and declaration the same act

server/api/pkg/gateway/guard.go

@@ -0,0 +1,38 @@
+package gateway
+
+import (
+	"net/http"
+
+	"github.com/labstack/echo/v5"
+	"github.com/shellhub-io/shellhub/pkg/api/authorizer"
+)
+
+// RequiresPermission refuses the request with 403 unless the role the request authenticated with
+// holds permission. It answers 403 rather than 401 because the caller is known and simply not
+// allowed; a request carrying no credential at all never reaches it.
+//
+// It lives here rather than beside the other route middleware because [Requires] installs it, and
+// the route middleware package imports this one.
+func RequiresPermission(permission authorizer.Permission) echo.MiddlewareFunc {
+	return func(next echo.HandlerFunc) echo.HandlerFunc {
+		return func(c *echo.Context) error {
+			if ctx, ok := From(c); !ok || !ctx.Role().HasPermission(permission) {
+				return c.NoContent(http.StatusForbidden)
+			}
+
+			return next(c)
+		}
+	}
+}
+
+// BlockAPIKey refuses with 403 a request that authenticated with an API key. It reads the header
+// rather than the resolved identity because a key is refused whether or not it was honoured.
+func BlockAPIKey(next echo.HandlerFunc) echo.HandlerFunc {
+	return func(c *echo.Context) error {
+		if key := c.Request().Header.Get("X-API-Key"); key != "" {
+			return c.NoContent(http.StatusForbidden)
+		}
+
+		return next(c)
+	}
+}

server/api/pkg/gateway/mount.go

@@ -0,0 +1,118 @@
+package gateway
+
+import (
+	"net/http"
+	"sort"
+	"sync"
+
+	"github.com/labstack/echo/v5"
+)
+
+// Target is the part of echo's routing API mounting needs. Both *echo.Echo and *echo.Group
+// satisfy it, and both answer with the address the route ended up at — the group prefix already
+// applied — which is the address the router reports and the authenticator matches on.
+type Target interface {
+	Add(method, path string, handler echo.HandlerFunc, middleware ...echo.MiddlewareFunc) echo.RouteInfo
+}
+
+// Mounter mounts routes onto one target and declares them against one router. Mounting and
+// declaring are the same act: a declaration cannot be recorded for a route nobody mounted, and a
+// route mounted through a mounter cannot escape the declaration.
+//
+// The router is carried separately from the target because a group does not name the router it
+// was carved from, and it is the router a claim belongs to.
+type Mounter struct {
+	router *echo.Echo
+	target Target
+}
+
+// MountOn returns a mounter adding routes to target and declaring them against router. Pass
+// router as target to mount on its root; pass a group of it to mount under that group's prefix.
+func MountOn(router *echo.Echo, target Target) *Mounter {
+	return &Mounter{router: router, target: target}
+}
+
+// GET mounts route at path answering GET, declares it against the mounter's router, and returns
+// the address it was mounted at — which is the address the declaration then carries. Each option's
+// guard wraps the handler in the order the option is written, so the guard written first is the
+// one that answers first.
+//
+// [POST], [PUT], [PATCH] and [DELETE] do the same for their methods.
+func GET(m *Mounter, path string, route Route, options ...RouteOption) echo.RouteInfo {
+	return mount(m, http.MethodGet, path, route, options)
+}
+
+// POST mounts route at path, answering POST. See [GET] for what mounting declares.
+func POST(m *Mounter, path string, route Route, options ...RouteOption) echo.RouteInfo {
+	return mount(m, http.MethodPost, path, route, options)
+}
+
+// PUT mounts route at path, answering PUT. See [GET] for what mounting declares.
+func PUT(m *Mounter, path string, route Route, options ...RouteOption) echo.RouteInfo {
+	return mount(m, http.MethodPut, path, route, options)
+}
+
+// PATCH mounts route at path, answering PATCH. See [GET] for what mounting declares.
+func PATCH(m *Mounter, path string, route Route, options ...RouteOption) echo.RouteInfo {
+	return mount(m, http.MethodPatch, path, route, options)
+}
+
+// DELETE mounts route at path, answering DELETE. See [GET] for what mounting declares.
+func DELETE(m *Mounter, path string, route Route, options ...RouteOption) echo.RouteInfo {
+	return mount(m, http.MethodDelete, path, route, options)
+}
+
+func mount(m *Mounter, method, path string, route Route, options []RouteOption) echo.RouteInfo {
+	declaration := route.declaration
+
+	guards := make([]echo.MiddlewareFunc, 0, len(options))
+
+	for _, option := range options {
+		if guard := option(&declaration); guard != nil {
+			guards = append(guards, guard)
+		}
+	}
+
+	info := m.target.Add(method, path, route.build(declaration), guards...)
+
+	declaration.Method, declaration.Path = info.Method, info.Path
+
+	declare(m.router, declaration)
+
+	return info
+}
+
+var tables = struct {
+	sync.Mutex
+	byRouter map[*echo.Echo][]Declaration
+}{byRouter: make(map[*echo.Echo][]Declaration)}
+
+func declare(router *echo.Echo, declaration Declaration) {
+	tables.Lock()
+	defer tables.Unlock()
+
+	tables.byRouter[router] = append(tables.byRouter[router], declaration)
+}
+
+// Declarations returns what every route mounted on router claims, ordered by address. The table
+// belongs to the router rather than to the process, so an invariant over it holds regardless of
+// which other routers — under which other editions — a neighbouring test built.
+//
+// Repeats are kept: two declarations sharing an address is how a shadowed route shows up, and
+// collapsing them here would hide it.
+func Declarations(router *echo.Echo) []Declaration {
+	tables.Lock()
+	defer tables.Unlock()
+
+	all := append([]Declaration(nil), tables.byRouter[router]...)
+
+	sort.Slice(all, func(i, j int) bool {
+		if all[i].Path != all[j].Path {
+			return all[i].Path < all[j].Path
+		}
+
+		return all[i].Method < all[j].Method
+	})
+
+	return all
+}

server/api/pkg/gateway/mount_test.go

@@ -0,0 +1,195 @@
+package gateway_test
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/labstack/echo/v5"
+	"github.com/shellhub-io/shellhub/pkg/api/authorizer"
+	"github.com/shellhub-io/shellhub/pkg/api/scope"
+	"github.com/shellhub-io/shellhub/server/api/pkg/gateway"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func rootOf(router *echo.Echo) *gateway.Mounter {
+	return gateway.MountOn(router, router)
+}
+
+func okRoute() gateway.Route {
+	return gateway.None(func(_ context.Context, _ scope.Scope, _ gateway.Actor, _ *probeRequest) error {
+		return nil
+	})
+}
+
+// TestMountingCompletesTheDeclarationWithItsAddress is the join this package exists to close:
+// echo hides a route's handler, so a claim can only be matched to a route if the claim was made
+// where the route was mounted. The group prefix is part of that address, and it is the prefixed
+// form the router reports and the authenticator matches on.
+func TestMountingCompletesTheDeclarationWithItsAddress(t *testing.T) {
+	e := probeRouter(t, true)
+
+	gateway.GET(gateway.MountOn(e, e.Group("/api")), "/devices", okRoute())
+	gateway.POST(rootOf(e), "/root", okRoute())
+
+	addresses := make([]string, 0)
+	for _, declaration := range gateway.Declarations(e) {
+		addresses = append(addresses, declaration.Address())
+	}
+
+	assert.Equal(t, []string{"GET /api/devices", "POST /root"}, addresses)
+
+	registered := make([]string, 0)
+	for _, route := range e.Router().Routes() {
+		registered = append(registered, route.Method+" "+route.Path)
+	}
+
+	assert.ElementsMatch(t, addresses, registered)
+}
+
+// TestDeclarationsBelongToTheRouterThatMountedThem keeps an invariant over one route table from
+// depending on which other routers a neighbouring test built — an edition-gated route registered
+// by one of them would otherwise read as a stale claim on this one.
+func TestDeclarationsBelongToTheRouterThatMountedThem(t *testing.T) {
+	first, second := probeRouter(t, true), probeRouter(t, true)
+
+	gateway.GET(rootOf(first), "/first", okRoute())
+	gateway.GET(rootOf(second), "/second", okRoute())
+
+	require.Len(t, gateway.Declarations(first), 1)
+	require.Len(t, gateway.Declarations(second), 1)
+
+	assert.Equal(t, "GET /first", gateway.Declarations(first)[0].Address())
+	assert.Equal(t, "GET /second", gateway.Declarations(second)[0].Address())
+}
+
+// TestRequiresDeclaresThePermitItEnforces pins what makes the declaration evidence rather than
+// documentation: the option that records the permission is the option that installs its guard, so
+// the two cannot drift.
+func TestRequiresDeclaresThePermitItEnforces(t *testing.T) {
+	cases := []struct {
+		description    string
+		role           string
+		expectedStatus int
+	}{
+		{
+			description:    "refuses a role without the permission",
+			role:           "observer",
+			expectedStatus: http.StatusForbidden,
+		},
+		{
+			description:    "admits a role holding it",
+			role:           "owner",
+			expectedStatus: http.StatusOK,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.description, func(t *testing.T) {
+			e := probeRouter(t, true)
+			gateway.GET(rootOf(e), "/probe", okRoute(), gateway.Requires(authorizer.DeviceRemove))
+
+			declarations := gateway.Declarations(e)
+			require.Len(t, declarations, 1)
+			assert.True(t, declarations[0].RequiresPermission)
+			assert.Equal(t, authorizer.DeviceRemove, declarations[0].Permission)
+
+			assert.Equal(t, tc.expectedStatus, probe(t, e, map[string]string{
+				"X-Tenant-ID": probeTenant,
+				"X-ID":        "user-id",
+				"X-Role":      tc.role,
+			}))
+		})
+	}
+}
+
+// TestNoAPIKeyDeclaresTheBlockItEnforces covers the other declarative guard: a route closed to API
+// keys says so, and refuses one.
+func TestNoAPIKeyDeclaresTheBlockItEnforces(t *testing.T) {
+	e := probeRouter(t, true)
+	gateway.GET(rootOf(e), "/probe", okRoute(), gateway.NoAPIKey())
+
+	declarations := gateway.Declarations(e)
+	require.Len(t, declarations, 1)
+	assert.True(t, declarations[0].BlocksAPIKey)
+
+	assert.Equal(t, http.StatusForbidden, probe(t, e, map[string]string{
+		"X-Tenant-ID": probeTenant,
+		"X-API-Key":   "a-key",
+	}))
+
+	assert.Equal(t, http.StatusOK, probe(t, e, map[string]string{
+		"X-Tenant-ID": probeTenant,
+		"X-ID":        "user-id",
+	}))
+}
+
+// TestGuardsRunInTheOrderTheyAreWritten is the risk in moving two guards out of the middleware
+// tail: the tail ran in registration order, and the options have to keep doing so — the guard that
+// answers first is what a caller sees.
+func TestGuardsRunInTheOrderTheyAreWritten(t *testing.T) {
+	order := make([]string, 0)
+
+	mark := func(name string) echo.MiddlewareFunc {
+		return func(next echo.HandlerFunc) echo.HandlerFunc {
+			return func(c *echo.Context) error {
+				order = append(order, name)
+
+				return next(c)
+			}
+		}
+	}
+
+	e := probeRouter(t, true)
+	gateway.GET(rootOf(e), "/probe", okRoute(),
+		gateway.Guard(mark("first")),
+		gateway.NoAPIKey(),
+		gateway.Guard(mark("second")),
+		gateway.Requires(authorizer.DeviceRemove),
+		gateway.Guard(mark("third")))
+
+	require.Equal(t, http.StatusOK, probe(t, e, map[string]string{
+		"X-Tenant-ID": probeTenant,
+		"X-ID":        "user-id",
+		"X-Role":      "owner",
+	}))
+
+	assert.Equal(t, []string{"first", "second", "third"}, order)
+}
+
+// TestGuardDeclaresNothing states the boundary the change stops at: a guard that is not a claim —
+// the tenant check, the legacy authorize middleware — runs, and the declaration does not pretend
+// to describe it.
+func TestGuardDeclaresNothing(t *testing.T) {
+	refuse := func(_ echo.HandlerFunc) echo.HandlerFunc {
+		return func(c *echo.Context) error {
+			return c.NoContent(http.StatusTeapot)
+		}
+	}
+
+	e := probeRouter(t, true)
+	gateway.GET(rootOf(e), "/probe", okRoute(), gateway.Guard(refuse))
+
+	declarations := gateway.Declarations(e)
+	require.Len(t, declarations, 1)
+	assert.False(t, declarations[0].RequiresPermission)
+	assert.False(t, declarations[0].BlocksAPIKey)
+
+	assert.Equal(t, http.StatusTeapot, probe(t, e, map[string]string{"X-Tenant-ID": probeTenant, "X-ID": "user-id"}))
+}
+
+func probe(t *testing.T, e *echo.Echo, headers map[string]string) int {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/probe", nil)
+	for name, value := range headers {
+		req.Header.Set(name, value)
+	}
+
+	rec := httptest.NewRecorder()
+	e.ServeHTTP(rec, req)
+
+	return rec.Code
+}