diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index 81b958a66df..f42eb8921d7 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -10,7 +10,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" + authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/logging" + "github.com/grafana/grafana/pkg/apimachinery/identity" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" @@ -24,12 +26,13 @@ const ( type filesConnector struct { getter RepoGetter + access authlib.AccessChecker parsers resources.ParserFactory clients resources.ClientFactory } -func NewFilesConnector(getter RepoGetter, parsers resources.ParserFactory, clients resources.ClientFactory) *filesConnector { - return &filesConnector{getter: getter, parsers: parsers, clients: clients} +func NewFilesConnector(getter RepoGetter, parsers resources.ParserFactory, clients resources.ClientFactory, access authlib.AccessChecker) *filesConnector { + return &filesConnector{getter: getter, parsers: parsers, clients: clients, access: access} } func (*filesConnector) New() runtime.Object { @@ -56,10 +59,10 @@ func (*filesConnector) NewConnectOptions() (runtime.Object, bool, string) { } // TODO: document the synchronous write and delete on the API Spec -func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { +func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { logger := logging.FromContext(ctx).With("logger", "files-connector", "repository_name", name) ctx = logging.Context(ctx, logger) - repo, err := s.getter.GetHealthyRepository(ctx, name) + repo, err := c.getter.GetHealthyRepository(ctx, name) if err != nil { logger.Debug("failed to find repository", "error", err) return nil, err @@ -70,12 +73,12 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. return nil, apierrors.NewBadRequest("repository does not support read-writing") } - parser, err := s.parsers.GetParser(ctx, readWriter) + parser, err := c.parsers.GetParser(ctx, readWriter) if err != nil { return nil, fmt.Errorf("failed to get parser: %w", err) } - clients, err := s.clients.Clients(ctx, repo.Config().Namespace) + clients, err := c.clients.Clients(ctx, repo.Config().Namespace) if err != nil { return nil, fmt.Errorf("failed to get clients: %w", err) } @@ -85,7 +88,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. return nil, fmt.Errorf("failed to get folder client: %w", err) } folders := resources.NewFolderManager(readWriter, folderClient, resources.NewEmptyFolderTree()) - dualReadWriter := resources.NewDualReadWriter(readWriter, parser, folders) + dualReadWriter := resources.NewDualReadWriter(readWriter, parser, folders, c.access) return withTimeout(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { query := r.URL.Query() @@ -107,7 +110,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. isDir := safepath.IsDir(filePath) if r.Method == http.MethodGet && isDir { - files, err := s.listFolderFiles(ctx, filePath, ref, readWriter) + files, err := c.listFolderFiles(ctx, filePath, ref, readWriter) if err != nil { responder.Error(err) return @@ -200,7 +203,18 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. } // listFolderFiles returns a list of files in a folder -func (s *filesConnector) listFolderFiles(ctx context.Context, filePath string, ref string, readWriter repository.ReaderWriter) (*provisioning.FileList, error) { +func (c *filesConnector) listFolderFiles(ctx context.Context, filePath string, ref string, readWriter repository.ReaderWriter) (*provisioning.FileList, error) { + id, err := identity.GetRequester(ctx) + if err != nil { + return nil, fmt.Errorf("missing auth info in context") + } + + // TODO: replace with access check on the repo itself + if !id.GetOrgRole().Includes(identity.RoleAdmin) { + return nil, apierrors.NewForbidden(resources.DashboardResource.GroupResource(), "", + fmt.Errorf("requires admin role")) + } + // TODO: Implement folder navigation if len(filePath) > 0 { return nil, apierrors.NewBadRequest("folder navigation not yet supported") diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 8f737d37b15..41ea564d97e 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -24,6 +24,7 @@ import ( "k8s.io/kube-openapi/pkg/spec3" "k8s.io/kube-openapi/pkg/validation/spec" + authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/logging" dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/identity" @@ -98,6 +99,7 @@ type APIBuilder struct { unified resource.ResourceClient secrets secrets.Service client client.ProvisioningV0alpha1Interface + access authlib.AccessChecker } // NewAPIBuilder creates an API builder. @@ -116,6 +118,7 @@ func NewAPIBuilder( legacyMigrator legacy.LegacyMigrator, storageStatus dualwrite.Service, secrets secrets.Service, + access authlib.AccessChecker, ) *APIBuilder { // HACK: Assume is only public if it is HTTPS isPublic := strings.HasPrefix(urlProvider(""), "https://") @@ -140,6 +143,7 @@ func NewAPIBuilder( storageStatus: storageStatus, unified: unified, secrets: secrets, + access: access, jobHistory: jobs.NewJobHistoryCache(), } } @@ -156,6 +160,7 @@ func RegisterAPIService( client resource.ResourceClient, // implements resource.RepositoryClient configProvider apiserver.RestConfigProvider, ghFactory *github.Factory, + access authlib.AccessClient, legacyMigrator legacy.LegacyMigrator, storageStatus dualwrite.Service, usageStatsService usagestats.Service, @@ -180,7 +185,7 @@ func RegisterAPIService( filepath.Join(cfg.DataPath, "clone"), // where repositories are cloned (temporarialy for now) configProvider, ghFactory, legacyMigrator, storageStatus, - secrets.NewSingleTenant(secretsSvc), + secrets.NewSingleTenant(secretsSvc), access, ) apiregistration.RegisterAPI(builder) usageStatsService.RegisterMetricsFunc(builder.collectProvisioningStats) @@ -217,21 +222,13 @@ func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { case provisioning.RepositoryResourceInfo.GetName(): // TODO: Support more fine-grained permissions than the basic roles. Especially on Enterprise. switch a.GetSubresource() { - case "": + case "", "test", "jobs": // Doing something with the repository itself. if id.GetOrgRole().Includes(identity.RoleAdmin) { return authorizer.DecisionAllow, "", nil } return authorizer.DecisionDeny, "admin role is required", nil - case "test", "export", "migrate": - // Testing doesn't make sense for non-admins. - // Exporting and migrating are potentially dangerous. - if id.GetOrgRole().Includes(identity.RoleAdmin) { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "admin role is required", nil - case "webhook": // When the resource is a webhook, we'll deal with permissions manually by checking signatures or similar in the webhook handler. // The user in this context is usually an anonymous user, but may also be an authenticated synthetic check by the Grafana instance's operator as well. @@ -239,19 +236,8 @@ func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { return authorizer.DecisionAllow, "", nil case "files": - // Reading files is allowed for everyone, so as to allow code reviews and similar. - // Writing files is only allowed fo Editor and higher. - isViewer := id.GetOrgRole().Includes(identity.RoleViewer) - isEditor := id.GetOrgRole().Includes(identity.RoleEditor) - isReadOperation := a.GetVerb() == apiutils.VerbGet - - if isEditor || (isViewer && isReadOperation) { - return authorizer.DecisionAllow, "", nil - } else if isReadOperation { - return authorizer.DecisionDeny, "viewer role is required for reads", nil - } else { - return authorizer.DecisionDeny, "editor role is required for edits", nil - } + // Access to files is controlled by the AccessClient + return authorizer.DecisionAllow, "", nil case "render": // This is used to read a blob from unified storage, for GitHub PR comments. @@ -362,7 +348,7 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI storage[provisioning.RepositoryResourceInfo.StoragePath("test")] = &testConnector{ getter: b, } - storage[provisioning.RepositoryResourceInfo.StoragePath("files")] = NewFilesConnector(b, b.parsers, b.clients) + storage[provisioning.RepositoryResourceInfo.StoragePath("files")] = NewFilesConnector(b, b.parsers, b.clients, b.access) storage[provisioning.RepositoryResourceInfo.StoragePath("resources")] = &listConnector{ getter: b, lister: b.resourceLister, diff --git a/pkg/registry/apis/provisioning/resources/dualwriter.go b/pkg/registry/apis/provisioning/resources/dualwriter.go index b15d66ae6cf..ea819149165 100644 --- a/pkg/registry/apis/provisioning/resources/dualwriter.go +++ b/pkg/registry/apis/provisioning/resources/dualwriter.go @@ -7,9 +7,13 @@ 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" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/apimachinery/utils" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" "github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath" @@ -21,10 +25,11 @@ type DualReadWriter struct { repo repository.ReaderWriter parser Parser folders *FolderManager + access authlib.AccessChecker } -func NewDualReadWriter(repo repository.ReaderWriter, parser Parser, folders *FolderManager) *DualReadWriter { - return &DualReadWriter{repo: repo, parser: parser, folders: folders} +func NewDualReadWriter(repo repository.ReaderWriter, parser Parser, folders *FolderManager, access authlib.AccessChecker) *DualReadWriter { + return &DualReadWriter{repo: repo, parser: parser, folders: folders, access: access} } func (r *DualReadWriter) Read(ctx context.Context, path string, ref string) (*ParsedResource, error) { @@ -43,6 +48,11 @@ 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) @@ -73,6 +83,10 @@ func (r *DualReadWriter) Delete(ctx context.Context, path string, ref string, me return nil, fmt.Errorf("parse file: %w", err) } + if err = r.authorize(ctx, parsed, utils.VerbDelete); err != nil { + return nil, err + } + parsed.Action = provisioning.ResourceActionDelete err = r.repo.Delete(ctx, path, ref, message) if err != nil { @@ -112,6 +126,10 @@ func (r *DualReadWriter) CreateFolder(ctx context.Context, path string, ref stri return nil, fmt.Errorf("not a folder path") } + if err := r.authorizeCreateFolder(ctx, path); err != nil { + return nil, err + } + // Now actually create the folder if err := r.repo.Create(ctx, path, ref, nil, message); err != nil { return nil, fmt.Errorf("failed to create folder: %w", err) @@ -167,6 +185,10 @@ func (r *DualReadWriter) CreateResource(ctx context.Context, path string, ref st return nil, fmt.Errorf("parse file: %w", err) } + if err = r.authorize(ctx, parsed, utils.VerbCreate); err != nil { + return nil, err + } + data, err = parsed.ToSaveBytes() if err != nil { return nil, err @@ -218,6 +240,10 @@ func (r *DualReadWriter) UpdateResource(ctx context.Context, path string, ref st return nil, fmt.Errorf("parse file: %w", err) } + if err = r.authorize(ctx, parsed, utils.VerbUpdate); err != nil { + return nil, err + } + data, err = parsed.ToSaveBytes() if err != nil { return nil, err @@ -250,3 +276,51 @@ func (r *DualReadWriter) UpdateResource(ctx context.Context, path string, ref st return parsed, nil } + +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, + }) + if err != nil { + return err + } + if !rsp.Allowed { + return apierrors.NewForbidden(parsed.GVR.GroupResource(), parsed.Obj.GetName(), + fmt.Errorf("no access to see embedded file")) + } + return nil +} + +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(), + }) + if err != nil { + return err + } + if !rsp.Allowed { + return apierrors.NewForbidden(FolderResource.GroupResource(), "", + fmt.Errorf("unable to create folder resource")) + } + return nil +} diff --git a/pkg/registry/apis/provisioning/resources/folders.go b/pkg/registry/apis/provisioning/resources/folders.go index 55913c75edf..a47af53d62d 100644 --- a/pkg/registry/apis/provisioning/resources/folders.go +++ b/pkg/registry/apis/provisioning/resources/folders.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/dynamic" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" @@ -105,6 +106,12 @@ func (fm *FolderManager) EnsureFolderExists(ctx context.Context, folder Folder, return fmt.Errorf("failed to check if folder exists: %w", err) } + // Always use the provisioning identity when writing + ctx, _, err = identity.WithProvisioningIdentity(ctx, cfg.GetNamespace()) + if err != nil { + return fmt.Errorf("unable to use provisioning identity %w", err) + } + obj = &unstructured.Unstructured{ Object: map[string]interface{}{ "spec": map[string]any{ diff --git a/pkg/registry/apis/provisioning/resources/parser.go b/pkg/registry/apis/provisioning/resources/parser.go index e13ca775ee7..505ed7ba996 100644 --- a/pkg/registry/apis/provisioning/resources/parser.go +++ b/pkg/registry/apis/provisioning/resources/parser.go @@ -217,7 +217,12 @@ func (f *ParsedResource) DryRun(ctx context.Context) error { return fmt.Errorf("no client configured") } - var err error + // Use the same identity that would eventually write the resource (via Run) + ctx, _, err := identity.WithProvisioningIdentity(ctx, f.Obj.GetNamespace()) + if err != nil { + return err + } + // FIXME: shouldn't we check for the specific error? // Dry run CREATE or UPDATE f.Existing, _ = f.Client.Get(ctx, f.Obj.GetName(), metav1.GetOptions{}) diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index 051049af119..8a6ef011285 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -76,7 +76,7 @@ func TestIntegrationProvisioning_CreatingAndGetting(t *testing.T) { require.Equal(t, http.StatusForbidden, statusCode) // Viewer can see file listing - rsp = helper.ViewerREST.Get(). + rsp = helper.AdminREST.Get(). Namespace("default"). Resource("repositories"). Name(name). @@ -300,10 +300,48 @@ func TestIntegrationProvisioning_RunLocalRepository(t *testing.T) { require.Equal(t, repo, val, "should have repo annotations") // Read the file we wrote - obj, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", targetPath) + wrapObj, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", targetPath) require.NoError(t, err, "read value") - name, _, _ = unstructured.NestedString(obj.Object, "resource", "file", "metadata", "name") - require.Equal(t, allPanels, name, "read the name out of the saved file") + + wrap := &unstructured.Unstructured{} + wrap.Object, _, err = unstructured.NestedMap(wrapObj.Object, "resource", "dryRun") + require.NoError(t, err) + meta, err := utils.MetaAccessor(wrap) + require.NoError(t, err) + require.Equal(t, allPanels, meta.GetName(), "read the name out of the saved file") + + // Check that an admin can update + meta.SetAnnotation("test", "from-provisioning") + body, err := json.Marshal(wrap.Object) + require.NoError(t, err) + result = helper.AdminREST.Put(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", targetPath). + Body(body). + SetHeader("Content-Type", "application/json"). + Do(ctx).StatusCode(&code) + require.Equal(t, 200, code) + require.NoError(t, result.Error(), "update as admin value") + raw, err = result.Raw() + require.NoError(t, err) + err = json.Unmarshal(raw, wrapper) + require.NoError(t, err) + anno, _, _ := unstructured.NestedString(wrapper.Resource.File.Object, "metadata", "annotations", "test") + require.Equal(t, "from-provisioning", anno, "should set the annotation") + + // But a viewer can not + result = helper.ViewerREST.Put(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", targetPath). + Body(body). + SetHeader("Content-Type", "application/json"). + Do(ctx).StatusCode(&code) + require.Equal(t, 403, code) + require.True(t, apierrors.IsForbidden(result.Error()), code) }) t.Run("fail using invalid paths", func(t *testing.T) {