From 7d9a9fa29cbe88e9744a2ff76d2f8e5b35bf66ac Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Tue, 20 Feb 2018 23:26:08 +0100 Subject: [PATCH] snapshots: change to snapshot list query Admins can see all snapshots. Other roles can only see their own snapshots. Added permission check for deleting snapshots - admins can delete any snapshot, other roles can delete their own snapshots or snapshots that they have access to via dashboard permissions. --- pkg/api/api.go | 2 +- pkg/api/dashboard_snapshot.go | 42 ++++++-- pkg/api/dashboard_snapshot_test.go | 97 +++++++++++++++++++ pkg/models/dashboard_snapshot.go | 10 +- pkg/services/sqlstore/dashboard_snapshot.go | 15 ++- .../sqlstore/dashboard_snapshot_test.go | 73 +++++++++++++- 6 files changed, 223 insertions(+), 16 deletions(-) create mode 100644 pkg/api/dashboard_snapshot_test.go diff --git a/pkg/api/api.go b/pkg/api/api.go index 1320663f630..8faf2ed5dde 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -106,7 +106,7 @@ func (hs *HttpServer) registerRoutes() { r.Post("/api/snapshots/", bind(m.CreateDashboardSnapshotCommand{}), CreateDashboardSnapshot) r.Get("/api/snapshot/shared-options/", GetSharingOptions) r.Get("/api/snapshots/:key", GetDashboardSnapshot) - r.Get("/api/snapshots-delete/:key", reqEditorRole, DeleteDashboardSnapshot) + r.Get("/api/snapshots-delete/:key", reqEditorRole, wrap(DeleteDashboardSnapshot)) // api renew session based on remember cookie r.Get("/api/login/ping", quota("session"), LoginApiPing) diff --git a/pkg/api/dashboard_snapshot.go b/pkg/api/dashboard_snapshot.go index a834bd4717d..c10302faf32 100644 --- a/pkg/api/dashboard_snapshot.go +++ b/pkg/api/dashboard_snapshot.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/metrics" "github.com/grafana/grafana/pkg/middleware" m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -56,6 +57,7 @@ func CreateDashboardSnapshot(c *middleware.Context, cmd m.CreateDashboardSnapsho }) } +// GET /api/snapshots/:key func GetDashboardSnapshot(c *middleware.Context) { key := c.Params(":key") query := &m.GetDashboardSnapshotQuery{Key: key} @@ -90,18 +92,43 @@ func GetDashboardSnapshot(c *middleware.Context) { c.JSON(200, dto) } -func DeleteDashboardSnapshot(c *middleware.Context) { +// GET /api/snapshots-delete/:key +func DeleteDashboardSnapshot(c *middleware.Context) Response { key := c.Params(":key") + + query := &m.GetDashboardSnapshotQuery{DeleteKey: key} + + err := bus.Dispatch(query) + if err != nil { + return ApiError(500, "Failed to get dashboard snapshot", err) + } + + if query.Result == nil { + return ApiError(404, "Failed to get dashboard snapshot", nil) + } + dashboard := query.Result.Dashboard + dashboardId := dashboard.Get("id").MustInt64() + + guardian := guardian.New(dashboardId, c.OrgId, c.SignedInUser) + canEdit, err := guardian.CanEdit() + if err != nil { + return ApiError(500, "Error while checking permissions for snapshot", err) + } + + if !canEdit && query.Result.UserId != c.SignedInUser.UserId { + return ApiError(403, "Access denied to this snapshot", nil) + } + cmd := &m.DeleteDashboardSnapshotCommand{DeleteKey: key} if err := bus.Dispatch(cmd); err != nil { - c.JsonApiErr(500, "Failed to delete dashboard snapshot", err) - return + return ApiError(500, "Failed to delete dashboard snapshot", err) } - c.JSON(200, util.DynMap{"message": "Snapshot deleted. It might take an hour before it's cleared from a CDN cache."}) + return Json(200, util.DynMap{"message": "Snapshot deleted. It might take an hour before it's cleared from a CDN cache."}) } +// GET /api/dashboard/snapshots func SearchDashboardSnapshots(c *middleware.Context) Response { query := c.Query("query") limit := c.QueryInt("limit") @@ -111,9 +138,10 @@ func SearchDashboardSnapshots(c *middleware.Context) Response { } searchQuery := m.GetDashboardSnapshotsQuery{ - Name: query, - Limit: limit, - OrgId: c.OrgId, + Name: query, + Limit: limit, + OrgId: c.OrgId, + SignedInUser: c.SignedInUser, } err := bus.Dispatch(&searchQuery) diff --git a/pkg/api/dashboard_snapshot_test.go b/pkg/api/dashboard_snapshot_test.go new file mode 100644 index 00000000000..87c2b9e99d4 --- /dev/null +++ b/pkg/api/dashboard_snapshot_test.go @@ -0,0 +1,97 @@ +package api + +import ( + "testing" + "time" + + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/components/simplejson" + m "github.com/grafana/grafana/pkg/models" + + . "github.com/smartystreets/goconvey/convey" +) + +func TestDashboardSnapshotApiEndpoint(t *testing.T) { + Convey("Given a single snapshot", t, func() { + jsonModel, _ := simplejson.NewJson([]byte(`{"id":100}`)) + + mockSnapshotResult := &m.DashboardSnapshot{ + Id: 1, + Dashboard: jsonModel, + Expires: time.Now().Add(time.Duration(1000) * time.Second), + UserId: 999999, + } + + bus.AddHandler("test", func(query *m.GetDashboardSnapshotQuery) error { + query.Result = mockSnapshotResult + return nil + }) + + bus.AddHandler("test", func(cmd *m.DeleteDashboardSnapshotCommand) error { + return nil + }) + + viewerRole := m.ROLE_VIEWER + editorRole := m.ROLE_EDITOR + aclMockResp := []*m.DashboardAclInfoDTO{} + bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { + query.Result = aclMockResp + return nil + }) + + teamResp := []*m.Team{} + bus.AddHandler("test", func(query *m.GetTeamsByUserQuery) error { + query.Result = teamResp + return nil + }) + + Convey("When user has editor role and is not in the ACL", func() { + Convey("Should not be able to delete snapshot", func() { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/snapshots-delete/12345", "/api/snapshots-delete/:key", m.ROLE_EDITOR, func(sc *scenarioContext) { + sc.handlerFunc = DeleteDashboardSnapshot + sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec() + + So(sc.resp.Code, ShouldEqual, 403) + }) + }) + }) + + Convey("When user is editor and dashboard has default ACL", func() { + aclMockResp = []*m.DashboardAclInfoDTO{ + {Role: &viewerRole, Permission: m.PERMISSION_VIEW}, + {Role: &editorRole, Permission: m.PERMISSION_EDIT}, + } + + Convey("Should be able to delete a snapshot", func() { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/snapshots-delete/12345", "/api/snapshots-delete/:key", m.ROLE_EDITOR, func(sc *scenarioContext) { + sc.handlerFunc = DeleteDashboardSnapshot + sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec() + + So(sc.resp.Code, ShouldEqual, 200) + respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + So(err, ShouldBeNil) + + So(respJSON.Get("message").MustString(), ShouldStartWith, "Snapshot deleted") + }) + }) + }) + + Convey("When user is editor and is the creator of the snapshot", func() { + aclMockResp = []*m.DashboardAclInfoDTO{} + mockSnapshotResult.UserId = TestUserID + + Convey("Should be able to delete a snapshot", func() { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/snapshots-delete/12345", "/api/snapshots-delete/:key", m.ROLE_EDITOR, func(sc *scenarioContext) { + sc.handlerFunc = DeleteDashboardSnapshot + sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec() + + So(sc.resp.Code, ShouldEqual, 200) + respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + So(err, ShouldBeNil) + + So(respJSON.Get("message").MustString(), ShouldStartWith, "Snapshot deleted") + }) + }) + }) + }) +} diff --git a/pkg/models/dashboard_snapshot.go b/pkg/models/dashboard_snapshot.go index 9273b88f291..ccb154f4a69 100644 --- a/pkg/models/dashboard_snapshot.go +++ b/pkg/models/dashboard_snapshot.go @@ -67,7 +67,8 @@ type DeleteExpiredSnapshotsCommand struct { } type GetDashboardSnapshotQuery struct { - Key string + Key string + DeleteKey string Result *DashboardSnapshot } @@ -76,9 +77,10 @@ type DashboardSnapshots []*DashboardSnapshot type DashboardSnapshotsList []*DashboardSnapshotDTO type GetDashboardSnapshotsQuery struct { - Name string - Limit int - OrgId int64 + Name string + Limit int + OrgId int64 + SignedInUser *SignedInUser Result DashboardSnapshotsList } diff --git a/pkg/services/sqlstore/dashboard_snapshot.go b/pkg/services/sqlstore/dashboard_snapshot.go index 0ef7f99da67..84850b2ec48 100644 --- a/pkg/services/sqlstore/dashboard_snapshot.go +++ b/pkg/services/sqlstore/dashboard_snapshot.go @@ -72,7 +72,7 @@ func DeleteDashboardSnapshot(cmd *m.DeleteDashboardSnapshotCommand) error { } func GetDashboardSnapshot(query *m.GetDashboardSnapshotQuery) error { - snapshot := m.DashboardSnapshot{Key: query.Key} + snapshot := m.DashboardSnapshot{Key: query.Key, DeleteKey: query.DeleteKey} has, err := x.Get(&snapshot) if err != nil { @@ -85,6 +85,8 @@ func GetDashboardSnapshot(query *m.GetDashboardSnapshotQuery) error { return nil } +// SearchDashboardSnapshots returns a list of all snapshots for admins +// for other roles, it returns snapshots created by the user func SearchDashboardSnapshots(query *m.GetDashboardSnapshotsQuery) error { var snapshots = make(m.DashboardSnapshotsList, 0) @@ -95,7 +97,16 @@ func SearchDashboardSnapshots(query *m.GetDashboardSnapshotsQuery) error { sess.Where("name LIKE ?", query.Name) } - sess.Where("org_id = ?", query.OrgId) + // admins can see all snapshots, everyone else can only see their own snapshots + if query.SignedInUser.OrgRole == m.ROLE_ADMIN { + sess.Where("org_id = ?", query.OrgId) + } else if !query.SignedInUser.IsAnonymous { + sess.Where("org_id = ? AND user_id = ?", query.OrgId, query.SignedInUser.UserId) + } else { + query.Result = snapshots + return nil + } + err := sess.Find(&snapshots) query.Result = snapshots return err diff --git a/pkg/services/sqlstore/dashboard_snapshot_test.go b/pkg/services/sqlstore/dashboard_snapshot_test.go index 50375088b4b..74a26068ece 100644 --- a/pkg/services/sqlstore/dashboard_snapshot_test.go +++ b/pkg/services/sqlstore/dashboard_snapshot_test.go @@ -14,17 +14,19 @@ func TestDashboardSnapshotDBAccess(t *testing.T) { Convey("Testing DashboardSnapshot data access", t, func() { InitTestDB(t) - Convey("Given saved snaphot", func() { + Convey("Given saved snapshot", func() { cmd := m.CreateDashboardSnapshotCommand{ Key: "hej", Dashboard: simplejson.NewFromAny(map[string]interface{}{ "hello": "mupp", }), + UserId: 1000, + OrgId: 1, } err := CreateDashboardSnapshot(&cmd) So(err, ShouldBeNil) - Convey("Should be able to get snaphot by key", func() { + Convey("Should be able to get snapshot by key", func() { query := m.GetDashboardSnapshotQuery{Key: "hej"} err = GetDashboardSnapshot(&query) So(err, ShouldBeNil) @@ -33,6 +35,73 @@ func TestDashboardSnapshotDBAccess(t *testing.T) { So(query.Result.Dashboard.Get("hello").MustString(), ShouldEqual, "mupp") }) + Convey("And the user has the admin role", func() { + Convey("Should return all the snapshots", func() { + query := m.GetDashboardSnapshotsQuery{ + OrgId: 1, + SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_ADMIN}, + } + err := SearchDashboardSnapshots(&query) + So(err, ShouldBeNil) + + So(query.Result, ShouldNotBeNil) + So(len(query.Result), ShouldEqual, 1) + }) + }) + + Convey("And the user has the editor role and has created a snapshot", func() { + Convey("Should return all the snapshots", func() { + query := m.GetDashboardSnapshotsQuery{ + OrgId: 1, + SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_EDITOR, UserId: 1000}, + } + err := SearchDashboardSnapshots(&query) + So(err, ShouldBeNil) + + So(query.Result, ShouldNotBeNil) + So(len(query.Result), ShouldEqual, 1) + }) + }) + + Convey("And the user has the editor role and has not created any snapshot", func() { + Convey("Should not return any snapshots", func() { + query := m.GetDashboardSnapshotsQuery{ + OrgId: 1, + SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_EDITOR, UserId: 2}, + } + err := SearchDashboardSnapshots(&query) + So(err, ShouldBeNil) + + So(query.Result, ShouldNotBeNil) + So(len(query.Result), ShouldEqual, 0) + }) + }) + + Convey("And the user is anonymous", func() { + cmd := m.CreateDashboardSnapshotCommand{ + Key: "strangesnapshotwithuserid0", + DeleteKey: "adeletekey", + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "hello": "mupp", + }), + UserId: 0, + OrgId: 1, + } + err := CreateDashboardSnapshot(&cmd) + So(err, ShouldBeNil) + + Convey("Should not return any snapshots", func() { + query := m.GetDashboardSnapshotsQuery{ + OrgId: 1, + SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_EDITOR, IsAnonymous: true, UserId: 0}, + } + err := SearchDashboardSnapshots(&query) + So(err, ShouldBeNil) + + So(query.Result, ShouldNotBeNil) + So(len(query.Result), ShouldEqual, 0) + }) + }) }) }) }