From a861b1b9bad6c752a5466b1a1a08ee7a8e613399 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Mon, 12 Jun 2017 23:05:32 +0200 Subject: [PATCH] WIP: check permissions for delete/post dashboard --- pkg/api/api.go | 4 +- pkg/api/dashboard.go | 46 ++++-- pkg/api/dashboard_test.go | 229 ++++++++++++++++++++++++------ pkg/services/guardian/guardian.go | 4 +- 4 files changed, 226 insertions(+), 57 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 5afec7e792e..94dc6c98d7f 100644 --- a/pkg/api/api.go +++ b/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) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 0eeeea94056..d59d9f9a065 100644 --- a/pkg/api/dashboard.go +++ b/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()}) diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 7ace884a8ae..f66805650e4 100644 --- a/pkg/api/dashboard_test.go +++ b/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) + }) + + 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 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) - - dash := dtos.DashboardFullWithMeta{} - err := json.NewDecoder(sc.resp.Body).Decode(&dash) - So(err, ShouldBeNil) + 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) + }) + + 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", 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() + role := models.ROLE_EDITOR - 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 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,21 +227,96 @@ 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 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() + + So(sc.resp.Code, ShouldEqual, 200) + + dash := dtos.DashboardFullWithMeta{} + err := json.NewDecoder(sc.resp.Body).Decode(&dash) + So(err, ShouldBeNil) + + 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) + }) +} diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 81669510f4b..3ed65c85465 100644 --- a/pkg/services/guardian/guardian.go +++ b/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 }