IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-15 10:03:33 -06:00
Code Issues Packages Projects Releases Wiki Activity

Do not check permissions while deleting external snapshot (#51897)

Browse Source
This commit is contained in:
idafurjes 2022-07-07 15:15:39 +02:00 committed by GitHub
parent 9ab210a7d7
commit ee88b44458
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
pkg/api/dashboard_snapshot.go
View File

@ -268,24 +268,28 @@ func (hs *HTTPServer) DeleteDashboardSnapshot(c *models.ReqContext) response.Res
return response.Error(404, "Failed to get dashboard snapshot", nil)
}
dashboardID := query.Result.Dashboard.Get("id").MustInt64()
guardian := guardian.New(c.Req.Context(), dashboardID, c.OrgId, c.SignedInUser)
canEdit, err := guardian.CanEdit()
// check for permissions only if the dahboard is found
if err != nil && !errors.Is(err, dashboards.ErrDashboardNotFound) {
return response.Error(500, "Error while checking permissions for snapshot", err)
}
if !canEdit && query.Result.UserId != c.SignedInUser.UserId && !errors.Is(err, dashboards.ErrDashboardNotFound) {
return response.Error(403, "Access denied to this snapshot", nil)
}
if query.Result.External {
err := deleteExternalDashboardSnapshot(query.Result.ExternalDeleteUrl)
if err != nil {
return response.Error(500, "Failed to delete external dashboard", err)
}
} else {
// When creating an external snapshot, its dashboard content is empty. This means that the mustInt here returns a 0,
// which before RBAC would result in a dashboard which has no ACL. A dashboard without an ACL would fallback
// to the user’s org role, which for editors and admins would essentially always be allowed here. With RBAC,
// all permissions must be explicit, so the lack of a rule for dashboard 0 means the guardian will reject.
dashboardID := query.Result.Dashboard.Get("id").MustInt64()
guardian := guardian.New(c.Req.Context(), dashboardID, c.OrgId, c.SignedInUser)
canEdit, err := guardian.CanEdit()
// check for permissions only if the dahboard is found
if err != nil && !errors.Is(err, dashboards.ErrDashboardNotFound) {
return response.Error(500, "Error while checking permissions for snapshot", err)
}
if !canEdit && query.Result.UserId != c.SignedInUser.UserId && !errors.Is(err, dashboards.ErrDashboardNotFound) {
return response.Error(403, "Access denied to this snapshot", nil)
}
}
cmd := &dashboardsnapshots.DeleteDashboardSnapshotCommand{DeleteKey: query.Result.DeleteKey}

7
pkg/api/dashboard_snapshot_test.go
View File

@ -65,11 +65,7 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
t.Run("When user has editor role and is not in the ACL", func(t *testing.T) {
loggedInUserScenarioWithRole(t, "Should not be able to delete snapshot when calling DELETE on",
"DELETE", "/api/snapshots/12345", "/api/snapshots/:key", models.ROLE_EDITOR, func(sc *scenarioContext) {
var externalRequest *http.Request
ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
externalRequest = req
})
hs := &HTTPServer{dashboardsnapshotsService: setUpSnapshotTest(t, 0, ts.URL)}
hs := &HTTPServer{dashboardsnapshotsService: setUpSnapshotTest(t, 0, "")}
sc.handlerFunc = hs.DeleteDashboardSnapshot
dashSvc := dashboards.NewFakeDashboardService(t)
@ -79,7 +75,6 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, 403, sc.resp.Code)
require.Nil(t, externalRequest)
}, sqlmock)
})