From 5acc27c814d371c5b2b953657fde7d98daf82b6e Mon Sep 17 00:00:00 2001 From: Luke Heath Date: Fri, 7 Aug 2026 16:21:12 -0500 Subject: [PATCH 1/3] Distinguish API-only endpoint-restriction 403s from role-based denials Requests denied by an API-only user's endpoint allowlist now return a distinct 403 message and log the route template and denial reason at info level, so these denials are identifiable in server logs and error responses instead of looking like role-based permission denials. Fixes #50813 --- ...inguish-api-only-endpoint-restriction-403s | 1 + server/service/middleware/auth/api_only.go | 44 ++++++++-- .../service/middleware/auth/api_only_test.go | 80 +++++++++++++++---- 3 files changed, 105 insertions(+), 20 deletions(-) create mode 100644 changes/50813-distinguish-api-only-endpoint-restriction-403s diff --git a/changes/50813-distinguish-api-only-endpoint-restriction-403s b/changes/50813-distinguish-api-only-endpoint-restriction-403s new file mode 100644 index 00000000000..05eaa31f845 --- /dev/null +++ b/changes/50813-distinguish-api-only-endpoint-restriction-403s @@ -0,0 +1 @@ +- Requests denied by an API-only user's endpoint restrictions now return a distinct 403 message ("endpoint not permitted for this API-only user") and are logged at info level with the route and denial reason, so they can be distinguished from role-based permission denials. diff --git a/server/service/middleware/auth/api_only.go b/server/service/middleware/auth/api_only.go index 9519e8f4000..d93b356c34d 100644 --- a/server/service/middleware/auth/api_only.go +++ b/server/service/middleware/auth/api_only.go @@ -2,9 +2,12 @@ package auth import ( "context" + "log/slog" + "net/http" apiendpoints "github.com/fleetdm/fleet/v4/server/api_endpoints" "github.com/fleetdm/fleet/v4/server/contexts/authz" + "github.com/fleetdm/fleet/v4/server/contexts/logging" "github.com/fleetdm/fleet/v4/server/contexts/viewer" "github.com/fleetdm/fleet/v4/server/fleet" eu "github.com/fleetdm/fleet/v4/server/platform/endpointer" @@ -34,9 +37,12 @@ var RouteTemplateRequestFunc = eu.RouteTemplateRequestFunc // For API-only users with a non-empty restriction list (rows in // user_api_endpoints), two checks are applied in order: // 1. The requested route must appear in the API endpoint catalog. If not, a -// permission error (403) is returned. +// 403 with EndpointRestrictionDeniedMessage is returned. // 2. The route must match one of the user's allowed endpoints. If not, a -// permission error (403) is returned. +// 403 with EndpointRestrictionDeniedMessage is returned. +// +// Both denials are logged at info level with the route template and denial +// reason so they can be distinguished from role-based permission denials. func APIOnlyEndpointCheck(next endpoint.Endpoint) endpoint.Endpoint { return apiOnlyEndpointCheck(apiendpoints.IsInCatalog, next) } @@ -54,7 +60,7 @@ func apiOnlyEndpointCheck(isInCatalog func(string) bool, next endpoint.Endpoint) fp := fleet.NewAPIEndpointFromTpl(requestMethod, routeTemplate).Fingerprint() if !isInCatalog(fp) { - return nil, permissionDenied(ctx) + return nil, endpointRestrictionDenied(ctx, routeTemplate, "endpoint not in API endpoint catalog") } // Check whether the requested endpoint matches any of the user's allowed endpoints. @@ -64,13 +70,39 @@ func apiOnlyEndpointCheck(isInCatalog func(string) bool, next endpoint.Endpoint) } } - return nil, permissionDenied(ctx) + return nil, endpointRestrictionDenied(ctx, routeTemplate, "endpoint not in user's allowed API endpoints") } } -func permissionDenied(ctx context.Context) error { +// EndpointRestrictionDeniedMessage is returned in the 403 response body when a +// request is denied by an API-only user's endpoint restrictions, so callers +// can tell these denials apart from role-based permission denials. The +// restriction list is not secret from the caller (they hold a valid token), so +// naming the gate here discloses nothing. +const EndpointRestrictionDeniedMessage = "endpoint not permitted for this API-only user" + +// endpointRestrictionDenied rejects the request with a 403 that identifies the +// endpoint restriction (rather than the user's role) as the gate. The denial +// is surfaced on the request log line at info level — role-based 403s log at +// debug, which is how endpoint-restriction denials went unnoticed during +// debugging. The user email and request method are already logged on that +// line; the route template and denial reason are added as extras. +func endpointRestrictionDenied(ctx context.Context, routeTemplate, reason string) error { if ac, ok := authz.FromContext(ctx); ok { ac.SetChecked() } - return fleet.NewPermissionError("forbidden") + logging.WithLevel(ctx, slog.LevelInfo) + logging.WithExtras(ctx, + "denied_by", "api_only_endpoint_restriction", + "denial_reason", reason, + "route", routeTemplate, + ) + // PermissionError alone won't do: the error encoder discards its message + // and renders a generic "Permission Denied" body. Wrapping it in a + // UserMessageError routes it through the encoder branch that includes the + // message in the response. + return fleet.NewUserMessageError( + fleet.NewPermissionError(EndpointRestrictionDeniedMessage), + http.StatusForbidden, + ) } diff --git a/server/service/middleware/auth/api_only_test.go b/server/service/middleware/auth/api_only_test.go index 2b0d7d9e1b7..f8171e40f7e 100644 --- a/server/service/middleware/auth/api_only_test.go +++ b/server/service/middleware/auth/api_only_test.go @@ -2,11 +2,13 @@ package auth import ( "context" + "log/slog" "net/http" "net/http/httptest" "testing" authzctx "github.com/fleetdm/fleet/v4/server/contexts/authz" + "github.com/fleetdm/fleet/v4/server/contexts/logging" "github.com/fleetdm/fleet/v4/server/contexts/viewer" "github.com/fleetdm/fleet/v4/server/fleet" eu "github.com/fleetdm/fleet/v4/server/platform/endpointer" @@ -46,6 +48,19 @@ func muxTemplate(pathSuffix string) string { return muxVersionSegment + pathSuffix } +// requireEndpointRestrictionDenied asserts that err is the 403 returned when an +// API-only user's endpoint restrictions deny a request: a UserMessageError +// carrying EndpointRestrictionDeniedMessage, distinguishable from a role-based +// permission denial. +func requireEndpointRestrictionDenied(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var umErr *fleet.UserMessageError + require.ErrorAs(t, err, &umErr) + require.Equal(t, http.StatusForbidden, umErr.StatusCode()) + require.Equal(t, EndpointRestrictionDeniedMessage, umErr.UserMessage()) +} + func TestAPIOnlyEndpointCheck(t *testing.T) { newNext := func() (func(context.Context, any) (any, error), *bool) { called := false @@ -139,8 +154,7 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { _, err := newEndpoint(next)(ctx, nil) require.Error(t, err) require.False(t, *called) - var permErr *fleet.PermissionError - require.ErrorAs(t, err, &permErr) + requireEndpointRestrictionDenied(t, err) }) t.Run("api-only user with restrictions, missing route template in context is rejected", func(t *testing.T) { @@ -156,8 +170,7 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { _, err := newEndpoint(next)(ctx, nil) require.Error(t, err) require.False(t, *called) - var permErr *fleet.PermissionError - require.ErrorAs(t, err, &permErr) + requireEndpointRestrictionDenied(t, err) }) t.Run("api-only user with restrictions, missing method and template are both rejected", func(t *testing.T) { @@ -172,8 +185,7 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { _, err := newEndpoint(next)(ctx, nil) require.Error(t, err) require.False(t, *called) - var permErr *fleet.PermissionError - require.ErrorAs(t, err, &permErr) + requireEndpointRestrictionDenied(t, err) }) t.Run("api-only user, method normalization is case-insensitive", func(t *testing.T) { @@ -255,8 +267,7 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { require.Error(t, err) require.False(t, *called) - var permErr *fleet.PermissionError - require.ErrorAs(t, err, &permErr) + requireEndpointRestrictionDenied(t, err) }) t.Run("api-only user, allow-list entry for non-catalog endpoint is still denied", func(t *testing.T) { @@ -274,8 +285,7 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { _, err := newEndpoint(next)(ctx, nil) require.Error(t, err) require.False(t, *called) - var permErr *fleet.PermissionError - require.ErrorAs(t, err, &permErr) + requireEndpointRestrictionDenied(t, err) }) t.Run("api-only user, wrong method for catalog endpoint is rejected at catalog step", func(t *testing.T) { @@ -292,8 +302,7 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { _, err := newEndpoint(next)(ctx, nil) require.Error(t, err) require.False(t, *called) - var permErr *fleet.PermissionError - require.ErrorAs(t, err, &permErr) + requireEndpointRestrictionDenied(t, err) }) t.Run("api-only user with restrictions, chart endpoint not in allow-list is rejected", func(t *testing.T) { @@ -312,8 +321,51 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { _, err := newEndpoint(next)(ctx, nil) require.Error(t, err) require.False(t, *called) - var permErr *fleet.PermissionError - require.ErrorAs(t, err, &permErr) + requireEndpointRestrictionDenied(t, err) + }) + + t.Run("denial surfaces route and reason on the request log line at info level", func(t *testing.T) { + cases := []struct { + name string + routeTpl string + wantReason string + }{ + { + name: "endpoint not in catalog", + routeTpl: muxTemplate("fleet/secret_admin_endpoint"), + wantReason: "endpoint not in API endpoint catalog", + }, + { + name: "endpoint not in allow-list", + routeTpl: muxTemplate("fleet/scripts/run"), + wantReason: "endpoint not in user's allowed API endpoints", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + next, called := newNext() + lc := &logging.LoggingContext{} + ctx := logging.NewContext(context.Background(), lc) + ctx = context.WithValue(ctx, kithttp.ContextKeyRequestMethod, "POST") + ctx = eu.WithRouteTemplate(ctx, c.routeTpl) + 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) + requireEndpointRestrictionDenied(t, err) + require.False(t, *called) + + require.NotNil(t, lc.ForceLevel) + require.Equal(t, slog.LevelInfo, *lc.ForceLevel) + require.Equal(t, []any{ + "denied_by", "api_only_endpoint_restriction", + "denial_reason", c.wantReason, + "route", c.routeTpl, + }, lc.Extras) + }) + } }) t.Run("api-only user with multiple allowed endpoints, accessing one of them", func(t *testing.T) { From adf5d6ae9c877b28c1b1b8eaad495bc4a8356e0a Mon Sep 17 00:00:00 2001 From: Luke Heath Date: Fri, 7 Aug 2026 16:27:02 -0500 Subject: [PATCH 2/3] Use t.Context() in new subtest --- server/service/middleware/auth/api_only_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/service/middleware/auth/api_only_test.go b/server/service/middleware/auth/api_only_test.go index f8171e40f7e..8890772a536 100644 --- a/server/service/middleware/auth/api_only_test.go +++ b/server/service/middleware/auth/api_only_test.go @@ -345,7 +345,7 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { t.Run(c.name, func(t *testing.T) { next, called := newNext() lc := &logging.LoggingContext{} - ctx := logging.NewContext(context.Background(), lc) + ctx := logging.NewContext(t.Context(), lc) ctx = context.WithValue(ctx, kithttp.ContextKeyRequestMethod, "POST") ctx = eu.WithRouteTemplate(ctx, c.routeTpl) ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{ From 7d60f3e9d4683d3ee827f73c81f12e9bd62a792c Mon Sep 17 00:00:00 2001 From: Luke Heath Date: Sat, 8 Aug 2026 10:50:22 -0500 Subject: [PATCH 3/3] Address review: distinct reason for missing route context, response-body test coverage - Fail closed with a distinct denial_reason when the request method or mux route template is missing from context, instead of reporting a misleading catalog miss with an empty route. - Pin the encoded 403 response (status + distinct message) at the unit level and assert the response body in the endpoint middleware integration test. --- server/service/integration_enterprise_test.go | 18 +++++++++++--- server/service/middleware/auth/api_only.go | 7 ++++++ .../service/middleware/auth/api_only_test.go | 24 ++++++++++++++++++- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 59d18f1b95f..cef22e7b452 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -67,6 +67,7 @@ import ( commonCalendar "github.com/fleetdm/fleet/v4/server/service/calendar" "github.com/fleetdm/fleet/v4/server/service/conditional_access_microsoft_proxy" "github.com/fleetdm/fleet/v4/server/service/contract" + "github.com/fleetdm/fleet/v4/server/service/middleware/auth" "github.com/fleetdm/fleet/v4/server/service/osquery_utils" "github.com/fleetdm/fleet/v4/server/service/redis_lock" "github.com/fleetdm/fleet/v4/server/service/schedule" @@ -31520,6 +31521,14 @@ func (s *integrationEnterpriseTestSuite) TestAPIOnlyUserEndpointMiddleware() { s.Do("GET", "/api/latest/fleet/hosts", nil, http.StatusOK) }) + // requireRestrictionDeniedBody asserts the 403 body carries the distinct + // endpoint-restriction message, distinguishing it from role-based denials. + requireRestrictionDeniedBody := func(t *testing.T, res *http.Response) { + body, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Contains(t, string(body), auth.EndpointRestrictionDeniedMessage) + } + // For api-only users with restrictions, requests to paths not in the API // endpoint catalog are rejected by the middleware before reaching the // service layer. @@ -31527,7 +31536,8 @@ func (s *integrationEnterpriseTestSuite) TestAPIOnlyUserEndpointMiddleware() { s.token = createAPIOnlyUser("api-only-mw-non-catalog-restricted", []map[string]any{ {"method": "GET", "path": "/api/v1/fleet/version"}, }) - s.Do("PATCH", "/api/latest/fleet/users/api_only/1", map[string]any{"name": "x"}, http.StatusForbidden) + res := s.Do("PATCH", "/api/latest/fleet/users/api_only/1", map[string]any{"name": "x"}, http.StatusForbidden) + requireRestrictionDeniedBody(t, res) }) // With endpoint restrictions, only explicitly allowed endpoints are reachable. @@ -31540,8 +31550,10 @@ func (s *integrationEnterpriseTestSuite) TestAPIOnlyUserEndpointMiddleware() { s.Do("GET", "/api/latest/fleet/version", nil, http.StatusOK) // These are in the catalog but not in the user's allow list. - s.Do("GET", "/api/latest/fleet/config", nil, http.StatusForbidden) - s.Do("GET", "/api/latest/fleet/hosts", nil, http.StatusForbidden) + res := s.Do("GET", "/api/latest/fleet/config", nil, http.StatusForbidden) + requireRestrictionDeniedBody(t, res) + res = s.Do("GET", "/api/latest/fleet/hosts", nil, http.StatusForbidden) + requireRestrictionDeniedBody(t, res) }) // Non-api-only users must not be affected by the middleware at all. diff --git a/server/service/middleware/auth/api_only.go b/server/service/middleware/auth/api_only.go index d93b356c34d..6c29efe6839 100644 --- a/server/service/middleware/auth/api_only.go +++ b/server/service/middleware/auth/api_only.go @@ -57,6 +57,13 @@ func apiOnlyEndpointCheck(isInCatalog func(string) bool, next endpoint.Endpoint) requestMethod, _ := ctx.Value(kithttp.ContextKeyRequestMethod).(string) routeTemplate, _ := eu.RouteTemplateFromContext(ctx) + // A missing method or route template means the transport wasn't wired + // with RouteTemplateRequestFunc; fail closed with a reason that points + // at the misconfiguration instead of a misleading catalog miss. + if requestMethod == "" || routeTemplate == "" { + return nil, endpointRestrictionDenied(ctx, routeTemplate, "request method or route template missing from request context") + } + fp := fleet.NewAPIEndpointFromTpl(requestMethod, routeTemplate).Fingerprint() if !isInCatalog(fp) { diff --git a/server/service/middleware/auth/api_only_test.go b/server/service/middleware/auth/api_only_test.go index 8890772a536..f5a79770b17 100644 --- a/server/service/middleware/auth/api_only_test.go +++ b/server/service/middleware/auth/api_only_test.go @@ -340,6 +340,11 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { routeTpl: muxTemplate("fleet/scripts/run"), wantReason: "endpoint not in user's allowed API endpoints", }, + { + name: "route template missing from context", + routeTpl: "", // RouteTemplateRequestFunc not wired + wantReason: "request method or route template missing from request context", + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -347,7 +352,9 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { lc := &logging.LoggingContext{} ctx := logging.NewContext(t.Context(), lc) ctx = context.WithValue(ctx, kithttp.ContextKeyRequestMethod, "POST") - ctx = eu.WithRouteTemplate(ctx, c.routeTpl) + if c.routeTpl != "" { + ctx = eu.WithRouteTemplate(ctx, c.routeTpl) + } ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{ APIOnly: true, APIEndpoints: []fleet.APIEndpointRef{{Method: "GET", Path: "/api/v1/fleet/hosts"}}, @@ -385,6 +392,21 @@ func TestAPIOnlyEndpointCheck(t *testing.T) { }) } +// TestEndpointRestrictionDeniedEncoding pins the HTTP response an +// endpoint-restriction denial encodes to: 403 with the distinct message in the +// body. This guards the UserMessageError wrapping — a bare PermissionError's +// message is discarded by the encoder and rendered as a generic "Permission +// Denied" body. +func TestEndpointRestrictionDeniedEncoding(t *testing.T) { + err := endpointRestrictionDenied(t.Context(), muxTemplate("fleet/hosts"), "endpoint not in user's allowed API endpoints") + + rec := httptest.NewRecorder() + eu.EncodeError(t.Context(), err, rec, nil) + + require.Equal(t, http.StatusForbidden, rec.Code) + require.Contains(t, rec.Body.String(), EndpointRestrictionDeniedMessage) +} + func TestRouteTemplateRequestFunc(t *testing.T) { // Register a route and route the request through mux so mux.CurrentRoute // returns a non-nil value, mirroring what happens in production.