Search v1: Add support for inherited folder permissions if nested folders are enabled (#63275)
* Add features dependency to SQLBuilder * Add features dependency to AccessControlDashboardPermissionFilter * Add test for folder inheritance * Dashboard permissions: Return recursive query * Recursive query for inherited folders * Modify search builder * Adjust db.SQLBuilder * Pass flag to SQLbuilder if CTEs are supported * Add support for mysql < 8.0 * Add benchmarking for search with nested folders * Set features to AlertStore * Update pkg/infra/db/sqlbuilder.go Co-authored-by: Ieva <ieva.vasiljeva@grafana.com> * Set features to LibraryElementService * SQLBuilder tests with nested folder flag set * Apply suggestion from code review Co-authored-by: IevaVasiljeva <ieva.vasiljeva@grafana.com> Co-authored-by: Emil Tullstedt <emil.tullstedt@grafana.com>
This commit is contained in:
co-authored by
Ieva
Emil Tullstedt
parent
2648fcb833
commit
988a120d6d
@@ -6,6 +6,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/services/annotations"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/services/tag"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
@@ -14,10 +15,11 @@ type RepositoryImpl struct {
|
||||
store store
|
||||
}
|
||||
|
||||
func ProvideService(db db.DB, cfg *setting.Cfg, tagService tag.Service) *RepositoryImpl {
|
||||
func ProvideService(db db.DB, cfg *setting.Cfg, features featuremgmt.FeatureToggles, tagService tag.Service) *RepositoryImpl {
|
||||
return &RepositoryImpl{
|
||||
store: &xormRepositoryImpl{
|
||||
cfg: cfg,
|
||||
features: features,
|
||||
db: db,
|
||||
log: log.New("annotations"),
|
||||
tagService: tagService,
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
|
||||
@@ -13,12 +14,13 @@ type CleanupServiceImpl struct {
|
||||
store store
|
||||
}
|
||||
|
||||
func ProvideCleanupService(db db.DB, cfg *setting.Cfg) *CleanupServiceImpl {
|
||||
func ProvideCleanupService(db db.DB, cfg *setting.Cfg, features featuremgmt.FeatureToggles) *CleanupServiceImpl {
|
||||
return &CleanupServiceImpl{
|
||||
store: &xormRepositoryImpl{
|
||||
cfg: cfg,
|
||||
db: db,
|
||||
log: log.New("annotations"),
|
||||
cfg: cfg,
|
||||
features: features,
|
||||
db: db,
|
||||
log: log.New("annotations"),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/services/annotations"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
|
||||
@@ -91,7 +92,7 @@ func TestAnnotationCleanUp(t *testing.T) {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
cfg := setting.NewCfg()
|
||||
cfg.AnnotationCleanupJobBatchSize = 1
|
||||
cleaner := ProvideCleanupService(fakeSQL, cfg)
|
||||
cleaner := ProvideCleanupService(fakeSQL, cfg, featuremgmt.WithFeatures())
|
||||
affectedAnnotations, affectedAnnotationTags, err := cleaner.Run(context.Background(), test.cfg)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -146,7 +147,7 @@ func TestOldAnnotationsAreDeletedFirst(t *testing.T) {
|
||||
// run the clean up task to keep one annotation.
|
||||
cfg := setting.NewCfg()
|
||||
cfg.AnnotationCleanupJobBatchSize = 1
|
||||
cleaner := &xormRepositoryImpl{cfg: cfg, log: log.New("test-logger"), db: fakeSQL}
|
||||
cleaner := &xormRepositoryImpl{cfg: cfg, log: log.New("test-logger"), db: fakeSQL, features: featuremgmt.WithFeatures()}
|
||||
_, err = cleaner.CleanAnnotations(context.Background(), setting.AnnotationCleanupSettings{MaxCount: 1}, alertAnnotationType)
|
||||
require.NoError(t, err)
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/annotations"
|
||||
"github.com/grafana/grafana/pkg/services/dashboards"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore/permissions"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore/searchstore"
|
||||
@@ -42,6 +43,7 @@ func validateTimeRange(item *annotations.Item) error {
|
||||
|
||||
type xormRepositoryImpl struct {
|
||||
cfg *setting.Cfg
|
||||
features featuremgmt.FeatureToggles
|
||||
db db.DB
|
||||
log log.Logger
|
||||
maximumTagsLength int64
|
||||
@@ -299,13 +301,15 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQue
|
||||
}
|
||||
}
|
||||
|
||||
var acFilter acFilter
|
||||
if !ac.IsDisabled(r.cfg) {
|
||||
acFilter, acArgs, err := getAccessControlFilter(query.SignedInUser)
|
||||
var err error
|
||||
acFilter, err = r.getAccessControlFilter(query.SignedInUser)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
sql.WriteString(fmt.Sprintf(" AND (%s)", acFilter))
|
||||
params = append(params, acArgs...)
|
||||
sql.WriteString(fmt.Sprintf(" AND (%s)", acFilter.where))
|
||||
params = append(params, acFilter.whereParams...)
|
||||
}
|
||||
|
||||
if query.Limit == 0 {
|
||||
@@ -314,6 +318,14 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQue
|
||||
|
||||
// order of ORDER BY arguments match the order of a sql index for performance
|
||||
sql.WriteString(" ORDER BY a.org_id, a.epoch_end DESC, a.epoch DESC" + r.db.GetDialect().Limit(query.Limit) + " ) dt on dt.id = annotation.id")
|
||||
if acFilter.recQueries != "" {
|
||||
var sb bytes.Buffer
|
||||
sb.WriteString(acFilter.recQueries)
|
||||
sb.WriteString(sql.String())
|
||||
sql = sb
|
||||
params = append(acFilter.recParams, params...)
|
||||
}
|
||||
|
||||
if err := sess.SQL(sql.String(), params...).Find(&items); err != nil {
|
||||
items = nil
|
||||
return err
|
||||
@@ -325,13 +337,23 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQue
|
||||
return items, err
|
||||
}
|
||||
|
||||
func getAccessControlFilter(user *user.SignedInUser) (string, []interface{}, error) {
|
||||
type acFilter struct {
|
||||
where string
|
||||
whereParams []interface{}
|
||||
recQueries string
|
||||
recParams []interface{}
|
||||
}
|
||||
|
||||
func (r *xormRepositoryImpl) getAccessControlFilter(user *user.SignedInUser) (acFilter, error) {
|
||||
var recQueries string
|
||||
var recQueriesParams []interface{}
|
||||
|
||||
if user == nil || user.Permissions[user.OrgID] == nil {
|
||||
return "", nil, errors.New("missing permissions")
|
||||
return acFilter{}, errors.New("missing permissions")
|
||||
}
|
||||
scopes, has := user.Permissions[user.OrgID][ac.ActionAnnotationsRead]
|
||||
if !has {
|
||||
return "", nil, errors.New("missing permissions")
|
||||
return acFilter{}, errors.New("missing permissions")
|
||||
}
|
||||
types, hasWildcardScope := ac.ParseScopes(ac.ScopeAnnotationsProvider.GetResourceScopeType(""), scopes)
|
||||
if hasWildcardScope {
|
||||
@@ -347,13 +369,27 @@ func getAccessControlFilter(user *user.SignedInUser) (string, []interface{}, err
|
||||
}
|
||||
// annotation read permission with scope annotations:type:dashboard allows listing annotations from dashboards which the user can view
|
||||
if t == annotations.Dashboard.String() {
|
||||
dashboardFilter, dashboardParams := permissions.NewAccessControlDashboardPermissionFilter(user, dashboards.PERMISSION_VIEW, searchstore.TypeDashboard).Where()
|
||||
recursiveQueriesAreSupported, err := r.db.RecursiveQueriesAreSupported()
|
||||
if err != nil {
|
||||
return acFilter{}, err
|
||||
}
|
||||
|
||||
filterRBAC := permissions.NewAccessControlDashboardPermissionFilter(user, dashboards.PERMISSION_VIEW, searchstore.TypeDashboard, r.features, recursiveQueriesAreSupported)
|
||||
dashboardFilter, dashboardParams := filterRBAC.Where()
|
||||
recQueries, recQueriesParams = filterRBAC.With()
|
||||
filter := fmt.Sprintf("a.dashboard_id IN(SELECT id FROM dashboard WHERE %s)", dashboardFilter)
|
||||
filters = append(filters, filter)
|
||||
params = dashboardParams
|
||||
}
|
||||
}
|
||||
return strings.Join(filters, " OR "), params, nil
|
||||
|
||||
f := acFilter{
|
||||
where: strings.Join(filters, " OR "),
|
||||
whereParams: params,
|
||||
recQueries: recQueries,
|
||||
recParams: recQueriesParams,
|
||||
}
|
||||
return f, nil
|
||||
}
|
||||
|
||||
func (r *xormRepositoryImpl) Delete(ctx context.Context, params *annotations.DeleteParams) error {
|
||||
|
||||
@@ -2,6 +2,7 @@ package annotationsimpl
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -10,14 +11,20 @@ import (
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/grafana/grafana/pkg/bus"
|
||||
"github.com/grafana/grafana/pkg/components/simplejson"
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/infra/tracing"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol/mock"
|
||||
"github.com/grafana/grafana/pkg/services/annotations"
|
||||
"github.com/grafana/grafana/pkg/services/dashboards"
|
||||
dashboardstore "github.com/grafana/grafana/pkg/services/dashboards/database"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/services/folder"
|
||||
"github.com/grafana/grafana/pkg/services/folder/folderimpl"
|
||||
"github.com/grafana/grafana/pkg/services/guardian"
|
||||
"github.com/grafana/grafana/pkg/services/quota/quotatest"
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore"
|
||||
"github.com/grafana/grafana/pkg/services/tag/tagimpl"
|
||||
@@ -495,7 +502,7 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) {
|
||||
sql := db.InitTestDB(t)
|
||||
|
||||
var maximumTagsLength int64 = 60
|
||||
repo := xormRepositoryImpl{db: sql, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(sql, sql.Cfg), maximumTagsLength: maximumTagsLength}
|
||||
repo := xormRepositoryImpl{db: sql, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(sql, sql.Cfg), maximumTagsLength: maximumTagsLength, features: featuremgmt.WithFeatures()}
|
||||
quotaService := quotatest.New(false, nil)
|
||||
dashboardStore, err := dashboardstore.ProvideDashboardStore(sql, sql.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sql, sql.Cfg), quotaService)
|
||||
require.NoError(t, err)
|
||||
@@ -550,7 +557,7 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) {
|
||||
UserID: 1,
|
||||
OrgID: 1,
|
||||
}
|
||||
role := setupRBACRole(t, repo, user)
|
||||
role := setupRBACRole(t, sql, user)
|
||||
|
||||
type testStruct struct {
|
||||
description string
|
||||
@@ -611,7 +618,7 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) {
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.description, func(t *testing.T) {
|
||||
user.Permissions = map[int64]map[string][]string{1: tc.permissions}
|
||||
setupRBACPermission(t, repo, role, user)
|
||||
setupRBACPermission(t, sql, role, user)
|
||||
|
||||
results, err := repo.Get(context.Background(), &annotations.ItemQuery{
|
||||
OrgID: 1,
|
||||
@@ -630,10 +637,163 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func setupRBACRole(t *testing.T, repo xormRepositoryImpl, user *user.SignedInUser) *accesscontrol.Role {
|
||||
func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("skipping integration test")
|
||||
}
|
||||
|
||||
orgID := int64(1)
|
||||
permissions := []accesscontrol.Permission{
|
||||
{
|
||||
Action: dashboards.ActionFoldersCreate,
|
||||
}, {
|
||||
Action: dashboards.ActionFoldersWrite,
|
||||
Scope: dashboards.ScopeFoldersAll,
|
||||
},
|
||||
}
|
||||
usr := &user.SignedInUser{
|
||||
UserID: 1,
|
||||
OrgID: orgID,
|
||||
Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(permissions)},
|
||||
}
|
||||
|
||||
var role *accesscontrol.Role
|
||||
|
||||
dashboardIDs := make([]int64, 0, folder.MaxNestedFolderDepth+1)
|
||||
annotationsTexts := make([]string, 0, folder.MaxNestedFolderDepth+1)
|
||||
|
||||
setupFolderStructure := func() *sqlstore.SQLStore {
|
||||
db := db.InitTestDB(t)
|
||||
|
||||
// enable nested folders so that the folder table is populated for all the tests
|
||||
features := featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)
|
||||
|
||||
origNewGuardian := guardian.New
|
||||
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: true, CanSaveValue: true})
|
||||
t.Cleanup(func() {
|
||||
guardian.New = origNewGuardian
|
||||
})
|
||||
|
||||
// dashboard store commands that should be called.
|
||||
dashStore, err := dashboardstore.ProvideDashboardStore(db, db.Cfg, features, tagimpl.ProvideService(db, db.Cfg), quotatest.New(false, nil))
|
||||
require.NoError(t, err)
|
||||
|
||||
folderSvc := folderimpl.ProvideService(mock.New(), bus.ProvideBus(tracing.InitializeTracerForTest()), db.Cfg, dashStore, folderimpl.ProvideDashboardFolderStore(db), db, features)
|
||||
|
||||
var maximumTagsLength int64 = 60
|
||||
repo := xormRepositoryImpl{db: db, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(db, db.Cfg), maximumTagsLength: maximumTagsLength, features: features}
|
||||
|
||||
parentUID := ""
|
||||
for i := 0; ; i++ {
|
||||
uid := fmt.Sprintf("f%d", i)
|
||||
f, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
|
||||
UID: uid,
|
||||
OrgID: orgID,
|
||||
Title: uid,
|
||||
SignedInUser: usr,
|
||||
ParentUID: parentUID,
|
||||
})
|
||||
if err != nil {
|
||||
if errors.Is(err, folder.ErrMaximumDepthReached) {
|
||||
break
|
||||
}
|
||||
|
||||
t.Log("unexpected error", "error", err)
|
||||
t.Fail()
|
||||
}
|
||||
|
||||
dashInFolder := dashboards.SaveDashboardCommand{
|
||||
UserID: usr.UserID,
|
||||
OrgID: orgID,
|
||||
IsFolder: false,
|
||||
Dashboard: simplejson.NewFromAny(map[string]interface{}{
|
||||
"title": fmt.Sprintf("Dashboard under %s", f.UID),
|
||||
}),
|
||||
FolderID: f.ID,
|
||||
FolderUID: f.UID,
|
||||
}
|
||||
dashboard, err := dashStore.SaveDashboard(context.Background(), dashInFolder)
|
||||
require.NoError(t, err)
|
||||
|
||||
dashboardIDs = append(dashboardIDs, dashboard.ID)
|
||||
|
||||
parentUID = f.UID
|
||||
|
||||
annotationTxt := fmt.Sprintf("annotation %d", i)
|
||||
dash1Annotation := &annotations.Item{
|
||||
OrgID: orgID,
|
||||
DashboardID: dashboard.ID,
|
||||
Epoch: 10,
|
||||
Text: annotationTxt,
|
||||
}
|
||||
err = repo.Add(context.Background(), dash1Annotation)
|
||||
require.NoError(t, err)
|
||||
|
||||
annotationsTexts = append(annotationsTexts, annotationTxt)
|
||||
}
|
||||
|
||||
role = setupRBACRole(t, db, usr)
|
||||
return db
|
||||
}
|
||||
|
||||
db := setupFolderStructure()
|
||||
|
||||
testCases := []struct {
|
||||
desc string
|
||||
features featuremgmt.FeatureToggles
|
||||
permissions map[string][]string
|
||||
expectedAnnotationText []string
|
||||
expectedError bool
|
||||
}{
|
||||
{
|
||||
desc: "Should find only annotations from dashboards under folders that user can read",
|
||||
features: featuremgmt.WithFeatures(),
|
||||
permissions: map[string][]string{
|
||||
accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard},
|
||||
dashboards.ActionDashboardsRead: {"folders:uid:f0"},
|
||||
},
|
||||
expectedAnnotationText: annotationsTexts[:1],
|
||||
},
|
||||
{
|
||||
desc: "Should find only annotations from dashboards under inherited folders if nested folder are enabled",
|
||||
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
|
||||
permissions: map[string][]string{
|
||||
accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard},
|
||||
dashboards.ActionDashboardsRead: {"folders:uid:f0"},
|
||||
},
|
||||
expectedAnnotationText: annotationsTexts[:],
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.desc, func(t *testing.T) {
|
||||
var maximumTagsLength int64 = 60
|
||||
repo := xormRepositoryImpl{db: db, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(db, db.Cfg), maximumTagsLength: maximumTagsLength, features: tc.features}
|
||||
|
||||
usr.Permissions = map[int64]map[string][]string{1: tc.permissions}
|
||||
setupRBACPermission(t, db, role, usr)
|
||||
|
||||
results, err := repo.Get(context.Background(), &annotations.ItemQuery{
|
||||
OrgID: 1,
|
||||
SignedInUser: usr,
|
||||
})
|
||||
if tc.expectedError {
|
||||
require.Error(t, err)
|
||||
return
|
||||
}
|
||||
require.NoError(t, err)
|
||||
require.Len(t, results, len(tc.expectedAnnotationText))
|
||||
for _, r := range results {
|
||||
assert.Contains(t, tc.expectedAnnotationText, r.Text)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func setupRBACRole(t *testing.T, db *sqlstore.SQLStore, user *user.SignedInUser) *accesscontrol.Role {
|
||||
t.Helper()
|
||||
var role *accesscontrol.Role
|
||||
err := repo.db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
|
||||
err := db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
|
||||
role = &accesscontrol.Role{
|
||||
OrgID: user.OrgID,
|
||||
UID: "test_role",
|
||||
@@ -662,9 +822,9 @@ func setupRBACRole(t *testing.T, repo xormRepositoryImpl, user *user.SignedInUse
|
||||
return role
|
||||
}
|
||||
|
||||
func setupRBACPermission(t *testing.T, repo xormRepositoryImpl, role *accesscontrol.Role, user *user.SignedInUser) {
|
||||
func setupRBACPermission(t *testing.T, db *sqlstore.SQLStore, role *accesscontrol.Role, user *user.SignedInUser) {
|
||||
t.Helper()
|
||||
err := repo.db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
|
||||
err := db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
|
||||
if _, err := sess.Exec("DELETE FROM permission WHERE role_id = ?", role.ID); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user