mirror of https://github.com/grafana/grafana
RBAC: Improve performance of dashboard filter query (#56813)
* RBAC: Move UserRolesFilter to domain package * Dashboard Permissions: Rewrite rbac filter to check access in sql * RBAC: Add break when wildcard is found * RBAC: Add tests for dashboard filter * RBAC: Update tests * RBAC: Cover more test cases Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>pull/57589/head
parent
d2a70bc42d
commit
7386f8652c
@ -1,148 +1,229 @@ |
||||
package permissions |
||||
package permissions_test |
||||
|
||||
import ( |
||||
"fmt" |
||||
"context" |
||||
"strconv" |
||||
"testing" |
||||
"time" |
||||
|
||||
"github.com/stretchr/testify/assert" |
||||
"github.com/stretchr/testify/require" |
||||
|
||||
"github.com/grafana/grafana/pkg/components/simplejson" |
||||
"github.com/grafana/grafana/pkg/infra/db" |
||||
"github.com/grafana/grafana/pkg/models" |
||||
"github.com/grafana/grafana/pkg/services/accesscontrol" |
||||
"github.com/grafana/grafana/pkg/services/dashboards" |
||||
"github.com/grafana/grafana/pkg/services/org" |
||||
"github.com/grafana/grafana/pkg/services/sqlstore" |
||||
"github.com/grafana/grafana/pkg/services/sqlstore/permissions" |
||||
"github.com/grafana/grafana/pkg/services/sqlstore/searchstore" |
||||
"github.com/grafana/grafana/pkg/services/user" |
||||
"github.com/grafana/grafana/pkg/util" |
||||
"github.com/stretchr/testify/assert" |
||||
"github.com/stretchr/testify/require" |
||||
) |
||||
|
||||
func TestNewAccessControlDashboardPermissionFilter(t *testing.T) { |
||||
randomType := "random_" + util.GenerateShortUID() |
||||
testCases := []struct { |
||||
permission models.PermissionType |
||||
queryType string |
||||
expectedDashboardActions []string |
||||
expectedFolderActions []string |
||||
}{ |
||||
func TestIntegration_DashboardPermissionFilter(t *testing.T) { |
||||
if testing.Short() { |
||||
t.Skip("skipping integration test") |
||||
} |
||||
|
||||
type testCase struct { |
||||
desc string |
||||
queryType string |
||||
permission models.PermissionType |
||||
permissions []accesscontrol.Permission |
||||
expectedResult int |
||||
} |
||||
|
||||
tests := []testCase{ |
||||
{ |
||||
queryType: searchstore.TypeAlertFolder, |
||||
permission: models.PERMISSION_ADMIN, |
||||
expectedDashboardActions: nil, |
||||
expectedFolderActions: []string{ |
||||
dashboards.ActionFoldersRead, |
||||
accesscontrol.ActionAlertingRuleRead, |
||||
accesscontrol.ActionAlertingRuleCreate, |
||||
desc: "Should be able to view all dashboards with wildcard scope", |
||||
permission: models.PERMISSION_VIEW, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeDashboardsAll}, |
||||
}, |
||||
expectedResult: 100, |
||||
}, |
||||
{ |
||||
queryType: searchstore.TypeAlertFolder, |
||||
permission: models.PERMISSION_EDIT, |
||||
expectedDashboardActions: nil, |
||||
expectedFolderActions: []string{ |
||||
dashboards.ActionFoldersRead, |
||||
accesscontrol.ActionAlertingRuleRead, |
||||
accesscontrol.ActionAlertingRuleCreate, |
||||
desc: "Should be able to view all dashboards with folder wildcard scope", |
||||
permission: models.PERMISSION_VIEW, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll}, |
||||
}, |
||||
expectedResult: 100, |
||||
}, |
||||
{ |
||||
queryType: searchstore.TypeAlertFolder, |
||||
permission: models.PERMISSION_VIEW, |
||||
expectedDashboardActions: nil, |
||||
expectedFolderActions: []string{ |
||||
dashboards.ActionFoldersRead, |
||||
accesscontrol.ActionAlertingRuleRead, |
||||
desc: "Should be able to view a subset of dashboards with dashboard scopes", |
||||
permission: models.PERMISSION_VIEW, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:110"}, |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:40"}, |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:22"}, |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:13"}, |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:55"}, |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:99"}, |
||||
}, |
||||
expectedResult: 6, |
||||
}, |
||||
{ |
||||
queryType: randomType, |
||||
permission: models.PERMISSION_ADMIN, |
||||
expectedDashboardActions: []string{ |
||||
dashboards.ActionDashboardsRead, |
||||
dashboards.ActionDashboardsWrite, |
||||
desc: "Should be able to view a subset of dashboards with dashboard action and folder scope", |
||||
permission: models.PERMISSION_VIEW, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:8"}, |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:10"}, |
||||
}, |
||||
expectedFolderActions: []string{ |
||||
dashboards.ActionFoldersRead, |
||||
dashboards.ActionDashboardsCreate, |
||||
expectedResult: 20, |
||||
}, |
||||
{ |
||||
desc: "Should be able to view all folders with folder wildcard", |
||||
permission: models.PERMISSION_VIEW, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:*"}, |
||||
}, |
||||
expectedResult: 10, |
||||
}, |
||||
{ |
||||
queryType: randomType, |
||||
permission: models.PERMISSION_EDIT, |
||||
expectedDashboardActions: []string{ |
||||
dashboards.ActionDashboardsRead, |
||||
dashboards.ActionDashboardsWrite, |
||||
desc: "Should be able to view a subset folders", |
||||
permission: models.PERMISSION_VIEW, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:3"}, |
||||
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:6"}, |
||||
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:9"}, |
||||
}, |
||||
expectedFolderActions: []string{ |
||||
dashboards.ActionFoldersRead, |
||||
dashboards.ActionDashboardsCreate, |
||||
expectedResult: 3, |
||||
}, |
||||
{ |
||||
desc: "Should return folders and dashboard with 'edit' permission", |
||||
permission: models.PERMISSION_EDIT, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:3"}, |
||||
{Action: dashboards.ActionDashboardsCreate, Scope: "folders:uid:3"}, |
||||
{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:33"}, |
||||
{Action: dashboards.ActionDashboardsWrite, Scope: "dashboards:uid:33"}, |
||||
}, |
||||
expectedResult: 2, |
||||
}, |
||||
{ |
||||
queryType: randomType, |
||||
desc: "Should return folders that users can read alerts from", |
||||
permission: models.PERMISSION_VIEW, |
||||
expectedDashboardActions: []string{ |
||||
dashboards.ActionDashboardsRead, |
||||
queryType: searchstore.TypeAlertFolder, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:3"}, |
||||
{Action: accesscontrol.ActionAlertingRuleRead, Scope: "folders:uid:3"}, |
||||
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:8"}, |
||||
{Action: accesscontrol.ActionAlertingRuleRead, Scope: "folders:uid:8"}, |
||||
}, |
||||
expectedFolderActions: []string{ |
||||
dashboards.ActionFoldersRead, |
||||
expectedResult: 2, |
||||
}, |
||||
{ |
||||
desc: "Should return folders that users can read alerts when user has read wildcard", |
||||
permission: models.PERMISSION_VIEW, |
||||
queryType: searchstore.TypeAlertFolder, |
||||
permissions: []accesscontrol.Permission{ |
||||
{Action: dashboards.ActionFoldersRead, Scope: "*"}, |
||||
{Action: accesscontrol.ActionAlertingRuleRead, Scope: "folders:uid:3"}, |
||||
{Action: accesscontrol.ActionAlertingRuleRead, Scope: "folders:uid:8"}, |
||||
}, |
||||
expectedResult: 2, |
||||
}, |
||||
} |
||||
|
||||
for _, testCase := range testCases { |
||||
t.Run(fmt.Sprintf("query type %s, permissions %s", testCase.queryType, testCase.permission), func(t *testing.T) { |
||||
filters := NewAccessControlDashboardPermissionFilter(&user.SignedInUser{}, testCase.permission, testCase.queryType) |
||||
for _, tt := range tests { |
||||
t.Run(tt.desc, func(t *testing.T) { |
||||
store := setupTest(t, 10, 100, tt.permissions) |
||||
usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}} |
||||
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType) |
||||
|
||||
var result int |
||||
err := store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { |
||||
q, params := filter.Where() |
||||
_, err := sess.SQL("SELECT COUNT(*) FROM dashboard WHERE "+q, params...).Get(&result) |
||||
return err |
||||
}) |
||||
require.NoError(t, err) |
||||
|
||||
require.Equal(t, testCase.expectedDashboardActions, filters.dashboardActions) |
||||
require.Equal(t, testCase.expectedFolderActions, filters.folderActions) |
||||
assert.Equal(t, tt.expectedResult, result) |
||||
}) |
||||
} |
||||
} |
||||
|
||||
func TestAccessControlDashboardPermissionFilter_Where(t *testing.T) { |
||||
testCases := []struct { |
||||
title string |
||||
dashboardActions []string |
||||
folderActions []string |
||||
expectedResult string |
||||
}{ |
||||
{ |
||||
title: "folder and dashboard actions are defined", |
||||
dashboardActions: []string{"test"}, |
||||
folderActions: []string{"test"}, |
||||
expectedResult: "((( 1 = 0 OR dashboard.folder_id IN(SELECT id FROM dashboard WHERE 1 = 0)) AND NOT dashboard.is_folder) OR ( 1 = 0 AND dashboard.is_folder))", |
||||
}, |
||||
{ |
||||
title: "folder actions are defined but not dashboard actions", |
||||
dashboardActions: nil, |
||||
folderActions: []string{"test"}, |
||||
expectedResult: "(( 1 = 0 AND dashboard.is_folder))", |
||||
}, |
||||
{ |
||||
title: "dashboard actions are defined but not folder actions", |
||||
dashboardActions: []string{"test"}, |
||||
folderActions: nil, |
||||
expectedResult: "((( 1 = 0 OR dashboard.folder_id IN(SELECT id FROM dashboard WHERE 1 = 0)) AND NOT dashboard.is_folder))", |
||||
}, |
||||
{ |
||||
title: "dashboard actions are defined but not folder actions", |
||||
dashboardActions: nil, |
||||
folderActions: nil, |
||||
expectedResult: "()", |
||||
}, |
||||
} |
||||
|
||||
for _, testCase := range testCases { |
||||
t.Run(testCase.title, func(t *testing.T) { |
||||
filter := AccessControlDashboardPermissionFilter{ |
||||
User: &user.SignedInUser{Permissions: map[int64]map[string][]string{}}, |
||||
dashboardActions: testCase.dashboardActions, |
||||
folderActions: testCase.folderActions, |
||||
func setupTest(t *testing.T, numFolders, numDashboards int, permissions []accesscontrol.Permission) db.DB { |
||||
store := db.InitTestDB(t) |
||||
err := store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { |
||||
dashes := make([]models.Dashboard, 0, numFolders+numDashboards) |
||||
for i := 1; i <= numFolders; i++ { |
||||
str := strconv.Itoa(i) |
||||
dashes = append(dashes, models.Dashboard{ |
||||
OrgId: 1, |
||||
Slug: str, |
||||
Uid: str, |
||||
Title: str, |
||||
IsFolder: true, |
||||
Data: simplejson.New(), |
||||
Created: time.Now(), |
||||
Updated: time.Now(), |
||||
}) |
||||
} |
||||
// Seed 100 dashboard
|
||||
for i := numFolders + 1; i <= numFolders+numDashboards; i++ { |
||||
str := strconv.Itoa(i) |
||||
folderID := numFolders |
||||
if i%numFolders != 0 { |
||||
folderID = i % numFolders |
||||
} |
||||
dashes = append(dashes, models.Dashboard{ |
||||
OrgId: 1, |
||||
IsFolder: false, |
||||
FolderId: int64(folderID), |
||||
Uid: str, |
||||
Slug: str, |
||||
Title: str, |
||||
Data: simplejson.New(), |
||||
Created: time.Now(), |
||||
Updated: time.Now(), |
||||
}) |
||||
} |
||||
|
||||
query, args := filter.Where() |
||||
_, err := sess.InsertMulti(&dashes) |
||||
if err != nil { |
||||
return err |
||||
} |
||||
|
||||
assert.Empty(t, args) |
||||
assert.Equal(t, testCase.expectedResult, query) |
||||
role := &accesscontrol.Role{ |
||||
OrgID: 0, |
||||
UID: "basic_viewer", |
||||
Name: "basic:viewer", |
||||
Updated: time.Now(), |
||||
Created: time.Now(), |
||||
} |
||||
_, err = sess.Insert(role) |
||||
if err != nil { |
||||
return err |
||||
} |
||||
|
||||
_, err = sess.Insert(accesscontrol.BuiltinRole{ |
||||
OrgID: 0, |
||||
RoleID: role.ID, |
||||
Role: "Viewer", |
||||
Created: time.Now(), |
||||
Updated: time.Now(), |
||||
}) |
||||
} |
||||
if err != nil { |
||||
return err |
||||
} |
||||
|
||||
for i := range permissions { |
||||
permissions[i].RoleID = role.ID |
||||
permissions[i].Created = time.Now() |
||||
permissions[i].Updated = time.Now() |
||||
} |
||||
if len(permissions) > 0 { |
||||
_, err = sess.InsertMulti(&permissions) |
||||
if err != nil { |
||||
return err |
||||
} |
||||
} |
||||
|
||||
return nil |
||||
}) |
||||
require.NoError(t, err) |
||||
return store |
||||
} |
||||
|
@ -0,0 +1,143 @@ |
||||
package permissions_test |
||||
|
||||
import ( |
||||
"context" |
||||
"fmt" |
||||
"strconv" |
||||
"testing" |
||||
"time" |
||||
|
||||
"github.com/grafana/grafana/pkg/components/simplejson" |
||||
"github.com/grafana/grafana/pkg/infra/db" |
||||
"github.com/grafana/grafana/pkg/models" |
||||
"github.com/grafana/grafana/pkg/services/accesscontrol" |
||||
"github.com/grafana/grafana/pkg/services/dashboards" |
||||
"github.com/grafana/grafana/pkg/services/org" |
||||
"github.com/grafana/grafana/pkg/services/sqlstore" |
||||
"github.com/grafana/grafana/pkg/services/sqlstore/permissions" |
||||
"github.com/grafana/grafana/pkg/services/user" |
||||
"github.com/stretchr/testify/assert" |
||||
"github.com/stretchr/testify/require" |
||||
) |
||||
|
||||
func benchmarkDashboardPermissionFilter(b *testing.B, numUsers, numDashboards int) { |
||||
store := setupBenchMark(b, numUsers, numDashboards) |
||||
b.ResetTimer() |
||||
for i := 0; i < b.N; i++ { |
||||
usr := &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{1: {}}} |
||||
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, models.PERMISSION_VIEW, "") |
||||
var result int |
||||
err := store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { |
||||
q, params := filter.Where() |
||||
_, err := sess.SQL("SELECT COUNT(*) FROM dashboard WHERE "+q, params...).Get(&result) |
||||
return err |
||||
}) |
||||
require.NoError(b, err) |
||||
assert.Equal(b, numDashboards, result) |
||||
} |
||||
} |
||||
|
||||
func setupBenchMark(b *testing.B, numUsers, numDashboards int) db.DB { |
||||
store := db.InitTestDB(b) |
||||
now := time.Now() |
||||
err := store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { |
||||
dashes := make([]models.Dashboard, 0, numDashboards) |
||||
for i := 1; i <= numDashboards; i++ { |
||||
str := strconv.Itoa(i) |
||||
dashes = append(dashes, models.Dashboard{ |
||||
OrgId: 1, |
||||
IsFolder: false, |
||||
Uid: str, |
||||
Slug: str, |
||||
Title: str, |
||||
Data: simplejson.New(), |
||||
Created: now, |
||||
Updated: now, |
||||
}) |
||||
} |
||||
|
||||
err := batch(len(dashes), 1000, func(start, end int) error { |
||||
_, err := sess.InsertMulti(dashes[start:end]) |
||||
return err |
||||
}) |
||||
require.NoError(b, err) |
||||
|
||||
roles := make([]accesscontrol.Role, 0, numUsers) |
||||
assignments := make([]accesscontrol.UserRole, 0, numUsers) |
||||
permissions := make([]accesscontrol.Permission, 0, numUsers*numDashboards) |
||||
for i := 1; i <= numUsers; i++ { |
||||
name := fmt.Sprintf("managed_%d", i) |
||||
roles = append(roles, accesscontrol.Role{ |
||||
UID: name, |
||||
Name: name, |
||||
Updated: now, |
||||
Created: now, |
||||
}) |
||||
assignments = append(assignments, accesscontrol.UserRole{ |
||||
RoleID: int64(i), |
||||
UserID: int64(i), |
||||
Created: now, |
||||
}) |
||||
for _, dash := range dashes { |
||||
permissions = append(permissions, accesscontrol.Permission{ |
||||
RoleID: int64(i), |
||||
Action: dashboards.ActionDashboardsRead, |
||||
Scope: dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dash.Uid), |
||||
Updated: now, |
||||
Created: now, |
||||
}) |
||||
} |
||||
} |
||||
|
||||
err = batch(len(roles), 5000, func(start, end int) error { |
||||
_, err := sess.InsertMulti(roles[start:end]) |
||||
return err |
||||
}) |
||||
require.NoError(b, err) |
||||
|
||||
err = batch(len(assignments), 5000, func(start, end int) error { |
||||
_, err := sess.InsertMulti(assignments[start:end]) |
||||
return err |
||||
}) |
||||
require.NoError(b, err) |
||||
|
||||
err = batch(len(permissions), 5000, func(start, end int) error { |
||||
_, err := sess.InsertMulti(permissions[start:end]) |
||||
return err |
||||
}) |
||||
require.NoError(b, err) |
||||
return nil |
||||
}) |
||||
|
||||
require.NoError(b, err) |
||||
return store |
||||
} |
||||
|
||||
func BenchmarkDashboardPermissionFilter_100_100(b *testing.B) { |
||||
benchmarkDashboardPermissionFilter(b, 100, 100) |
||||
} |
||||
|
||||
func BenchmarkDashboardPermissionFilter_100_1000(b *testing.B) { |
||||
benchmarkDashboardPermissionFilter(b, 100, 1000) |
||||
} |
||||
|
||||
func BenchmarkDashboardPermissionFilter_300_10000(b *testing.B) { |
||||
benchmarkDashboardPermissionFilter(b, 300, 10000) |
||||
} |
||||
|
||||
func batch(count, batchSize int, eachFn func(start, end int) error) error { |
||||
for i := 0; i < count; { |
||||
end := i + batchSize |
||||
if end > count { |
||||
end = count |
||||
} |
||||
|
||||
if err := eachFn(i, end); err != nil { |
||||
return err |
||||
} |
||||
|
||||
i = end |
||||
} |
||||
|
||||
return nil |
||||
} |
Loading…
Reference in new issue