RBAC: Change annotation filter to use dashboard based annotation scopes (#78635)

change annotation filter to use dash based annotation scopes

pkg/services/annotations/accesscontrol/accesscontrol.go

@@ -49,13 +49,17 @@ func (authz *AuthService) Authorize(ctx context.Context, orgID int64, user ident
 	if !has {
 		return nil, ErrReadForbidden.Errorf("user does not have permission to read annotations")
 	}
-
 	scopeTypes := annotationScopeTypes(scopes)
+	_, canAccessOrgAnnotations := scopeTypes[annotations.Organization.String()]
+	_, canAccessDashAnnotations := scopeTypes[annotations.Dashboard.String()]
+	if authz.features.IsEnabled(ctx, featuremgmt.FlagAnnotationPermissionUpdate) {
+		canAccessDashAnnotations = true
+	}
 
 	var visibleDashboards map[string]int64
 	var err error
-	if _, ok := scopeTypes[annotations.Dashboard.String()]; ok {
+	if canAccessDashAnnotations {
-		visibleDashboards, err = authz.userVisibleDashboards(ctx, user, orgID)
+		visibleDashboards, err = authz.dashboardsWithVisibleAnnotations(ctx, user, orgID)
 		if err != nil {
 			return nil, ErrAccessControlInternal.Errorf("failed to fetch dashboards: %w", err)
 		}
@@ -63,18 +67,24 @@ func (authz *AuthService) Authorize(ctx context.Context, orgID int64, user ident
 
 	return &AccessResources{
 		Dashboards:               visibleDashboards,
-		ScopeTypes: scopeTypes,
+		CanAccessDashAnnotations: canAccessDashAnnotations,
+		CanAccessOrgAnnotations:  canAccessOrgAnnotations,
 	}, nil
 }
 
-func (authz *AuthService) userVisibleDashboards(ctx context.Context, user identity.Requester, orgID int64) (map[string]int64, error) {
+func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context, user identity.Requester, orgID int64) (map[string]int64, error) {
 	recursiveQueriesSupported, err := authz.db.RecursiveQueriesAreSupported()
 	if err != nil {
 		return nil, err
 	}
 
+	filterType := searchstore.TypeDashboard
+	if authz.features.IsEnabled(ctx, featuremgmt.FlagAnnotationPermissionUpdate) {
+		filterType = searchstore.TypeAnnotation
+	}
+
 	filters := []any{
-		permissions.NewAccessControlDashboardPermissionFilter(user, dashboardaccess.PERMISSION_VIEW, searchstore.TypeDashboard, authz.features, recursiveQueriesSupported),
+		permissions.NewAccessControlDashboardPermissionFilter(user, dashboardaccess.PERMISSION_VIEW, filterType, authz.features, recursiveQueriesSupported),
 		searchstore.OrgFilter{OrgId: orgID},
 	}
 

pkg/services/annotations/accesscontrol/accesscontrol_test.go

@@ -8,7 +8,6 @@ import (
 	"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"
@@ -16,11 +15,6 @@ import (
 	"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")
@@ -28,8 +22,6 @@ func TestIntegrationAuthorize(t *testing.T) {
 
 	sql := db.InitTestDB(t)
 
-	authz := NewAuthService(sql, featuremgmt.WithFeatures())
-
 	dash1 := testutil.CreateDashboard(t, sql, featuremgmt.WithFeatures(), dashboards.SaveDashboardCommand{
 		UserID: 1,
 		OrgID:  1,
@@ -55,6 +47,7 @@ func TestIntegrationAuthorize(t *testing.T) {
 	type testCase struct {
 		name              string
 		permissions       map[string][]string
+		featureToggle     string
 		expectedResources *AccessResources
 		expectedErr       error
 	}
@@ -68,7 +61,45 @@ func TestIntegrationAuthorize(t *testing.T) {
 			},
 			expectedResources: &AccessResources{
 				Dashboards:               map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}, orgScopeType: {}},
+				CanAccessOrgAnnotations:  true,
+				CanAccessDashAnnotations: true,
+			},
+		},
+		{
+			name: "should have no dashboards if missing annotation read permission on dashboards and FlagAnnotationPermissionUpdate is enabled",
+			permissions: map[string][]string{
+				accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsAll},
+				dashboards.ActionDashboardsRead:     {dashboards.ScopeDashboardsAll},
+			},
+			featureToggle: featuremgmt.FlagAnnotationPermissionUpdate,
+			expectedResources: &AccessResources{
+				Dashboards:               nil,
+				CanAccessOrgAnnotations:  true,
+				CanAccessDashAnnotations: true,
+			},
+		},
+		{
+			name: "should have dashboard and organization scope and all dashboards if FlagAnnotationPermissionUpdate is enabled",
+			permissions: map[string][]string{
+				accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeOrganization, dashboards.ScopeDashboardsAll},
+			},
+			featureToggle: featuremgmt.FlagAnnotationPermissionUpdate,
+			expectedResources: &AccessResources{
+				Dashboards:               map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID},
+				CanAccessOrgAnnotations:  true,
+				CanAccessDashAnnotations: true,
+			},
+		},
+		{
+			name: "should have dashboard and organization scope and all dashboards if FlagAnnotationPermissionUpdate is enabled and folder based scope is used",
+			permissions: map[string][]string{
+				accesscontrol.ActionAnnotationsRead: {accesscontrol.ScopeAnnotationsTypeOrganization, dashboards.ScopeFoldersAll},
+			},
+			featureToggle: featuremgmt.FlagAnnotationPermissionUpdate,
+			expectedResources: &AccessResources{
+				Dashboards:               map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID},
+				CanAccessOrgAnnotations:  true,
+				CanAccessDashAnnotations: true,
 			},
 		},
 		{
@@ -79,7 +110,7 @@ func TestIntegrationAuthorize(t *testing.T) {
 			},
 			expectedResources: &AccessResources{
 				Dashboards:              nil,
-				ScopeTypes: map[any]struct{}{orgScopeType: {}},
+				CanAccessOrgAnnotations: true,
 			},
 		},
 		{
@@ -90,7 +121,19 @@ func TestIntegrationAuthorize(t *testing.T) {
 			},
 			expectedResources: &AccessResources{
 				Dashboards:               map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
+			},
+		},
+		{
+			name: "should have only dashboard scope and all dashboards if FlagAnnotationPermissionUpdate is enabled",
+			permissions: map[string][]string{
+				accesscontrol.ActionAnnotationsRead: {dashboards.ScopeDashboardsAll},
+			},
+			featureToggle: featuremgmt.FlagAnnotationPermissionUpdate,
+			expectedResources: &AccessResources{
+				Dashboards:               map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID},
+				CanAccessOrgAnnotations:  false,
+				CanAccessDashAnnotations: true,
 			},
 		},
 		{
@@ -101,7 +144,19 @@ func TestIntegrationAuthorize(t *testing.T) {
 			},
 			expectedResources: &AccessResources{
 				Dashboards:               map[string]int64{dash1.UID: dash1.ID},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
+			},
+		},
+		{
+			name: "should have only dashboard scope and only dashboard 1 if FlagAnnotationPermissionUpdate is enabled",
+			permissions: map[string][]string{
+				accesscontrol.ActionAnnotationsRead: {dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dash1.UID)},
+			},
+			featureToggle: featuremgmt.FlagAnnotationPermissionUpdate,
+			expectedResources: &AccessResources{
+				Dashboards:               map[string]int64{dash1.UID: dash1.ID},
+				CanAccessOrgAnnotations:  false,
+				CanAccessDashAnnotations: true,
 			},
 		},
 	}
