Guardian: Introduce additional constructors (#59577)
* Guardian: Use dashboard UID instead of ID * Apply suggestions from code review Introduce several guardian constructors and each time use the most appropriate one.
This commit is contained in:
@@ -17,6 +17,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"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/util"
|
||||
)
|
||||
@@ -120,7 +121,10 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d
|
||||
|
||||
if isParentFolderChanged {
|
||||
// Check that the user is allowed to add a dashboard to the folder
|
||||
guardian := guardian.New(ctx, dash.Id, dto.OrgId, dto.User)
|
||||
guardian, err := guardian.NewByDashboard(ctx, dash, dto.OrgId, dto.User)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if canSave, err := guardian.CanCreate(dash.FolderId, dash.IsFolder); err != nil || !canSave {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -140,7 +144,11 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d
|
||||
}
|
||||
}
|
||||
|
||||
guard := guardian.New(ctx, dash.GetDashboardIdForSavePermissionCheck(), dto.OrgId, dto.User)
|
||||
guard, err := getGuardianForSavePermissionCheck(ctx, dash, dto.User)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if dash.Id == 0 {
|
||||
if canCreate, err := guard.CanCreate(dash.FolderId, dash.IsFolder); err != nil || !canCreate {
|
||||
if err != nil {
|
||||
@@ -183,6 +191,26 @@ func (dr *DashboardServiceImpl) DeleteOrphanedProvisionedDashboards(ctx context.
|
||||
return dr.dashboardStore.DeleteOrphanedProvisionedDashboards(ctx, cmd)
|
||||
}
|
||||
|
||||
// getGuardianForSavePermissionCheck returns the guardian to be used for checking permission of dashboard
|
||||
// It replaces deleted Dashboard.GetDashboardIdForSavePermissionCheck()
|
||||
func getGuardianForSavePermissionCheck(ctx context.Context, d *models.Dashboard, user *user.SignedInUser) (guardian.DashboardGuardian, error) {
|
||||
newDashboard := d.Id == 0
|
||||
|
||||
if newDashboard {
|
||||
// if it's a new dashboard/folder check the parent folder permissions
|
||||
guard, err := guardian.New(ctx, d.FolderId, d.OrgId, user)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return guard, nil
|
||||
}
|
||||
guard, err := guardian.NewByDashboard(ctx, d, d.OrgId, user)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return guard, nil
|
||||
}
|
||||
|
||||
func validateDashboardRefreshInterval(dash *models.Dashboard) error {
|
||||
if setting.MinRefreshInterval == "" {
|
||||
return nil
|
||||
|
||||
@@ -108,7 +108,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sqlStore)
|
||||
assert.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, int64(0), sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, "", sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -128,7 +128,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
require.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.otherSavedFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.otherSavedFolder.Id, sc.dashboardGuardianMock.DashID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -148,7 +148,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
require.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.savedDashInFolder.Uid, sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -169,7 +169,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
require.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.savedDashInFolder.Uid, sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -190,7 +190,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
assert.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.savedDashInGeneralFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.savedDashInGeneralFolder.Uid, sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -211,7 +211,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
require.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.savedDashInFolder.Uid, sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -232,7 +232,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
require.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.savedDashInGeneralFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.savedDashInGeneralFolder.Uid, sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -253,7 +253,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
assert.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.savedDashInFolder.Uid, sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -274,7 +274,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
require.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.savedDashInGeneralFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.savedDashInGeneralFolder.Uid, sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
@@ -295,7 +295,7 @@ func TestIntegrationIntegratedDashboardService(t *testing.T) {
|
||||
err := callSaveWithError(t, cmd, sc.sqlStore)
|
||||
require.Equal(t, dashboards.ErrDashboardUpdateAccessDenied, err)
|
||||
|
||||
assert.Equal(t, sc.savedDashInFolder.Id, sc.dashboardGuardianMock.DashId)
|
||||
assert.Equal(t, sc.savedDashInFolder.Uid, sc.dashboardGuardianMock.DashUID)
|
||||
assert.Equal(t, cmd.OrgId, sc.dashboardGuardianMock.OrgId)
|
||||
assert.Equal(t, cmd.UserId, sc.dashboardGuardianMock.User.UserID)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user