RBAC: Only check for the write action when listing editable dashboards/folders (#88518)

* only check for the write action when listing editable resources

* test fix
This commit is contained in:
Ieva 2024-06-10 14:44:34 +03:00 committed by GitHub
parent 84b638fb26
commit 35d0597367
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 90 additions and 137 deletions

View File

@ -25,10 +25,10 @@ type clause struct {
}
type accessControlDashboardPermissionFilter struct {
user identity.Requester
dashboardActions []string
folderActions []string
features featuremgmt.FeatureToggles
user identity.Requester
dashboardAction string
folderAction string
features featuremgmt.FeatureToggles
where clause
// any recursive CTE queries (if supported)
@ -51,38 +51,32 @@ type PermissionsFilter interface {
func NewAccessControlDashboardPermissionFilter(user identity.Requester, permissionLevel dashboardaccess.PermissionType, queryType string, features featuremgmt.FeatureToggles, recursiveQueriesAreSupported bool) PermissionsFilter {
needEdit := permissionLevel > dashboardaccess.PERMISSION_VIEW
var folderActions []string
var dashboardActions []string
var folderAction string
var dashboardAction string
if queryType == searchstore.TypeFolder {
folderActions = append(folderActions, dashboards.ActionFoldersRead)
folderAction = dashboards.ActionFoldersRead
//folderAction = append(folderAction, dashboards.ActionFoldersRead)
if needEdit {
folderActions = append(folderActions, dashboards.ActionDashboardsCreate)
folderAction = dashboards.ActionDashboardsCreate
}
} else if queryType == searchstore.TypeDashboard {
dashboardActions = append(dashboardActions, dashboards.ActionDashboardsRead)
dashboardAction = dashboards.ActionDashboardsRead
if needEdit {
dashboardActions = append(dashboardActions, dashboards.ActionDashboardsWrite)
dashboardAction = dashboards.ActionDashboardsWrite
}
} else if queryType == searchstore.TypeAlertFolder {
folderActions = append(
folderActions,
dashboards.ActionFoldersRead,
accesscontrol.ActionAlertingRuleRead,
)
folderAction = accesscontrol.ActionAlertingRuleRead
if needEdit {
folderActions = append(
folderActions,
accesscontrol.ActionAlertingRuleCreate,
)
folderAction = accesscontrol.ActionAlertingRuleCreate
}
} else if queryType == searchstore.TypeAnnotation {
dashboardActions = append(dashboardActions, accesscontrol.ActionAnnotationsRead)
dashboardAction = accesscontrol.ActionAnnotationsRead
} else {
folderActions = append(folderActions, dashboards.ActionFoldersRead)
dashboardActions = append(dashboardActions, dashboards.ActionDashboardsRead)
folderAction = dashboards.ActionFoldersRead
dashboardAction = dashboards.ActionDashboardsRead
if needEdit {
folderActions = append(folderActions, dashboards.ActionDashboardsCreate)
dashboardActions = append(dashboardActions, dashboards.ActionDashboardsWrite)
folderAction = dashboards.ActionDashboardsCreate
dashboardAction = dashboards.ActionDashboardsWrite
}
}
@ -90,12 +84,12 @@ func NewAccessControlDashboardPermissionFilter(user identity.Requester, permissi
if features.IsEnabledGlobally(featuremgmt.FlagPermissionsFilterRemoveSubquery) {
f = &accessControlDashboardPermissionFilterNoFolderSubquery{
accessControlDashboardPermissionFilter: accessControlDashboardPermissionFilter{
user: user, folderActions: folderActions, dashboardActions: dashboardActions, features: features,
user: user, folderAction: folderAction, dashboardAction: dashboardAction, features: features,
recursiveQueriesAreSupported: recursiveQueriesAreSupported,
},
}
} else {
f = &accessControlDashboardPermissionFilter{user: user, folderActions: folderActions, dashboardActions: dashboardActions, features: features,
f = &accessControlDashboardPermissionFilter{user: user, folderAction: folderAction, dashboardAction: dashboardAction, features: features,
recursiveQueriesAreSupported: recursiveQueriesAreSupported,
}
}
@ -117,7 +111,7 @@ func (f *accessControlDashboardPermissionFilter) Where() (string, []any) {
// Check if user has no permissions required for search to skip expensive query
func (f *accessControlDashboardPermissionFilter) hasRequiredActions() bool {
permissions := f.user.GetPermissions()
requiredActions := append(f.folderActions, f.dashboardActions...)
requiredActions := []string{f.folderAction, f.dashboardAction}
for _, action := range requiredActions {
if _, ok := permissions[action]; ok {
return true
@ -155,24 +149,16 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
// currently it's used for the extended JWT module (when the user is authenticated via a JWT token generated by Grafana)
useSelfContainedPermissions := f.user.IsAuthenticatedBy(login.ExtendedJWTModule)
if len(f.dashboardActions) > 0 {
toCheck := actionsToCheck(f.dashboardActions, f.user.GetPermissions(), dashWildcards, folderWildcards)
if len(f.dashboardAction) > 0 {
toCheck := actionsToCheck(f.dashboardAction, f.user.GetPermissions(), dashWildcards, folderWildcards)
if len(toCheck) > 0 {
if !useSelfContainedPermissions {
builder.WriteString("(dashboard.uid IN (SELECT identifier FROM permission WHERE kind = 'dashboards' AND attribute = 'uid'")
builder.WriteString(rolesFilter)
args = append(args, params...)
if len(toCheck) == 1 {
builder.WriteString(" AND action = ?")
args = append(args, toCheck[0])
} else {
builder.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope, identifier HAVING COUNT(action) = ?")
args = append(args, toCheck...)
args = append(args, len(toCheck))
}
builder.WriteString(") AND NOT dashboard.is_folder)")
builder.WriteString(" AND action = ?) AND NOT dashboard.is_folder)")
args = append(args, toCheck[0])
} else {
actions := parseStringSliceFromInterfaceSlice(toCheck)
@ -193,15 +179,8 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
permSelector.WriteString("(SELECT identifier FROM permission WHERE kind = 'folders' AND attribute = 'uid'")
permSelector.WriteString(rolesFilter)
permSelectorArgs = append(permSelectorArgs, params...)
if len(toCheck) == 1 {
permSelector.WriteString(" AND action = ?")
permSelectorArgs = append(permSelectorArgs, toCheck[0])
} else {
permSelector.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope, identifier HAVING COUNT(action) = ?")
permSelectorArgs = append(permSelectorArgs, toCheck...)
permSelectorArgs = append(permSelectorArgs, len(toCheck))
}
permSelector.WriteString(" AND action = ?")
permSelectorArgs = append(permSelectorArgs, toCheck[0])
} else {
actions := parseStringSliceFromInterfaceSlice(toCheck)
@ -262,25 +241,19 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
permSelector.Reset()
permSelectorArgs = permSelectorArgs[:0]
if len(f.folderActions) > 0 {
if len(f.dashboardActions) > 0 {
if len(f.folderAction) > 0 {
if len(f.dashboardAction) > 0 {
builder.WriteString(" OR ")
}
toCheck := actionsToCheck(f.folderActions, f.user.GetPermissions(), folderWildcards)
toCheck := actionsToCheck(f.folderAction, f.user.GetPermissions(), folderWildcards)
if len(toCheck) > 0 {
if !useSelfContainedPermissions {
permSelector.WriteString("(SELECT identifier FROM permission WHERE kind = 'folders' AND attribute = 'uid'")
permSelector.WriteString(rolesFilter)
permSelectorArgs = append(permSelectorArgs, params...)
if len(toCheck) == 1 {
permSelector.WriteString(" AND action = ?")
permSelectorArgs = append(permSelectorArgs, toCheck[0])
} else {
permSelector.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope, identifier HAVING COUNT(action) = ?")
permSelectorArgs = append(permSelectorArgs, toCheck...)
permSelectorArgs = append(permSelectorArgs, len(toCheck))
}
permSelector.WriteString(" AND action = ?")
permSelectorArgs = append(permSelectorArgs, toCheck[0])
} else {
actions := parseStringSliceFromInterfaceSlice(toCheck)
@ -369,27 +342,16 @@ func (f *accessControlDashboardPermissionFilter) addRecQry(queryName string, whe
})
}
func actionsToCheck(actions []string, permissions map[string][]string, wildcards ...accesscontrol.Wildcards) []any {
toCheck := make([]any, 0, len(actions))
for _, a := range actions {
var hasWildcard bool
outer:
for _, scope := range permissions[a] {
for _, w := range wildcards {
if w.Contains(scope) {
hasWildcard = true
break outer
}
func actionsToCheck(action string, permissions map[string][]string, wildcards ...accesscontrol.Wildcards) []any {
for _, scope := range permissions[action] {
for _, w := range wildcards {
if w.Contains(scope) {
return []any{}
}
}
if !hasWildcard {
toCheck = append(toCheck, a)
}
}
return toCheck
return []any{action}
}
func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTable string, leftCol string, rightTableCol string, orgID int64) (string, []any) {

View File

@ -53,24 +53,16 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
// currently it's used for the extended JWT module (when the user is authenticated via a JWT token generated by Grafana)
useSelfContainedPermissions := f.user.GetAuthenticatedBy() == login.ExtendedJWTModule
if len(f.dashboardActions) > 0 {
toCheck := actionsToCheck(f.dashboardActions, f.user.GetPermissions(), dashWildcards, folderWildcards)
if len(f.dashboardAction) > 0 {
toCheck := actionsToCheck(f.dashboardAction, f.user.GetPermissions(), dashWildcards, folderWildcards)
if len(toCheck) > 0 {
if !useSelfContainedPermissions {
builder.WriteString("(dashboard.uid IN (SELECT identifier FROM permission WHERE kind = 'dashboards' AND attribute = 'uid'")
builder.WriteString(rolesFilter)
args = append(args, params...)
if len(toCheck) == 1 {
builder.WriteString(" AND action = ?")
args = append(args, toCheck[0])
} else {
builder.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope, identifier HAVING COUNT(action) = ?")
args = append(args, toCheck...)
args = append(args, len(toCheck))
}
builder.WriteString(") AND NOT dashboard.is_folder)")
builder.WriteString(" AND action = ?) AND NOT dashboard.is_folder)")
args = append(args, toCheck[0])
} else {
actions := parseStringSliceFromInterfaceSlice(toCheck)
@ -91,15 +83,8 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
permSelector.WriteString("(SELECT identifier FROM permission WHERE kind = 'folders' AND attribute = 'uid'")
permSelector.WriteString(rolesFilter)
permSelectorArgs = append(permSelectorArgs, params...)
if len(toCheck) == 1 {
permSelector.WriteString(" AND action = ?")
permSelectorArgs = append(permSelectorArgs, toCheck[0])
} else {
permSelector.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope, identifier HAVING COUNT(action) = ?")
permSelectorArgs = append(permSelectorArgs, toCheck...)
permSelectorArgs = append(permSelectorArgs, len(toCheck))
}
permSelector.WriteString(" AND action = ?")
permSelectorArgs = append(permSelectorArgs, toCheck[0])
} else {
actions := parseStringSliceFromInterfaceSlice(toCheck)
@ -160,25 +145,19 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
permSelector.Reset()
permSelectorArgs = permSelectorArgs[:0]
if len(f.folderActions) > 0 {
if len(f.dashboardActions) > 0 {
if len(f.folderAction) > 0 {
if len(f.dashboardAction) > 0 {
builder.WriteString(" OR ")
}
toCheck := actionsToCheck(f.folderActions, f.user.GetPermissions(), folderWildcards)
toCheck := actionsToCheck(f.folderAction, f.user.GetPermissions(), folderWildcards)
if len(toCheck) > 0 {
if !useSelfContainedPermissions {
permSelector.WriteString("(SELECT identifier FROM permission WHERE kind = 'folders' AND attribute = 'uid'")
permSelector.WriteString(rolesFilter)
permSelectorArgs = append(permSelectorArgs, params...)
if len(toCheck) == 1 {
permSelector.WriteString(" AND action = ?")
permSelectorArgs = append(permSelectorArgs, toCheck[0])
} else {
permSelector.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope, identifier HAVING COUNT(action) = ?")
permSelectorArgs = append(permSelectorArgs, toCheck...)
permSelectorArgs = append(permSelectorArgs, len(toCheck))
}
permSelector.WriteString(" AND action = ?")
permSelectorArgs = append(permSelectorArgs, toCheck[0])
} else {
actions := parseStringSliceFromInterfaceSlice(toCheck)

View File

@ -125,6 +125,7 @@ func TestBuilder_RBAC(t *testing.T) {
testsCases := []struct {
desc string
userPermissions []accesscontrol.Permission
level dashboardaccess.PermissionType
features featuremgmt.FeatureToggles
expectedParams []any
}{
@ -140,6 +141,7 @@ func TestBuilder_RBAC(t *testing.T) {
userPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"},
},
level: dashboardaccess.PERMISSION_VIEW,
features: featuremgmt.WithFeatures(),
expectedParams: []any{
int64(1),
@ -150,8 +152,6 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"dashboards:read",
"dashboards:write",
2,
int64(1),
int64(1),
int64(1),
@ -160,8 +160,6 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"dashboards:read",
"dashboards:write",
2,
int64(1),
int64(1),
0,
@ -169,8 +167,39 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"folders:read",
},
},
{
desc: "user with write permission",
userPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsWrite, Scope: "dashboards:uid:1"},
},
level: dashboardaccess.PERMISSION_EDIT,
features: featuremgmt.WithFeatures(),
expectedParams: []any{
int64(1),
int64(1),
int64(1),
0,
"Viewer",
int64(1),
0,
"dashboards:write",
int64(1),
int64(1),
int64(1),
0,
"Viewer",
int64(1),
0,
"dashboards:write",
int64(1),
int64(1),
0,
"Viewer",
int64(1),
0,
"dashboards:create",
2,
},
},
{
@ -178,6 +207,7 @@ func TestBuilder_RBAC(t *testing.T) {
userPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"},
},
level: dashboardaccess.PERMISSION_VIEW,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
expectedParams: []any{
int64(1),
@ -188,8 +218,6 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"dashboards:read",
"dashboards:write",
2,
int64(1),
int64(1),
int64(1),
@ -198,8 +226,6 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"folders:read",
"dashboards:create",
2,
int64(1),
int64(1),
int64(1),
@ -208,8 +234,6 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"dashboards:read",
"dashboards:write",
2,
int64(1),
},
},
@ -218,6 +242,7 @@ func TestBuilder_RBAC(t *testing.T) {
userPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"},
},
level: dashboardaccess.PERMISSION_VIEW,
features: featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery),
expectedParams: []any{
int64(1),
@ -228,8 +253,6 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"dashboards:read",
"dashboards:write",
2,
int64(1),
int64(1),
0,
@ -237,8 +260,6 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"dashboards:read",
"dashboards:write",
2,
int64(1),
int64(1),
0,
@ -246,15 +267,14 @@ func TestBuilder_RBAC(t *testing.T) {
int64(1),
0,
"folders:read",
"dashboards:create",
2,
},
},
{
desc: "user with view permission with nesting and remove subquery",
desc: "user with edit permission with nesting and remove subquery",
userPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"},
{Action: dashboards.ActionDashboardsWrite, Scope: "dashboards:uid:1"},
},
level: dashboardaccess.PERMISSION_EDIT,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
expectedParams: []any{
int64(1),
@ -264,9 +284,7 @@ func TestBuilder_RBAC(t *testing.T) {
"Viewer",
int64(1),
0,
"dashboards:read",
"dashboards:write",
2,
int64(1),
int64(1),
int64(1),
@ -274,9 +292,7 @@ func TestBuilder_RBAC(t *testing.T) {
"Viewer",
int64(1),
0,
"folders:read",
"dashboards:create",
2,
int64(1),
int64(1),
int64(1),
@ -284,9 +300,7 @@ func TestBuilder_RBAC(t *testing.T) {
"Viewer",
int64(1),
0,
"dashboards:read",
"dashboards:write",
2,
},
},
}
@ -309,15 +323,13 @@ func TestBuilder_RBAC(t *testing.T) {
user.Permissions = map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tc.userPermissions)}
}
level := dashboardaccess.PERMISSION_EDIT
builder := &searchstore.Builder{
Filters: []any{
searchstore.OrgFilter{OrgId: user.OrgID},
searchstore.TitleSorter{},
permissions.NewAccessControlDashboardPermissionFilter(
user,
level,
tc.level,
"",
tc.features,
recursiveQueriesAreSupported,