diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index 31065552cec..0a440bb4b5b 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -127,7 +127,13 @@ func (hs *HTTPServer) PostAnnotation(c *contextmodel.ReqContext) response.Respon } if canSave, err := hs.canCreateAnnotation(c, cmd.DashboardId); err != nil || !canSave { - return dashboardGuardianResponse(err) + if !hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { + return dashboardGuardianResponse(err) + } else if err != nil { + return response.Error(http.StatusInternalServerError, "Error while checking annotation permissions", err) + } else { + return response.Error(http.StatusForbidden, "Access denied to save the annotation", nil) + } } if cmd.Text == "" { @@ -273,8 +279,10 @@ func (hs *HTTPServer) UpdateAnnotation(c *contextmodel.ReqContext) response.Resp return resp } - if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave { - return dashboardGuardianResponse(err) + if !hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { + if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave { + return dashboardGuardianResponse(err) + } } userID, err := identity.UserIdentifier(c.SignedInUser.GetNamespacedID()) @@ -334,8 +342,10 @@ func (hs *HTTPServer) PatchAnnotation(c *contextmodel.ReqContext) response.Respo return resp } - if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave { - return dashboardGuardianResponse(err) + if !hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { + if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave { + return dashboardGuardianResponse(err) + } } userID, err := identity.UserIdentifier(c.SignedInUser.GetNamespacedID()) @@ -437,7 +447,13 @@ func (hs *HTTPServer) MassDeleteAnnotations(c *contextmodel.ReqContext) response canSave, err := hs.canMassDeleteAnnotations(c, dashboardId) if err != nil || !canSave { - return dashboardGuardianResponse(err) + if !hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { + return dashboardGuardianResponse(err) + } else if err != nil { + return response.Error(http.StatusInternalServerError, "Error while checking annotation permissions", err) + } else { + return response.Error(http.StatusForbidden, "Access denied to mass delete annotations", nil) + } } err = hs.annotationsRepo.Delete(c.Req.Context(), deleteParams) @@ -492,13 +508,15 @@ func (hs *HTTPServer) DeleteAnnotationByID(c *contextmodel.ReqContext) response. return response.Error(http.StatusBadRequest, "annotationId is invalid", err) } - annotation, resp := findAnnotationByID(c.Req.Context(), hs.annotationsRepo, annotationID, c.SignedInUser) - if resp != nil { - return resp - } + if !hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { + annotation, resp := findAnnotationByID(c.Req.Context(), hs.annotationsRepo, annotationID, c.SignedInUser) + if resp != nil { + return resp + } - if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave { - return dashboardGuardianResponse(err) + if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave { + return dashboardGuardianResponse(err) + } } err = hs.annotationsRepo.Delete(c.Req.Context(), &annotations.DeleteParams{ @@ -658,6 +676,16 @@ func AnnotationTypeScopeResolver(annotationsRepo annotations.Repository, feature } func (hs *HTTPServer) canCreateAnnotation(c *contextmodel.ReqContext, dashboardId int64) (bool, error) { + if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { + if dashboardId != 0 { + evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, dashboards.ScopeDashboardsProvider.GetResourceScope(strconv.FormatInt(dashboardId, 10))) + return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) + } else { // organization annotations + evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeOrganization) + return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) + } + } + if dashboardId != 0 { evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeDashboard) if canSave, err := hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator); err != nil || !canSave { @@ -672,6 +700,16 @@ func (hs *HTTPServer) canCreateAnnotation(c *contextmodel.ReqContext, dashboardI } func (hs *HTTPServer) canMassDeleteAnnotations(c *contextmodel.ReqContext, dashboardID int64) (bool, error) { + if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { + if dashboardID == 0 { + evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsDelete, accesscontrol.ScopeAnnotationsTypeOrganization) + return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) + } else { + evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsDelete, dashboards.ScopeDashboardsProvider.GetResourceScope(strconv.FormatInt(dashboardID, 10))) + return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) + } + } + if dashboardID == 0 { evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsDelete, accesscontrol.ScopeAnnotationsTypeOrganization) return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) diff --git a/pkg/api/annotations_test.go b/pkg/api/annotations_test.go index e1057f7858a..c8e4a0d101a 100644 --- a/pkg/api/annotations_test.go +++ b/pkg/api/annotations_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -16,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/annotations/annotationstest" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder/foldertest" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/setting" @@ -23,12 +25,16 @@ import ( ) func TestAPI_Annotations(t *testing.T) { + dashUID := "test-dash" + folderUID := "test-folder" + type testCase struct { desc string path string method string body string expectedCode int + featureFlags []any permissions []accesscontrol.Permission } @@ -61,6 +67,30 @@ func TestAPI_Annotations(t *testing.T) { expectedCode: http.StatusForbidden, permissions: []accesscontrol.Permission{}, }, + { + desc: "should be able to fetch dashboard annotation by id with correct dashboard scope with annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodGet, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsRead, Scope: dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashUID)}}, + }, + { + desc: "should be able to fetch dashboard annotation by id with correct folder scope with annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodGet, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsRead, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)}}, + }, + { + desc: "should not be able to fetch dashboard annotation by id with the old dashboard scope when annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodGet, + expectedCode: http.StatusForbidden, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsRead, Scope: accesscontrol.ScopeAnnotationsTypeDashboard}}, + }, { desc: "should be able to fetch annotation tags with correct permission", path: "/api/annotations/tags", @@ -89,6 +119,30 @@ func TestAPI_Annotations(t *testing.T) { expectedCode: http.StatusForbidden, permissions: []accesscontrol.Permission{}, }, + { + desc: "should be able to update dashboard annotation with correct dashboard scope with annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodPut, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsWrite, Scope: dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashUID)}}, + }, + { + desc: "should be able to update dashboard annotation with correct folder scope with annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodPut, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)}}, + }, + { + desc: "should not be able to update dashboard annotation with the old dashboard scope when annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodPut, + expectedCode: http.StatusForbidden, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsWrite, Scope: accesscontrol.ScopeAnnotationsTypeDashboard}}, + }, { desc: "should be able to update organization annotation with correct permission", path: "/api/annotations/1", @@ -117,6 +171,30 @@ func TestAPI_Annotations(t *testing.T) { expectedCode: http.StatusForbidden, permissions: []accesscontrol.Permission{}, }, + { + desc: "should be able to patch dashboard annotation with correct dashboard scope with annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodPatch, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsWrite, Scope: dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashUID)}}, + }, + { + desc: "should be able to patch dashboard annotation with correct folder scope with annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodPatch, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)}}, + }, + { + desc: "should not be able to patch dashboard annotation with the old dashboard scope when annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodPatch, + expectedCode: http.StatusForbidden, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsWrite, Scope: accesscontrol.ScopeAnnotationsTypeDashboard}}, + }, { desc: "should be able to patch organization annotation with correct permission", path: "/api/annotations/1", @@ -147,6 +225,33 @@ func TestAPI_Annotations(t *testing.T) { expectedCode: http.StatusForbidden, permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsCreate, Scope: accesscontrol.ScopeAnnotationsTypeOrganization}}, }, + { + desc: "should be able to create dashboard annotation with correct dashboard scope with annotationPermissionUpdate enabled", + path: "/api/annotations", + method: http.MethodPost, + body: "{\"dashboardId\": 2,\"text\": \"test\"}", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsCreate, Scope: dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashUID)}}, + }, + { + desc: "should be able to create dashboard annotation with correct folder scope with annotationPermissionUpdate enabled", + path: "/api/annotations", + method: http.MethodPost, + body: "{\"dashboardId\": 2,\"text\": \"test\"}", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsCreate, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)}}, + }, + { + desc: "should not be able to create dashboard annotation with the old dashboard scope when annotationPermissionUpdate enabled", + path: "/api/annotations", + method: http.MethodPost, + body: "{\"dashboardId\": 2,\"text\": \"test\"}", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + expectedCode: http.StatusForbidden, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsCreate, Scope: accesscontrol.ScopeAnnotationsTypeDashboard}}, + }, { desc: "should be able to create organization annotation with correct permission", path: "/api/annotations", @@ -177,6 +282,30 @@ func TestAPI_Annotations(t *testing.T) { expectedCode: http.StatusForbidden, permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsDelete, Scope: accesscontrol.ScopeAnnotationsTypeOrganization}}, }, + { + desc: "should be able to delete dashboard annotation with correct dashboard scope with annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodDelete, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsDelete, Scope: dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashUID)}}, + }, + { + desc: "should be able to delete dashboard annotation with correct folder scope with annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodDelete, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsDelete, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)}}, + }, + { + desc: "should not be able to delete dashboard annotation with the old dashboard scope when annotationPermissionUpdate enabled", + path: "/api/annotations/2", + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + method: http.MethodDelete, + expectedCode: http.StatusForbidden, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsDelete, Scope: accesscontrol.ScopeAnnotationsTypeDashboard}}, + }, { desc: "should be able to delete organization annotation with correct permission", path: "/api/annotations/1", @@ -222,22 +351,59 @@ func TestAPI_Annotations(t *testing.T) { expectedCode: http.StatusForbidden, permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsDelete, Scope: accesscontrol.ScopeAnnotationsTypeOrganization}}, }, + { + desc: "should be able to mass delete dashboard annotation with correct dashboard scope with annotationPermissionUpdate enabled", + path: "/api/annotations/mass-delete", + body: "{\"dashboardId\": 2, \"panelId\": 1}", + method: http.MethodPost, + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsDelete, Scope: dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashUID)}}, + }, + { + desc: "should be able to mass delete dashboard annotation with correct folder scope with annotationPermissionUpdate enabled", + path: "/api/annotations/mass-delete", + body: "{\"dashboardId\": 2, \"panelId\": 1}", + method: http.MethodPost, + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + expectedCode: http.StatusOK, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsDelete, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderUID)}}, + }, + { + desc: "should not be able to mass delete dashboard annotation with the old dashboard scope when annotationPermissionUpdate enabled", + path: "/api/annotations/mass-delete", + body: "{\"dashboardId\": 2, \"panelId\": 1}", + method: http.MethodPost, + featureFlags: []any{featuremgmt.FlagAnnotationPermissionUpdate}, + expectedCode: http.StatusForbidden, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionAnnotationsDelete, Scope: accesscontrol.ScopeAnnotationsTypeDashboard}}, + }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - setUpRBACGuardian(t) + // Don't need access to dashboards if annotationPermissionUpdate is enabled + if len(tt.featureFlags) == 0 { + setUpRBACGuardian(t) + } server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() repo := annotationstest.NewFakeAnnotationsRepo() _ = repo.Save(context.Background(), &annotations.Item{ID: 1, DashboardID: 0}) _ = repo.Save(context.Background(), &annotations.Item{ID: 2, DashboardID: 1}) hs.annotationsRepo = repo + hs.Features = featuremgmt.WithFeatures(tt.featureFlags...) + dashService := &dashboards.FakeDashboardService{} + dashService.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{UID: dashUID, FolderUID: folderUID, FolderID: 1}, nil) + folderService := &foldertest.FakeService{} + folderService.ExpectedFolder = &folder.Folder{UID: folderUID, ID: 1} + folderDB := &foldertest.FakeFolderStore{} + folderDB.On("GetFolderByID", mock.Anything, mock.Anything, mock.Anything).Return(&folder.Folder{UID: folderUID, ID: 1}, nil) + hs.DashboardService = dashService + hs.folderService = folderService hs.AccessControl = acimpl.ProvideAccessControl(hs.Cfg) - features := featuremgmt.WithFeatures() - dashSvc := &dashboards.FakeDashboardService{} - folderSvc := &foldertest.FakeService{} - hs.AccessControl.RegisterScopeAttributeResolver(AnnotationTypeScopeResolver(hs.annotationsRepo, features, dashSvc, folderSvc)) + hs.AccessControl.RegisterScopeAttributeResolver(AnnotationTypeScopeResolver(hs.annotationsRepo, hs.Features, dashService, folderService)) + hs.AccessControl.RegisterScopeAttributeResolver(dashboards.NewDashboardIDScopeResolver(folderDB, dashService, folderService)) }) var body io.Reader if tt.body != "" { @@ -252,6 +418,7 @@ func TestAPI_Annotations(t *testing.T) { }) } } + func TestService_AnnotationTypeScopeResolver(t *testing.T) { rootDashUID := "root-dashboard" folderDashUID := "folder-dashboard" diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 3694037f6c3..9d5439236b2 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -144,7 +144,11 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response } annotationPermissions := &dtos.AnnotationPermission{} - hs.getAnnotationPermissionsByScope(c, &annotationPermissions.Dashboard, accesscontrol.ScopeAnnotationsTypeDashboard) + if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { + hs.getAnnotationPermissionsByScope(c, &annotationPermissions.Dashboard, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dash.UID)) + } else { + hs.getAnnotationPermissionsByScope(c, &annotationPermissions.Dashboard, accesscontrol.ScopeAnnotationsTypeDashboard) + } hs.getAnnotationPermissionsByScope(c, &annotationPermissions.Organization, accesscontrol.ScopeAnnotationsTypeOrganization) meta := dtos.DashboardMeta{ diff --git a/public/app/features/dashboard/state/DashboardModel.ts b/public/app/features/dashboard/state/DashboardModel.ts index c008a075363..31af7a3a5d3 100644 --- a/public/app/features/dashboard/state/DashboardModel.ts +++ b/public/app/features/dashboard/state/DashboardModel.ts @@ -1181,6 +1181,10 @@ export class DashboardModel implements TimeModel { } else { canEdit = !!this.meta.annotationsPermissions?.dashboard.canEdit; } + + if (config.featureToggles.annotationPermissionUpdate) { + return canEdit; + } return this.canEditDashboard() && canEdit; } @@ -1193,18 +1197,26 @@ export class DashboardModel implements TimeModel { } else { canDelete = !!this.meta.annotationsPermissions?.dashboard.canDelete; } + + if (config.featureToggles.annotationPermissionUpdate) { + return canDelete; + } return canDelete && this.canEditDashboard(); } canAddAnnotations() { // When the builtin annotations are disabled, we should not add any in the UI const found = this.annotations.list.find((item) => item.builtIn === 1); - if (found?.enable === false || !this.canEditDashboard()) { + if (found?.enable === false) { return false; } // If RBAC is enabled there are additional conditions to check. - return Boolean(this.meta.annotationsPermissions?.dashboard.canAdd); + if (config.featureToggles.annotationPermissionUpdate) { + return Boolean(this.meta.annotationsPermissions?.dashboard.canAdd); + } + + return Boolean(this.meta.annotationsPermissions?.dashboard.canAdd) && this.canEditDashboard(); } canEditDashboard() {