From b4863002c98ce57d060df1a01bc1edeec92e99d0 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 11 Apr 2018 00:23:50 +0200 Subject: [PATCH] permission: fix user with org viewer save/move permissions User with org viewer role should not be able to save/move dashboards in/to general folder. --- pkg/models/dashboards.go | 5 + pkg/services/dashboards/dashboard_service.go | 10 + .../dashboards/dashboard_service_test.go | 1 + .../dashboards/folder_service_test.go | 2 + pkg/services/sqlstore/dashboard.go | 13 ++ .../dashboard_service_integration_test.go | 194 +++++++++++++++--- 6 files changed, 191 insertions(+), 34 deletions(-) diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 8cd2b01811c..33d1dd64db6 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -224,6 +224,10 @@ func GetFolderUrl(folderUid string, slug string) string { return fmt.Sprintf("%s/dashboards/f/%s/%s", setting.AppSubUrl, folderUid, slug) } +type ValidateDashboardBeforeSaveResult struct { + IsParentFolderChanged bool +} + // // COMMANDS // @@ -268,6 +272,7 @@ type ValidateDashboardBeforeSaveCommand struct { OrgId int64 Dashboard *Dashboard Overwrite bool + Result *ValidateDashboardBeforeSaveResult } // diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index 02a6ffc8330..1656bb74c9c 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -103,6 +103,16 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, return nil, err } + if validateBeforeSaveCmd.Result.IsParentFolderChanged { + folderGuardian := guardian.New(dash.FolderId, dto.OrgId, dto.User) + if canSave, err := folderGuardian.CanSave(); err != nil || !canSave { + if err != nil { + return nil, err + } + return nil, models.ErrDashboardUpdateAccessDenied + } + } + guard := guardian.New(dash.GetDashboardIdForSavePermissionCheck(), dto.OrgId, dto.User) if canSave, err := guard.CanSave(); err != nil || !canSave { if err != nil { diff --git a/pkg/services/dashboards/dashboard_service_test.go b/pkg/services/dashboards/dashboard_service_test.go index 965b10655b3..d2c5863d994 100644 --- a/pkg/services/dashboards/dashboard_service_test.go +++ b/pkg/services/dashboards/dashboard_service_test.go @@ -51,6 +51,7 @@ func TestDashboardService(t *testing.T) { }) bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + cmd.Result = &models.ValidateDashboardBeforeSaveResult{} return nil }) diff --git a/pkg/services/dashboards/folder_service_test.go b/pkg/services/dashboards/folder_service_test.go index 6c0413d1878..1e678e3b1a1 100644 --- a/pkg/services/dashboards/folder_service_test.go +++ b/pkg/services/dashboards/folder_service_test.go @@ -32,6 +32,7 @@ func TestFolderService(t *testing.T) { }) bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + cmd.Result = &models.ValidateDashboardBeforeSaveResult{} return models.ErrDashboardUpdateAccessDenied }) @@ -92,6 +93,7 @@ func TestFolderService(t *testing.T) { }) bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + cmd.Result = &models.ValidateDashboardBeforeSaveResult{} return nil }) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 8a89c3d942c..beda4b150ca 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -544,6 +544,10 @@ func getExistingDashboardByIdOrUidForUpdate(sess *DBSession, cmd *m.ValidateDash dash.SetId(existingByUid.Id) dash.SetUid(existingByUid.Uid) existing = existingByUid + + if !dash.IsFolder { + cmd.Result.IsParentFolderChanged = true + } } if (existing.IsFolder && !dash.IsFolder) || @@ -551,6 +555,10 @@ func getExistingDashboardByIdOrUidForUpdate(sess *DBSession, cmd *m.ValidateDash return m.ErrDashboardTypeMismatch } + if !dash.IsFolder && dash.FolderId != existing.FolderId { + cmd.Result.IsParentFolderChanged = true + } + // check for is someone else has written in between if dash.Version != existing.Version { if cmd.Overwrite { @@ -586,6 +594,10 @@ func getExistingDashboardByTitleAndFolder(sess *DBSession, cmd *m.ValidateDashbo return m.ErrDashboardFolderWithSameNameAsDashboard } + if !dash.IsFolder && (dash.FolderId != existing.FolderId || dash.Id == 0) { + cmd.Result.IsParentFolderChanged = true + } + if cmd.Overwrite { dash.SetId(existing.Id) dash.SetUid(existing.Uid) @@ -599,6 +611,7 @@ func getExistingDashboardByTitleAndFolder(sess *DBSession, cmd *m.ValidateDashbo } func ValidateDashboardBeforeSave(cmd *m.ValidateDashboardBeforeSaveCommand) (err error) { + cmd.Result = &m.ValidateDashboardBeforeSaveResult{} return inTransaction(func(sess *DBSession) error { if err = getExistingDashboardByIdOrUidForUpdate(sess, cmd); err != nil { return err diff --git a/pkg/services/sqlstore/dashboard_service_integration_test.go b/pkg/services/sqlstore/dashboard_service_integration_test.go index d005270c33c..21ebc6505bb 100644 --- a/pkg/services/sqlstore/dashboard_service_integration_test.go +++ b/pkg/services/sqlstore/dashboard_service_integration_test.go @@ -74,7 +74,7 @@ func TestIntegratedDashboardService(t *testing.T) { Convey("Given organization B", func() { var otherOrgId int64 = 2 - Convey("When saving a dashboard with id that are saved in organization A", func() { + Convey("When creating a dashboard with same id as dashboard in organization A", func() { cmd := models.SaveDashboardCommand{ OrgId: otherOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -93,7 +93,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) permissionScenario("Given user has permission to save", true, func(sc *dashboardPermissionScenarioContext) { - Convey("When saving a dashboard with uid that are saved in organization A", func() { + Convey("When creating a dashboard with same uid as dashboard in organization A", func() { var otherOrgId int64 = 2 cmd := models.SaveDashboardCommand{ OrgId: otherOrgId, @@ -106,7 +106,7 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(cmd) - Convey("It should create dashboard in other organization", func() { + Convey("It should create a new dashboard in organization B", func() { So(res, ShouldNotBeNil) query := models.GetDashboardQuery{OrgId: otherOrgId, Uid: savedDashInFolder.Uid} @@ -126,7 +126,7 @@ func TestIntegratedDashboardService(t *testing.T) { permissionScenario("Given user has no permission to save", false, func(sc *dashboardPermissionScenarioContext) { - Convey("When trying to create a new dashboard in the General folder", func() { + Convey("When creating a new dashboard in the General folder", func() { cmd := models.SaveDashboardCommand{ OrgId: testOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -138,7 +138,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd) - Convey("It should call dashboard guardian with correct arguments and result in access denied error", func() { + Convey("It should create dashboard guardian for General Folder with correct arguments and result in access denied error", func() { So(err, ShouldNotBeNil) So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) @@ -148,7 +148,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to create a new dashboard in other folder", func() { + Convey("When creating a new dashboard in other folder", func() { cmd := models.SaveDashboardCommand{ OrgId: testOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -161,7 +161,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd) - Convey("It should call dashboard guardian with correct arguments and rsult in access denied error", func() { + Convey("It should create dashboard guardian for other folder with correct arguments and rsult in access denied error", func() { So(err, ShouldNotBeNil) So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) @@ -171,7 +171,54 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update a dashboard by existing id in the General folder", func() { + Convey("When creating a new dashboard by existing title in folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "title": savedDashInFolder.Title, + }), + FolderId: savedFolder.Id, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, savedFolder.Id) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When creating a new dashboard by existing uid in folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "uid": savedDashInFolder.Uid, + "title": "New dash", + }), + FolderId: savedFolder.Id, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, savedFolder.Id) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When updating a dashboard by existing id in the General folder", func() { cmd := models.SaveDashboardCommand{ OrgId: testOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -185,7 +232,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd) - Convey("It should call dashboard guardian with correct arguments and result in access denied error", func() { + Convey("It should create dashboard guardian for dashboard with correct arguments and result in access denied error", func() { So(err, ShouldNotBeNil) So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) @@ -195,7 +242,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update a dashboard by existing id in other folder", func() { + Convey("When updating a dashboard by existing id in other folder", func() { cmd := models.SaveDashboardCommand{ OrgId: testOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -209,7 +256,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd) - Convey("It should call dashboard guardian with correct arguments and result in access denied error", func() { + Convey("It should create dashboard guardian for dashboard with correct arguments and result in access denied error", func() { So(err, ShouldNotBeNil) So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) @@ -218,6 +265,102 @@ func TestIntegratedDashboardService(t *testing.T) { So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) }) }) + + Convey("When moving a dashboard by existing id to other folder from General folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": savedDashInGeneralFolder.Id, + "title": "Dash", + }), + FolderId: otherSavedFolder.Id, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for other folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, otherSavedFolder.Id) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When moving a dashboard by existing id to the General folder from other folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": savedDashInFolder.Id, + "title": "Dash", + }), + FolderId: 0, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for General folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, 0) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When moving a dashboard by existing uid to other folder from General folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "uid": savedDashInGeneralFolder.Uid, + "title": "Dash", + }), + FolderId: otherSavedFolder.Id, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for other folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, otherSavedFolder.Id) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When moving a dashboard by existing uid to the General folder from other folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "uid": savedDashInFolder.Uid, + "title": "Dash", + }), + FolderId: 0, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for General folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, 0) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) }) // Given user has permission to save @@ -668,7 +811,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing folder to a dashboard using id", func() { + Convey("When updating existing folder to a dashboard using id", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -687,7 +830,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing dashboard to a folder using id", func() { + Convey("When updating existing dashboard to a folder using id", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -706,7 +849,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing folder to a dashboard using uid", func() { + Convey("When updating existing folder to a dashboard using uid", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -725,7 +868,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing dashboard to a folder using uid", func() { + Convey("When updating existing dashboard to a folder using uid", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -744,7 +887,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing folder to a dashboard using title", func() { + Convey("When updating existing folder to a dashboard using title", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -762,7 +905,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing dashboard to a folder using title", func() { + Convey("When updating existing dashboard to a folder using title", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -850,23 +993,6 @@ func callSaveWithError(cmd models.SaveDashboardCommand) error { return err } -func dashboardServiceScenario(desc string, mock *guardian.FakeDashboardGuardian, fn scenarioFunc) { - Convey(desc, func() { - origNewDashboardGuardian := guardian.New - guardian.MockDashboardGuardian(mock) - - sc := &scenarioContext{ - dashboardGuardianMock: mock, - } - - defer func() { - guardian.New = origNewDashboardGuardian - }() - - fn(sc) - }) -} - func saveTestDashboard(title string, orgId int64, folderId int64) *models.Dashboard { cmd := models.SaveDashboardCommand{ OrgId: orgId,