public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Access control: Add access control sql filter to org user queries (#43961)

Browse Source 
* Add access control SQL filter to org user queries
This commit is contained in:
Karl Persson
2022-01-13 14:40:32 +01:00
committed by GitHub
parent ccd9e46dda
commit f999910dc6
11 changed files with 266 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/accesscontrol/filter.go
+12 -1
View File
@@ -10,7 +10,9 @@ import (
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
)
var sqlIDAcceptList = map[string]struct{}{}
var sqlIDAcceptList = map[string]struct{}{
"org_user.user_id": {},
}
type SQLDialect interface {
DriverName() string
@@ -97,3 +99,12 @@ func postgresQuery(scopes []string, sqlID, prefix string) (string, []interface{}
)
`, sqlID, sqlID), args
}
// SetAcceptListForTest allow us to mutate the list for blackbox testing
func SetAcceptListForTest(list map[string]struct{}) func() {
original := sqlIDAcceptList
sqlIDAcceptList = list
return func() {
sqlIDAcceptList = original
}
}
pkg/services/accesscontrol/filter_bench_test.go
+11 -9
View File
@@ -1,4 +1,4 @@
package accesscontrol
package accesscontrol_test
import (
"context"
@@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/sqlstore"
)
@@ -21,19 +22,20 @@ func benchmarkFilter(b *testing.B, numDs, numPermissions int) {
b.ResetTimer()
// set sqlIDAcceptList before running tests
sqlIDAcceptList = map[string]struct{}{
restore := accesscontrol.SetAcceptListForTest(map[string]struct{}{
"data_source.id": {},
}
})
defer restore()
for i := 0; i < b.N; i++ {
baseSql := `SELECT data_source.* FROM data_source WHERE`
query, args, err := Filter(
query, args, err := accesscontrol.Filter(
context.Background(),
&FakeDriver{name: "sqlite3"},
"data_source.id",
"datasources",
"datasources:read",
&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: GroupScopesByAction(permissions)}},
&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(permissions)}},
)
require.NoError(b, err)
@@ -46,7 +48,7 @@ func benchmarkFilter(b *testing.B, numDs, numPermissions int) {
}
}
func setupFilterBenchmark(b *testing.B, numDs, numPermissions int) (*sqlstore.SQLStore, []*Permission) {
func setupFilterBenchmark(b *testing.B, numDs, numPermissions int) (*sqlstore.SQLStore, []*accesscontrol.Permission) {
b.Helper()
store := sqlstore.InitTestDB(b)
@@ -62,11 +64,11 @@ func setupFilterBenchmark(b *testing.B, numDs, numPermissions int) (*sqlstore.SQ
numPermissions = numDs
}
permissions := make([]*Permission, 0, numPermissions)
permissions := make([]*accesscontrol.Permission, 0, numPermissions)
for i := 1; i <= numPermissions; i++ {
permissions = append(permissions, &Permission{
permissions = append(permissions, &accesscontrol.Permission{
Action: "datasources:read",
Scope: Scope("datasources", "id", strconv.Itoa(i)),
Scope: accesscontrol.Scope("datasources", "id", strconv.Itoa(i)),
})
}
pkg/services/accesscontrol/filter_test.go
+21 -18
View File
@@ -1,4 +1,4 @@
package accesscontrol
package accesscontrol_test
import (
"context"
@@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/sqlstore"
)
@@ -18,7 +19,7 @@ type filterTest struct {
sqlID string
action string
prefix string
permissions []*Permission
permissions []*accesscontrol.Permission
expectedQuery string
expectedArgs []interface{}
}
@@ -31,7 +32,7 @@ func TestFilter(t *testing.T) {
sqlID: "data_source.id",
prefix: "datasources",
action: "datasources:query",
permissions: []*Permission{
permissions: []*accesscontrol.Permission{
{Action: "datasources:query", Scope: "datasources:id:1"},
{Action: "datasources:query", Scope: "datasources:id:2"},
{Action: "datasources:query", Scope: "datasources:id:3"},
@@ -65,7 +66,7 @@ func TestFilter(t *testing.T) {
sqlID: "dashboard.id",
prefix: "dashboards",
action: "dashboards:read",
permissions: []*Permission{
permissions: []*accesscontrol.Permission{
{Action: "dashboards:read", Scope: "dashboards:id:1"},
{Action: "dashboards:read", Scope: "dashboards:id:2"},
{Action: "dashboards:read", Scope: "dashboards:id:5"},
@@ -95,7 +96,7 @@ func TestFilter(t *testing.T) {
sqlID: "user.id",
prefix: "users",
action: "users:read",
permissions: []*Permission{
permissions: []*accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1"},
{Action: "users:read", Scope: "users:id:100"},
// Other permissions
@@ -123,21 +124,22 @@ func TestFilter(t *testing.T) {
}
// set sqlIDAcceptList before running tests
sqlIDAcceptList = map[string]struct{}{
restore := accesscontrol.SetAcceptListForTest(map[string]struct{}{
"user.id": {},
"dashboard.id": {},
"data_source.id": {},
}
})
defer restore()
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
query, args, err := Filter(
query, args, err := accesscontrol.Filter(
context.Background(),
FakeDriver{name: tt.driverName},
tt.sqlID,
tt.prefix,
tt.action,
&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: GroupScopesByAction(tt.permissions)}},
&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}},
)
require.NoError(t, err)
assert.Equal(t, tt.expectedQuery, query)
@@ -153,7 +155,7 @@ func TestFilter(t *testing.T) {
type filterDatasourcesTestCase struct {
desc string
sqlID string
permissions []*Permission
permissions []*accesscontrol.Permission
expectedDataSources []string
expectErr bool
}
@@ -163,7 +165,7 @@ func TestFilter_Datasources(t *testing.T) {
{
desc: "expect all data sources to be returned",
sqlID: "data_source.id",
permissions: []*Permission{
permissions: []*accesscontrol.Permission{
{Action: "datasources:read", Scope: "datasources:*"},
},
expectedDataSources: []string{"ds:1", "ds:2", "ds:3", "ds:4", "ds:5", "ds:6", "ds:7", "ds:8", "ds:9", "ds:10"},
@@ -171,13 +173,13 @@ func TestFilter_Datasources(t *testing.T) {
{
desc: "expect no data sources to be returned",
sqlID: "data_source.id",
permissions: []*Permission{},
permissions: []*accesscontrol.Permission{},
expectedDataSources: []string{},
},
{
desc: "expect data sources with id 3, 7 and 8 to be returned",
sqlID: "data_source.id",
permissions: []*Permission{
permissions: []*accesscontrol.Permission{
{Action: "datasources:read", Scope: "datasources:id:3"},
{Action: "datasources:read", Scope: "datasources:id:7"},
{Action: "datasources:read", Scope: "datasources:id:8"},
@@ -187,7 +189,7 @@ func TestFilter_Datasources(t *testing.T) {
{
desc: "expect error if sqlID is not in the accept list",
sqlID: "other.id",
permissions: []*Permission{
permissions: []*accesscontrol.Permission{
{Action: "datasources:read", Scope: "datasources:id:3"},
{Action: "datasources:read", Scope: "datasources:id:7"},
{Action: "datasources:read", Scope: "datasources:id:8"},
@@ -198,9 +200,10 @@ func TestFilter_Datasources(t *testing.T) {
}
// set sqlIDAcceptList before running tests
sqlIDAcceptList = map[string]struct{}{
restore := accesscontrol.SetAcceptListForTest(map[string]struct{}{
"data_source.id": {},
}
})
defer restore()
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
@@ -216,13 +219,13 @@ func TestFilter_Datasources(t *testing.T) {
}
baseSql := `SELECT data_source.* FROM data_source WHERE`
query, args, err := Filter(
query, args, err := accesscontrol.Filter(
context.Background(),
&FakeDriver{name: "sqlite3"},
tt.sqlID,
"datasources",
"datasources:read",
&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: GroupScopesByAction(tt.permissions)}},
&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}},
)
if !tt.expectErr {
pkg/services/sqlstore/org_users.go
+19
View File
@@ -8,6 +8,7 @@ import (
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/util"
)
@@ -111,6 +112,15 @@ func (ss *SQLStore) GetOrgUsers(ctx context.Context, query *models.GetOrgUsersQu
// service accounts table in the modelling
whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %t", x.Dialect().Quote("user"), query.IsServiceAccount))
if ss.Cfg.FeatureToggles["accesscontrol"] {
q, args, err := accesscontrol.Filter(ctx, ss.Dialect, "org_user.user_id", "users", "org.users:read", query.User)
if err != nil {
return err
}
whereConditions = append(whereConditions, q)
whereParams = append(whereParams, args...)
}
if query.Query != "" {
queryWithWildcards := "%" + query.Query + "%"
whereConditions = append(whereConditions, "(email "+dialect.LikeStr()+" ? OR name "+dialect.LikeStr()+" ? OR login "+dialect.LikeStr()+" ?)")
@@ -165,6 +175,15 @@ func (ss *SQLStore) SearchOrgUsers(ctx context.Context, query *models.SearchOrgU
// service accounts table in the modelling
whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %t", x.Dialect().Quote("user"), query.IsServiceAccount))
if ss.Cfg.FeatureToggles["accesscontrol"] {
q, args, err := accesscontrol.Filter(ctx, ss.Dialect, "org_user.user_id", "users", "org.users:read", query.User)
if err != nil {
return err
}
whereConditions = append(whereConditions, q)
whereParams = append(whereParams, args...)
}
if query.Query != "" {
queryWithWildcards := "%" + query.Query + "%"
whereConditions = append(whereConditions, "(email "+dialect.LikeStr()+" ? OR name "+dialect.LikeStr()+" ? OR login "+dialect.LikeStr()+" ?)")
pkg/services/sqlstore/org_users_test.go
+176
View File
@@ -0,0 +1,176 @@
package sqlstore
import (
"context"
"fmt"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/models"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
)
type getOrgUsersTestCase struct {
desc string
query *models.GetOrgUsersQuery
expectedNumUsers int
}
func TestSQLStore_GetOrgUsers(t *testing.T) {
tests := []getOrgUsersTestCase{
{
desc: "should return all users",
query: &models.GetOrgUsersQuery{
OrgId: 1,
User: &models.SignedInUser{
OrgId: 1,
Permissions: map[int64]map[string][]string{1: {ac.ActionOrgUsersRead: {ac.ScopeUsersAll}}},
},
},
expectedNumUsers: 10,
},
{
desc: "should return no users",
query: &models.GetOrgUsersQuery{
OrgId: 1,
User: &models.SignedInUser{
OrgId: 1,
Permissions: map[int64]map[string][]string{1: {ac.ActionOrgUsersRead: {""}}},
},
},
expectedNumUsers: 0,
},
{
desc: "should return some users",
query: &models.GetOrgUsersQuery{
OrgId: 1,
User: &models.SignedInUser{
OrgId: 1,
Permissions: map[int64]map[string][]string{1: {ac.ActionOrgUsersRead: {
"users:id:1",
"users:id:5",
"users:id:9",
}}},
},
},
expectedNumUsers: 3,
},
}
store := InitTestDB(t)
store.Cfg.FeatureToggles = map[string]bool{"accesscontrol": true}
seedOrgUsers(t, store, 10)
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
err := store.GetOrgUsers(context.Background(), tt.query)
require.NoError(t, err)
require.Len(t, tt.query.Result, tt.expectedNumUsers)
if !hasWildcardScope(tt.query.User, ac.ActionOrgUsersRead) {
for _, u := range tt.query.Result {
assert.Contains(t, tt.query.User.Permissions[tt.query.User.OrgId][ac.ActionOrgUsersRead], fmt.Sprintf("users:id:%d", u.UserId))
}
}
})
}
}
type searchOrgUsersTestCase struct {
desc string
query *models.SearchOrgUsersQuery
expectedNumUsers int
}
func TestSQLStore_SearchOrgUsers(t *testing.T) {
tests := []searchOrgUsersTestCase{
{
desc: "should return all users",
query: &models.SearchOrgUsersQuery{
OrgID: 1,
User: &models.SignedInUser{
OrgId: 1,
Permissions: map[int64]map[string][]string{1: {ac.ActionOrgUsersRead: {ac.ScopeUsersAll}}},
},
},
expectedNumUsers: 10,
},
{
desc: "should return no users",
query: &models.SearchOrgUsersQuery{
OrgID: 1,
User: &models.SignedInUser{
OrgId: 1,
Permissions: map[int64]map[string][]string{1: {ac.ActionOrgUsersRead: {""}}},
},
},
expectedNumUsers: 0,
},
{
desc: "should return some users",
query: &models.SearchOrgUsersQuery{
OrgID: 1,
User: &models.SignedInUser{
OrgId: 1,
Permissions: map[int64]map[string][]string{1: {ac.ActionOrgUsersRead: {
"users:id:1",
"users:id:5",
"users:id:9",
}}},
},
},
expectedNumUsers: 3,
},
}
store := InitTestDB(t)
store.Cfg.FeatureToggles = map[string]bool{"accesscontrol": true}
seedOrgUsers(t, store, 10)
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
err := store.SearchOrgUsers(context.Background(), tt.query)
require.NoError(t, err)
assert.Len(t, tt.query.Result.OrgUsers, tt.expectedNumUsers)
if !hasWildcardScope(tt.query.User, ac.ActionOrgUsersRead) {
for _, u := range tt.query.Result.OrgUsers {
assert.Contains(t, tt.query.User.Permissions[tt.query.User.OrgId][ac.ActionOrgUsersRead], fmt.Sprintf("users:id:%d", u.UserId))
}
}
})
}
}
func seedOrgUsers(t *testing.T, store *SQLStore, numUsers int) {
t.Helper()
// Seed users
for i := 1; i <= numUsers; i++ {
user, err := store.CreateUser(context.Background(), models.CreateUserCommand{
Login: fmt.Sprintf("user-%d", i),
OrgId: 1,
})
require.NoError(t, err)
if i != 1 {
err = store.AddOrgUser(context.Background(), &models.AddOrgUserCommand{
Role: "Viewer",
OrgId: 1,
UserId: user.Id,
})
require.NoError(t, err)
}
}
}
func hasWildcardScope(user *models.SignedInUser, action string) bool {
for _, scope := range user.Permissions[user.OrgId][action] {
if strings.HasSuffix(scope, ":*") {
return true
}
}
return false
}