Skip to content

Commit

Permalink
[v10.2.x] Snapshots: Require delete within same org (backport) (#84730)
Browse files Browse the repository at this point in the history
Snapshots: Require delete within same org (backport) (#84707)

* check orgId on delete

* test from main

(cherry picked from commit d80f83b)

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
  • Loading branch information
grafana-delivery-bot[bot] and ryantxu committed Mar 20, 2024
1 parent 47275a6 commit 0dd4492
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 11 deletions.
3 changes: 3 additions & 0 deletions pkg/api/dashboard_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,9 @@ func (hs *HTTPServer) DeleteDashboardSnapshot(c *contextmodel.ReqContext) respon
if queryResult == nil {
return response.Error(http.StatusNotFound, "Failed to get dashboard snapshot", nil)
}
if queryResult.OrgID != c.OrgID {
return response.Error(http.StatusUnauthorized, "OrgID mismatch", nil)
}

if queryResult.External {
err := deleteExternalDashboardSnapshot(queryResult.ExternalDeleteURL)
Expand Down
22 changes: 11 additions & 11 deletions pkg/api/dashboard_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ import (
"testing"
"time"

"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/web/webtest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/db/dbtest"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboardsnapshots"
"github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web/webtest"
)

func TestHTTPServer_DeleteDashboardSnapshot(t *testing.T) {
Expand Down Expand Up @@ -148,12 +148,11 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
sc.handlerFunc = hs.DeleteDashboardSnapshotByDeleteKey
sc.fakeReqWithParams("GET", sc.url, map[string]string{"deleteKey": "12345"}).exec()

require.Equal(t, 200, sc.resp.Code)
require.Equal(t, 200, sc.resp.Code, "BODY: "+sc.resp.Body.String())
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, http.MethodGet, externalRequest.Method)
assert.Equal(t, ts.URL, fmt.Sprintf("http://%s", externalRequest.Host))
Expand Down Expand Up @@ -271,7 +270,7 @@ func TestGetDashboardSnapshotNotFound(t *testing.T) {
sc.handlerFunc = hs.DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()

assert.Equal(t, http.StatusNotFound, sc.resp.Code)
assert.Equal(t, http.StatusNotFound, sc.resp.Code, "BODY: "+sc.resp.Body.String())
}, sqlmock)

loggedInUserScenarioWithRole(t,
Expand All @@ -282,7 +281,7 @@ func TestGetDashboardSnapshotNotFound(t *testing.T) {
sc.handlerFunc = hs.DeleteDashboardSnapshotByDeleteKey
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"deleteKey": "12345"}).exec()

assert.Equal(t, http.StatusNotFound, sc.resp.Code)
assert.Equal(t, http.StatusNotFound, sc.resp.Code, "BODY: "+sc.resp.Body.String())
}, sqlmock)
}

Expand Down Expand Up @@ -345,7 +344,7 @@ func TestGetDashboardSnapshotFailure(t *testing.T) {
sc.handlerFunc = hs.DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()

assert.Equal(t, http.StatusForbidden, sc.resp.Code)
assert.Equal(t, http.StatusForbidden, sc.resp.Code, "BODY: "+sc.resp.Body.String())
}, sqlmock)

loggedInUserScenarioWithRole(t,
Expand All @@ -356,7 +355,7 @@ func TestGetDashboardSnapshotFailure(t *testing.T) {
sc.handlerFunc = hs.DeleteDashboardSnapshotByDeleteKey
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"deleteKey": "12345"}).exec()

assert.Equal(t, http.StatusInternalServerError, sc.resp.Code)
assert.Equal(t, http.StatusInternalServerError, sc.resp.Code, "BODY: "+sc.resp.Body.String())
}, sqlmock)

loggedInUserScenarioWithRole(t,
Expand All @@ -367,7 +366,7 @@ func TestGetDashboardSnapshotFailure(t *testing.T) {
sc.handlerFunc = hs.DeleteDashboardSnapshotByDeleteKey
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"deleteKey": "12345"}).exec()

assert.Equal(t, http.StatusForbidden, sc.resp.Code)
assert.Equal(t, http.StatusForbidden, sc.resp.Code, "BODY: "+sc.resp.Body.String())
}, sqlmock)
}

Expand All @@ -391,6 +390,7 @@ func setUpSnapshotTest(t *testing.T, userId int64, deleteUrl string) dashboardsn

res := &dashboardsnapshots.DashboardSnapshot{
ID: 1,
OrgID: 1,
Key: "12345",
DeleteKey: "54321",
Dashboard: jsonModel,
Expand Down

0 comments on commit 0dd4492

Please sign in to comment.