check that the user has RBAC permissions to save dashboard annotation (#47882)

pkg/api/annotations.go

@@ -64,22 +64,7 @@ func (hs *HTTPServer) PostAnnotation(c *models.ReqContext) response.Response {
 		return response.Error(http.StatusBadRequest, "bad request data", err)
 	}
 
-	var canSave bool
+	if canSave, err := hs.canCreateAnnotation(c, cmd.DashboardId); err != nil || !canSave {
-	var err error
-	if cmd.DashboardId != 0 {
-		canSave, err = canSaveDashboardAnnotation(c, cmd.DashboardId)
-	} else { // organization annotations
-		if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
-			canSave = canSaveOrganizationAnnotation(c)
-		} else {
-			// This is an additional validation needed only for FGAC Organization Annotations.
-			// It is not possible to do it in the middleware because we need to look
-			// into the request to determine if this is a Organization annotation or not
-			canSave, err = hs.canCreateOrganizationAnnotation(c)
-		}
-	}
-
-	if err != nil || !canSave {
 		return dashboardGuardianResponse(err)
 	}
 
@@ -198,16 +183,7 @@ func (hs *HTTPServer) UpdateAnnotation(c *models.ReqContext) response.Response {
 		return resp
 	}
 
-	canSave := true
+	if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave {
-	if annotation.GetType() == annotations.Dashboard {
-		canSave, err = canSaveDashboardAnnotation(c, annotation.DashboardId)
-	} else {
-		if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
-			canSave = canSaveOrganizationAnnotation(c)
-		}
-	}
-
-	if err != nil || !canSave {
 		return dashboardGuardianResponse(err)
 	}
 
@@ -245,15 +221,7 @@ func (hs *HTTPServer) PatchAnnotation(c *models.ReqContext) response.Response {
 		return resp
 	}
 
-	canSave := true
+	if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave {
-	if annotation.GetType() == annotations.Dashboard {
-		canSave, err = canSaveDashboardAnnotation(c, annotation.DashboardId)
-	} else {
-		if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
-			canSave = canSaveOrganizationAnnotation(c)
-		}
-	}
-	if err != nil || !canSave {
 		return dashboardGuardianResponse(err)
 	}
 
@@ -364,16 +332,7 @@ func (hs *HTTPServer) DeleteAnnotationByID(c *models.ReqContext) response.Respon
 		return resp
 	}
 
-	canSave := true
+	if canSave, err := hs.canSaveAnnotation(c, annotation); err != nil || !canSave {
-	if annotation.GetType() == annotations.Dashboard {
-		canSave, err = canSaveDashboardAnnotation(c, annotation.DashboardId)
-	} else {
-		if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
-			canSave = canSaveOrganizationAnnotation(c)
-		}
-	}
-
-	if err != nil || !canSave {
 		return dashboardGuardianResponse(err)
 	}
 
@@ -388,7 +347,18 @@ func (hs *HTTPServer) DeleteAnnotationByID(c *models.ReqContext) response.Respon
 	return response.Success("Annotation deleted")
 }
 
-func canSaveDashboardAnnotation(c *models.ReqContext, dashboardID int64) (bool, error) {
+func (hs *HTTPServer) canSaveAnnotation(c *models.ReqContext, annotation *annotations.ItemDTO) (bool, error) {
+	if annotation.GetType() == annotations.Dashboard {
+		return canEditDashboard(c, annotation.DashboardId)
+	} else {
+		if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
+			return c.SignedInUser.HasRole(models.ROLE_EDITOR), nil
+		}
+		return true, nil
+	}
+}
+
+func canEditDashboard(c *models.ReqContext, dashboardID int64) (bool, error) {
 	guard := guardian.New(c.Req.Context(), dashboardID, c.OrgId, c.SignedInUser)
 	if canEdit, err := guard.CanEdit(); err != nil || !canEdit {
 		return false, err
@@ -397,10 +367,6 @@ func canSaveDashboardAnnotation(c *models.ReqContext, dashboardID int64) (bool,
 	return true, nil
 }
 
-func canSaveOrganizationAnnotation(c *models.ReqContext) bool {
-	return c.SignedInUser.HasRole(models.ROLE_EDITOR)
-}
-
 func findAnnotationByID(ctx context.Context, repo annotations.Repository, annotationID int64, user *models.SignedInUser) (*annotations.ItemDTO, response.Response) {
 	query := &annotations.ItemQuery{
 		AnnotationId: annotationID,
@@ -478,9 +444,23 @@ func AnnotationTypeScopeResolver() (string, accesscontrol.AttributeScopeResolveF
 	return accesscontrol.ScopeAnnotationsProvider.GetResourceScope(""), annotationTypeResolver
 }
 
-func (hs *HTTPServer) canCreateOrganizationAnnotation(c *models.ReqContext) (bool, error) {
+func (hs *HTTPServer) canCreateAnnotation(c *models.ReqContext, dashboardId int64) (bool, error) {
-	evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeOrganization)
+	if dashboardId != 0 {
-	return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator)
+		if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
+			evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeDashboard)
+			if canSave, err := hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator); err != nil || !canSave {
+				return canSave, err
+			}
+		}
+		return canEditDashboard(c, dashboardId)
+	} else { // organization annotations
+		if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
+			evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeOrganization)
+			return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator)
+		} else {
+			return c.SignedInUser.HasRole(models.ROLE_EDITOR), nil
+		}
+	}
 }
 
 func (hs *HTTPServer) canMassDeleteAnnotations(c *models.ReqContext, dashboardID int64) (bool, error) {
@@ -494,7 +474,7 @@ func (hs *HTTPServer) canMassDeleteAnnotations(c *models.ReqContext, dashboardID
 			return false, err
 		}
 
-		canSave, err = canSaveDashboardAnnotation(c, dashboardID)
+		canSave, err = canEditDashboard(c, dashboardID)
 		if err != nil || !canSave {
 			return false, err
 		}

pkg/api/annotations_test.go

@@ -615,6 +615,18 @@ func TestAPI_Annotations_AccessControl(t *testing.T) {
 			},
 			want: http.StatusForbidden,
 		},
+		{
+			name: "AccessControl create dashboard annotation with incorrect permissions is forbidden",
+			args: args{
+				permissions: []*accesscontrol.Permission{{
+					Action: accesscontrol.ActionAnnotationsCreate, Scope: accesscontrol.ScopeAnnotationsTypeOrganization,
+				}},
+				url:    "/api/annotations",
+				method: http.MethodPost,
+				body:   mockRequestBody(postDashboardCmd),
+			},
+			want: http.StatusForbidden,
+		},
 		{
 			name: "AccessControl create organization annotation with permissions is allowed",
 			args: args{