Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 15 additions & 3 deletions server/service/integration_enterprise_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -31520,14 +31521,23 @@ 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.
t.Run("non-catalog path is rejected for restricted users", func(t *testing.T) {
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.
Expand All @@ -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.
Expand Down
51 changes: 45 additions & 6 deletions server/service/middleware/auth/api_only.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand All @@ -51,10 +57,17 @@ 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) {
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.
Expand All @@ -64,13 +77,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,
)
}
102 changes: 88 additions & 14 deletions server/service/middleware/auth/api_only_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
}
Comment on lines +51 to +62

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the changed test before inspecting encoder-related call sites.
ast-grep outline server/service/middleware/auth/api_only_test.go --items all

# Locate the error type, its encoder, and request-level tests that cover it.
rg -n -C 5 --type go \
  'NewUserMessageError|UserMessageError|UserMessage\(\)|StatusCode\(\)|PermissionError' \
  server

Repository: fleetdm/fleet

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== api_only_test.go relevant lines =="
sed -n '1,140p' server/service/middleware/auth/api_only_test.go

echo
echo "== endpointor error encoding relevant lines =="
sed -n '90,215p' server/platform/endpointer/transport_error.go

echo
echo "== apiOnlyEndpointRestrictionDenied identifier sources =="
rg -n -C 8 --type go 'apiOnlyEndpointRestrictionDenied|EndpointRestrictionDeniedMessage|requireEndpointRestrictionDenied|NewUserMessageError\(.*EndpointRestrictionDenied' server || true

echo
echo "== request-level tests mentioning 403 and restrictive messages =="
rg -n --type go 'EndpointRestrictionDeniedMessage|requireEndpointRestrictionDenied|StatusCode\(\) == http\.StatusForbidden|require\.Equal\(t, http\.StatusForbidden' server/service server/platform || true

Repository: fleetdm/fleet

Length of output: 30407


Add a request-level assertion for endpoint restriction denials.

requireEndpointRestrictionDenied only checks the error returned by the middleware. Add a real HTTP response assertion that this error encodes http.StatusForbidden with EndpointRestrictionDeniedMessage in the request flow.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/service/middleware/auth/api_only_test.go` around lines 51 - 62, Extend
the endpoint restriction denial tests around requireEndpointRestrictionDenied
with a request-level assertion that executes the middleware flow and verifies
the HTTP response status is http.StatusForbidden and the response body contains
EndpointRestrictionDeniedMessage. Keep the existing error-level assertions
unchanged and use the established request/response test helpers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 7d60f3e: a unit test (TestEndpointRestrictionDeniedEncoding) pins the encoded 403 response (status + message in body) through the real endpointer.EncodeError path, and TestAPIOnlyUserEndpointMiddleware now asserts the response body contains EndpointRestrictionDeniedMessage on both denial paths in the full request flow.


func TestAPIOnlyEndpointCheck(t *testing.T) {
newNext := func() (func(context.Context, any) (any, error), *bool) {
called := false
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -312,8 +321,58 @@ 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",
},
{
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) {
next, called := newNext()
lc := &logging.LoggingContext{}
ctx := logging.NewContext(t.Context(), lc)
ctx = context.WithValue(ctx, kithttp.ContextKeyRequestMethod, "POST")
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"}},
}})

_, 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) {
Expand All @@ -333,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.
Expand Down
Loading