public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Secrets: Add authz checks for the single-tenant SecureValue client (#108216)

Browse Source
This commit is contained in:
Matheus Macabu
2025-07-21 11:09:07 +02:00
committed by GitHub
parent 47abf5f811
commit f7e55f2c5d
4 changed files with 124 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/registry/apis/secret/secure_value_client.go
+52 -1
View File
@@ -5,17 +5,21 @@ import (
"fmt"
claims "github.com/grafana/authlib/types"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/client-go/dynamic"
secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1"
"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"
authsvc "github.com/grafana/grafana/pkg/services/apiserver/auth/authorizer"
)
// SecureValueClient is a CRUD client for the secure value API.
@@ -25,14 +29,16 @@ type secureValueClient struct {
namespace string
service contracts.SecureValueService
validator contracts.SecureValueValidator
access authorizer.Authorizer
}
var _ SecureValueClient = &secureValueClient{}
func ProvideSecureValueClient(service contracts.SecureValueService, validator contracts.SecureValueValidator) SecureValueClient {
func ProvideSecureValueClient(service contracts.SecureValueService, validator contracts.SecureValueValidator, access claims.AccessClient) SecureValueClient {
return &secureValueClient{
service: service,
validator: validator,
access: authsvc.NewResourceAuthorizer(access),
}
}
@@ -62,6 +68,10 @@ func (c *secureValueClient) Create(ctx context.Context, obj *unstructured.Unstru
return nil, fmt.Errorf("namespace is required")
}
if err := c.checkAccess(ctx, obj.GetName(), utils.VerbCreate); err != nil {
return nil, err
}
sv, err := fromUnstructured(obj)
if err != nil {
return nil, err
@@ -96,6 +106,10 @@ func (c *secureValueClient) Get(ctx context.Context, name string, _ metav1.GetOp
return nil, fmt.Errorf("name is required")
}
if err := c.checkAccess(ctx, name, utils.VerbGet); err != nil {
return nil, err
}
sv, err := c.service.Read(ctx, xkube.Namespace(c.namespace), name)
if err != nil {
return nil, err
@@ -110,6 +124,10 @@ func (c *secureValueClient) Update(ctx context.Context, obj *unstructured.Unstru
return nil, fmt.Errorf("namespace is required")
}
if err := c.checkAccess(ctx, obj.GetName(), utils.VerbUpdate); err != nil {
return nil, err
}
oldUnstructured, err := c.Get(ctx, obj.GetName(), metav1.GetOptions{})
if err != nil {
return nil, err
@@ -154,6 +172,10 @@ func (c *secureValueClient) Delete(ctx context.Context, name string, _ metav1.De
return fmt.Errorf("name is required")
}
if err := c.checkAccess(ctx, name, utils.VerbDelete); err != nil {
return err
}
_, err := c.service.Delete(ctx, xkube.Namespace(c.namespace), name)
return err
}
@@ -164,6 +186,10 @@ func (c *secureValueClient) List(ctx context.Context, _ metav1.ListOptions) (*un
return nil, fmt.Errorf("namespace is required")
}
if err := c.checkAccess(ctx, "", utils.VerbList); err != nil {
return nil, err
}
list, err := c.service.List(ctx, xkube.Namespace(c.namespace))
if err != nil {
return nil, err
@@ -214,6 +240,31 @@ func (c *secureValueClient) ApplyStatus(_ context.Context, _ string, _ *unstruct
return nil, fmt.Errorf("applyStatus is not supported")
}
func (c *secureValueClient) checkAccess(ctx context.Context, name, verb string) error {
gr := secretv1beta1.SecureValuesResourceInfo.GroupResource()
decision, reason, err := c.access.Authorize(ctx, authorizer.AttributesRecord{
Verb: verb,
Namespace: c.namespace,
APIGroup: secretv1beta1.APIGroup,
APIVersion: secretv1beta1.APIVersion,
Resource: gr.Resource,
Subresource: "",
Name: name,
ResourceRequest: true,
})
if err != nil {
return apierrors.NewForbidden(gr, name, fmt.Errorf("failed to check access: %w", err))
}
if decision != authorizer.DecisionAllow {
return apierrors.NewForbidden(gr, name, fmt.Errorf("no access to %s: %s", verb, reason))
}
return nil
}
func toUnstructured(sv *secretv1beta1.SecureValue) (*unstructured.Unstructured, error) {
unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(sv)
if err != nil {
pkg/registry/apis/secret/secure_value_client_test.go
+66 -1
View File
@@ -4,6 +4,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/utils/ptr"
@@ -21,11 +22,15 @@ func TestIntegration_SecureValueClient_CRUD(t *testing.T) {
client := ProvideSecureValueClient(
setup.SecureValueService,
validator,
setup.AccessClient,
)
ns := "stacks-1234"
ctx := testutils.CreateUserAuthContext(t.Context(), ns, map[string][]string{
"securevalues:read": {"securevalues:uid:*"},
"securevalues:create": {"securevalues:uid:*"},
"securevalues:read": {"securevalues:uid:*"},
"securevalues:write": {"securevalues:uid:*"},
"securevalues:delete": {"securevalues:uid:*"},
})
nsClient, err := client.Client(ctx, ns)
@@ -107,3 +112,63 @@ func TestIntegration_SecureValueClient_CRUD(t *testing.T) {
require.ErrorIs(t, err, contracts.ErrSecureValueNotFound)
require.Nil(t, read)
}
func Test_SecureValueClient_CRUD_NoPermissions(t *testing.T) {
setup := testutils.Setup(t)
validator := validator.ProvideSecureValueValidator()
client := ProvideSecureValueClient(
setup.SecureValueService,
validator,
setup.AccessClient,
)
ns := "stacks-1234"
ctx := testutils.CreateUserAuthContext(t.Context(), ns, nil)
nsClient, err := client.Client(ctx, ns)
require.NoError(t, err)
require.NotNil(t, nsClient)
sv := &secretv1beta1.SecureValue{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sv",
Namespace: ns,
},
}
unstructured, err := toUnstructured(sv)
require.NoError(t, err)
// Create
created, err := nsClient.Create(ctx, unstructured, metav1.CreateOptions{})
var apiErr *apierrors.StatusError
require.ErrorAs(t, err, &apiErr)
require.Equal(t, apiErr.ErrStatus.Reason, metav1.StatusReasonForbidden)
require.Nil(t, created)
// Read
read, err := nsClient.Get(ctx, sv.Name, metav1.GetOptions{})
require.ErrorAs(t, err, &apiErr)
require.Equal(t, apiErr.ErrStatus.Reason, metav1.StatusReasonForbidden)
require.Nil(t, created)
// Update
updated, err := nsClient.Update(ctx, unstructured, metav1.UpdateOptions{})
require.ErrorAs(t, err, &apiErr)
require.Equal(t, apiErr.ErrStatus.Reason, metav1.StatusReasonForbidden)
require.Nil(t, updated)
// List
list, err := nsClient.List(ctx, metav1.ListOptions{})
require.ErrorAs(t, err, &apiErr)
require.Equal(t, apiErr.ErrStatus.Reason, metav1.StatusReasonForbidden)
require.Nil(t, list)
// Delete
err = nsClient.Delete(ctx, sv.Name, metav1.DeleteOptions{})
require.ErrorAs(t, err, &apiErr)
require.Equal(t, apiErr.ErrStatus.Reason, metav1.StatusReasonForbidden)
require.Nil(t, read)
}
pkg/registry/apis/secret/testutils/testutils.go
+4 -2
View File
@@ -23,7 +23,7 @@ import (
"github.com/grafana/grafana/pkg/registry/apis/secret/service"
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/storage/secret/database"
@@ -70,7 +70,7 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut {
require.NoError(t, err)
// Initialize access client + access control
accessControl := &actest.FakeAccessControl{ExpectedEvaluate: true}
accessControl := acimpl.ProvideAccessControl(nil)
accessClient := accesscontrol.NewLegacyAccessClient(accessControl, accesscontrol.ResourceAuthorizerOptions{
Resource: "securevalues",
Attr: "uid",
@@ -132,6 +132,7 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut {
EncryptedValueStorage: encryptedValueStorage,
SQLKeeper: sqlKeeper,
Database: database,
AccessClient: accessClient,
}
}
@@ -143,6 +144,7 @@ type Sut struct {
EncryptedValueStorage contracts.EncryptedValueStorage
SQLKeeper *sqlkeeper.SQLKeeper
Database *database.Database
AccessClient types.AccessClient
}
type CreateSvConfig struct {
pkg/server/wire_gen.go
+2 -2
View File
@@ -777,7 +777,7 @@ func Initialize(cfg *setting.Cfg, opts Options, apiOpts api.ServerOptions) (*Ser
}
secureValueService := service12.ProvideSecureValueService(tracer, accessClient, databaseDatabase, secureValueMetadataStorage, keeperMetadataStorage, ossKeeperService)
secureValueValidator := validator3.ProvideSecureValueValidator()
secureValueClient := secret.ProvideSecureValueClient(secureValueService, secureValueValidator)
secureValueClient := secret.ProvideSecureValueClient(secureValueService, secureValueValidator, accessClient)
decryptAuthorizer := decrypt.ProvideDecryptAuthorizer(tracer)
decryptStorage, err := metadata.ProvideDecryptStorage(tracer, ossKeeperService, keeperMetadataStorage, secureValueMetadataStorage, decryptAuthorizer, registerer)
if err != nil {
@@ -1330,7 +1330,7 @@ func InitializeForTest(t sqlutil.ITestDB, testingT interface {
}
secureValueService := service12.ProvideSecureValueService(tracer, accessClient, databaseDatabase, secureValueMetadataStorage, keeperMetadataStorage, ossKeeperService)
secureValueValidator := validator3.ProvideSecureValueValidator()
secureValueClient := secret.ProvideSecureValueClient(secureValueService, secureValueValidator)
secureValueClient := secret.ProvideSecureValueClient(secureValueService, secureValueValidator, accessClient)
decryptAuthorizer := decrypt.ProvideDecryptAuthorizer(tracer)
decryptStorage, err := metadata.ProvideDecryptStorage(tracer, ossKeeperService, keeperMetadataStorage, secureValueMetadataStorage, decryptAuthorizer, registerer)
if err != nil {