Access control: Support filter on several actions (#46524)

* Add support for several actions when creating a acccess control sql
filter

pkg/services/accesscontrol/database/resource_permissions.go

@@ -361,14 +361,14 @@ func (s *AccessControlStore) getResourcesPermissions(sess *sqlstore.DBSession, o
 
 	initialLength := len(args)
 
-	userFilter, err := accesscontrol.Filter(context.Background(), "u.id", "users", accesscontrol.ActionOrgUsersRead, query.User)
+	userFilter, err := accesscontrol.Filter(query.User, "u.id", "users", accesscontrol.ActionOrgUsersRead)
 	if err != nil {
 		return nil, err
 	}
 	user := userSelect + userFrom + where + " AND " + userFilter.Where
 	args = append(args, userFilter.Args...)
 
-	teamFilter, err := accesscontrol.Filter(context.Background(), "t.id", "teams", accesscontrol.ActionTeamsRead, query.User)
+	teamFilter, err := accesscontrol.Filter(query.User, "t.id", "teams", accesscontrol.ActionTeamsRead)
 	if err != nil {
 		return nil, err
 	}

pkg/services/accesscontrol/filter.go

@@ -1,7 +1,6 @@
 package accesscontrol
 
 import (
-	"context"
 	"errors"
 	"strconv"
 	"strings"
@@ -26,10 +25,14 @@ var (
 	allowAllQuery = SQLFilter{" 1 = 1", nil}
 )
 
+type SQLFilter struct {
+	Where string
+	Args  []interface{}
+}
+
 // Filter creates a where clause to restrict the view of a query based on a users permissions
-// Scopes for a certain action will be compared against prefix:id:sqlID where prefix is the scope prefix and sqlID
+// Scopes that exists for all actions will be parsed and compared against the supplied sqlID
-// is the id to generate scope from e.g. user.id
+func Filter(user *models.SignedInUser, sqlID, prefix string, actions ...string) (SQLFilter, error) {
-func Filter(ctx context.Context, sqlID, prefix, action string, user *models.SignedInUser) (SQLFilter, error) {
 	if _, ok := sqlIDAcceptList[sqlID]; !ok {
 		return denyQuery, errors.New("sqlID is not in the accept list")
 	}
@@ -37,26 +40,33 @@ func Filter(ctx context.Context, sqlID, prefix, action string, user *models.Sign
 		return denyQuery, errors.New("missing permissions")
 	}
 
-	var hasWildcard bool
+	wildcards := 0
-	var ids []interface{}
+	result := make(map[int64]int)
-	for _, scope := range user.Permissions[user.OrgId][action] {
+	for _, a := range actions {
-		if strings.HasPrefix(scope, prefix) || scope == "*" {
+		ids, hasWildcard := parseScopes(prefix, user.Permissions[user.OrgId][a])
-			if id := strings.TrimPrefix(scope, prefix); id == "*" || id == ":*" || id == ":id:*" {
+		if hasWildcard {
-				hasWildcard = true
+			wildcards += 1
-				break
+			continue
-			}
+		}
-			if id, err := parseScopeID(scope); err == nil {
+		if len(ids) == 0 {
-				ids = append(ids, id)
+			return denyQuery, nil
-			}
+		}
+		for _, id := range ids {
+			result[id] += 1
 		}
 	}
 
-	if hasWildcard {
+	// return early if every action has wildcard scope
+	if wildcards == len(actions) {
 		return allowAllQuery, nil
 	}
 
-	if len(ids) == 0 {
+	var ids []interface{}
-		return denyQuery, nil
+	for id, count := range result {
+		// if an id exist for every action include it in the filter
+		if count+wildcards == len(actions) {
+			ids = append(ids, id)
+		}
 	}
 
 	query := strings.Builder{}
@@ -70,6 +80,20 @@ func Filter(ctx context.Context, sqlID, prefix, action string, user *models.Sign
 	return SQLFilter{query.String(), ids}, nil
 }
 
+func parseScopes(prefix string, scopes []string) (ids []int64, hasWildcard bool) {
+	for _, scope := range scopes {
+		if strings.HasPrefix(scope, prefix) || scope == "*" {
+			if id := strings.TrimPrefix(scope, prefix); id == "*" || id == ":*" || id == ":id:*" {
+				return nil, true
+			}
+			if id, err := parseScopeID(scope); err == nil {
+				ids = append(ids, id)
+			}
+		}
+	}
+	return ids, false
+}
+
 func parseScopeID(scope string) (int64, error) {
 	return strconv.ParseInt(scope[strings.LastIndex(scope, ":")+1:], 10, 64)
 }

pkg/services/accesscontrol/filter_bench_test.go

@@ -32,11 +32,10 @@ func benchmarkFilter(b *testing.B, numDs, numPermissions int) {
 	for i := 0; i < b.N; i++ {
 		baseSql := `SELECT data_source.* FROM data_source WHERE`
 		acFilter, err := accesscontrol.Filter(
-			context.Background(),
+			&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(permissions)}},
 			"data_source.id",
 			"datasources",
 			"datasources:read",
-			&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(permissions)}},
 		)
 		require.NoError(b, err)
 

pkg/services/accesscontrol/filter_test.go

@@ -14,9 +14,12 @@ import (
 )
 
 type filterDatasourcesTestCase struct {
-	desc                string
+	desc        string
-	sqlID               string
+	sqlID       string
-	permissions         []*accesscontrol.Permission
+	prefix      string
+	actions     []string
+	permissions map[string][]string
+
 	expectedDataSources []string
 	expectErr           bool
 }
@@ -24,63 +27,95 @@ type filterDatasourcesTestCase struct {
 func TestFilter_Datasources(t *testing.T) {
 	tests := []filterDatasourcesTestCase{
 		{
-			desc:  "expect all data sources to be returned",
+			desc:    "expect all data sources to be returned",
-			sqlID: "data_source.id",
+			sqlID:   "data_source.id",
-			permissions: []*accesscontrol.Permission{
+			prefix:  "datasources",
-				{Action: "datasources:read", Scope: "datasources:*"},
+			actions: []string{"datasources:read"},
+			permissions: map[string][]string{
+				"datasources:read": {"datasources:*"},
 			},
 			expectedDataSources: []string{"ds:1", "ds:2", "ds:3", "ds:4", "ds:5", "ds:6", "ds:7", "ds:8", "ds:9", "ds:10"},
 		},
 		{
-			desc:  "expect all data sources for wildcard id scope to be returned",
+			desc:    "expect all data sources for wildcard id scope to be returned",
-			sqlID: "data_source.id",
+			sqlID:   "data_source.id",
-			permissions: []*accesscontrol.Permission{
+			prefix:  "datasources",
-				{Action: "datasources:read", Scope: "datasources:id:*"},
+			actions: []string{"datasources:read"},
+			permissions: map[string][]string{
+				"datasources:read": {"datasources:id:*"},
 			},
 			expectedDataSources: []string{"ds:1", "ds:2", "ds:3", "ds:4", "ds:5", "ds:6", "ds:7", "ds:8", "ds:9", "ds:10"},
 		},
 		{
-			desc:  "expect all data sources for wildcard scope to be returned",
+			desc:    "expect all data sources for wildcard scope to be returned",
-			sqlID: "data_source.id",
+			sqlID:   "data_source.id",
-			permissions: []*accesscontrol.Permission{
+			prefix:  "datasources",
-				{Action: "datasources:read", Scope: "*"},
+			actions: []string{"datasources:read"},
+			permissions: map[string][]string{
+				"datasources:read": {"*"},
 			},
 			expectedDataSources: []string{"ds:1", "ds:2", "ds:3", "ds:4", "ds:5", "ds:6", "ds:7", "ds:8", "ds:9", "ds:10"},
 		},
 		{
 			desc:                "expect no data sources to be returned",
 			sqlID:               "data_source.id",
-			permissions:         []*accesscontrol.Permission{},
+			prefix:              "datasources",
+			actions:             []string{"datasources:read"},
+			permissions:         map[string][]string{},
 			expectedDataSources: []string{},
 		},
 		{
-			desc:  "expect data sources with id 3, 7 and 8 to be returned",
+			desc:    "expect data sources with id 3, 7 and 8 to be returned",
-			sqlID: "data_source.id",
+			sqlID:   "data_source.id",
-			permissions: []*accesscontrol.Permission{
+			prefix:  "datasources",
-				{Action: "datasources:read", Scope: "datasources:id:3"},
+			actions: []string{"datasources:read"},
-				{Action: "datasources:read", Scope: "datasources:id:7"},
+			permissions: map[string][]string{
-				{Action: "datasources:read", Scope: "datasources:id:8"},
+				"datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"},
 			},
 			expectedDataSources: []string{"ds:3", "ds:7", "ds:8"},
 		},
 		{
-			desc:  "expect no data sources to be returned for malformed scope",
+			desc:    "expect no data sources to be returned for malformed scope",
-			sqlID: "data_source.id",
+			sqlID:   "data_source.id",
-			permissions: []*accesscontrol.Permission{
+			prefix:  "datasources",
-				{Action: "datasources:read", Scope: "datasources:id:1*"},
+			actions: []string{"datasources:read"},
+			permissions: map[string][]string{
+				"datasources:read": {"datasources:id:1*"},
 			},
-			expectedDataSources: []string{},
 		},
 		{
-			desc:  "expect error if sqlID is not in the accept list",
+			desc:    "expect error if sqlID is not in the accept list",
-			sqlID: "other.id",
+			sqlID:   "other.id",
-			permissions: []*accesscontrol.Permission{
+			prefix:  "datasources",
-				{Action: "datasources:read", Scope: "datasources:id:3"},
+			actions: []string{"datasources:read"},
-				{Action: "datasources:read", Scope: "datasources:id:7"},
+			permissions: map[string][]string{
-				{Action: "datasources:read", Scope: "datasources:id:8"},
+				"datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"},
+			},
+			expectErr: true,
+		},
+		{
+			desc:    "expect data sources that users has several actions for",
+			sqlID:   "data_source.id",
+			prefix:  "datasources",
+			actions: []string{"datasources:read", "datasources:write"},
+			permissions: map[string][]string{
+				"datasources:read":  {"datasources:id:3", "datasources:id:7", "datasources:id:8"},
+				"datasources:write": {"datasources:id:3", "datasources:id:8"},
+			},
+			expectedDataSources: []string{"ds:3", "ds:8"},
+			expectErr:           false,
+		},
+		{
+			desc:    "expect data sources that users has several actions for",
+			sqlID:   "data_source.id",
+			prefix:  "datasources",
+			actions: []string{"datasources:read", "datasources:write"},
+			permissions: map[string][]string{
+				"datasources:read":  {"datasources:id:3", "datasources:id:7", "datasources:id:8"},
+				"datasources:write": {"datasources:*", "datasources:id:8"},
 			},
 			expectedDataSources: []string{"ds:3", "ds:7", "ds:8"},
-			expectErr:           true,
+			expectErr:           false,
 		},
 	}
 
@@ -105,11 +140,13 @@ func TestFilter_Datasources(t *testing.T) {
 
 			baseSql := `SELECT data_source.* FROM data_source WHERE`
 			acFilter, err := accesscontrol.Filter(
-				context.Background(),
+				&models.SignedInUser{
+					OrgId:       1,
+					Permissions: map[int64]map[string][]string{1: tt.permissions},
+				},
 				tt.sqlID,
-				"datasources",
+				tt.prefix,
-				"datasources:read",
+				tt.actions...,
-				&models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}},
 			)
 
 			if !tt.expectErr {

pkg/services/accesscontrol/models.go

@@ -241,11 +241,6 @@ type SetResourcePermissionCommand struct {
 	Permission  string
 }
 
-type SQLFilter struct {
-	Where string
-	Args  []interface{}
-}
-
 const (
 	GlobalOrgID = 0
 	// Permission actions

pkg/services/serviceaccounts/database/database.go

@@ -317,7 +317,7 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts(ctx context.Context,
 				s.sqlStore.Dialect.BooleanStr(true)))
 
 		if s.sqlStore.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) {
-			acFilter, err := accesscontrol.Filter(ctx, "org_user.user_id", "serviceaccounts", "serviceaccounts:read", query.User)
+			acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "serviceaccounts", serviceaccounts.ActionRead)
 			if err != nil {
 				return err
 			}

pkg/services/sqlstore/org_users.go

@@ -119,7 +119,7 @@ func (ss *SQLStore) GetOrgUsers(ctx context.Context, query *models.GetOrgUsersQu
 	whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %t", x.Dialect().Quote("user"), query.IsServiceAccount))
 
 	if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) && query.User != nil {
-		acFilter, err := accesscontrol.Filter(ctx, "org_user.user_id", "users", "org.users:read", query.User)
+		acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users", accesscontrol.ActionOrgUsersRead)
 		if err != nil {
 			return err
 		}
@@ -184,7 +184,7 @@ func (ss *SQLStore) SearchOrgUsers(ctx context.Context, query *models.SearchOrgU
 	whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %t", x.Dialect().Quote("user"), query.IsServiceAccount))
 
 	if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) {
-		acFilter, err := accesscontrol.Filter(ctx, "org_user.user_id", "users", "org.users:read", query.User)
+		acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users", accesscontrol.ActionOrgUsersRead)
 		if err != nil {
 			return err
 		}

pkg/services/sqlstore/permissions/dashboard.go

@@ -1,7 +1,6 @@
 package permissions
 
 import (
-	"context"
 	"strings"
 
 	"github.com/grafana/grafana/pkg/models"
@@ -83,30 +82,32 @@ type AccessControlDashboardPermissionFilter struct {
 }
 
 func (f AccessControlDashboardPermissionFilter) Where() (string, []interface{}) {
-	folderAction := dashboards.ActionFoldersRead
+	folderActions := []string{dashboards.ActionFoldersRead}
-	dashboardAction := accesscontrol.ActionDashboardsRead
+	dashboardActions := []string{accesscontrol.ActionDashboardsRead}
 	if f.PermissionLevel == models.PERMISSION_EDIT {
-		folderAction = accesscontrol.ActionDashboardsCreate
+		folderActions = append(folderActions, accesscontrol.ActionDashboardsCreate)
-		dashboardAction = accesscontrol.ActionDashboardsWrite
+		dashboardActions = append(dashboardActions, accesscontrol.ActionDashboardsWrite)
 	}
 
+	var args []interface{}
 	builder := strings.Builder{}
-
 	builder.WriteString("(((")
 
-	dashFilter, _ := accesscontrol.Filter(context.Background(), "dashboard.id", "dashboards", dashboardAction, f.User)
+	dashFilter, _ := accesscontrol.Filter(f.User, "dashboard.id", "dashboards", dashboardActions...)
 	builder.WriteString(dashFilter.Where)
+	args = append(args, dashFilter.Args...)
 
 	builder.WriteString(" OR ")
 
-	dashFolderFilter, _ := accesscontrol.Filter(context.Background(), "dashboard.folder_id", "folders", dashboardAction, f.User)
+	dashFolderFilter, _ := accesscontrol.Filter(f.User, "dashboard.folder_id", "folders", dashboardActions...)
 	builder.WriteString(dashFolderFilter.Where)
-
 	builder.WriteString(") AND NOT dashboard.is_folder) OR (")
+	args = append(args, dashFolderFilter.Args...)
 
-	folderFilter, _ := accesscontrol.Filter(context.Background(), "dashboard.id", "folders", folderAction, f.User)
+	folderFilter, _ := accesscontrol.Filter(f.User, "dashboard.id", "folders", folderActions...)
 	builder.WriteString(folderFilter.Where)
 	builder.WriteString(" AND dashboard.is_folder))")
+	args = append(args, folderFilter.Args...)
 
-	return builder.String(), append(dashFilter.Args, append(dashFolderFilter.Args, folderFilter.Args...)...)
+	return builder.String(), args
 }

pkg/services/sqlstore/team.go

@@ -229,7 +229,7 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu
 		err      error
 	)
 	if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) {
-		acFilter, err = ac.Filter(ctx, "team.id", "teams", ac.ActionTeamsRead, query.SignedInUser)
+		acFilter, err = ac.Filter(query.SignedInUser, "team.id", "teams", ac.ActionTeamsRead)
 		if err != nil {
 			return err
 		}
@@ -528,10 +528,8 @@ func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMem
 	// Note we assume that checking SignedInUser is allowed to see team members for this team has already been performed
 	// If the signed in user is not set no member will be returned
 	if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) {
-		*acFilter, err = ac.Filter(ctx,
+		sqlID := fmt.Sprintf("%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id"))
-			fmt.Sprintf("%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")),
+		*acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users", ac.ActionOrgUsersRead)
-			"users", ac.ActionOrgUsersRead, query.SignedInUser,
-		)
 		if err != nil {
 			return err
 		}