Annotations: Fix usage of dashboard table for permissions (#98774)

This commit is contained in:
Stephanie Hingtgen 2025-01-10 07:35:38 -07:00 committed by GitHub
parent 86e8147df3
commit 215eb31c7a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 107 additions and 61 deletions

View File

@ -7,6 +7,7 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
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/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/sqlstore/permissions"
@ -29,12 +30,14 @@ var (
type AuthService struct {
db db.DB
features featuremgmt.FeatureToggles
dashSvc dashboards.DashboardService
}
func NewAuthService(db db.DB, features featuremgmt.FeatureToggles) *AuthService {
func NewAuthService(db db.DB, features featuremgmt.FeatureToggles, dashSvc dashboards.DashboardService) *AuthService {
return &AuthService{
db: db,
features: features,
dashSvc: dashSvc,
}
}
@ -133,26 +136,21 @@ func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context,
})
}
sb := &searchstore.Builder{Dialect: authz.db.GetDialect(), Filters: filters, Features: authz.features}
// This is a limit for a batch size, not for the end query result.
var limit int64 = 1000
if query.Page == 0 {
query.Page = 1
}
sql, params := sb.ToSQL(limit, query.Page)
visibleDashboards := make(map[string]int64)
var res []dashboardProjection
err = authz.db.WithDbSession(ctx, func(sess *db.Session) error {
return sess.SQL(sql, params...).Find(&res)
dashs, err := authz.dashSvc.SearchDashboards(ctx, &dashboards.FindPersistedDashboardsQuery{
OrgId: query.SignedInUser.GetOrgID(),
Filters: filters,
SignedInUser: query.SignedInUser,
Page: query.Page,
Type: filterType,
Limit: 1000,
})
if err != nil {
return nil, err
}
for _, p := range res {
visibleDashboards[p.UID] = p.ID
visibleDashboards := make(map[string]int64)
for _, d := range dashs {
visibleDashboards[d.UID] = d.ID
}
return visibleDashboards, nil

View File

@ -7,13 +7,25 @@ import (
"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/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/annotations"
"github.com/grafana/grafana/pkg/services/annotations/testutil"
"github.com/grafana/grafana/pkg/services/authz/zanzana"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/database"
dashboardsservice "github.com/grafana/grafana/pkg/services/dashboards/service"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"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/supportbundles/supportbundlestest"
"github.com/grafana/grafana/pkg/services/tag/tagimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/tests/testsuite"
)
@ -28,27 +40,46 @@ func TestIntegrationAuthorize(t *testing.T) {
}
sql, cfg := db.InitTestDBWithCfg(t)
dash1 := testutil.CreateDashboard(t, sql, cfg, featuremgmt.WithFeatures(), dashboards.SaveDashboardCommand{
UserID: 1,
OrgID: 1,
Dashboard: simplejson.NewFromAny(map[string]any{
"title": "Dashboard 1",
}),
})
dash2 := testutil.CreateDashboard(t, sql, cfg, featuremgmt.WithFeatures(), dashboards.SaveDashboardCommand{
UserID: 1,
OrgID: 1,
Dashboard: simplejson.NewFromAny(map[string]any{
"title": "Dashboard 2",
}),
})
origNewDashboardGuardian := guardian.New
defer func() { guardian.New = origNewDashboardGuardian }()
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
folderStore := folderimpl.ProvideDashboardFolderStore(sql)
fStore := folderimpl.ProvideStore(sql)
dashStore, err := database.ProvideDashboardStore(sql, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sql))
require.NoError(t, err)
ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient())
folderSvc := folderimpl.ProvideService(fStore, accesscontrolmock.New(), bus.ProvideBus(tracing.InitializeTracerForTest()),
dashStore, folderStore, sql, featuremgmt.WithFeatures(),
supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())
dashSvc, err := dashboardsservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, featuremgmt.WithFeatures(), accesscontrolmock.NewMockedPermissionsService(), accesscontrolmock.NewMockedPermissionsService(),
ac, folderSvc, fStore, nil, zanzana.NewNoopClient(), nil, nil, nil, quotatest.New(false, nil), nil)
require.NoError(t, err)
u := &user.SignedInUser{
UserID: 1,
OrgID: 1,
}
dash1, err := dashSvc.SaveDashboard(context.Background(), &dashboards.SaveDashboardDTO{
User: u,
OrgID: 1,
Dashboard: &dashboards.Dashboard{
Title: "Dashboard 1",
Data: simplejson.New(),
},
}, false)
require.NoError(t, err)
dash2, err := dashSvc.SaveDashboard(context.Background(), &dashboards.SaveDashboardDTO{
User: u,
OrgID: 1,
Dashboard: &dashboards.Dashboard{
Title: "Dashboard 2",
Data: simplejson.New(),
},
}, false)
require.NoError(t, err)
role := testutil.SetupRBACRole(t, sql, u)
type testCase struct {
@ -172,8 +203,7 @@ func TestIntegrationAuthorize(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
u.Permissions = map[int64]map[string][]string{1: tc.permissions}
testutil.SetupRBACPermission(t, sql, role, u)
authz := NewAuthService(sql, featuremgmt.WithFeatures(tc.featureToggle))
authz := NewAuthService(sql, featuremgmt.WithFeatures(tc.featureToggle), dashSvc)
query := annotations.ItemQuery{SignedInUser: u, OrgID: 1}
resources, err := authz.Authorize(context.Background(), query)

View File

@ -11,8 +11,3 @@ type AccessResources struct {
// Skip filtering
SkipAccessControlFilter bool
}
type dashboardProjection struct {
ID int64 `xorm:"id"`
UID string `xorm:"uid"`
}

View File

@ -5,6 +5,7 @@ import (
"github.com/grafana/grafana/pkg/services/annotations/accesscontrol"
"github.com/grafana/grafana/pkg/services/annotations/annotationsimpl/loki"
"github.com/grafana/grafana/pkg/services/dashboards"
alertingStore "github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/infra/db"
@ -31,6 +32,7 @@ func ProvideService(
tagService tag.Service,
tracer tracing.Tracer,
ruleStore *alertingStore.DBstore,
dashSvc dashboards.DashboardService,
) *RepositoryImpl {
l := log.New("annotations")
l.Debug("Initializing annotations service")
@ -51,7 +53,7 @@ func ProvideService(
return &RepositoryImpl{
db: db,
features: features,
authZ: accesscontrol.NewAuthService(db, features),
authZ: accesscontrol.NewAuthService(db, features, dashSvc),
reader: read,
writer: write,
}

View File

@ -16,16 +16,19 @@ import (
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/annotations"
"github.com/grafana/grafana/pkg/services/annotations/testutil"
"github.com/grafana/grafana/pkg/services/authz/zanzana"
"github.com/grafana/grafana/pkg/services/dashboards"
dashboardstore "github.com/grafana/grafana/pkg/services/dashboards/database"
"github.com/grafana/grafana/pkg/services/dashboards/database"
dashboardsservice "github.com/grafana/grafana/pkg/services/dashboards/service"
"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"
alertingStore "github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
"github.com/grafana/grafana/pkg/services/tag/tagimpl"
"github.com/grafana/grafana/pkg/services/user"
@ -49,8 +52,22 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) {
features := featuremgmt.WithFeatures()
tagService := tagimpl.ProvideService(sql)
ruleStore := alertingStore.SetupStoreForTesting(t, sql)
origNewDashboardGuardian := guardian.New
defer func() { guardian.New = origNewDashboardGuardian }()
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{})
folderStore := folderimpl.ProvideDashboardFolderStore(sql)
fStore := folderimpl.ProvideStore(sql)
dashStore, err := database.ProvideDashboardStore(sql, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sql))
require.NoError(t, err)
ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient())
folderSvc := folderimpl.ProvideService(fStore, accesscontrolmock.New(), bus.ProvideBus(tracing.InitializeTracerForTest()),
dashStore, folderStore, sql, featuremgmt.WithFeatures(),
supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())
dashSvc, err := dashboardsservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, featuremgmt.WithFeatures(), accesscontrolmock.NewMockedPermissionsService(), accesscontrolmock.NewMockedPermissionsService(),
ac, folderSvc, fStore, nil, zanzana.NewNoopClient(), nil, nil, nil, quotatest.New(false, nil), nil)
require.NoError(t, err)
repo := ProvideService(sql, cfg, features, tagService, tracing.InitializeTracerForTest(), ruleStore)
repo := ProvideService(sql, cfg, features, tagService, tracing.InitializeTracerForTest(), ruleStore, dashSvc)
dashboard1 := testutil.CreateDashboard(t, sql, cfg, features, dashboards.SaveDashboardCommand{
UserID: 1,
@ -70,8 +87,6 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) {
}),
})
var err error
dash1Annotation := &annotations.Item{
OrgID: 1,
DashboardID: 1,
@ -208,7 +223,7 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) {
allDashboards := make([]dashInfo, 0, folder.MaxNestedFolderDepth+1)
annotationsTexts := make([]string, 0, folder.MaxNestedFolderDepth+1)
setupFolderStructure := func() db.DB {
setupFolderStructure := func() (db.DB, dashboards.DashboardService) {
sql, cfg := db.InitTestDBWithCfg(t)
// enable nested folders so that the folder table is populated for all the tests
@ -216,7 +231,7 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) {
tagService := tagimpl.ProvideService(sql)
dashStore, err := dashboardstore.ProvideDashboardStore(sql, cfg, features, tagService)
dashStore, err := database.ProvideDashboardStore(sql, cfg, features, tagService)
require.NoError(t, err)
origNewGuardian := guardian.New
@ -227,8 +242,12 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) {
ac := acimpl.ProvideAccessControl(features, zanzana.NewNoopClient())
fStore := folderimpl.ProvideStore(sql)
folderStore := folderimpl.ProvideDashboardFolderStore(sql)
folderSvc := folderimpl.ProvideService(fStore, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore,
folderimpl.ProvideDashboardFolderStore(sql), sql, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())
folderStore, sql, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())
dashSvc, err := dashboardsservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, features, accesscontrolmock.NewMockedPermissionsService(), accesscontrolmock.NewMockedPermissionsService(),
ac, folderSvc, fStore, nil, zanzana.NewNoopClient(), nil, nil, nil, quotatest.New(false, nil), nil)
require.NoError(t, err)
cfg.AnnotationMaximumTagsLength = 60
@ -253,16 +272,18 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) {
t.Fail()
}
dashboard := testutil.CreateDashboard(t, sql, cfg, features, dashboards.SaveDashboardCommand{
UserID: usr.UserID,
dashboard, err := dashSvc.SaveDashboard(context.Background(), &dashboards.SaveDashboardDTO{
User: usr,
OrgID: orgID,
Dashboard: &dashboards.Dashboard{
IsFolder: false,
Dashboard: simplejson.NewFromAny(map[string]any{
"title": fmt.Sprintf("Dashboard under %s", f.UID),
}),
Title: fmt.Sprintf("Dashboard under %s", f.UID),
Data: simplejson.New(),
FolderID: f.ID, // nolint:staticcheck
FolderUID: f.UID,
})
},
}, false)
require.NoError(t, err)
allDashboards = append(allDashboards, dashInfo{UID: dashboard.UID, ID: dashboard.ID})
@ -282,10 +303,10 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) {
}
role = testutil.SetupRBACRole(t, sql, usr)
return sql
return sql, dashSvc
}
sql := setupFolderStructure()
sql, dashSvc := setupFolderStructure()
testCases := []struct {
desc string
@ -319,7 +340,7 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) {
cfg := setting.NewCfg()
cfg.AnnotationMaximumTagsLength = 60
ruleStore := alertingStore.SetupStoreForTesting(t, sql)
repo := ProvideService(sql, cfg, tc.features, tagimpl.ProvideService(sql), tracing.InitializeTracerForTest(), ruleStore)
repo := ProvideService(sql, cfg, tc.features, tagimpl.ProvideService(sql), tracing.InitializeTracerForTest(), ruleStore, dashSvc)
usr.Permissions = map[int64]map[string][]string{1: tc.permissions}
testutil.SetupRBACPermission(t, sql, role, usr)

View File

@ -35,7 +35,7 @@ func newPublicDashboardServiceImpl(
}
tagService := tagimpl.ProvideService(store)
if annotationsRepo == nil {
annotationsRepo = annotationsimpl.ProvideService(store, cfg, featuremgmt.WithFeatures(), tagService, tracing.InitializeTracerForTest(), nil)
annotationsRepo = annotationsimpl.ProvideService(store, cfg, featuremgmt.WithFeatures(), tagService, tracing.InitializeTracerForTest(), nil, dashboardService)
}
if publicDashboardStore == nil {