diff --git a/pkg/services/annotations/accesscontrol/accesscontrol.go b/pkg/services/annotations/accesscontrol/accesscontrol.go new file mode 100644 index 00000000000..96092254820 --- /dev/null +++ b/pkg/services/annotations/accesscontrol/accesscontrol.go @@ -0,0 +1,125 @@ +package accesscontrol + +import ( + "context" + + "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/auth/identity" + "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/sqlstore/permissions" + "github.com/grafana/grafana/pkg/services/sqlstore/searchstore" + "github.com/grafana/grafana/pkg/util/errutil" +) + +var ( + ErrReadForbidden = errutil.NewBase( + errutil.StatusForbidden, + "annotations.accesscontrol.read", + errutil.WithPublicMessage("User missing permissions"), + ) + ErrAccessControlInternal = errutil.NewBase( + errutil.StatusInternal, + "annotations.accesscontrol.internal", + errutil.WithPublicMessage("Internal error while checking permissions"), + ) +) + +type AuthService struct { + db db.DB + features featuremgmt.FeatureToggles +} + +func NewAuthService(db db.DB, features featuremgmt.FeatureToggles) *AuthService { + return &AuthService{ + db: db, + features: features, + } +} + +// Authorize checks if the user has permission to read annotations, then returns a struct containing dashboards and scope types that the user has access to. +func (authz *AuthService) Authorize(ctx context.Context, orgID int64, user identity.Requester) (*AccessResources, error) { + if user == nil || user.IsNil() { + return nil, ErrReadForbidden.Errorf("missing user") + } + + scopes, has := user.GetPermissions()[ac.ActionAnnotationsRead] + if !has { + return nil, ErrReadForbidden.Errorf("user does not have permission to read annotations") + } + + scopeTypes := annotationScopeTypes(scopes) + + var visibleDashboards map[string]int64 + var err error + if _, ok := scopeTypes[annotations.Dashboard.String()]; ok { + visibleDashboards, err = authz.userVisibleDashboards(ctx, user, orgID) + if err != nil { + return nil, ErrAccessControlInternal.Errorf("failed to fetch dashboards: %w", err) + } + } + + return &AccessResources{ + Dashboards: visibleDashboards, + ScopeTypes: scopeTypes, + }, nil +} + +func (authz *AuthService) userVisibleDashboards(ctx context.Context, user identity.Requester, orgID int64) (map[string]int64, error) { + recursiveQueriesSupported, err := authz.db.RecursiveQueriesAreSupported() + if err != nil { + return nil, err + } + + filters := []any{ + permissions.NewAccessControlDashboardPermissionFilter(user, dashboards.PERMISSION_VIEW, searchstore.TypeDashboard, authz.features, recursiveQueriesSupported), + searchstore.OrgFilter{OrgId: orgID}, + } + + sb := &searchstore.Builder{Dialect: authz.db.GetDialect(), Filters: filters, Features: authz.features} + + visibleDashboards := make(map[string]int64) + + var page int64 = 1 + var limit int64 = 1000 + for { + var res []dashboardProjection + sql, params := sb.ToSQL(limit, page) + + err = authz.db.WithDbSession(ctx, func(sess *db.Session) error { + return sess.SQL(sql, params...).Find(&res) + }) + if err != nil { + return nil, err + } + + for _, p := range res { + visibleDashboards[p.UID] = p.ID + } + + // if the result is less than the limit, we have reached the end + if len(res) < int(limit) { + break + } + + page++ + } + + return visibleDashboards, nil +} + +func annotationScopeTypes(scopes []string) map[any]struct{} { + allScopeTypes := map[any]struct{}{ + annotations.Dashboard.String(): {}, + annotations.Organization.String(): {}, + } + + types, hasWildcardScope := ac.ParseScopes(ac.ScopeAnnotationsProvider.GetResourceScopeType(""), scopes) + if hasWildcardScope { + types = allScopeTypes + } + + return types +} diff --git a/pkg/services/annotations/accesscontrol/accesscontrol_test.go b/pkg/services/annotations/accesscontrol/accesscontrol_test.go new file mode 100644 index 00000000000..0fe70183e9f --- /dev/null +++ b/pkg/services/annotations/accesscontrol/accesscontrol_test.go @@ -0,0 +1,130 @@ +package accesscontrol + +import ( + "context" + "fmt" + "testing" + + "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/annotations" + "github.com/grafana/grafana/pkg/services/annotations/testutil" + "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/user" + "github.com/stretchr/testify/require" +) + +var ( + dashScopeType = annotations.Dashboard.String() + orgScopeType = annotations.Organization.String() +) + +func TestIntegrationAuthorize(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + sql := db.InitTestDB(t) + + authz := NewAuthService(sql, featuremgmt.WithFeatures()) + + dash1 := testutil.CreateDashboard(t, sql, featuremgmt.WithFeatures(), dashboards.SaveDashboardCommand{ + UserID: 1, + OrgID: 1, + Dashboard: simplejson.NewFromAny(map[string]any{ + "title": "Dashboard 1", + }), + }) + + dash2 := testutil.CreateDashboard(t, sql, featuremgmt.WithFeatures(), dashboards.SaveDashboardCommand{ + UserID: 1, + OrgID: 1, + Dashboard: simplejson.NewFromAny(map[string]any{ + "title": "Dashboard 2", + }), + }) + + u := &user.SignedInUser{ + UserID: 1, + OrgID: 1, + } + role := testutil.SetupRBACRole(t, sql, u) + + type testCase struct { + name string + permissions map[string][]string + expectedResources *AccessResources + expectedErr error + } + + testCases := []testCase{ + { + name: "should have both scopes and all dashboards", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsAll}, + dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, + }, + expectedResources: &AccessResources{ + Dashboards: map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID}, + ScopeTypes: map[any]struct{}{dashScopeType: {}, orgScopeType: {}}, + }, + }, + { + name: "should have only organization scope and no dashboards", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeOrganization}, + dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, + }, + expectedResources: &AccessResources{ + Dashboards: nil, + ScopeTypes: map[any]struct{}{orgScopeType: {}}, + }, + }, + { + name: "should have only dashboard scope and all dashboards", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard}, + dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, + }, + expectedResources: &AccessResources{ + Dashboards: map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + }, + }, + { + name: "should have only dashboard scope and only dashboard 1", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard}, + dashboards.ActionDashboardsRead: {fmt.Sprintf("dashboards:uid:%s", dash1.UID)}, + }, + expectedResources: &AccessResources{ + Dashboards: map[string]int64{dash1.UID: dash1.ID}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + u.Permissions = map[int64]map[string][]string{1: tc.permissions} + testutil.SetupRBACPermission(t, sql, role, u) + + resources, err := authz.Authorize(context.Background(), 1, u) + require.NoError(t, err) + + if tc.expectedResources.Dashboards != nil { + require.Equal(t, tc.expectedResources.Dashboards, resources.Dashboards) + } + + if tc.expectedResources.ScopeTypes != nil { + require.Equal(t, tc.expectedResources.ScopeTypes, resources.ScopeTypes) + } + + if tc.expectedErr != nil { + require.Equal(t, tc.expectedErr, err) + } + }) + } +} diff --git a/pkg/services/annotations/accesscontrol/models.go b/pkg/services/annotations/accesscontrol/models.go new file mode 100644 index 00000000000..e316beb0a1b --- /dev/null +++ b/pkg/services/annotations/accesscontrol/models.go @@ -0,0 +1,14 @@ +package accesscontrol + +// AccessResources contains resources that are used to filter annotations based on RBAC. +type AccessResources struct { + // Dashboards is a map of dashboard UIDs to IDs + Dashboards map[string]int64 + // ScopeTypes contains the scope types that the user has access to. At most `dashboard` and `organization` + ScopeTypes map[any]struct{} +} + +type dashboardProjection struct { + ID int64 `xorm:"id"` + UID string `xorm:"uid"` +} diff --git a/pkg/services/annotations/annotationsimpl/annotations.go b/pkg/services/annotations/annotationsimpl/annotations.go index 9929984db1f..db1c745be66 100644 --- a/pkg/services/annotations/annotationsimpl/annotations.go +++ b/pkg/services/annotations/annotationsimpl/annotations.go @@ -3,6 +3,8 @@ package annotationsimpl import ( "context" + "github.com/grafana/grafana/pkg/services/annotations/accesscontrol" + "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/annotations" @@ -12,19 +14,25 @@ import ( ) type RepositoryImpl struct { - store store + db db.DB + authZ *accesscontrol.AuthService + features featuremgmt.FeatureToggles + store store } -func ProvideService(db db.DB, cfg *setting.Cfg, features featuremgmt.FeatureToggles, tagService tag.Service) *RepositoryImpl { +func ProvideService( + db db.DB, + cfg *setting.Cfg, + features featuremgmt.FeatureToggles, + tagService tag.Service, +) *RepositoryImpl { + l := log.New("annotations") + return &RepositoryImpl{ - store: &xormRepositoryImpl{ - cfg: cfg, - features: features, - db: db, - log: log.New("annotations"), - tagService: tagService, - maximumTagsLength: cfg.AnnotationMaximumTagsLength, - }, + db: db, + features: features, + authZ: accesscontrol.NewAuthService(db, features), + store: NewXormStore(cfg, l, db, tagService), } } @@ -43,7 +51,12 @@ func (r *RepositoryImpl) Update(ctx context.Context, item *annotations.Item) err } func (r *RepositoryImpl) Find(ctx context.Context, query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) { - return r.store.Get(ctx, query) + resources, err := r.authZ.Authorize(ctx, query.OrgID, query.SignedInUser) + if err != nil { + return make([]*annotations.ItemDTO, 0), err + } + + return r.store.Get(ctx, query, resources) } func (r *RepositoryImpl) Delete(ctx context.Context, params *annotations.DeleteParams) error { diff --git a/pkg/services/annotations/annotationsimpl/annotations_test.go b/pkg/services/annotations/annotationsimpl/annotations_test.go new file mode 100644 index 00000000000..ed35b13d051 --- /dev/null +++ b/pkg/services/annotations/annotationsimpl/annotations_test.go @@ -0,0 +1,335 @@ +package annotationsimpl + +import ( + "context" + "errors" + "fmt" + "testing" + + "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/acimpl" + "github.com/grafana/grafana/pkg/services/annotations" + "github.com/grafana/grafana/pkg/services/annotations/testutil" + "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" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIntegrationAnnotationListingWithRBAC(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + sql := db.InitTestDB(t) + + cfg := setting.NewCfg() + cfg.AnnotationMaximumTagsLength = 60 + + features := featuremgmt.WithFeatures() + tagService := tagimpl.ProvideService(sql) + + repo := ProvideService(sql, cfg, features, tagService) + + dashboard1 := testutil.CreateDashboard(t, sql, features, dashboards.SaveDashboardCommand{ + UserID: 1, + OrgID: 1, + IsFolder: false, + Dashboard: simplejson.NewFromAny(map[string]any{ + "title": "Dashboard 1", + }), + }) + + _ = testutil.CreateDashboard(t, sql, features, dashboards.SaveDashboardCommand{ + UserID: 1, + OrgID: 1, + IsFolder: false, + Dashboard: simplejson.NewFromAny(map[string]any{ + "title": "Dashboard 2", + }), + }) + + var err error + + dash1Annotation := &annotations.Item{ + OrgID: 1, + DashboardID: 1, + Epoch: 10, + } + err = repo.Save(context.Background(), dash1Annotation) + require.NoError(t, err) + + dash2Annotation := &annotations.Item{ + OrgID: 1, + DashboardID: 2, + Epoch: 10, + Tags: []string{"foo:bar"}, + } + err = repo.Save(context.Background(), dash2Annotation) + require.NoError(t, err) + + organizationAnnotation := &annotations.Item{ + OrgID: 1, + Epoch: 10, + } + err = repo.Save(context.Background(), organizationAnnotation) + require.NoError(t, err) + + u := &user.SignedInUser{ + UserID: 1, + OrgID: 1, + } + role := testutil.SetupRBACRole(t, sql, u) + + type testStruct struct { + description string + permissions map[string][]string + expectedAnnotationIds []int64 + expectedError bool + } + + testCases := []testStruct{ + { + description: "Should find all annotations when has permissions to list all annotations and read all dashboards", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsAll}, + dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, + }, + expectedAnnotationIds: []int64{dash1Annotation.ID, dash2Annotation.ID, organizationAnnotation.ID}, + }, + { + description: "Should find all dashboard annotations", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard}, + dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, + }, + expectedAnnotationIds: []int64{dash1Annotation.ID, dash2Annotation.ID}, + }, + { + description: "Should find only annotations from dashboards that user can read", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard}, + dashboards.ActionDashboardsRead: {fmt.Sprintf("dashboards:uid:%s", dashboard1.UID)}, + }, + expectedAnnotationIds: []int64{dash1Annotation.ID}, + }, + { + description: "Should find no annotations if user can't view dashboards or organization annotations", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard}, + }, + expectedAnnotationIds: []int64{}, + }, + { + description: "Should find only organization annotations", + permissions: map[string][]string{ + accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeOrganization}, + dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, + }, + expectedAnnotationIds: []int64{organizationAnnotation.ID}, + }, + { + description: "Should error if user doesn't have annotation read permissions", + permissions: map[string][]string{ + dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, + }, + expectedError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + u.Permissions = map[int64]map[string][]string{1: tc.permissions} + testutil.SetupRBACPermission(t, sql, role, u) + + results, err := repo.Find(context.Background(), &annotations.ItemQuery{ + OrgID: 1, + SignedInUser: u, + }) + if tc.expectedError { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Len(t, results, len(tc.expectedAnnotationIds)) + for _, r := range results { + assert.Contains(t, tc.expectedAnnotationIds, r.ID) + } + }) + } +} + +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 + + type dashInfo struct { + UID string + ID int64 + } + + allDashboards := make([]dashInfo, 0, folder.MaxNestedFolderDepth+1) + annotationsTexts := make([]string, 0, folder.MaxNestedFolderDepth+1) + + setupFolderStructure := func() *sqlstore.SQLStore { + sql := db.InitTestDB(t) + + // enable nested folders so that the folder table is populated for all the tests + features := featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders) + + tagService := tagimpl.ProvideService(sql) + + dashStore, err := dashboardstore.ProvideDashboardStore(sql, sql.Cfg, features, tagService, quotatest.New(false, nil)) + require.NoError(t, err) + + origNewGuardian := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: true, CanSaveValue: true}) + t.Cleanup(func() { + guardian.New = origNewGuardian + }) + + ac := acimpl.ProvideAccessControl(sql.Cfg) + folderSvc := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), sql.Cfg, dashStore, folderimpl.ProvideDashboardFolderStore(sql), sql, features) + + cfg := setting.NewCfg() + cfg.AnnotationMaximumTagsLength = 60 + + store := NewXormStore(cfg, log.New("annotation.test"), sql, tagService) + + 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() + } + + dashboard := testutil.CreateDashboard(t, sql, features, dashboards.SaveDashboardCommand{ + UserID: usr.UserID, + OrgID: orgID, + IsFolder: false, + Dashboard: simplejson.NewFromAny(map[string]any{ + "title": fmt.Sprintf("Dashboard under %s", f.UID), + }), + FolderID: f.ID, + FolderUID: f.UID, + }) + + allDashboards = append(allDashboards, dashInfo{UID: dashboard.UID, ID: dashboard.ID}) + + parentUID = f.UID + + annotationTxt := fmt.Sprintf("annotation %d", i) + dash1Annotation := &annotations.Item{ + OrgID: orgID, + DashboardID: dashboard.ID, + Epoch: 10, + Text: annotationTxt, + } + err = store.Add(context.Background(), dash1Annotation) + require.NoError(t, err) + + annotationsTexts = append(annotationsTexts, annotationTxt) + } + + role = testutil.SetupRBACRole(t, sql, usr) + return sql + } + + sql := 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) { + cfg := setting.NewCfg() + cfg.AnnotationMaximumTagsLength = 60 + + repo := ProvideService(sql, cfg, tc.features, tagimpl.ProvideService(sql)) + + usr.Permissions = map[int64]map[string][]string{1: tc.permissions} + testutil.SetupRBACPermission(t, sql, role, usr) + + results, err := repo.Find(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) + } + }) + } +} diff --git a/pkg/services/annotations/annotationsimpl/cleanup.go b/pkg/services/annotations/annotationsimpl/cleanup.go index a3380d9b4df..b81597122f0 100644 --- a/pkg/services/annotations/annotationsimpl/cleanup.go +++ b/pkg/services/annotations/annotationsimpl/cleanup.go @@ -5,7 +5,6 @@ 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" ) @@ -14,14 +13,9 @@ type CleanupServiceImpl struct { store store } -func ProvideCleanupService(db db.DB, cfg *setting.Cfg, features featuremgmt.FeatureToggles) *CleanupServiceImpl { +func ProvideCleanupService(db db.DB, cfg *setting.Cfg) *CleanupServiceImpl { return &CleanupServiceImpl{ - store: &xormRepositoryImpl{ - cfg: cfg, - features: features, - db: db, - log: log.New("annotations"), - }, + store: NewXormStore(cfg, log.New("annotations"), db, nil), } } diff --git a/pkg/services/annotations/annotationsimpl/cleanup_test.go b/pkg/services/annotations/annotationsimpl/cleanup_test.go index b072224d446..3b94ef8fdd5 100644 --- a/pkg/services/annotations/annotationsimpl/cleanup_test.go +++ b/pkg/services/annotations/annotationsimpl/cleanup_test.go @@ -11,7 +11,6 @@ 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" ) @@ -92,7 +91,7 @@ func TestAnnotationCleanUp(t *testing.T) { t.Run(test.name, func(t *testing.T) { cfg := setting.NewCfg() cfg.AnnotationCleanupJobBatchSize = 1 - cleaner := ProvideCleanupService(fakeSQL, cfg, featuremgmt.WithFeatures()) + cleaner := ProvideCleanupService(fakeSQL, cfg) affectedAnnotations, affectedAnnotationTags, err := cleaner.Run(context.Background(), test.cfg) require.NoError(t, err) @@ -147,7 +146,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, features: featuremgmt.WithFeatures()} + cleaner := NewXormStore(cfg, log.New("annotation.test"), fakeSQL, nil) _, err = cleaner.CleanAnnotations(context.Background(), setting.AnnotationCleanupSettings{MaxCount: 1}, alertAnnotationType) require.NoError(t, err) diff --git a/pkg/services/annotations/annotationsimpl/store.go b/pkg/services/annotations/annotationsimpl/store.go index 935afa85bd9..d367bf1e64e 100644 --- a/pkg/services/annotations/annotationsimpl/store.go +++ b/pkg/services/annotations/annotationsimpl/store.go @@ -3,6 +3,8 @@ package annotationsimpl import ( "context" + "github.com/grafana/grafana/pkg/services/annotations/accesscontrol" + "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/setting" ) @@ -11,7 +13,7 @@ type store interface { Add(ctx context.Context, items *annotations.Item) error AddMany(ctx context.Context, items []annotations.Item) error Update(ctx context.Context, item *annotations.Item) error - Get(ctx context.Context, query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) + Get(ctx context.Context, query *annotations.ItemQuery, accessResources *accesscontrol.AccessResources) ([]*annotations.ItemDTO, error) Delete(ctx context.Context, params *annotations.DeleteParams) error GetTags(ctx context.Context, query *annotations.TagsQuery) (annotations.FindTagsResult, error) CleanAnnotations(ctx context.Context, cfg setting.AnnotationCleanupSettings, annotationType string) (int64, error) diff --git a/pkg/services/annotations/annotationsimpl/time.go b/pkg/services/annotations/annotationsimpl/time.go new file mode 100644 index 00000000000..e47449d0c8f --- /dev/null +++ b/pkg/services/annotations/annotationsimpl/time.go @@ -0,0 +1,8 @@ +package annotationsimpl + +import "time" + +var ( + // timeNow is an equivalent time.Now() that can be replaced in tests + timeNow = time.Now +) diff --git a/pkg/services/annotations/annotationsimpl/xorm_store.go b/pkg/services/annotations/annotationsimpl/xorm_store.go index 76bb9281386..649ce774fdf 100644 --- a/pkg/services/annotations/annotationsimpl/xorm_store.go +++ b/pkg/services/annotations/annotationsimpl/xorm_store.go @@ -5,25 +5,21 @@ import ( "context" "errors" "fmt" + "strconv" "strings" "time" + "github.com/grafana/grafana/pkg/services/annotations/accesscontrol" + "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" - ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/services/auth/identity" - "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" "github.com/grafana/grafana/pkg/services/tag" "github.com/grafana/grafana/pkg/setting" ) -var timeNow = time.Now - // Update the item so that EpochEnd >= Epoch func validateTimeRange(item *annotations.Item) error { if item.EpochEnd == 0 { @@ -42,12 +38,19 @@ func validateTimeRange(item *annotations.Item) error { } type xormRepositoryImpl struct { - cfg *setting.Cfg - features featuremgmt.FeatureToggles - db db.DB - log log.Logger - maximumTagsLength int64 - tagService tag.Service + cfg *setting.Cfg + db db.DB + log log.Logger + tagService tag.Service +} + +func NewXormStore(cfg *setting.Cfg, l log.Logger, db db.DB, tagService tag.Service) *xormRepositoryImpl { + return &xormRepositoryImpl{ + cfg: cfg, + db: db, + log: l.New("store", "xorm"), + tagService: tagService, + } } func (r *xormRepositoryImpl) Add(ctx context.Context, item *annotations.Item) error { @@ -237,7 +240,7 @@ func tagSet[T any](fn func(T) int64, list []T) map[int64]struct{} { return set } -func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) { +func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQuery, accessResources *accesscontrol.AccessResources) ([]*annotations.ItemDTO, error) { var sql bytes.Buffer params := make([]interface{}, 0) items := make([]*annotations.ItemDTO, 0) @@ -339,12 +342,11 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQue } } - acFilter, err := r.getAccessControlFilter(query.SignedInUser) + acFilter, err := r.getAccessControlFilter(query.SignedInUser, accessResources) if err != nil { return err } - sql.WriteString(fmt.Sprintf(" AND (%s)", acFilter.where)) - params = append(params, acFilter.whereParams...) + sql.WriteString(fmt.Sprintf(" AND (%s)", acFilter)) if query.Limit == 0 { query.Limit = 100 @@ -352,13 +354,6 @@ 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 @@ -371,64 +366,37 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQue return items, err } -type acFilter struct { - where string - whereParams []interface{} - recQueries string - recParams []interface{} -} - -func (r *xormRepositoryImpl) getAccessControlFilter(user identity.Requester) (acFilter, error) { - var recQueries string - var recQueriesParams []interface{} - - if user == nil || user.IsNil() { - return acFilter{}, errors.New("missing permissions") - } - - scopes, has := user.GetPermissions()[ac.ActionAnnotationsRead] - if !has { - return acFilter{}, errors.New("missing permissions") - } - types, hasWildcardScope := ac.ParseScopes(ac.ScopeAnnotationsProvider.GetResourceScopeType(""), scopes) - if hasWildcardScope { - types = map[interface{}]struct{}{annotations.Dashboard.String(): {}, annotations.Organization.String(): {}} - } - +func (r *xormRepositoryImpl) getAccessControlFilter(user identity.Requester, accessResources *accesscontrol.AccessResources) (string, error) { var filters []string - var params []interface{} - for t := range types { - // annotation read permission with scope annotations:type:organization allows listing annotations that are not associated with a dashboard - if t == annotations.Organization.String() { - filters = append(filters, "a.dashboard_id = 0") - } - // annotation read permission with scope annotations:type:dashboard allows listing annotations from dashboards which the user can view - if t == annotations.Dashboard.String() { - 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() - leftJoin := filterRBAC.LeftJoin() - filter := fmt.Sprintf("a.dashboard_id IN(SELECT id FROM dashboard WHERE %s)", dashboardFilter) - if leftJoin != "" { - filter = fmt.Sprintf("a.dashboard_id IN(SELECT dashboard.id FROM dashboard LEFT OUTER JOIN %s WHERE %s)", leftJoin, dashboardFilter) - } - filters = append(filters, filter) - params = dashboardParams - } + if _, has := accessResources.ScopeTypes[annotations.Organization.String()]; has { + filters = append(filters, "a.dashboard_id = 0") } - f := acFilter{ - where: strings.Join(filters, " OR "), - whereParams: params, - recQueries: recQueries, - recParams: recQueriesParams, + if _, has := accessResources.ScopeTypes[annotations.Dashboard.String()]; has { + var dashboardIDs []int64 + for _, id := range accessResources.Dashboards { + dashboardIDs = append(dashboardIDs, id) + } + + var inClause string + if len(dashboardIDs) == 0 { + inClause = "SELECT * FROM (SELECT 0 LIMIT 0) tt" // empty set + } else { + b := make([]byte, 0, 3*len(dashboardIDs)) + + b = strconv.AppendInt(b, dashboardIDs[0], 10) + for _, num := range dashboardIDs[1:] { + b = append(b, ',') + b = strconv.AppendInt(b, num, 10) + } + + inClause = string(b) + } + filters = append(filters, fmt.Sprintf("a.dashboard_id IN (%s)", inClause)) } - return f, nil + + return strings.Join(filters, " OR "), nil } func (r *xormRepositoryImpl) Delete(ctx context.Context, params *annotations.DeleteParams) error { @@ -541,8 +509,8 @@ func (r *xormRepositoryImpl) validateTagsLength(item *annotations.Item) error { } } estimatedTagsLength += 1 // trailing: ] - if estimatedTagsLength > int(r.maximumTagsLength) { - return annotations.ErrBaseTagLimitExceeded.Errorf("tags length (%d) exceeds the maximum allowed (%d): modify the configuration to increase it", estimatedTagsLength, r.maximumTagsLength) + if estimatedTagsLength > int(r.cfg.AnnotationMaximumTagsLength) { + return annotations.ErrBaseTagLimitExceeded.Errorf("tags length (%d) exceeds the maximum allowed (%d): modify the configuration to increase it", estimatedTagsLength, r.cfg.AnnotationMaximumTagsLength) } return nil } @@ -550,7 +518,7 @@ func (r *xormRepositoryImpl) validateTagsLength(item *annotations.Item) error { func (r *xormRepositoryImpl) CleanAnnotations(ctx context.Context, cfg setting.AnnotationCleanupSettings, annotationType string) (int64, error) { var totalAffected int64 if cfg.MaxAge > 0 { - cutoffDate := time.Now().Add(-cfg.MaxAge).UnixNano() / int64(time.Millisecond) + cutoffDate := timeNow().Add(-cfg.MaxAge).UnixNano() / int64(time.Millisecond) deleteQuery := `DELETE FROM annotation WHERE id IN (SELECT id FROM (SELECT id FROM annotation WHERE %s AND created < %v ORDER BY id DESC %s) a)` sql := fmt.Sprintf(deleteQuery, annotationType, cutoffDate, r.db.GetDialect().Limit(r.cfg.AnnotationCleanupJobBatchSize)) diff --git a/pkg/services/annotations/annotationsimpl/xorm_store_test.go b/pkg/services/annotations/annotationsimpl/xorm_store_test.go index d6f626df37b..55c8a253c9a 100644 --- a/pkg/services/annotations/annotationsimpl/xorm_store_test.go +++ b/pkg/services/annotations/annotationsimpl/xorm_store_test.go @@ -2,30 +2,24 @@ package annotationsimpl import ( "context" - "errors" "fmt" "strings" "testing" - "time" + + "github.com/grafana/grafana/pkg/services/annotations/testutil" + "github.com/grafana/grafana/pkg/services/featuremgmt" + + annotation_ac "github.com/grafana/grafana/pkg/services/annotations/accesscontrol" "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" "github.com/grafana/grafana/pkg/services/tag/tagimpl" @@ -33,15 +27,21 @@ import ( "github.com/grafana/grafana/pkg/setting" ) +var ( + dashScopeType = annotations.Dashboard.String() + orgScopeType = annotations.Organization.String() +) + func TestIntegrationAnnotations(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } sql := db.InitTestDB(t) - var maximumTagsLength int64 = 60 - repo := xormRepositoryImpl{db: sql, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(sql), maximumTagsLength: maximumTagsLength, - features: featuremgmt.WithFeatures(), - } + + cfg := setting.NewCfg() + cfg.AnnotationMaximumTagsLength = 60 + + store := NewXormStore(cfg, log.New("annotation.test"), sql, tagimpl.ProvideService(sql)) testUser := &user.SignedInUser{ OrgID: 1, @@ -66,30 +66,23 @@ func TestIntegrationAnnotations(t *testing.T) { assert.NoError(t, err) }) - quotaService := quotatest.New(false, nil) - dashboardStore, err := dashboardstore.ProvideDashboardStore(sql, sql.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sql), quotaService) - require.NoError(t, err) - - testDashboard1 := dashboards.SaveDashboardCommand{ + dashboard := testutil.CreateDashboard(t, sql, featuremgmt.WithFeatures(), dashboards.SaveDashboardCommand{ UserID: 1, OrgID: 1, Dashboard: simplejson.NewFromAny(map[string]any{ "title": "Dashboard 1", }), - } + }) - dashboard, err := dashboardStore.SaveDashboard(context.Background(), testDashboard1) - require.NoError(t, err) - - testDashboard2 := dashboards.SaveDashboardCommand{ + dashboard2 := testutil.CreateDashboard(t, sql, featuremgmt.WithFeatures(), dashboards.SaveDashboardCommand{ UserID: 1, OrgID: 1, Dashboard: simplejson.NewFromAny(map[string]any{ "title": "Dashboard 2", }), - } - dashboard2, err := dashboardStore.SaveDashboard(context.Background(), testDashboard2) - require.NoError(t, err) + }) + + var err error annotation := &annotations.Item{ OrgID: 1, @@ -101,7 +94,7 @@ func TestIntegrationAnnotations(t *testing.T) { Tags: []string{"outage", "error", "type:outage", "server:server-1"}, Data: simplejson.NewFromAny(map[string]any{"data1": "I am a cool data", "data2": "I am another cool data"}), } - err = repo.Add(context.Background(), annotation) + err = store.Add(context.Background(), annotation) require.NoError(t, err) assert.Greater(t, annotation.ID, int64(0)) assert.Equal(t, annotation.Epoch, annotation.EpochEnd) @@ -116,7 +109,7 @@ func TestIntegrationAnnotations(t *testing.T) { EpochEnd: 20, Tags: []string{"outage", "type:outage", "server:server-1", "error"}, } - err = repo.Add(context.Background(), annotation2) + err = store.Add(context.Background(), annotation2) require.NoError(t, err) assert.Greater(t, annotation2.ID, int64(0)) assert.Equal(t, int64(20), annotation2.Epoch) @@ -130,11 +123,11 @@ func TestIntegrationAnnotations(t *testing.T) { Epoch: 15, Tags: []string{"deploy"}, } - err = repo.Add(context.Background(), organizationAnnotation1) + err = store.Add(context.Background(), organizationAnnotation1) require.NoError(t, err) assert.Greater(t, organizationAnnotation1.ID, int64(0)) - globalAnnotation2 := &annotations.Item{ + organizationAnnotation2 := &annotations.Item{ OrgID: 1, UserID: 1, Text: "rollback", @@ -142,16 +135,22 @@ func TestIntegrationAnnotations(t *testing.T) { Epoch: 17, Tags: []string{"rollback"}, } - err = repo.Add(context.Background(), globalAnnotation2) + err = store.Add(context.Background(), organizationAnnotation2) require.NoError(t, err) - assert.Greater(t, globalAnnotation2.ID, int64(0)) + assert.Greater(t, organizationAnnotation2.ID, int64(0)) + t.Run("Can query for annotation by dashboard id", func(t *testing.T) { - items, err := repo.Get(context.Background(), &annotations.ItemQuery{ + items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, DashboardID: dashboard.ID, From: 0, To: 15, SignedInUser: testUser, + }, &annotation_ac.AccessResources{ + Dashboards: map[string]int64{ + dashboard.UID: dashboard.ID, + }, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, }) require.NoError(t, err) @@ -170,9 +169,9 @@ func TestIntegrationAnnotations(t *testing.T) { Text: "rollback", Type: "", Epoch: 17, - Tags: []string{strings.Repeat("a", int(maximumTagsLength+1))}, + Tags: []string{strings.Repeat("a", int(cfg.AnnotationMaximumTagsLength+1))}, } - err = repo.Add(context.Background(), badAnnotation) + err = store.Add(context.Background(), badAnnotation) require.Error(t, err) require.ErrorIs(t, err, annotations.ErrBaseTagLimitExceeded) @@ -187,11 +186,12 @@ func TestIntegrationAnnotations(t *testing.T) { } } - err := repo.AddMany(context.Background(), items) + err := store.AddMany(context.Background(), items) require.NoError(t, err) query := &annotations.ItemQuery{OrgID: 100, SignedInUser: testUser} - inserted, err := repo.Get(context.Background(), query) + accRes := &annotation_ac.AccessResources{ScopeTypes: map[any]struct{}{orgScopeType: {}}} + inserted, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) assert.Len(t, inserted, count) for _, ins := range inserted { @@ -213,20 +213,26 @@ func TestIntegrationAnnotations(t *testing.T) { } items[0].Tags = []string{"type:test"} - err := repo.AddMany(context.Background(), items) + err := store.AddMany(context.Background(), items) require.NoError(t, err) query := &annotations.ItemQuery{OrgID: 101, SignedInUser: testUser} - inserted, err := repo.Get(context.Background(), query) + accRes := &annotation_ac.AccessResources{ScopeTypes: map[any]struct{}{orgScopeType: {}}} + inserted, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) assert.Len(t, inserted, count) }) t.Run("Can query for annotation by id", func(t *testing.T) { - items, err := repo.Get(context.Background(), &annotations.ItemQuery{ + items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, AnnotationID: annotation2.ID, SignedInUser: testUser, + }, &annotation_ac.AccessResources{ + Dashboards: map[string]int64{ + dashboard2.UID: dashboard2.ID, + }, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, }) require.NoError(t, err) assert.Len(t, items, 1) @@ -234,78 +240,99 @@ func TestIntegrationAnnotations(t *testing.T) { }) t.Run("Should not find any when item is outside time range", func(t *testing.T) { - items, err := repo.Get(context.Background(), &annotations.ItemQuery{ + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, DashboardID: 1, From: 12, To: 15, SignedInUser: testUser, - }) + }, accRes) require.NoError(t, err) assert.Empty(t, items) }) t.Run("Should not find one when tag filter does not match", func(t *testing.T) { - items, err := repo.Get(context.Background(), &annotations.ItemQuery{ + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, DashboardID: 1, From: 1, To: 15, Tags: []string{"asd"}, SignedInUser: testUser, - }) + }, accRes) require.NoError(t, err) assert.Empty(t, items) }) t.Run("Should not find one when type filter does not match", func(t *testing.T) { - items, err := repo.Get(context.Background(), &annotations.ItemQuery{ + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, DashboardID: 1, From: 1, To: 15, Type: "alert", SignedInUser: testUser, - }) + }, accRes) require.NoError(t, err) assert.Empty(t, items) }) t.Run("Should find one when all tag filters does match", func(t *testing.T) { - items, err := repo.Get(context.Background(), &annotations.ItemQuery{ + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, DashboardID: 1, From: 1, To: 15, // this will exclude the second test annotation Tags: []string{"outage", "error"}, SignedInUser: testUser, - }) + }, accRes) require.NoError(t, err) assert.Len(t, items, 1) }) t.Run("Should find two annotations using partial match", func(t *testing.T) { - items, err := repo.Get(context.Background(), &annotations.ItemQuery{ + accRes := &annotation_ac.AccessResources{ScopeTypes: map[any]struct{}{orgScopeType: {}}} + items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, From: 1, To: 25, MatchAny: true, Tags: []string{"rollback", "deploy"}, SignedInUser: testUser, - }) + }, accRes) require.NoError(t, err) assert.Len(t, items, 2) }) t.Run("Should find one when all key value tag filters does match", func(t *testing.T) { - items, err := repo.Get(context.Background(), &annotations.ItemQuery{ + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, DashboardID: 1, From: 1, To: 15, Tags: []string{"type:outage", "server:server-1"}, SignedInUser: testUser, - }) + }, accRes) require.NoError(t, err) assert.Len(t, items, 1) }) @@ -318,11 +345,15 @@ func TestIntegrationAnnotations(t *testing.T) { To: 15, SignedInUser: testUser, } - items, err := repo.Get(context.Background(), query) + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) annotationId := items[0].ID - err = repo.Update(context.Background(), &annotations.Item{ + err = store.Update(context.Background(), &annotations.Item{ ID: annotationId, OrgID: 1, Text: "something new", @@ -330,7 +361,7 @@ func TestIntegrationAnnotations(t *testing.T) { }) require.NoError(t, err) - items, err = repo.Get(context.Background(), query) + items, err = store.Get(context.Background(), query, accRes) require.NoError(t, err) assert.Equal(t, annotationId, items[0].ID) @@ -349,11 +380,15 @@ func TestIntegrationAnnotations(t *testing.T) { To: 15, SignedInUser: testUser, } - items, err := repo.Get(context.Background(), query) + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) annotationId := items[0].ID - err = repo.Update(context.Background(), &annotations.Item{ + err = store.Update(context.Background(), &annotations.Item{ ID: annotationId, OrgID: 1, Text: "something new", @@ -361,7 +396,7 @@ func TestIntegrationAnnotations(t *testing.T) { }) require.NoError(t, err) - items, err = repo.Get(context.Background(), query) + items, err = store.Get(context.Background(), query, accRes) require.NoError(t, err) assert.Equal(t, annotationId, items[0].ID) @@ -378,11 +413,15 @@ func TestIntegrationAnnotations(t *testing.T) { To: 15, SignedInUser: testUser, } - items, err := repo.Get(context.Background(), query) + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) annotationId := items[0].ID - err = repo.Update(context.Background(), &annotations.Item{ + err = store.Update(context.Background(), &annotations.Item{ ID: annotationId, OrgID: 1, Text: "something new", @@ -390,7 +429,7 @@ func TestIntegrationAnnotations(t *testing.T) { }) require.NoError(t, err) - items, err = repo.Get(context.Background(), query) + items, err = store.Get(context.Background(), query, accRes) require.NoError(t, err) assert.Equal(t, annotationId, items[0].ID) @@ -407,12 +446,16 @@ func TestIntegrationAnnotations(t *testing.T) { To: 15, SignedInUser: testUser, } - items, err := repo.Get(context.Background(), query) + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) annotationId := items[0].ID data := simplejson.NewFromAny(map[string]any{"data": "I am a data", "data2": "I am also a data"}) - err = repo.Update(context.Background(), &annotations.Item{ + err = store.Update(context.Background(), &annotations.Item{ ID: annotationId, OrgID: 1, Text: "something new", @@ -421,7 +464,7 @@ func TestIntegrationAnnotations(t *testing.T) { }) require.NoError(t, err) - items, err = repo.Get(context.Background(), query) + items, err = store.Get(context.Background(), query, accRes) require.NoError(t, err) assert.Equal(t, annotationId, items[0].ID) @@ -439,14 +482,18 @@ func TestIntegrationAnnotations(t *testing.T) { To: 15, SignedInUser: testUser, } - items, err := repo.Get(context.Background(), query) + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{"foo": 1}, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) annotationId := items[0].ID - err = repo.Delete(context.Background(), &annotations.DeleteParams{ID: annotationId, OrgID: 1}) + err = store.Delete(context.Background(), &annotations.DeleteParams{ID: annotationId, OrgID: 1}) require.NoError(t, err) - items, err = repo.Get(context.Background(), query) + items, err = store.Get(context.Background(), query, accRes) require.NoError(t, err) assert.Empty(t, items) }) @@ -462,29 +509,36 @@ func TestIntegrationAnnotations(t *testing.T) { Tags: []string{"test"}, PanelID: 20, } - err = repo.Add(context.Background(), annotation3) + err = store.Add(context.Background(), annotation3) require.NoError(t, err) + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{ + dashboard2.UID: dashboard2.ID, + }, + ScopeTypes: map[any]struct{}{dashScopeType: {}}, + } + query := &annotations.ItemQuery{ OrgID: 1, AnnotationID: annotation3.ID, SignedInUser: testUser, } - items, err := repo.Get(context.Background(), query) + items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) dashboardId := items[0].DashboardID panelId := items[0].PanelID - err = repo.Delete(context.Background(), &annotations.DeleteParams{DashboardID: dashboardId, PanelID: panelId, OrgID: 1}) + err = store.Delete(context.Background(), &annotations.DeleteParams{DashboardID: dashboardId, PanelID: panelId, OrgID: 1}) require.NoError(t, err) - items, err = repo.Get(context.Background(), query) + items, err = store.Get(context.Background(), query, accRes) require.NoError(t, err) assert.Empty(t, items) }) t.Run("Should find tags by key", func(t *testing.T) { - result, err := repo.GetTags(context.Background(), &annotations.TagsQuery{ + result, err := store.GetTags(context.Background(), &annotations.TagsQuery{ OrgID: 1, Tag: "server", }) @@ -495,7 +549,7 @@ func TestIntegrationAnnotations(t *testing.T) { }) t.Run("Should find tags by value", func(t *testing.T) { - result, err := repo.GetTags(context.Background(), &annotations.TagsQuery{ + result, err := store.GetTags(context.Background(), &annotations.TagsQuery{ OrgID: 1, Tag: "outage", }) @@ -508,7 +562,7 @@ func TestIntegrationAnnotations(t *testing.T) { }) t.Run("Should not find tags in other org", func(t *testing.T) { - result, err := repo.GetTags(context.Background(), &annotations.TagsQuery{ + result, err := store.GetTags(context.Background(), &annotations.TagsQuery{ OrgID: 0, Tag: "server-1", }) @@ -517,7 +571,7 @@ func TestIntegrationAnnotations(t *testing.T) { }) t.Run("Should not find tags that do not exist", func(t *testing.T) { - result, err := repo.GetTags(context.Background(), &annotations.TagsQuery{ + result, err := store.GetTags(context.Background(), &annotations.TagsQuery{ OrgID: 0, Tag: "unknown:tag", }) @@ -527,359 +581,6 @@ func TestIntegrationAnnotations(t *testing.T) { }) } -func TestIntegrationAnnotationListingWithRBAC(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } - sql := db.InitTestDB(t) - - var maximumTagsLength int64 = 60 - repo := xormRepositoryImpl{db: sql, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(sql), maximumTagsLength: maximumTagsLength, features: featuremgmt.WithFeatures()} - quotaService := quotatest.New(false, nil) - dashboardStore, err := dashboardstore.ProvideDashboardStore(sql, sql.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sql), quotaService) - require.NoError(t, err) - - testDashboard1 := dashboards.SaveDashboardCommand{ - UserID: 1, - OrgID: 1, - IsFolder: false, - Dashboard: simplejson.NewFromAny(map[string]any{ - "title": "Dashboard 1", - }), - } - dashboard, err := dashboardStore.SaveDashboard(context.Background(), testDashboard1) - require.NoError(t, err) - dash1UID := dashboard.UID - - testDashboard2 := dashboards.SaveDashboardCommand{ - UserID: 1, - OrgID: 1, - Dashboard: simplejson.NewFromAny(map[string]any{ - "title": "Dashboard 2", - }), - } - _, err = dashboardStore.SaveDashboard(context.Background(), testDashboard2) - require.NoError(t, err) - - dash1Annotation := &annotations.Item{ - OrgID: 1, - DashboardID: 1, - Epoch: 10, - } - err = repo.Add(context.Background(), dash1Annotation) - require.NoError(t, err) - - dash2Annotation := &annotations.Item{ - OrgID: 1, - DashboardID: 2, - Epoch: 10, - Tags: []string{"foo:bar"}, - } - err = repo.Add(context.Background(), dash2Annotation) - require.NoError(t, err) - - organizationAnnotation := &annotations.Item{ - OrgID: 1, - Epoch: 10, - } - err = repo.Add(context.Background(), organizationAnnotation) - require.NoError(t, err) - - user := &user.SignedInUser{ - UserID: 1, - OrgID: 1, - } - role := setupRBACRole(t, sql, user) - - type testStruct struct { - description string - permissions map[string][]string - expectedAnnotationIds []int64 - expectedError bool - } - - testCases := []testStruct{ - { - description: "Should find all annotations when has permissions to list all annotations and read all dashboards", - permissions: map[string][]string{ - accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsAll}, - dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, - }, - expectedAnnotationIds: []int64{dash1Annotation.ID, dash2Annotation.ID, organizationAnnotation.ID}, - }, - { - description: "Should find all dashboard annotations", - permissions: map[string][]string{ - accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard}, - dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, - }, - expectedAnnotationIds: []int64{dash1Annotation.ID, dash2Annotation.ID}, - }, - { - description: "Should find only annotations from dashboards that user can read", - permissions: map[string][]string{ - accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard}, - dashboards.ActionDashboardsRead: {fmt.Sprintf("dashboards:uid:%s", dash1UID)}, - }, - expectedAnnotationIds: []int64{dash1Annotation.ID}, - }, - { - description: "Should find no annotations if user can't view dashboards or organization annotations", - permissions: map[string][]string{ - accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeDashboard}, - }, - expectedAnnotationIds: []int64{}, - }, - { - description: "Should find only organization annotations", - permissions: map[string][]string{ - accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeOrganization}, - dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, - }, - expectedAnnotationIds: []int64{organizationAnnotation.ID}, - }, - { - description: "Should error if user doesn't have annotation read permissions", - permissions: map[string][]string{ - dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, - }, - expectedError: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - user.Permissions = map[int64]map[string][]string{1: tc.permissions} - setupRBACPermission(t, sql, role, user) - - results, err := repo.Get(context.Background(), &annotations.ItemQuery{ - OrgID: 1, - SignedInUser: user, - }) - if tc.expectedError { - require.Error(t, err) - return - } - require.NoError(t, err) - assert.Len(t, results, len(tc.expectedAnnotationIds)) - for _, r := range results { - assert.Contains(t, tc.expectedAnnotationIds, r.ID) - } - }) - } -} - -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), 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), 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]any{ - "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), 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 := db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - role = &accesscontrol.Role{ - OrgID: user.OrgID, - UID: "test_role", - Name: "test:role", - Updated: time.Now(), - Created: time.Now(), - } - _, err := sess.Insert(role) - if err != nil { - return err - } - - _, err = sess.Insert(accesscontrol.UserRole{ - OrgID: role.OrgID, - RoleID: role.ID, - UserID: user.UserID, - Created: time.Now(), - }) - if err != nil { - return err - } - return nil - }) - - require.NoError(t, err) - return role -} - -func setupRBACPermission(t *testing.T, db *sqlstore.SQLStore, role *accesscontrol.Role, user *user.SignedInUser) { - t.Helper() - 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 - } - - var acPermission []accesscontrol.Permission - for action, scopes := range user.Permissions[user.OrgID] { - for _, scope := range scopes { - acPermission = append(acPermission, accesscontrol.Permission{ - RoleID: role.ID, Action: action, Scope: scope, Created: time.Now(), Updated: time.Now(), - }) - } - } - - if _, err := sess.InsertMulti(&acPermission); err != nil { - return err - } - - return nil - }) - - require.NoError(t, err) -} - func BenchmarkFindTags_10k(b *testing.B) { benchmarkFindTags(b, 10000) } @@ -890,8 +591,9 @@ func BenchmarkFindTags_100k(b *testing.B) { func benchmarkFindTags(b *testing.B, numAnnotations int) { sql := db.InitTestDB(b) - var maximumTagsLength int64 = 60 - repo := xormRepositoryImpl{db: sql, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(sql), maximumTagsLength: maximumTagsLength} + cfg := setting.NewCfg() + cfg.AnnotationMaximumTagsLength = 60 + store := xormRepositoryImpl{db: sql, cfg: cfg, log: log.New("annotation.test"), tagService: tagimpl.ProvideService(sql)} type annotationTag struct { ID int64 `xorm:"pk autoincr 'id'"` @@ -950,12 +652,12 @@ func benchmarkFindTags(b *testing.B, numAnnotations int) { Tags: []string{"outage", "error", "type:outage", "server:server-1"}, Data: simplejson.NewFromAny(map[string]any{"data1": "I am a cool data", "data2": "I am another cool data"}), } - err = repo.Add(context.Background(), &annotationWithTheTag) + err = store.Add(context.Background(), &annotationWithTheTag) require.NoError(b, err) b.ResetTimer() for i := 0; i < b.N; i++ { - result, err := repo.GetTags(context.Background(), &annotations.TagsQuery{ + result, err := store.GetTags(context.Background(), &annotations.TagsQuery{ OrgID: 1, Tag: "outage", }) diff --git a/pkg/services/annotations/testutil/testutil.go b/pkg/services/annotations/testutil/testutil.go new file mode 100644 index 00000000000..aa340cb4f1b --- /dev/null +++ b/pkg/services/annotations/testutil/testutil.go @@ -0,0 +1,95 @@ +package testutil + +import ( + "context" + "testing" + "time" + + "github.com/grafana/grafana/pkg/services/accesscontrol" + "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/quota/quotatest" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/tag/tagimpl" + "github.com/grafana/grafana/pkg/services/user" + "github.com/stretchr/testify/require" +) + +func SetupRBACRole(t *testing.T, db *sqlstore.SQLStore, user *user.SignedInUser) *accesscontrol.Role { + t.Helper() + + var role *accesscontrol.Role + err := db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + role = &accesscontrol.Role{ + OrgID: user.OrgID, + UID: "test_role", + Name: "test:role", + Updated: time.Now(), + Created: time.Now(), + } + _, err := sess.Insert(role) + if err != nil { + return err + } + + _, err = sess.Insert(accesscontrol.UserRole{ + OrgID: role.OrgID, + RoleID: role.ID, + UserID: user.UserID, + Created: time.Now(), + }) + if err != nil { + return err + } + return nil + }) + require.NoError(t, err) + + return role +} + +func SetupRBACPermission(t *testing.T, db *sqlstore.SQLStore, role *accesscontrol.Role, user *user.SignedInUser) { + t.Helper() + + 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 + } + + var acPermission []accesscontrol.Permission + for action, scopes := range user.Permissions[user.OrgID] { + for _, scope := range scopes { + acPermission = append(acPermission, accesscontrol.Permission{ + RoleID: role.ID, Action: action, Scope: scope, Created: time.Now(), Updated: time.Now(), + }) + } + } + + if _, err := sess.InsertMulti(&acPermission); err != nil { + return err + } + + return nil + }) + + require.NoError(t, err) +} + +func CreateDashboard(t *testing.T, sql *sqlstore.SQLStore, features featuremgmt.FeatureToggles, cmd dashboards.SaveDashboardCommand) *dashboards.Dashboard { + t.Helper() + + dashboardStore, err := dashboardstore.ProvideDashboardStore( + sql, + sql.Cfg, + features, + tagimpl.ProvideService(sql), + quotatest.New(false, nil), + ) + require.NoError(t, err) + + dash, err := dashboardStore.SaveDashboard(context.Background(), cmd) + require.NoError(t, err) + + return dash +}