RBAC: Add and resolve action sets when searching user's permissions (#88694)

* include and resolve action sets when fetching user's permissions

* expand both action and action prefix (returns an empty set for the one that isn't specified)
Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* if action is specified, check for exact match; also extend tests
This commit is contained in:
Ieva 2024-06-12 11:20:19 +03:00 committed by GitHub
parent 12d5251c12
commit 34c40f959f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 189 additions and 11 deletions

View File

@ -76,6 +76,7 @@ type Options struct {
type SearchOptions struct {
ActionPrefix string // Needed for the PoC v1, it's probably going to be removed.
Action string
ActionSets []string
Scope string
NamespacedID string // ID of the identity (ex: user:3, service-account:4)
wildcards Wildcards // private field computed based on the Scope

View File

@ -422,7 +422,16 @@ func (s *Service) DeclarePluginRoles(ctx context.Context, ID, name string, regs
return nil
}
// TODO potential changes needed here?
func GetActionFilter(options accesscontrol.SearchOptions) func(action string) bool {
return func(action string) bool {
if options.ActionPrefix != "" {
return strings.HasPrefix(action, options.ActionPrefix)
} else {
return action == options.Action
}
}
}
// SearchUsersPermissions returns all users' permissions filtered by action prefixes
func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Requester,
options accesscontrol.SearchOptions) (map[int64][]accesscontrol.Permission, error) {
@ -437,7 +446,6 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Reque
// Reroute to the user specific implementation of search permissions
// because it leverages the user permission cache.
// TODO
userPerms, err := s.SearchUserPermissions(ctx, usr.GetOrgID(), options)
if err != nil {
return nil, err
@ -463,6 +471,12 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Reque
return nil, err
}
if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) {
options.ActionSets = s.actionResolver.ResolveAction(options.Action)
options.ActionSets = append(options.ActionSets,
s.actionResolver.ResolveActionPrefix(options.ActionPrefix)...)
}
// Get managed permissions (DB)
usersPermissions, err := s.store.SearchUsersPermissions(ctx, usr.GetOrgID(), options)
if err != nil {
@ -522,6 +536,12 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Reque
}
}
if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) && len(options.ActionSets) > 0 {
for id, perms := range res {
res[id] = s.actionResolver.ExpandActionSetsWithFilter(perms, GetActionFilter(options))
}
}
return res, nil
}
@ -566,6 +586,12 @@ func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, search
}
}
if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) {
searchOptions.ActionSets = s.actionResolver.ResolveAction(searchOptions.Action)
searchOptions.ActionSets = append(searchOptions.ActionSets,
s.actionResolver.ResolveActionPrefix(searchOptions.ActionPrefix)...)
}
// Get permissions from the DB
dbPermissions, err := s.store.SearchUsersPermissions(ctx, orgID, searchOptions)
if err != nil {
@ -573,6 +599,10 @@ func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, search
}
permissions = append(permissions, dbPermissions[userID]...)
if s.features.IsEnabled(ctx, featuremgmt.FlagAccessActionSets) && len(searchOptions.ActionSets) != 0 {
permissions = s.actionResolver.ExpandActionSetsWithFilter(permissions, GetActionFilter(searchOptions))
}
key := accesscontrol.GetSearchPermissionCacheKey(&user.SignedInUser{UserID: userID, OrgID: orgID}, searchOptions)
s.cache.Set(key, permissions, cacheTTL)

View File

