Permissions: Fix inherited folder permissions can prevent new permissions being added to a dashboard (#33329)

In the case permissions has been added on dashboard(s). Later permissions for the 
parent folder of the dashboard is edited in such a way that dashboard in that folder 
has a permission that is a duplicate of an inherited one. This PR changes so that 
duplicate permissions are now filtered out from /api/dashboards/id/<dashboard id>/permissions.
Duplicate permission are not filtered out if the permission on dashboard is higher 
than on the inherited folder.

Fixes #33296

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
This commit is contained in:
Marcus Efraimsson 2021-04-28 14:42:18 +02:00 committed by GitHub
parent 6d95f2f1db
commit 7e6db1ee7e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 90 additions and 1 deletions

View File

@ -24,7 +24,7 @@ func (hs *HTTPServer) GetDashboardPermissionList(c *models.ReqContext) response.
return dashboardGuardianResponse(err)
}
acl, err := g.GetAcl()
acl, err := g.GetACLWithoutDuplicates()
if err != nil {
return response.Error(500, "Failed to get dashboard permissions", err)
}

View File

@ -22,7 +22,14 @@ type DashboardGuardian interface {
CanAdmin() (bool, error)
HasPermission(permission models.PermissionType) (bool, error)
CheckPermissionBeforeUpdate(permission models.PermissionType, updatePermissions []*models.DashboardAcl) (bool, error)
// GetAcl returns ACL.
GetAcl() ([]*models.DashboardAclInfoDTO, error)
// GetACLWithoutDuplicates returns ACL and strips any permission
// that already has an inherited permission with higher or equal
// permission.
GetACLWithoutDuplicates() ([]*models.DashboardAclInfoDTO, error)
GetHiddenACL(*setting.Cfg) ([]*models.DashboardAcl, error)
}
@ -202,6 +209,42 @@ func (g *dashboardGuardianImpl) GetAcl() ([]*models.DashboardAclInfoDTO, error)
return g.acl, nil
}
func (g *dashboardGuardianImpl) GetACLWithoutDuplicates() ([]*models.DashboardAclInfoDTO, error) {
acl, err := g.GetAcl()
if err != nil {
return nil, err
}
nonInherited := []*models.DashboardAclInfoDTO{}
inherited := []*models.DashboardAclInfoDTO{}
for _, aclItem := range acl {
if aclItem.Inherited {
inherited = append(inherited, aclItem)
} else {
nonInherited = append(nonInherited, aclItem)
}
}
result := []*models.DashboardAclInfoDTO{}
for _, nonInheritedAclItem := range nonInherited {
duplicate := false
for _, inheritedAclItem := range inherited {
if nonInheritedAclItem.IsDuplicateOf(inheritedAclItem) && nonInheritedAclItem.Permission <= inheritedAclItem.Permission {
duplicate = true
break
}
}
if !duplicate {
result = append(result, nonInheritedAclItem)
}
}
result = append(inherited, result...)
return result, nil
}
func (g *dashboardGuardianImpl) getTeams() ([]*models.TeamDTO, error) {
if g.teams != nil {
return g.teams, nil
@ -290,6 +333,10 @@ func (g *FakeDashboardGuardian) GetAcl() ([]*models.DashboardAclInfoDTO, error)
return g.GetAclValue, nil
}
func (g *FakeDashboardGuardian) GetACLWithoutDuplicates() ([]*models.DashboardAclInfoDTO, error) {
return g.GetAcl()
}
func (g *FakeDashboardGuardian) GetHiddenACL(cfg *setting.Cfg) ([]*models.DashboardAcl, error) {
return g.GetHiddenAclValue, nil
}

View File

@ -726,3 +726,45 @@ func TestGuardianGetHiddenACL(t *testing.T) {
})
})
}
func TestGuardianGetAclWithoutDuplicates(t *testing.T) {
t.Run("Get hidden ACL tests", func(t *testing.T) {
t.Cleanup(bus.ClearBusHandlers)
bus.AddHandler("test", func(query *models.GetDashboardAclInfoListQuery) error {
query.Result = []*models.DashboardAclInfoDTO{
{Inherited: true, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_EDIT},
{Inherited: false, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_VIEW},
{Inherited: false, UserId: 2, UserLogin: "user2", Permission: models.PERMISSION_ADMIN},
{Inherited: true, UserId: 4, UserLogin: "user4", Permission: models.PERMISSION_ADMIN},
{Inherited: false, UserId: 4, UserLogin: "user4", Permission: models.PERMISSION_ADMIN},
{Inherited: false, UserId: 5, UserLogin: "user5", Permission: models.PERMISSION_EDIT},
{Inherited: true, UserId: 6, UserLogin: "user6", Permission: models.PERMISSION_VIEW},
{Inherited: false, UserId: 6, UserLogin: "user6", Permission: models.PERMISSION_EDIT},
}
return nil
})
t.Run("Should get acl without duplicates", func(t *testing.T) {
user := &models.SignedInUser{
OrgId: orgID,
UserId: 1,
Login: "user1",
}
g := New(dashboardID, orgID, user)
acl, err := g.GetACLWithoutDuplicates()
require.NoError(t, err)
require.NotNil(t, acl)
require.Len(t, acl, 6)
require.ElementsMatch(t, []*models.DashboardAclInfoDTO{
{Inherited: true, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_EDIT},
{Inherited: true, UserId: 4, UserLogin: "user4", Permission: models.PERMISSION_ADMIN},
{Inherited: true, UserId: 6, UserLogin: "user6", Permission: models.PERMISSION_VIEW},
{Inherited: false, UserId: 2, UserLogin: "user2", Permission: models.PERMISSION_ADMIN},
{Inherited: false, UserId: 5, UserLogin: "user5", Permission: models.PERMISSION_EDIT},
{Inherited: false, UserId: 6, UserLogin: "user6", Permission: models.PERMISSION_EDIT},
}, acl)
})
})
}