WIP: check permissions for delete/post dashboard

pkg/api/api.go

@@ -234,7 +234,7 @@ func (hs *HttpServer) registerRoutes() {
 
 		// Dashboard
 		r.Group("/dashboards", func() {
-			r.Combo("/db/:slug").Get(wrap(GetDashboard)).Delete(DeleteDashboard)
+			r.Combo("/db/:slug").Get(wrap(GetDashboard)).Delete(wrap(DeleteDashboard))
 
 			r.Get("/id/:dashboardId/versions", wrap(GetDashboardVersions))
 			r.Get("/id/:dashboardId/versions/:id", wrap(GetDashboardVersion))
@@ -242,7 +242,7 @@ func (hs *HttpServer) registerRoutes() {
 
 			r.Post("/calculate-diff", bind(dtos.CalculateDiffOptions{}), wrap(CalculateDashboardDiff))
 
-			r.Post("/db", reqEditorRole, bind(m.SaveDashboardCommand{}), wrap(PostDashboard))
+			r.Post("/db", bind(m.SaveDashboardCommand{}), wrap(PostDashboard))
 			r.Get("/file/:file", GetDashboardFromJsonFile)
 			r.Get("/home", wrap(GetHomeDashboard))
 			r.Get("/tags", GetDashboardTags)

pkg/api/dashboard.go

@@ -47,7 +47,7 @@ func GetDashboard(c *middleware.Context) Response {
 
 	dash := query.Result
 
-	canView, canEdit, canSave, err := getPermissions(dash, c.OrgRole, c.IsGrafanaAdmin, c.OrgId, c.UserId)
+	canView, canEdit, canSave, err := getPermissions(dash, c.OrgRole, c.IsGrafanaAdmin, c.UserId)
 	if err != nil {
 		return ApiError(500, "Error while checking dashboard permissions", err)
 	}
@@ -97,7 +97,7 @@ func GetDashboard(c *middleware.Context) Response {
 	return Json(200, dto)
 }
 
-func getPermissions(dash *m.Dashboard, orgRole m.RoleType, isGrafanaAdmin bool, orgId int64, userId int64) (bool, bool, bool, error) {
+func getPermissions(dash *m.Dashboard, orgRole m.RoleType, isGrafanaAdmin bool, userId int64) (bool, bool, bool, error) {
 	if !dash.HasAcl {
 		return true, canEditDashboard(orgRole), orgRole == m.ROLE_ADMIN || orgRole == m.ROLE_EDITOR, nil
 	}
@@ -108,7 +108,7 @@ func getPermissions(dash *m.Dashboard, orgRole m.RoleType, isGrafanaAdmin bool,
 		dashId = dash.ParentId
 	}
 
-	canView, canEdit, canSave, err := guardian.CheckDashboardPermissions(dashId, orgRole, isGrafanaAdmin, orgId, userId)
+	canView, canEdit, canSave, err := guardian.CheckDashboardPermissions(dashId, orgRole, isGrafanaAdmin, userId)
 	if err != nil {
 		return false, false, false, err
 	}
@@ -127,24 +127,31 @@ func getUserLogin(userId int64) string {
 	}
 }
 
-func DeleteDashboard(c *middleware.Context) {
+func DeleteDashboard(c *middleware.Context) Response {
 	slug := c.Params(":slug")
 
 	query := m.GetDashboardQuery{Slug: slug, OrgId: c.OrgId}
 	if err := bus.Dispatch(&query); err != nil {
-		c.JsonApiErr(404, "Dashboard not found", nil)
-		return
+		return ApiError(404, "Dashboard not found", err)
+	}
+
+	_, _, canSave, err := getPermissions(query.Result, c.OrgRole, c.IsGrafanaAdmin, c.UserId)
+	if err != nil {
+		return ApiError(500, "Error while checking dashboard permissions", err)
+	}
+
+	if !canSave {
+		return ApiError(403, "Does not have permission to delete this dashboard", nil)
 	}
 
 	cmd := m.DeleteDashboardCommand{Slug: slug, OrgId: c.OrgId}
 	if err := bus.Dispatch(&cmd); err != nil {
-		c.JsonApiErr(500, "Failed to delete dashboard", err)
-		return
+		return ApiError(500, "Failed to delete dashboard", err)
 	}
 
 	var resp = map[string]interface{}{"title": query.Result.Title}
 
-	c.JSON(200, resp)
+	return Json(200, resp)
 }
 
 func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
@@ -153,6 +160,25 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
 
 	dash := cmd.GetDashboardModel()
 
+	query := m.GetDashboardQuery{Slug: dash.Slug, OrgId: c.OrgId}
+	err := bus.Dispatch(&query)
+	if err == nil {
+		dash.IsFolder = query.Result.IsFolder
+		if cmd.ParentId == 0 {
+			dash.ParentId = query.Result.ParentId
+		}
+		dash.HasAcl = query.Result.HasAcl
+	}
+
+	_, _, canSave, err := getPermissions(dash, c.OrgRole, c.IsGrafanaAdmin, c.UserId)
+	if err != nil {
+		return ApiError(500, "Error while checking dashboard permissions", err)
+	}
+
+	if !canSave {
+		return ApiError(403, "Does not have permission to save this dashboard", nil)
+	}
+
 	// Check if Title is empty
 	if dash.Title == "" {
 		return ApiError(400, m.ErrDashboardTitleEmpty.Error(), nil)
@@ -178,7 +204,7 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
 		return ApiError(500, "Invalid alert data. Cannot save dashboard", err)
 	}
 
-	err := bus.Dispatch(&cmd)
+	err = bus.Dispatch(&cmd)
 	if err != nil {
 		if err == m.ErrDashboardWithSameNameExists {
 			return Json(412, util.DynMap{"status": "name-exists", "message": err.Error()})

pkg/api/dashboard_test.go

@@ -2,11 +2,18 @@ package api
 
 import (
 	"encoding/json"
+	"path/filepath"
 	"testing"
 
+	macaron "gopkg.in/macaron.v1"
+
+	"github.com/go-macaron/session"
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/components/simplejson"
+	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/alerting"
 
 	. "github.com/smartystreets/goconvey/convey"
 )
@@ -22,56 +29,79 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			return nil
 		})
 
+		cmd := models.SaveDashboardCommand{
+			Dashboard: simplejson.NewFromAny(map[string]interface{}{
+				"parentId": fakeDash.ParentId,
+				"title":    fakeDash.Title,
+			}),
+		}
+
 		Convey("When user is an Org Viewer", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", models.ROLE_VIEWER, func(sc *scenarioContext) {
-				sc.handlerFunc = GetDashboard
-				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
-				So(sc.resp.Code, ShouldEqual, 200)
+			role := models.ROLE_VIEWER
 
-				dash := dtos.DashboardFullWithMeta{}
-				err := json.NewDecoder(sc.resp.Body).Decode(&dash)
-				So(err, ShouldBeNil)
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				dash := GetDashboardShouldReturn200(sc)
 
 				Convey("Should not be able to edit or save dashboard", func() {
 					So(dash.Meta.CanEdit, ShouldBeFalse)
 					So(dash.Meta.CanSave, ShouldBeFalse)
 				})
 			})
+
+			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				CallDeleteDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
 			})
 
-		Convey("When user is an Org Read Only Editor", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", models.ROLE_READ_ONLY_EDITOR, func(sc *scenarioContext) {
-				sc.handlerFunc = GetDashboard
-				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
-				So(sc.resp.Code, ShouldEqual, 200)
+			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
+				CallPostDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
+			})
+		})
 
-				dash := dtos.DashboardFullWithMeta{}
-				err := json.NewDecoder(sc.resp.Body).Decode(&dash)
-				So(err, ShouldBeNil)
+		Convey("When user is an Org Read Only Editor", func() {
+			role := models.ROLE_READ_ONLY_EDITOR
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				dash := GetDashboardShouldReturn200(sc)
 
 				Convey("Should be able to edit but not save the dashboard", func() {
 					So(dash.Meta.CanEdit, ShouldBeTrue)
 					So(dash.Meta.CanSave, ShouldBeFalse)
 				})
 			})
+
+			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				CallDeleteDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
 			})
 
