diff --git a/pkg/api/dashboard_permission.go b/pkg/api/dashboard_permission.go index 4ae1ce4d36e16..c4fbbbdd13d41 100644 --- a/pkg/api/dashboard_permission.go +++ b/pkg/api/dashboard_permission.go @@ -126,6 +126,10 @@ func (hs *HTTPServer) UpdateDashboardPermissions(c *models.ReqContext) response. return response.Error(400, err.Error(), err) } + if err == guardian.ErrGuardianOverrideDuplicate { + return response.Error(409, err.Error(), err) + } + return response.Error(500, "Error while checking dashboard permissions", err) } diff --git a/pkg/services/dashboards/database/acl.go b/pkg/services/dashboards/database/acl.go index 940a2ab7aad16..37eb9dd79347a 100644 --- a/pkg/services/dashboards/database/acl.go +++ b/pkg/services/dashboards/database/acl.go @@ -63,15 +63,20 @@ func (d *DashboardStore) GetDashboardAclInfoList(ctx context.Context, query *mod d.is_folder, CASE WHEN (da.dashboard_id = -1 AND d.folder_id > 0) OR da.dashboard_id = d.folder_id THEN ` + d.dialect.BooleanStr(true) + ` ELSE ` + falseStr + ` END AS inherited FROM dashboard as d + -- get folder dashboards for reference purposes LEFT JOIN dashboard folder on folder.id = d.folder_id + -- get the acls under the following conditions LEFT JOIN dashboard_acl AS da ON + -- if it is for this dashboard da.dashboard_id = d.id OR + -- or if it is for the parent folder da.dashboard_id = d.folder_id OR ( - -- include default permissions --> - da.org_id = -1 AND ( - (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR - (folder.id IS NULL AND d.has_acl = ` + falseStr + `) + -- include default permissions if dashboard has no acl AND ((folder is set without acl) + -- OR ( folder is NOT set and dashboard has no acl set )) + da.org_id = -1 AND d.has_acl = ` + falseStr + ` AND ( + (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR + (folder.id IS NULL) ) ) LEFT JOIN ` + d.dialect.Quote("user") + ` AS u ON u.id = da.user_id diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 847654c416c23..c5bc462b6352d 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -201,6 +201,8 @@ func (d *DashboardStore) UpdateDashboardACL(ctx context.Context, dashboardID int return fmt.Errorf("deleting from dashboard_acl failed: %w", err) } + hasAcl := false + for _, item := range items { if item.UserID == 0 && item.TeamID == 0 && (item.Role == nil || !item.Role.IsValid()) { return models.ErrDashboardAclInfoMissing @@ -214,10 +216,12 @@ func (d *DashboardStore) UpdateDashboardACL(ctx context.Context, dashboardID int if _, err := sess.Insert(item); err != nil { return err } + + hasAcl = true } // Update dashboard HasAcl flag - dashboard := models.Dashboard{HasAcl: true} + dashboard := models.Dashboard{HasAcl: hasAcl} _, err = sess.Cols("has_acl").Where("id=?", dashboardID).Update(&dashboard) return err }) diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index fcf278642efc5..506ba9d584a13 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -12,8 +12,9 @@ import ( ) var ( - ErrGuardianPermissionExists = errors.New("permission already exists") - ErrGuardianOverride = errors.New("you can only override a permission to be higher") + ErrGuardianPermissionExists = errors.New("permission already exists") + ErrGuardianOverride = errors.New("you can only override a permission to be higher") + ErrGuardianOverrideDuplicate = errors.New("you can not override with duplicate permissions") ) // DashboardGuardian to be used for guard against operations without access on dashboard and acl @@ -205,9 +206,15 @@ func (g *dashboardGuardianImpl) CheckPermissionBeforeUpdate(permission models.Pe continue } - if a.IsDuplicateOf(existingPerm) && a.Permission <= existingPerm.Permission { - return false, ErrGuardianOverride + if a.IsDuplicateOf(existingPerm) { + return false, ErrGuardianOverrideDuplicate } + // Disabling the following check because it goes against the way we are using permissions https://soracom.slack.com/archives/C9PHDH1QB/p1555664739066900 + // existing permissions returned by GetAcl above include default parent folder permissions if this dashboard has never had permissions set before + // and is inside a folder. That could possibly cause an error _before_ we are able to set the permissions catch 22. + // if a.Permission <= existingPerm.Permission { + // return false, ErrGuardianOverride + // } } }