diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index 43c582f6c73..8d3a2455f1f 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -167,8 +167,10 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { '' as user_login, '' as user_email, '' as user_group - FROM dashboard_acl as da, dashboard as dash - WHERE dash.id = ? AND dash.has_acl = 0 AND da.dashboard_id = -1 + FROM dashboard_acl as da, + dashboard as dash + LEFT JOIN dashboard folder on dash.folder_id = folder.id + WHERE dash.id = ? AND (dash.has_acl = 0 or folder.has_acl = 0) AND da.dashboard_id = -1 ` query.Result = make([]*m.DashboardAclInfoDTO, 0) diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index 252d85368a3..55f4b24808b 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/pkg/services/sqlstore/dashboard_acl_test.go @@ -25,6 +25,22 @@ func TestDashboardAclDataAccess(t *testing.T) { So(err, ShouldEqual, m.ErrDashboardAclInfoMissing) }) + Convey("Given dashboard folder with default permissions", func() { + Convey("When reading dashboard acl should include acl for parent folder", func() { + query := m.GetDashboardAclInfoListQuery{DashboardId: childDash.Id, OrgId: 1} + + err := GetDashboardAclInfoList(&query) + So(err, ShouldBeNil) + + So(len(query.Result), ShouldEqual, 2) + defaultPermissionsId := -1 + So(query.Result[0].DashboardId, ShouldEqual, defaultPermissionsId) + So(*query.Result[0].Role, ShouldEqual, m.ROLE_VIEWER) + So(query.Result[1].DashboardId, ShouldEqual, defaultPermissionsId) + So(*query.Result[1].Role, ShouldEqual, m.ROLE_EDITOR) + }) + }) + Convey("Given dashboard folder permission", func() { err := SetDashboardAcl(&m.SetDashboardAclCommand{ OrgId: 1, @@ -66,6 +82,31 @@ func TestDashboardAclDataAccess(t *testing.T) { }) }) + Convey("Given child dashboard permission in folder with no permissions", func() { + err := SetDashboardAcl(&m.SetDashboardAclCommand{ + OrgId: 1, + UserId: currentUser.Id, + DashboardId: childDash.Id, + Permission: m.PERMISSION_EDIT, + }) + So(err, ShouldBeNil) + + Convey("When reading dashboard acl should include default acl for parent folder and the child acl", func() { + query := m.GetDashboardAclInfoListQuery{OrgId: 1, DashboardId: childDash.Id} + + err := GetDashboardAclInfoList(&query) + So(err, ShouldBeNil) + + defaultPermissionsId := -1 + So(len(query.Result), ShouldEqual, 3) + So(query.Result[0].DashboardId, ShouldEqual, defaultPermissionsId) + So(*query.Result[0].Role, ShouldEqual, m.ROLE_VIEWER) + So(query.Result[1].DashboardId, ShouldEqual, defaultPermissionsId) + So(*query.Result[1].Role, ShouldEqual, m.ROLE_EDITOR) + So(query.Result[2].DashboardId, ShouldEqual, childDash.Id) + }) + }) + Convey("Should be able to add dashboard permission", func() { setDashAclCmd := m.SetDashboardAclCommand{ OrgId: 1, diff --git a/public/app/features/dashboard/acl/acl.ts b/public/app/features/dashboard/acl/acl.ts index 492320fce80..d7d79efb9a0 100644 --- a/public/app/features/dashboard/acl/acl.ts +++ b/public/app/features/dashboard/acl/acl.ts @@ -50,7 +50,7 @@ export class AclCtrl { } prepareViewModel(item: DashboardAcl): DashboardAcl { - item.inherited = this.dashboard.id !== item.dashboardId; + item.inherited = !this.dashboard.meta.isFolder && this.dashboard.id !== item.dashboardId; item.sortRank = 0; if (item.userId > 0) { @@ -138,6 +138,10 @@ export class AclCtrl { } isDuplicate(origItem, newItem) { + if (origItem.inherited) { + return false; + } + return (origItem.role && newItem.role && origItem.role === newItem.role) || (origItem.userId && newItem.userId && origItem.userId === newItem.userId) || (origItem.userGroupId && newItem.userGroupId && origItem.userGroupId === newItem.userGroupId); diff --git a/public/app/features/dashboard/acl/specs/acl_specs.ts b/public/app/features/dashboard/acl/specs/acl_specs.ts index 99a5cfd2339..479db4b4de9 100644 --- a/public/app/features/dashboard/acl/specs/acl_specs.ts +++ b/public/app/features/dashboard/acl/specs/acl_specs.ts @@ -9,7 +9,7 @@ describe('AclCtrl', () => { }; const dashboardSrv = { - getCurrent: sinon.stub().returns({id: 1}) + getCurrent: sinon.stub().returns({id: 1, meta: { isFolder: false }}) }; beforeEach(angularMocks.module('grafana.core')); @@ -130,7 +130,7 @@ describe('AclCtrl', () => { backendSrv.post.reset(); ctx.ctrl.items = []; - const userGroupItem = { + const userGroupItem = { id: 2, name: 'ug1', }; @@ -147,4 +147,34 @@ describe('AclCtrl', () => { expect(ctx.ctrl.items.length).to.eql(1); }); }); + + describe('when one inherited and one not inherited user group permission are added', () => { + beforeEach(() => { + backendSrv.get.reset(); + backendSrv.post.reset(); + ctx.ctrl.items = []; + + const inheritedUserGroupItem = { + id: 2, + name: 'ug1', + dashboardId: -1 + }; + + ctx.ctrl.items.push(inheritedUserGroupItem); + + const userGroupItem = { + id: 2, + name: 'ug1', + }; + ctx.ctrl.groupPicked(userGroupItem); + }); + + it('should not throw a validation error', () => { + expect(ctx.ctrl.error).to.eql(''); + }); + + it('should add both permissions', () => { + expect(ctx.ctrl.items.length).to.eql(2); + }); + }); });