-		Convey("When user is an Org Editor", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				sc.handlerFunc = GetDashboard
-				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
+			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
+				CallPostDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
+			})
+		})
 
-				So(sc.resp.Code, ShouldEqual, 200)
+		Convey("When user is an Org Editor", func() {
+			role := models.ROLE_EDITOR
 
-				dash := dtos.DashboardFullWithMeta{}
-				err := json.NewDecoder(sc.resp.Body).Decode(&dash)
-				So(err, ShouldBeNil)
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				dash := GetDashboardShouldReturn200(sc)
 
 				Convey("Should be able to edit or save dashboard", func() {
 					So(dash.Meta.CanEdit, ShouldBeTrue)
 					So(dash.Meta.CanSave, ShouldBeTrue)
 				})
 			})
+
+			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				CallDeleteDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 200)
+			})
+
+			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
+				CallPostDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 200)
+			})
 		})
 	})
 
@@ -90,13 +120,22 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			return nil
 		})
 
+		cmd := models.SaveDashboardCommand{
+			ParentId: fakeDash.ParentId,
+			Dashboard: simplejson.NewFromAny(map[string]interface{}{
+				"parentId": fakeDash.ParentId,
+				"title":    fakeDash.Title,
+			}),
+		}
+
 		Convey("When user is an Org Viewer and has no permissions for this dashboard", func() {
+			role := models.ROLE_VIEWER
 			bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
 				query.Result = []*models.DashboardAclInfoDTO{}
 				return nil
 			})
 
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", models.ROLE_VIEWER, func(sc *scenarioContext) {
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				sc.handlerFunc = GetDashboard
 				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
 
