diff --git a/pkg/api/dashboard_snapshot.go b/pkg/api/dashboard_snapshot.go index a367c69dd5c..3dbe4e08657 100644 --- a/pkg/api/dashboard_snapshot.go +++ b/pkg/api/dashboard_snapshot.go @@ -366,17 +366,19 @@ func (hs *HTTPServer) DeleteDashboardSnapshot(c *contextmodel.ReqContext) respon if dashboardID != 0 { g, err := guardian.New(c.Req.Context(), dashboardID, c.OrgID, c.SignedInUser) if err != nil { - return response.Err(err) - } + if !errors.Is(err, dashboards.ErrDashboardNotFound) { + return response.Err(err) + } + } else { + canEdit, err := g.CanEdit() + // check for permissions only if the dashboard is found + if err != nil && !errors.Is(err, dashboards.ErrDashboardNotFound) { + return response.Error(http.StatusInternalServerError, "Error while checking permissions for snapshot", err) + } - canEdit, err := g.CanEdit() - // check for permissions only if the dashboard is found - if err != nil && !errors.Is(err, dashboards.ErrDashboardNotFound) { - return response.Error(http.StatusInternalServerError, "Error while checking permissions for snapshot", err) - } - - if !canEdit && queryResult.UserID != c.SignedInUser.UserID && !errors.Is(err, dashboards.ErrDashboardNotFound) { - return response.Error(http.StatusForbidden, "Access denied to this snapshot", nil) + if !canEdit && queryResult.UserID != c.SignedInUser.UserID && !errors.Is(err, dashboards.ErrDashboardNotFound) { + return response.Error(http.StatusForbidden, "Access denied to this snapshot", nil) + } } } diff --git a/pkg/api/dashboard_snapshot_test.go b/pkg/api/dashboard_snapshot_test.go index 9512506ef7c..5cede0b7c58 100644 --- a/pkg/api/dashboard_snapshot_test.go +++ b/pkg/api/dashboard_snapshot_test.go @@ -21,6 +21,7 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/team/teamtest" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util/errutil" ) func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) { @@ -124,6 +125,51 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) { } dashSvc.On("GetDashboardACLInfoList", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardACLInfoListQuery")).Return(qResult, nil) + loggedInUserScenarioWithRole(t, "Should not be able to delete a snapshot when fetching guardian fails during calling DELETE on", "DELETE", + "/api/snapshots/12345", "/api/snapshots/:key", org.RoleEditor, func(sc *scenarioContext) { + ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(200) + }) + dashSvc := dashboards.NewFakeDashboardService(t) + dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(nil, errutil.Error{PublicMessage: "some error"}).Maybe() + + guardian.InitLegacyGuardian(sc.sqlStore, dashSvc, teamSvc) + d := setUpSnapshotTest(t, 0, ts.URL) + hs := buildHttpServer(d, true) + hs.DashboardService = dashSvc + sc.handlerFunc = hs.DeleteDashboardSnapshot + sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec() + + assert.Equal(t, http.StatusInternalServerError, sc.resp.Code) + }, sqlmock) + + loggedInUserScenarioWithRole(t, "Should be able to delete a snapshot from a deleted dashboard when calling DELETE on", "DELETE", + "/api/snapshots/12345", "/api/snapshots/:key", org.RoleEditor, func(sc *scenarioContext) { + var externalRequest *http.Request + ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(200) + externalRequest = req + }) + dashSvc := dashboards.NewFakeDashboardService(t) + dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(nil, dashboards.ErrDashboardNotFound).Maybe() + + guardian.InitLegacyGuardian(sc.sqlStore, dashSvc, teamSvc) + d := setUpSnapshotTest(t, 0, ts.URL) + hs := buildHttpServer(d, true) + hs.DashboardService = dashSvc + sc.handlerFunc = hs.DeleteDashboardSnapshot + sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec() + + assert.Equal(t, 200, sc.resp.Code) + respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + require.NoError(t, err) + + assert.True(t, strings.HasPrefix(respJSON.Get("message").MustString(), "Snapshot deleted")) + assert.Equal(t, 1, respJSON.Get("id").MustInt()) + assert.Equal(t, ts.URL, fmt.Sprintf("http://%s", externalRequest.Host)) + assert.Equal(t, "/", externalRequest.URL.EscapedPath()) + }, sqlmock) + loggedInUserScenarioWithRole(t, "Should be able to delete a snapshot when calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", org.RoleEditor, func(sc *scenarioContext) { var externalRequest *http.Request