RBAC: Permission check performance improvements for the new search (#60729)

* Add checker and update the resource filter function for new search

* Add tests for checker

* small fixes

* handle location for panels correctly

* clean up checker code and extend the tests for it

* more fixes, but tests don't quite work yet

* a small change to return error

* cleanup

* more simplification

* fix tests

* correct wrong argument ordering & use constant

* Apply suggestions from code review

Co-authored-by: Artur Wierzbicki <artur.wierzbicki@grafana.com>

* import

* check general folder from permission checker function

* handle root folder aka general folder properly

* update tests

* clean up

* lint

* add fix from main

Co-authored-by: Karl Persson <kalle.persson@grafana.com>
Co-authored-by: Artur Wierzbicki <artur.wierzbicki@grafana.com>
This commit is contained in:
Ieva
2023-01-27 12:12:30 +00:00
committed by GitHub
co-authored by Artur Wierzbicki Karl Persson
parent dab3fac01b
commit eb9ef34272
27 changed files with 289 additions and 119 deletions
+57
View File
@@ -0,0 +1,57 @@
package accesscontrol
import (
"github.com/grafana/grafana/pkg/services/user"
)
func Checker(user *user.SignedInUser, action string) func(scopes ...string) bool {
if user.Permissions == nil || user.Permissions[user.OrgID] == nil {
return func(scopes ...string) bool { return false }
}
userScopes, ok := user.Permissions[user.OrgID][action]
if !ok {
return func(scopes ...string) bool { return false }
}
lookup := make(map[string]bool, len(userScopes))
for i := range userScopes {
lookup[userScopes[i]] = true
}
var checkedWildcards bool
var hasWildcard bool
return func(scopes ...string) bool {
if !checkedWildcards {
wildcards := wildcardsFromScopes(scopes...)
for _, w := range wildcards {
if _, ok := lookup[w]; ok {
hasWildcard = true
break
}
}
checkedWildcards = true
}
if hasWildcard {
return true
}
for _, s := range scopes {
if lookup[s] {
return true
}
}
return false
}
}
func wildcardsFromScopes(scopes ...string) Wildcards {
prefixes := make([]string, len(scopes))
for _, scope := range scopes {
prefixes = append(prefixes, ScopePrefix(scope))
}
return WildcardsFromPrefixes(prefixes)
}
+111
View File
@@ -0,0 +1,111 @@
package accesscontrol
import (
"strconv"
"testing"
"github.com/grafana/grafana/pkg/services/user"
"github.com/stretchr/testify/assert"
)
type testData struct {
uid string
folderUid string
}
func (d testData) Scopes() []string {
return []string{
"dashboards:uid:" + d.uid,
"folders:uid:" + d.folderUid,
}
}
func generateTestData() []testData {
var data []testData
for i := 1; i < 100; i++ {
data = append(data, testData{
uid: strconv.Itoa(i),
folderUid: strconv.Itoa(i + 100),
})
}
return data
}
func Test_Checker(t *testing.T) {
data := generateTestData()
type testCase struct {
desc string
user *user.SignedInUser
expectedLen int
}
tests := []testCase{
{
desc: "should pass for every entity with dashboard wildcard scope",
user: &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {"dashboards:read": {"dashboards:*"}}},
},
expectedLen: len(data),
},
{
desc: "should pass for every entity with folder wildcard scope",
user: &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {"dashboards:read": {"folders:*"}}},
},
expectedLen: len(data),
},
{
desc: "should only pass for for 3 scopes",
user: &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {"dashboards:read": {"dashboards:uid:4", "dashboards:uid:50", "dashboards:uid:99"}}},
},
expectedLen: 3,
},
{
desc: "should only pass 4 with secondary supported scope",
user: &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {"dashboards:read": {"folders:uid:104", "folders:uid:150", "folders:uid:154", "folders:uid:199"}}},
},
expectedLen: 4,
},
{
desc: "should only pass 4 with some dashboard and some folder scopes",
user: &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {"dashboards:read": {"dashboards:uid:1", "dashboards:uid:2", "folders:uid:154", "folders:uid:199"}}},
},
expectedLen: 4,
},
{
desc: "should only pass 2 with overlapping dashboard and folder scopes",
user: &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {"dashboards:read": {"dashboards:uid:101", "dashboards:uid:2", "folders:uid:101", "folders:uid:102"}}},
},
expectedLen: 2,
},
{
desc: "should pass none for missing action",
user: &user.SignedInUser{
OrgID: 1,
Permissions: map[int64]map[string][]string{1: {}},
},
expectedLen: 0,
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
check := Checker(tt.user, "dashboards:read")
numPasses := 0
for _, d := range data {
if ok := check(d.Scopes()...); ok {
numPasses++
}
}
assert.Equal(t, tt.expectedLen, numPasses)
})
}
}
+16 -9
View File
@@ -142,19 +142,26 @@ func (s scopeProviderImpl) GetResourceAllIDScope() string {
return GetResourceAllIDScope(s.root)
}
// WildcardsFromPrefix generates valid wildcards from prefix
// datasource:uid: => "*", "datasource:*", "datasource:uid:*"
func WildcardsFromPrefix(prefix string) Wildcards {
return WildcardsFromPrefixes([]string{prefix})
}
// WildcardsFromPrefixes generates valid wildcards from prefixes
// datasource:uid: => "*", "datasource:*", "datasource:uid:*"
func WildcardsFromPrefixes(prefixes []string) Wildcards {
var b strings.Builder
wildcards := Wildcards{"*"}
parts := strings.Split(prefix, ":")
for _, p := range parts {
if p == "" {
continue
for _, prefix := range prefixes {
parts := strings.Split(prefix, ":")
for _, p := range parts {
if p == "" {
continue
}
b.WriteString(p)
b.WriteRune(':')
wildcards = append(wildcards, b.String()+"*")
}
b.WriteString(p)
b.WriteRune(':')
wildcards = append(wildcards, b.String()+"*")
b.Reset()
}
return wildcards
}