Secret: add ability to configure extra owner decrypters (#111301)

---------

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
Co-authored-by: Stephanie Hingtgen <stephanie.hingtgen@grafana.com>
This commit is contained in:
Daniele Stefano Ferru 2025-09-19 14:41:56 +02:00 committed by GitHub
parent b63e3fd3ae
commit e69cc03ef9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 164 additions and 71 deletions

View File

@ -4,6 +4,8 @@ import (
"context"
"errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1"
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
)
@ -24,5 +26,5 @@ type DecryptStorage interface {
// DecryptAuthorizer is the interface for authorizing decryption requests.
type DecryptAuthorizer interface {
Authorize(ctx context.Context, namespace xkube.Namespace, secureValueName string, secureValueDecrypters []string) (identity string, allowed bool)
Authorize(ctx context.Context, namespace xkube.Namespace, secureValueName string, secureValueDecrypters []string, owner []metav1.OwnerReference) (identity string, allowed bool)
}

View File

@ -4,11 +4,12 @@ import (
"context"
"strings"
"github.com/grafana/authlib/authn"
claims "github.com/grafana/authlib/types"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/grafana/authlib/authn"
claims "github.com/grafana/authlib/types"
secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1"
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
@ -17,16 +18,26 @@ import (
// decryptAuthorizer is the authorizer implementation for decrypt operations.
type decryptAuthorizer struct {
tracer trace.Tracer
extra []ExtraOwnerDecrypter
}
func ProvideDecryptAuthorizer(tracer trace.Tracer) contracts.DecryptAuthorizer {
type ExtraOwnerDecrypter struct {
Identity string
Group string
}
func ProvideDecryptAuthorizer(
tracer trace.Tracer,
extra []ExtraOwnerDecrypter,
) contracts.DecryptAuthorizer {
return &decryptAuthorizer{
tracer: tracer,
extra: extra,
}
}
// authorize checks whether the auth info token has the right permissions to decrypt the secure value.
func (a *decryptAuthorizer) Authorize(ctx context.Context, ns xkube.Namespace, secureValueName string, secureValueDecrypters []string) (id string, isAllowed bool) {
// Authorize checks whether the auth info token has the right permissions to decrypt the secure value.
func (a *decryptAuthorizer) Authorize(ctx context.Context, ns xkube.Namespace, secureValueName string, secureValueDecrypters []string, owners []metav1.OwnerReference) (id string, isAllowed bool) {
ctx, span := a.tracer.Start(ctx, "DecryptAuthorizer.Authorize", trace.WithAttributes(
attribute.String("name", secureValueName),
attribute.StringSlice("decrypters", secureValueDecrypters),
@ -69,16 +80,27 @@ func (a *decryptAuthorizer) Authorize(ctx context.Context, ns xkube.Namespace, s
return serviceIdentity, false
}
// Finally check whether the service identity is allowed to decrypt this secure value.
allowed := false
// Check whether the service identity is allowed to decrypt this secure value.
for _, decrypter := range secureValueDecrypters {
if decrypter == serviceIdentity {
allowed = true
break
return serviceIdentity, true
}
}
return serviceIdentity, allowed
// finally check if the owner matches any hardcoded service identities
if owners != nil && a.extra != nil {
for _, extra := range a.extra {
if extra.Identity == serviceIdentity {
for _, owner := range owners {
if strings.HasPrefix(owner.APIVersion, extra.Group) {
return serviceIdentity, true
}
}
}
}
}
return serviceIdentity, false
}
// Adapted from https://github.com/grafana/authlib/blob/1492b99410603ca15730a1805a9220ce48232bc3/authz/client.go#L138

View File

@ -4,10 +4,12 @@ import (
"context"
"testing"
"github.com/grafana/authlib/authn"
"github.com/grafana/authlib/types"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/trace/noop"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/grafana/authlib/authn"
"github.com/grafana/authlib/types"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
@ -19,143 +21,143 @@ func TestDecryptAuthorizer(t *testing.T) {
t.Run("when no auth info is present, it returns false", func(t *testing.T) {
ctx := context.Background()
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when token permissions are empty, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
})
t.Run("when service identity is empty, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "", []string{})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when service identity is empty string, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), " ", []string{})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.Empty(t, identity)
require.False(t, allowed)
})
t.Run("when permission format is malformed (missing verb), it returns false", func(t *testing.T) {
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
// nameless
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
// named
ctx = createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues/name"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
})
t.Run("when permission verb is not exactly `decrypt`, it returns false", func(t *testing.T) {
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
// nameless
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues:*"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
// named
ctx = createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues/name:something"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
})
t.Run("when permission does not have 2 or 3 parts, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
})
t.Run("when permission has group that is not `secret.grafana.app`, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"wrong.group/securevalues/invalid:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
})
t.Run("when permission has resource that is not `securevalues`, it returns false", func(t *testing.T) {
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
// nameless
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/invalid-resource:decrypt"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
// named
ctx = createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/invalid-resource/name:decrypt"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", nil)
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", nil, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
})
t.Run("when the allow list is empty, it allows all identities", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"}, nil)
require.NotEmpty(t, identity)
require.True(t, allowed)
})
t.Run("when the identity doesn't match any allowed decrypters, it returns false", func(t *testing.T) {
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
// nameless
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues:decrypt"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"group2"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"group2"}, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
// named
ctx = createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues/name:decrypt"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", []string{"group2"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", []string{"group2"}, nil)
require.NotEmpty(t, identity)
require.False(t, allowed)
})
t.Run("when the identity matches an allowed decrypter, it returns true", func(t *testing.T) {
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
// nameless
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues:decrypt"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"}, nil)
require.True(t, allowed)
require.Equal(t, "identity", identity)
// named
ctx = createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues/name:decrypt"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "name", []string{"identity"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "name", []string{"identity"}, nil)
require.True(t, allowed)
require.Equal(t, "identity", identity)
})
@ -168,77 +170,134 @@ func TestDecryptAuthorizer(t *testing.T) {
"wrong.group/securevalues/group2:decrypt",
"secret.grafana.app/securevalues/identity:decrypt", // old style of identity+permission
})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "name1", []string{"identity"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "name1", []string{"identity"}, nil)
require.True(t, allowed)
require.Equal(t, "identity", identity)
identity, allowed = authorizer.Authorize(ctx, defaultNs, "name2", []string{"identity"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "name2", []string{"identity"}, nil)
require.True(t, allowed)
require.Equal(t, "identity", identity)
})
t.Run("when empty secure value name with specific permission, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues/name:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"}, nil)
require.Equal(t, "identity", identity)
require.False(t, allowed)
})
t.Run("when permission has an extra / but no name, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues/:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"}, nil)
require.Equal(t, "identity", identity)
require.False(t, allowed)
})
t.Run("when the decrypters list is empty, meaning nothing can decrypt the secure value, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "name", []string{})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "name", []string{}, nil)
require.Equal(t, "identity", identity)
require.False(t, allowed)
})
t.Run("when one of decrypters matches the identity, it returns true", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity1", []string{"secret.grafana.app/securevalues:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity1", "identity2", "identity3"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity1", "identity2", "identity3"}, nil)
require.Equal(t, "identity1", identity)
require.True(t, allowed)
})
t.Run("when one of extra owner decrypters matches the identity, it returns true", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity1", []string{"secret.grafana.app/securevalues:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer, []ExtraOwnerDecrypter{
{
Identity: "identity1",
Group: "test.grafana.app",
},
})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{}, []metav1.OwnerReference{
{
APIVersion: "test.grafana.app/v1",
Kind: "Test",
Name: "test",
},
})
require.Equal(t, "identity1", identity)
require.True(t, allowed)
})
t.Run("when there are extra owner decrypters but it does not match the identity, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity1", []string{"secret.grafana.app/securevalues:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer, []ExtraOwnerDecrypter{
{
Identity: "identity2",
Group: "test.grafana.app",
},
})
_, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{}, []metav1.OwnerReference{
{
APIVersion: "test.grafana.app/v1",
Kind: "Test",
Name: "test",
},
})
require.False(t, allowed)
})
t.Run("when one of extra owner decrypters matches the identity but not the group, it returns false", func(t *testing.T) {
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity1", []string{"secret.grafana.app/securevalues:decrypt"})
authorizer := ProvideDecryptAuthorizer(tracer, []ExtraOwnerDecrypter{
{
Identity: "identity1",
Group: "wrong.grafana.app",
},
})
_, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{}, []metav1.OwnerReference{
{
APIVersion: "test.grafana.app/v1",
Kind: "Test",
Name: "test",
},
})
require.False(t, allowed)
})
t.Run("permissions must be case-sensitive and return false", func(t *testing.T) {
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
ctx := createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"SECRET.grafana.app/securevalues:decrypt"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"})
identity, allowed := authorizer.Authorize(ctx, defaultNs, "", []string{"identity"}, nil)
require.Equal(t, "identity", identity)
require.False(t, allowed)
ctx = createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/SECUREVALUES:decrypt"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", []string{"identity"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", []string{"identity"}, nil)
require.Equal(t, "identity", identity)
require.False(t, allowed)
ctx = createAuthContext(context.Background(), defaultNs.String(), "identity", []string{"secret.grafana.app/securevalues:DECRYPT"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", []string{"identity"})
identity, allowed = authorizer.Authorize(ctx, defaultNs, "", []string{"identity"}, nil)
require.Equal(t, "identity", identity)
require.False(t, allowed)
})
t.Run("when namespace doesn't match the token's, it returns false", func(t *testing.T) {
authorizer := ProvideDecryptAuthorizer(tracer)
authorizer := ProvideDecryptAuthorizer(tracer, nil)
ctx := createAuthContext(context.Background(), "namespace1", "identity", []string{"secret.grafana.app/securevalues:decrypt"})
identity, allowed := authorizer.Authorize(ctx, "namespace2", "", []string{"identity"})
identity, allowed := authorizer.Authorize(ctx, "namespace2", "", []string{"identity"}, nil)
require.Empty(t, identity)
require.False(t, allowed)
})

View File

@ -3,6 +3,8 @@ package decrypt
import (
"context"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
)
@ -12,6 +14,6 @@ type NoopAlwaysAllowedAuthorizer struct{}
var _ contracts.DecryptAuthorizer = &NoopAlwaysAllowedAuthorizer{}
func (a *NoopAlwaysAllowedAuthorizer) Authorize(context.Context, xkube.Namespace, string, []string) (string, bool) {
func (a *NoopAlwaysAllowedAuthorizer) Authorize(context.Context, xkube.Namespace, string, []string, []metav1.OwnerReference) (string, bool) {
return "", true
}

View File

@ -139,7 +139,7 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut {
secureValueService := service.ProvideSecureValueService(tracer, accessClient, database, secureValueMetadataStorage, secureValueValidator, secureValueMutator, keeperMetadataStorage, keeperService, nil)
decryptAuthorizer := decrypt.ProvideDecryptAuthorizer(tracer)
decryptAuthorizer := decrypt.ProvideDecryptAuthorizer(tracer, nil)
decryptStorage, err := metadata.ProvideDecryptStorage(tracer, keeperService, keeperMetadataStorage, secureValueMetadataStorage, decryptAuthorizer, nil)
require.NoError(t, err)

View File

@ -437,6 +437,7 @@ var wireBasicSet = wire.NewSet(
secretmetadata.ProvideKeeperMetadataStorage,
secretmetadata.ProvideDecryptStorage,
secretdecrypt.ProvideDecryptAuthorizer,
wire.Value([]secretdecrypt.ExtraOwnerDecrypter(nil)),
secretdecrypt.ProvideDecryptService,
secretinline.ProvideInlineSecureValueService,
secretencryption.ProvideDataKeyStorage,

File diff suppressed because one or more lines are too long

View File

@ -5,14 +5,15 @@ import (
"fmt"
"time"
claims "github.com/grafana/authlib/types"
"github.com/grafana/grafana-app-sdk/logging"
"github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/trace"
"google.golang.org/grpc/metadata"
claims "github.com/grafana/authlib/types"
"github.com/grafana/grafana-app-sdk/logging"
secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1"
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
@ -108,7 +109,7 @@ func (s *decryptStorage) Decrypt(ctx context.Context, namespace xkube.Namespace,
return "", contracts.ErrDecryptNotFound
}
decrypterIdentity, authorized := s.decryptAuthorizer.Authorize(ctx, namespace, name, sv.Spec.Decrypters)
decrypterIdentity, authorized := s.decryptAuthorizer.Authorize(ctx, namespace, name, sv.Spec.Decrypters, sv.OwnerReferences)
if !authorized {
return "", contracts.ErrDecryptNotAuthorized
}