Merge pull request #11554 from grafana/11553_viewer_general_access

User with org viewer role permission fixes

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
 }
 
 //

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 {

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
 				})
 

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
 			})
 

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

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,

public/app/features/dashboard/folder_picker/folder_picker.ts

@@ -19,9 +19,12 @@ export class FolderPickerCtrl {
   newFolderNameTouched: boolean;
   hasValidationError: boolean;
   validationError: any;
+  isEditor: boolean;
 
   /** @ngInject */
-  constructor(private backendSrv, private validationSrv) {
+  constructor(private backendSrv, private validationSrv, private contextSrv) {
+    this.isEditor = this.contextSrv.isEditor;
+
     if (!this.labelClass) {
       this.labelClass = 'width-7';
     }
@@ -38,19 +41,20 @@ export class FolderPickerCtrl {
 
     return this.backendSrv.get('api/search', params).then(result => {
       if (
-        query === '' ||
-        query.toLowerCase() === 'g' ||
-        query.toLowerCase() === 'ge' ||
-        query.toLowerCase() === 'gen' ||
-        query.toLowerCase() === 'gene' ||
-        query.toLowerCase() === 'gener' ||
-        query.toLowerCase() === 'genera' ||
-        query.toLowerCase() === 'general'
+        this.isEditor &&
+        (query === '' ||
+          query.toLowerCase() === 'g' ||
+          query.toLowerCase() === 'ge' ||
+          query.toLowerCase() === 'gen' ||
+          query.toLowerCase() === 'gene' ||
+          query.toLowerCase() === 'gener' ||
+          query.toLowerCase() === 'genera' ||
+          query.toLowerCase() === 'general')
       ) {
         result.unshift({ title: this.rootName, id: 0 });
       }
 
-      if (this.enableCreateNew && query === '') {
+      if (this.isEditor && this.enableCreateNew && query === '') {
         result.unshift({ title: '-- New Folder --', id: -1 });
       }