From cfa1a2c55feea58bd7f82e4e03df2ea712ef34a7 Mon Sep 17 00:00:00 2001 From: Ieva Date: Fri, 21 Jul 2023 15:23:01 +0100 Subject: [PATCH] RBAC: Split non-empty scopes into `kind`, `attribute` and `identifier` fields for better search performance (#71933) * add a feature toggle * add the fields for attribute, kind and identifier to permission Co-authored-by: Kalle Persson * set the new fields when new permissions are stored * add migrations Co-authored-by: Kalle Persson * remove comments * Update pkg/services/accesscontrol/migrator/migrator.go Co-authored-by: Gabriel MABILLE * feedback: put column migrations behind the feature toggle, added an index, changed how wildcard scopes are split * PR feedback: add a comment and revert an accidentally changed file * PR feedback: handle the case with : in resource identifier * switch from checking feature toggle through cfg to checking it through featuremgmt * don't put the column migrations behind a feature toggle after all - this breaks permission queries from db --------- Co-authored-by: Kalle Persson Co-authored-by: Gabriel MABILLE --- .../feature-toggles/index.md | 1 + .../src/types/featureToggles.gen.ts | 1 + pkg/api/folder_bench_test.go | 4 +- pkg/services/accesscontrol/acimpl/service.go | 15 ++++++- .../accesscontrol/acimpl/service_test.go | 8 +--- .../accesscontrol/database/database_test.go | 3 +- .../accesscontrol/migrator/migrator.go | 44 +++++++++++++++++++ pkg/services/accesscontrol/models.go | 21 +++++++++ pkg/services/accesscontrol/models_test.go | 30 +++++++++++++ .../ossaccesscontrol/permissions_services.go | 18 ++++---- .../resourcepermissions/service.go | 8 ++-- .../resourcepermissions/service_test.go | 3 +- .../resourcepermissions/store.go | 11 +++-- .../resourcepermissions/store_test.go | 3 +- pkg/services/featuremgmt/registry.go | 8 ++++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 ++ .../guardian/accesscontrol_guardian_test.go | 4 +- .../migrations/accesscontrol/migrations.go | 16 +++++++ .../api/alerting/api_backtesting_test.go | 2 +- pkg/tests/api/alerting/api_prometheus_test.go | 3 +- pkg/tests/api/alerting/api_ruler_test.go | 5 ++- pkg/tests/api/alerting/api_testing_test.go | 3 +- 23 files changed, 179 insertions(+), 37 deletions(-) create mode 100644 pkg/services/accesscontrol/migrator/migrator.go diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index ca6d1247c80..181189d4fdc 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -67,6 +67,7 @@ Some features are enabled by default. You can disable these feature by setting t | `enableDatagridEditing` | Enables the edit functionality in the datagrid panel | | `dataSourcePageHeader` | Apply new pageHeader UI in data source edit page | | `sqlDatasourceDatabaseSelection` | Enables previous SQL data source dataset dropdown behavior | +| `splitScopes` | Support faster dashboard and folder search by splitting permission scopes into parts | ## Experimental feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 867ee5df994..060f24d5441 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -114,4 +114,5 @@ export interface FeatureToggles { disableTraceQLStreaming?: boolean; grafanaAPIServer?: boolean; featureToggleAdminPage?: boolean; + splitScopes?: boolean; } diff --git a/pkg/api/folder_bench_test.go b/pkg/api/folder_bench_test.go index e91f05ec13f..4d3025804b8 100644 --- a/pkg/api/folder_bench_test.go +++ b/pkg/api/folder_bench_test.go @@ -425,10 +425,10 @@ func setupServer(b testing.TB, sc benchScenario, features *featuremgmt.FeatureMa folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), sc.cfg, dashStore, folderStore, sc.db, features) folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( - sc.cfg, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc) + features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc) require.NoError(b, err) dashboardPermissions, err := ossaccesscontrol.ProvideDashboardPermissions( - sc.cfg, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc) + features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc) require.NoError(b, err) dashboardSvc, err := dashboardservice.ProvideDashboardServiceImpl( diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index c0d6f4b7814..e0cad1791db 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -19,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/api" "github.com/grafana/grafana/pkg/services/accesscontrol/database" + "github.com/grafana/grafana/pkg/services/accesscontrol/migrator" "github.com/grafana/grafana/pkg/services/accesscontrol/pluginutils" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/user" @@ -31,15 +32,25 @@ const ( cacheTTL = 10 * time.Second ) -func ProvideService(cfg *setting.Cfg, store db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService, +func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService, accessControl accesscontrol.AccessControl, features *featuremgmt.FeatureManager) (*Service, error) { - service := ProvideOSSService(cfg, database.ProvideService(store), cache, features) + service := ProvideOSSService(cfg, database.ProvideService(db), cache, features) api.NewAccessControlAPI(routeRegister, accessControl, service, features).RegisterAPIEndpoints() if err := accesscontrol.DeclareFixedRoles(service, cfg); err != nil { return nil, err } + if cfg.IsFeatureToggleEnabled(featuremgmt.FlagSplitScopes) { + // Migrating scopes that haven't been split yet to have kind, attribute and identifier in the DB + // This will be removed once we've: + // 1) removed the feature toggle and + // 2) have released enough versions not to support a version without split scopes + if err := migrator.MigrateScopeSplit(db, service.log); err != nil { + return nil, err + } + } + return service, nil } diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index 51a1debc7a1..ff0ea62508d 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" @@ -57,15 +56,12 @@ func TestUsageMetrics(t *testing.T) { cfg := setting.NewCfg() cfg.RBACEnabled = tt.enabled - s, errInitAc := ProvideService( + s := ProvideOSSService( cfg, - db.InitTestDB(t), - routing.NewRouteRegister(), + database.ProvideService(db.InitTestDB(t)), localcache.ProvideService(), - actest.FakeAccessControl{}, featuremgmt.WithFeatures(), ) - require.NoError(t, errInitAc) assert.Equal(t, tt.expectedValue, s.GetUsageStats(context.Background())["stats.oss.accesscontrol.enabled.count"]) }) } diff --git a/pkg/services/accesscontrol/database/database_test.go b/pkg/services/accesscontrol/database/database_test.go index 45a81415a0d..567f13ea7b6 100644 --- a/pkg/services/accesscontrol/database/database_test.go +++ b/pkg/services/accesscontrol/database/database_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" rs "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgimpl" "github.com/grafana/grafana/pkg/services/quota/quotatest" @@ -314,7 +315,7 @@ func setupTestEnv(t testing.TB) (*AccessControlStore, rs.Store, user.Service, te cfg.AutoAssignOrgRole = "Viewer" cfg.AutoAssignOrgId = 1 acstore := ProvideService(sql) - permissionStore := rs.NewStore(sql) + permissionStore := rs.NewStore(sql, featuremgmt.WithFeatures()) teamService := teamimpl.ProvideService(sql, cfg) orgService, err := orgimpl.ProvideService(sql, cfg, quotatest.New(false, nil)) require.NoError(t, err) diff --git a/pkg/services/accesscontrol/migrator/migrator.go b/pkg/services/accesscontrol/migrator/migrator.go new file mode 100644 index 00000000000..b3f2c68fa7e --- /dev/null +++ b/pkg/services/accesscontrol/migrator/migrator.go @@ -0,0 +1,44 @@ +package migrator + +import ( + "context" + "time" + + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore" +) + +func MigrateScopeSplit(db db.DB, log log.Logger) error { + t := time.Now() + var count = 0 + err := db.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + var permissions []accesscontrol.Permission + + err := sess.SQL("SELECT * FROM permission WHERE NOT scope = '' AND identifier = ''").Find(&permissions) + if err != nil { + return err + } + + for i, p := range permissions { + count++ + kind, attribute, identifier := p.SplitScope() + + permissions[i].Kind = kind + permissions[i].Attribute = attribute + permissions[i].Identifier = identifier + + _, err := sess.Exec("UPDATE permission SET kind = ?, attribute = ?, identifier = ? WHERE id = ?", permissions[i].Kind, permissions[i].Attribute, permissions[i].Identifier, permissions[i].ID) + if err != nil { + return err + } + } + + return nil + }) + + log.Debug("Migrated permissions ", "count", count, "in", time.Since(t)) + + return err +} diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index fef6e5466a4..dcb8535d1d4 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -182,6 +182,10 @@ type Permission struct { Action string `json:"action"` Scope string `json:"scope"` + Kind string `json:"-"` + Attribute string `json:"-"` + Identifier string `json:"-"` + Updated time.Time `json:"updated"` Created time.Time `json:"created"` } @@ -193,6 +197,23 @@ func (p Permission) OSSPermission() Permission { } } +// SplitScope returns kind, attribute and Identifier +func (p Permission) SplitScope() (string, string, string) { + if p.Scope == "" { + return "", "", "" + } + + fragments := strings.Split(p.Scope, ":") + switch l := len(fragments); l { + case 1: // Splitting a wildcard scope "*" -> kind: "*"; attribute: "*"; identifier: "*" + return fragments[0], fragments[0], fragments[0] + case 2: // Splitting a wildcard scope with specified kind "dashboards:*" -> kind: "dashboards"; attribute: "*"; identifier: "*" + return fragments[0], fragments[1], fragments[1] + default: // Splitting a scope with all fields specified "dashboards:uid:my_dash" -> kind: "dashboards"; attribute: "uid"; identifier: "my_dash" + return fragments[0], fragments[1], strings.Join(fragments[2:], ":") + } +} + type GetUserPermissionsQuery struct { OrgID int64 UserID int64 diff --git a/pkg/services/accesscontrol/models_test.go b/pkg/services/accesscontrol/models_test.go index 03e94295ce1..fa15762535b 100644 --- a/pkg/services/accesscontrol/models_test.go +++ b/pkg/services/accesscontrol/models_test.go @@ -3,6 +3,7 @@ package accesscontrol import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -108,3 +109,32 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) { }) } } + +func TestPermission_ScopeSplit(t *testing.T) { + type testCase struct { + desc string + scope string + kind string + attribute string + identifier string + } + + tests := []testCase{ + {desc: "all fields should be empty for empty scope", scope: "", kind: "", attribute: "", identifier: ""}, + {desc: "all fields should be set to * for wildcard", scope: "*", kind: "*", attribute: "*", identifier: "*"}, + {desc: "kind should be specified and attribute and identifier should be * for a wildcard with kind prefix", scope: "dashboards:*", kind: "dashboards", attribute: "*", identifier: "*"}, + {desc: "all fields should be set correctly", scope: "dashboards:uid:123", kind: "dashboards", attribute: "uid", identifier: "123"}, + {desc: "can handle a case with : in the uid", scope: "datasources:uid:weird:name", kind: "datasources", attribute: "uid", identifier: "weird:name"}, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + p := Permission{Scope: tt.scope} + + kind, attribute, identifier := p.SplitScope() + assert.Equal(t, tt.kind, kind) + assert.Equal(t, tt.attribute, attribute) + assert.Equal(t, tt.identifier, identifier) + }) + } +} diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index 79b075b1366..4da58168c98 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/serviceaccounts" @@ -18,7 +19,6 @@ import ( "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/team/teamimpl" "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/setting" ) type TeamPermissionsService struct { @@ -40,7 +40,7 @@ var ( ) func ProvideTeamPermissions( - cfg *setting.Cfg, router routing.RouteRegister, sql db.DB, + features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, license licensing.Licensing, service accesscontrol.Service, teamService team.Service, userService user.Service, ) (*TeamPermissionsService, error) { @@ -98,7 +98,7 @@ func ProvideTeamPermissions( }, } - srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql, teamService, userService) + srv, err := resourcepermissions.New(options, features, router, license, ac, service, sql, teamService, userService) if err != nil { return nil, err } @@ -114,7 +114,7 @@ var DashboardEditActions = append(DashboardViewActions, []string{dashboards.Acti var DashboardAdminActions = append(DashboardEditActions, []string{dashboards.ActionDashboardsPermissionsRead, dashboards.ActionDashboardsPermissionsWrite}...) func ProvideDashboardPermissions( - cfg *setting.Cfg, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, + features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, license licensing.Licensing, dashboardStore dashboards.Store, folderService folder.Service, service accesscontrol.Service, teamService team.Service, userService user.Service, ) (*DashboardPermissionsService, error) { @@ -178,7 +178,7 @@ func ProvideDashboardPermissions( RoleGroup: "Dashboards", } - srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql, teamService, userService) + srv, err := resourcepermissions.New(options, features, router, license, ac, service, sql, teamService, userService) if err != nil { return nil, err } @@ -201,7 +201,7 @@ var FolderEditActions = append(FolderViewActions, []string{ var FolderAdminActions = append(FolderEditActions, []string{dashboards.ActionFoldersPermissionsRead, dashboards.ActionFoldersPermissionsWrite}...) func ProvideFolderPermissions( - cfg *setting.Cfg, router routing.RouteRegister, sql db.DB, accesscontrol accesscontrol.AccessControl, + features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, accesscontrol accesscontrol.AccessControl, license licensing.Licensing, dashboardStore dashboards.Store, folderService folder.Service, service accesscontrol.Service, teamService team.Service, userService user.Service, ) (*FolderPermissionsService, error) { @@ -238,7 +238,7 @@ func ProvideFolderPermissions( WriterRoleName: "Folder permission writer", RoleGroup: "Folders", } - srv, err := resourcepermissions.New(options, cfg, router, license, accesscontrol, service, sql, teamService, userService) + srv, err := resourcepermissions.New(options, features, router, license, accesscontrol, service, sql, teamService, userService) if err != nil { return nil, err } @@ -301,7 +301,7 @@ type ServiceAccountPermissionsService struct { } func ProvideServiceAccountPermissions( - cfg *setting.Cfg, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, + features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, license licensing.Licensing, serviceAccountRetrieverService *retriever.Service, service accesscontrol.Service, teamService team.Service, userService user.Service, ) (*ServiceAccountPermissionsService, error) { @@ -330,7 +330,7 @@ func ProvideServiceAccountPermissions( RoleGroup: "Service accounts", } - srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql, teamService, userService) + srv, err := resourcepermissions.New(options, features, router, license, ac, service, sql, teamService, userService) if err != nil { return nil, err } diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index 4dd6503ddc7..8f80c461450 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -8,11 +8,11 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/setting" ) type Store interface { @@ -52,7 +52,7 @@ type Store interface { } func New( - options Options, cfg *setting.Cfg, router routing.RouteRegister, license licensing.Licensing, + options Options, features featuremgmt.FeatureToggles, router routing.RouteRegister, license licensing.Licensing, ac accesscontrol.AccessControl, service accesscontrol.Service, sqlStore db.DB, teamService team.Service, userService user.Service, ) (*Service, error) { @@ -77,8 +77,7 @@ func New( s := &Service{ ac: ac, - cfg: cfg, - store: NewStore(sqlStore), + store: NewStore(sqlStore, features), options: options, license: license, permissions: permissions, @@ -102,7 +101,6 @@ func New( // Service is used to create access control sub system including api / and service for managed resource permission type Service struct { - cfg *setting.Cfg ac accesscontrol.AccessControl service accesscontrol.Service store Store diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index 512e07e70c2..dbf040b4cb6 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/service_test.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing/licensingtest" "github.com/grafana/grafana/pkg/services/org/orgimpl" "github.com/grafana/grafana/pkg/services/quota/quotatest" @@ -244,7 +245,7 @@ func setupTestEnvironment(t *testing.T, ops Options) (*Service, *sqlstore.SQLSto ac := acimpl.ProvideAccessControl(cfg) acService := &actest.FakeService{} service, err := New( - ops, cfg, routing.NewRouteRegister(), license, + ops, featuremgmt.WithFeatures(), routing.NewRouteRegister(), license, ac, acService, sql, teamSvc, userSvc, ) require.NoError(t, err) diff --git a/pkg/services/accesscontrol/resourcepermissions/store.go b/pkg/services/accesscontrol/resourcepermissions/store.go index 7f7784c5f02..ae7cced19d6 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store.go +++ b/pkg/services/accesscontrol/resourcepermissions/store.go @@ -8,18 +8,20 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/util" ) -func NewStore(sql db.DB) *store { - return &store{sql} +func NewStore(sql db.DB, features featuremgmt.FeatureToggles) *store { + return &store{sql, features} } type store struct { - sql db.DB + sql db.DB + features featuremgmt.FeatureToggles } type flatResourcePermission struct { @@ -648,6 +650,9 @@ func (s *store) createPermissions(sess *db.Session, roleID int64, resource, reso p.RoleID = roleID p.Created = time.Now() p.Updated = time.Now() + if s.features.IsEnabled(featuremgmt.FlagSplitScopes) { + p.Kind, p.Attribute, p.Identifier = p.SplitScope() + } permissions = append(permissions, p) } diff --git a/pkg/services/accesscontrol/resourcepermissions/store_test.go b/pkg/services/accesscontrol/resourcepermissions/store_test.go index d0447303d0b..50ed2015c6d 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/store_test.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgimpl" "github.com/grafana/grafana/pkg/services/quota/quotatest" @@ -520,7 +521,7 @@ func seedResourcePermissions(t *testing.T, store *store, sql *sqlstore.SQLStore, func setupTestEnv(t testing.TB) (*store, *sqlstore.SQLStore) { sql := db.InitTestDB(t) - return NewStore(sql), sql + return NewStore(sql, featuremgmt.WithFeatures()), sql } func TestStore_IsInherited(t *testing.T) { diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 925d716e8cd..9b828ea5024 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -657,5 +657,13 @@ var ( Owner: grafanaOperatorExperienceSquad, RequiresRestart: true, }, + { + Name: "splitScopes", + Description: "Support faster dashboard and folder search by splitting permission scopes into parts", + Stage: FeatureStagePublicPreview, + FrontendOnly: false, + Owner: grafanaAuthnzSquad, + RequiresRestart: true, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index a1caf4da325..8ca9a481c06 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -95,3 +95,4 @@ mlExpressions,experimental,@grafana/alerting-squad,false,false,false,false disableTraceQLStreaming,experimental,@grafana/observability-traces-and-profiling,false,false,false,true grafanaAPIServer,experimental,@grafana/grafana-app-platform-squad,false,false,false,false featureToggleAdminPage,experimental,@grafana/grafana-operator-experience-squad,false,false,true,false +splitScopes,preview,@grafana/grafana-authnz-team,false,false,true,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 3840d333f57..555fc2dd0f1 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -390,4 +390,8 @@ const ( // FlagFeatureToggleAdminPage // Enable admin page for managing feature toggles from the Grafana front-end FlagFeatureToggleAdminPage = "featureToggleAdminPage" + + // FlagSplitScopes + // Support faster dashboard and folder search by splitting permission scopes into parts + FlagSplitScopes = "splitScopes" ) diff --git a/pkg/services/guardian/accesscontrol_guardian_test.go b/pkg/services/guardian/accesscontrol_guardian_test.go index bb4f914c3bb..886180ec78d 100644 --- a/pkg/services/guardian/accesscontrol_guardian_test.go +++ b/pkg/services/guardian/accesscontrol_guardian_test.go @@ -629,10 +629,10 @@ func setupAccessControlGuardianTest(t *testing.T, uid string, require.NoError(t, err) folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( - cfg, routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, foldertest.NewFakeService(), ac, teamSvc, userSvc) + featuremgmt.WithFeatures(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, foldertest.NewFakeService(), ac, teamSvc, userSvc) require.NoError(t, err) dashboardPermissions, err := ossaccesscontrol.ProvideDashboardPermissions( - cfg, routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, foldertest.NewFakeService(), ac, teamSvc, userSvc) + featuremgmt.WithFeatures(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, foldertest.NewFakeService(), ac, teamSvc, userSvc) require.NoError(t, err) g, err := NewAccessControlDashboardGuardian(context.Background(), cfg, dash.ID, &user.SignedInUser{OrgID: 1}, store, ac, folderPermissions, dashboardPermissions, dashboardSvc) diff --git a/pkg/services/sqlstore/migrations/accesscontrol/migrations.go b/pkg/services/sqlstore/migrations/accesscontrol/migrations.go index 2b8a0b47944..a7819085f71 100644 --- a/pkg/services/sqlstore/migrations/accesscontrol/migrations.go +++ b/pkg/services/sqlstore/migrations/accesscontrol/migrations.go @@ -170,4 +170,20 @@ func AddMigration(mg *migrator.Migrator) { mg.AddMigration("add column hidden to role table", migrator.NewAddColumnMigration(roleV1, &migrator.Column{ Name: "hidden", Type: migrator.DB_Bool, Nullable: false, Default: "0", })) + + mg.AddMigration("permission kind migration", migrator.NewAddColumnMigration(permissionV1, &migrator.Column{ + Name: "kind", Type: migrator.DB_NVarchar, Length: 40, Default: "''", + })) + + mg.AddMigration("permission attribute migration", migrator.NewAddColumnMigration(permissionV1, &migrator.Column{ + Name: "attribute", Type: migrator.DB_NVarchar, Length: 40, Default: "''", + })) + + mg.AddMigration("permission identifier migration", migrator.NewAddColumnMigration(permissionV1, &migrator.Column{ + Name: "identifier", Type: migrator.DB_NVarchar, Length: 40, Default: "''", + })) + + mg.AddMigration("add permission identifier index", migrator.NewAddIndexMigration(permissionV1, &migrator.Index{ + Cols: []string{"identifier"}, + })) } diff --git a/pkg/tests/api/alerting/api_backtesting_test.go b/pkg/tests/api/alerting/api_backtesting_test.go index f54dbf2933c..286245fa500 100644 --- a/pkg/tests/api/alerting/api_backtesting_test.go +++ b/pkg/tests/api/alerting/api_backtesting_test.go @@ -111,7 +111,7 @@ func TestBacktesting(t *testing.T) { }) // access control permissions store - permissionsStore := resourcepermissions.NewStore(env.SQLStore) + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures()) _, err := permissionsStore.SetUserResourcePermission(context.Background(), accesscontrol.GlobalOrgID, accesscontrol.User{ID: testUserId}, diff --git a/pkg/tests/api/alerting/api_prometheus_test.go b/pkg/tests/api/alerting/api_prometheus_test.go index 1616d338265..cf301d5ee96 100644 --- a/pkg/tests/api/alerting/api_prometheus_test.go +++ b/pkg/tests/api/alerting/api_prometheus_test.go @@ -18,6 +18,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" + "github.com/grafana/grafana/pkg/services/featuremgmt" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/org" @@ -663,7 +664,7 @@ func TestIntegrationPrometheusRulesPermissions(t *testing.T) { apiClient := newAlertingApiClient(grafanaListedAddr, "grafana", "password") // access control permissions store - permissionsStore := resourcepermissions.NewStore(store) + permissionsStore := resourcepermissions.NewStore(store, featuremgmt.WithFeatures()) // Create the namespace we'll save our alerts to. apiClient.CreateFolder(t, "folder1", "folder1") diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index e4c492d07bb..2fdf91bc572 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/datasources" datasourceService "github.com/grafana/grafana/pkg/services/datasources/service" + "github.com/grafana/grafana/pkg/services/featuremgmt" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/org" @@ -41,7 +42,7 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { }) grafanaListedAddr, store := testinfra.StartGrafana(t, dir, path) - permissionsStore := resourcepermissions.NewStore(store) + permissionsStore := resourcepermissions.NewStore(store, featuremgmt.WithFeatures()) // Create a user to make authenticated requests userID := createUser(t, store, user.CreateUserCommand{ @@ -868,7 +869,7 @@ func TestIntegrationRuleUpdate(t *testing.T) { AppModeProduction: true, }) grafanaListedAddr, store := testinfra.StartGrafana(t, dir, path) - permissionsStore := resourcepermissions.NewStore(store) + permissionsStore := resourcepermissions.NewStore(store, featuremgmt.WithFeatures()) // Create a user to make authenticated requests userID := createUser(t, store, user.CreateUserCommand{ diff --git a/pkg/tests/api/alerting/api_testing_test.go b/pkg/tests/api/alerting/api_testing_test.go index 1f434ad6ca6..eee30ad69b8 100644 --- a/pkg/tests/api/alerting/api_testing_test.go +++ b/pkg/tests/api/alerting/api_testing_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/featuremgmt" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/org" @@ -267,7 +268,7 @@ func TestGrafanaRuleConfig(t *testing.T) { }) // access control permissions store - permissionsStore := resourcepermissions.NewStore(env.SQLStore) + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures()) _, err := permissionsStore.SetUserResourcePermission(context.Background(), accesscontrol.GlobalOrgID, accesscontrol.User{ID: testUserId},