@@ -104,15 +143,27 @@ func TestDashboardApiEndpoint(t *testing.T) {
 					So(sc.resp.Code, ShouldEqual, 403)
 				})
 			})
+
+			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				CallDeleteDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
+			})
+
+			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
+				CallPostDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
+			})
 		})
 
 		Convey("When user is an Org Editor and has no permissions for this dashboard", func() {
+			role := models.ROLE_EDITOR
+
 			bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
 				query.Result = []*models.DashboardAclInfoDTO{}
 				return nil
 			})
 
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", models.ROLE_EDITOR, func(sc *scenarioContext) {
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				sc.handlerFunc = GetDashboard
 				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
 
@@ -120,9 +171,21 @@ func TestDashboardApiEndpoint(t *testing.T) {
 					So(sc.resp.Code, ShouldEqual, 403)
 				})
 			})
+
+			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				CallDeleteDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
+			})
+
+			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
+				CallPostDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
+			})
 		})
 
 		Convey("When user is an Org Viewer but has an edit permission", func() {
+			role := models.ROLE_VIEWER
+
 			mockResult := []*models.DashboardAclInfoDTO{
 				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, PermissionType: models.PERMISSION_EDIT},
 			}
@@ -132,24 +195,29 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				return nil
 			})
 
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", models.ROLE_VIEWER, func(sc *scenarioContext) {
-				sc.handlerFunc = GetDashboard
-				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
-
-				So(sc.resp.Code, ShouldEqual, 200)
-
-				dash := dtos.DashboardFullWithMeta{}
-				err := json.NewDecoder(sc.resp.Body).Decode(&dash)
-				So(err, ShouldBeNil)
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				dash := GetDashboardShouldReturn200(sc)
 
 				Convey("Should be able to get dashboard with edit rights", func() {
 					So(dash.Meta.CanEdit, ShouldBeTrue)
 					So(dash.Meta.CanSave, ShouldBeTrue)
 				})
 			})
