diff --git a/pkg/registry/apis/secret/register.go b/pkg/registry/apis/secret/register.go index 86caa751fa2..652e9b2a6eb 100644 --- a/pkg/registry/apis/secret/register.go +++ b/pkg/registry/apis/secret/register.go @@ -133,7 +133,7 @@ func (b *SecretAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API secureValueResource.StoragePath(): reststorage.NewSecureValueRest(b.secureValueMetadataStorage, secureValueResource), // The `reststorage.KeeperRest` struct will implement interfaces for CRUDL operations on `keeper`. - keeperResource.StoragePath(): reststorage.NewKeeperRest(b.keeperMetadataStorage, keeperResource), + keeperResource.StoragePath(): reststorage.NewKeeperRest(b.keeperMetadataStorage, b.accessClient, keeperResource), } apiGroupInfo.VersionedResourcesStorageMap[secretv0alpha1.VERSION] = secureRestStorage diff --git a/pkg/registry/apis/secret/reststorage/keeper_rest.go b/pkg/registry/apis/secret/reststorage/keeper_rest.go index 9bae6d4e95c..c7c2eb25e8f 100644 --- a/pkg/registry/apis/secret/reststorage/keeper_rest.go +++ b/pkg/registry/apis/secret/reststorage/keeper_rest.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + claims "github.com/grafana/authlib/types" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,13 +37,14 @@ var ( // KeeperRest is an implementation of CRUDL operations on a `keeper` backed by TODO. type KeeperRest struct { storage contracts.KeeperMetadataStorage + accessClient claims.AccessClient resource utils.ResourceInfo tableConverter rest.TableConvertor } // NewKeeperRest is a returns a constructed `*KeeperRest`. -func NewKeeperRest(storage contracts.KeeperMetadataStorage, resource utils.ResourceInfo) *KeeperRest { - return &KeeperRest{storage, resource, resource.TableConverter()} +func NewKeeperRest(storage contracts.KeeperMetadataStorage, accessClient claims.AccessClient, resource utils.ResourceInfo) *KeeperRest { + return &KeeperRest{storage, accessClient, resource, resource.TableConverter()} } // New returns an empty `*Keeper` that is used by the `Create` method. @@ -80,6 +82,21 @@ func (s *KeeperRest) List(ctx context.Context, options *internalversion.ListOpti return nil, fmt.Errorf("missing namespace") } + user, ok := claims.AuthInfoFrom(ctx) + if !ok { + return nil, fmt.Errorf("missing auth info in context") + } + + hasPermissionFor, err := s.accessClient.Compile(ctx, user, claims.ListRequest{ + Group: secretv0alpha1.GROUP, + Resource: secretv0alpha1.KeeperResourceInfo.GetName(), + Namespace: namespace, + Verb: utils.VerbGet, + }) + if err != nil { + return nil, fmt.Errorf("failed to compile checker: %w", err) + } + labelSelector := options.LabelSelector if labelSelector == nil { labelSelector = labels.Everything() @@ -93,6 +110,11 @@ func (s *KeeperRest) List(ctx context.Context, options *internalversion.ListOpti allowedKeepers := make([]secretv0alpha1.Keeper, 0) for _, keeper := range keepersList { + // Check whether the user has permission to access this specific Keeper in the namespace. + if !hasPermissionFor(keeper.Name, "") { + continue + } + if labelSelector.Matches(labels.Set(keeper.Labels)) { allowedKeepers = append(allowedKeepers, keeper) } @@ -134,11 +156,16 @@ func (s *KeeperRest) Create( return nil, fmt.Errorf("expected Keeper for create") } + user, ok := claims.AuthInfoFrom(ctx) + if !ok { + return nil, fmt.Errorf("missing auth info in context") + } + if err := createValidation(ctx, obj); err != nil { return nil, err } - createdKeeper, err := s.storage.Create(ctx, kp, "todo-user") + createdKeeper, err := s.storage.Create(ctx, kp, user.GetUID()) if err != nil { var kErr xkube.ErrorLister if errors.As(err, &kErr) { @@ -161,6 +188,11 @@ func (s *KeeperRest) Update( forceAllowCreate bool, options *metav1.UpdateOptions, ) (runtime.Object, bool, error) { + user, ok := claims.AuthInfoFrom(ctx) + if !ok { + return nil, false, fmt.Errorf("missing auth info in context") + } + oldObj, err := s.Get(ctx, name, &metav1.GetOptions{}) if err != nil { return nil, false, err @@ -189,7 +221,7 @@ func (s *KeeperRest) Update( newKeeper.Annotations = xkube.CleanAnnotations(newKeeper.Annotations) // Current implementation replaces everything passed in the spec, so it is not a PATCH. Do we want/need to support that? - updatedKeeper, err := s.storage.Update(ctx, newKeeper, "todo-user") + updatedKeeper, err := s.storage.Update(ctx, newKeeper, user.GetUID()) if err != nil { var kErr xkube.ErrorLister if errors.As(err, &kErr) { diff --git a/pkg/registry/apis/secret/reststorage/secure_value_rest.go b/pkg/registry/apis/secret/reststorage/secure_value_rest.go index 832c53207fa..76e3b9bd37d 100644 --- a/pkg/registry/apis/secret/reststorage/secure_value_rest.go +++ b/pkg/registry/apis/secret/reststorage/secure_value_rest.go @@ -7,6 +7,7 @@ import ( "strconv" "strings" + claims "github.com/grafana/authlib/types" "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -136,7 +137,7 @@ func (s *SecureValueRest) Create( ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, - options *metav1.CreateOptions, + _ *metav1.CreateOptions, ) (runtime.Object, error) { sv, ok := obj.(*secretv0alpha1.SecureValue) if !ok { @@ -147,12 +148,17 @@ func (s *SecureValueRest) Create( return nil, err } - createdSecureValue, err := s.storage.Create(ctx, sv, "todo-user") - if err != nil { - return nil, fmt.Errorf("failed to create secure value: %w", err) + user, ok := claims.AuthInfoFrom(ctx) + if !ok { + return nil, fmt.Errorf("missing auth info in context") } - return createdSecureValue, nil + createdSecureValueMetadata, err := s.storage.Create(ctx, sv, user.GetUID()) + if err != nil { + return nil, fmt.Errorf("creating secure value %+w", err) + } + + return createdSecureValueMetadata, nil } // Update a `securevalue`'s `value`. The second return parameter indicates whether the resource was newly created. @@ -161,10 +167,10 @@ func (s *SecureValueRest) Update( ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, - createValidation rest.ValidateObjectFunc, + _ rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, - forceAllowCreate bool, - options *metav1.UpdateOptions, + _forceAllowCreate bool, + _ *metav1.UpdateOptions, ) (runtime.Object, bool, error) { oldObj, err := s.Get(ctx, name, &metav1.GetOptions{}) if err != nil { @@ -190,16 +196,20 @@ func (s *SecureValueRest) Update( // TODO: do we need to do this here again? Probably not, but double-check! newSecureValue.Annotations = xkube.CleanAnnotations(newSecureValue.Annotations) - // Current implementation replaces everything passed in the spec, so it is not a PATCH. Do we want/need to support that? - updatedSecureValue, err := s.storage.Update(ctx, newSecureValue, "todo-user") - if err != nil { - return nil, false, fmt.Errorf("failed to update secure value: %w", err) + user, ok := claims.AuthInfoFrom(ctx) + if !ok { + return nil, false, fmt.Errorf("missing auth info in context") } - return updatedSecureValue, false, nil + // Current implementation replaces everything passed in the spec, so it is not a PATCH. Do we want/need to support that? + updatedSecureValueMetadata, err := s.storage.Update(ctx, newSecureValue, user.GetUID()) + if err != nil { + return updatedSecureValueMetadata, false, fmt.Errorf("updating secure value metadata: %+w", err) + } + + return updatedSecureValueMetadata, false, nil } -// Delete calls the inner `store` (persistence) in order to delete the `securevalue`. // The second return parameter `bool` indicates whether the delete was instant or not. It always is for `securevalues`. func (s *SecureValueRest) Delete(ctx context.Context, name string, _ rest.ValidateObjectFunc, _ *metav1.DeleteOptions) (runtime.Object, bool, error) { namespace, ok := request.NamespaceFrom(ctx)