Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

Skip membership catalog test for API-only users if they have access to everything (#44473)

Browse Source 
Fixes #44403 

Skip membership catalog test for API-only users if they have access to
everything.
This commit is contained in:
Juan Fernandez
2026-04-30 11:24:31 -04:00
committed by GitHub
parent bb15f9d786
commit e58fdca545
4 changed files with 51 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/service/integration_core_test.go
+3 -3
View File
@@ -535,15 +535,15 @@ func (s *integrationTestSuite) TestModifyAPIOnlyUser() {
"name": "New Name",
}, http.StatusUnprocessableEntity)
// An API-only user cannot reach this admin endpoint: the api_only middleware
// rejects it at the catalog check (the user-management endpoint is not in the catalog).
// An API-only user cannot modify itself: the service layer rejects the
// self-modify attempt with 422.
//
// This is to protect against privilege escalation vulnerability.
s.token = apiUserToken
defer func() { s.token = s.getTestAdminToken() }()
s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/users/api_only/%d", apiUserID), map[string]any{
"name": "Self Update",
}, http.StatusForbidden)
}, http.StatusUnprocessableEntity)
s.token = s.getTestAdminToken()
s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/users/api_only/%d", apiUserID), map[string]any{
server/service/integration_enterprise_test.go
+8 -8
View File
@@ -29132,8 +29132,10 @@ func (s *integrationEnterpriseTestSuite) TestAPIOnlyUserEndpointMiddleware() {
return createResp.Token
}
// With no endpoint restrictions the user can reach any endpoint in the catalog.
t.Run("no restrictions allows all catalog endpoints", func(t *testing.T) {
// With no endpoint restrictions the api_only middleware skips entirely, so
// the user can reach any registered route — gated only by role-based authz
// further down the chain.
t.Run("no restrictions skips the middleware", func(t *testing.T) {
s.token = createAPIOnlyUser("api-only-mw-no-restrictions", nil)
s.Do("GET", "/api/latest/fleet/version", nil, http.StatusOK)
@@ -29141,12 +29143,10 @@ func (s *integrationEnterpriseTestSuite) TestAPIOnlyUserEndpointMiddleware() {
s.Do("GET", "/api/latest/fleet/hosts", nil, http.StatusOK)
})
// Paths not registered in the API endpoint catalog are always rejected for
// api-only users, regardless of whether they have endpoint restrictions.
t.Run("non-catalog path is rejected", func(t *testing.T) {
s.token = createAPIOnlyUser("api-only-mw-non-catalog-unrestricted", nil)
s.Do("PATCH", "/api/latest/fleet/users/api_only/1", map[string]any{"name": "x"}, http.StatusForbidden)
// 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"},
})
server/service/middleware/auth/api_only.go
+15 -18
View File
@@ -19,22 +19,24 @@ import (
var RouteTemplateRequestFunc = eu.RouteTemplateRequestFunc
// APIOnlyEndpointCheck returns an endpoint.Endpoint middleware that enforces
// access control for API-only users (api_only=true). It must be wired inside
// AuthenticatedUser (so a Viewer is already in context when it runs) and the
// enclosing transport must register RouteTemplateRequestFunc as a ServerBefore
// option so the mux route template is available in context.
// access control for API-only users (api_only=true) that have configured
// endpoint restrictions. It must be wired inside AuthenticatedUser (so a Viewer
// is already in context when it runs) and the enclosing transport must register
// RouteTemplateRequestFunc as a ServerBefore option so the mux route template
// is available in context.
//
// For non-API-only users the check is skipped entirely. When there is no Viewer
// in context, the call passes through — AuthenticatedUser guarantees that any
// request that needs a Viewer has already been rejected before reaching here.
// The check is skipped entirely for: non-API-only users, requests with no
// Viewer in context (AuthenticatedUser already rejects those), and API-only
// users with no endpoint restrictions configured — the latter are granted
// access to every registered route, gated only by role-based authz further
// down the chain.
//
// For API-only users two checks are applied in order:
// 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.
// 2. If the user has configured endpoint restrictions (rows in
// user_api_endpoints), the route must match one of them. If not, a
// permission error (403) is returned. An empty restriction list grants
// full access to all catalog endpoints.
// 2. The route must match one of the user's allowed endpoints. If not, a
// permission error (403) is returned.
func APIOnlyEndpointCheck(next endpoint.Endpoint) endpoint.Endpoint {
return apiOnlyEndpointCheck(apiendpoints.IsInCatalog, next)
}
@@ -42,7 +44,7 @@ func APIOnlyEndpointCheck(next endpoint.Endpoint) endpoint.Endpoint {
func apiOnlyEndpointCheck(isInCatalog func(string) bool, next endpoint.Endpoint) endpoint.Endpoint {
return func(ctx context.Context, request any) (any, error) {
v, ok := viewer.FromContext(ctx)
if !ok || v.User == nil || !v.User.APIOnly {
if !ok || v.User == nil || !v.User.APIOnly || len(v.User.APIEndpoints) == 0 {
return next(ctx, request)
}
@@ -55,11 +57,6 @@ func apiOnlyEndpointCheck(isInCatalog func(string) bool, next endpoint.Endpoint)
return nil, permissionDenied(ctx)
}
// No endpoint restrictions: full access to all catalog endpoints.
if len(v.User.APIEndpoints) == 0 {
return next(ctx, request)
}
// Check whether the requested endpoint matches any of the user's allowed endpoints.
for _, ep := range v.User.APIEndpoints {
if fleet.NewAPIEndpointFromTpl(ep.Method, ep.Path).Fingerprint() == fp {
server/service/middleware/auth/api_only_test.go
+25 -10
View File
@@ -125,10 +125,15 @@ func TestAPIOnlyEndpointCheck(t *testing.T) {
require.True(t, *called)
})
t.Run("api-only user, endpoint not in catalog", func(t *testing.T) {
t.Run("api-only user with restrictions, endpoint not in catalog", func(t *testing.T) {
next, called := newNext()
ctx := ctxWithMethod("GET", muxTemplate("fleet/secret_admin_endpoint"))
ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{APIOnly: true}})
ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{
APIOnly: true,
// Non-empty restrictions force the middleware to run; the route
// is not in the catalog, so the catalog check rejects it.
APIEndpoints: []fleet.APIEndpointRef{{Method: "GET", Path: "/api/v1/fleet/hosts"}},
}})
_, err := newEndpoint(next)(ctx, nil)
require.Error(t, err)
@@ -137,12 +142,15 @@ func TestAPIOnlyEndpointCheck(t *testing.T) {
require.ErrorAs(t, err, &permErr)
})
t.Run("api-only user, missing route template in context is rejected", func(t *testing.T) {
t.Run("api-only user with restrictions, missing route template in context is rejected", func(t *testing.T) {
next, called := newNext()
// routeTemplateKey deliberately not set (simulates RouteTemplateRequestFunc failure).
ctx := context.Background()
ctx = context.WithValue(ctx, kithttp.ContextKeyRequestMethod, "GET")
ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{APIOnly: true}})
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)
@@ -151,11 +159,14 @@ func TestAPIOnlyEndpointCheck(t *testing.T) {
require.ErrorAs(t, err, &permErr)
})
t.Run("api-only user, missing method and template are both rejected", func(t *testing.T) {
t.Run("api-only user with restrictions, missing method and template are both rejected", func(t *testing.T) {
next, called := newNext()
// Neither method nor template set — empty fingerprint never matches catalog.
ctx := context.Background()
ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{APIOnly: true}})
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)
@@ -165,12 +176,13 @@ func TestAPIOnlyEndpointCheck(t *testing.T) {
})
t.Run("api-only user, method normalization is case-insensitive", func(t *testing.T) {
// Lower-case method must normalize to the same fingerprint as upper-case.
// Lower-case method must normalize to the same fingerprint as upper-case
// when matching against the catalog and the user's allow-list.
next, called := newNext()
ctx := ctxWithMethod("get", muxTemplate("fleet/hosts")) // lower-case
ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{
APIOnly: true,
APIEndpoints: nil,
APIEndpoints: []fleet.APIEndpointRef{{Method: "GET", Path: "/api/v1/fleet/hosts"}},
}})
_, err := newEndpoint(next)(ctx, nil)
@@ -178,7 +190,7 @@ func TestAPIOnlyEndpointCheck(t *testing.T) {
require.True(t, *called)
})
t.Run("api-only user, rejection marks authz context as checked", func(t *testing.T) {
t.Run("api-only user with restrictions, rejection marks authz context as checked", func(t *testing.T) {
// Ensures authzcheck middleware does not emit a spurious "Missing
// authorization check" log when we deny an api_only user.
next, called := newNext()
@@ -186,7 +198,10 @@ func TestAPIOnlyEndpointCheck(t *testing.T) {
ctx := authzctx.NewContext(context.Background(), ac)
ctx = context.WithValue(ctx, kithttp.ContextKeyRequestMethod, "GET")
ctx = eu.WithRouteTemplate(ctx, muxTemplate("fleet/secret_admin_endpoint"))
ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{APIOnly: true}})
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)