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/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.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 +} 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) + } +}