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 <kalle.persson@grafana.com> * set the new fields when new permissions are stored * add migrations Co-authored-by: Kalle Persson <kalle.persson@grafana.com> * remove comments * Update pkg/services/accesscontrol/migrator/migrator.go Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com> * 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 <kalle.persson@grafana.com> Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
This commit is contained in:
co-authored by
Kalle Persson
Gabriel MABILLE
parent
59eb2f68b7
commit
cfa1a2c55f
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user