Enforce API-only endpoint restrictions on chart routes (#49477)
# Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Summary Enforced API-only endpoint restrictions on chart endpoints, matching the pattern already used by the activity bounded context. Also added `RouteTemplateRequestFunc` to chart route server options so the middleware can read the matched mux route template from context. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ### Reproduction Created an API-only user with a restrictive endpoint allow-list (only `GET /api/v1/fleet/hosts`). Confirmed that: - Allowed endpoint (`/api/latest/fleet/hosts`) returns 200 - Non-allowed cataloged endpoint (`/api/latest/fleet/users`) returns 403 - Chart endpoint (`/api/latest/fleet/charts/uptime`) returned 200 before the fix (the bug) - After the fix, chart endpoint correctly returns 403 ### Unit test Added a test case in `server/service/middleware/auth/api_only_test.go` that verifies an API-only user with endpoint restrictions is denied access to chart endpoints not in their allow-list. The chart endpoint is included in the test catalog (matching production), so the test exercises the allow-list rejection path. All 17 tests in the auth middleware package pass. ### Local verification 1. Confirmed the chart middleware in `cmd/fleet/serve.go` previously called `auth.AuthenticatedUser(svc, next)` without `APIOnlyEndpointCheck` wrapping 2. Verified the activity bounded context (same file) already uses `auth.APIOnlyEndpointCheck(next)` as the correct pattern 3. Applied the same wrapping to the chart middleware 4. Added `RouteTemplateRequestFunc` to `server/chart/internal/service/endpoint_utils.go` so the route template is available in context (required by `APIOnlyEndpointCheck`) 5. Ran `go test ./server/service/middleware/auth/ -v` with all 17 tests passing 6. Ran `make lint-go-incremental` with 0 issues
This commit is contained in:
@@ -0,0 +1 @@
|
||||
Enforced API-only endpoint restrictions on chart endpoints.
|
||||
+3
-1
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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"))
|
||||
|
||||
Reference in New Issue
Block a user