From 417db13efbcc2a4b2e5ef60ba8b93bec2a9bda1e Mon Sep 17 00:00:00 2001 From: Dan Cech Date: Thu, 24 May 2018 02:55:16 -0400 Subject: [PATCH] Fix dashboard snapshot deletion (#12025) * fix issue deleting dashboard snapshots by deleteKey, add dedicated endopoint for authenticated requests, update frontend, tests & docs. --- docs/sources/http_api/snapshot.md | 75 +++++++++++++++++-- pkg/api/api.go | 3 +- pkg/api/common_test.go | 25 +++++++ pkg/api/dashboard_snapshot.go | 29 +++++-- pkg/api/dashboard_snapshot_test.go | 27 +++++-- pkg/models/dashboard_snapshot.go | 1 - public/app/features/snapshot/snapshot_ctrl.ts | 7 +- 7 files changed, 141 insertions(+), 26 deletions(-) diff --git a/docs/sources/http_api/snapshot.md b/docs/sources/http_api/snapshot.md index dce3b0a9160..5a76d0118b3 100644 --- a/docs/sources/http_api/snapshot.md +++ b/docs/sources/http_api/snapshot.md @@ -70,7 +70,7 @@ JSON Body schema: Content-Type: application/json { "deleteKey":"XXXXXXX", - "deleteUrl":"myurl/dashboard/snapshot/XXXXXXX", + "deleteUrl":"myurl/api/snapshots-delete/XXXXXXX", "key":"YYYYYYY", "url":"myurl/dashboard/snapshot/YYYYYYY" } @@ -81,7 +81,46 @@ Keys: - **deleteKey** – Key generated to delete the snapshot - **key** – Key generated to share the dashboard -## Get Snapshot by Id +## Get list of Snapshots + +`GET /api/dashboard/snapshots` + +Query parameters: + +- **query** – Search Query +- **limit** – Limit the number of returned results + +**Example Request**: + +```http +GET /api/dashboard/snapshots HTTP/1.1 +Accept: application/json +Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk +``` + +**Example Response**: + +```http +HTTP/1.1 200 +Content-Type: application/json + +[ + { + "id":8, + "name":"Home", + "key":"YYYYYYY", + "orgId":1, + "userId":1, + "external":false, + "externalUrl":"", + "expires":"2200-13-32T25:23:23+02:00", + "created":"2200-13-32T28:24:23+02:00", + "updated":"2200-13-32T28:24:23+02:00" + } +] +``` + +## Get Snapshot by Key `GET /api/snapshots/:key` @@ -90,7 +129,6 @@ Keys: ```http GET /api/snapshots/YYYYYYY HTTP/1.1 Accept: application/json -Content-Type: application/json Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk ``` @@ -140,16 +178,15 @@ Content-Type: application/json } ``` -## Delete Snapshot by deleteKey +## Delete Snapshot by Key -`GET /api/snapshots-delete/:deleteKey` +`DELETE /api/snapshots/:key` **Example Request**: ```http -GET /api/snapshots/YYYYYYY HTTP/1.1 +DELETE /api/snapshots/YYYYYYY HTTP/1.1 Accept: application/json -Content-Type: application/json Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk ``` @@ -159,5 +196,27 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk HTTP/1.1 200 Content-Type: application/json -{"message":"Snapshot deleted. It might take an hour before it's cleared from a CDN cache."} +{"message":"Snapshot deleted. It might take an hour before it's cleared from any CDN caches."} +``` + +## Delete Snapshot by deleteKey + +This API call can be used without authentication by using the secret delete key for the snapshot. + +`GET /api/snapshots-delete/:deleteKey` + +**Example Request**: + +```http +GET /api/snapshots-delete/XXXXXXX HTTP/1.1 +Accept: application/json +``` + +**Example Response**: + +```http +HTTP/1.1 200 +Content-Type: application/json + +{"message":"Snapshot deleted. It might take an hour before it's cleared from any CDN caches."} ``` \ No newline at end of file diff --git a/pkg/api/api.go b/pkg/api/api.go index 493f9eb9d01..b44d2827506 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -107,7 +107,8 @@ 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, wrap(DeleteDashboardSnapshot)) + r.Get("/api/snapshots-delete/:deleteKey", wrap(DeleteDashboardSnapshotByDeleteKey)) + r.Delete("/api/snapshots/:key", reqEditorRole, wrap(DeleteDashboardSnapshot)) // api renew session based on remember cookie r.Get("/api/login/ping", quota("session"), LoginAPIPing) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index a4a547d8bbf..40c438b607a 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -46,6 +46,31 @@ func loggedInUserScenarioWithRole(desc string, method string, url string, routeP }) } +func anonymousUserScenario(desc string, method string, url string, routePattern string, fn scenarioFunc) { + Convey(desc+" "+url, func() { + defer bus.ClearBusHandlers() + + sc := setupScenarioContext(url) + sc.defaultHandler = wrap(func(c *m.ReqContext) Response { + sc.context = c + if sc.handlerFunc != nil { + return sc.handlerFunc(sc.context) + } + + return nil + }) + + switch method { + case "GET": + sc.m.Get(routePattern, sc.defaultHandler) + case "DELETE": + sc.m.Delete(routePattern, sc.defaultHandler) + } + + fn(sc) + }) +} + func (sc *scenarioContext) fakeReq(method, url string) *scenarioContext { sc.resp = httptest.NewRecorder() req, err := http.NewRequest(method, url, nil) diff --git a/pkg/api/dashboard_snapshot.go b/pkg/api/dashboard_snapshot.go index f474d357df5..e4e9c9d040f 100644 --- a/pkg/api/dashboard_snapshot.go +++ b/pkg/api/dashboard_snapshot.go @@ -91,11 +91,31 @@ func GetDashboardSnapshot(c *m.ReqContext) { c.JSON(200, dto) } -// GET /api/snapshots-delete/:key +// GET /api/snapshots-delete/:deleteKey +func DeleteDashboardSnapshotByDeleteKey(c *m.ReqContext) Response { + key := c.Params(":deleteKey") + + query := &m.GetDashboardSnapshotQuery{DeleteKey: key} + + err := bus.Dispatch(query) + if err != nil { + return Error(500, "Failed to get dashboard snapshot", err) + } + + cmd := &m.DeleteDashboardSnapshotCommand{DeleteKey: query.Result.DeleteKey} + + if err := bus.Dispatch(cmd); err != nil { + return Error(500, "Failed to delete dashboard snapshot", err) + } + + return JSON(200, util.DynMap{"message": "Snapshot deleted. It might take an hour before it's cleared from any CDN caches."}) +} + +// DELETE /api/snapshots/:key func DeleteDashboardSnapshot(c *m.ReqContext) Response { key := c.Params(":key") - query := &m.GetDashboardSnapshotQuery{DeleteKey: key} + query := &m.GetDashboardSnapshotQuery{Key: key} err := bus.Dispatch(query) if err != nil { @@ -118,13 +138,13 @@ func DeleteDashboardSnapshot(c *m.ReqContext) Response { return Error(403, "Access denied to this snapshot", nil) } - cmd := &m.DeleteDashboardSnapshotCommand{DeleteKey: key} + cmd := &m.DeleteDashboardSnapshotCommand{DeleteKey: query.Result.DeleteKey} if err := bus.Dispatch(cmd); err != nil { return Error(500, "Failed to delete dashboard snapshot", err) } - return 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 any CDN caches."}) } // GET /api/dashboard/snapshots @@ -154,7 +174,6 @@ func SearchDashboardSnapshots(c *m.ReqContext) Response { Id: snapshot.Id, Name: snapshot.Name, Key: snapshot.Key, - DeleteKey: snapshot.DeleteKey, OrgId: snapshot.OrgId, UserId: snapshot.UserId, External: snapshot.External, diff --git a/pkg/api/dashboard_snapshot_test.go b/pkg/api/dashboard_snapshot_test.go index 87c2b9e99d4..5e7637a24e1 100644 --- a/pkg/api/dashboard_snapshot_test.go +++ b/pkg/api/dashboard_snapshot_test.go @@ -47,15 +47,30 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) { 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) { + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) { sc.handlerFunc = DeleteDashboardSnapshot - sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec() + sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec() So(sc.resp.Code, ShouldEqual, 403) }) }) }) + Convey("When user is anonymous", func() { + Convey("Should be able to delete snapshot by deleteKey", func() { + anonymousUserScenario("When calling GET on", "GET", "/api/snapshots-delete/12345", "/api/snapshots-delete/:deleteKey", func(sc *scenarioContext) { + sc.handlerFunc = DeleteDashboardSnapshotByDeleteKey + sc.fakeReqWithParams("GET", sc.url, map[string]string{"deleteKey": "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 dashboard has default ACL", func() { aclMockResp = []*m.DashboardAclInfoDTO{ {Role: &viewerRole, Permission: m.PERMISSION_VIEW}, @@ -63,9 +78,9 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) { } 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) { + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) { sc.handlerFunc = DeleteDashboardSnapshot - sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec() + sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec() So(sc.resp.Code, ShouldEqual, 200) respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) @@ -81,9 +96,9 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) { 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) { + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) { sc.handlerFunc = DeleteDashboardSnapshot - sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec() + sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec() So(sc.resp.Code, ShouldEqual, 200) respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) diff --git a/pkg/models/dashboard_snapshot.go b/pkg/models/dashboard_snapshot.go index ec8b19f3c18..3024ba94122 100644 --- a/pkg/models/dashboard_snapshot.go +++ b/pkg/models/dashboard_snapshot.go @@ -29,7 +29,6 @@ type DashboardSnapshotDTO struct { Id int64 `json:"id"` Name string `json:"name"` Key string `json:"key"` - DeleteKey string `json:"deleteKey"` OrgId int64 `json:"orgId"` UserId int64 `json:"userId"` External bool `json:"external"` diff --git a/public/app/features/snapshot/snapshot_ctrl.ts b/public/app/features/snapshot/snapshot_ctrl.ts index 1edeafe0f6f..1dde4876cd5 100644 --- a/public/app/features/snapshot/snapshot_ctrl.ts +++ b/public/app/features/snapshot/snapshot_ctrl.ts @@ -15,12 +15,9 @@ export class SnapshotsCtrl { removeSnapshotConfirmed(snapshot) { _.remove(this.snapshots, { key: snapshot.key }); - this.backendSrv.get('/api/snapshots-delete/' + snapshot.deleteKey).then( + this.backendSrv.delete('/api/snapshots/' + snapshot.key).then( + () => {}, () => { - this.$rootScope.appEvent('alert-success', ['Snapshot deleted', '']); - }, - () => { - this.$rootScope.appEvent('alert-error', ['Unable to delete snapshot', '']); this.snapshots.push(snapshot); } );