diff --git a/changes/fix-chart-api-only-enforcement b/changes/fix-chart-api-only-enforcement new file mode 100644 index 0000000000..0b1422b9e0 --- /dev/null +++ b/changes/fix-chart-api-only-enforcement @@ -0,0 +1 @@ +Enforced API-only endpoint restrictions on chart endpoints. diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index fff3ec9da5..e465cc3bcf 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -1116,8 +1116,10 @@ func createChartBoundedContext(dbConns *common_mysql.DBConnections, svc fleet.Se chartSvc.RegisterDataset(&chart.UptimeDataset{}) chartSvc.RegisterDataset(&chart.CVEDataset{}) // Create auth middleware for chart bounded context + // Makes sure that api_only users are subject to endpoint + // restrictions on chart routes. chartAuthMiddleware := func(next endpoint.Endpoint) endpoint.Endpoint { - return auth.AuthenticatedUser(svc, next) + return auth.AuthenticatedUser(svc, auth.APIOnlyEndpointCheck(next)) } chartRoutes := chartRoutesFn(chartAuthMiddleware) return chartSvc, chartRoutes diff --git a/server/chart/internal/service/endpoint_utils.go b/server/chart/internal/service/endpoint_utils.go index cff0598c5b..58eda7b985 100644 --- a/server/chart/internal/service/endpoint_utils.go +++ b/server/chart/internal/service/endpoint_utils.go @@ -54,6 +54,12 @@ var _ eu.Endpointer[handlerFunc] = &chartEndpointer{} func newChartEndpointer(svc api.Service, authMiddleware endpoint.Middleware, opts []kithttp.ServerOption, r *mux.Router, versions ...string, ) *eu.CommonEndpointer[handlerFunc] { + // Append RouteTemplateRequestFunc so the api_only endpoint middleware + // can read the matched mux route template from context. + // + // Full-slice expression prevents aliasing into the caller's backing array + // if it happens to have spare capacity. + opts = append(opts[:len(opts):len(opts)], kithttp.ServerBefore(eu.RouteTemplateRequestFunc)) return &eu.CommonEndpointer[handlerFunc]{ EP: &chartEndpointer{ svc: svc, diff --git a/server/service/middleware/auth/api_only_test.go b/server/service/middleware/auth/api_only_test.go index d4014a7915..2b0d7d9e1b 100644 --- a/server/service/middleware/auth/api_only_test.go +++ b/server/service/middleware/auth/api_only_test.go @@ -24,6 +24,7 @@ var testCatalogEndpoints = []fleet.APIEndpoint{ fleet.NewAPIEndpointFromTpl("GET", "/api/v1/fleet/hosts"), fleet.NewAPIEndpointFromTpl("GET", "/api/v1/fleet/hosts/:id"), fleet.NewAPIEndpointFromTpl("POST", "/api/v1/fleet/scripts/run"), + fleet.NewAPIEndpointFromTpl("GET", "/api/v1/fleet/charts/:metric"), } // testIsInCatalog builds a fingerprint set from testCatalogEndpoints and @@ -295,6 +296,26 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { require.ErrorAs(t, err, &permErr) }) + t.Run("api-only user with restrictions, chart endpoint not in allow-list is rejected", func(t *testing.T) { + // Chart endpoint is in the catalog (testCatalogEndpoints), so the + // catalog check passes. The user's allow-list does not include charts, + // so the allow-list check rejects the request. + next, called := newNext() + ctx := ctxWithMethod("GET", muxTemplate("fleet/charts/{metric}")) + ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{ + APIOnly: true, + APIEndpoints: []fleet.APIEndpointRef{ + {Method: "GET", Path: "/api/v1/fleet/hosts"}, + }, + }}) + + _, err := newEndpoint(next)(ctx, nil) + require.Error(t, err) + require.False(t, *called) + var permErr *fleet.PermissionError + require.ErrorAs(t, err, &permErr) + }) + t.Run("api-only user with multiple allowed endpoints, accessing one of them", func(t *testing.T) { next, called := newNext() ctx := ctxWithMethod("POST", muxTemplate("fleet/scripts/run"))