@@ -111,6 +166,8 @@ func TestIntegrationAuthorize(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))
+
 			resources, err := authz.Authorize(context.Background(), 1, u)
 			require.NoError(t, err)
 
@@ -118,9 +175,8 @@ func TestIntegrationAuthorize(t *testing.T) {
 				require.Equal(t, tc.expectedResources.Dashboards, resources.Dashboards)
 			}
 
-			if tc.expectedResources.ScopeTypes != nil {
+			require.Equal(t, tc.expectedResources.CanAccessDashAnnotations, resources.CanAccessDashAnnotations)
-				require.Equal(t, tc.expectedResources.ScopeTypes, resources.ScopeTypes)
+			require.Equal(t, tc.expectedResources.CanAccessOrgAnnotations, resources.CanAccessOrgAnnotations)
-			}
 
 			if tc.expectedErr != nil {
 				require.Equal(t, tc.expectedErr, err)

pkg/services/annotations/accesscontrol/models.go

@@ -4,8 +4,10 @@ package accesscontrol
 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`
+	// CanAccessDashAnnotations true if the user is allowed to access some dashboard annotations
-	ScopeTypes map[any]struct{}
+	CanAccessDashAnnotations bool
+	// CanAccessOrgAnnotations true if the user is allowed to access organization annotations
+	CanAccessOrgAnnotations bool
 }
 
 type dashboardProjection struct {

pkg/services/annotations/annotationsimpl/xorm_store.go

@@ -369,11 +369,11 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQue
 func (r *xormRepositoryImpl) getAccessControlFilter(user identity.Requester, accessResources *accesscontrol.AccessResources) (string, error) {
 	var filters []string
 
-	if _, has := accessResources.ScopeTypes[annotations.Organization.String()]; has {
+	if accessResources.CanAccessOrgAnnotations {
 		filters = append(filters, "a.dashboard_id = 0")
 	}
 
-	if _, has := accessResources.ScopeTypes[annotations.Dashboard.String()]; has {
+	if accessResources.CanAccessDashAnnotations {
 		var dashboardIDs []int64
 		for _, id := range accessResources.Dashboards {
 			dashboardIDs = append(dashboardIDs, id)

pkg/services/annotations/annotationsimpl/xorm_store_test.go

@@ -6,11 +6,6 @@ import (
 	"strings"
 	"testing"
 
-	"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"
 
@@ -19,7 +14,10 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/annotations"
+	annotation_ac "github.com/grafana/grafana/pkg/services/annotations/accesscontrol"
+	"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/sqlstore"
 	"github.com/grafana/grafana/pkg/services/tag"
 	"github.com/grafana/grafana/pkg/services/tag/tagimpl"
@@ -27,11 +25,6 @@ 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")
@@ -150,7 +143,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 				Dashboards: map[string]int64{
 					dashboard.UID: dashboard.ID,
 				},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			})
 
 			require.NoError(t, err)
@@ -190,7 +183,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 
 			require.NoError(t, err)
 			query := &annotations.ItemQuery{OrgID: 100, SignedInUser: testUser}
-			accRes := &annotation_ac.AccessResources{ScopeTypes: map[any]struct{}{orgScopeType: {}}}
+			accRes := &annotation_ac.AccessResources{CanAccessOrgAnnotations: true}
 			inserted, err := store.Get(context.Background(), query, accRes)
 			require.NoError(t, err)
 			assert.Len(t, inserted, count)
@@ -217,7 +210,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 
 			require.NoError(t, err)
 			query := &annotations.ItemQuery{OrgID: 101, SignedInUser: testUser}
-			accRes := &annotation_ac.AccessResources{ScopeTypes: map[any]struct{}{orgScopeType: {}}}
+			accRes := &annotation_ac.AccessResources{CanAccessOrgAnnotations: true}
 			inserted, err := store.Get(context.Background(), query, accRes)
 			require.NoError(t, err)
 			assert.Len(t, inserted, count)
@@ -232,7 +225,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 				Dashboards: map[string]int64{
 					dashboard2.UID: dashboard2.ID,
 				},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			})
 			require.NoError(t, err)
 			assert.Len(t, items, 1)
@@ -242,7 +235,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 		t.Run("Should not find any when item is outside time range", func(t *testing.T) {
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), &annotations.ItemQuery{
 				OrgID:        1,
@@ -258,7 +251,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 		t.Run("Should not find one when tag filter does not match", func(t *testing.T) {
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), &annotations.ItemQuery{
 				OrgID:        1,
@@ -275,7 +268,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 		t.Run("Should not find one when type filter does not match", func(t *testing.T) {
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), &annotations.ItemQuery{
 				OrgID:        1,
@@ -292,7 +285,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 		t.Run("Should find one when all tag filters does match", func(t *testing.T) {
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), &annotations.ItemQuery{
 				OrgID:        1,
@@ -307,7 +300,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 		})
 
 		t.Run("Should find two annotations using partial match", func(t *testing.T) {
-			accRes := &annotation_ac.AccessResources{ScopeTypes: map[any]struct{}{orgScopeType: {}}}
+			accRes := &annotation_ac.AccessResources{CanAccessOrgAnnotations: true}
 			items, err := store.Get(context.Background(), &annotations.ItemQuery{
 				OrgID:        1,
 				From:         1,
@@ -323,7 +316,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 		t.Run("Should find one when all key value tag filters does match", func(t *testing.T) {
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), &annotations.ItemQuery{
 				OrgID:        1,
@@ -347,7 +340,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 			}
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), query, accRes)
 			require.NoError(t, err)
@@ -382,7 +375,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 			}
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), query, accRes)
 			require.NoError(t, err)
@@ -415,7 +408,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 			}
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), query, accRes)
 			require.NoError(t, err)
@@ -448,7 +441,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 			}
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), query, accRes)
 			require.NoError(t, err)
@@ -484,7 +477,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 			}
 			accRes := &annotation_ac.AccessResources{
 				Dashboards:               map[string]int64{"foo": 1},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 			items, err := store.Get(context.Background(), query, accRes)
 			require.NoError(t, err)
@@ -516,7 +509,7 @@ func TestIntegrationAnnotations(t *testing.T) {
 				Dashboards: map[string]int64{
 					dashboard2.UID: dashboard2.ID,
 				},
-				ScopeTypes: map[any]struct{}{dashScopeType: {}},
+				CanAccessDashAnnotations: true,
 			}
 
 			query := &annotations.ItemQuery{

pkg/services/sqlstore/permissions/dashboard.go

@@ -75,6 +75,8 @@ func NewAccessControlDashboardPermissionFilter(user identity.Requester, permissi
 				accesscontrol.ActionAlertingRuleCreate,
 			)
 		}
+	} else if queryType == searchstore.TypeAnnotation {
+		dashboardActions = append(dashboardActions, accesscontrol.ActionAnnotationsRead)
 	} else {
 		folderActions = append(folderActions, dashboards.ActionFoldersRead)
 		dashboardActions = append(dashboardActions, dashboards.ActionDashboardsRead)

pkg/services/sqlstore/searchstore/filters.go

@@ -13,6 +13,7 @@ const (
 	TypeFolder      = "dash-folder"
 	TypeDashboard   = "dash-db"
 	TypeAlertFolder = "dash-folder-alerting"
+	TypeAnnotation  = "dash-annotation"
 )
 
 type TypeFilter struct {