From efeb7405e8fbadbb20c9a750026c594d91f8e808 Mon Sep 17 00:00:00 2001 From: Jet Chiang Date: Mon, 19 Oct 2026 11:05:02 -0400 Subject: [PATCH 1/3] k8s rbac Signed-off-by: Jet Chiang --- .../references/scoped-authorization.md | 26 ++-- .../grpcserver/agenttemplate_harness_test.go | 10 +- .../grpcserver/resource_authorization_test.go | 101 ++++++++++++++ go/core/internal/grpcserver/tool_test.go | 27 +++- go/core/internal/mcp/sandboxes_test.go | 82 +++++++++++ go/core/internal/service/kubeauth/scope.go | 31 +++++ .../internal/service/kubeauth/scope_test.go | 60 ++++++++ go/core/internal/service/kubecrud/service.go | 20 +-- .../internal/service/kubecrud/service_test.go | 5 + go/core/internal/service/model/discovery.go | 32 +++-- .../internal/service/model/discovery_test.go | 95 ++++++++++--- go/core/internal/service/model/service.go | 7 +- .../service/prompttemplate/service.go | 78 ++++++----- .../service/prompttemplate/service_test.go | 86 +++++++++++- .../internal/service/sandbox/service_test.go | 56 ++++++++ go/core/internal/service/system/service.go | 4 +- .../internal/service/system/service_test.go | 115 +++++++++++++++- go/core/internal/service/system/substrate.go | 69 +++++++++- go/core/internal/service/tool/service.go | 91 +++++++++--- go/core/internal/service/tool/service_test.go | 129 ++++++++++++++++-- 20 files changed, 982 insertions(+), 142 deletions(-) create mode 100644 go/core/internal/grpcserver/resource_authorization_test.go diff --git a/.agents/skills/kagent-dev/references/scoped-authorization.md b/.agents/skills/kagent-dev/references/scoped-authorization.md index 9a6afdbe5c..87f289fe90 100644 --- a/.agents/skills/kagent-dev/references/scoped-authorization.md +++ b/.agents/skills/kagent-dev/references/scoped-authorization.md @@ -23,17 +23,13 @@ is represented as two `AnyOf` clauses. The first contains both predicates in `Al The shared scope types and attribute names live in [`go/api/authorization`](../../../../go/api/authorization/scope.go). Services use those public types directly. -## Trusted resource attributes +## Resource identities -Use [`kubeauth.Resource`](../../../../go/core/internal/service/kubeauth/scope.go) to construct authorization input from a Kubernetes object. It supplies `namespace` and `name` from object metadata. - -For a single-resource operation: - -- Read and delete: load the stored object, then authorize it. -- Create: validate and normalize the proposed object, then authorize it before checking or writing storage. -- Update: authorize the stored object before any write. - -Do not treat a request reference as stored resource data. Request references are suitable for loading an object, not for constructing its trusted attributes. +Current object checks use `auth.Resource{Type, Namespace, Name}`. +Validate the requested reference and authorize its exact namespace/name before reading protected content or writing storage. +This ordering prevents a denied caller from distinguishing a missing object from an existing one. +Creates validate and normalize the proposed object identity before checking authorization. +Do not add caller-controlled attributes beyond this identity without defining their trusted source and validation. ## Collection flow @@ -54,10 +50,12 @@ Pass scopes as ordinary function arguments. Do not place policy decisions in req ## Service integration -- `AgentTemplate` and `Harness` use `kubecrud.NewService`, which requires a `CollectionAuthorizer` and always applies collection scopes. -- `ModelConfig` requests require a `CollectionAuthorizer`; `Service.List` filters the returned Kubernetes list before transport conversion. -- `ListConfiguredProviders` reads the separate `ModelProviderConfig` resource and remains outside this scope. -- Legacy `AgentHarness` and `SandboxAgent` operations remain outside this scope. +- `Agent`, `AgentTemplate`, `Harness`, and `SandboxTemplate` use `kubecrud.NewService` with a required `CollectionAuthorizer`. +- `ModelConfig` and `ModelProviderConfig` use separate typed CRUD services and resource types. Configured-provider reads and refresh are authorized independently from ModelConfig operations. +- Prompt libraries, tool discovery, and WorkerPool summary rows use `kubeauth.CollectionMatcher` for custom collection projection. +- Flat tool discovery must filter through the same exact owning-server identities as server discovery. +- WorkerPool scope is independent from the mixed summary's whole-inventory Substrate permission. +- Static provider definitions and supported tool-server types expose no configured object and remain available after authentication. The default OSS no-op authorizer implements `CollectionAuthorizer` and returns `ScopeAll`, preserving current OSS visibility. diff --git a/go/core/internal/grpcserver/agenttemplate_harness_test.go b/go/core/internal/grpcserver/agenttemplate_harness_test.go index c81bcaccfa..627c82e025 100644 --- a/go/core/internal/grpcserver/agenttemplate_harness_test.go +++ b/go/core/internal/grpcserver/agenttemplate_harness_test.go @@ -37,10 +37,10 @@ func newTemplateAndHarnessConnection(t *testing.T, objects ...ctrlclient.Object) return newConfigurationConnection(t, kubeClient) } -func newConfigurationConnection(t *testing.T, kubeClient ctrlclient.Client) *grpc.ClientConn { +func newConfigurationConnection(t *testing.T, kubeClient ctrlclient.Client, configure ...func(*Config)) *grpc.ClientConn { t.Helper() listener := bufconn.Listen(DefaultMaxMessageSize) - server, err := New(Config{ + config := Config{ Listener: listener, Authenticator: &authimpl.InsecureAuthenticator{}, SystemService: testSystemService(), @@ -48,7 +48,11 @@ func newConfigurationConnection(t *testing.T, kubeClient ctrlclient.Client) *grp AgentTemplateService: kubecrud.NewService(kubeClient, &pkgauth.NoopAuthorizer{}, &v1alpha3.AgentTemplate{}, &v1alpha3.AgentTemplateList{}, "AgentTemplate"), HarnessService: kubecrud.NewService(kubeClient, &pkgauth.NoopAuthorizer{}, &v1alpha3.Harness{}, &v1alpha3.HarnessList{}, "Harness"), SandboxTemplateService: kubecrud.NewService(kubeClient, &pkgauth.NoopAuthorizer{}, &v1alpha3.SandboxTemplate{}, &v1alpha3.SandboxTemplateList{}, v1alpha3.SandboxTemplateKind), - }) + } + for _, apply := range configure { + apply(&config) + } + server, err := New(config) if err != nil { t.Fatalf("New() error = %v", err) } diff --git a/go/core/internal/grpcserver/resource_authorization_test.go b/go/core/internal/grpcserver/resource_authorization_test.go new file mode 100644 index 0000000000..d2aee73b89 --- /dev/null +++ b/go/core/internal/grpcserver/resource_authorization_test.go @@ -0,0 +1,101 @@ +package grpcserver + +import ( + "context" + "errors" + "sync/atomic" + "testing" + + atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" + apiv1 "github.com/kagent-dev/kagent/go/api/gen/kagent/api/v1" + "github.com/kagent-dev/kagent/go/api/v1alpha3" + modelservice "github.com/kagent-dev/kagent/go/core/internal/service/model" + promptservice "github.com/kagent-dev/kagent/go/core/internal/service/prompttemplate" + systemservice "github.com/kagent-dev/kagent/go/core/internal/service/system" + "github.com/kagent-dev/kagent/go/core/pkg/auth" +) + +type resourceAuthorizer struct{} + +func (resourceAuthorizer) Check(_ context.Context, _ auth.Principal, verb auth.Verb, resource auth.Resource) error { + if resource.Name != "allowed" || (resource.Type == "ModelProviderConfig" && verb == auth.VerbUpdate) { + return errors.New("denied") + } + return nil +} + +func (resourceAuthorizer) Scope(_ context.Context, _ auth.Principal, _ auth.Verb, resource string) (apiauthorization.AuthorizationScope, error) { + if resource == "Substrate" { + return apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}, nil + } + return apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf, AnyOf: []apiauthorization.ScopeClause{{All: []apiauthorization.ScopePredicate{{Attribute: apiauthorization.AttributeName, Operator: apiauthorization.ScopeIn, Values: []string{"allowed"}}}}}}, nil +} + +type deniedProviderRefresher struct{ calls atomic.Int32 } + +func (r *deniedProviderRefresher) RefreshModelProviderConfigModels(context.Context, string, string) ([]string, error) { + r.calls.Add(1) + return []string{"unexpected"}, nil +} + +func TestKubernetesResourceScopesThroughGeneratedClients(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, v1alpha3.AddToScheme(scheme)) + require.NoError(t, atev1alpha1.AddToScheme(scheme)) + kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "allowed", Labels: map[string]string{"kagent.dev/prompt-library": "true"}}, Data: map[string]string{"visible": "prompt"}}, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "hidden", Labels: map[string]string{"kagent.dev/prompt-library": "true"}}, Data: map[string]string{"private": "prompt"}}, + &v1alpha3.ModelProviderConfig{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "allowed"}, Status: v1alpha3.ModelProviderConfigStatus{Conditions: []metav1.Condition{{Type: v1alpha3.ModelProviderConfigConditionTypeReady, Status: metav1.ConditionTrue}}, DiscoveredModels: []string{"cached"}}}, + &v1alpha3.ModelProviderConfig{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "hidden"}, Status: v1alpha3.ModelProviderConfigStatus{Conditions: []metav1.Condition{{Type: v1alpha3.ModelProviderConfigConditionTypeReady, Status: metav1.ConditionTrue}}}}, + &atev1alpha1.WorkerPool{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "allowed"}}, + &atev1alpha1.WorkerPool{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "hidden"}}, + ).Build() + authorizer := resourceAuthorizer{} + refresher := &deniedProviderRefresher{} + connection := newConfigurationConnection(t, kube, func(config *Config) { + config.PromptTemplateService = promptservice.NewService(kube, authorizer) + config.ModelService = modelservice.NewService(kube, authorizer, "team", modelservice.WithProviderModelRefresher(refresher)) + config.SystemService = systemservice.NewService(kube, []string{"team"}, authorizer, nil) + }) + ctx := metadata.NewOutgoingContext(t.Context(), metadata.Pairs("x-user-id", "reader")) + prompts := apiv1.NewPromptTemplateServiceClient(connection) + libraries, err := prompts.ListPromptTemplates(ctx, &apiv1.ListPromptTemplatesRequest{Namespace: "team"}) + require.NoError(t, err) + require.Len(t, libraries.GetPromptTemplates(), 1) + require.Equal(t, "allowed", libraries.GetPromptTemplates()[0].GetRef().GetName()) + _, err = prompts.GetPromptTemplate(ctx, &apiv1.GetPromptTemplateRequest{Ref: &apiv1.ResourceReference{Namespace: "team", Name: "hidden"}}) + assertGRPCCode(t, err, codes.PermissionDenied) + _, err = prompts.CreatePromptTemplate(ctx, &apiv1.CreatePromptTemplateRequest{Ref: &apiv1.ResourceReference{Namespace: "team", Name: "denied"}, Data: map[string]string{"key": "value"}}) + assertGRPCCode(t, err, codes.PermissionDenied) + + models := apiv1.NewModelServiceClient(connection) + providers, err := models.ListConfiguredProviders(ctx, &apiv1.ListConfiguredProvidersRequest{}) + require.NoError(t, err) + require.Len(t, providers.GetProviders(), 1) + require.Equal(t, "allowed", providers.GetProviders()[0].GetName()) + cached, err := models.ListProviderModels(ctx, &apiv1.ListProviderModelsRequest{ProviderName: "allowed"}) + require.NoError(t, err) + require.Equal(t, []string{"cached"}, cached.GetModels()) + _, err = models.ListProviderModels(ctx, &apiv1.ListProviderModelsRequest{ProviderName: "hidden"}) + assertGRPCCode(t, err, codes.PermissionDenied) + _, err = models.ListProviderModels(ctx, &apiv1.ListProviderModelsRequest{ProviderName: "allowed", Refresh: true}) + assertGRPCCode(t, err, codes.PermissionDenied) + require.Zero(t, refresher.calls.Load()) + + summary, err := apiv1.NewSystemServiceClient(connection).GetSubstrateSummary(ctx, &apiv1.GetSubstrateSummaryRequest{Namespace: "team"}) + require.NoError(t, err) + require.Len(t, summary.GetWorkerPools(), 1) + require.Equal(t, "allowed", summary.GetWorkerPools()[0].GetRef().GetName()) + require.Zero(t, summary.GetActorCount()) + require.Empty(t, summary.GetActorTemplates()) +} diff --git a/go/core/internal/grpcserver/tool_test.go b/go/core/internal/grpcserver/tool_test.go index a6b44c45ca..5c2369842a 100644 --- a/go/core/internal/grpcserver/tool_test.go +++ b/go/core/internal/grpcserver/tool_test.go @@ -23,6 +23,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" apiv1 "github.com/kagent-dev/kagent/go/api/gen/kagent/api/v1" "github.com/kagent-dev/kagent/go/api/structuredobject" "github.com/kagent-dev/kagent/go/api/v1alpha3" @@ -93,6 +94,16 @@ func (a *toolGRPCAuthorizer) Check(_ context.Context, _ pkgAuth.Principal, verb return nil } +func (a *toolGRPCAuthorizer) Scope(_ context.Context, _ pkgAuth.Principal, verb pkgAuth.Verb, _ string) (apiauthorization.AuthorizationScope, error) { + a.mu.Lock() + defer a.mu.Unlock() + a.verb = verb + if a.deny { + return apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}, nil + } + return apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAll}, nil +} + func (a *toolGRPCAuthorizer) lastVerb() pkgAuth.Verb { a.mu.Lock() defer a.mu.Unlock() @@ -143,8 +154,8 @@ func TestToolServiceGeneratedClient(t *testing.T) { if len(listedServers.GetToolServers()) != 1 || listedServers.GetToolServers()[0].GetDiscoveredTools()[0].GetName() != "move_task" { t.Fatalf("ListToolServers() = %+v", listedServers.GetToolServers()) } - if authorizer.lastVerb() != pkgAuth.VerbGet { - t.Fatalf("ListToolServers() authorization verb = %q, want get", authorizer.lastVerb()) + if authorizer.lastVerb() != pkgAuth.VerbList { + t.Fatalf("ListToolServers() authorization verb = %q, want list", authorizer.lastVerb()) } serverTypes, err := toolClient.ListToolServerTypes(ctx, &apiv1.ListToolServerTypesRequest{}) @@ -254,7 +265,17 @@ func TestToolServiceGeneratedClient(t *testing.T) { assertGRPCCode(t, err, codes.InvalidArgument) authorizer.setDenied(true) - _, err = toolClient.ListToolServers(ctx, &apiv1.ListToolServersRequest{}) + deniedServers, err := toolClient.ListToolServers(ctx, &apiv1.ListToolServersRequest{}) + assertGRPCCode(t, err, codes.OK) + if len(deniedServers.GetToolServers()) != 0 { + t.Fatal("denied server list exposed resources") + } + deniedTools, err := toolClient.ListTools(ctx, &apiv1.ListToolsRequest{}) + assertGRPCCode(t, err, codes.OK) + if len(deniedTools.GetTools()) != 0 { + t.Fatal("denied flat tool list exposed resources") + } + _, err = toolClient.CallMCPAppTool(ctx, &apiv1.CallMCPAppToolRequest{Server: mcpRef, ToolName: "move_task", Arguments: arguments}) assertGRPCCode(t, err, codes.PermissionDenied) authorizer.setDenied(false) diff --git a/go/core/internal/mcp/sandboxes_test.go b/go/core/internal/mcp/sandboxes_test.go index d334efb7f5..4eb71bc394 100644 --- a/go/core/internal/mcp/sandboxes_test.go +++ b/go/core/internal/mcp/sandboxes_test.go @@ -2,10 +2,12 @@ package mcp import ( "bytes" + "context" "errors" "image" "image/png" "io" + "net/http" "net/http/httptest" "strings" "testing" @@ -14,8 +16,15 @@ import ( "github.com/a2aproject/a2a-go/v2/a2asrv" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" + "github.com/kagent-dev/kagent/go/api/v1alpha3" + "github.com/kagent-dev/kagent/go/core/internal/service/kubecrud" "github.com/kagent-dev/kagent/go/core/internal/service/sandbox" + "github.com/kagent-dev/kagent/go/core/pkg/auth" ) func TestSandboxToolsRegisteredAndValidateBeforeDispatch(t *testing.T) { @@ -112,3 +121,76 @@ func TestReadFileContent(t *testing.T) { }) } } + +// The MCP template discovery path must apply the same scope as the CRUD API. +type templateListAuthorizer struct { + auth.NoopAuthorizer + scope apiauthorization.AuthorizationScope + scopeErr error + verb auth.Verb + resourceType string +} + +var _ auth.CollectionAuthorizer = (*templateListAuthorizer)(nil) + +func (a *templateListAuthorizer) Scope(_ context.Context, _ auth.Principal, verb auth.Verb, resourceType string) (apiauthorization.AuthorizationScope, error) { + a.verb, a.resourceType = verb, resourceType + return a.scope, a.scopeErr +} + +func TestSandboxTemplateToolFiltersAuthorizedTemplates(t *testing.T) { + for _, test := range []struct { + name string + scope apiauthorization.AuthorizationScope + scopeErr error + want []any + wantError bool + }{ + {name: "all", scope: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAll}, want: []any{ + map[string]any{"namespace": "other", "name": "scratch"}, + map[string]any{"namespace": "team-a", "name": "hidden"}, + map[string]any{"namespace": "team-a", "name": "scratch"}, + }}, + {name: "namespace and name", scope: apiauthorization.AuthorizationScope{ + Kind: apiauthorization.ScopeAnyOf, + AnyOf: []apiauthorization.ScopeClause{{All: []apiauthorization.ScopePredicate{ + {Attribute: apiauthorization.AttributeNamespace, Operator: apiauthorization.ScopeIn, Values: []string{"team-a"}}, + {Attribute: apiauthorization.AttributeName, Operator: apiauthorization.ScopeIn, Values: []string{"scratch"}}, + }}}, + }, want: []any{map[string]any{"namespace": "team-a", "name": "scratch"}}}, + {name: "none", scope: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}, want: []any{}}, + {name: "invalid scope", scope: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf}, wantError: true}, + {name: "scope error", scopeErr: errors.New("authorization unavailable"), wantError: true}, + } { + t.Run(test.name, func(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, v1alpha3.AddToScheme(scheme)) + kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + &v1alpha3.SandboxTemplate{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "scratch"}}, + &v1alpha3.SandboxTemplate{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "hidden"}}, + &v1alpha3.SandboxTemplate{ObjectMeta: metav1.ObjectMeta{Namespace: "other", Name: "scratch"}}, + ).Build() + authorizer := &templateListAuthorizer{scope: test.scope, scopeErr: test.scopeErr} + templates := kubecrud.NewService(kube, authorizer, &v1alpha3.SandboxTemplate{}, &v1alpha3.SandboxTemplateList{}, v1alpha3.SandboxTemplateKind) + handler, err := New(testSessionService(), testCheckpointService(), &a2asrv.InterceptedHandler{Handler: &fakeGateway{}}, nil, templates) + require.NoError(t, err) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handler.ServeHTTP(w, r.WithContext(auth.AuthSessionTo(r.Context(), testSession{}))) + })) + defer server.Close() + response := rawMCPCall(t, server.URL, "tools/call", map[string]any{ + "name": "list_sandbox_templates", "arguments": map[string]any{}, + }, false) + result := response["result"].(map[string]any) + require.Equal(t, auth.VerbList, authorizer.verb) + require.Equal(t, v1alpha3.SandboxTemplateKind, authorizer.resourceType) + if test.wantError { + require.Equal(t, true, result["isError"]) + require.Empty(t, result["structuredContent"].(map[string]any)["templates"]) + return + } + require.NotEqual(t, true, result["isError"]) + require.Equal(t, test.want, result["structuredContent"].(map[string]any)["templates"]) + }) + } +} diff --git a/go/core/internal/service/kubeauth/scope.go b/go/core/internal/service/kubeauth/scope.go index a44fbad3d1..efb58118c9 100644 --- a/go/core/internal/service/kubeauth/scope.go +++ b/go/core/internal/service/kubeauth/scope.go @@ -1,12 +1,15 @@ package kubeauth import ( + "context" "fmt" "slices" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" + "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" + "github.com/kagent-dev/kagent/go/core/pkg/auth" ) // Matcher is a validated authorization scope that can be applied to Kubernetes objects. @@ -88,3 +91,31 @@ func (m Matcher) Matches(object metav1.Object) bool { } return false } + +// CollectionMatcher resolves and validates a caller's Kubernetes object scope. +// Custom collection services use the same authentication and failure handling so +// a policy lookup failure cannot silently become an unrestricted collection. +func CollectionMatcher(ctx context.Context, authorizer auth.CollectionAuthorizer, resourceType string) (Matcher, error) { + session, ok := auth.AuthSessionFrom(ctx) + if !ok { + return Matcher{}, serviceerrors.NewUnauthenticated("Authentication is required", nil) + } + if authorizer == nil { + return Matcher{}, serviceerrors.NewInternal("Authorization is not configured", nil) + } + scope, err := authorizer.Scope(ctx, session.Principal(), auth.VerbList, resourceType) + if err != nil { + return Matcher{}, serviceerrors.NewUnavailable("Failed to read the "+resourceType+" authorization scope", err) + } + matcher, err := CompileScope(scope) + if err != nil { + return Matcher{}, serviceerrors.NewInternal("Failed to apply the "+resourceType+" authorization scope", err) + } + return matcher, nil +} + +// Empty reports whether no object can match. Services can avoid reading a denied +// collection, so its storage availability does not gate an independent section. +func (m Matcher) Empty() bool { + return m.scope.Kind == "" || m.scope.Kind == apiauthorization.ScopeNone +} diff --git a/go/core/internal/service/kubeauth/scope_test.go b/go/core/internal/service/kubeauth/scope_test.go index 3930d9aadc..1af50317bd 100644 --- a/go/core/internal/service/kubeauth/scope_test.go +++ b/go/core/internal/service/kubeauth/scope_test.go @@ -1,12 +1,18 @@ package kubeauth_test import ( + "context" + "errors" "testing" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" + authimpl "github.com/kagent-dev/kagent/go/core/internal/httpserver/auth" "github.com/kagent-dev/kagent/go/core/internal/service/kubeauth" + "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" + "github.com/kagent-dev/kagent/go/core/pkg/auth" ) func TestMatcher(t *testing.T) { @@ -115,3 +121,57 @@ func TestCompileScopeRejectsInvalidScopes(t *testing.T) { }) } } + +type collectionAuthorizer struct { + auth.NoopAuthorizer + scope apiauthorization.AuthorizationScope + err error + principal auth.Principal + verb auth.Verb + resource string +} + +func (a *collectionAuthorizer) Scope(_ context.Context, principal auth.Principal, verb auth.Verb, resource string) (apiauthorization.AuthorizationScope, error) { + a.principal, a.verb, a.resource = principal, verb, resource + return a.scope, a.err +} + +func TestCollectionMatcher(t *testing.T) { + principal := auth.Principal{User: auth.User{ID: "reader"}} + ctx := auth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{P: principal}) + for _, test := range []struct { + name string + scope apiauthorization.AuthorizationScope + err error + code serviceerrors.Code + matches bool + }{ + {name: "all", scope: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAll}, matches: true}, + {name: "none", scope: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}}, + {name: "invalid", scope: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf}, code: serviceerrors.CodeInternal}, + {name: "unavailable", err: errors.New("evaluator unavailable"), code: serviceerrors.CodeUnavailable}, + } { + t.Run(test.name, func(t *testing.T) { + authorizer := &collectionAuthorizer{scope: test.scope, err: test.err} + matcher, err := kubeauth.CollectionMatcher(ctx, authorizer, "WorkerPool") + if test.code != "" { + require.True(t, serviceerrors.IsCode(err, test.code)) + require.False(t, matcher.Matches(&metav1.ObjectMeta{Namespace: "team", Name: "pool"})) + if test.err != nil { + require.ErrorIs(t, err, test.err) + } + } else { + require.NoError(t, err) + require.Equal(t, test.matches, matcher.Matches(&metav1.ObjectMeta{Namespace: "team", Name: "pool"})) + require.Equal(t, test.scope.Kind == apiauthorization.ScopeNone, matcher.Empty()) + } + require.Equal(t, principal, authorizer.principal) + require.Equal(t, auth.VerbList, authorizer.verb) + require.Equal(t, "WorkerPool", authorizer.resource) + }) + } + _, err := kubeauth.CollectionMatcher(t.Context(), auth.NoopAuthorizer{}, "WorkerPool") + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeUnauthenticated)) + _, err = kubeauth.CollectionMatcher(ctx, nil, "WorkerPool") + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeInternal)) +} diff --git a/go/core/internal/service/kubecrud/service.go b/go/core/internal/service/kubecrud/service.go index 65b0ff7c8f..6543680e82 100644 --- a/go/core/internal/service/kubecrud/service.go +++ b/go/core/internal/service/kubecrud/service.go @@ -13,7 +13,6 @@ import ( utilvalidation "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/controller-runtime/pkg/client" - apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" "github.com/kagent-dev/kagent/go/core/internal/service/kubeauth" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" "github.com/kagent-dev/kagent/go/core/pkg/auth" @@ -45,13 +44,12 @@ func NewService[T Object, L client.ObjectList]( } func (s *Service[T, L]) List(ctx context.Context, namespace string) ([]T, error) { - scope, err := s.scope(ctx, auth.VerbList) + matcher, err := kubeauth.CollectionMatcher(ctx, s.authorizer, s.resource) if err != nil { return nil, err } - matcher, err := kubeauth.CompileScope(scope) - if err != nil { - return nil, serviceerrors.NewInternal("Failed to apply the "+s.resource+" authorization scope", err) + if matcher.Empty() { + return []T{}, nil } list := s.list.DeepCopyObject().(L) if err := s.client.List(ctx, list, client.InNamespace(namespace)); err != nil { @@ -144,18 +142,6 @@ func (s *Service[T, L]) Delete(ctx context.Context, ref types.NamespacedName) er return nil } -func (s *Service[T, L]) scope(ctx context.Context, verb auth.Verb) (apiauthorization.AuthorizationScope, error) { - session, ok := auth.AuthSessionFrom(ctx) - if !ok || session == nil { - return apiauthorization.AuthorizationScope{}, serviceerrors.NewUnauthenticated("Failed to get authenticated principal", fmt.Errorf("no session found")) - } - scope, err := s.authorizer.Scope(ctx, session.Principal(), verb, s.resource) - if err != nil { - return apiauthorization.AuthorizationScope{}, serviceerrors.NewUnavailable("Failed to read the "+s.resource+" authorization scope", err) - } - return scope, nil -} - func (s *Service[T, L]) get(ctx context.Context, ref types.NamespacedName) (T, error) { var zero T object := s.object.DeepCopyObject().(T) diff --git a/go/core/internal/service/kubecrud/service_test.go b/go/core/internal/service/kubecrud/service_test.go index 8dc05d7e68..59c60ed2df 100644 --- a/go/core/internal/service/kubecrud/service_test.go +++ b/go/core/internal/service/kubecrud/service_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -183,6 +184,10 @@ func TestSandboxTemplateAuthorization(t *testing.T) { {verb: auth.VerbDelete, resource: auth.Resource{Type: v1alpha3.SandboxTemplateKind, Namespace: "team", Name: "created"}}, }, authorizer.checkCalls) authorizer.checkErr = errors.New("denied") + _, err = service.Create(ctx, &v1alpha3.SandboxTemplate{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "forbidden"}}) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + err = kubeClient.Get(ctx, types.NamespacedName{Namespace: "team", Name: "forbidden"}, &v1alpha3.SandboxTemplate{}) + require.True(t, apierrors.IsNotFound(err)) require.Error(t, service.Delete(ctx, types.NamespacedName{Namespace: "team", Name: "a"})) require.NoError(t, kubeClient.Get(ctx, types.NamespacedName{Namespace: "team", Name: "a"}, &v1alpha3.SandboxTemplate{})) authorizer.scope = apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf} diff --git a/go/core/internal/service/model/discovery.go b/go/core/internal/service/model/discovery.go index 34a5dca67d..32dcf6a5d7 100644 --- a/go/core/internal/service/model/discovery.go +++ b/go/core/internal/service/model/discovery.go @@ -6,13 +6,13 @@ import ( "reflect" "strings" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "sigs.k8s.io/controller-runtime/pkg/client" + "k8s.io/apimachinery/pkg/types" "github.com/kagent-dev/kagent/go/adk/pkg/models" "github.com/kagent-dev/kagent/go/api/v1alpha3" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" + "github.com/kagent-dev/kagent/go/core/pkg/auth" ) // ProviderModelRefresher refreshes the discovered models for one provider config. @@ -353,13 +353,13 @@ func (s *Service) ListSupportedModelProviders(context.Context) []ProviderDefinit } func (s *Service) ListConfiguredProviders(ctx context.Context) ([]ConfiguredProvider, error) { - var modelProviderConfigList v1alpha3.ModelProviderConfigList - if err := s.kubeClient.List(ctx, &modelProviderConfigList, client.InNamespace(s.defaultNamespace)); err != nil { - return nil, serviceerrors.NewInternal("Failed to list model provider configs", err) + providerConfigs, err := s.providerConfigs.List(ctx, s.defaultNamespace) + if err != nil { + return nil, err } - var providers []ConfiguredProvider - for _, providerConfig := range modelProviderConfigList.Items { + providers := make([]ConfiguredProvider, 0, len(providerConfigs)) + for _, providerConfig := range providerConfigs { if meta.IsStatusConditionTrue(providerConfig.Status.Conditions, v1alpha3.ModelProviderConfigConditionTypeReady) { providers = append(providers, ConfiguredProvider{ Name: providerConfig.Name, @@ -376,8 +376,17 @@ func (s *Service) GetProviderModels(ctx context.Context, request GetProviderMode return ProviderModelsResult{}, serviceerrors.NewInvalidArgument("Model provider name is required", nil) } + ref := types.NamespacedName{Namespace: s.defaultNamespace, Name: request.Name} var models []string if request.Refresh { + // The RPC also serves cached reads, so its transport read policy cannot + // reject this mutation. A share must never trigger provider discovery. + if _, shared := auth.ShareContextFrom(ctx); shared { + return ProviderModelsResult{}, serviceerrors.NewPermissionDenied("Session shares cannot refresh provider models", nil) + } + if err := s.providerConfigs.Authorize(ctx, auth.VerbUpdate, ref); err != nil { + return ProviderModelsResult{}, err + } if s.providerModelRefresher == nil { return ProviderModelsResult{}, serviceerrors.NewInternal( "Failed to refresh models for model provider", @@ -390,12 +399,9 @@ func (s *Service) GetProviderModels(ctx context.Context, request GetProviderMode } models = refreshedModels } else { - providerConfig := &v1alpha3.ModelProviderConfig{} - if err := s.kubeClient.Get(ctx, client.ObjectKey{Namespace: s.defaultNamespace, Name: request.Name}, providerConfig); err != nil { - if apierrors.IsNotFound(err) { - return ProviderModelsResult{}, serviceerrors.NewNotFound(err.Error(), err) - } - return ProviderModelsResult{}, serviceerrors.NewInternal("Failed to get model provider config", err) + providerConfig, err := s.providerConfigs.Get(ctx, ref) + if err != nil { + return ProviderModelsResult{}, err } if len(providerConfig.Status.DiscoveredModels) == 0 { return ProviderModelsResult{}, serviceerrors.NewNotFound("No models discovered for model provider, try refreshing", nil) diff --git a/go/core/internal/service/model/discovery_test.go b/go/core/internal/service/model/discovery_test.go index 4b884e728e..6a5f5d4b81 100644 --- a/go/core/internal/service/model/discovery_test.go +++ b/go/core/internal/service/model/discovery_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -14,9 +15,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/kagent-dev/kagent/go/adk/pkg/models" + apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" "github.com/kagent-dev/kagent/go/api/v1alpha3" + authimpl "github.com/kagent-dev/kagent/go/core/internal/httpserver/auth" "github.com/kagent-dev/kagent/go/core/internal/service/model" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" + pkgauth "github.com/kagent-dev/kagent/go/core/pkg/auth" ) type recordingProviderModelRefresher struct { @@ -139,9 +143,9 @@ func TestDiscoveryConfiguredProviders(t *testing.T) { Status: v1alpha3.ModelProviderConfigStatus{Conditions: readyCondition}, }, ).Build() - service := model.NewService(kubeClient, nil, "service-ns") + service := model.NewService(kubeClient, pkgauth.NoopAuthorizer{}, "service-ns") - providers, err := service.ListConfiguredProviders(context.Background()) + providers, err := service.ListConfiguredProviders(discoveryContext(t)) require.NoError(t, err) assert.ElementsMatch(t, []model.ConfiguredProvider{ { @@ -159,9 +163,9 @@ func TestDiscoveryConfiguredProviders(t *testing.T) { func TestDiscoveryConfiguredProviderListError(t *testing.T) { listErr := errors.New("list failed") - service := model.NewService(&listErrorClient{err: listErr}, nil, "service-ns") + service := model.NewService(&listErrorClient{err: listErr}, pkgauth.NoopAuthorizer{}, "service-ns") - _, err := service.ListConfiguredProviders(context.Background()) + _, err := service.ListConfiguredProviders(discoveryContext(t)) require.Error(t, err) assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodeInternal)) assert.ErrorIs(t, err, listErr) @@ -175,18 +179,18 @@ func TestDiscoveryProviderModels(t *testing.T) { DiscoveredModels: []string{"model-a", "model-b"}, }, }).Build() - service := model.NewService(kubeClient, nil, "service-ns") + service := model.NewService(kubeClient, pkgauth.NoopAuthorizer{}, "service-ns") - result, err := service.GetProviderModels(context.Background(), model.GetProviderModelsRequest{Name: "configured"}) + result, err := service.GetProviderModels(discoveryContext(t), model.GetProviderModelsRequest{Name: "configured"}) require.NoError(t, err) assert.Equal(t, model.ProviderModelsResult{Provider: "configured", Models: []string{"model-a", "model-b"}}, result) } func TestDiscoveryProviderModelsRefresh(t *testing.T) { refresher := &recordingProviderModelRefresher{models: []string{"fresh-model"}} - service := model.NewService(nil, nil, "service-ns", model.WithProviderModelRefresher(refresher)) + service := model.NewService(nil, pkgauth.NoopAuthorizer{}, "service-ns", model.WithProviderModelRefresher(refresher)) - result, err := service.GetProviderModels(context.Background(), model.GetProviderModelsRequest{Name: "configured", Refresh: true}) + result, err := service.GetProviderModels(discoveryContext(t), model.GetProviderModelsRequest{Name: "configured", Refresh: true}) require.NoError(t, err) assert.Equal(t, model.ProviderModelsResult{Provider: "configured", Models: []string{"fresh-model"}}, result) assert.Equal(t, 1, refresher.calls) @@ -199,22 +203,22 @@ func TestDiscoveryProviderModelsErrors(t *testing.T) { kubeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&v1alpha3.ModelProviderConfig{ ObjectMeta: metav1.ObjectMeta{Name: "empty", Namespace: "service-ns"}, }).Build() - service := model.NewService(kubeClient, nil, "service-ns") + service := model.NewService(kubeClient, pkgauth.NoopAuthorizer{}, "service-ns") t.Run("empty provider name", func(t *testing.T) { - _, err := service.GetProviderModels(context.Background(), model.GetProviderModelsRequest{}) + _, err := service.GetProviderModels(discoveryContext(t), model.GetProviderModelsRequest{}) require.Error(t, err) assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodeInvalidArgument)) }) t.Run("missing provider config", func(t *testing.T) { - _, err := service.GetProviderModels(context.Background(), model.GetProviderModelsRequest{Name: "missing"}) + _, err := service.GetProviderModels(discoveryContext(t), model.GetProviderModelsRequest{Name: "missing"}) require.Error(t, err) assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodeNotFound)) }) t.Run("empty discovered models", func(t *testing.T) { - _, err := service.GetProviderModels(context.Background(), model.GetProviderModelsRequest{Name: "empty"}) + _, err := service.GetProviderModels(discoveryContext(t), model.GetProviderModelsRequest{Name: "empty"}) require.Error(t, err) assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodeNotFound)) assert.Equal(t, "No models discovered for model provider, try refreshing", serviceerrors.MessageOf(err)) @@ -222,9 +226,9 @@ func TestDiscoveryProviderModelsErrors(t *testing.T) { t.Run("cached lookup error", func(t *testing.T) { getErr := errors.New("get failed") - getService := model.NewService(&getErrorClient{err: getErr}, nil, "service-ns") + getService := model.NewService(&getErrorClient{err: getErr}, pkgauth.NoopAuthorizer{}, "service-ns") - _, err := getService.GetProviderModels(context.Background(), model.GetProviderModelsRequest{Name: "configured"}) + _, err := getService.GetProviderModels(discoveryContext(t), model.GetProviderModelsRequest{Name: "configured"}) require.Error(t, err) assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodeInternal)) assert.ErrorIs(t, err, getErr) @@ -233,9 +237,9 @@ func TestDiscoveryProviderModelsErrors(t *testing.T) { t.Run("refresh error", func(t *testing.T) { refreshErr := errors.New("refresh failed") refresher := &recordingProviderModelRefresher{err: refreshErr} - refreshService := model.NewService(nil, nil, "service-ns", model.WithProviderModelRefresher(refresher)) + refreshService := model.NewService(nil, pkgauth.NoopAuthorizer{}, "service-ns", model.WithProviderModelRefresher(refresher)) - _, err := refreshService.GetProviderModels(context.Background(), model.GetProviderModelsRequest{Name: "configured", Refresh: true}) + _, err := refreshService.GetProviderModels(discoveryContext(t), model.GetProviderModelsRequest{Name: "configured", Refresh: true}) require.Error(t, err) assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodeInternal)) assert.ErrorIs(t, err, refreshErr) @@ -295,3 +299,62 @@ func TestOllamaCatalogCoversCloudAndLocal(t *testing.T) { "%q should route to the cloud when tagged", name) } } + +func discoveryContext(t *testing.T) context.Context { + t.Helper() + return pkgauth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{P: pkgauth.Principal{User: pkgauth.User{ID: "provider-user"}}}) +} + +func TestConfiguredProviderAuthorization(t *testing.T) { + ctx := discoveryContext(t) + ready := v1alpha3.ModelProviderConfigStatus{Conditions: []metav1.Condition{{Type: v1alpha3.ModelProviderConfigConditionTypeReady, Status: metav1.ConditionTrue}}, DiscoveredModels: []string{"cached"}} + kube := fake.NewClientBuilder().WithScheme(discoveryScheme(t)).WithObjects( + &v1alpha3.ModelProviderConfig{ObjectMeta: metav1.ObjectMeta{Namespace: "service-ns", Name: "allowed"}, Status: ready}, + &v1alpha3.ModelProviderConfig{ObjectMeta: metav1.ObjectMeta{Namespace: "service-ns", Name: "denied"}, Status: ready}, + &v1alpha3.ModelProviderConfig{ObjectMeta: metav1.ObjectMeta{Namespace: "service-ns", Name: "unready"}}, + ).Build() + authorizer := &recordingAuthorizer{scope: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf, AnyOf: []apiauthorization.ScopeClause{{All: []apiauthorization.ScopePredicate{{Attribute: apiauthorization.AttributeName, Operator: apiauthorization.ScopeIn, Values: []string{"allowed", "unready"}}}}}}} + refresher := &recordingProviderModelRefresher{models: []string{"fresh"}} + service := model.NewService(kube, authorizer, "service-ns", model.WithProviderModelRefresher(refresher)) + providers, err := service.ListConfiguredProviders(ctx) + require.NoError(t, err) + require.Len(t, providers, 1) + require.Equal(t, "allowed", providers[0].Name) + require.Equal(t, pkgauth.VerbList, authorizer.scopeVerb) + require.Equal(t, "ModelProviderConfig", authorizer.scopeType) + _, err = service.GetProviderModels(ctx, model.GetProviderModelsRequest{Name: "allowed"}) + require.NoError(t, err) + require.Equal(t, authorizationCall{verb: pkgauth.VerbGet, resource: pkgauth.Resource{Type: "ModelProviderConfig", Namespace: "service-ns", Name: "allowed"}}, authorizer.checkCalls[0]) + _, err = service.GetProviderModels(ctx, model.GetProviderModelsRequest{Name: "allowed", Refresh: true}) + require.NoError(t, err) + require.Equal(t, authorizationCall{verb: pkgauth.VerbUpdate, resource: pkgauth.Resource{Type: "ModelProviderConfig", Namespace: "service-ns", Name: "allowed"}}, authorizer.checkCalls[1]) + require.Equal(t, 1, refresher.calls) + + for _, refresh := range []bool{false, true} { + t.Run(fmt.Sprintf("denied refresh=%t", refresh), func(t *testing.T) { + denied := model.NewService(kube, denyAuthorizer{}, "service-ns", model.WithProviderModelRefresher(refresher)) + _, err := denied.GetProviderModels(ctx, model.GetProviderModelsRequest{Name: "denied", Refresh: refresh}) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.Equal(t, 1, refresher.calls) + _, err = denied.GetProviderModels(t.Context(), model.GetProviderModelsRequest{Name: "allowed", Refresh: refresh}) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeUnauthenticated)) + }) + } + shared := pkgauth.ShareContextTo(ctx, &pkgauth.ShareContext{UserID: "owner", SessionID: "session"}) + _, err = service.GetProviderModels(shared, model.GetProviderModelsRequest{Name: "allowed", Refresh: true}) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.Equal(t, 1, refresher.calls) + + denied := model.NewService(kube, denyAuthorizer{}, "service-ns") + providers, err = denied.ListConfiguredProviders(ctx) + require.NoError(t, err) + require.Empty(t, providers) + require.NotEmpty(t, denied.ListSupportedModelProviders(ctx)) + require.NotEmpty(t, denied.ListSupportedModels(ctx)) + unavailable := model.NewService(kube, unavailableAuthorizer{}, "service-ns") + _, err = unavailable.ListConfiguredProviders(ctx) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeUnavailable)) + authorizer.scope = apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf} + _, err = service.ListConfiguredProviders(ctx) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeInternal)) +} diff --git a/go/core/internal/service/model/service.go b/go/core/internal/service/model/service.go index 9b3dfec414..1f9da4771c 100644 --- a/go/core/internal/service/model/service.go +++ b/go/core/internal/service/model/service.go @@ -22,11 +22,15 @@ import ( var modelConfigGVK = v1alpha3.GroupVersion.WithKind("ModelConfig") // modelConfigResource names ModelConfig in authorization decisions. -const modelConfigResource = "ModelConfig" +const ( + modelConfigResource = "ModelConfig" + modelProviderConfigResource = "ModelProviderConfig" +) type Service struct { kubeClient client.Client modelConfigs *kubecrud.Service[*v1alpha3.ModelConfig, *v1alpha3.ModelConfigList] + providerConfigs *kubecrud.Service[*v1alpha3.ModelProviderConfig, *v1alpha3.ModelProviderConfigList] defaultNamespace string providerModelRefresher ProviderModelRefresher } @@ -59,6 +63,7 @@ func NewService(kubeClient client.Client, authorizer auth.CollectionAuthorizer, service := &Service{ kubeClient: kubeClient, modelConfigs: kubecrud.NewService(kubeClient, authorizer, &v1alpha3.ModelConfig{}, &v1alpha3.ModelConfigList{}, modelConfigResource), + providerConfigs: kubecrud.NewService(kubeClient, authorizer, &v1alpha3.ModelProviderConfig{}, &v1alpha3.ModelProviderConfigList{}, modelProviderConfigResource), defaultNamespace: defaultNamespace, } for _, option := range options { diff --git a/go/core/internal/service/prompttemplate/service.go b/go/core/internal/service/prompttemplate/service.go index 6630cc7216..ca0c94b41d 100644 --- a/go/core/internal/service/prompttemplate/service.go +++ b/go/core/internal/service/prompttemplate/service.go @@ -14,18 +14,20 @@ import ( utilvalidation "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/kagent-dev/kagent/go/core/internal/service/kubeauth" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" "github.com/kagent-dev/kagent/go/core/pkg/auth" ) const ( - promptLibraryLabelKey = "kagent.dev/prompt-library" - promptLibraryLabelVal = "true" + promptTemplateResource = "PromptTemplate" + promptLibraryLabelKey = "kagent.dev/prompt-library" + promptLibraryLabelVal = "true" ) type Service struct { kubeClient client.Client - authorizer auth.Authorizer + authorizer auth.CollectionAuthorizer } type Summary struct { @@ -47,18 +49,23 @@ type CreateRequest struct { Data map[string]string } -func NewService(kubeClient client.Client, authorizer auth.Authorizer) *Service { +func NewService(kubeClient client.Client, authorizer auth.CollectionAuthorizer) *Service { return &Service{kubeClient: kubeClient, authorizer: authorizer} } func (s *Service) List(ctx context.Context, namespace string) ([]Summary, error) { - if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: "PromptTemplate"}); err != nil { - return nil, err - } if namespace == "" { return nil, serviceerrors.NewInvalidArgument("namespace query parameter is required", nil) } + matcher, err := kubeauth.CollectionMatcher(ctx, s.authorizer, promptTemplateResource) + if err != nil { + return nil, err + } + + if matcher.Empty() { + return []Summary{}, nil + } list := &corev1.ConfigMapList{} if err := s.kubeClient.List( ctx, @@ -70,7 +77,9 @@ func (s *Service) List(ctx context.Context, namespace string) ([]Summary, error) result := make([]Summary, 0, len(list.Items)) for index := range list.Items { - result = append(result, summarize(&list.Items[index])) + if matcher.Matches(&list.Items[index]) { + result = append(result, summarize(&list.Items[index])) + } } slices.SortFunc(result, func(left, right Summary) int { return cmp.Compare(left.Name, right.Name) @@ -82,25 +91,22 @@ func (s *Service) Get(ctx context.Context, ref types.NamespacedName) (Detail, er if err := validateRef(ref); err != nil { return Detail{}, err } - if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: "PromptTemplate", Namespace: ref.Namespace, Name: ref.Name}); err != nil { + if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: promptTemplateResource, Namespace: ref.Namespace, Name: ref.Name}); err != nil { return Detail{}, err } - configMap := &corev1.ConfigMap{} - if err := s.kubeClient.Get(ctx, ref, configMap); err != nil { - if apierrors.IsNotFound(err) { - return Detail{}, serviceerrors.NewNotFound("ConfigMap not found", err) - } - return Detail{}, serviceerrors.NewInternal("Failed to get ConfigMap", err) + configMap, err := s.library(ctx, ref) + if err != nil { + return Detail{}, err } return detail(configMap), nil } func (s *Service) Create(ctx context.Context, request CreateRequest) (Detail, error) { - if err := s.authorize(ctx, auth.VerbCreate, auth.Resource{Type: "PromptTemplate"}); err != nil { + if err := validateCreateRequest(request); err != nil { return Detail{}, err } - if err := validateCreateRequest(request); err != nil { + if err := s.authorize(ctx, auth.VerbCreate, auth.Resource{Type: promptTemplateResource, Namespace: request.Namespace, Name: request.Name}); err != nil { return Detail{}, err } @@ -125,19 +131,16 @@ func (s *Service) Update(ctx context.Context, ref types.NamespacedName, data map if err := validateRef(ref); err != nil { return Detail{}, err } - if err := s.authorize(ctx, auth.VerbUpdate, auth.Resource{Type: "PromptTemplate", Namespace: ref.Namespace, Name: ref.Name}); err != nil { + if err := s.authorize(ctx, auth.VerbUpdate, auth.Resource{Type: promptTemplateResource, Namespace: ref.Namespace, Name: ref.Name}); err != nil { return Detail{}, err } if len(data) == 0 { return Detail{}, serviceerrors.NewInvalidArgument("at least one template key is required", nil) } - configMap := &corev1.ConfigMap{} - if err := s.kubeClient.Get(ctx, ref, configMap); err != nil { - if apierrors.IsNotFound(err) { - return Detail{}, serviceerrors.NewNotFound("ConfigMap not found", err) - } - return Detail{}, serviceerrors.NewInternal("Failed to get ConfigMap", err) + configMap, err := s.library(ctx, ref) + if err != nil { + return Detail{}, err } configMap.Data = cloneStringMap(data) if err := s.kubeClient.Update(ctx, configMap); err != nil { @@ -150,16 +153,13 @@ func (s *Service) Delete(ctx context.Context, ref types.NamespacedName) error { if err := validateRef(ref); err != nil { return err } - if err := s.authorize(ctx, auth.VerbDelete, auth.Resource{Type: "PromptTemplate", Namespace: ref.Namespace, Name: ref.Name}); err != nil { + if err := s.authorize(ctx, auth.VerbDelete, auth.Resource{Type: promptTemplateResource, Namespace: ref.Namespace, Name: ref.Name}); err != nil { return err } - configMap := &corev1.ConfigMap{} - if err := s.kubeClient.Get(ctx, ref, configMap); err != nil { - if apierrors.IsNotFound(err) { - return serviceerrors.NewNotFound("ConfigMap not found", err) - } - return serviceerrors.NewInternal("Failed to get ConfigMap", err) + configMap, err := s.library(ctx, ref) + if err != nil { + return err } if err := s.kubeClient.Delete(ctx, configMap); err != nil { return serviceerrors.NewInternal("Failed to delete ConfigMap", err) @@ -239,3 +239,19 @@ func validateCreateRequest(request CreateRequest) error { } return nil } + +// A prompt reference addresses a library, not an arbitrary ConfigMap. Direct +// reads and mutations retain the same boundary as label-filtered discovery. +func (s *Service) library(ctx context.Context, ref types.NamespacedName) (*corev1.ConfigMap, error) { + configMap := &corev1.ConfigMap{} + if err := s.kubeClient.Get(ctx, ref, configMap); err != nil { + if apierrors.IsNotFound(err) { + return nil, serviceerrors.NewNotFound("Prompt library not found", err) + } + return nil, serviceerrors.NewInternal("Failed to get prompt library", err) + } + if configMap.Labels[promptLibraryLabelKey] != promptLibraryLabelVal { + return nil, serviceerrors.NewNotFound("Prompt library not found", nil) + } + return configMap, nil +} diff --git a/go/core/internal/service/prompttemplate/service_test.go b/go/core/internal/service/prompttemplate/service_test.go index 423f1f74c2..5c6cacd462 100644 --- a/go/core/internal/service/prompttemplate/service_test.go +++ b/go/core/internal/service/prompttemplate/service_test.go @@ -15,6 +15,7 @@ import ( ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" authimpl "github.com/kagent-dev/kagent/go/core/internal/httpserver/auth" "github.com/kagent-dev/kagent/go/core/internal/service/prompttemplate" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" @@ -23,6 +24,10 @@ import ( type denyAuthorizer struct{} +func (denyAuthorizer) Scope(context.Context, pkgauth.Principal, pkgauth.Verb, string) (apiauthorization.AuthorizationScope, error) { + return apiauthorization.AuthorizationScope{}, errors.New("denied") +} + func (denyAuthorizer) Check(context.Context, pkgauth.Principal, pkgauth.Verb, pkgauth.Resource) error { return errors.New("denied") } @@ -30,7 +35,7 @@ func (denyAuthorizer) Check(context.Context, pkgauth.Principal, pkgauth.Verb, pk func TestServiceCRUDAndValidation(t *testing.T) { scheme := runtime.NewScheme() require.NoError(t, corev1.AddToScheme(scheme)) - newService := func(authorizer pkgauth.Authorizer, objects ...ctrlclient.Object) (*prompttemplate.Service, ctrlclient.Client, context.Context) { + newService := func(authorizer pkgauth.CollectionAuthorizer, objects ...ctrlclient.Object) (*prompttemplate.Service, ctrlclient.Client, context.Context) { kubeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() service := prompttemplate.NewService(kubeClient, authorizer) ctx := pkgauth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{ @@ -138,10 +143,87 @@ func TestServiceCRUDAndValidation(t *testing.T) { t.Run("authorization and authentication", func(t *testing.T) { service, _, ctx := newService(denyAuthorizer{}) _, err := service.List(ctx, "team") - assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied), err) + assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodeUnavailable), err) service, _, _ = newService(&pkgauth.NoopAuthorizer{}) _, err = service.List(context.Background(), "team") assert.True(t, serviceerrors.IsCode(err, serviceerrors.CodeUnauthenticated), err) }) } + +type libraryAuthorizer struct { + pkgauth.NoopAuthorizer + scope apiauthorization.AuthorizationScope + verb pkgauth.Verb + resource pkgauth.Resource + deny bool +} + +func (a *libraryAuthorizer) Check(_ context.Context, _ pkgauth.Principal, verb pkgauth.Verb, resource pkgauth.Resource) error { + a.verb, a.resource = verb, resource + if a.deny { + return errors.New("denied") + } + return nil +} + +func (a *libraryAuthorizer) Scope(_ context.Context, _ pkgauth.Principal, verb pkgauth.Verb, resourceType string) (apiauthorization.AuthorizationScope, error) { + a.verb, a.resource = verb, pkgauth.Resource{Type: resourceType} + return a.scope, nil +} + +func TestPromptLibraryScopeAndObjectBoundary(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + ref := types.NamespacedName{Namespace: "team", Name: "library"} + kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "library", Labels: map[string]string{"kagent.dev/prompt-library": "true", "keep": "label"}}, Data: map[string]string{"visible": "prompt"}, BinaryData: map[string][]byte{"asset": {1}}}, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "hidden", Labels: map[string]string{"kagent.dev/prompt-library": "true"}}, Data: map[string]string{"private": "secret"}}, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "ordinary"}, Data: map[string]string{"private": "secret"}}, + ).Build() + authorizer := &libraryAuthorizer{scope: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf, AnyOf: []apiauthorization.ScopeClause{{All: []apiauthorization.ScopePredicate{ + {Attribute: apiauthorization.AttributeNamespace, Operator: apiauthorization.ScopeIn, Values: []string{"team"}}, + {Attribute: apiauthorization.AttributeName, Operator: apiauthorization.ScopeIn, Values: []string{"library"}}, + }}}}} + service := prompttemplate.NewService(kube, authorizer) + ctx := pkgauth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{P: pkgauth.Principal{User: pkgauth.User{ID: "user"}}}) + listed, err := service.List(ctx, "team") + require.NoError(t, err) + require.Equal(t, []prompttemplate.Summary{{Namespace: "team", Name: "library", Keys: []string{"visible"}, KeyCount: 2}}, listed) + require.Equal(t, pkgauth.VerbList, authorizer.verb) + require.Equal(t, pkgauth.Resource{Type: "PromptTemplate"}, authorizer.resource) + authorizer.scope = apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone} + listed, err = service.List(ctx, "team") + require.NoError(t, err) + require.NotNil(t, listed) + require.Empty(t, listed) + + authorizer.deny = true + _, err = service.Create(ctx, prompttemplate.CreateRequest{Namespace: "team", Name: "denied", Data: map[string]string{"key": "value"}}) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.Equal(t, pkgauth.VerbCreate, authorizer.verb) + require.Equal(t, pkgauth.Resource{Type: "PromptTemplate", Namespace: "team", Name: "denied"}, authorizer.resource) + require.True(t, apierrors.IsNotFound(kube.Get(ctx, types.NamespacedName{Namespace: "team", Name: "denied"}, &corev1.ConfigMap{}))) + _, err = service.Update(ctx, ref, map[string]string{"changed": "no"}) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.True(t, serviceerrors.IsCode(service.Delete(ctx, ref), serviceerrors.CodePermissionDenied)) + stored := &corev1.ConfigMap{} + require.NoError(t, kube.Get(ctx, ref, stored)) + require.Equal(t, map[string]string{"visible": "prompt"}, stored.Data) + + authorizer.deny = false + ordinary := types.NamespacedName{Namespace: "team", Name: "ordinary"} + _, err = service.Get(ctx, ordinary) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeNotFound)) + _, err = service.Update(ctx, ordinary, map[string]string{"changed": "no"}) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeNotFound)) + require.True(t, serviceerrors.IsCode(service.Delete(ctx, ordinary), serviceerrors.CodeNotFound)) + require.NoError(t, kube.Get(ctx, ordinary, stored)) + require.Equal(t, map[string]string{"private": "secret"}, stored.Data) + _, err = service.Update(ctx, ref, map[string]string{"changed": "yes"}) + require.NoError(t, err) + require.NoError(t, kube.Get(ctx, ref, stored)) + require.Equal(t, "label", stored.Labels["keep"]) + require.Equal(t, "true", stored.Labels["kagent.dev/prompt-library"]) + require.Equal(t, []byte{1}, stored.BinaryData["asset"]) +} diff --git a/go/core/internal/service/sandbox/service_test.go b/go/core/internal/service/sandbox/service_test.go index 4f05122aed..746cd9357b 100644 --- a/go/core/internal/service/sandbox/service_test.go +++ b/go/core/internal/service/sandbox/service_test.go @@ -16,12 +16,14 @@ import ( "google.golang.org/protobuf/proto" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" apiv1 "github.com/kagent-dev/kagent/go/api/gen/kagent/api/v1" "github.com/kagent-dev/kagent/go/api/v1alpha3" "github.com/kagent-dev/kagent/go/core/internal/database" "github.com/kagent-dev/kagent/go/core/internal/dbtest" + "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" "github.com/kagent-dev/kagent/go/core/internal/substrate" "github.com/kagent-dev/kagent/go/core/pkg/auth" ) @@ -306,3 +308,57 @@ func TestSandboxResolvesCurrentGrantsBeforeResume(t *testing.T) { require.Less(t, slices.Index(calls, "policy"), slices.Index(calls, "resume")) require.Contains(t, calls, "policy") } + +type templateDenyingAuthorizer struct { + templates []auth.Resource + verbs []auth.Verb +} + +var _ auth.Authorizer = (*templateDenyingAuthorizer)(nil) + +func (a *templateDenyingAuthorizer) Check(_ context.Context, _ auth.Principal, verb auth.Verb, resource auth.Resource) error { + if resource.Type == v1alpha3.SandboxTemplateKind { + a.templates = append(a.templates, resource) + a.verbs = append(a.verbs, verb) + return errors.New("template denied") + } + return nil +} + +func TestSandboxCreateRejectsDeniedTemplateBeforePersistence(t *testing.T) { + for _, test := range []struct { + name string + objects []client.Object + }{ + {name: "existing template", objects: []client.Object{&v1alpha3.SandboxTemplate{ + ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "scratch"}, + }}}, + {name: "missing template on retry"}, + } { + t.Run(test.name, func(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, v1alpha3.AddToScheme(scheme)) + authorizer := &templateDenyingAuthorizer{} + // Missing store and actor dependencies make any persistence or runtime + // work fail the test. Denial must precede both, even when a repeated + // request names a template that no longer exists in Kubernetes. + service := &Service{config: Config{ + Kube: fake.NewClientBuilder().WithScheme(scheme).WithObjects(test.objects...).Build(), + Authorizer: authorizer, + }} + ctx := auth.AuthSessionTo(t.Context(), testSession("alice")) + request := createRequest() + for range 2 { + instance, err := service.Create(ctx, request) + require.Nil(t, instance) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.Equal(t, "SandboxTemplate access denied", serviceerrors.MessageOf(err)) + } + require.Equal(t, []auth.Resource{ + {Type: v1alpha3.SandboxTemplateKind, Namespace: "team-a", Name: "scratch"}, + {Type: v1alpha3.SandboxTemplateKind, Namespace: "team-a", Name: "scratch"}, + }, authorizer.templates) + require.Equal(t, []auth.Verb{auth.VerbGet, auth.VerbGet}, authorizer.verbs) + }) + } +} diff --git a/go/core/internal/service/system/service.go b/go/core/internal/service/system/service.go index d590f1cbde..38fb882341 100644 --- a/go/core/internal/service/system/service.go +++ b/go/core/internal/service/system/service.go @@ -34,7 +34,7 @@ type ATEClient interface { type Service struct { kubeClient client.Client observedNamespaces []string - authorizer auth.Authorizer + authorizer auth.CollectionAuthorizer ateClient ATEClient } @@ -46,7 +46,7 @@ type Namespace struct { func NewService( kubeClient client.Client, observedNamespaces []string, - authorizer auth.Authorizer, + authorizer auth.CollectionAuthorizer, ateClient ATEClient, ) *Service { return &Service{ diff --git a/go/core/internal/service/system/service_test.go b/go/core/internal/service/system/service_test.go index 2b033acb72..8dfa030575 100644 --- a/go/core/internal/service/system/service_test.go +++ b/go/core/internal/service/system/service_test.go @@ -21,6 +21,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" apiv1 "github.com/kagent-dev/kagent/go/api/gen/kagent/api/v1" authimpl "github.com/kagent-dev/kagent/go/core/internal/httpserver/auth" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" @@ -30,15 +31,20 @@ import ( type systemDenyAuthorizer struct{} +func (systemDenyAuthorizer) Scope(context.Context, pkgAuth.Principal, pkgAuth.Verb, string) (apiauthorization.AuthorizationScope, error) { + return apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}, nil +} + func (systemDenyAuthorizer) Check(context.Context, pkgAuth.Principal, pkgAuth.Verb, pkgAuth.Resource) error { return errors.New("denied") } type fakeATEClient struct { - templates []*ateapipb.ActorTemplate - actors []*ateapipb.Actor - workers []*ateapipb.Worker - err error + templates []*ateapipb.ActorTemplate + templateReads int + actors []*ateapipb.Actor + workers []*ateapipb.Worker + err error // The number of rows a page answers with, whatever the caller asked for. Zero // means "everything in one page". ate-api may answer with fewer rows than asked // for, so a fake that always fills the request would hide a caller that stops @@ -51,6 +57,7 @@ type fakeATEClient struct { } func (client *fakeATEClient) ListActorTemplates(_ context.Context, atespace string) ([]*ateapipb.ActorTemplate, error) { + client.templateReads++ templates := []*ateapipb.ActorTemplate{} for _, template := range client.templates { if atespace == "" || template.GetMetadata().GetAtespace() == atespace { @@ -604,3 +611,103 @@ func TestSubstrateEmptyPagesPreserveUpstreamTokens(t *testing.T) { assert.Empty(t, workers.ATEAPIError) assert.Equal(t, "upstream:next/cursor=", workers.NextPageToken) } + +type summaryAuthorizer struct { + pkgAuth.NoopAuthorizer + pools apiauthorization.AuthorizationScope + inventory apiauthorization.AuthorizationScope + failType string + calls []string +} + +func (a *summaryAuthorizer) Scope(_ context.Context, _ pkgAuth.Principal, verb pkgAuth.Verb, resource string) (apiauthorization.AuthorizationScope, error) { + a.calls = append(a.calls, string(verb)+" "+resource) + if resource == a.failType { + return apiauthorization.AuthorizationScope{}, errors.New("policy unavailable") + } + if resource == "WorkerPool" { + return a.pools, nil + } + return a.inventory, nil +} + +func TestSummaryAuthorizesPoolAndInventorySectionsIndependently(t *testing.T) { + all := apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAll} + none := apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone} + constrained := apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf, AnyOf: []apiauthorization.ScopeClause{{All: []apiauthorization.ScopePredicate{ + {Attribute: apiauthorization.AttributeNamespace, Operator: apiauthorization.ScopeIn, Values: []string{"team"}}, + {Attribute: apiauthorization.AttributeName, Operator: apiauthorization.ScopeIn, Values: []string{"allowed"}}, + }}}} + for _, test := range []struct { + name string + pools apiauthorization.AuthorizationScope + inventory apiauthorization.AuthorizationScope + observed []string + requested string + failType string + wantPools int + wantATE bool + code serviceerrors.Code + }{ + {name: "pool only", pools: constrained, inventory: none, wantPools: 1}, + {name: "all pools", pools: all, inventory: none, wantPools: 3}, + {name: "inventory only", pools: none, inventory: all, wantATE: true}, + {name: "neither", pools: none, inventory: none}, + {name: "configured intersection", pools: all, inventory: none, observed: []string{"team"}, wantPools: 2}, + {name: "request cannot escape configuration", pools: all, inventory: none, observed: []string{"team"}, requested: "other"}, + {name: "request cannot escape policy", pools: constrained, inventory: none, requested: "other"}, + {name: "pool lookup failure", failType: "WorkerPool", inventory: all, code: serviceerrors.CodeUnavailable}, + {name: "inventory lookup failure", pools: all, failType: "Substrate", code: serviceerrors.CodeUnavailable}, + {name: "invalid pool scope", pools: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf}, inventory: all, code: serviceerrors.CodeInternal}, + {name: "unsupported inventory scope", pools: all, inventory: constrained, code: serviceerrors.CodeInternal}, + {name: "invalid inventory scope", pools: all, code: serviceerrors.CodeInternal}, + } { + t.Run(test.name, func(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, atev1alpha1.AddToScheme(scheme)) + kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + &atev1alpha1.WorkerPool{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "allowed"}}, + &atev1alpha1.WorkerPool{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "hidden"}}, + &atev1alpha1.WorkerPool{ObjectMeta: metav1.ObjectMeta{Namespace: "other", Name: "allowed"}}, + ).Build() + authorizer := &summaryAuthorizer{pools: test.pools, inventory: test.inventory, failType: test.failType} + ate := &fakeATEClient{actors: []*ateapipb.Actor{substrateActor("actor", "team", ateapipb.ActorState_ACTOR_STATE_RUNNING, "team", "worker")}} + service := system.NewService(kube, test.observed, authorizer, ate) + ctx := pkgAuth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{P: pkgAuth.Principal{User: pkgAuth.User{ID: "reader"}}}) + result, err := service.GetSubstrateSummary(ctx, test.requested, "team") + if test.code != "" { + require.True(t, serviceerrors.IsCode(err, test.code)) + } else { + require.NoError(t, err) + require.Len(t, result.WorkerPools, test.wantPools) + require.Equal(t, []string{"list WorkerPool", "get Substrate"}, authorizer.calls) + if test.wantATE { + require.EqualValues(t, 1, result.ActorCount) + } else { + require.Zero(t, result.ActorCount) + require.Empty(t, result.ActorTemplates) + } + } + if test.wantATE { + require.Positive(t, ate.actorReads) + require.Positive(t, ate.workerReads) + require.Positive(t, ate.templateReads) + } else { + require.Zero(t, ate.actorReads) + require.Zero(t, ate.workerReads) + require.Zero(t, ate.templateReads) + } + }) + } +} + +func TestSummaryDeniedPoolsDoNotRequireKubernetes(t *testing.T) { + authorizer := &summaryAuthorizer{pools: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}, inventory: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAll}} + service := system.NewService(nil, nil, authorizer, &fakeATEClient{}) + ctx := pkgAuth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{P: pkgAuth.Principal{User: pkgAuth.User{ID: "reader"}}}) + result, err := service.GetSubstrateSummary(ctx, "", "") + require.NoError(t, err) + require.Empty(t, result.WorkerPools) + _, err = service.GetSubstrateSummary(t.Context(), "", "") + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeUnauthenticated)) +} diff --git a/go/core/internal/service/system/substrate.go b/go/core/internal/service/system/substrate.go index 19e3b87061..b672cef9c1 100644 --- a/go/core/internal/service/system/substrate.go +++ b/go/core/internal/service/system/substrate.go @@ -11,7 +11,9 @@ import ( atev1alpha1 "go.ate.dev/substrate/pkg/api/v1alpha1" "go.ate.dev/substrate/pkg/proto/ateapipb" + apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" apiv1 "github.com/kagent-dev/kagent/go/api/gen/kagent/api/v1" + "github.com/kagent-dev/kagent/go/core/internal/service/kubeauth" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" "github.com/kagent-dev/kagent/go/core/internal/substrate" "github.com/kagent-dev/kagent/go/core/pkg/auth" @@ -21,6 +23,11 @@ import ( // How many rows a list call asks ate-api for when the caller names no page size. const defaultSubstratePageSize int32 = 50 +const ( + workerPoolResource = "WorkerPool" + substrateResource = "Substrate" +) + // How many ate-api pages a walk will read before giving up. Each page carries its own // timeout, so nothing else bounds the loop against a cyclic next_page_token. At // ate-api's 1,000 rows a page this allows ten million. @@ -76,10 +83,18 @@ type SubstrateActorStatusCount struct { // ate-api reports no totals, so every count costs a walk of its pages. The walk holds // one page at a time and keeps only tallies. func (s *Service) GetSubstrateSummary(ctx context.Context, requestedNamespace, atespace string) (SubstrateSummary, error) { - namespaces, err := s.substrateScope(ctx, requestedNamespace) + poolMatcher, err := kubeauth.CollectionMatcher(ctx, s.authorizer, workerPoolResource) + if err != nil { + return SubstrateSummary{}, err + } + inventoryAllowed, err := s.summaryInventoryAllowed(ctx) if err != nil { return SubstrateSummary{}, err } + // Pool configuration and ATE inventory are separate authorization domains. + // Keep the legacy ATE namespace behavior until that inventory has its own + // object contract; it cannot widen the Kubernetes pool read boundary. + namespaces := s.substrateNamespaces(requestedNamespace) result := SubstrateSummary{ WorkerPools: []atev1alpha1.WorkerPool{}, @@ -88,17 +103,29 @@ func (s *Service) GetSubstrateSummary(ctx context.Context, requestedNamespace, a ComputedAt: time.Now().UTC(), } - for _, namespace := range namespaces { + poolNamespaces := s.workerPoolNamespaces(requestedNamespace) + if poolMatcher.Empty() { + poolNamespaces = nil + } + for _, namespace := range poolNamespaces { workerPools, err := s.listWorkerPools(ctx, namespace) if err != nil { return SubstrateSummary{}, serviceerrors.NewInternal("Failed to list substrate resources from Kubernetes", err) } - result.WorkerPools = append(result.WorkerPools, workerPools...) + for index := range workerPools { + if poolMatcher.Matches(&workerPools[index]) { + result.WorkerPools = append(result.WorkerPools, workerPools[index]) + } + } } slices.SortStableFunc(result.WorkerPools, func(left, right atev1alpha1.WorkerPool) int { return strings.Compare(left.Namespace+"/"+left.Name, right.Namespace+"/"+right.Name) }) + if !inventoryAllowed { + return result, nil + } + allowAll, allowed := substrateScopeFilter(namespaces) // Three independent reads: none gates the others, so one failure leaves the rest @@ -147,7 +174,7 @@ func (s *Service) GetSubstrateSummary(ctx context.Context, requestedNamespace, a // ListSubstrateActors returns one upstream page without changing its order or token. func (s *Service) ListSubstrateActors(ctx context.Context, input *apiv1.ListSubstrateActorsRequest) (SubstrateActorPage, error) { - if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: "Substrate"}); err != nil { + if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: substrateResource}); err != nil { return SubstrateActorPage{}, err } result := SubstrateActorPage{ComputedAt: time.Now().UTC()} @@ -228,7 +255,7 @@ func walkSubstrate[Row any]( // substrateScope authorizes the caller and resolves the namespaces a read covers. // ATE-only actor reads authorize independently of Kubernetes scope. func (s *Service) substrateScope(ctx context.Context, requestedNamespace string) ([]string, error) { - if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: "Substrate"}); err != nil { + if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: substrateResource}); err != nil { return nil, err } return s.substrateNamespaces(requestedNamespace), nil @@ -263,3 +290,35 @@ func substratePageSize(requested int32) int32 { } return requested } + +// The mixed summary must distinguish a denied section from an evaluator failure. +// Substrate remains a whole-inventory capability here: accepting object clauses +// would claim isolation that these ATE reads cannot yet enforce. +func (s *Service) summaryInventoryAllowed(ctx context.Context) (bool, error) { + principal, err := authenticatedPrincipal(ctx) + if err != nil { + return false, err + } + scope, err := s.authorizer.Scope(ctx, principal, auth.VerbGet, substrateResource) + if err != nil { + return false, serviceerrors.NewUnavailable("Failed to read the Substrate authorization scope", err) + } + if len(scope.AnyOf) != 0 { + return false, serviceerrors.NewInternal("Substrate inventory does not support object scopes", nil) + } + switch scope.Kind { + case apiauthorization.ScopeAll: + return true, nil + case apiauthorization.ScopeNone: + return false, nil + default: + return false, serviceerrors.NewInternal("Invalid Substrate authorization scope", nil) + } +} + +func (s *Service) workerPoolNamespaces(requested string) []string { + if requested != "" && len(s.observedNamespaces) != 0 && !slices.Contains(s.observedNamespaces, requested) { + return nil + } + return s.substrateNamespaces(requested) +} diff --git a/go/core/internal/service/tool/service.go b/go/core/internal/service/tool/service.go index e4dfc6bfc4..b9fbae33fd 100644 --- a/go/core/internal/service/tool/service.go +++ b/go/core/internal/service/tool/service.go @@ -9,12 +9,15 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kagent-dev/kagent/go/api/v1alpha3" "github.com/kagent-dev/kagent/go/core/internal/database" + "github.com/kagent-dev/kagent/go/core/internal/service/kubeauth" "github.com/kagent-dev/kagent/go/core/internal/service/secretmaterial" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" "github.com/kagent-dev/kagent/go/core/internal/utils" @@ -24,9 +27,9 @@ import ( type ServerType string -const ( - ServerTypeRemoteMCPServer ServerType = "RemoteMCPServer" -) +const ServerTypeRemoteMCPServer ServerType = "RemoteMCPServer" + +const toolServerResource = "ToolServer" var remoteMCPServerGVK = v1alpha3.GroupVersion.WithKind(string(ServerTypeRemoteMCPServer)) @@ -44,7 +47,7 @@ type MCPClient interface { type Service struct { kubeClient client.Client discoveryStore DiscoveryStore - authorizer auth.Authorizer + authorizer auth.CollectionAuthorizer defaultNamespace string mcpClient MCPClient } @@ -77,7 +80,7 @@ type MCPAppTool struct { func NewService( kubeClient client.Client, discoveryStore DiscoveryStore, - authorizer auth.Authorizer, + authorizer auth.CollectionAuthorizer, defaultNamespace string, mcpClient MCPClient, ) *Service { @@ -94,23 +97,34 @@ func NewService( } func (s *Service) ListTools(ctx context.Context) ([]database.Tool, error) { - if err := requireSession(ctx); err != nil { + servers, err := s.visibleServers(ctx) + if err != nil { return nil, err } + if len(servers) == 0 { + return []database.Tool{}, nil + } tools, err := s.discoveryStore.ListTools(ctx) if err != nil { return nil, serviceerrors.NewInternal("Failed to list tools", err) } - return tools, nil + visible := make(map[[2]string]struct{}, len(servers)) + for _, server := range servers { + visible[[2]string{server.Name, server.GroupKind}] = struct{}{} + } + result := make([]database.Tool, 0, len(tools)) + for _, tool := range tools { + if _, ok := visible[[2]string{tool.ServerName, tool.GroupKind}]; ok { + result = append(result, tool) + } + } + return result, nil } func (s *Service) ListToolServers(ctx context.Context) ([]ToolServer, error) { - if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: "ToolServer"}); err != nil { - return nil, err - } - servers, err := s.discoveryStore.ListToolServers(ctx) + servers, err := s.visibleServers(ctx) if err != nil { - return nil, serviceerrors.NewInternal("Failed to list ToolServers from database", err) + return nil, err } if len(servers) == 0 { return []ToolServer{}, nil @@ -135,6 +149,42 @@ func (s *Service) ListToolServers(ctx context.Context) ([]ToolServer, error) { return result, nil } +// Both discovery APIs authorize the owning server before returning tool metadata. +// A tool's name alone cannot establish its parent: kind is part of the join even +// though a policy grant intentionally covers kinds at the same namespace/name. +func (s *Service) visibleServers(ctx context.Context) ([]database.ToolServer, error) { + matcher, err := kubeauth.CollectionMatcher(ctx, s.authorizer, toolServerResource) + if err != nil { + return nil, err + } + if matcher.Empty() { + return []database.ToolServer{}, nil + } + servers, err := s.discoveryStore.ListToolServers(ctx) + if err != nil { + return nil, serviceerrors.NewInternal("Failed to list ToolServers from database", err) + } + result := make([]database.ToolServer, 0, len(servers)) + for _, server := range servers { + namespace, name, ok := strings.Cut(server.Name, "/") + if !ok || len(utilvalidation.IsDNS1123Label(namespace)) != 0 || len(utilvalidation.IsDNS1123Subdomain(name)) != 0 { + continue + } + if !supportedDiscoveryKind(server.GroupKind) { + continue + } + identity := &metav1.ObjectMeta{Namespace: namespace, Name: name} + if matcher.Matches(identity) { + result = append(result, server) + } + } + return result, nil +} + +func supportedDiscoveryKind(groupKind string) bool { + return groupKind == remoteMCPServerGVK.GroupKind().String() || groupKind == "Service" +} + func (s *Service) CreateToolServer(ctx context.Context, request CreateToolServerRequest) (client.Object, error) { supportedTypes := s.supportedServerTypes() if !slices.Contains(supportedTypes, request.Type) { @@ -159,7 +209,7 @@ func (s *Service) CreateToolServer(ctx context.Context, request CreateToolServer if err != nil { return nil, serviceerrors.NewInvalidArgument("Invalid ToolServer metadata", err) } - if err := s.authorize(ctx, auth.VerbCreate, auth.Resource{Type: "ToolServer", Namespace: ref.Namespace, Name: ref.Name}); err != nil { + if err := s.authorize(ctx, auth.VerbCreate, auth.Resource{Type: toolServerResource, Namespace: ref.Namespace, Name: ref.Name}); err != nil { return nil, err } if err := secretmaterial.ValidateMaterials(request.Secrets); err != nil { @@ -185,7 +235,7 @@ func (s *Service) DeleteToolServer(ctx context.Context, ref types.NamespacedName if ref.Namespace == "" || ref.Name == "" { return serviceerrors.NewInvalidArgument("ToolServer namespace and name are required", nil) } - if err := s.authorize(ctx, auth.VerbDelete, auth.Resource{Type: "ToolServer", Namespace: ref.Namespace, Name: ref.Name}); err != nil { + if err := s.authorize(ctx, auth.VerbDelete, auth.Resource{Type: toolServerResource, Namespace: ref.Namespace, Name: ref.Name}); err != nil { return err } @@ -195,10 +245,13 @@ func (s *Service) DeleteToolServer(ctx context.Context, ref types.NamespacedName } groupKind := "" for _, server := range servers { - if server.Name == ref.String() { - groupKind = server.GroupKind - break + if server.Name != ref.String() || !supportedDiscoveryKind(server.GroupKind) { + continue } + if groupKind != "" && groupKind != server.GroupKind { + return serviceerrors.NewFailedPrecondition("ToolServer reference matches multiple resource kinds", nil) + } + groupKind = server.GroupKind } if groupKind == "" { return serviceerrors.NewNotFound("ToolServer not found", nil) @@ -226,7 +279,7 @@ func (s *Service) DeleteToolServer(ctx context.Context, ref types.NamespacedName } func (s *Service) ListToolServerTypes(ctx context.Context) ([]ServerType, error) { - if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: "ToolServerType"}); err != nil { + if err := requireSession(ctx); err != nil { return nil, err } return s.supportedServerTypes(), nil @@ -291,7 +344,7 @@ func (s *Service) authorizeMCP(ctx context.Context, verb auth.Verb, ref MCPServe if ref.Ref.Namespace == "" || ref.Ref.Name == "" { return serviceerrors.NewInvalidArgument("ToolServer namespace and name are required", nil) } - return s.authorize(ctx, verb, auth.Resource{Type: "ToolServer", Namespace: ref.Ref.Namespace, Name: ref.Ref.Name}) + return s.authorize(ctx, verb, auth.Resource{Type: toolServerResource, Namespace: ref.Ref.Namespace, Name: ref.Ref.Name}) } func (s *Service) authorize(ctx context.Context, verb auth.Verb, resource auth.Resource) error { diff --git a/go/core/internal/service/tool/service_test.go b/go/core/internal/service/tool/service_test.go index 73843ea79c..844c3d132c 100644 --- a/go/core/internal/service/tool/service_test.go +++ b/go/core/internal/service/tool/service_test.go @@ -16,6 +16,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" "github.com/kagent-dev/kagent/go/api/v1alpha3" "github.com/kagent-dev/kagent/go/core/internal/database" authimpl "github.com/kagent-dev/kagent/go/core/internal/httpserver/auth" @@ -72,6 +73,8 @@ func (f *fakeMCPClient) ReadResource(_ context.Context, ref MCPServerRef, uri st } type recordingAuthorizer struct { + scope *apiauthorization.AuthorizationScope + scopeErr error err error verb pkgauth.Verb resource pkgauth.Resource @@ -83,6 +86,17 @@ func (a *recordingAuthorizer) Check(_ context.Context, _ pkgauth.Principal, verb return a.err } +func (a *recordingAuthorizer) Scope(_ context.Context, _ pkgauth.Principal, verb pkgauth.Verb, resourceType string) (apiauthorization.AuthorizationScope, error) { + a.verb, a.resource = verb, pkgauth.Resource{Type: resourceType} + if a.scopeErr != nil { + return apiauthorization.AuthorizationScope{}, a.scopeErr + } + if a.scope != nil { + return *a.scope, nil + } + return apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAll}, nil +} + func TestServiceDiscoveryAndAuthorization(t *testing.T) { store := &fakeDiscoveryStore{ tools: []database.Tool{{ID: "server-tool", ServerName: "default/server", GroupKind: "RemoteMCPServer.api.kagent.dev", Description: "server"}}, @@ -106,7 +120,7 @@ func TestServiceDiscoveryAndAuthorization(t *testing.T) { require.NoError(t, err) require.Len(t, servers, 1) assert.Equal(t, "server-tool", servers[0].DiscoveredTools[0].Name) - assert.Equal(t, pkgauth.VerbGet, authorizer.verb) + assert.Equal(t, pkgauth.VerbList, authorizer.verb) assert.Equal(t, pkgauth.Resource{Type: "ToolServer"}, authorizer.resource) types, err := service.ListToolServerTypes(ctx) @@ -264,24 +278,24 @@ func toolTestKube(t *testing.T, objects ...client.Object) client.Client { func TestListToolServersGroupsByExactIdentity(t *testing.T) { store := &fakeDiscoveryStore{ servers: []database.ToolServer{ - {Name: "shared", GroupKind: "remote"}, - {Name: "shared", GroupKind: "local"}, - {Name: "empty", GroupKind: "remote"}, + {Name: "default/shared", GroupKind: "RemoteMCPServer.api.kagent.dev"}, + {Name: "default/shared", GroupKind: "Service"}, + {Name: "default/empty", GroupKind: "RemoteMCPServer.api.kagent.dev"}, }, tools: []database.Tool{ - {ID: "first", ServerName: "shared", GroupKind: "remote", Description: "first description"}, - {ID: "local", ServerName: "shared", GroupKind: "local"}, - {ID: "second", ServerName: "shared", GroupKind: "remote"}, - {ID: "orphan", ServerName: "absent", GroupKind: "remote"}, + {ID: "first", ServerName: "default/shared", GroupKind: "RemoteMCPServer.api.kagent.dev", Description: "first description"}, + {ID: "local", ServerName: "default/shared", GroupKind: "Service"}, + {ID: "second", ServerName: "default/shared", GroupKind: "RemoteMCPServer.api.kagent.dev"}, + {ID: "orphan", ServerName: "default/absent", GroupKind: "RemoteMCPServer.api.kagent.dev"}, }, } service := NewService(toolTestKube(t), store, &recordingAuthorizer{}, "default", &fakeMCPClient{}) servers, err := service.ListToolServers(toolTestContext()) require.NoError(t, err) require.Equal(t, []ToolServer{ - {Ref: "shared", GroupKind: "remote", DiscoveredTools: []*v1alpha3.MCPTool{{Name: "first", Description: "first description"}, {Name: "second"}}}, - {Ref: "shared", GroupKind: "local", DiscoveredTools: []*v1alpha3.MCPTool{{Name: "local"}}}, - {Ref: "empty", GroupKind: "remote", DiscoveredTools: []*v1alpha3.MCPTool{}}, + {Ref: "default/shared", GroupKind: "RemoteMCPServer.api.kagent.dev", DiscoveredTools: []*v1alpha3.MCPTool{{Name: "first", Description: "first description"}, {Name: "second"}}}, + {Ref: "default/shared", GroupKind: "Service", DiscoveredTools: []*v1alpha3.MCPTool{{Name: "local"}}}, + {Ref: "default/empty", GroupKind: "RemoteMCPServer.api.kagent.dev", DiscoveredTools: []*v1alpha3.MCPTool{}}, }, servers) } @@ -291,7 +305,98 @@ func TestListToolServersSkipsToolReadWithoutServers(t *testing.T) { servers, err := service.ListToolServers(toolTestContext()) require.NoError(t, err) require.Equal(t, []ToolServer{}, servers) - store.servers = []database.ToolServer{{Name: "server", GroupKind: "kind"}} + store.servers = []database.ToolServer{{Name: "default/server", GroupKind: "RemoteMCPServer.api.kagent.dev"}} _, err = service.ListToolServers(toolTestContext()) require.ErrorIs(t, err, store.toolsErr) } + +func TestDiscoveryScopesBothServerAndFlatToolLists(t *testing.T) { + remote := remoteMCPServerGVK.GroupKind().String() + servers := []database.ToolServer{ + {Name: "team/allowed", GroupKind: remote}, + {Name: "team/allowed", GroupKind: "Service"}, + {Name: "team/denied", GroupKind: remote}, + {Name: "other/allowed", GroupKind: remote}, + {Name: "unqualified", GroupKind: remote}, + {Name: "team/extra/slash", GroupKind: remote}, + {Name: "team/allowed", GroupKind: "Unknown"}, + } + store := &fakeDiscoveryStore{servers: servers} + for i, server := range servers { + store.tools = append(store.tools, database.Tool{ID: string(rune('a' + i)), ServerName: server.Name, GroupKind: server.GroupKind, Description: "private description"}) + } + store.tools = append(store.tools, database.Tool{ID: "orphan", ServerName: "team/missing", GroupKind: remote}) + scope := apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf, AnyOf: []apiauthorization.ScopeClause{{All: []apiauthorization.ScopePredicate{ + {Attribute: apiauthorization.AttributeNamespace, Operator: apiauthorization.ScopeIn, Values: []string{"team"}}, + {Attribute: apiauthorization.AttributeName, Operator: apiauthorization.ScopeIn, Values: []string{"allowed"}}, + }}}} + authorizer := &recordingAuthorizer{scope: &scope} + service := NewService(toolTestKube(t), store, authorizer, "default", &fakeMCPClient{}) + listed, err := service.ListToolServers(toolTestContext()) + require.NoError(t, err) + require.Len(t, listed, 2) + require.Equal(t, "a", listed[0].DiscoveredTools[0].Name) + require.Equal(t, "b", listed[1].DiscoveredTools[0].Name) + flat, err := service.ListTools(toolTestContext()) + require.NoError(t, err) + require.Equal(t, store.tools[:2], flat) + require.Equal(t, pkgauth.VerbList, authorizer.verb) + require.Equal(t, pkgauth.Resource{Type: "ToolServer"}, authorizer.resource) + + scope.Kind, scope.AnyOf = apiauthorization.ScopeNone, nil + store.err = errors.New("denied collection must not be read") + listed, err = service.ListToolServers(toolTestContext()) + require.NoError(t, err) + require.NotNil(t, listed) + require.Empty(t, listed) + flat, err = service.ListTools(toolTestContext()) + require.NoError(t, err) + require.NotNil(t, flat) + require.Empty(t, flat) + scope.Kind = apiauthorization.ScopeAnyOf + _, err = service.ListToolServers(toolTestContext()) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeInternal)) + authorizer.scopeErr = errors.New("policy service unavailable") + _, err = service.ListTools(toolTestContext()) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeUnavailable)) + // Capability definitions reveal no configured object and remain available. + _, err = service.ListToolServerTypes(toolTestContext()) + require.NoError(t, err) + _, err = service.ListToolServerTypes(t.Context()) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeUnauthenticated)) +} + +func TestAmbiguousServerDeletionPreservesBothObjects(t *testing.T) { + ref := types.NamespacedName{Namespace: "team", Name: "shared"} + kube := toolTestKube(t, + &v1alpha3.RemoteMCPServer{ObjectMeta: metav1.ObjectMeta{Namespace: ref.Namespace, Name: ref.Name}}, + &corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: ref.Namespace, Name: ref.Name}}, + ) + store := &fakeDiscoveryStore{servers: []database.ToolServer{ + {Name: ref.String(), GroupKind: remoteMCPServerGVK.GroupKind().String()}, + {Name: ref.String(), GroupKind: "Service"}, + }} + service := NewService(kube, store, &recordingAuthorizer{}, "default", &fakeMCPClient{}) + err := service.DeleteToolServer(toolTestContext(), ref) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodeFailedPrecondition)) + require.NoError(t, kube.Get(t.Context(), ref, &v1alpha3.RemoteMCPServer{})) + require.NoError(t, kube.Get(t.Context(), ref, &corev1.Service{})) +} + +func TestDeniedMCPAppOperationsDoNotReachClient(t *testing.T) { + ref := MCPServerRef{Ref: types.NamespacedName{Namespace: "team", Name: "server"}, GroupKind: remoteMCPServerGVK.GroupKind().String()} + authorizer := &recordingAuthorizer{err: errors.New("denied")} + // A nil runtime client is a trap: a denied call must return PermissionDenied, + // never the missing-client precondition or an outbound resolution error. + service := NewService(nil, &fakeDiscoveryStore{}, authorizer, "default", nil) + _, err := service.ListMCPAppTools(toolTestContext(), ref) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.Equal(t, pkgauth.VerbGet, authorizer.verb) + _, err = service.ReadMCPAppResource(toolTestContext(), ref, "ui://resource") + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.Equal(t, pkgauth.VerbGet, authorizer.verb) + _, err = service.CallMCPAppTool(toolTestContext(), ref, "tool", nil) + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.Equal(t, pkgauth.VerbCreate, authorizer.verb) + require.Equal(t, pkgauth.Resource{Type: "ToolServer", Namespace: "team", Name: "server"}, authorizer.resource) +} From 8823b2d3dc7bd65efcf7ed6b9af514cab1f4af5c Mon Sep 17 00:00:00 2001 From: Jet Chiang Date: Mon, 19 Oct 2026 12:22:22 -0400 Subject: [PATCH 2/3] fix import Signed-off-by: Jet Chiang --- go/core/internal/grpcserver/resource_authorization_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/core/internal/grpcserver/resource_authorization_test.go b/go/core/internal/grpcserver/resource_authorization_test.go index d2aee73b89..88b016f0d6 100644 --- a/go/core/internal/grpcserver/resource_authorization_test.go +++ b/go/core/internal/grpcserver/resource_authorization_test.go @@ -6,8 +6,8 @@ import ( "sync/atomic" "testing" - atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" "github.com/stretchr/testify/require" + atev1alpha1 "go.ate.dev/substrate/pkg/api/v1alpha1" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" corev1 "k8s.io/api/core/v1" From 3748a85cff9c5ff31e5aae83bf420469912baa16 Mon Sep 17 00:00:00 2001 From: Jet Chiang Date: Mon, 19 Oct 2026 17:13:59 -0400 Subject: [PATCH 3/3] review comments Signed-off-by: Jet Chiang --- .../references/scoped-authorization.md | 3 +- design/EP-1270-scoped-authorization.md | 31 +++++-- .../grpcserver/resource_authorization_test.go | 49 ++++++++-- .../internal/service/system/service_test.go | 90 ++++++++++--------- go/core/internal/service/system/substrate.go | 43 ++------- 5 files changed, 126 insertions(+), 90 deletions(-) diff --git a/.agents/skills/kagent-dev/references/scoped-authorization.md b/.agents/skills/kagent-dev/references/scoped-authorization.md index 87f289fe90..21a3ef569d 100644 --- a/.agents/skills/kagent-dev/references/scoped-authorization.md +++ b/.agents/skills/kagent-dev/references/scoped-authorization.md @@ -54,7 +54,8 @@ Pass scopes as ordinary function arguments. Do not place policy decisions in req - `ModelConfig` and `ModelProviderConfig` use separate typed CRUD services and resource types. Configured-provider reads and refresh are authorized independently from ModelConfig operations. - Prompt libraries, tool discovery, and WorkerPool summary rows use `kubeauth.CollectionMatcher` for custom collection projection. - Flat tool discovery must filter through the same exact owning-server identities as server discovery. -- WorkerPool scope is independent from the mixed summary's whole-inventory Substrate permission. +- `GetSubstrateSummary` requires `Check(VerbGet, Resource{Type: "SystemSummary"})` before any resource reads; denial fails the whole request with permission denied. WorkerPool rows then use their own `VerbList` scope. +- The summary capability covers its ATE inventory. Dedicated actor and worker endpoints independently require `Check(VerbGet, Resource{Type: "Substrate"})`. Both capabilities have empty namespace/name; neither uses `Scope(VerbGet, ...)`. - Static provider definitions and supported tool-server types expose no configured object and remain available after authentication. The default OSS no-op authorizer implements `CollectionAuthorizer` and returns `ScopeAll`, preserving current OSS visibility. diff --git a/design/EP-1270-scoped-authorization.md b/design/EP-1270-scoped-authorization.md index 03c3c0d809..329525f6e3 100644 --- a/design/EP-1270-scoped-authorization.md +++ b/design/EP-1270-scoped-authorization.md @@ -6,15 +6,25 @@ Kagent needs resource-scoped authorization for configuration resources without c Authorization decisions use trusted resource identity. Unauthorized resources are omitted from collections, and unauthorized operations fail with the standard permission-denied response. -## Initial scope +## Protected resources | Resource type | Operations | Attributes | | --- | --- | --- | +| `Agent` | list, get, create, update, delete | `namespace`, `name` | | `AgentTemplate` | list, get, create, update, delete | `namespace`, `name` | | `Harness` | list, create, delete | `namespace`, `name` | +| `SandboxTemplate` | list, create, delete | `namespace`, `name` | | `ModelConfig` | list, get, create, update, delete | `namespace`, `name` | +| `ModelProviderConfig` | list configured providers, get cached models, update to refresh models | `namespace`, `name` | +| `ToolServer` | list servers and tools, create, delete, MCP operations | `namespace`, `name` | +| `PromptTemplate` | list, get, create, update, delete prompt libraries | `namespace`, `name` | +| `WorkerPool` | list rows within the system summary | `namespace`, `name` | -`ListConfiguredProviders` derives entries from the separate `ModelProviderConfig` resource and is not changed by this design. +`ListConfiguredProviders` applies the `ModelProviderConfig` list scope before projecting ready providers. Cached provider-model reads require get on the exact provider config; refresh requires update before invoking discovery. Session shares cannot refresh models, even when the authorizer permits update. + +Tool discovery applies the `ToolServer` list scope to exact owning-server identities. Flat tool lists contain only tools belonging to those same visible servers. MCP tool discovery and resource reads require get on the referenced server; tool invocation requires create. Prompt-library identity is the namespace and name of its backing ConfigMap, rather than an individual prompt key. + +Static provider definitions and supported tool-server types contain no configured resource and remain available after authentication. ## Goals @@ -27,7 +37,8 @@ Authorization decisions use trusted resource identity. Unauthorized resources ar ## Non-goals - Define roles, policies, claims, subjects, grants, or catalog keys. -- Protect `SandboxAgent`, `AgentHarness`, `Session`, `ModelProviderConfig`, tool server, or prompt template resources. +- Extend this configuration-resource contract to legacy `SandboxAgent` or `AgentHarness` resources or to `Session` authorization. +- Filter ATE actor, worker, or actor-template inventory using Kubernetes object scopes. - Expose policy-engine, SQL, Kubernetes, or other backend expressions. - Predict authorization for UI controls. @@ -42,7 +53,7 @@ A collection decision may allow the complete collection, deny the complete colle For example, a principal may be allowed resources named `agent-a` or `agent-b` in `team-a`, as well as any resource in `shared`. The relationship between name and namespace must remain intact in the authorization decision. -Only namespace and name are supported for the initial resource set. Additional attributes or operations require a separate design decision based on a concrete authorization need. +Only namespace and name are supported for scoped resource collections. Additional attributes or operations require a separate design decision based on a concrete authorization need. Unsupported or invalid authorization decisions fail closed. @@ -62,6 +73,16 @@ A protected collection returns only resources permitted by its collection decisi A decision that permits no resources returns an empty collection. An authorization failure or a decision that cannot be safely applied fails the request; it never broadens access. +## Summary and Substrate capabilities + +`GetSubstrateSummary` first requires `Check(VerbGet, Resource{Type: "SystemSummary"})`, with empty namespace and name. This is a whole-summary capability that policies may grant to operators or administrators. Denial fails the request with permission denied before any Kubernetes or ATE reads; the service does not return an apparently empty inventory for denied access. + +After this check, WorkerPool rows use `Scope(VerbList, "WorkerPool")` and are intersected with configured and requested namespaces. A `NONE` pool scope returns no pool rows while still allowing ATE inventory under the summary capability. Invalid scopes and evaluator errors fail the request. + +The summary capability grants the ATE counts and actor-template inventory included in the summary. It does not require or imply permission to use the dedicated actor and worker endpoints. Those endpoints independently require `Check(VerbGet, Resource{Type: "Substrate"})`, also with empty namespace and name. External authorizers may intentionally grant these two capabilities independently. Existing ATE atespace and worker-namespace filters remain in place; WorkerPool scope does not isolate ATE inventory. + +`Scope` is reserved for `VerbList` collection filtering. Neither capability uses `Scope(VerbGet, ...)`. + ## Client behavior Catalog responses do not include create, update, or delete capability hints for presentation logic. Such hints duplicate policy decisions, can become stale, and couple the public API to a particular client experience. @@ -72,7 +93,7 @@ A client may therefore display an action that the caller cannot complete. The at The default OSS installation permits the complete protected collections and their existing operations. External authorization integrations may narrow that access. -Resources outside the initial scope retain their existing authorization behavior. +Resources outside this scope retain their existing authorization behavior. ## Alternatives considered diff --git a/go/core/internal/grpcserver/resource_authorization_test.go b/go/core/internal/grpcserver/resource_authorization_test.go index 88b016f0d6..074d5c84a5 100644 --- a/go/core/internal/grpcserver/resource_authorization_test.go +++ b/go/core/internal/grpcserver/resource_authorization_test.go @@ -24,19 +24,19 @@ import ( "github.com/kagent-dev/kagent/go/core/pkg/auth" ) -type resourceAuthorizer struct{} +type resourceAuthorizer struct{ denySummary bool } -func (resourceAuthorizer) Check(_ context.Context, _ auth.Principal, verb auth.Verb, resource auth.Resource) error { +func (a resourceAuthorizer) Check(_ context.Context, _ auth.Principal, verb auth.Verb, resource auth.Resource) error { + if resource.Type == "SystemSummary" && !a.denySummary { + return nil + } if resource.Name != "allowed" || (resource.Type == "ModelProviderConfig" && verb == auth.VerbUpdate) { return errors.New("denied") } return nil } -func (resourceAuthorizer) Scope(_ context.Context, _ auth.Principal, _ auth.Verb, resource string) (apiauthorization.AuthorizationScope, error) { - if resource == "Substrate" { - return apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}, nil - } +func (resourceAuthorizer) Scope(_ context.Context, _ auth.Principal, _ auth.Verb, _ string) (apiauthorization.AuthorizationScope, error) { return apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf, AnyOf: []apiauthorization.ScopeClause{{All: []apiauthorization.ScopePredicate{{Attribute: apiauthorization.AttributeName, Operator: apiauthorization.ScopeIn, Values: []string{"allowed"}}}}}}, nil } @@ -65,7 +65,7 @@ func TestKubernetesResourceScopesThroughGeneratedClients(t *testing.T) { connection := newConfigurationConnection(t, kube, func(config *Config) { config.PromptTemplateService = promptservice.NewService(kube, authorizer) config.ModelService = modelservice.NewService(kube, authorizer, "team", modelservice.WithProviderModelRefresher(refresher)) - config.SystemService = systemservice.NewService(kube, []string{"team"}, authorizer, nil) + config.SystemService = systemservice.NewService(kube, []string{"team"}, authorizer, emptySystemATEClient{}) }) ctx := metadata.NewOutgoingContext(t.Context(), metadata.Pairs("x-user-id", "reader")) prompts := apiv1.NewPromptTemplateServiceClient(connection) @@ -99,3 +99,38 @@ func TestKubernetesResourceScopesThroughGeneratedClients(t *testing.T) { require.Zero(t, summary.GetActorCount()) require.Empty(t, summary.GetActorTemplates()) } + +func TestProviderModelRefreshDeniedThroughShareRPC(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, v1alpha3.AddToScheme(scheme)) + kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + &v1alpha3.ModelProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "provider"}, + Status: v1alpha3.ModelProviderConfigStatus{DiscoveredModels: []string{"cached"}}, + }, + ).Build() + refresher := &deniedProviderRefresher{} + connection := newConfigurationConnection(t, kube, func(config *Config) { + config.ShareStore = &testShareStore{ + sessionShare: &apiv1.SessionShare{SessionId: testSessionID.String()}, ownerUserID: "owner", + } + config.ModelService = modelservice.NewService(kube, &auth.NoopAuthorizer{}, "team", modelservice.WithProviderModelRefresher(refresher)) + }) + ctx := metadata.NewOutgoingContext(t.Context(), metadata.Pairs("x-user-id", "visitor", "x-share-token", "share")) + models := apiv1.NewModelServiceClient(connection) + cached, err := models.ListProviderModels(ctx, &apiv1.ListProviderModelsRequest{ProviderName: "provider"}) + require.NoError(t, err) + require.Equal(t, []string{"cached"}, cached.GetModels()) + _, err = models.ListProviderModels(ctx, &apiv1.ListProviderModelsRequest{ProviderName: "provider", Refresh: true}) + assertGRPCCode(t, err, codes.PermissionDenied) + require.Zero(t, refresher.calls.Load()) +} + +func TestSummaryCapabilityDeniedThroughGeneratedClient(t *testing.T) { + connection := newConfigurationConnection(t, nil, func(config *Config) { + config.SystemService = systemservice.NewService(nil, nil, resourceAuthorizer{denySummary: true}, nil) + }) + ctx := metadata.NewOutgoingContext(t.Context(), metadata.Pairs("x-user-id", "reader")) + _, err := apiv1.NewSystemServiceClient(connection).GetSubstrateSummary(ctx, &apiv1.GetSubstrateSummaryRequest{}) + assertGRPCCode(t, err, codes.PermissionDenied) +} diff --git a/go/core/internal/service/system/service_test.go b/go/core/internal/service/system/service_test.go index 8dfa030575..f28c0743c5 100644 --- a/go/core/internal/service/system/service_test.go +++ b/go/core/internal/service/system/service_test.go @@ -615,23 +615,31 @@ func TestSubstrateEmptyPagesPreserveUpstreamTokens(t *testing.T) { type summaryAuthorizer struct { pkgAuth.NoopAuthorizer pools apiauthorization.AuthorizationScope - inventory apiauthorization.AuthorizationScope - failType string + denyType string + failScope bool calls []string } +func (a *summaryAuthorizer) Check(_ context.Context, _ pkgAuth.Principal, verb pkgAuth.Verb, resource pkgAuth.Resource) error { + a.calls = append(a.calls, string(verb)+" "+resource.Type) + if resource.Type == a.denyType { + return errors.New("denied") + } + if resource.Namespace != "" || resource.Name != "" { + return errors.New("capability must not have an object identity") + } + return nil +} + func (a *summaryAuthorizer) Scope(_ context.Context, _ pkgAuth.Principal, verb pkgAuth.Verb, resource string) (apiauthorization.AuthorizationScope, error) { a.calls = append(a.calls, string(verb)+" "+resource) - if resource == a.failType { + if a.failScope { return apiauthorization.AuthorizationScope{}, errors.New("policy unavailable") } - if resource == "WorkerPool" { - return a.pools, nil - } - return a.inventory, nil + return a.pools, nil } -func TestSummaryAuthorizesPoolAndInventorySectionsIndependently(t *testing.T) { +func TestSummaryRequiresCapabilityAndScopesPools(t *testing.T) { all := apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAll} none := apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone} constrained := apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf, AnyOf: []apiauthorization.ScopeClause{{All: []apiauthorization.ScopePredicate{ @@ -641,26 +649,20 @@ func TestSummaryAuthorizesPoolAndInventorySectionsIndependently(t *testing.T) { for _, test := range []struct { name string pools apiauthorization.AuthorizationScope - inventory apiauthorization.AuthorizationScope observed []string requested string - failType string + failScope bool wantPools int - wantATE bool code serviceerrors.Code }{ - {name: "pool only", pools: constrained, inventory: none, wantPools: 1}, - {name: "all pools", pools: all, inventory: none, wantPools: 3}, - {name: "inventory only", pools: none, inventory: all, wantATE: true}, - {name: "neither", pools: none, inventory: none}, - {name: "configured intersection", pools: all, inventory: none, observed: []string{"team"}, wantPools: 2}, - {name: "request cannot escape configuration", pools: all, inventory: none, observed: []string{"team"}, requested: "other"}, - {name: "request cannot escape policy", pools: constrained, inventory: none, requested: "other"}, - {name: "pool lookup failure", failType: "WorkerPool", inventory: all, code: serviceerrors.CodeUnavailable}, - {name: "inventory lookup failure", pools: all, failType: "Substrate", code: serviceerrors.CodeUnavailable}, - {name: "invalid pool scope", pools: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf}, inventory: all, code: serviceerrors.CodeInternal}, - {name: "unsupported inventory scope", pools: all, inventory: constrained, code: serviceerrors.CodeInternal}, - {name: "invalid inventory scope", pools: all, code: serviceerrors.CodeInternal}, + {name: "scoped pools", pools: constrained, wantPools: 1}, + {name: "all pools", pools: all, wantPools: 3}, + {name: "no pools", pools: none}, + {name: "configured intersection", pools: all, observed: []string{"team"}, wantPools: 2}, + {name: "request cannot escape configuration", pools: all, observed: []string{"team"}, requested: "other"}, + {name: "request cannot escape policy", pools: constrained, requested: "other"}, + {name: "pool lookup failure", failScope: true, code: serviceerrors.CodeUnavailable}, + {name: "invalid pool scope", pools: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAnyOf}, code: serviceerrors.CodeInternal}, } { t.Run(test.name, func(t *testing.T) { scheme := runtime.NewScheme() @@ -670,39 +672,45 @@ func TestSummaryAuthorizesPoolAndInventorySectionsIndependently(t *testing.T) { &atev1alpha1.WorkerPool{ObjectMeta: metav1.ObjectMeta{Namespace: "team", Name: "hidden"}}, &atev1alpha1.WorkerPool{ObjectMeta: metav1.ObjectMeta{Namespace: "other", Name: "allowed"}}, ).Build() - authorizer := &summaryAuthorizer{pools: test.pools, inventory: test.inventory, failType: test.failType} + // Summary access is independent from the dedicated Substrate endpoints. + authorizer := &summaryAuthorizer{pools: test.pools, denyType: "Substrate", failScope: test.failScope} ate := &fakeATEClient{actors: []*ateapipb.Actor{substrateActor("actor", "team", ateapipb.ActorState_ACTOR_STATE_RUNNING, "team", "worker")}} service := system.NewService(kube, test.observed, authorizer, ate) ctx := pkgAuth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{P: pkgAuth.Principal{User: pkgAuth.User{ID: "reader"}}}) result, err := service.GetSubstrateSummary(ctx, test.requested, "team") + require.Equal(t, []string{"get SystemSummary", "list WorkerPool"}, authorizer.calls) if test.code != "" { require.True(t, serviceerrors.IsCode(err, test.code)) - } else { - require.NoError(t, err) - require.Len(t, result.WorkerPools, test.wantPools) - require.Equal(t, []string{"list WorkerPool", "get Substrate"}, authorizer.calls) - if test.wantATE { - require.EqualValues(t, 1, result.ActorCount) - } else { - require.Zero(t, result.ActorCount) - require.Empty(t, result.ActorTemplates) - } - } - if test.wantATE { - require.Positive(t, ate.actorReads) - require.Positive(t, ate.workerReads) - require.Positive(t, ate.templateReads) - } else { require.Zero(t, ate.actorReads) require.Zero(t, ate.workerReads) require.Zero(t, ate.templateReads) + return } + require.NoError(t, err) + require.Len(t, result.WorkerPools, test.wantPools) + require.EqualValues(t, 1, result.ActorCount) + require.Positive(t, ate.actorReads) + require.Positive(t, ate.workerReads) + require.Positive(t, ate.templateReads) }) } } +func TestSummaryDeniedBeforeResourceReads(t *testing.T) { + authorizer := &summaryAuthorizer{denyType: "SystemSummary"} + // Nil clients ensure a denied request cannot read Kubernetes or ATE inventory. + service := system.NewService(nil, nil, authorizer, nil) + ctx := pkgAuth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{P: pkgAuth.Principal{User: pkgAuth.User{ID: "reader"}}}) + _, err := service.GetSubstrateSummary(ctx, "", "") + require.True(t, serviceerrors.IsCode(err, serviceerrors.CodePermissionDenied)) + require.Equal(t, []string{"get SystemSummary"}, authorizer.calls) + // The separate inventory capability is still allowed. + _, err = system.NewService(nil, nil, authorizer, &fakeATEClient{}).ListSubstrateActors(ctx, &apiv1.ListSubstrateActorsRequest{}) + require.NoError(t, err) +} + func TestSummaryDeniedPoolsDoNotRequireKubernetes(t *testing.T) { - authorizer := &summaryAuthorizer{pools: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}, inventory: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeAll}} + authorizer := &summaryAuthorizer{pools: apiauthorization.AuthorizationScope{Kind: apiauthorization.ScopeNone}} service := system.NewService(nil, nil, authorizer, &fakeATEClient{}) ctx := pkgAuth.AuthSessionTo(t.Context(), &authimpl.SimpleSession{P: pkgAuth.Principal{User: pkgAuth.User{ID: "reader"}}}) result, err := service.GetSubstrateSummary(ctx, "", "") diff --git a/go/core/internal/service/system/substrate.go b/go/core/internal/service/system/substrate.go index b672cef9c1..fe1c9684bd 100644 --- a/go/core/internal/service/system/substrate.go +++ b/go/core/internal/service/system/substrate.go @@ -11,7 +11,6 @@ import ( atev1alpha1 "go.ate.dev/substrate/pkg/api/v1alpha1" "go.ate.dev/substrate/pkg/proto/ateapipb" - apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" apiv1 "github.com/kagent-dev/kagent/go/api/gen/kagent/api/v1" "github.com/kagent-dev/kagent/go/core/internal/service/kubeauth" "github.com/kagent-dev/kagent/go/core/internal/service/serviceerrors" @@ -24,8 +23,9 @@ import ( const defaultSubstratePageSize int32 = 50 const ( - workerPoolResource = "WorkerPool" - substrateResource = "Substrate" + workerPoolResource = "WorkerPool" + substrateResource = "Substrate" + systemSummaryResource = "SystemSummary" ) // How many ate-api pages a walk will read before giving up. Each page carries its own @@ -78,16 +78,16 @@ type SubstrateActorStatusCount struct { Count int64 } -// GetSubstrateSummary counts the inventory without sending it. +// GetSubstrateSummary requires the SystemSummary capability and filters pool rows +// by their collection scope before counting the ATE inventory. // // ate-api reports no totals, so every count costs a walk of its pages. The walk holds // one page at a time and keeps only tallies. func (s *Service) GetSubstrateSummary(ctx context.Context, requestedNamespace, atespace string) (SubstrateSummary, error) { - poolMatcher, err := kubeauth.CollectionMatcher(ctx, s.authorizer, workerPoolResource) - if err != nil { + if err := s.authorize(ctx, auth.VerbGet, auth.Resource{Type: systemSummaryResource}); err != nil { return SubstrateSummary{}, err } - inventoryAllowed, err := s.summaryInventoryAllowed(ctx) + poolMatcher, err := kubeauth.CollectionMatcher(ctx, s.authorizer, workerPoolResource) if err != nil { return SubstrateSummary{}, err } @@ -122,10 +122,6 @@ func (s *Service) GetSubstrateSummary(ctx context.Context, requestedNamespace, a return strings.Compare(left.Namespace+"/"+left.Name, right.Namespace+"/"+right.Name) }) - if !inventoryAllowed { - return result, nil - } - allowAll, allowed := substrateScopeFilter(namespaces) // Three independent reads: none gates the others, so one failure leaves the rest @@ -291,31 +287,6 @@ func substratePageSize(requested int32) int32 { return requested } -// The mixed summary must distinguish a denied section from an evaluator failure. -// Substrate remains a whole-inventory capability here: accepting object clauses -// would claim isolation that these ATE reads cannot yet enforce. -func (s *Service) summaryInventoryAllowed(ctx context.Context) (bool, error) { - principal, err := authenticatedPrincipal(ctx) - if err != nil { - return false, err - } - scope, err := s.authorizer.Scope(ctx, principal, auth.VerbGet, substrateResource) - if err != nil { - return false, serviceerrors.NewUnavailable("Failed to read the Substrate authorization scope", err) - } - if len(scope.AnyOf) != 0 { - return false, serviceerrors.NewInternal("Substrate inventory does not support object scopes", nil) - } - switch scope.Kind { - case apiauthorization.ScopeAll: - return true, nil - case apiauthorization.ScopeNone: - return false, nil - default: - return false, serviceerrors.NewInternal("Invalid Substrate authorization scope", nil) - } -} - func (s *Service) workerPoolNamespaces(requested string) []string { if requested != "" && len(s.observedNamespaces) != 0 && !slices.Contains(s.observedNamespaces, requested) { return nil