From 7374df7945dbc5661f82ac77e1ec85159c5f615a Mon Sep 17 00:00:00 2001 From: Matheus Macabu Date: Fri, 1 Aug 2025 13:57:51 +0200 Subject: [PATCH] Secrets: Add inline secure value create method (#108987) --- .../secret/service/inline_secure_value.go | 64 ++++++++- .../service/inline_secure_value_test.go | 127 ++++++++++++++++++ .../apis/secret/testutils/testutils.go | 32 ++++- .../secret/metadata/secure_value_test.go | 4 +- 4 files changed, 223 insertions(+), 4 deletions(-) diff --git a/pkg/registry/apis/secret/service/inline_secure_value.go b/pkg/registry/apis/secret/service/inline_secure_value.go index 062f601ba6e..c2a6b97e451 100644 --- a/pkg/registry/apis/secret/service/inline_secure_value.go +++ b/pkg/registry/apis/secret/service/inline_secure_value.go @@ -5,9 +5,11 @@ import ( "errors" "fmt" + "github.com/grafana/authlib/authn" authlib "github.com/grafana/authlib/types" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1" @@ -15,6 +17,7 @@ import ( "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" + "github.com/grafana/grafana/pkg/util" ) type inlineSecureValueService struct { @@ -158,7 +161,66 @@ func (s *inlineSecureValueService) canIdentityReadSecureValue(ctx context.Contex } func (s *inlineSecureValueService) CreateInline(ctx context.Context, owner common.ObjectReference, value common.RawSecureValue) (string, error) { - return "", fmt.Errorf("not implemented yet") + ctx, span := s.tracer.Start(ctx, "InlineSecureValueService.CreateInline", 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 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) + } + + serviceIdentityList, ok := authInfo.GetExtra()[authn.ServiceIdentityKey] + if !ok || len(serviceIdentityList) != 1 { + return "", fmt.Errorf("expected exactly one service identity, found %d", len(serviceIdentityList)) + } + serviceIdentity := serviceIdentityList[0] + + 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 value.IsZero() { + return "", fmt.Errorf("trying to create an inline secure value with empty value") + } + + // TODO(2025-07-31): when we migrate to using the common type, we don't need this conversion. + secret := secretv1beta1.ExposedSecureValue(value) + + spec := &secretv1beta1.SecureValue{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sv-" + util.GenerateShortUID(), + Namespace: owner.Namespace, + OwnerReferences: []metav1.OwnerReference{owner.ToOwnerReference()}, + }, + Spec: secretv1beta1.SecureValueSpec{ + Description: fmt.Sprintf("Inline secure value for %s/%s in %s/%s", owner.Kind, owner.Name, owner.APIVersion, owner.APIVersion), + Value: &secret, + Decrypters: []string{ + serviceIdentity, + }, + }, + } + + createdSv, err := s.secureValueService.Create(ctx, spec, authInfo.GetUID()) + if err != nil { + return "", fmt.Errorf("error creating secure value %s for owner %v: %w", spec.Name, owner, err) + } + + return createdSv.GetName(), nil } func (s *inlineSecureValueService) DeleteWhenOwnedByResource(ctx context.Context, owner common.ObjectReference, name string) error { diff --git a/pkg/registry/apis/secret/service/inline_secure_value_test.go b/pkg/registry/apis/secret/service/inline_secure_value_test.go index a358ff13cbd..fe486df5a9b 100644 --- a/pkg/registry/apis/secret/service/inline_secure_value_test.go +++ b/pkg/registry/apis/secret/service/inline_secure_value_test.go @@ -292,3 +292,130 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) { require.Error(t, err) }) } + +func TestIntegration_InlineSecureValue_CreateInline(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 creates an inline secure value", func(t *testing.T) { + t.Parallel() + + tu := testutils.Setup(t) + + secret := common.NewSecretValue("test-value") + + serviceIdentity := "service-identity" + + createAuthCtx := testutils.CreateOBOAuthContext(t.Context(), serviceIdentity, owner.Namespace, nil, nil) + + svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, nil) + + createdName, err := svc.CreateInline(createAuthCtx, owner, secret) + require.NoError(t, err) + require.NotEmpty(t, createdName) + + decryptAuthCtx := testutils.CreateServiceAuthContext(t.Context(), serviceIdentity, owner.Namespace, []string{"secret.grafana.app/securevalues:decrypt"}) + + decryptedValues, err := tu.DecryptService.Decrypt(decryptAuthCtx, owner.Namespace, createdName) + require.NoError(t, err) + + decryptedResult, ok := decryptedValues[createdName] + require.True(t, ok) + require.Equal(t, decryptedResult.Value().DangerouslyExposeAndConsumeValue(), secret.DangerouslyExposeAndConsumeValue()) + }) + + 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.CreateInline(t.Context(), common.ObjectReference{}, "") + 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() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + createAuthCtx := testutils.CreateServiceAuthContext(t.Context(), "service-identity", defaultNs, nil) + + _, err := svc.CreateInline(createAuthCtx, common.ObjectReference{}, "") + 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" + createAuthCtx := testutils.CreateOBOAuthContext(t.Context(), "service-identity", reqNs, nil, nil) + + _, err := svc.CreateInline(createAuthCtx, owner, "") + 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) + + createAuthCtx := testutils.CreateOBOAuthContext(t.Context(), "service-identity", defaultNs, nil, nil) + + _, err := svc.CreateInline(createAuthCtx, common.ObjectReference{}, "") + 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, + } + + createAuthCtx := testutils.CreateOBOAuthContext(t.Context(), "service-identity", defaultNs, nil, nil) + + _, err := svc.CreateInline(createAuthCtx, owner, "") + require.Error(t, err) + + owner.APIGroup = "prometheus.datasource.grafana.app" + _, err = svc.CreateInline(createAuthCtx, owner, "") + require.Error(t, err) + + owner.APIVersion = "v1alpha1" + _, err = svc.CreateInline(createAuthCtx, owner, "") + require.Error(t, err) + + owner.Kind = "DataSourceConfig" + _, err = svc.CreateInline(createAuthCtx, owner, "") + require.Error(t, err) + owner.Kind = "" + + owner.Name = "test-datasource" + _, err = svc.CreateInline(createAuthCtx, owner, "") + require.Error(t, err) + }) + + t.Run("when an empty secret is provided it returns an error", func(t *testing.T) { + t.Parallel() + + svc := service.ProvideInlineSecureValueService(tracer, nil, nil) + + createAuthCtx := testutils.CreateOBOAuthContext(t.Context(), "service-identity", defaultNs, nil, nil) + + _, err := svc.CreateInline(createAuthCtx, owner, "") + require.Error(t, err) + }) +} diff --git a/pkg/registry/apis/secret/testutils/testutils.go b/pkg/registry/apis/secret/testutils/testutils.go index ebe8b109b81..3e742439bb0 100644 --- a/pkg/registry/apis/secret/testutils/testutils.go +++ b/pkg/registry/apis/secret/testutils/testutils.go @@ -244,8 +244,9 @@ func CreateUserAuthContext(ctx context.Context, namespace string, permissions ma return types.WithAuthInfo(ctx, requester) } -func CreateServiceAuthContext(ctx context.Context, serviceIdentity string, permissions []string) context.Context { +func CreateServiceAuthContext(ctx context.Context, serviceIdentity string, namespace string, permissions []string) context.Context { requester := &identity.StaticRequester{ + Namespace: namespace, AccessTokenClaims: &authn.Claims[authn.AccessTokenClaims]{ Rest: authn.AccessTokenClaims{ Permissions: permissions, @@ -256,3 +257,32 @@ func CreateServiceAuthContext(ctx context.Context, serviceIdentity string, permi return types.WithAuthInfo(ctx, requester) } + +// CreateOBOAuthContext emulates a context where the request is made on-behalf-of (OBO) a user, with an access token. +func CreateOBOAuthContext( + ctx context.Context, + serviceIdentity string, + namespace string, + userPermissions map[string][]string, + delegatedPermissions []string, +) context.Context { + requester := &identity.StaticRequester{ + Namespace: namespace, + Type: types.TypeUser, + UserID: 1, + Permissions: map[int64]map[string][]string{ + 1: userPermissions, + }, + AccessTokenClaims: &authn.Claims[authn.AccessTokenClaims]{ + Rest: authn.AccessTokenClaims{ + ServiceIdentity: serviceIdentity, + DelegatedPermissions: delegatedPermissions, + Actor: &authn.ActorClaims{ + Subject: "user:1", + }, + }, + }, + } + + return types.WithAuthInfo(ctx, requester) +} diff --git a/pkg/storage/secret/metadata/secure_value_test.go b/pkg/storage/secret/metadata/secure_value_test.go index 9400735cdd1..ce1513d95cf 100644 --- a/pkg/storage/secret/metadata/secure_value_test.go +++ b/pkg/storage/secret/metadata/secure_value_test.go @@ -403,7 +403,7 @@ func TestStateMachine(t *testing.T) { }, "decrypt": func(t *rapid.T) { input := decryptGen.Draw(t, "decryptInput") - authCtx := testutils.CreateServiceAuthContext(t.Context(), input.decrypter, []string{fmt.Sprintf("secret.grafana.app/securevalues/%+v:decrypt", input.name)}) + authCtx := testutils.CreateServiceAuthContext(t.Context(), input.decrypter, input.namespace, []string{fmt.Sprintf("secret.grafana.app/securevalues/%+v:decrypt", input.name)}) modelResult, modelErr := model.decrypt(input.decrypter, input.namespace, input.name) result, err := sut.DecryptService.Decrypt(authCtx, input.namespace, input.name) if err != nil || modelErr != nil { @@ -440,7 +440,7 @@ func TestSecureValueServiceExampleBased(t *testing.T) { require.NoError(t, err) require.Equal(t, sv.Status.Version, deletedSv.Status.Version) - authCtx := testutils.CreateServiceAuthContext(t.Context(), sv.Spec.Decrypters[0], []string{fmt.Sprintf("secret.grafana.app/securevalues/%+v:decrypt", sv.Name)}) + authCtx := testutils.CreateServiceAuthContext(t.Context(), sv.Spec.Decrypters[0], sv.Namespace, []string{fmt.Sprintf("secret.grafana.app/securevalues/%+v:decrypt", sv.Name)}) result, err := sut.DecryptService.Decrypt(authCtx, sv.Namespace, sv.Name) require.NoError(t, err) require.Equal(t, 1, len(result))