From cfb5f5601f15ca59c7743b95be50bdaa06ae60cc Mon Sep 17 00:00:00 2001 From: Matheus Macabu Date: Thu, 31 Jul 2025 16:42:40 +0200 Subject: [PATCH] Secrets: Add inline secure value can reference method (#108965) --- pkg/apimachinery/identity/static.go | 2 +- pkg/registry/apis/secret/contracts/inline.go | 19 ++ .../secret/service/inline_secure_value.go | 166 ++++++++++ .../service/inline_secure_value_test.go | 294 ++++++++++++++++++ 4 files changed, 480 insertions(+), 1 deletion(-) create mode 100644 pkg/registry/apis/secret/contracts/inline.go create mode 100644 pkg/registry/apis/secret/service/inline_secure_value.go create mode 100644 pkg/registry/apis/secret/service/inline_secure_value_test.go diff --git a/pkg/apimachinery/identity/static.go b/pkg/apimachinery/identity/static.go index 6c0473ae0a9..afe505e261e 100644 --- a/pkg/apimachinery/identity/static.go +++ b/pkg/apimachinery/identity/static.go @@ -106,7 +106,7 @@ func (u *StaticRequester) GetExtra() map[string][]string { } result := map[string][]string{} - if u.AccessTokenClaims.Rest.ServiceIdentity != "" { + if u.AccessTokenClaims != nil && u.AccessTokenClaims.Rest.ServiceIdentity != "" { result[authnlib.ServiceIdentityKey] = []string{u.AccessTokenClaims.Rest.ServiceIdentity} } return result diff --git a/pkg/registry/apis/secret/contracts/inline.go b/pkg/registry/apis/secret/contracts/inline.go new file mode 100644 index 00000000000..4f677fa5c79 --- /dev/null +++ b/pkg/registry/apis/secret/contracts/inline.go @@ -0,0 +1,19 @@ +package contracts + +import ( + "context" + + common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" +) + +type InlineSecureValueSupport interface { + // Check that the request user can reference a secret in the context of a given resource (owner) + CanReference(ctx context.Context, owner common.ObjectReference, values common.InlineSecureValues) error + + // CreateInline creates a secret that is owned by the referenced object + // returns the name of the created secret or an error + CreateInline(ctx context.Context, owner common.ObjectReference, value common.RawSecureValue) (string, error) + + // DeleteWhenOwnedByResource removes secrets if and only if they are owned by a referenced object + DeleteWhenOwnedByResource(ctx context.Context, owner common.ObjectReference, name string) error +} diff --git a/pkg/registry/apis/secret/service/inline_secure_value.go b/pkg/registry/apis/secret/service/inline_secure_value.go new file mode 100644 index 00000000000..062f601ba6e --- /dev/null +++ b/pkg/registry/apis/secret/service/inline_secure_value.go @@ -0,0 +1,166 @@ +package service + +import ( + "context" + "errors" + "fmt" + + authlib "github.com/grafana/authlib/types" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + "k8s.io/apimachinery/pkg/runtime/schema" + + secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1" + common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" + "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" + "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" +) + +type inlineSecureValueService struct { + tracer trace.Tracer + secureValueService contracts.SecureValueService + accessChecker authlib.AccessChecker +} + +func ProvideInlineSecureValueService( + tracer trace.Tracer, + secureValueService contracts.SecureValueService, + accessClient authlib.AccessClient, +) contracts.InlineSecureValueSupport { + return &inlineSecureValueService{ + tracer: tracer, + secureValueService: secureValueService, + accessChecker: accessClient, + } +} + +func (s *inlineSecureValueService) CanReference(ctx context.Context, owner common.ObjectReference, values common.InlineSecureValues) error { + ctx, span := s.tracer.Start(ctx, "InlineSecureValueService.CanReference", trace.WithAttributes( + attribute.String("owner.namespace", owner.Namespace), + attribute.String("owner.apiGroup", owner.APIGroup), + attribute.String("owner.apiVersion", owner.APIVersion), + attribute.String("owner.kind", owner.Kind), + attribute.String("owner.name", owner.Name), + )) + defer span.End() + + authInfo, ok := authlib.AuthInfoFrom(ctx) + if !ok { + return fmt.Errorf("missing auth info in context") + } + + if owner.Namespace == "" || !authlib.NamespaceMatches(authInfo.GetNamespace(), owner.Namespace) { + return fmt.Errorf("owner namespace %s does not match auth info namespace %s", owner.Namespace, authInfo.GetNamespace()) + } + + if owner.APIGroup == "" || owner.APIVersion == "" || owner.Kind == "" || owner.Name == "" { + return fmt.Errorf("owner reference must have a valid API group, API version, kind and name") + } + + if len(values) == 0 { + return fmt.Errorf("no inline secure values provided") + } + + for field, value := range values { + if value.Name == "" { + return fmt.Errorf("field %s has an empty secure value name", field) + } + + if !value.Create.IsZero() { + return fmt.Errorf("field %s has 'create' set, which is not allowed", field) + } + + if value.Remove { + return fmt.Errorf("field %s has 'remove' set, which is not allowed", field) + } + + owned, err := s.isSecureValueOwnedByResource(ctx, owner, value.Name) + if err != nil { + return fmt.Errorf("field %s had an error checking secure value ownership: %w", field, err) + } + + if !owned { + if err := s.canIdentityReadSecureValue(ctx, xkube.Namespace(owner.Namespace), value.Name); err != nil { + return fmt.Errorf("field %s: identity cannot read secure value %s: %w", field, value.Name, err) + } + } + } + + return nil +} + +func (s *inlineSecureValueService) isSecureValueOwnedByResource(ctx context.Context, owner common.ObjectReference, name string) (bool, error) { + sv, err := s.secureValueService.Read(ctx, xkube.Namespace(owner.Namespace), name) + if err != nil { + if errors.Is(err, contracts.ErrSecureValueNotFound) { + return false, err + } + + return false, fmt.Errorf("error reading secure value %s: %w", name, err) + } + + secureValueOwners := sv.GetOwnerReferences() + if len(secureValueOwners) > 1 { + return false, fmt.Errorf("bug found: secure value %s with multiple owners, expected only one", name) + } + + if len(secureValueOwners) == 1 { + actualOwner := secureValueOwners[0] + + gv, err := schema.ParseGroupVersion(actualOwner.APIVersion) + if err != nil { + return false, fmt.Errorf("bug found: secure value %s should have valid group version here: %w", name, err) + } + if gv.Group == "" { + return false, fmt.Errorf("bug found: secure value %s should have a non-empty group in the owner reference", name) + } + + sameOwner := owner.APIGroup == gv.Group && owner.Kind == actualOwner.Kind && owner.Name == actualOwner.Name + if sameOwner { + return true, nil // The secure value is owned by the same owner reference, pass! + } + + return false, fmt.Errorf("secure value %s is not owned by %v but by %v", name, owner, actualOwner) + } + + // not owned + return false, nil +} + +func (s *inlineSecureValueService) canIdentityReadSecureValue(ctx context.Context, namespace xkube.Namespace, name string) error { + authInfo, ok := authlib.AuthInfoFrom(ctx) + if !ok { + return fmt.Errorf("missing auth info in context") + } + + // If the secure value is shared, we always need a user/svc account in the context. + if authInfo.GetIdentityType() != authlib.TypeUser && authInfo.GetIdentityType() != authlib.TypeServiceAccount { + return fmt.Errorf("identity type %s not allowed, expected either %s or %s", authInfo.GetIdentityType(), authlib.TypeUser, authlib.TypeServiceAccount) + } + + resp, err := s.accessChecker.Check(ctx, authInfo, authlib.CheckRequest{ + Verb: utils.VerbGet, + Group: secretv1beta1.APIGroup, + Resource: secretv1beta1.SecureValuesResourceInfo.GroupResource().Resource, + Namespace: namespace.String(), + Name: name, + }) + if err != nil { + return fmt.Errorf("checking access for secure value %s: %w", name, err) + } + + if !resp.Allowed { + return fmt.Errorf("identity is not allowed to reference secure value %s", name) + } + + return nil +} + +func (s *inlineSecureValueService) CreateInline(ctx context.Context, owner common.ObjectReference, value common.RawSecureValue) (string, error) { + return "", fmt.Errorf("not implemented yet") +} + +func (s *inlineSecureValueService) DeleteWhenOwnedByResource(ctx context.Context, owner common.ObjectReference, name string) error { + return fmt.Errorf("not implemented yet") +} diff --git a/pkg/registry/apis/secret/service/inline_secure_value_test.go b/pkg/registry/apis/secret/service/inline_secure_value_test.go new file mode 100644 index 00000000000..a358ff13cbd --- /dev/null +++ b/pkg/registry/apis/secret/service/inline_secure_value_test.go @@ -0,0 +1,294 @@ +package service_test + +import ( + "testing" + + common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/registry/apis/secret/service" + "github.com/grafana/grafana/pkg/registry/apis/secret/testutils" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestIntegration_InlineSecureValue_CanReference(t *testing.T) { + t.Parallel() + + tracer := noop.NewTracerProvider().Tracer("test") + + defaultNs := "org-1234" + owner := common.ObjectReference{ + APIGroup: "prometheus.datasource.grafana.app", + APIVersion: "v1alpha1", + Kind: "DataSourceConfig", + Name: "test-datasource", + Namespace: defaultNs, + } + + t.Run("happy path with owned and shared secure values", func(t *testing.T) { + t.Parallel() + + tu := testutils.Setup(t) + + sv1 := "test-secure-value-1" + createdSv1, err := tu.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) { + cfg.Sv.Name = sv1 + cfg.Sv.Namespace = defaultNs + cfg.Sv.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()} + }) + require.NoError(t, err) + require.NotNil(t, createdSv1) + + sv2 := "test-secure-value-2" + createdSv2, err := tu.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) { + cfg.Sv.Name = sv2 + cfg.Sv.Namespace = defaultNs + }) + require.NoError(t, err) + require.NotNil(t, createdSv2) + + values := common.InlineSecureValues{ + "fieldA": {Name: sv1}, + "fieldB": {Name: sv2}, + } + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{ + "securevalues:read": {"securevalues:uid:" + sv2}, + }) + + svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, tu.AccessClient) + + err = svc.CanReference(ctx, owner, values) + require.NoError(t, err) + }) + + t.Run("when the auth info is missing it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + err := svc.CanReference(t.Context(), common.ObjectReference{}, common.InlineSecureValues{}) + require.Error(t, err) + }) + + t.Run("when the owner namespace does not match auth info namespace it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + reqNs := "org-2345" + ctx := testutils.CreateUserAuthContext(t.Context(), reqNs, map[string][]string{}) + + err := svc.CanReference(ctx, owner, common.InlineSecureValues{}) + require.Error(t, err) + }) + + t.Run("when the owner namespace is empty it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{}) + + err := svc.CanReference(ctx, common.ObjectReference{}, common.InlineSecureValues{}) + require.Error(t, err) + }) + + t.Run("when the owner reference has empty fields it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + owner := common.ObjectReference{ + Namespace: defaultNs, + } + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{}) + + err := svc.CanReference(ctx, owner, common.InlineSecureValues{}) + require.Error(t, err) + + owner.APIGroup = "prometheus.datasource.grafana.app" + require.Error(t, svc.CanReference(ctx, owner, common.InlineSecureValues{})) + owner.APIGroup = "" + + owner.APIVersion = "v1alpha1" + require.Error(t, svc.CanReference(ctx, owner, common.InlineSecureValues{})) + owner.APIVersion = "" + + owner.Kind = "DataSourceConfig" + require.Error(t, svc.CanReference(ctx, owner, common.InlineSecureValues{})) + owner.Kind = "" + + owner.Name = "test-datasource" + require.Error(t, svc.CanReference(ctx, owner, common.InlineSecureValues{})) + owner.Name = "" + }) + + t.Run("when no secure values are provided it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{}) + + err := svc.CanReference(ctx, owner, common.InlineSecureValues{}) + require.Error(t, err) + }) + + t.Run("when one of the secure values does not have a `name`, it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + values := common.InlineSecureValues{ + "fieldA": {Name: ""}, + } + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{}) + + err := svc.CanReference(ctx, owner, values) + require.Error(t, err) + }) + + t.Run("when one of the secure values has `create` field set, it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + values := common.InlineSecureValues{ + "fieldA": { + Name: "test-sv", + Create: common.NewSecretValue("test"), + }, + } + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{}) + + err := svc.CanReference(ctx, owner, values) + require.Error(t, err) + }) + + t.Run("when one of the secure values has `remove` field set, it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + values := common.InlineSecureValues{ + "fieldA": { + Name: "test-sv", + Remove: true, + }, + } + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{}) + + err := svc.CanReference(ctx, owner, values) + require.Error(t, err) + }) + + t.Run("when the secure value does not exist, it returns an error", func(t *testing.T) { + t.Parallel() + + tu := testutils.Setup(t) + svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, nil) + + values := common.InlineSecureValues{ + "fieldA": {Name: "non-existent-sv"}, + } + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{}) + + err := svc.CanReference(ctx, owner, values) + require.Error(t, err) + }) + + t.Run("when the secure value is owned by a different resource, it returns an error", func(t *testing.T) { + t.Parallel() + + tu := testutils.Setup(t) + + differentOwner := common.ObjectReference{ + APIGroup: "prometheus.datasource.grafana.app", + APIVersion: "v1alpha1", + Kind: "DataSourceConfig", + Name: "different-datasource", + Namespace: defaultNs, + } + + sv1 := "test-secure-value-1" + createdSv1, err := tu.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) { + cfg.Sv.Name = sv1 + cfg.Sv.Namespace = defaultNs + cfg.Sv.OwnerReferences = []metav1.OwnerReference{differentOwner.ToOwnerReference()} + }) + require.NoError(t, err) + require.NotNil(t, createdSv1) + + values := common.InlineSecureValues{ + "fieldA": {Name: sv1}, + } + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{}) + + svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, nil) + + err = svc.CanReference(ctx, owner, values) + require.Error(t, err) + }) + + t.Run("when the request identity is not a user nor a service account, it returns an error", func(t *testing.T) { + t.Parallel() + + tu := testutils.Setup(t) + + sv1 := "test-secure-value-1" + _, err := tu.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) { + cfg.Sv.Name = sv1 + cfg.Sv.Namespace = defaultNs + }) + require.NoError(t, err) + + values := common.InlineSecureValues{ + "fieldA": {Name: sv1}, + } + + ctx := identity.WithServiceIdentityContext(t.Context(), 1234) + + svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, nil) + + err = svc.CanReference(ctx, owner, values) + require.Error(t, err) + }) + + t.Run("when the identity does not have permissions to read the secure value, it returns an error", func(t *testing.T) { + t.Parallel() + + tu := testutils.Setup(t) + + sv1 := "test-secure-value-1" + _, err := tu.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) { + cfg.Sv.Name = sv1 + cfg.Sv.Namespace = defaultNs + }) + require.NoError(t, err) + + values := common.InlineSecureValues{ + "fieldA": {Name: sv1}, + } + + svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, tu.AccessClient) + + ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{ + "securevalues:read": {"securevalues:uid:another-sv"}, // can read, but another resource! + }) + + err = svc.CanReference(ctx, owner, values) + require.Error(t, err) + + ctx = testutils.CreateUserAuthContext(t.Context(), defaultNs, nil) + + err = svc.CanReference(ctx, owner, values) + require.Error(t, err) + }) +}