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

@@ -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,17 +178,38 @@ Content-Type: application/json
 }
 ```
 
+## Delete Snapshot by Key
+
+`DELETE /api/snapshots/:key`
+
+**Example Request**:
+
+```http
+DELETE /api/snapshots/YYYYYYY HTTP/1.1
+Accept: application/json
+Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
+```
+
+**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."}
+```
+
 ## 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/YYYYYYY HTTP/1.1
+GET /api/snapshots-delete/XXXXXXX HTTP/1.1
 Accept: application/json
-Content-Type: application/json
-Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
 ```
 
 **Example Response**:
@@ -159,5 +218,5 @@ 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."}
 ```
\ No newline at end of file

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)

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)

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,

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())

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"`

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);
       }
     );