From 34c40f959f744e65567e9f0b2a8dcc89bdfed452 Mon Sep 17 00:00:00 2001 From: Ieva Date: Wed, 12 Jun 2024 11:20:19 +0300 Subject: [PATCH] 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 * if action is specified, check for exact match; also extend tests --- pkg/services/accesscontrol/accesscontrol.go | 1 + pkg/services/accesscontrol/acimpl/service.go | 34 ++++++- .../accesscontrol/acimpl/service_test.go | 93 +++++++++++++++++-- .../accesscontrol/database/database.go | 18 +++- pkg/services/accesscontrol/resolvers.go | 8 ++ .../accesscontrol/resourcepermissions/fake.go | 8 ++ .../resourcepermissions/store.go | 38 ++++++++ 7 files changed, 189 insertions(+), 11 deletions(-) diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index ecd2d7aaebf..80a9781867b 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -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 diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index 371e25c79f7..ae8db4b6d11 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -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) diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index ed3628f0ef0..ffe845519d9 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -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{ diff --git a/pkg/services/accesscontrol/database/database.go b/pkg/services/accesscontrol/database/database.go index 22b7dc3daf5..43e8564505f 100644 --- a/pkg/services/accesscontrol/database/database.go +++ b/pkg/services/accesscontrol/database/database.go @@ -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 diff --git a/pkg/services/accesscontrol/resolvers.go b/pkg/services/accesscontrol/resolvers.go index 35942adee30..c7ec78eff4a 100644 --- a/pkg/services/accesscontrol/resolvers.go +++ b/pkg/services/accesscontrol/resolvers.go @@ -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 diff --git a/pkg/services/accesscontrol/resourcepermissions/fake.go b/pkg/services/accesscontrol/resourcepermissions/fake.go index 86b455baa80..7e0665a6d37 100644 --- a/pkg/services/accesscontrol/resourcepermissions/fake.go +++ b/pkg/services/accesscontrol/resourcepermissions/fake.go @@ -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) {} diff --git a/pkg/services/accesscontrol/resourcepermissions/store.go b/pkg/services/accesscontrol/resourcepermissions/store.go index bd8a7666671..6711144e803 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store.go +++ b/pkg/services/accesscontrol/resourcepermissions/store.go @@ -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) }