@ -3,6 +3,7 @@ package acimpl
import (
"context"
"fmt"
"strings"
"testing"
"github.com/stretchr/testify/assert"
@ -599,13 +600,15 @@ func TestService_SearchUsersPermissions(t *testing.T) {
func TestService_SearchUserPermissions(t *testing.T) {
ctx := context.Background()
tests := []struct {
name string
searchOption accesscontrol.SearchOptions
ramRoles map[string]*accesscontrol.RoleDTO // BasicRole => RBAC BasicRole
storedPerms map[int64][]accesscontrol.Permission // UserID => Permissions
storedRoles map[int64][]string // UserID => Roles
want []accesscontrol.Permission
wantErr bool
name string
searchOption accesscontrol.SearchOptions
withActionSets bool
actionSets map[string][]string
ramRoles map[string]*accesscontrol.RoleDTO // BasicRole => RBAC BasicRole
storedPerms map[int64][]accesscontrol.Permission // UserID => Permissions
storedRoles map[int64][]string // UserID => Roles
want []accesscontrol.Permission
wantErr bool
}{
{
name: "ram only",
@ -726,10 +729,86 @@ func TestService_SearchUserPermissions(t *testing.T) {
{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
},
},
{
name: "check action sets are correctly included if an action is specified",
searchOption: accesscontrol.SearchOptions{
Action: "dashboards:read",
NamespacedID: fmt.Sprintf("%s:1", identity.NamespaceUser),
},
withActionSets: true,
actionSets: map[string][]string{
"dashboards:view": {"dashboards:read"},
"dashboards:edit": {"dashboards:read", "dashboards:write", "dashboards:read-advanced"},
},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{
{Action: "dashboards:read", Scope: "dashboards:uid:ram"},
}},
},
storedRoles: map[int64][]string{
1: {string(roletype.RoleEditor)},
},
storedPerms: map[int64][]accesscontrol.Permission{
1: {
{Action: "dashboards:read", Scope: "dashboards:uid:stored"},
{Action: "dashboards:edit", Scope: "dashboards:uid:stored2"},
{Action: "dashboards:view", Scope: "dashboards:uid:stored3"},
},
},
want: []accesscontrol.Permission{
{Action: "dashboards:read", Scope: "dashboards:uid:ram"},
{Action: "dashboards:read", Scope: "dashboards:uid:stored"},
{Action: "dashboards:read", Scope: "dashboards:uid:stored2"},
{Action: "dashboards:read", Scope: "dashboards:uid:stored3"},
},
},
{
name: "check action sets are correctly included if an action prefix is specified",
searchOption: accesscontrol.SearchOptions{
ActionPrefix: "dashboards",
NamespacedID: fmt.Sprintf("%s:1", identity.NamespaceUser),
},
withActionSets: true,
actionSets: map[string][]string{
"dashboards:view": {"dashboards:read"},
"folders:view": {"dashboards:read", "folders:read"},
"dashboards:edit": {"dashboards:read", "dashboards:write"},
},
ramRoles: map[string]*accesscontrol.RoleDTO{
string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{
{Action: "dashboards:read", Scope: "dashboards:uid:ram"},
}},
},
storedRoles: map[int64][]string{
1: {string(roletype.RoleEditor)},
},
storedPerms: map[int64][]accesscontrol.Permission{
1: {
{Action: "dashboards:read", Scope: "dashboards:uid:stored"},
{Action: "folders:view", Scope: "folders:uid:stored2"},
{Action: "dashboards:edit", Scope: "dashboards:uid:stored3"},
},
},
want: []accesscontrol.Permission{
{Action: "dashboards:read", Scope: "dashboards:uid:ram"},
{Action: "dashboards:read", Scope: "dashboards:uid:stored"},
{Action: "dashboards:read", Scope: "folders:uid:stored2"},
{Action: "dashboards:read", Scope: "dashboards:uid:stored3"},
{Action: "dashboards:write", Scope: "dashboards:uid:stored3"},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac := setupTestEnv(t)
if tt.withActionSets {
ac.features = featuremgmt.WithFeatures(featuremgmt.FlagAccessActionSets)
actionSetSvc := resourcepermissions.NewActionSetService()
for set, actions := range tt.actionSets {
actionSetSvc.StoreActionSet(strings.Split(set, ":")[0], strings.Split(set, ":")[1], actions)
}
ac.actionResolver = actionSetSvc
}
ac.roles = tt.ramRoles
ac.store = actest.FakeStore{

View File

@ -228,10 +228,24 @@ func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID i
if options.ActionPrefix != "" {
q += ` AND p.action LIKE ?`
params = append(params, options.ActionPrefix+"%")
if len(options.ActionSets) > 0 {
q += ` OR p.action IN ( ? ` + strings.Repeat(", ?", len(options.ActionSets)-1) + ")"
for _, a := range options.ActionSets {
params = append(params, a)
}
}
}
if options.Action != "" {
q += ` AND p.action = ?`
params = append(params, options.Action)
if len(options.ActionSets) == 0 {
q += ` AND p.action = ?`
params = append(params, options.Action)
} else {
actions := append(options.ActionSets, options.Action)
q += ` AND p.action IN ( ? ` + strings.Repeat(", ?", len(actions)-1) + ")"
for _, a := range actions {
params = append(params, a)
}
}
}
if options.Scope != "" {
// Search for scope and wildcard that include the scope

View File

@ -16,7 +16,15 @@ type ScopeAttributeResolver interface {
}
type ActionResolver interface {
// ExpandActionSets takes a set of permissions that might include some action set permissions, and returns a set of permissions with action sets expanded into underlying permissions
ExpandActionSets(permissions []Permission) []Permission
// ExpandActionSetsWithFilter works like ExpandActionSets, but it also takes a function for action filtering. When action sets are expanded into the underlying permissions,
// only those permissions whose action is matched by actionMatcher are included.
ExpandActionSetsWithFilter(permissions []Permission, actionMatcher func(action string) bool) []Permission
// ResolveAction returns all action sets that include the given action
ResolveAction(action string) []string
// ResolveActionPrefix returns all action sets that include at least one action with the specified prefix
ResolveActionPrefix(prefix string) []string
}
// ScopeAttributeResolverFunc is an adapter to allow functions to implement ScopeAttributeResolver interface

View File

@ -13,6 +13,10 @@ func (f *FakeActionSetSvc) ResolveAction(action string) []string {
return f.ExpectedActionSets
}
func (f *FakeActionSetSvc) ResolveActionPrefix(prefix string) []string {
return f.ExpectedActionSets
}
func (f *FakeActionSetSvc) ResolveActionSet(actionSet string) []string {
return f.ExpectedActions
}
@ -21,4 +25,8 @@ func (f *FakeActionSetSvc) ExpandActionSets(permissions []accesscontrol.Permissi
return f.ExpectedPermissions
}
func (f *FakeActionSetSvc) ExpandActionSetsWithFilter(permissions []accesscontrol.Permission, actionMatcher func(action string) bool) []accesscontrol.Permission {
return f.ExpectedPermissions
}
func (f *FakeActionSetSvc) StoreActionSet(resource, permission string, actions []string) {}

View File

@ -737,6 +737,31 @@ func managedPermission(action, resource string, resourceID, resourceAttribute st
}
}
// ResolveActionPrefix returns all action sets that include at least one action with the specified prefix
func (s *InMemoryActionSets) ResolveActionPrefix(prefix string) []string {
if prefix == "" {
return []string{}
}
sets := make([]string, 0, len(s.actionSetToActions))
for set, actions := range s.actionSetToActions {
// Only use action sets for folders and dashboards for now
// We need to verify that action sets for other resources do not share names with actions (eg, `datasources:read`)
if !isFolderOrDashboardAction(set) {
continue
}
for _, action := range actions {
if strings.HasPrefix(action, prefix) {
sets = append(sets, set)
break
}
}
}
return sets
}
func (s *InMemoryActionSets) ResolveAction(action string) []string {
actionSets := s.actionToActionSets[action]
sets := make([]string, 0, len(actionSets))
@ -766,7 +791,17 @@ func isFolderOrDashboardAction(action string) bool {
return strings.HasPrefix(action, dashboards.ScopeDashboardsRoot) || strings.HasPrefix(action, dashboards.ScopeFoldersRoot)
}
// ExpandActionSets takes a set of permissions that might include some action set permissions, and returns a set of permissions with action sets expanded into underlying permissions
func (s *InMemoryActionSets) ExpandActionSets(permissions []accesscontrol.Permission) []accesscontrol.Permission {
actionMatcher := func(_ string) bool {
return true
}
return s.ExpandActionSetsWithFilter(permissions, actionMatcher)
}
// ExpandActionSetsWithFilter works like ExpandActionSets, but it also takes a function for action filtering. When action sets are expanded into the underlying permissions,
// only those permissions whose action is matched by actionMatcher are included.
func (s *InMemoryActionSets) ExpandActionSetsWithFilter(permissions []accesscontrol.Permission, actionMatcher func(action string) bool) []accesscontrol.Permission {
var expandedPermissions []accesscontrol.Permission
for _, permission := range permissions {
resolvedActions := s.ResolveActionSet(permission.Action)
@ -775,6 +810,9 @@ func (s *InMemoryActionSets) ExpandActionSets(permissions []accesscontrol.Permis
continue
}
for _, action := range resolvedActions {
if !actionMatcher(action) {
continue
}
permission.Action = action
expandedPermissions = append(expandedPermissions, permission)
}