From d86ed679b14d172aa0e3945eed32d7c738b03ecd Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 23 Apr 2018 09:23:14 +0200 Subject: [PATCH] return inherited property for permissions --- pkg/models/dashboard_acl.go | 1 + pkg/services/guardian/guardian.go | 14 +----------- pkg/services/guardian/guardian_test.go | 8 +++---- pkg/services/sqlstore/dashboard_acl.go | 6 +++-- pkg/services/sqlstore/dashboard_acl_test.go | 25 +++++++++++++++++++++ 5 files changed, 35 insertions(+), 19 deletions(-) diff --git a/pkg/models/dashboard_acl.go b/pkg/models/dashboard_acl.go index 4ef8061486b..5fc09bd16b5 100644 --- a/pkg/models/dashboard_acl.go +++ b/pkg/models/dashboard_acl.go @@ -69,6 +69,7 @@ type DashboardAclInfoDTO struct { Slug string `json:"slug"` IsFolder bool `json:"isFolder"` Url string `json:"url"` + Inherited bool `json:"inherited"` } func (dto *DashboardAclInfoDTO) hasSameRoleAs(other *DashboardAclInfoDTO) bool { diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 700f22d8d26..bf455adc7ca 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -154,12 +154,7 @@ func (g *dashboardGuardianImpl) CheckPermissionBeforeUpdate(permission m.Permiss // validate overridden permissions to be higher for _, a := range acl { for _, existingPerm := range existingPermissions { - // handle default permissions - if existingPerm.DashboardId == -1 { - existingPerm.DashboardId = g.dashId - } - - if a.DashboardId == existingPerm.DashboardId { + if !existingPerm.Inherited { continue } @@ -187,13 +182,6 @@ func (g *dashboardGuardianImpl) GetAcl() ([]*m.DashboardAclInfoDTO, error) { return nil, err } - for _, a := range query.Result { - // handle default permissions - if a.DashboardId == -1 { - a.DashboardId = g.dashId - } - } - g.acl = query.Result return g.acl, nil } diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go index 9de12c60fea..abf92ae0555 100644 --- a/pkg/services/guardian/guardian_test.go +++ b/pkg/services/guardian/guardian_test.go @@ -217,13 +217,13 @@ func (sc *scenarioContext) parentFolderPermissionScenario(pt permissionType, per switch pt { case USER: - folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, UserId: userID, Permission: permission}} + folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, UserId: userID, Permission: permission, Inherited: true}} case TEAM: - folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, TeamId: teamID, Permission: permission}} + folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, TeamId: teamID, Permission: permission, Inherited: true}} case EDITOR: - folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, Role: &editorRole, Permission: permission}} + folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, Role: &editorRole, Permission: permission, Inherited: true}} case VIEWER: - folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, Role: &viewerRole, Permission: permission}} + folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, Role: &viewerRole, Permission: permission, Inherited: true}} } permissionScenario(fmt.Sprintf("and parent folder has %s with permission to %s", pt.String(), permission.String()), childDashboardID, sc, folderPermissionList, func(sc *scenarioContext) { diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index 2034ccf0d30..0b195c4562b 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -67,7 +67,8 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { '' as title, '' as slug, '' as uid,` + - falseStr + ` AS is_folder + falseStr + ` AS is_folder,` + + falseStr + ` AS inherited FROM dashboard_acl as da WHERE da.dashboard_id = -1` query.Result = make([]*m.DashboardAclInfoDTO, 0) @@ -94,7 +95,8 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { d.title, d.slug, d.uid, - d.is_folder + d.is_folder, + CASE WHEN (da.dashboard_id = -1 AND d.folder_id > 0) OR da.dashboard_id = d.folder_id THEN ` + dialect.BooleanStr(true) + ` ELSE ` + falseStr + ` END AS inherited FROM dashboard as d LEFT JOIN dashboard folder on folder.id = d.folder_id LEFT JOIN dashboard_acl AS da ON diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index 8fbb9c0d813..c68ffd08cd7 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/pkg/services/sqlstore/dashboard_acl_test.go @@ -26,6 +26,22 @@ func TestDashboardAclDataAccess(t *testing.T) { }) Convey("Given dashboard folder with default permissions", func() { + Convey("When reading folder acl should include default acl", func() { + query := m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.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[0].Inherited, ShouldBeFalse) + So(query.Result[1].DashboardId, ShouldEqual, defaultPermissionsId) + So(*query.Result[1].Role, ShouldEqual, m.ROLE_EDITOR) + So(query.Result[1].Inherited, ShouldBeFalse) + }) + Convey("When reading dashboard acl should include acl for parent folder", func() { query := m.GetDashboardAclInfoListQuery{DashboardId: childDash.Id, OrgId: 1} @@ -36,8 +52,10 @@ func TestDashboardAclDataAccess(t *testing.T) { defaultPermissionsId := -1 So(query.Result[0].DashboardId, ShouldEqual, defaultPermissionsId) So(*query.Result[0].Role, ShouldEqual, m.ROLE_VIEWER) + So(query.Result[0].Inherited, ShouldBeTrue) So(query.Result[1].DashboardId, ShouldEqual, defaultPermissionsId) So(*query.Result[1].Role, ShouldEqual, m.ROLE_EDITOR) + So(query.Result[1].Inherited, ShouldBeTrue) }) }) @@ -94,7 +112,9 @@ func TestDashboardAclDataAccess(t *testing.T) { So(len(query.Result), ShouldEqual, 2) So(query.Result[0].DashboardId, ShouldEqual, savedFolder.Id) + So(query.Result[0].Inherited, ShouldBeTrue) So(query.Result[1].DashboardId, ShouldEqual, childDash.Id) + So(query.Result[1].Inherited, ShouldBeFalse) }) }) }) @@ -118,9 +138,12 @@ func TestDashboardAclDataAccess(t *testing.T) { 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[0].Inherited, ShouldBeTrue) So(query.Result[1].DashboardId, ShouldEqual, defaultPermissionsId) So(*query.Result[1].Role, ShouldEqual, m.ROLE_EDITOR) + So(query.Result[1].Inherited, ShouldBeTrue) So(query.Result[2].DashboardId, ShouldEqual, childDash.Id) + So(query.Result[2].Inherited, ShouldBeFalse) }) }) @@ -209,8 +232,10 @@ func TestDashboardAclDataAccess(t *testing.T) { defaultPermissionsId := -1 So(query.Result[0].DashboardId, ShouldEqual, defaultPermissionsId) So(*query.Result[0].Role, ShouldEqual, m.ROLE_VIEWER) + So(query.Result[0].Inherited, ShouldBeFalse) So(query.Result[1].DashboardId, ShouldEqual, defaultPermissionsId) So(*query.Result[1].Role, ShouldEqual, m.ROLE_EDITOR) + So(query.Result[1].Inherited, ShouldBeFalse) }) }) })