From 664e5255fe43d954d5a2b24dc1a482fbaf91474b Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Fri, 11 Apr 2025 17:47:26 +0300 Subject: [PATCH] Provisioning: Use role based access when the target does not yet exist (#103862) * role based fallback * disable permissions cache with provisioning * fallback to role based * test with editor (not admin) * test with editor (not admin) * fix imports * lint * editor can create folders --- .../apis/provisioning/resources/dualwriter.go | 112 +++++++++--------- pkg/services/authz/rbac.go | 17 ++- pkg/tests/apis/provisioning/helper_test.go | 15 ++- .../apis/provisioning/provisioning_test.go | 4 +- 4 files changed, 75 insertions(+), 73 deletions(-) diff --git a/pkg/registry/apis/provisioning/resources/dualwriter.go b/pkg/registry/apis/provisioning/resources/dualwriter.go index b8313aa99d7..94b6a2430ae 100644 --- a/pkg/registry/apis/provisioning/resources/dualwriter.go +++ b/pkg/registry/apis/provisioning/resources/dualwriter.go @@ -7,8 +7,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/google/uuid" - authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/logging" "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" @@ -48,16 +46,16 @@ func (r *DualReadWriter) Read(ctx context.Context, path string, ref string) (*Pa return nil, fmt.Errorf("parse file: %w", err) } - // Authorize the parsed resource - if err = r.authorize(ctx, parsed, utils.VerbGet); err != nil { - return nil, err - } - // Fail as we use the dry run for this response and it's not about updating the resource if err := parsed.DryRun(ctx); err != nil { return nil, fmt.Errorf("run dry run: %w", err) } + // Authorize based on the existing resource + if err = r.authorize(ctx, parsed, utils.VerbGet); err != nil { + return nil, err + } + return parsed, nil } @@ -195,21 +193,6 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, path s return nil, err } - // Verify that we can create (or update) the referenced resource - verb := utils.VerbUpdate - if create { - verb = utils.VerbCreate - } - if err = r.authorize(ctx, parsed, verb); err != nil { - return nil, err - } - - // Always use the provisioning identity when writing - ctx, _, err = identity.WithProvisioningIdentity(ctx, parsed.Obj.GetNamespace()) - if err != nil { - return nil, fmt.Errorf("unable to use provisioning identity %w", err) - } - // Make sure the value is valid if err := parsed.DryRun(ctx); err != nil { logger := logging.FromContext(ctx).With("path", path, "name", parsed.Obj.GetName(), "ref", ref) @@ -224,11 +207,26 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, path s return nil, fmt.Errorf("errors while parsing file [%v]", parsed.Errors) } + // Verify that we can create (or update) the referenced resource + verb := utils.VerbUpdate + if parsed.Action == provisioning.ResourceActionCreate { + verb = utils.VerbCreate + } + if err = r.authorize(ctx, parsed, verb); err != nil { + return nil, err + } + data, err = parsed.ToSaveBytes() if err != nil { return nil, err } + // Always use the provisioning identity when writing + ctx, _, err = identity.WithProvisioningIdentity(ctx, parsed.Obj.GetNamespace()) + if err != nil { + return nil, fmt.Errorf("unable to use provisioning identity %w", err) + } + // Create or update if create { err = r.repo.Create(ctx, path, ref, data, message) @@ -255,49 +253,47 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, path s } func (r *DualReadWriter) authorize(ctx context.Context, parsed *ParsedResource, verb string) error { - auth, ok := authlib.AuthInfoFrom(ctx) - if !ok { - return fmt.Errorf("missing auth info in context") - } - rsp, err := r.access.Check(ctx, auth, authlib.CheckRequest{ - Group: parsed.GVR.Group, - Resource: parsed.GVR.Resource, - Namespace: parsed.Obj.GetNamespace(), - Name: parsed.Obj.GetName(), - Folder: parsed.Meta.GetFolder(), - Verb: verb, - }) + id, err := identity.GetRequester(ctx) if err != nil { - return err + return apierrors.NewUnauthorized(err.Error()) } - if !rsp.Allowed { - return apierrors.NewForbidden(parsed.GVR.GroupResource(), parsed.Obj.GetName(), - fmt.Errorf("no access to see embedded file")) + + // Use configured permissions for get+delete + if parsed.Existing != nil && (verb == utils.VerbGet || verb == utils.VerbDelete) { + rsp, err := r.access.Check(ctx, id, authlib.CheckRequest{ + Group: parsed.GVR.Group, + Resource: parsed.GVR.Resource, + Namespace: parsed.Existing.GetNamespace(), + Name: parsed.Existing.GetName(), + Folder: parsed.Meta.GetFolder(), + Verb: utils.VerbGet, + }) + if err != nil || !rsp.Allowed { + return apierrors.NewForbidden(parsed.GVR.GroupResource(), parsed.Obj.GetName(), + fmt.Errorf("no access to read the embedded file")) + } } - return nil + + // Simple role based access for now + if id.GetOrgRole().Includes(identity.RoleEditor) { + return nil + } + + return apierrors.NewForbidden(parsed.GVR.GroupResource(), parsed.Obj.GetName(), + fmt.Errorf("must be admin or editor to access files from provisioning")) } func (r *DualReadWriter) authorizeCreateFolder(ctx context.Context, _ string) error { - auth, ok := authlib.AuthInfoFrom(ctx) - if !ok { - return fmt.Errorf("missing auth info in context") - } - rsp, err := r.access.Check(ctx, auth, authlib.CheckRequest{ - Group: FolderResource.Group, - Resource: FolderResource.Resource, - Namespace: r.repo.Config().GetNamespace(), - Verb: utils.VerbCreate, - - // TODO: Currently this checks if you can create a new folder in root - // Ideally we should check the path and use the explicit parent and new id - Name: "f" + uuid.NewString(), - }) + id, err := identity.GetRequester(ctx) if err != nil { - return err + return apierrors.NewUnauthorized(err.Error()) } - if !rsp.Allowed { - return apierrors.NewForbidden(FolderResource.GroupResource(), "", - fmt.Errorf("unable to create folder resource")) + + // Simple role based access for now + if id.GetOrgRole().Includes(identity.RoleEditor) { + return nil } - return nil + + return apierrors.NewForbidden(FolderResource.GroupResource(), "", + fmt.Errorf("must be admin or editor to access folders with provisioning")) } diff --git a/pkg/services/authz/rbac.go b/pkg/services/authz/rbac.go index 3822596d6ba..e15603b7461 100644 --- a/pkg/services/authz/rbac.go +++ b/pkg/services/authz/rbac.go @@ -8,11 +8,6 @@ import ( "time" "github.com/fullstorydev/grpchan/inprocgrpc" - authnlib "github.com/grafana/authlib/authn" - authzlib "github.com/grafana/authlib/authz" - authzv1 "github.com/grafana/authlib/authz/proto/v1" - "github.com/grafana/authlib/cache" - authlib "github.com/grafana/authlib/types" grpcAuth "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/trace" @@ -21,6 +16,11 @@ import ( "google.golang.org/grpc/credentials/insecure" "k8s.io/client-go/rest" + authnlib "github.com/grafana/authlib/authn" + authzlib "github.com/grafana/authlib/authz" + authzv1 "github.com/grafana/authlib/authz/proto/v1" + "github.com/grafana/authlib/cache" + authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" @@ -60,6 +60,13 @@ func ProvideAuthZClient( return nil, errors.New("authZGRPCServer feature toggle is required for cloud and grpc mode") } + // Provisioning uses mode 4 (read+write only to unified storage) + // For G12 launch, we can disable caching for this and find a more scalable solution soon + // most likely this would involve passing the RV (timestamp!) in each check method + if features.IsEnabledGlobally(featuremgmt.FlagProvisioning) { + authCfg.cacheTTL = 0 + } + switch authCfg.mode { case clientModeCloud: rbacClient, err := newRemoteRBACClient(authCfg, tracer) diff --git a/pkg/tests/apis/provisioning/helper_test.go b/pkg/tests/apis/provisioning/helper_test.go index d59a01421ca..57ab7a1fabf 100644 --- a/pkg/tests/apis/provisioning/helper_test.go +++ b/pkg/tests/apis/provisioning/helper_test.go @@ -48,6 +48,7 @@ type provisioningTestHelper struct { Folders *apis.K8sResourceClient Dashboards *apis.K8sResourceClient AdminREST *rest.RESTClient + EditorREST *rest.RESTClient ViewerREST *rest.RESTClient } @@ -256,13 +257,10 @@ func runGrafana(t *testing.T, options ...grafanaOption) *provisioningTestHelper }) // Repo client, but less guard rails. Useful for subresources. We'll need this later... - restClient := helper.Org1.Admin.RESTClient(t, &schema.GroupVersion{ - Group: "provisioning.grafana.app", Version: "v0alpha1", - }) - - viewerClient := helper.Org1.Viewer.RESTClient(t, &schema.GroupVersion{ - Group: "provisioning.grafana.app", Version: "v0alpha1", - }) + gv := &schema.GroupVersion{Group: "provisioning.grafana.app", Version: "v0alpha1"} + adminClient := helper.Org1.Admin.RESTClient(t, gv) + editorClient := helper.Org1.Editor.RESTClient(t, gv) + viewerClient := helper.Org1.Viewer.RESTClient(t, gv) deleteAll := func(client *apis.K8sResourceClient) error { ctx := context.Background() @@ -287,7 +285,8 @@ func runGrafana(t *testing.T, options ...grafanaOption) *provisioningTestHelper K8sTestHelper: helper, Repositories: repositories, - AdminREST: restClient, + AdminREST: adminClient, + EditorREST: editorClient, ViewerREST: viewerClient, Jobs: jobs, Folders: folders, diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index 6b531807381..10beaf670e0 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -283,8 +283,8 @@ func TestIntegrationProvisioning_RunLocalRepository(t *testing.T) { require.Equal(t, http.StatusNotFound, code) require.True(t, apierrors.IsNotFound(result.Error())) - // Now try again with POST - result = helper.AdminREST.Post(). + // Now try again with POST (as an editor) + result = helper.EditorREST.Post(). Namespace("default"). Resource("repositories"). Name(repo).