From 939156bc2f186067bdc6f8733920739e8a9f6d6d Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Tue, 10 May 2022 08:10:59 -0400 Subject: [PATCH] Dashboard: Fix dashboard update permission check (#48746) (#48853) * Change dash permission check for dashboards that are moved to a different folder (cherry picked from commit 6923b4c6c6787e53d05694d3788864f6e3bd5afb) Co-authored-by: Ieva --- pkg/services/dashboards/database/database.go | 4 ---- .../dashboards/manager/dashboard_service.go | 5 ++-- .../dashboard_service_integration_test.go | 24 +++++++++---------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index c3d5285f30b..2935e536a83 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -326,10 +326,6 @@ func getExistingDashboardByIdOrUidForUpdate(sess *sqlstore.DBSession, dash *mode dash.SetId(existingByUid.Id) dash.SetUid(existingByUid.Uid) existing = existingByUid - - if !dash.IsFolder { - isParentFolderChanged = true - } } if (existing.IsFolder && !dash.IsFolder) || diff --git a/pkg/services/dashboards/manager/dashboard_service.go b/pkg/services/dashboards/manager/dashboard_service.go index 9c288620e16..9d84340e941 100644 --- a/pkg/services/dashboards/manager/dashboard_service.go +++ b/pkg/services/dashboards/manager/dashboard_service.go @@ -110,8 +110,9 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d } if isParentFolderChanged { - folderGuardian := guardian.New(ctx, dash.FolderId, dto.OrgId, dto.User) - if canSave, err := folderGuardian.CanSave(); err != nil || !canSave { + // Check that the user is allowed to add a dashboard to the folder + guardian := guardian.New(ctx, dash.Id, dto.OrgId, dto.User) + if canSave, err := guardian.CanCreate(dash.FolderId, dash.IsFolder); err != nil || !canSave { if err != nil { return nil, err } diff --git a/pkg/services/dashboards/manager/dashboard_service_integration_test.go b/pkg/services/dashboards/manager/dashboard_service_integration_test.go index 99a9d6cb97a..dfd700b8bad 100644 --- a/pkg/services/dashboards/manager/dashboard_service_integration_test.go +++ b/pkg/services/dashboards/manager/dashboard_service_integration_test.go @@ -127,7 +127,7 @@ func TestIntegratedDashboardService(t *testing.T) { assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserId) }) - permissionScenario(t, "When creating a new dashboard by existing title in folder, it should create dashboard guardian for folder with correct arguments and result in access denied error", + permissionScenario(t, "When creating a new dashboard by existing title in folder, it should create dashboard guardian for dashboard with correct arguments and result in access denied error", canSave, func(t *testing.T, sc *permissionScenarioContext) { cmd := models.SaveDashboardCommand{ OrgId: testOrgID, @@ -142,12 +142,12 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd, sc.sqlStore) require.Equal(t, models.ErrDashboardUpdateAccessDenied, err) - assert.Equal(t, sc.savedFolder.Id, sc.dashboardGuardianMock.DashId) + assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId) assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId) assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserId) }) - permissionScenario(t, "When creating a new dashboard by existing UID in folder, it should create dashboard guardian for folder with correct arguments and result in access denied error", + permissionScenario(t, "When creating a new dashboard by existing UID in folder, it should create dashboard guardian for dashboard with correct arguments and result in access denied error", canSave, func(t *testing.T, sc *permissionScenarioContext) { cmd := models.SaveDashboardCommand{ OrgId: testOrgID, @@ -163,7 +163,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd, sc.sqlStore) require.Equal(t, models.ErrDashboardUpdateAccessDenied, err) - assert.Equal(t, sc.savedFolder.Id, sc.dashboardGuardianMock.DashId) + assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId) assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId) assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserId) }) @@ -210,7 +210,7 @@ func TestIntegratedDashboardService(t *testing.T) { assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserId) }) - permissionScenario(t, "When moving a dashboard by existing ID to other folder from General folder, it should create dashboard guardian for other folder with correct arguments and result in access denied error", + permissionScenario(t, "When moving a dashboard by existing ID to other folder from General folder, it should create dashboard guardian for dashboard with correct arguments and result in access denied error", canSave, func(t *testing.T, sc *permissionScenarioContext) { cmd := models.SaveDashboardCommand{ OrgId: testOrgID, @@ -226,12 +226,12 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd, sc.sqlStore) require.Equal(t, models.ErrDashboardUpdateAccessDenied, err) - assert.Equal(t, sc.otherSavedFolder.Id, sc.dashboardGuardianMock.DashId) + assert.Equal(t, sc.savedDashInGeneralFolder.Id, sc.dashboardGuardianMock.DashId) assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId) assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserId) }) - permissionScenario(t, "When moving a dashboard by existing id to the General folder from other folder, it should create dashboard guardian for General folder with correct arguments and result in access denied error", + permissionScenario(t, "When moving a dashboard by existing id to the General folder from other folder, it should create dashboard guardian for dashboard with correct arguments and result in access denied error", canSave, func(t *testing.T, sc *permissionScenarioContext) { cmd := models.SaveDashboardCommand{ OrgId: testOrgID, @@ -247,12 +247,12 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd, sc.sqlStore) assert.Equal(t, models.ErrDashboardUpdateAccessDenied, err) - assert.Equal(t, int64(0), sc.dashboardGuardianMock.DashId) + assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId) assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId) assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserId) }) - permissionScenario(t, "When moving a dashboard by existing uid to other folder from General folder, it should create dashboard guardian for other folder with correct arguments and result in access denied error", + permissionScenario(t, "When moving a dashboard by existing uid to other folder from General folder, it should create dashboard guardian for dashboard with correct arguments and result in access denied error", canSave, func(t *testing.T, sc *permissionScenarioContext) { cmd := models.SaveDashboardCommand{ OrgId: testOrgID, @@ -268,12 +268,12 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd, sc.sqlStore) require.Equal(t, models.ErrDashboardUpdateAccessDenied, err) - assert.Equal(t, sc.otherSavedFolder.Id, sc.dashboardGuardianMock.DashId) + assert.Equal(t, sc.savedDashInGeneralFolder.Id, sc.dashboardGuardianMock.DashId) assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId) assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserId) }) - permissionScenario(t, "When moving a dashboard by existing UID to the General folder from other folder, it should create dashboard guardian for General folder with correct arguments and result in access denied error", + permissionScenario(t, "When moving a dashboard by existing UID to the General folder from other folder, it should create dashboard guardian for dashboard with correct arguments and result in access denied error", canSave, func(t *testing.T, sc *permissionScenarioContext) { cmd := models.SaveDashboardCommand{ OrgId: testOrgID, @@ -289,7 +289,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd, sc.sqlStore) require.Equal(t, models.ErrDashboardUpdateAccessDenied, err) - assert.Equal(t, int64(0), sc.dashboardGuardianMock.DashId) + assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId) assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId) assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserId) })