From 6c728f8dec622e6a09bc7bf7c99468439d2eb370 Mon Sep 17 00:00:00 2001 From: Charandas <542168+charandas@users.noreply.github.com> Date: Sun, 2 Nov 2025 13:14:08 -0800 Subject: [PATCH] Provisioning: allow access check to proceed even when non access policy (#112946) * Provisioning: allow access check to proceed even when non access policy * Provisioning: access checker needs this for MT * add permissions registration * remove scopes * use in MT for now * no need to document an internal flag here * revert vscode change * refactor the authZ permission evaluation and mapper code to allow evaluating unscoped actions beyond creation * update wire * gofmt * add boolean to struct --------- Co-authored-by: IevaVasiljeva --- pkg/registry/apis/apis.go | 1 + .../apis/provisioning/accesscontrol.go | 129 ++++++++++++++++++ .../apis/provisioning/dependencies.go | 24 ++++ pkg/registry/apis/provisioning/register.go | 51 ++++--- pkg/registry/apis/wireset.go | 1 + pkg/server/wire_gen.go | 12 +- pkg/services/authz/rbac/mapper.go | 85 +++++++----- pkg/services/authz/rbac/service.go | 30 ++-- 8 files changed, 268 insertions(+), 65 deletions(-) create mode 100644 pkg/registry/apis/provisioning/accesscontrol.go create mode 100644 pkg/registry/apis/provisioning/dependencies.go diff --git a/pkg/registry/apis/apis.go b/pkg/registry/apis/apis.go index 98a09583c85..a1d323f996a 100644 --- a/pkg/registry/apis/apis.go +++ b/pkg/registry/apis/apis.go @@ -30,6 +30,7 @@ func ProvideRegistryServiceSink( _ *provisioning.APIBuilder, _ *ofrep.APIBuilder, _ *secret.DependencyRegisterer, + _ *provisioning.DependencyRegisterer, ) *Service { return &Service{} } diff --git a/pkg/registry/apis/provisioning/accesscontrol.go b/pkg/registry/apis/provisioning/accesscontrol.go new file mode 100644 index 00000000000..e56ab7b06e2 --- /dev/null +++ b/pkg/registry/apis/provisioning/accesscontrol.go @@ -0,0 +1,129 @@ +package provisioning + +import ( + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/org" +) + +const ( + // Repositories + ActionProvisioningRepositoriesCreate = "provisioning.repositories:create" // CREATE. + ActionProvisioningRepositoriesWrite = "provisioning.repositories:write" // UPDATE. + ActionProvisioningRepositoriesRead = "provisioning.repositories:read" // GET + LIST. + ActionProvisioningRepositoriesDelete = "provisioning.repositories:delete" // DELETE. + + // Jobs + ActionProvisioningJobsCreate = "provisioning.jobs:create" // CREATE. + ActionProvisioningJobsWrite = "provisioning.jobs:write" // UPDATE. + ActionProvisioningJobsRead = "provisioning.jobs:read" // GET + LIST. + ActionProvisioningJobsDelete = "provisioning.jobs:delete" // DELETE. + + // Historic Jobs + ActionProvisioningHistoricJobsRead = "provisioning.historicjobs:read" // GET + LIST. +) + +func registerAccessControlRoles(service accesscontrol.Service) error { + // Repositories + repositoriesReader := accesscontrol.RoleRegistration{ + Role: accesscontrol.RoleDTO{ + Name: "fixed:provisioning.repositories:reader", + DisplayName: "Repositories Reader", + Description: "Read and list provisioning repositories.", + Group: "Provisioning", + Permissions: []accesscontrol.Permission{ + { + Action: ActionProvisioningRepositoriesRead, + }, + }, + }, + Grants: []string{string(org.RoleAdmin)}, + } + + repositoriesWriter := accesscontrol.RoleRegistration{ + Role: accesscontrol.RoleDTO{ + Name: "fixed:provisioning.repositories:writer", + DisplayName: "Repositories Writer", + Description: "Create, update and delete provisioning repositories.", + Group: "Provisioning", + Permissions: []accesscontrol.Permission{ + { + Action: ActionProvisioningRepositoriesCreate, + }, + { + Action: ActionProvisioningRepositoriesRead, + }, + { + Action: ActionProvisioningRepositoriesWrite, + }, + { + Action: ActionProvisioningRepositoriesDelete, + }, + }, + }, + Grants: []string{string(org.RoleAdmin)}, + } + + // Jobs + jobsReader := accesscontrol.RoleRegistration{ + Role: accesscontrol.RoleDTO{ + Name: "fixed:provisioning.jobs:reader", + DisplayName: "Jobs Reader", + Description: "Read and list provisioning jobs.", + Group: "Provisioning", + Permissions: []accesscontrol.Permission{ + { + Action: ActionProvisioningJobsRead, + }, + }, + }, + Grants: []string{string(org.RoleAdmin)}, + } + + jobsWriter := accesscontrol.RoleRegistration{ + Role: accesscontrol.RoleDTO{ + Name: "fixed:provisioning.jobs:writer", + DisplayName: "Jobs Writer", + Description: "Create, update and delete provisioning jobs.", + Group: "Provisioning", + Permissions: []accesscontrol.Permission{ + { + Action: ActionProvisioningJobsCreate, + }, + { + Action: ActionProvisioningJobsRead, + }, + { + Action: ActionProvisioningJobsWrite, + }, + { + Action: ActionProvisioningJobsDelete, + }, + }, + }, + Grants: []string{string(org.RoleAdmin)}, + } + + // Historic Jobs + historicJobsReader := accesscontrol.RoleRegistration{ + Role: accesscontrol.RoleDTO{ + Name: "fixed:provisioning.historicjobs:reader", + DisplayName: "Historic Jobs Reader", + Description: "Read and list provisioning historic jobs.", + Group: "Provisioning", + Permissions: []accesscontrol.Permission{ + { + Action: ActionProvisioningHistoricJobsRead, + }, + }, + }, + Grants: []string{string(org.RoleAdmin)}, + } + + return service.DeclareFixedRoles( + repositoriesReader, + repositoriesWriter, + jobsReader, + jobsWriter, + historicJobsReader, + ) +} diff --git a/pkg/registry/apis/provisioning/dependencies.go b/pkg/registry/apis/provisioning/dependencies.go new file mode 100644 index 00000000000..f573fb59a80 --- /dev/null +++ b/pkg/registry/apis/provisioning/dependencies.go @@ -0,0 +1,24 @@ +package provisioning + +import ( + "fmt" + + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/setting" +) + +// DependencyRegisterer is set to satisfy wire gen and make sure the `RegisterDependencies` is called. +type DependencyRegisterer struct{} + +func RegisterDependencies( + cfg *setting.Cfg, + accessControlService accesscontrol.Service, + features featuremgmt.FeatureToggles, +) (*DependencyRegisterer, error) { + if err := registerAccessControlRoles(accessControlService); err != nil { + return nil, fmt.Errorf("registering access control roles: %w", err) + } + + return &DependencyRegisterer{}, nil +} diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 2f31c2b6aa8..9899ac2a0da 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -87,7 +87,8 @@ type APIBuilder struct { // onlyApiServer used to disable starting controllers for the standalone API server. // HACK:This will be removed once we have proper wire providers for the controllers. // TODO: Set this up in the standalone API server - onlyApiServer bool + onlyApiServer bool + useExclusivelyAccessCheckerForAuthz bool allowedTargets []provisioning.SyncTargetType allowImageRendering bool @@ -147,6 +148,7 @@ func NewAPIBuilder( minSyncInterval time.Duration, registry prometheus.Registerer, newStandaloneClientFactoryFunc func(loopbackConfigProvider apiserver.RestConfigProvider) resources.ClientFactory, // optional, only used for standalone apiserver + useExclusivelyAccessCheckerForAuthz bool, ) *APIBuilder { var clients resources.ClientFactory if newStandaloneClientFactoryFunc != nil { @@ -158,26 +160,27 @@ func NewAPIBuilder( resourceLister := resources.NewResourceListerForMigrations(unified, legacyMigrator, storageStatus) b := &APIBuilder{ - onlyApiServer: onlyApiServer, - tracer: tracer, - usageStats: usageStats, - features: features, - repoFactory: repoFactory, - clients: clients, - parsers: parsers, - repositoryResources: resources.NewRepositoryResourcesFactory(parsers, clients, resourceLister), - resourceLister: resourceLister, - legacyMigrator: legacyMigrator, - storageStatus: storageStatus, - unified: unified, - access: access, - jobHistoryConfig: jobHistoryConfig, - extraWorkers: extraWorkers, - restConfigGetter: restConfigGetter, - allowedTargets: allowedTargets, - allowImageRendering: allowImageRendering, - registry: registry, - validator: repository.NewValidator(minSyncInterval, allowedTargets, allowImageRendering), + onlyApiServer: onlyApiServer, + tracer: tracer, + usageStats: usageStats, + features: features, + repoFactory: repoFactory, + clients: clients, + parsers: parsers, + repositoryResources: resources.NewRepositoryResourcesFactory(parsers, clients, resourceLister), + resourceLister: resourceLister, + legacyMigrator: legacyMigrator, + storageStatus: storageStatus, + unified: unified, + access: access, + jobHistoryConfig: jobHistoryConfig, + extraWorkers: extraWorkers, + restConfigGetter: restConfigGetter, + allowedTargets: allowedTargets, + allowImageRendering: allowImageRendering, + registry: registry, + validator: repository.NewValidator(minSyncInterval, allowedTargets, allowImageRendering), + useExclusivelyAccessCheckerForAuthz: useExclusivelyAccessCheckerForAuthz, } for _, builder := range extraBuilders { @@ -267,6 +270,7 @@ func RegisterAPIService( cfg.ProvisioningMinSyncInterval, reg, nil, + false, // TODO: first, test this on the MT side before we enable it by default in ST as well ) apiregistration.RegisterAPI(builder) return builder, nil @@ -283,7 +287,9 @@ func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { } info, ok := authlib.AuthInfoFrom(ctx) - if ok && authlib.IsIdentityType(info.GetIdentityType(), authlib.TypeAccessPolicy) { + // when running as standalone API server, the identity type may not always match TypeAccessPolicy + // so we allow it to use the access checker if there is any auth info available + if ok && (authlib.IsIdentityType(info.GetIdentityType(), authlib.TypeAccessPolicy) || b.useExclusivelyAccessCheckerForAuthz) { res, err := b.access.Check(ctx, info, authlib.CheckRequest{ Verb: a.GetVerb(), Group: a.GetAPIGroup(), @@ -291,6 +297,7 @@ func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { Name: a.GetName(), Namespace: a.GetNamespace(), Subresource: a.GetSubresource(), + Path: a.GetPath(), }, "") if err != nil { return authorizer.DecisionDeny, "failed to perform authorization", err diff --git a/pkg/registry/apis/wireset.go b/pkg/registry/apis/wireset.go index b753747d478..20bc5c3bf1f 100644 --- a/pkg/registry/apis/wireset.go +++ b/pkg/registry/apis/wireset.go @@ -49,6 +49,7 @@ var WireSet = wire.NewSet( // Secrets secret.RegisterDependencies, // Provisioning + provisioning.RegisterDependencies, provisioningExtras, // Each must be added here *and* in the ServiceSink above diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index cc02de5c578..42ef5427c90 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -888,7 +888,11 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api if err != nil { return nil, err } - apiregistryService := apiregistry.ProvideRegistryServiceSink(dashboardsAPIBuilder, snapshotsAPIBuilder, dataSourceAPIBuilder, folderAPIBuilder, identityAccessManagementAPIBuilder, queryAPIBuilder, userStorageAPIBuilder, apiBuilder, provisioningAPIBuilder, ofrepAPIBuilder, dependencyRegisterer) + provisioningDependencyRegisterer, err := provisioning2.RegisterDependencies(cfg, acimplService, featureToggles) + if err != nil { + return nil, err + } + apiregistryService := apiregistry.ProvideRegistryServiceSink(dashboardsAPIBuilder, snapshotsAPIBuilder, dataSourceAPIBuilder, folderAPIBuilder, identityAccessManagementAPIBuilder, queryAPIBuilder, userStorageAPIBuilder, apiBuilder, provisioningAPIBuilder, ofrepAPIBuilder, dependencyRegisterer, provisioningDependencyRegisterer) teamPermissionsService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, featureToggles, routeRegisterImpl, sqlStore, accessControl, ossLicensingService, acimplService, teamService, userService, actionSetService) if err != nil { return nil, err @@ -1518,7 +1522,11 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac if err != nil { return nil, err } - apiregistryService := apiregistry.ProvideRegistryServiceSink(dashboardsAPIBuilder, snapshotsAPIBuilder, dataSourceAPIBuilder, folderAPIBuilder, identityAccessManagementAPIBuilder, queryAPIBuilder, userStorageAPIBuilder, apiBuilder, provisioningAPIBuilder, ofrepAPIBuilder, dependencyRegisterer) + provisioningDependencyRegisterer, err := provisioning2.RegisterDependencies(cfg, acimplService, featureToggles) + if err != nil { + return nil, err + } + apiregistryService := apiregistry.ProvideRegistryServiceSink(dashboardsAPIBuilder, snapshotsAPIBuilder, dataSourceAPIBuilder, folderAPIBuilder, identityAccessManagementAPIBuilder, queryAPIBuilder, userStorageAPIBuilder, apiBuilder, provisioningAPIBuilder, ofrepAPIBuilder, dependencyRegisterer, provisioningDependencyRegisterer) teamPermissionsService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, featureToggles, routeRegisterImpl, sqlStore, accessControl, ossLicensingService, acimplService, teamService, userService, actionSetService) if err != nil { return nil, err diff --git a/pkg/services/authz/rbac/mapper.go b/pkg/services/authz/rbac/mapper.go index 5ef0a252ed8..f2db9256d92 100644 --- a/pkg/services/authz/rbac/mapper.go +++ b/pkg/services/authz/rbac/mapper.go @@ -24,17 +24,18 @@ type Mapping interface { AllActions() []string // HasFolderSupport returns true if the translation supports folders. HasFolderSupport() bool - // SkipScopeOnCreate returns true if the translation does not require a scope on create. - SkipScopeOnCreate() bool + // SkipScope returns true if the translation does not require a scope for the given verb. + SkipScope(verb string) bool } type translation struct { - resource string - attribute string - verbMapping map[string]string - actionSetMapping map[string][]string - folderSupport bool - skipScopeOnCreate bool + resource string + attribute string + verbMapping map[string]string + actionSetMapping map[string][]string + folderSupport bool + // actions to skip scope on, e.g., create actions + skipScopeOnVerb map[string]bool // use this option if you need to limit access to users that can access all resources useWildcardScope bool } @@ -77,8 +78,11 @@ func (t translation) HasFolderSupport() bool { return t.folderSupport } -func (t translation) SkipScopeOnCreate() bool { - return t.skipScopeOnCreate +func (t translation) SkipScope(verb string) bool { + if t.skipScopeOnVerb != nil { + return t.skipScopeOnVerb[verb] + } + return false } // MapperRegistry is a registry of mappers that maps a group and resource to a translation. @@ -92,7 +96,7 @@ type MapperRegistry interface { type mapper map[string]map[string]translation -func newResourceTranslation(resource string, attribute string, folderSupport, skipScopeOnCreate bool) translation { +func newResourceTranslation(resource string, attribute string, folderSupport bool, skipScopeOnVerb map[string]bool) translation { defaultMapping := func(r string) map[string]string { return map[string]string{ utils.VerbGet: fmt.Sprintf("%s:read", r), @@ -109,17 +113,17 @@ func newResourceTranslation(resource string, attribute string, folderSupport, sk } return translation{ - resource: resource, - attribute: attribute, - verbMapping: defaultMapping(resource), - folderSupport: folderSupport, - skipScopeOnCreate: skipScopeOnCreate, + resource: resource, + attribute: attribute, + verbMapping: defaultMapping(resource), + folderSupport: folderSupport, + skipScopeOnVerb: skipScopeOnVerb, } } // newDashboardTranslation creates a translation for dashboards and also maps the actions to action sets func newDashboardTranslation() translation { - dashTranslation := newResourceTranslation("dashboards", "uid", true, false) + dashTranslation := newResourceTranslation("dashboards", "uid", true, nil) actionSetMapping := make(map[string][]string) for verb, rbacAction := range dashTranslation.verbMapping { @@ -152,7 +156,7 @@ func newDashboardTranslation() translation { // newFolderTranslation creates a translation for folders and also maps the actions to action sets func newFolderTranslation() translation { - folderTranslation := newResourceTranslation("folders", "uid", true, false) + folderTranslation := newResourceTranslation("folders", "uid", true, nil) actionSetMapping := make(map[string][]string) for verb, rbacAction := range folderTranslation.verbMapping { @@ -179,21 +183,33 @@ func newFolderTranslation() translation { } func NewMapperRegistry() MapperRegistry { + skipScopeOnAllVerbs := map[string]bool{ + utils.VerbCreate: true, + utils.VerbGet: true, + utils.VerbUpdate: true, + utils.VerbPatch: true, + utils.VerbDelete: true, + utils.VerbDeleteCollection: true, + utils.VerbList: true, + utils.VerbWatch: true, + utils.VerbGetPermissions: true, + utils.VerbSetPermissions: true, + } + mapper := mapper(map[string]map[string]translation{ "dashboard.grafana.app": { "dashboards": newDashboardTranslation(), - "librarypanels": newResourceTranslation("library.panels", "uid", true, false), + "librarypanels": newResourceTranslation("library.panels", "uid", true, nil), }, "folder.grafana.app": { "folders": newFolderTranslation(), }, "iam.grafana.app": { // Users is a special case. We translate user permissions from id to uid based. - "users": newResourceTranslation("users", "uid", false, true), - "serviceaccounts": newResourceTranslation("serviceaccounts", "uid", false, true), + "users": newResourceTranslation("users", "uid", false, map[string]bool{utils.VerbCreate: true}), + "serviceaccounts": newResourceTranslation("serviceaccounts", "uid", false, map[string]bool{utils.VerbCreate: true}), // Teams is a special case. We translate user permissions from id to uid based. - "teams": newResourceTranslation("teams", "uid", false, true), - // No need to skip scope on create for roles because we translate `permissions:type:delegate` to `roles:*`` + "teams": newResourceTranslation("teams", "uid", false, map[string]bool{utils.VerbCreate: true}), "coreroles": translation{ resource: "roles", attribute: "uid", @@ -202,8 +218,9 @@ func NewMapperRegistry() MapperRegistry { utils.VerbList: "roles:read", utils.VerbWatch: "roles:read", }, - folderSupport: false, - skipScopeOnCreate: false, + folderSupport: false, + // No need to skip scope on create for roles because we translate `permissions:type:delegate` to `roles:*`` + skipScopeOnVerb: nil, }, "roles": translation{ resource: "roles", @@ -218,8 +235,9 @@ func NewMapperRegistry() MapperRegistry { utils.VerbList: "roles:read", utils.VerbWatch: "roles:read", }, - folderSupport: false, - skipScopeOnCreate: false, + folderSupport: false, + // No need to skip scope on create for roles because we translate `permissions:type:delegate` to `roles:*`` + skipScopeOnVerb: nil, }, "rolebindings": translation{ resource: "rolebindings", @@ -238,9 +256,14 @@ func NewMapperRegistry() MapperRegistry { folderSupport: false, }, }, + "provisioning.grafana.app": { + "repositories": newResourceTranslation("provisioning.repositories", "uid", false, skipScopeOnAllVerbs), + "jobs": newResourceTranslation("provisioning.jobs", "uid", false, skipScopeOnAllVerbs), + "historicjobs": newResourceTranslation("provisioning.historicjobs", "uid", false, skipScopeOnAllVerbs), + }, "secret.grafana.app": { - "securevalues": newResourceTranslation("secret.securevalues", "uid", false, false), - "keepers": newResourceTranslation("secret.keepers", "uid", false, false), + "securevalues": newResourceTranslation("secret.securevalues", "uid", false, nil), + "keepers": newResourceTranslation("secret.keepers", "uid", false, nil), }, "query.grafana.app": { "query": translation{ @@ -249,8 +272,8 @@ func NewMapperRegistry() MapperRegistry { verbMapping: map[string]string{ utils.VerbCreate: "datasources:query", }, - folderSupport: false, - skipScopeOnCreate: false, + folderSupport: false, + skipScopeOnVerb: nil, }, }, }) diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index 6ba8e0a8838..e45346740cc 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -617,18 +617,28 @@ func (s *Service) checkPermission(ctx context.Context, scopeMap map[string]bool, return false, status.Error(codes.NotFound, "unsupported resource") } - if req.Verb == utils.VerbCreate { - // Resource doesn't require scope on create, so allow if the user has the action - if t.SkipScopeOnCreate() { - return scopeMap[""], nil - } - // If creating a resource that goes in a folder, but no folder is specified, - // assume parent folder is the general folder - if t.HasFolderSupport() && req.ParentFolder == "" { - req.ParentFolder = accesscontrol.GeneralFolderUID - } + if t.SkipScope(req.Verb) { + return scopeMap[""], nil } + // If creating a resource that goes in a folder, but no folder is specified, + // assume parent folder is the general folder + if req.Verb == utils.VerbCreate && t.HasFolderSupport() && req.ParentFolder == "" { + req.ParentFolder = accesscontrol.GeneralFolderUID + } + + //if req.Verb == utils.VerbCreate { + // // Resource doesn't require scope on create, so allow if the user has the action + // if t.SkipScopeOnCreate() { + // return scopeMap[""], nil + // } + // // If creating a resource that goes in a folder, but no folder is specified, + // // assume parent folder is the general folder + // if t.HasFolderSupport() && req.ParentFolder == "" { + // req.ParentFolder = accesscontrol.GeneralFolderUID + // } + //} + // Wildcard grant, no further checks needed if scopeMap["*"] { return true, nil