+
+			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				CallDeleteDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 200)
+			})
+
+			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
+				CallPostDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 200)
+			})
 		})
 
 		Convey("When user is an Org Editor but has a view permission", func() {
+			role := models.ROLE_EDITOR
+
 			mockResult := []*models.DashboardAclInfoDTO{
 				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, PermissionType: models.PERMISSION_VIEW},
 			}
@@ -159,7 +227,29 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				return nil
 			})
 
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", models.ROLE_VIEWER, func(sc *scenarioContext) {
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				dash := GetDashboardShouldReturn200(sc)
+
+				Convey("Should not be able to edit or save dashboard", func() {
+					So(dash.Meta.CanEdit, ShouldBeFalse)
+					So(dash.Meta.CanSave, ShouldBeFalse)
+				})
+			})
+
+			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
+				CallDeleteDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
+			})
+
+			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
+				CallPostDashboard(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
+			})
+		})
+	})
+}
+
+func GetDashboardShouldReturn200(sc *scenarioContext) dtos.DashboardFullWithMeta {
 	sc.handlerFunc = GetDashboard
 	sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
 
@@ -169,11 +259,64 @@ func TestDashboardApiEndpoint(t *testing.T) {
 	err := json.NewDecoder(sc.resp.Body).Decode(&dash)
 	So(err, ShouldBeNil)
 
-				Convey("Should not be able to edit or save dashboard", func() {
-					So(dash.Meta.CanEdit, ShouldBeFalse)
-					So(dash.Meta.CanSave, ShouldBeFalse)
+	return dash
+}
+
+func CallDeleteDashboard(sc *scenarioContext) {
+	bus.AddHandler("test", func(cmd *models.DeleteDashboardCommand) error {
+		return nil
+	})
+
+	sc.handlerFunc = DeleteDashboard
+	sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
+}
+
+func CallPostDashboard(sc *scenarioContext) {
+	bus.AddHandler("test", func(cmd *alerting.ValidateDashboardAlertsCommand) error {
+		return nil
+	})
+
+	bus.AddHandler("test", func(cmd *models.SaveDashboardCommand) error {
+		cmd.Result = &models.Dashboard{Id: 2, Slug: "Dash", Version: 2}
+		return nil
 	})
+
+	bus.AddHandler("test", func(cmd *alerting.UpdateDashboardAlertsCommand) error {
+		return nil
 	})
+
+	sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
+}
+
+func postDashboardScenario(desc string, url string, routePattern string, role models.RoleType, cmd models.SaveDashboardCommand, fn scenarioFunc) {
+	Convey(desc+" "+url, func() {
+		defer bus.ClearBusHandlers()
+
+		sc := &scenarioContext{
+			url: url,
+		}
+		viewsPath, _ := filepath.Abs("../../public/views")
+
+		sc.m = macaron.New()
+		sc.m.Use(macaron.Renderer(macaron.RenderOptions{
+			Directory: viewsPath,
+			Delims:    macaron.Delims{Left: "[[", Right: "]]"},
+		}))
+
+		sc.m.Use(middleware.GetContextHandler())
+		sc.m.Use(middleware.Sessioner(&session.Options{}))
+
+		sc.defaultHandler = wrap(func(c *middleware.Context) Response {
+			sc.context = c
+			sc.context.UserId = TestUserID
+			sc.context.OrgId = TestOrgID
+			sc.context.OrgRole = role
+
+			return PostDashboard(c, cmd)
 		})
+
+		sc.m.Post(routePattern, sc.defaultHandler)
+
+		fn(sc)
 	})
 }

pkg/services/guardian/guardian.go

@@ -5,7 +5,7 @@ import (
 	m "github.com/grafana/grafana/pkg/models"
 )
 
-// RemoveRestrictedDashboards filters out dashboards from the list that the user does have access to
+// FilterRestrictedDashboards filters out dashboards from the list that the user does have access to
 func FilterRestrictedDashboards(dashList []int64, orgId int64, userId int64) ([]int64, error) {
 	user, err := getUser(userId)
 	if err != nil {
@@ -59,7 +59,7 @@ func CanDeleteFromAcl(dashboardId int64, role m.RoleType, isGrafanaAdmin bool, o
 }
 
 // CheckDashboardPermissions determines if a user has permission to view, edit or save a dashboard
-func CheckDashboardPermissions(dashboardId int64, role m.RoleType, isGrafanaAdmin bool, orgId int64, userId int64) (bool, bool, bool, error) {
+func CheckDashboardPermissions(dashboardId int64, role m.RoleType, isGrafanaAdmin bool, userId int64) (bool, bool, bool, error) {
 	if role == m.ROLE_ADMIN || isGrafanaAdmin {
 		return true, true, true, nil
 	}