diff --git a/pkg/services/annotations/accesscontrol/accesscontrol.go b/pkg/services/annotations/accesscontrol/accesscontrol.go index 67bdaa5f441..071cf21e870 100644 --- a/pkg/services/annotations/accesscontrol/accesscontrol.go +++ b/pkg/services/annotations/accesscontrol/accesscontrol.go @@ -49,32 +49,42 @@ 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 { - visibleDashboards, err = authz.userVisibleDashboards(ctx, user, orgID) + if canAccessDashAnnotations { + visibleDashboards, err = authz.dashboardsWithVisibleAnnotations(ctx, user, orgID) if err != nil { return nil, ErrAccessControlInternal.Errorf("failed to fetch dashboards: %w", err) } } return &AccessResources{ - Dashboards: visibleDashboards, - ScopeTypes: scopeTypes, + Dashboards: visibleDashboards, + 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}, } diff --git a/pkg/services/annotations/accesscontrol/accesscontrol_test.go b/pkg/services/annotations/accesscontrol/accesscontrol_test.go index 0fe70183e9f..7bb4d74f2a6 100644 --- a/pkg/services/annotations/accesscontrol/accesscontrol_test.go +++ b/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 } @@ -67,8 +60,46 @@ func TestIntegrationAuthorize(t *testing.T) { dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, }, expectedResources: &AccessResources{ - Dashboards: map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID}, - ScopeTypes: map[any]struct{}{dashScopeType: {}, orgScopeType: {}}, + Dashboards: map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID}, + 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, }, }, { @@ -78,8 +109,8 @@ func TestIntegrationAuthorize(t *testing.T) { dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, }, expectedResources: &AccessResources{ - Dashboards: nil, - ScopeTypes: map[any]struct{}{orgScopeType: {}}, + Dashboards: nil, + CanAccessOrgAnnotations: true, }, }, { @@ -89,8 +120,20 @@ func TestIntegrationAuthorize(t *testing.T) { dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, }, expectedResources: &AccessResources{ - Dashboards: map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID}, - ScopeTypes: map[any]struct{}{dashScopeType: {}}, + Dashboards: map[string]int64{dash1.UID: dash1.ID, dash2.UID: dash2.ID}, + 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, }, }, { @@ -100,8 +143,20 @@ func TestIntegrationAuthorize(t *testing.T) { dashboards.ActionDashboardsRead: {fmt.Sprintf("dashboards:uid:%s", dash1.UID)}, }, expectedResources: &AccessResources{ - Dashboards: map[string]int64{dash1.UID: dash1.ID}, - ScopeTypes: map[any]struct{}{dashScopeType: {}}, + Dashboards: map[string]int64{dash1.UID: dash1.ID}, + 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.ScopeTypes, resources.ScopeTypes) - } + require.Equal(t, tc.expectedResources.CanAccessDashAnnotations, resources.CanAccessDashAnnotations) + require.Equal(t, tc.expectedResources.CanAccessOrgAnnotations, resources.CanAccessOrgAnnotations) 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 index e316beb0a1b..7b74de929d8 100644 --- a/pkg/services/annotations/accesscontrol/models.go +++ b/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` - ScopeTypes map[any]struct{} + // CanAccessDashAnnotations true if the user is allowed to access some dashboard annotations + CanAccessDashAnnotations bool + // CanAccessOrgAnnotations true if the user is allowed to access organization annotations + CanAccessOrgAnnotations bool } type dashboardProjection struct { diff --git a/pkg/services/annotations/annotationsimpl/xorm_store.go b/pkg/services/annotations/annotationsimpl/xorm_store.go index 649ce774fdf..19a7a846d82 100644 --- a/pkg/services/annotations/annotationsimpl/xorm_store.go +++ b/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) diff --git a/pkg/services/annotations/annotationsimpl/xorm_store_test.go b/pkg/services/annotations/annotationsimpl/xorm_store_test.go index 55c8a253c9a..92b27405ac3 100644 --- a/pkg/services/annotations/annotationsimpl/xorm_store_test.go +++ b/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) @@ -241,8 +234,8 @@ 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: {}}, + Dashboards: map[string]int64{"foo": 1}, + CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, @@ -257,8 +250,8 @@ 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: {}}, + Dashboards: map[string]int64{"foo": 1}, + CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, @@ -274,8 +267,8 @@ 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: {}}, + Dashboards: map[string]int64{"foo": 1}, + CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, @@ -291,8 +284,8 @@ 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: {}}, + Dashboards: map[string]int64{"foo": 1}, + 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, @@ -322,8 +315,8 @@ 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: {}}, + Dashboards: map[string]int64{"foo": 1}, + CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), &annotations.ItemQuery{ OrgID: 1, @@ -346,8 +339,8 @@ func TestIntegrationAnnotations(t *testing.T) { SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, - ScopeTypes: map[any]struct{}{dashScopeType: {}}, + Dashboards: map[string]int64{"foo": 1}, + CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) @@ -381,8 +374,8 @@ func TestIntegrationAnnotations(t *testing.T) { SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, - ScopeTypes: map[any]struct{}{dashScopeType: {}}, + Dashboards: map[string]int64{"foo": 1}, + CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) @@ -414,8 +407,8 @@ func TestIntegrationAnnotations(t *testing.T) { SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, - ScopeTypes: map[any]struct{}{dashScopeType: {}}, + Dashboards: map[string]int64{"foo": 1}, + CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) @@ -447,8 +440,8 @@ func TestIntegrationAnnotations(t *testing.T) { SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, - ScopeTypes: map[any]struct{}{dashScopeType: {}}, + Dashboards: map[string]int64{"foo": 1}, + CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) @@ -483,8 +476,8 @@ func TestIntegrationAnnotations(t *testing.T) { SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, - ScopeTypes: map[any]struct{}{dashScopeType: {}}, + Dashboards: map[string]int64{"foo": 1}, + 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{ diff --git a/pkg/services/sqlstore/permissions/dashboard.go b/pkg/services/sqlstore/permissions/dashboard.go index 813343e2491..3c58e6a80e8 100644 --- a/pkg/services/sqlstore/permissions/dashboard.go +++ b/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) diff --git a/pkg/services/sqlstore/searchstore/filters.go b/pkg/services/sqlstore/searchstore/filters.go index 49cd2c1496c..8ba509e7169 100644 --- a/pkg/services/sqlstore/searchstore/filters.go +++ b/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 {