Snapshots: Fix deleting snapshot with non existent dashboard ID (#64345)

* Add test for deleting snapshot for non existent dashboard

* Add test for failure to fetch guardian other than ErrDashboardNotFound

* Fix dashboard snapshot delete
This commit is contained in:
Sofia Papagiannaki
2023-03-08 10:12:02 +02:00
committed by GitHub
parent 05191d083d
commit 43095d84e4
2 changed files with 58 additions and 10 deletions

View File

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

View File

@@ -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