From 774d884f5c96d61072d121c43df84bdafe7df83e Mon Sep 17 00:00:00 2001 From: kolaente Date: Mon, 1 Jun 2026 14:33:50 +0200 Subject: [PATCH] test(api/v2): assert admin project id via structured json --- pkg/routes/api/v2/admin_projects.go | 7 +----- pkg/routes/routes.go | 26 ++++--------------- pkg/webtests/huma_admin_test.go | 39 ++++++++++++++++------------- 3 files changed, 27 insertions(+), 45 deletions(-) diff --git a/pkg/routes/api/v2/admin_projects.go b/pkg/routes/api/v2/admin_projects.go index 2e5f3e93f..8e7b51d3d 100644 --- a/pkg/routes/api/v2/admin_projects.go +++ b/pkg/routes/api/v2/admin_projects.go @@ -27,16 +27,11 @@ import ( "github.com/danielgtaylor/huma/v2" ) -// AdminProjectList.ReadAll returns []*models.Project, so the wire shape is a -// plain paginated list of projects. type adminProjectListBody struct { Body Paginated[*models.Project] } -// RegisterAdminProjectRoutes wires the admin project list onto the Huma API. -// The instance-admin + admin-panel-license gate is applied by the Echo -// middleware on the /api/v2/admin path prefix (see gateV2AdminRoutes in -// pkg/routes/routes.go), not here — there is no per-handler permission check. +// Permissions are enforced by the gateV2AdminRoutes path middleware, not per-handler. func RegisterAdminProjectRoutes(api huma.API) { tags := []string{"admin"} diff --git a/pkg/routes/routes.go b/pkg/routes/routes.go index d8ff5c61d..9bfd530ba 100644 --- a/pkg/routes/routes.go +++ b/pkg/routes/routes.go @@ -376,27 +376,15 @@ func noStoreCacheControl() echo.MiddlewareFunc { } } -// v2AdminPathPrefix is the URL prefix every gated admin operation lives under. const v2AdminPathPrefix = "/api/v2/admin" -// gateV2AdminRoutes applies the existing v1 admin gate (license feature + -// instance admin, both 404-on-failure) to every /api/v2/admin request and -// passes everything else through untouched. -// -// v2 is a single Huma API mounted on the /api/v2 Echo group, so unlike v1 — -// which builds a dedicated `/admin` Echo sub-group and attaches the gate as -// group middleware — we can't simply construct a gated sub-group without -// splitting the Huma API (which would split the OpenAPI spec, dropping admin -// operations out of /api/v2/openapi.json). Instead we reuse the exact same -// RequireFeature/RequireInstanceAdmin functions as a path-scoped middleware on -// the shared group: the checks run before Huma's handler, in the same order as -// v1 (feature first, then admin), and return the identical 404. Keeping one -// Huma API means admin routes stay in the unified v2 spec and docs. +// gateV2AdminRoutes reuses v1's RequireFeature/RequireInstanceAdmin gate (both +// 404-on-failure) as path-scoped middleware: splitting v2 into a gated Echo +// sub-group would split the Huma API and drop admin ops from the OpenAPI spec. func gateV2AdminRoutes() echo.MiddlewareFunc { feature := RequireFeature(license.FeatureAdminPanel) admin := RequireInstanceAdmin() return func(next echo.HandlerFunc) echo.HandlerFunc { - // Compose feature → admin → next, evaluated once at setup. gated := feature(admin(next)) return func(c *echo.Context) error { if strings.HasPrefix(c.Request().URL.Path, v2AdminPathPrefix) { @@ -417,12 +405,8 @@ func registerAPIRoutesV2(e *echo.Echo, a *echo.Group) { // apply to v2 resource endpoints too. setupRateLimit(a, config.RateLimitKind.GetString()) setupMetricsMiddleware(a) - // The admin gate must run after the token middleware (it reads the - // authenticated user from the JWT claims) and after the rate limit and - // metrics middleware so requests rejected by the gate are still rate - // limited and measured — RequireInstanceAdmin does a DB read per request, - // so an unauthenticated flood to /api/v2/admin/* would otherwise hit the - // DB unbounded. It is scoped by path so only /api/v2/admin/* is gated. + // Must come after rate limiting: the gate does a per-request admin DB read, + // so an unauthenticated flood to /api/v2/admin/* would otherwise be unbounded. a.Use(gateV2AdminRoutes()) api := apiv2.NewAPI(e, a) diff --git a/pkg/webtests/huma_admin_test.go b/pkg/webtests/huma_admin_test.go index c241d3505..36f25de15 100644 --- a/pkg/webtests/huma_admin_test.go +++ b/pkg/webtests/huma_admin_test.go @@ -17,6 +17,7 @@ package webtests import ( + "encoding/json" "net/http" "testing" @@ -28,12 +29,8 @@ import ( "github.com/stretchr/testify/require" ) -// TestHumaAdminProjects exercises the v2 admin gate via GET /api/v2/admin/projects. -// It mirrors v1's TestAdmin_ListProjects but additionally asserts that the same -// two-stage gate (license feature + instance admin) v1 uses on /admin carries -// through to the Huma-backed /api/v2/admin group, returning 404 (not 403) on -// failure. The RFC 9457 error body is asserted once globally in -// TestHuma_ErrorShapeIsRFC9457, so here we only assert the status codes. +// The error body shape is covered by TestHuma_ErrorShapeIsRFC9457; this test +// only asserts gate status codes (404 on failure, matching v1). func TestHumaAdminProjects(t *testing.T) { t.Run("non-admin user gets 404", func(t *testing.T) { e, err := setupTestEnv() @@ -54,9 +51,7 @@ func TestHumaAdminProjects(t *testing.T) { t.Run("admin without the feature gets 404", func(t *testing.T) { e, err := setupTestEnv() require.NoError(t, err) - // A valid license that lacks the admin panel feature still gates the - // route. Match the sibling subtests' set/defer-reset pattern so the - // license state never bleeds into other tests. + // Empty feature set = licensed instance without the admin feature. license.SetForTests([]license.Feature{}) defer license.ResetForTests() @@ -76,15 +71,23 @@ func TestHumaAdminProjects(t *testing.T) { res := adminReq(t, e, http.MethodGet, "/api/v2/admin/projects", admin, "") require.Equal(t, http.StatusOK, res.Code, res.Body.String()) - body := res.Body.String() - // v2 wraps lists in the Paginated envelope. - assert.Contains(t, body, `"items":`) - assert.Contains(t, body, `"total":`) - // Project 6 is owned by user6, not shared with user1 — the admin list - // surfaces it regardless of ownership. - assert.Contains(t, body, `"id":6`) - // Project 22 is archived; the admin list includes archived projects. - assert.Contains(t, body, `"id":22`) + + var envelope struct { + Items []struct { + ID int64 `json:"id"` + } `json:"items"` + Total int64 `json:"total"` + } + require.NoError(t, json.Unmarshal(res.Body.Bytes(), &envelope)) + + ids := make(map[int64]bool, len(envelope.Items)) + for _, item := range envelope.Items { + ids[item.ID] = true + } + // Project 6 (owned by user6, not shared with user1) proves the list ignores ownership. + assert.True(t, ids[6], "expected project 6 in the admin list, got items %v", ids) + // Project 22 is archived, proving the list includes archived projects. + assert.True(t, ids[22], "expected archived project 22 in the admin list, got items %v", ids) }) t.Run("unauthenticated caller gets 401", func(t *testing.T) {