diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index 3eda12471e0..0650fd7daf5 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -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} diff --git a/pkg/services/authz/rbac/service_test.go b/pkg/services/authz/rbac/service_test.go index ba710182e2e..884ade0613a 100644 --- a/pkg/services/authz/rbac/service_test.go +++ b/pkg/services/authz/rbac/service_test.go @@ -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 {