From 8ab6ba86dd6fbb9c421eeaa4e1017676ba95e5c8 Mon Sep 17 00:00:00 2001 From: Tamal Saha Date: Thu, 25 Jun 2026 19:28:38 +0600 Subject: [PATCH 1/2] Enforce authz on resourcegraph and renderrawgraph endpoints The ResourceGraph and RenderRawGraph endpoints returned the object graph for an arbitrary user-supplied source resource without checking whether the caller was allowed to read it. The graph response exposes the names and namespaces of related objects across the cluster, so any user able to call the endpoint could enumerate cluster topology for resources they otherwise cannot see. Gate both endpoints with an RBAC check, mirroring the per-object authorization already done by the core genericresource/podview storages: - When a source object is supplied, require "get" access to it before returning its graph. - RenderRawGraph additionally renders the whole-cluster graph when no source is given; that path now requires cluster-wide read access (get on */*). The rbacAuthorizer is now passed to both storages in apiserver.go. Signed-off-by: Tamal Saha --- pkg/apiserver/apiserver.go | 4 +- pkg/registry/meta/renderrawgraph/storage.go | 74 ++++++++++++++++++++- pkg/registry/meta/resourcegraph/storage.go | 47 ++++++++++++- 3 files changed, 121 insertions(+), 4 deletions(-) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index c2933e231a..58b70b423b 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -326,12 +326,12 @@ func (c completedConfig) New(ctx context.Context) (*UIServer, error) { v1alpha1storage[rsapi.ResourceChartPresetQueries] = chartpresetquery.NewStorage(ctrlClient) v1alpha1storage[rsapi.ResourceClusterStatuses] = clusterstatusstorage.NewStorage(ctrlClient, kc) v1alpha1storage[rsapi.ResourceRenderDashboards] = renderdashboard.NewStorage(ctrlClient) - v1alpha1storage[rsapi.ResourceRenderRawGraphs] = renderrawgraph.NewStorage(ctrlClient) + v1alpha1storage[rsapi.ResourceRenderRawGraphs] = renderrawgraph.NewStorage(ctrlClient, rbacAuthorizer) v1alpha1storage[rsapi.ResourceRenders] = render.NewStorage(ctrlClient, rbacAuthorizer) v1alpha1storage[rsapi.ResourceResourceBlockDefinitions] = resourceblockdefinition.NewStorage() v1alpha1storage[rsapi.ResourceResourceCalculators] = resourcecalculatorstorage.NewStorage(ctrlClient, cid, rbacAuthorizer) v1alpha1storage[rsapi.ResourceResourceDescriptors] = resourcedescriptor.NewStorage() - v1alpha1storage[rsapi.ResourceResourceGraphs] = resourcegraph.NewStorage(ctrlClient) + v1alpha1storage[rsapi.ResourceResourceGraphs] = resourcegraph.NewStorage(ctrlClient, rbacAuthorizer) v1alpha1storage[rsapi.ResourceResourceLayouts] = resourcelayout.NewStorage(ctrlClient) v1alpha1storage[rsapi.ResourceResourceOutlines] = resourceoutline.NewStorage() v1alpha1storage[rsapi.ResourceResourceManifests] = resourcemanifests.NewStorage(ctrlClient) diff --git a/pkg/registry/meta/renderrawgraph/storage.go b/pkg/registry/meta/renderrawgraph/storage.go index c12f911ea0..bb0d81b9ae 100644 --- a/pkg/registry/meta/renderrawgraph/storage.go +++ b/pkg/registry/meta/renderrawgraph/storage.go @@ -18,6 +18,7 @@ package renderrawgraph import ( "context" + "errors" "strings" "kubeops.dev/ui-server/pkg/graph" @@ -26,6 +27,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/authorization/authorizer" + apirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" kmapi "kmodules.xyz/client-go/api/v1" rsapi "kmodules.xyz/resource-metadata/apis/meta/v1alpha1" @@ -34,6 +37,7 @@ import ( type Storage struct { kc client.Client + a authorizer.Authorizer } var ( @@ -44,9 +48,10 @@ var ( _ rest.SingularNameProvider = &Storage{} ) -func NewStorage(kc client.Client) *Storage { +func NewStorage(kc client.Client, a authorizer.Authorizer) *Storage { return &Storage{ kc: kc, + a: a, } } @@ -90,7 +95,16 @@ func (r *Storage) Create(ctx context.Context, obj runtime.Object, _ rest.Validat Namespace: in.Request.Source.Ref.Namespace, Name: in.Request.Source.Ref.Name, } + + // The graph response exposes the names and namespaces of related objects across + // the cluster, so only serve it to callers who can read the source object. + if err := r.authorizeGetSource(ctx, src); err != nil { + return nil, err + } oid = src.OID() + } else if err := r.authorizeClusterRead(ctx); err != nil { + // Without a source the whole cluster graph is rendered; require cluster-wide read access. + return nil, err } resp, err := graph.Render(oid) @@ -100,3 +114,61 @@ func (r *Storage) Create(ctx context.Context, obj runtime.Object, _ rest.Validat in.Response = resp return in, nil } + +// authorizeGetSource ensures the requesting user is allowed to "get" the source object +// before its object graph is returned. +func (r *Storage) authorizeGetSource(ctx context.Context, src kmapi.ObjectID) error { + user, ok := apirequest.UserFrom(ctx) + if !ok { + return apierrors.NewBadRequest("missing user info in request context") + } + + mapping, err := r.kc.RESTMapper().RESTMapping(schema.GroupKind{Group: src.Group, Kind: src.Kind}) + if err != nil { + return apierrors.NewInternalError(err) + } + + attrs := authorizer.AttributesRecord{ + User: user, + Verb: "get", + APIGroup: mapping.Resource.Group, + Resource: mapping.Resource.Resource, + Namespace: src.Namespace, + Name: src.Name, + ResourceRequest: true, + } + decision, why, err := r.a.Authorize(ctx, attrs) + if err != nil { + return apierrors.NewInternalError(err) + } + if decision != authorizer.DecisionAllow { + return apierrors.NewForbidden(mapping.Resource.GroupResource(), src.Name, errors.New(why)) + } + return nil +} + +// authorizeClusterRead requires cluster-wide read access, used when the whole cluster +// graph (no specific source) is requested. Only callers granted "get" on all resources +// in all groups (e.g. cluster-admin) are allowed. +func (r *Storage) authorizeClusterRead(ctx context.Context) error { + user, ok := apirequest.UserFrom(ctx) + if !ok { + return apierrors.NewBadRequest("missing user info in request context") + } + + attrs := authorizer.AttributesRecord{ + User: user, + Verb: "get", + APIGroup: "*", + Resource: "*", + ResourceRequest: true, + } + decision, why, err := r.a.Authorize(ctx, attrs) + if err != nil { + return apierrors.NewInternalError(err) + } + if decision != authorizer.DecisionAllow { + return apierrors.NewForbidden(schema.GroupResource{Resource: rsapi.ResourceRenderRawGraphs}, "", errors.New(why)) + } + return nil +} diff --git a/pkg/registry/meta/resourcegraph/storage.go b/pkg/registry/meta/resourcegraph/storage.go index e093a4e7a7..0d2bb67f5c 100644 --- a/pkg/registry/meta/resourcegraph/storage.go +++ b/pkg/registry/meta/resourcegraph/storage.go @@ -18,6 +18,7 @@ package resourcegraph import ( "context" + "errors" "strings" "kubeops.dev/ui-server/pkg/graph" @@ -26,6 +27,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/authorization/authorizer" + apirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" kmapi "kmodules.xyz/client-go/api/v1" rsapi "kmodules.xyz/resource-metadata/apis/meta/v1alpha1" @@ -34,6 +37,7 @@ import ( type Storage struct { kc client.Client + a authorizer.Authorizer } var ( @@ -44,9 +48,10 @@ var ( _ rest.SingularNameProvider = &Storage{} ) -func NewStorage(kc client.Client) *Storage { +func NewStorage(kc client.Client, a authorizer.Authorizer) *Storage { return &Storage{ kc: kc, + a: a, } } @@ -88,6 +93,14 @@ func (r *Storage) Create(ctx context.Context, obj runtime.Object, _ rest.Validat Namespace: in.Request.Source.Ref.Namespace, Name: in.Request.Source.Ref.Name, } + + // Only return the object graph to callers who are allowed to get the source object. + // The graph response exposes the names and namespaces of related objects across the + // cluster, so it must not be served to users who cannot read the source itself. + if err := r.authorizeGetSource(ctx, src); err != nil { + return nil, err + } + resp, err := graph.ResourceGraph(r.kc.RESTMapper(), src, kmapi.EdgeLabelValues()) if err != nil { return nil, err @@ -95,3 +108,35 @@ func (r *Storage) Create(ctx context.Context, obj runtime.Object, _ rest.Validat in.Response = resp return in, nil } + +// authorizeGetSource ensures the requesting user is allowed to "get" the source object +// before its object graph is returned. +func (r *Storage) authorizeGetSource(ctx context.Context, src kmapi.ObjectID) error { + user, ok := apirequest.UserFrom(ctx) + if !ok { + return apierrors.NewBadRequest("missing user info in request context") + } + + mapping, err := r.kc.RESTMapper().RESTMapping(schema.GroupKind{Group: src.Group, Kind: src.Kind}) + if err != nil { + return apierrors.NewInternalError(err) + } + + attrs := authorizer.AttributesRecord{ + User: user, + Verb: "get", + APIGroup: mapping.Resource.Group, + Resource: mapping.Resource.Resource, + Namespace: src.Namespace, + Name: src.Name, + ResourceRequest: true, + } + decision, why, err := r.a.Authorize(ctx, attrs) + if err != nil { + return apierrors.NewInternalError(err) + } + if decision != authorizer.DecisionAllow { + return apierrors.NewForbidden(mapping.Resource.GroupResource(), src.Name, errors.New(why)) + } + return nil +} From a156a9e2635de897995b1d852060c5761632f9b2 Mon Sep 17 00:00:00 2001 From: Tamal Saha Date: Thu, 25 Jun 2026 20:13:50 +0600 Subject: [PATCH 2/2] test: cover authz gating of resourcegraph and renderrawgraph Add unit tests with a fake authorizer (and a fake client with a static RESTMapper) asserting: - resourcegraph: a source-scoped request denies with Forbidden and authorizes "get" on the referenced object; missing user info is a BadRequest. - renderrawgraph: a source-scoped request authorizes "get" on the object, while the no-source whole-cluster graph requires cluster-wide read (get */*). Signed-off-by: Tamal Saha --- .../meta/renderrawgraph/storage_test.go | 96 +++++++++++++++++++ .../meta/resourcegraph/storage_test.go | 92 ++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 pkg/registry/meta/renderrawgraph/storage_test.go create mode 100644 pkg/registry/meta/resourcegraph/storage_test.go diff --git a/pkg/registry/meta/renderrawgraph/storage_test.go b/pkg/registry/meta/renderrawgraph/storage_test.go new file mode 100644 index 0000000000..ef2e81e36e --- /dev/null +++ b/pkg/registry/meta/renderrawgraph/storage_test.go @@ -0,0 +1,96 @@ +/* +Copyright AppsCode Inc. and Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package renderrawgraph + +import ( + "context" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/authorization/authorizer" + apirequest "k8s.io/apiserver/pkg/endpoints/request" + kmapi "kmodules.xyz/client-go/api/v1" + rsapi "kmodules.xyz/resource-metadata/apis/meta/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +type fakeAuthorizer struct { + decision authorizer.Decision + got authorizer.Attributes +} + +func (f *fakeAuthorizer) Authorize(_ context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { + f.got = a + return f.decision, "because test", nil +} + +func ctxWithUser() context.Context { + return apirequest.WithUser(context.Background(), &user.DefaultInfo{Name: "tester"}) +} + +func testMapper() meta.RESTMapper { + m := meta.NewDefaultRESTMapper([]schema.GroupVersion{{Group: "apps", Version: "v1"}}) + m.Add(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, meta.RESTScopeNamespace) + return m +} + +// TestCreateDeniedForUnreadableSource verifies a source-scoped request requires "get" on +// the referenced object. +func TestCreateDeniedForUnreadableSource(t *testing.T) { + fa := &fakeAuthorizer{decision: authorizer.DecisionDeny} + kc := fake.NewClientBuilder().WithRESTMapper(testMapper()).Build() + r := NewStorage(kc, fa) + + in := &rsapi.RenderRawGraph{ + Request: &rsapi.RenderRawGraphRequest{ + Source: &kmapi.ObjectInfo{ + Resource: kmapi.ResourceID{Group: "apps", Kind: "Deployment"}, + Ref: kmapi.ObjectReference{Namespace: "ns1", Name: "dep1"}, + }, + }, + } + _, err := r.Create(ctxWithUser(), in, nil, nil) + if !apierrors.IsForbidden(err) { + t.Fatalf("expected Forbidden, got %v", err) + } + if fa.got.GetVerb() != "get" || fa.got.GetAPIGroup() != "apps" || fa.got.GetResource() != "deployments" || + fa.got.GetNamespace() != "ns1" || fa.got.GetName() != "dep1" { + t.Errorf("unexpected authz attributes: verb=%q group=%q resource=%q ns=%q name=%q", + fa.got.GetVerb(), fa.got.GetAPIGroup(), fa.got.GetResource(), fa.got.GetNamespace(), fa.got.GetName()) + } +} + +// TestCreateDeniedForWholeGraph verifies the no-source whole-cluster graph requires +// cluster-wide read access (get on */*). +func TestCreateDeniedForWholeGraph(t *testing.T) { + fa := &fakeAuthorizer{decision: authorizer.DecisionDeny} + kc := fake.NewClientBuilder().WithRESTMapper(testMapper()).Build() + r := NewStorage(kc, fa) + + in := &rsapi.RenderRawGraph{Request: &rsapi.RenderRawGraphRequest{}} + _, err := r.Create(ctxWithUser(), in, nil, nil) + if !apierrors.IsForbidden(err) { + t.Fatalf("expected Forbidden, got %v", err) + } + if fa.got.GetVerb() != "get" || fa.got.GetAPIGroup() != "*" || fa.got.GetResource() != "*" { + t.Errorf("expected cluster-wide read (get */*), got verb=%q group=%q resource=%q", + fa.got.GetVerb(), fa.got.GetAPIGroup(), fa.got.GetResource()) + } +} diff --git a/pkg/registry/meta/resourcegraph/storage_test.go b/pkg/registry/meta/resourcegraph/storage_test.go new file mode 100644 index 0000000000..724ef310a9 --- /dev/null +++ b/pkg/registry/meta/resourcegraph/storage_test.go @@ -0,0 +1,92 @@ +/* +Copyright AppsCode Inc. and Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resourcegraph + +import ( + "context" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/authorization/authorizer" + apirequest "k8s.io/apiserver/pkg/endpoints/request" + kmapi "kmodules.xyz/client-go/api/v1" + rsapi "kmodules.xyz/resource-metadata/apis/meta/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +type fakeAuthorizer struct { + decision authorizer.Decision + got authorizer.Attributes +} + +func (f *fakeAuthorizer) Authorize(_ context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { + f.got = a + return f.decision, "because test", nil +} + +func ctxWithUser() context.Context { + return apirequest.WithUser(context.Background(), &user.DefaultInfo{Name: "tester"}) +} + +func testMapper() meta.RESTMapper { + m := meta.NewDefaultRESTMapper([]schema.GroupVersion{{Group: "apps", Version: "v1"}}) + m.Add(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, meta.RESTScopeNamespace) + return m +} + +func deploymentRequest() *rsapi.ResourceGraph { + return &rsapi.ResourceGraph{ + Request: &rsapi.ResourceGraphRequest{ + Source: kmapi.ObjectInfo{ + Resource: kmapi.ResourceID{Group: "apps", Kind: "Deployment"}, + Ref: kmapi.ObjectReference{Namespace: "ns1", Name: "dep1"}, + }, + }, + } +} + +// TestCreateDeniedForUnreadableSource verifies the object graph is only returned to a +// caller allowed to "get" the source object. +func TestCreateDeniedForUnreadableSource(t *testing.T) { + fa := &fakeAuthorizer{decision: authorizer.DecisionDeny} + kc := fake.NewClientBuilder().WithRESTMapper(testMapper()).Build() + r := NewStorage(kc, fa) + + _, err := r.Create(ctxWithUser(), deploymentRequest(), nil, nil) + if !apierrors.IsForbidden(err) { + t.Fatalf("expected Forbidden, got %v", err) + } + if fa.got.GetVerb() != "get" || fa.got.GetAPIGroup() != "apps" || fa.got.GetResource() != "deployments" || + fa.got.GetNamespace() != "ns1" || fa.got.GetName() != "dep1" { + t.Errorf("unexpected authz attributes: verb=%q group=%q resource=%q ns=%q name=%q", + fa.got.GetVerb(), fa.got.GetAPIGroup(), fa.got.GetResource(), fa.got.GetNamespace(), fa.got.GetName()) + } +} + +func TestCreateMissingUserIsBadRequest(t *testing.T) { + fa := &fakeAuthorizer{decision: authorizer.DecisionAllow} + kc := fake.NewClientBuilder().WithRESTMapper(testMapper()).Build() + r := NewStorage(kc, fa) + + _, err := r.Create(context.Background(), deploymentRequest(), nil, nil) + if !apierrors.IsBadRequest(err) { + t.Fatalf("expected BadRequest for missing user, got %v", err) + } +}