root
/
grafana
mirror of https://github.com/grafana/grafana.git
1
0
Fork 0
Code Issues Actions 137 Packages Projects Releases Wiki Activity

Access Control: Fix plugin async install role registration

This commit is contained in:
Todd Treece 2025-10-07 12:39:11 -04:00
parent 84a2f41016
commit 6441713d49
No known key found for this signature in database
GPG Key ID: 62C2BA7902C6558C
3 changed files with 276 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
pkg/services/accesscontrol/acimpl/service.go
View File

@ -6,6 +6,7 @@ import (
"slices"
"strconv"
"strings"
"sync"
"time"
"github.com/prometheus/client_golang/prometheus"
@ -108,9 +109,11 @@ type Service struct {
features featuremgmt.FeatureToggles
log log.Logger
registrations accesscontrol.RegistrationList
rolesMu sync.RWMutex
roles map[string]*accesscontrol.RoleDTO
store accesscontrol.Store
permRegistry permreg.PermissionRegistry
isInitialized bool
}
func (s *Service) GetUsageStats(_ context.Context) map[string]any {
@ -139,11 +142,13 @@ func (s *Service) getUserPermissions(ctx context.Context, user identity.Requeste
defer span.End()
permissions := make([]accesscontrol.Permission, 0)
s.rolesMu.RLock()
for _, builtin := range accesscontrol.GetOrgRoles(user) {
if basicRole, ok := s.roles[builtin]; ok {
permissions = append(permissions, basicRole.Permissions...)
}
}
s.rolesMu.RUnlock()
permissions = append(permissions, SharedWithMeFolderPermission)
// we don't care about the error here, if this fails we get 0 and no
@ -170,9 +175,11 @@ func (s *Service) getBasicRolePermissions(ctx context.Context, role string, orgI
defer span.End()
var permissions []accesscontrol.Permission
s.rolesMu.RLock()
if basicRole, ok := s.roles[role]; ok {
permissions = basicRole.Permissions
}
s.rolesMu.RUnlock()
// Fetch managed role permissions assigned to basic roles
dbPermissions, err := s.store.GetBasicRolesPermissions(ctx, accesscontrol.GetUserPermissionsQuery{
@ -423,33 +430,43 @@ func (s *Service) RegisterFixedRoles(ctx context.Context) error {
_, span := tracer.Start(ctx, "accesscontrol.acimpl.RegisterFixedRoles")
defer span.End()
s.registrations.Range(func(registration accesscontrol.RoleRegistration) bool {
for br := range accesscontrol.BuiltInRolesWithParents(registration.Grants) {
if basicRole, ok := s.roles[br]; ok {
for _, p := range registration.Role.Permissions {
if registration.Role.IsPlugin() && p.Action == pluginaccesscontrol.ActionAppAccess {
s.log.Debug("Plugin is attempting to grant access permission, but this permission is already granted by default and will be ignored",
"role", registration.Role.Name, "permission", p.Action, "scope", p.Scope)
continue
}
perm := accesscontrol.Permission{
Action: p.Action,
Scope: p.Scope,
}
s.rolesMu.Lock()
defer s.rolesMu.Unlock()
perm.Kind, perm.Attribute, perm.Identifier = accesscontrol.SplitScope(perm.Scope)
basicRole.Permissions = append(basicRole.Permissions, perm)
}
} else {
s.log.Error("Unknown builtin role", "builtInRole", br)
}
}
s.registrations.Range(func(registration accesscontrol.RoleRegistration) bool {
s.registerRolesLocked(registration)
return true
})
s.isInitialized = true
return nil
}
// registerRolesLocked processes a single role registration and adds permissions to basic roles.
// Must be called with s.rolesMu locked.
func (s *Service) registerRolesLocked(registration accesscontrol.RoleRegistration) {
for br := range accesscontrol.BuiltInRolesWithParents(registration.Grants) {
if basicRole, ok := s.roles[br]; ok {
for _, p := range registration.Role.Permissions {
if registration.Role.IsPlugin() && p.Action == pluginaccesscontrol.ActionAppAccess {
s.log.Debug("Plugin is attempting to grant access permission, but this permission is already granted by default and will be ignored",
"role", registration.Role.Name, "permission", p.Action, "scope", p.Scope)
continue
}
perm := accesscontrol.Permission{
Action: p.Action,
Scope: p.Scope,
}
perm.Kind, perm.Attribute, perm.Identifier = accesscontrol.SplitScope(perm.Scope)
basicRole.Permissions = append(basicRole.Permissions, perm)
}
} else {
s.log.Error("Unknown builtin role", "builtInRole", br)
}
}
}
// DeclarePluginRoles allow the caller to declare, to the service, plugin roles and their assignments
// to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
func (s *Service) DeclarePluginRoles(ctx context.Context, ID, name string, regs []plugins.RoleRegistration) error {
@ -476,6 +493,16 @@ func (s *Service) DeclarePluginRoles(ctx context.Context, ID, name string, regs
s.log.Debug("Registering plugin role", "role", r.Role.Name)
s.registrations.Append(r)
s.rolesMu.RLock()
initialized := s.isInitialized
s.rolesMu.RUnlock()
if initialized {
s.rolesMu.Lock()
s.registerRolesLocked(r)
s.rolesMu.Unlock()
s.cache.Flush()
}
}
return nil
@ -513,6 +540,7 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Reque
// Filter ram permissions
basicPermissions := map[string][]accesscontrol.Permission{}
s.rolesMu.RLock()
for role, basicRole := range s.roles {
for i := range basicRole.Permissions {
if PermissionMatchesSearchOptions(basicRole.Permissions[i], &options) {
@ -520,6 +548,7 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Reque
}
}
}
s.rolesMu.RUnlock()
usersRoles, err := s.store.GetUsersBasicRoles(ctx, nil, usr.GetOrgID())
if err != nil {
@ -630,6 +659,7 @@ func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, search
return nil, fmt.Errorf("found no basic roles for user %d in organisation %d", searchOptions.UserID, orgID)
}
permissions := make([]accesscontrol.Permission, 0)
s.rolesMu.RLock()
for _, builtin := range roles {
if basicRole, ok := s.roles[builtin]; ok {
for _, permission := range basicRole.Permissions {
@ -639,6 +669,7 @@ func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, search
}
}
}
s.rolesMu.RUnlock()
searchOptions.ActionSets = s.actionResolver.ResolveAction(searchOptions.Action)
searchOptions.ActionSets = append(searchOptions.ActionSets,
@ -741,7 +772,15 @@ func (s *Service) SyncUserRoles(ctx context.Context, orgID int64, cmd accesscont
}
func (s *Service) GetStaticRoles(ctx context.Context) map[string]*accesscontrol.RoleDTO {
return s.roles
s.rolesMu.RLock()
defer s.rolesMu.RUnlock()
// Return a copy to avoid external modifications
rolesCopy := make(map[string]*accesscontrol.RoleDTO, len(s.roles))
for k, v := range s.roles {
rolesCopy[k] = v
}
return rolesCopy
}
func (s *Service) GetRoleByName(ctx context.Context, orgID int64, roleName string) (*accesscontrol.RoleDTO, error) {
@ -749,7 +788,10 @@ func (s *Service) GetRoleByName(ctx context.Context, orgID int64, roleName strin
defer span.End()
err := accesscontrol.ErrRoleNotFound
if _, ok := s.roles[roleName]; ok {
s.rolesMu.RLock()
_, isBasicRole := s.roles[roleName]
s.rolesMu.RUnlock()
if isBasicRole {
return nil, err
}

16
pkg/services/accesscontrol/acimpl/service_bench_test.go
View File

@ -14,6 +14,8 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
"github.com/grafana/grafana/pkg/services/accesscontrol/database"
"github.com/grafana/grafana/pkg/services/accesscontrol/permreg"
"github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
@ -28,12 +30,14 @@ func setupBenchEnv(b *testing.B, usersCount, resourceCount int) (accesscontrol.S
sqlStore := db.InitTestDB(b)
store := database.ProvideService(sqlStore)
acService := &Service{
cfg: setting.NewCfg(),
log: log.New("accesscontrol-test"),
registrations: accesscontrol.RegistrationList{},
store: store,
roles: accesscontrol.BuildBasicRoleDefinitions(),
cache: localcache.New(1*time.Second, 1*time.Second),
cfg: setting.NewCfg(),
log: log.New("accesscontrol-test"),
registrations: accesscontrol.RegistrationList{},
store: store,
roles: accesscontrol.BuildBasicRoleDefinitions(),
cache: localcache.New(1*time.Second, 1*time.Second),
permRegistry: permreg.ProvidePermissionRegistry(),
actionResolver: resourcepermissions.NewActionSetService(),
}
// Prepare default permissions

213
pkg/services/accesscontrol/acimpl/service_test.go
View File

@ -31,7 +31,7 @@ func TestMain(m *testing.M) {
testsuite.Run(m)
}
func setupTestEnv(t testing.TB) *Service {
func setupTestEnv(t testing.TB, registerRoles bool) *Service {
t.Helper()
cfg := setting.NewCfg()
@ -46,7 +46,11 @@ func setupTestEnv(t testing.TB) *Service {
permRegistry: permreg.ProvidePermissionRegistry(),
actionResolver: resourcepermissions.NewActionSetService(),
}
require.NoError(t, ac.RegisterFixedRoles(context.Background()))
if registerRoles {
require.NoError(t, ac.RegisterFixedRoles(context.Background()))
}
return ac
}
@ -145,7 +149,7 @@ func TestIntegrationService_DeclareFixedRoles(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
// Reset the registations
ac.registrations = accesscontrol.RegistrationList{}
@ -260,7 +264,7 @@ func TestIntegrationService_DeclarePluginRoles(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
// Reset the registations
ac.registrations = accesscontrol.RegistrationList{}
@ -361,7 +365,7 @@ func TestIntegrationService_RegisterFixedRoles(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
ac.registrations.Append(tt.registrations...)
@ -389,6 +393,193 @@ func TestIntegrationService_RegisterFixedRoles(t *testing.T) {
}
}
func TestIntegrationService_DeclarePluginRoles_DynamicRegistration(t *testing.T) {
testutil.SkipIntegrationTestInShortMode(t)
tests := []struct {
name string
postRegister bool
pluginID string
registrations []plugins.RoleRegistration
wantGrantedRoles []string
wantPermissions []string
wantPermCount int
}{
{
name: "should register plugin roles before initialization",
postRegister: false,
pluginID: "test-app",
registrations: []plugins.RoleRegistration{
{
Role: plugins.Role{
Name: "Reader",
Description: "Test reader role",
Permissions: []plugins.Permission{
{Action: "test-app:read", Scope: "test-app:*"},
},
},
Grants: []string{string(identity.RoleViewer)},
},
},
wantGrantedRoles: []string{string(identity.RoleViewer)},
wantPermissions: []string{"test-app:read"},
wantPermCount: 1,
},
{
name: "should register plugin roles dynamically after initialization",
postRegister: true,
pluginID: "dynamic-app",
registrations: []plugins.RoleRegistration{
{
Role: plugins.Role{
Name: "DynamicReader",
Description: "Dynamic reader role",
Permissions: []plugins.Permission{
{Action: "dynamic-app:read", Scope: "dynamic-app:*"},
{Action: "dynamic-app:query", Scope: "dynamic-app:*"},
},
},
Grants: []string{string(identity.RoleViewer)},
},
},
wantGrantedRoles: []string{string(identity.RoleViewer)},
wantPermissions: []string{"dynamic-app:read", "dynamic-app:query"},
wantPermCount: 2,
},
{
name: "should grant to multiple roles dynamically",
postRegister: true,
pluginID: "multi-app",
registrations: []plugins.RoleRegistration{
{
Role: plugins.Role{
Name: "MultiReader",
Description: "Multi reader role",
Permissions: []plugins.Permission{
{Action: "multi-app:read", Scope: "multi-app:*"},
},
},
Grants: []string{string(identity.RoleViewer), string(identity.RoleEditor)},
},
},
wantGrantedRoles: []string{string(identity.RoleViewer), string(identity.RoleEditor)},
wantPermissions: []string{"multi-app:read"},
wantPermCount: 1,
},
{
name: "should register multiple permissions dynamically",
postRegister: true,
pluginID: "batch-app",
registrations: []plugins.RoleRegistration{
{
Role: plugins.Role{
Name: "BatchReader",
Permissions: []plugins.Permission{
{Action: "batch-app:read"},
{Action: "batch-app:write"},
{Action: "batch-app:delete"},
},
},
Grants: []string{string(identity.RoleEditor)},
},
},
wantGrantedRoles: []string{string(identity.RoleEditor)},
wantPermissions: []string{"batch-app:read", "batch-app:write", "batch-app:delete"},
wantPermCount: 3,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac := setupTestEnv(t, tt.postRegister)
initialPermCounts := make(map[string]int)
for _, roleName := range tt.wantGrantedRoles {
if role, ok := ac.roles[roleName]; ok {
initialPermCounts[roleName] = len(role.Permissions)
}
}
err := ac.DeclarePluginRoles(context.Background(), tt.pluginID, tt.pluginID, tt.registrations)
require.NoError(t, err)
if !tt.postRegister {
err = ac.RegisterFixedRoles(context.Background())
require.NoError(t, err)
}
// Verify permissions were added to expected roles
for _, roleName := range tt.wantGrantedRoles {
role, ok := ac.roles[roleName]
require.True(t, ok, "Role %s should exist", roleName)
expectedCount := initialPermCounts[roleName] + tt.wantPermCount
assert.Equal(t, expectedCount, len(role.Permissions))
actions := make([]string, 0, len(role.Permissions))
for _, perm := range role.Permissions {
actions = append(actions, perm.Action)
}
for _, wantAction := range tt.wantPermissions {
assert.Contains(t, actions, wantAction, "Role %s should have permission %s", roleName, wantAction)
}
}
})
}
}
func TestIntegrationService_DeclarePluginRoles_UserPermissions(t *testing.T) {
testutil.SkipIntegrationTestInShortMode(t)
t.Run("dynamic registration should be reflected in user permissions", func(t *testing.T) {
ac := setupTestEnv(t, true)
// Create a test user with Viewer role
testUser := &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: identity.RoleViewer,
}
// Get initial permissions
initialPerms, err := ac.getUserPermissions(context.Background(), testUser, accesscontrol.Options{})
require.NoError(t, err)
initialCount := len(initialPerms)
// Register plugin roles dynamically
err = ac.DeclarePluginRoles(context.Background(), "perm-test-app", "Perm Test App", []plugins.RoleRegistration{
{
Role: plugins.Role{
Name: "PermTester",
Description: "Permission test role",
Permissions: []plugins.Permission{
{Action: "perm-test-app:test", Scope: "perm-test-app:*"},
},
},
Grants: []string{string(identity.RoleViewer)},
},
})
require.NoError(t, err)
updatedPerms, err := ac.getUserPermissions(context.Background(), testUser, accesscontrol.Options{})
require.NoError(t, err)
require.Greater(t, len(updatedPerms), initialCount)
hasNewPerm := false
hasNewScope := false
for _, perm := range updatedPerms {
if perm.Action == "perm-test-app:test" {
hasNewPerm = true
}
if perm.Scope == "perm-test-app:*" {
hasNewScope = true
}
}
assert.True(t, hasNewPerm)
assert.True(t, hasNewScope)
})
}
func TestIntegrationService_SearchUsersPermissions(t *testing.T) {
testutil.SkipIntegrationTestInShortMode(t)
@ -585,7 +776,7 @@ func TestIntegrationService_SearchUsersPermissions(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
ac.roles = tt.ramRoles
ac.store = actest.FakeStore{
@ -817,7 +1008,7 @@ func TestIntegrationService_SearchUserPermissions(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
if tt.withActionSets {
actionSetSvc := resourcepermissions.NewActionSetService()
for set, actions := range tt.actionSets {
@ -913,7 +1104,7 @@ func TestIntegrationService_SaveExternalServiceRole(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
ac.cfg.ManagedServiceAccountsEnabled = true
ac.features = featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAccounts)
for _, r := range tt.runs {
@ -969,7 +1160,7 @@ func TestIntegrationService_DeleteExternalServiceRole(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
ac.cfg.ManagedServiceAccountsEnabled = true
ac.features = featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAccounts)
@ -1006,7 +1197,7 @@ func TestIntegrationService_GetRoleByName(t *testing.T) {
t.Run("when the role does not exists, it returns an error", func(t *testing.T) {
t.Parallel()
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
ac.registrations = accesscontrol.RegistrationList{}
role, err := ac.GetRoleByName(ctx, 0, "not-found-role")
@ -1019,7 +1210,7 @@ func TestIntegrationService_GetRoleByName(t *testing.T) {
roleName := "fixed:test:test"
ac := setupTestEnv(t)
ac := setupTestEnv(t, true)
ac.registrations = accesscontrol.RegistrationList{}
ac.registrations.Append(accesscontrol.RoleRegistration{
Role: accesscontrol.RoleDTO{Name: roleName},