From 30219176e73d0a7cef2cf3f10f4e44ebdc45d975 Mon Sep 17 00:00:00 2001 From: Roberto Jimenez Sanchez Date: Tue, 16 Dec 2025 12:43:44 +0100 Subject: [PATCH] More improvements --- pkg/registry/apis/provisioning/files.go | 3 +- .../apis/provisioning/resources/dualwriter.go | 466 ++++++------------ 2 files changed, 165 insertions(+), 304 deletions(-) diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index ad9bc4bc472..8280d86d5f4 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -105,7 +105,8 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. return } folders := resources.NewFolderManager(readWriter, folderClient, resources.NewEmptyFolderTree()) - dualReadWriter := resources.NewDualReadWriter(readWriter, parser, folders, c.access) + authorizer := resources.NewRepositoryAuthorizer(repo.Config(), c.access) + dualReadWriter := resources.NewDualReadWriter(readWriter, parser, folders, authorizer) query := r.URL.Query() opts := resources.DualWriteOptions{ Ref: query.Get("ref"), diff --git a/pkg/registry/apis/provisioning/resources/dualwriter.go b/pkg/registry/apis/provisioning/resources/dualwriter.go index 76de27874de..968d3da27c9 100644 --- a/pkg/registry/apis/provisioning/resources/dualwriter.go +++ b/pkg/registry/apis/provisioning/resources/dualwriter.go @@ -7,9 +7,9 @@ import ( 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/schema" - authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/logging" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/apps/provisioning/pkg/repository" @@ -19,180 +19,13 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/utils" ) -/* -DualReadWriter Authorization and Operation Behavior - -DualReadWriter manages file operations for provisioning repositories, handling both repository writes -and Grafana DB synchronization. It performs resource-level authorization checks using the access checker. - -Authorization Context: - - Standard provisioning Authorizer validates repository-level access before DualReadWriter is called - - DualReadWriter performs additional resource-level authorization (e.g., folder permissions for dashboards) - - Authorization checks use the access checker with the requester's identity and folder context - - Folder context comes from the EXISTING resource's folder (if it exists) or the NEW resource's folder - -Branch Behavior: - - Configured Branch (empty ref or matches repo default): Updates both repository and Grafana DB - - Other Branches: Only updates repository, Grafana DB remains unchanged - - Some operations (directory move/delete) are restricted on configured branch - -Operation Authorization and Behavior Table: -┌───────────────────┬──────────────────┬──────────────────────────────────────────────────────────────────────────┐ -│ Operation │ Branch Type │ Authorization & Behavior │ -├───────────────────┼──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ Read (GET) │ Any Branch │ Auth: VerbGet on resource in its folder │ -│ │ │ Folder: From parsed resource metadata │ -│ │ │ Behavior: │ -│ │ │ 1. Read file from repository │ -│ │ │ 2. Parse file │ -│ │ │ 3. Run DryRun validation │ -│ │ │ 4. Authorize (VerbGet) │ -│ │ │ 5. Return parsed resource │ -├───────────────────┼──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ CreateResource │ Configured │ Auth: VerbCreate if new resource, VerbUpdate if exists │ -│ (POST file) │ Branch │ Folder: Existing resource's folder OR new resource's folder │ -│ │ │ Behavior: │ -│ │ │ 1. Parse file content │ -│ │ │ 2. Run DryRun validation │ -│ │ │ 3. Check if resource exists (ensureExisting) │ -│ │ │ 4. Authorize (VerbCreate if new, VerbUpdate if exists) │ -│ │ │ 5. Create file in repository │ -│ │ │ 6. Create folder path in Grafana (if needed) │ -│ │ │ 7. Run resource create/update in Grafana DB │ -│ ├──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ │ Other Branches │ Auth: Same as configured branch │ -│ │ │ Behavior: Same as configured branch BUT skip steps 6-7 │ -│ │ │ (Grafana DB not updated) │ -├───────────────────┼──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ UpdateResource │ Configured │ Auth: VerbUpdate if exists, VerbCreate if new │ -│ (PUT file) │ Branch │ Folder: Existing resource's folder OR new resource's folder │ -│ │ │ Behavior: │ -│ │ │ 1. Parse file content │ -│ │ │ 2. Run DryRun validation │ -│ │ │ 3. Check if resource exists (ensureExisting) │ -│ │ │ 4. Authorize (VerbUpdate if exists, VerbCreate if new) │ -│ │ │ 5. Update file in repository │ -│ │ │ 6. Create folder path in Grafana (if needed) │ -│ │ │ 7. Run resource create/update in Grafana DB │ -│ ├──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ │ Other Branches │ Auth: Same as configured branch │ -│ │ │ Behavior: Same as configured branch BUT skip steps 6-7 │ -│ │ │ (Grafana DB not updated) │ -├───────────────────┼──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ Delete (file) │ Configured │ Auth: VerbDelete on resource in its folder │ -│ (DELETE file) │ Branch │ Folder: Existing resource's folder (via ensureExisting) │ -│ │ │ Behavior: │ -│ │ │ 1. Read file from repository │ -│ │ │ 2. Parse file │ -│ │ │ 3. Check if resource exists (ensureExisting) │ -│ │ │ 4. Authorize (VerbDelete) │ -│ │ │ 5. Set Action to Delete │ -│ │ │ 6. Run DryRun validation (unless skipped) │ -│ │ │ 7. Delete file from repository │ -│ │ │ 8. Run resource delete in Grafana DB │ -│ ├──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ │ Other Branches │ Auth: Same as configured branch │ -│ │ │ Behavior: Same as configured branch BUT skip step 8 │ -│ │ │ (Grafana DB not updated) │ -├───────────────────┼──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ CreateFolder │ Configured │ Auth: VerbCreate on folder │ -│ (POST dir/) │ Branch │ Folder: Parent folder context (currently empty) │ -│ │ │ Behavior: │ -│ │ │ 1. Validate path is directory │ -│ │ │ 2. Authorize folder creation (VerbCreate) │ -│ │ │ 3. Create folder in repository │ -│ │ │ 4. Ensure folder path exists in Grafana │ -│ │ │ 5. Return folder info with URLs │ -│ ├──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ │ Other Branches │ Auth: Same as configured branch │ -│ │ │ Behavior: Same as configured branch BUT skip steps 4-5 │ -│ │ │ (Grafana DB not updated) │ -├───────────────────┼──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ Delete (folder) │ Configured │ NOT ALLOWED - Returns HTTP 405 Method Not Allowed │ -│ (DELETE dir/) │ Branch │ Error: "directory delete operations are not available for configured │ -│ │ │ branch. Use bulk delete operations via the jobs API instead" │ -│ ├──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ │ Other Branches │ Auth: VerbDelete on folder │ -│ │ │ Folder: The folder path itself │ -│ │ │ Behavior: │ -│ │ │ 1. Authorize folder deletion (VerbDelete) │ -│ │ │ 2. Delete folder from repository │ -│ │ │ 3. Return folder delete response │ -│ │ │ Note: Grafana DB not updated (branch operation) │ -├───────────────────┼──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ MoveResource │ Configured │ NOT ALLOWED for directories - Returns HTTP 405 Method Not Allowed │ -│ (POST with │ Branch │ Error: "directory move operations are not available for configured │ -│ originalPath) │ │ branch. Use bulk move operations via the jobs API instead" │ -│ │ │ │ -│ (directory move) │ │ ALLOWED for files: │ -│ │ │ Auth: Two checks required: │ -│ │ │ 1. VerbDelete on original resource in its existing folder │ -│ │ │ 2. VerbCreate (if new) or VerbUpdate (if exists) on destination │ -│ │ │ Folder: Existing resource's folder for both source and destination │ -│ │ │ Behavior: │ -│ │ │ 1. Read and parse original file │ -│ │ │ 2. Check if original resource exists (ensureExisting) │ -│ │ │ 3. Authorize delete on original (VerbDelete) │ -│ │ │ 4. Parse destination file (original or updated content) │ -│ │ │ 5. Run DryRun on destination │ -│ │ │ 6. Check if destination resource exists (ensureExisting) │ -│ │ │ 7. Authorize destination (VerbCreate if new, VerbUpdate if exists) │ -│ │ │ 8. Perform move in repository (or delete+create if content changes) │ -│ │ │ 9. Create folder path in Grafana (if needed) │ -│ │ │ 10. Delete old resource from Grafana (if name changed) │ -│ │ │ 11. Create/update new resource in Grafana DB │ -│ ├──────────────────┼──────────────────────────────────────────────────────────────────────────┤ -│ │ Other Branches │ Auth (files): Same as configured branch │ -│ │ │ Behavior (files): Same BUT skip steps 9-11 (Grafana DB not updated) │ -│ │ │ │ -│ │ │ Auth (directories): │ -│ │ │ 1. VerbDelete on original folder │ -│ │ │ 2. VerbCreate on destination folder │ -│ │ │ Behavior (directories): │ -│ │ │ 1. Authorize delete on original folder (VerbDelete) │ -│ │ │ 2. Authorize create on destination folder (VerbCreate) │ -│ │ │ 3. Perform move in repository │ -│ │ │ 4. Return folder move response │ -│ │ │ Note: Grafana DB not updated (branch operation) │ -└───────────────────┴──────────────────┴──────────────────────────────────────────────────────────────────────────┘ - -Authorization Details: -- authorize() checks permissions on resources (dashboards, etc.) - * Uses parsed.Existing.folder if resource exists, otherwise uses parsed.Meta.folder - * Calls access.Check() with resource Group/Resource/Namespace/Name/Verb and folder context - * Returns Forbidden (403) if not allowed - -- authorizeFolder() checks permissions on folders - * Used for folder create/delete operations - * Calls access.Check() with Folder resource and verb - * For create operations, uses empty name (checking parent folder permissions) - * Returns Forbidden (403) if not allowed - -Key Concepts: -- Configured Branch: The default branch set in repository config (empty ref or explicit match) -- Other Branches: Any non-default branch specified via "ref" query parameter -- ensureExisting: Populates parsed.Existing by querying Grafana DB for resource by name -- DryRun: Validates resource before applying (checks schema, required fields, etc.) -- Grafana DB Update: Only happens on configured branch; ensures repository and DB stay in sync -- Provisioning Identity: All write operations use provisioning service identity for Grafana DB changes -- Folder Context: Authorization checks include folder path to validate granular permissions -- Move Operations: Require authorization at both source (delete) and destination (create/update) -- Bulk Operations: Directory moves/deletes on configured branch must use Jobs API for safety - -Restrictions: -- Directory delete on configured branch: Not allowed (use Jobs API) -- Directory move on configured branch: Not allowed (use Jobs API) -- PUT on directory: Returns Method Not Supported (405) -- Operations on unhealthy repositories: Write operations require healthy repository -*/ - // DualReadWriter is a wrapper around a repository that can read from and write resources // into both the Git repository as well as in Grafana. It isn't a dual writer in the sense of what unistore handling calls dual writing. type DualReadWriter struct { - repo repository.ReaderWriter - parser Parser - folders *FolderManager - access authlib.AccessChecker + repo repository.ReaderWriter + parser Parser + folders *FolderManager + authorizer Authorizer } type DualWriteOptions struct { @@ -208,8 +41,8 @@ type DualWriteOptions struct { Branch string // Configured default branch } -func NewDualReadWriter(repo repository.ReaderWriter, parser Parser, folders *FolderManager, access authlib.AccessChecker) *DualReadWriter { - return &DualReadWriter{repo: repo, parser: parser, folders: folders, access: access} +func NewDualReadWriter(repo repository.ReaderWriter, parser Parser, folders *FolderManager, authorizer Authorizer) *DualReadWriter { + return &DualReadWriter{repo: repo, parser: parser, folders: folders, authorizer: authorizer} } func (r *DualReadWriter) Read(ctx context.Context, path string, ref string) (*ParsedResource, error) { @@ -237,8 +70,7 @@ func (r *DualReadWriter) Read(ctx context.Context, path string, ref string) (*Pa return nil, fmt.Errorf("error running dryRun: %w", err) } - // Authorize based on the existing resource - if err = r.authorize(ctx, parsed, utils.VerbGet); err != nil { + if err = r.authorizer.AuthorizeResource(ctx, parsed, utils.VerbGet); err != nil { return nil, err } @@ -246,7 +78,7 @@ func (r *DualReadWriter) Read(ctx context.Context, path string, ref string) (*Pa } func (r *DualReadWriter) Delete(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { - if err := repository.IsWriteAllowed(r.repo.Config(), opts.Ref); err != nil { + if err := r.authorizer.AuthorizeWrite(ctx, opts.Ref); err != nil { return nil, err } @@ -272,15 +104,7 @@ func (r *DualReadWriter) Delete(ctx context.Context, opts DualWriteOptions) (*Pa return nil, fmt.Errorf("parse file: %w", err) } - // Populate the existing resource to ensure we check permissions in the correct folder - if err = r.ensureExisting(ctx, parsed); err != nil { - return nil, err - } - - // Authorize after simple checks but before performing the operation - // This checks against the existing resource's folder (if it exists) to ensure - // we validate permissions where the resource actually lives - if err = r.authorize(ctx, parsed, utils.VerbDelete); err != nil { + if err = r.authorizer.AuthorizeResource(ctx, parsed, utils.VerbDelete); err != nil { return nil, err } @@ -312,7 +136,7 @@ func (r *DualReadWriter) Delete(ctx context.Context, opts DualWriteOptions) (*Pa // CreateFolder creates a new folder in the repository // FIXME: fix signature to return ParsedResource func (r *DualReadWriter) CreateFolder(ctx context.Context, opts DualWriteOptions) (*provisioning.ResourceWrapper, error) { - if err := repository.IsWriteAllowed(r.repo.Config(), opts.Ref); err != nil { + if err := r.authorizer.AuthorizeWrite(ctx, opts.Ref); err != nil { return nil, err } @@ -320,9 +144,12 @@ func (r *DualReadWriter) CreateFolder(ctx context.Context, opts DualWriteOptions return nil, fmt.Errorf("not a folder path") } - if err := r.authorizeFolder(ctx, opts.Path, utils.VerbCreate); err != nil { + // For create operations, use empty name to check parent folder permissions + folderParsed := folderParsedResource(opts.Path, opts.Ref, r.repo.Config(), "") + if err := r.authorizer.AuthorizeResource(ctx, folderParsed, utils.VerbCreate); err != nil { return nil, err } + // TODO: authorized to create folders under first existing ancestor folder // Now actually create the folder if err := r.repo.Create(ctx, opts.Path, opts.Ref, nil, opts.Message); err != nil { @@ -370,17 +197,90 @@ func (r *DualReadWriter) CreateFolder(ctx context.Context, opts DualWriteOptions // CreateResource creates a new resource in the repository func (r *DualReadWriter) CreateResource(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { - return r.createOrUpdate(ctx, true, opts) + if err := r.authorizer.AuthorizeWrite(ctx, opts.Ref); err != nil { + return nil, err + } + + info := &repository.FileInfo{ + Data: opts.Data, + Path: opts.Path, + Ref: opts.Ref, + } + + parsed, err := r.parser.Parse(ctx, info) + if err != nil { + return nil, err + } + + // TODO: check if the resource does not exist in the database. + + // Make sure the value is valid + if !opts.SkipDryRun { + if err := parsed.DryRun(ctx); err != nil { + logger := logging.FromContext(ctx).With("path", opts.Path, "name", parsed.Obj.GetName(), "ref", opts.Ref) + logger.Warn("failed to dry run resource on create", "error", err) + + return nil, fmt.Errorf("error running dryRun: %w", err) + } + } + + if len(parsed.Errors) > 0 { + // Now returns BadRequest (400) for validation errors + return nil, fmt.Errorf("errors while parsing file [%v]", parsed.Errors) + } + + // TODO: is this the right way? + // Check if resource already exists - create should fail if it does + if err = r.ensureExisting(ctx, parsed); err != nil { + return nil, err + } + if parsed.Existing != nil { + return nil, apierrors.NewConflict(parsed.GVR.GroupResource(), parsed.Obj.GetName(), + fmt.Errorf("resource already exists")) + } + + // Authorization check: Check if we can create the resource in the folder from the file + if err = r.authorizer.AuthorizeResource(ctx, parsed, utils.VerbCreate); err != nil { + return nil, err + } + + // TODO: authorized to create folders under first existing ancestor folder + + 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) + } + + // TODO: handle the error repository.ErrFileAlreadyExists + err = r.repo.Create(ctx, opts.Path, opts.Ref, data, opts.Message) + if err != nil { + return nil, err // raw error is useful + } + + // Directly update the grafana database + // Behaves the same running sync after writing + // FIXME: to make sure if behaves in the same way as in sync, we should + // we should refactor the code to use the same function. + if r.shouldUpdateGrafanaDB(opts, parsed) { + if _, err := r.folders.EnsureFolderPathExist(ctx, opts.Path); err != nil { + return nil, fmt.Errorf("ensure folder path exists: %w", err) + } + + err = parsed.Run(ctx) + } + + return parsed, err } // UpdateResource updates a resource in the repository func (r *DualReadWriter) UpdateResource(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { - return r.createOrUpdate(ctx, false, opts) -} - -// Create or updates a resource in the repository -func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts DualWriteOptions) (*ParsedResource, error) { - if err := repository.IsWriteAllowed(r.repo.Config(), opts.Ref); err != nil { + if err := r.authorizer.AuthorizeWrite(ctx, opts.Ref); err != nil { return nil, err } @@ -399,7 +299,7 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts D if !opts.SkipDryRun { if err := parsed.DryRun(ctx); err != nil { logger := logging.FromContext(ctx).With("path", opts.Path, "name", parsed.Obj.GetName(), "ref", opts.Ref) - logger.Warn("failed to dry run resource on create", "error", err) + logger.Warn("failed to dry run resource on update", "error", err) return nil, fmt.Errorf("error running dryRun: %w", err) } @@ -410,20 +310,15 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts D return nil, fmt.Errorf("errors while parsing file [%v]", parsed.Errors) } - // Populate existing resource if it exists + // Populate existing resource to check permissions in the correct folder if err = r.ensureExisting(ctx, parsed); err != nil { return nil, err } - // Authorization check: - // - If resource exists (parsed.Existing != nil): Check if we can update it in its current folder - // - If resource doesn't exist: Check if we can create it in the folder from the file - // This prevents unauthorized overwrites of existing resources and ensures proper folder permissions - verb := utils.VerbUpdate - if parsed.Existing == nil && parsed.Action == provisioning.ResourceActionCreate { - verb = utils.VerbCreate - } - if err = r.authorize(ctx, parsed, verb); err != nil { + // TODO: what to do with a name or kind change? + + // Authorization check: Check if we can update the existing resource in its current folder + if err = r.authorizer.AuthorizeResource(ctx, parsed, utils.VerbUpdate); err != nil { return nil, err } @@ -438,12 +333,7 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts D return nil, fmt.Errorf("unable to use provisioning identity %w", err) } - // Create or update - if create { - err = r.repo.Create(ctx, opts.Path, opts.Ref, data, opts.Message) - } else { - err = r.repo.Update(ctx, opts.Path, opts.Ref, data, opts.Message) - } + err = r.repo.Update(ctx, opts.Path, opts.Ref, data, opts.Message) if err != nil { return nil, err // raw error is useful } @@ -465,7 +355,7 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts D // MoveResource moves a resource from one path to another in the repository func (r *DualReadWriter) MoveResource(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { - if err := repository.IsWriteAllowed(r.repo.Config(), opts.Ref); err != nil { + if err := r.authorizer.AuthorizeWrite(ctx, opts.Ref); err != nil { return nil, err } @@ -505,12 +395,15 @@ func (r *DualReadWriter) moveDirectory(ctx context.Context, opts DualWriteOption } // Check permissions to delete the original folder - if err := r.authorizeFolder(ctx, opts.OriginalPath, utils.VerbDelete); err != nil { + originalFolderID := ParseFolder(opts.OriginalPath, r.repo.Config().Name).ID + originalFolderParsed := folderParsedResource(opts.OriginalPath, opts.Ref, r.repo.Config(), originalFolderID) + if err := r.authorizer.AuthorizeResource(ctx, originalFolderParsed, utils.VerbDelete); err != nil { return nil, fmt.Errorf("not authorized to move from original folder: %w", err) } - // Check permissions to create at the new folder location - if err := r.authorizeFolder(ctx, opts.Path, utils.VerbCreate); err != nil { + // Check permissions to create at the new folder location (empty name for create) + newFolderParsed := folderParsedResource(opts.Path, opts.Ref, r.repo.Config(), "") + if err := r.authorizer.AuthorizeResource(ctx, newFolderParsed, utils.VerbCreate); err != nil { return nil, fmt.Errorf("not authorized to move to new folder: %w", err) } @@ -570,7 +463,7 @@ func (r *DualReadWriter) moveFile(ctx context.Context, opts DualWriteOptions) (* } // Authorize delete on the original path (checks existing resource's folder if it exists) - if err = r.authorize(ctx, parsed, utils.VerbDelete); err != nil { + if err = r.authorizer.AuthorizeResource(ctx, parsed, utils.VerbDelete); err != nil { return nil, fmt.Errorf("not authorized to delete original file: %w", err) } @@ -617,10 +510,10 @@ func (r *DualReadWriter) moveFile(ctx context.Context, opts DualWriteOptions) (* // - If resource exists at destination: Check if we can update it in its folder // - If no resource at destination: Check if we can create in the new folder verb := utils.VerbUpdate - if newParsed.Existing == nil && newParsed.Action == provisioning.ResourceActionCreate { + if newParsed.Existing == nil { verb = utils.VerbCreate } - if err = r.authorize(ctx, newParsed, verb); err != nil { + if err = r.authorizer.AuthorizeResource(ctx, newParsed, verb); err != nil { return nil, fmt.Errorf("not authorized for destination: %w", err) } @@ -679,53 +572,6 @@ func (r *DualReadWriter) moveFile(ctx context.Context, opts DualWriteOptions) (* return newParsed, nil } -// authorize checks if the requester has permission to perform the specified verb on the resource. -// The provisioning service operates with admin privileges and validates based on resource-level permissions. -// -// IMPORTANT: If parsed.Existing is set, this checks permissions against the EXISTING resource's folder. -// Otherwise, it checks against the NEW resource's folder (from parsed.Meta). -// This ensures we validate permissions in the correct folder where the resource actually exists or will be created. -func (r *DualReadWriter) authorize(ctx context.Context, parsed *ParsedResource, verb string) error { - id, err := identity.GetRequester(ctx) - if err != nil { - return apierrors.NewUnauthorized(err.Error()) - } - - // Determine which resource and folder to use for authorization - var resourceName string - var folder string - - if parsed.Existing != nil { - // Check against existing resource in its current folder - resourceName = parsed.Existing.GetName() - if meta, err := utils.MetaAccessor(parsed.Existing); err == nil { - folder = meta.GetFolder() - } else { - // Fallback to file metadata if we can't extract from existing - folder = parsed.Meta.GetFolder() - } - } else { - // Check against new resource in the folder from the file - resourceName = parsed.Obj.GetName() - folder = parsed.Meta.GetFolder() - } - - // Check resource-level permissions via access checker - rsp, err := r.access.Check(ctx, id, authlib.CheckRequest{ - Group: parsed.GVR.Group, - Resource: parsed.GVR.Resource, - Namespace: id.GetNamespace(), - Name: resourceName, - Verb: verb, - }, folder) - if err != nil || !rsp.Allowed { - return apierrors.NewForbidden(parsed.GVR.GroupResource(), resourceName, - fmt.Errorf("no access to perform %s on the resource in folder '%s'", verb, folder)) - } - - return nil -} - // ensureExisting populates parsed.Existing if a resource with the given name exists in storage. // Returns nil if no resource exists, if Client is nil, or if Existing is already populated. // This is used before authorization checks to ensure we validate permissions against the actual @@ -747,42 +593,6 @@ func (r *DualReadWriter) ensureExisting(ctx context.Context, parsed *ParsedResou return nil } -// authorizeFolder validates that the requester has permission to perform the specified verb on a folder. -// This is used for folder operations (create, delete, update) where we don't have a ParsedResource. -func (r *DualReadWriter) authorizeFolder(ctx context.Context, path string, verb string) error { - id, err := identity.GetRequester(ctx) - if err != nil { - return apierrors.NewUnauthorized(err.Error()) - } - - // For folder operations, we need to determine the parent folder for permission checks - parentFolder := "" - // TODO: Extract parent folder from path if needed for hierarchical permission checks - - // For create operations, use empty name (checking if we can create in the parent) - // For other operations, the folder path is the resource name - name := path - if verb == utils.VerbCreate { - name = "" // Empty name for creation checks - } - - // Check folder permissions via access checker - rsp, err := r.access.Check(ctx, id, authlib.CheckRequest{ - Group: FolderResource.Group, - Resource: FolderResource.Resource, - Namespace: id.GetNamespace(), - Name: name, - Verb: verb, - }, parentFolder) - - if err != nil || !rsp.Allowed { - return apierrors.NewForbidden(FolderResource.GroupResource(), path, - fmt.Errorf("no access to perform %s on folder", verb)) - } - - return nil -} - func (r *DualReadWriter) deleteFolder(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { // Reject directory delete operations for configured branch - use bulk operations instead if r.isConfiguredBranch(opts) { @@ -797,7 +607,9 @@ func (r *DualReadWriter) deleteFolder(ctx context.Context, opts DualWriteOptions } // Check permissions to delete the folder - if err := r.authorizeFolder(ctx, opts.Path, utils.VerbDelete); err != nil { + folderID := ParseFolder(opts.Path, r.repo.Config().Name).ID + folderParsed := folderParsedResource(opts.Path, opts.Ref, r.repo.Config(), folderID) + if err := r.authorizer.AuthorizeResource(ctx, folderParsed, utils.VerbDelete); err != nil { return nil, err } @@ -829,6 +641,54 @@ func getPathType(isDir bool) string { return "file (no trailing '/')" } +// folderParsedResource creates a ParsedResource for a folder path. +// This is used for authorization checks on folder operations. +// For create operations, name should be empty string to check parent permissions. +// For other operations, name should be the folder ID derived from the path. +func folderParsedResource(path, ref string, repo *provisioning.Repository, name string) *ParsedResource { + folderObj := &unstructured.Unstructured{} + folderObj.SetName(name) + folderObj.SetNamespace(repo.Namespace) + + // TODO: which parent? top existing ancestor. + + meta, _ := utils.MetaAccessor(folderObj) + if meta != nil { + // Set parent folder for folder operations + parentFolder := "" + if path != "" { + parentPath := safepath.Dir(path) + if parentPath != "" { + parentFolder = ParseFolder(parentPath, repo.Name).ID + } else { + parentFolder = RootFolder(repo) + } + } + meta.SetFolder(parentFolder) + } + + return &ParsedResource{ + Info: &repository.FileInfo{ + Path: path, + Ref: ref, + }, + Obj: folderObj, + Meta: meta, + GVK: schema.GroupVersionKind{ + Group: FolderResource.Group, + Version: FolderResource.Version, + Kind: "Folder", + }, + GVR: FolderResource, + Repo: provisioning.ResourceRepositoryInfo{ + Type: repo.Spec.Type, + Namespace: repo.Namespace, + Name: repo.Name, + Title: repo.Spec.Title, + }, + } +} + func folderDeleteResponse(ctx context.Context, path, ref string, repo repository.Repository) (*ParsedResource, error) { urls, err := getFolderURLs(ctx, path, ref, repo) if err != nil {