AuthZ: Recover from missing split scope (#111492)

* AuthZ: Recover from missing split scope

* Follow up changes

* Add test

* better log

* Add a comment to getScopeMap

* Punctuation
This commit is contained in:
Gabriel MABILLE
2025-09-24 13:24:21 +02:00
committed by GitHub
parent 73cc4587bf
commit b63ba0269f
2 changed files with 30 additions and 5 deletions
+9 -3
View File
@@ -455,7 +455,7 @@ func (s *Service) getUserPermissions(ctx context.Context, ns types.NamespaceInfo
if err != nil {
return nil, err
}
scopeMap := getScopeMap(permissions)
scopeMap := s.getScopeMap(permissions)
scopeMap, err = s.resolveScopeMap(ctx, ns, scopeMap)
if err != nil {
@@ -485,7 +485,7 @@ func (s *Service) getAnonymousPermissions(ctx context.Context, ns types.Namespac
if err != nil {
return nil, err
}
scopeMap := getScopeMap(permissions)
scopeMap := s.getScopeMap(permissions)
s.permCache.Set(ctx, anonPermKey, scopeMap)
return scopeMap, nil
})
@@ -637,9 +637,15 @@ func (s *Service) checkPermission(ctx context.Context, scopeMap map[string]bool,
return s.checkInheritedPermissions(ctx, scopeMap, req)
}
func getScopeMap(permissions []accesscontrol.Permission) map[string]bool {
func (s *Service) getScopeMap(permissions []accesscontrol.Permission) map[string]bool {
permMap := make(map[string]bool, len(permissions))
for _, perm := range permissions {
// We've had cases where the scope wasn't split properly,
// failing wildcard checks. This is a recovery mechanism.
if perm.Kind == "" && perm.Scope != "" {
s.logger.Warn("found unsplit permission scope", "scope", perm.Scope)
perm.Kind, perm.Attribute, perm.Identifier = accesscontrol.SplitScope(perm.Scope)
}
// If has any wildcard, return immediately
if perm.Kind == "*" || perm.Attribute == "*" || perm.Identifier == "*" {
return map[string]bool{"*": true}
+21 -2
View File
@@ -323,7 +323,7 @@ func TestService_checkPermission(t *testing.T) {
s.folderCache.Set(context.Background(), folderCacheKey("default"), newFolderTree(tc.folders))
tc.check.Namespace = types.NamespaceInfo{Value: "default", OrgID: 1}
got, err := s.checkPermission(context.Background(), getScopeMap(tc.permissions), &tc.check)
got, err := s.checkPermission(context.Background(), s.getScopeMap(tc.permissions), &tc.check)
require.NoError(t, err)
assert.Equal(t, tc.expected, got)
})
@@ -897,7 +897,7 @@ func TestService_listPermission(t *testing.T) {
}
tc.list.Namespace = types.NamespaceInfo{Value: "default", OrgID: 1}
got, err := s.listPermission(context.Background(), getScopeMap(tc.permissions), &tc.list)
got, err := s.listPermission(context.Background(), s.getScopeMap(tc.permissions), &tc.list)
require.NoError(t, err)
assert.Equal(t, tc.expectedAll, got.All)
assert.ElementsMatch(t, tc.expectedItems, got.Items)
@@ -1086,6 +1086,25 @@ func TestService_Check(t *testing.T) {
},
expected: true,
},
{
// We've had cases where permissions were saved to the database
// without splitting the scope into 'kind', 'attribute', and 'identifier'.
// Our wildcard check depends on this separation to work correctly.
// This test makes sure we can still handle those unsplit permissions.
name: "should split wildcard scope if needed",
req: &authzv1.CheckRequest{
Namespace: "org-12",
Subject: "user:test-uid",
Group: "iam.grafana.app",
Resource: "teams",
Verb: "get",
Name: "t1",
},
permissions: []accesscontrol.Permission{
{Action: "teams:read", Scope: "teams:*"},
},
expected: true,
},
}
t.Run("User permission check", func(t *testing.T) {
for _, tc := range testCases {