diff --git a/pkg/services/authz/rbac/service.go b/pkg/services/authz/rbac/service.go index 4c3f3ba0393..c1d7d55551f 100644 --- a/pkg/services/authz/rbac/service.go +++ b/pkg/services/authz/rbac/service.go @@ -186,6 +186,150 @@ func (s *Service) Check(ctx context.Context, req *authzv1.CheckRequest) (*authzv return &authzv1.CheckResponse{Allowed: allowed}, nil } +// BatchCheck implements authzv1.AuthzServiceServer.BatchCheck +// This performs multiple access checks in a single request with optimized batching. +// 1. Validates the subject once +// 2. Groups checks by (namespace, action) to load permissions once per group +// 3. Reuses the folder tree across checks +func (s *Service) BatchCheck(ctx context.Context, req *authzv1.BatchCheckRequest) (*authzv1.BatchCheckResponse, error) { + ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.BatchCheck") + defer span.End() + + checks := req.GetChecks() + span.SetAttributes(attribute.Int("check_count", len(checks))) + + ctxLogger := s.logger.FromContext(ctx).New( + "subject", req.GetSubject(), + "check_count", len(checks), + ) + defer func(start time.Time) { + ctxLogger.Debug("BatchCheck execution time", "duration", time.Since(start).Milliseconds()) + }(time.Now()) + + // Early check for auth info - required for namespace validation + if _, has := types.AuthInfoFrom(ctx); !has { + return nil, status.Error(codes.Internal, "could not get auth info from context") + } + + if len(checks) == 0 { + return &authzv1.BatchCheckResponse{ + Results: make(map[string]*authzv1.BatchCheckResult), + Zookie: &authzv1.Zookie{Timestamp: time.Now().UnixMilli()}, + }, nil + } + + // Validate subject once for all checks + userUID, idType, err := s.validateSubject(ctx, req.GetSubject()) + if err != nil { + ctxLogger.Error("invalid subject", "error", err) + // Return all checks as denied with the same error + results := make(map[string]*authzv1.BatchCheckResult, len(checks)) + for _, item := range checks { + results[item.GetCorrelationId()] = &authzv1.BatchCheckResult{ + Allowed: false, + Error: err.Error(), + } + } + return &authzv1.BatchCheckResponse{Results: results, Zookie: &authzv1.Zookie{Timestamp: time.Now().UnixMilli()}}, nil + } + + results := make(map[string]*authzv1.BatchCheckResult, len(checks)) + + // Group checks by (namespace, action) to batch permission lookups + type checkGroup struct { + namespace types.NamespaceInfo + action string + actionSets []string + items []*authzv1.BatchCheckItem + checkReqs []*checkRequest + } + groups := make(map[string]*checkGroup) + + // First pass: validate and group checks + for _, item := range checks { + ns, err := validateNamespace(ctx, item.GetNamespace()) + if err != nil { + results[item.GetCorrelationId()] = &authzv1.BatchCheckResult{Allowed: false, Error: err.Error()} + continue + } + + action, actionSets, err := s.validateAction(ctx, item.GetGroup(), item.GetResource(), item.GetVerb()) + if err != nil { + results[item.GetCorrelationId()] = &authzv1.BatchCheckResult{Allowed: false, Error: err.Error()} + continue + } + + // Create the internal check request + checkReq := &checkRequest{ + Namespace: ns, + UserUID: userUID, + IdentityType: idType, + Action: action, + ActionSets: actionSets, + Group: item.GetGroup(), + Resource: item.GetResource(), + Verb: item.GetVerb(), + Name: item.GetName(), + ParentFolder: item.GetFolder(), + } + + // Group by namespace + action + groupKey := ns.Value + ":" + action + if g, ok := groups[groupKey]; ok { + g.items = append(g.items, item) + g.checkReqs = append(g.checkReqs, checkReq) + } else { + groups[groupKey] = &checkGroup{ + namespace: ns, + action: action, + actionSets: actionSets, + items: []*authzv1.BatchCheckItem{item}, + checkReqs: []*checkRequest{checkReq}, + } + } + } + + // Second pass: process each group with shared permissions + for _, group := range groups { + // Set namespace in context for this group (required by store methods) + groupCtx := request.WithNamespace(ctx, group.namespace.Value) + + // Try to get cached permissions first, then fall back to store + permissions, err := s.getCachedIdentityPermissions(groupCtx, group.namespace, idType, userUID, group.action) + if err != nil { + // Cache miss - fetch from store + permissions, err = s.getIdentityPermissions(groupCtx, group.namespace, idType, userUID, group.action, group.actionSets) + if err != nil { + ctxLogger.Error("could not get permissions", "namespace", group.namespace.Value, "action", group.action, "error", err) + for _, item := range group.items { + results[item.GetCorrelationId()] = &authzv1.BatchCheckResult{Allowed: false, Error: err.Error()} + } + continue + } + } + + // Check each item in the group using the shared permissions + for i, item := range group.items { + checkReq := group.checkReqs[i] + + allowed, err := s.checkPermission(groupCtx, permissions, checkReq) + if err != nil { + results[item.GetCorrelationId()] = &authzv1.BatchCheckResult{Allowed: false, Error: err.Error()} + continue + } + + results[item.GetCorrelationId()] = &authzv1.BatchCheckResult{Allowed: allowed} + } + } + + span.SetAttributes(attribute.Int("groups_processed", len(groups))) + + return &authzv1.BatchCheckResponse{ + Results: results, + Zookie: &authzv1.Zookie{Timestamp: time.Now().UnixMilli()}, + }, nil +} + func (s *Service) List(ctx context.Context, req *authzv1.ListRequest) (*authzv1.ListResponse, error) { ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.List") defer span.End() diff --git a/pkg/services/authz/rbac/service_test.go b/pkg/services/authz/rbac/service_test.go index c3a9f29a3c1..2a5faea7c68 100644 --- a/pkg/services/authz/rbac/service_test.go +++ b/pkg/services/authz/rbac/service_test.go @@ -1829,6 +1829,613 @@ func TestService_CacheList(t *testing.T) { }) } +func TestService_BatchCheck(t *testing.T) { + callingService := authn.NewAccessTokenAuthInfo(authn.Claims[authn.AccessTokenClaims]{ + Claims: jwt.Claims{ + Subject: types.NewTypeID(types.TypeAccessPolicy, "some-service"), + Audience: []string{"authzservice"}, + }, + Rest: authn.AccessTokenClaims{Namespace: "org-12"}, + }) + + t.Run("Require auth info", func(t *testing.T) { + s := setupService() + ctx := context.Background() + _, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "user:test-uid", + Checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "could not get auth info") + }) + + t.Run("Empty checks returns empty results", func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "user:test-uid", + Checks: []*authzv1.BatchCheckItem{}, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.Empty(t, resp.Results) + }) + + type batchCheckTestCase struct { + name string + checks []*authzv1.BatchCheckItem + permissions []accesscontrol.Permission + folders []store.Folder + expectedResults map[string]bool + expectedErrors map[string]bool // true if error expected for this correlation ID + expectGlobalError bool + } + + t.Run("Request validation", func(t *testing.T) { + testCases := []batchCheckTestCase{ + { + name: "should return error for invalid namespace", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + expectedResults: map[string]bool{"check1": false}, + expectedErrors: map[string]bool{"check1": true}, + }, + { + name: "should return error for namespace mismatch", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-13", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + expectedResults: map[string]bool{"check1": false}, + expectedErrors: map[string]bool{"check1": true}, + }, + { + name: "should return error for unknown group", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "unknown.grafana.app", + Resource: "unknown", + Verb: "get", + Name: "u1", + CorrelationId: "check1", + }, + }, + expectedResults: map[string]bool{"check1": false}, + expectedErrors: map[string]bool{"check1": true}, + }, + { + name: "should return error for unknown verb", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "unknown", + Name: "dash1", + CorrelationId: "check1", + }, + }, + expectedResults: map[string]bool{"check1": false}, + expectedErrors: map[string]bool{"check1": true}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + userID := &store.UserIdentifiers{UID: "test-uid", ID: 1} + store := &fakeStore{ + userID: userID, + userPermissions: tc.permissions, + } + s.store = store + s.permissionStore = store + s.identityStore = &fakeIdentityStore{} + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "user:test-uid", + Checks: tc.checks, + }) + require.NoError(t, err) + require.NotNil(t, resp) + for corrID, expectedAllowed := range tc.expectedResults { + result, ok := resp.Results[corrID] + require.True(t, ok, "result for %s not found", corrID) + require.Equal(t, expectedAllowed, result.Allowed, "unexpected allowed for %s", corrID) + if tc.expectedErrors[corrID] { + require.NotEmpty(t, result.Error, "expected error for %s", corrID) + } + } + }) + } + }) + + t.Run("User permission checks", func(t *testing.T) { + testCases := []batchCheckTestCase{ + { + name: "should allow user with permission on single resource", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + permissions: []accesscontrol.Permission{{Action: "dashboards:read", Scope: "dashboards:uid:dash1"}}, + expectedResults: map[string]bool{"check1": true}, + }, + { + name: "should deny user without permission", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + permissions: []accesscontrol.Permission{{Action: "dashboards:read", Scope: "dashboards:uid:dash2"}}, + expectedResults: map[string]bool{"check1": false}, + }, + { + name: "should handle multiple checks with mixed results", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash2", + CorrelationId: "check2", + }, + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash3", + CorrelationId: "check3", + }, + }, + permissions: []accesscontrol.Permission{ + {Action: "dashboards:read", Scope: "dashboards:uid:dash1"}, + {Action: "dashboards:read", Scope: "dashboards:uid:dash3"}, + }, + expectedResults: map[string]bool{ + "check1": true, + "check2": false, + "check3": true, + }, + }, + { + name: "should handle wildcard permission", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash2", + CorrelationId: "check2", + }, + }, + permissions: []accesscontrol.Permission{{Action: "dashboards:read", Scope: "*", Kind: "*"}}, + expectedResults: map[string]bool{"check1": true, "check2": true}, + }, + { + name: "should handle folder inheritance", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + Folder: "child", + CorrelationId: "check1", + }, + }, + permissions: []accesscontrol.Permission{ + {Action: "dashboards:read", Scope: "folders:uid:parent", Kind: "folders", Attribute: "uid", Identifier: "parent"}, + }, + folders: []store.Folder{ + {UID: "parent"}, + {UID: "child", ParentUID: strPtr("parent")}, + }, + expectedResults: map[string]bool{"check1": true}, + }, + { + name: "should handle action sets", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + permissions: []accesscontrol.Permission{{Action: "dashboards:admin", Scope: "dashboards:uid:dash1"}}, + expectedResults: map[string]bool{"check1": true}, + }, + { + name: "should handle checks across different resources", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + { + Namespace: "org-12", + Group: "folder.grafana.app", + Resource: "folders", + Verb: "get", + Name: "fold1", + CorrelationId: "check2", + }, + }, + permissions: []accesscontrol.Permission{ + {Action: "dashboards:read", Scope: "dashboards:uid:dash1"}, + {Action: "folders:read", Scope: "folders:uid:fold1"}, + }, + expectedResults: map[string]bool{"check1": true, "check2": true}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + userID := &store.UserIdentifiers{UID: "test-uid", ID: 1} + store := &fakeStore{ + userID: userID, + userPermissions: tc.permissions, + folders: tc.folders, + } + s.store = store + s.permissionStore = store + s.folderStore = store + s.identityStore = &fakeIdentityStore{} + + if tc.folders != nil { + s.folderCache.Set(ctx, folderCacheKey("org-12"), newFolderTree(tc.folders)) + } + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "user:test-uid", + Checks: tc.checks, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.Len(t, resp.Results, len(tc.expectedResults)) + for corrID, expectedAllowed := range tc.expectedResults { + result, ok := resp.Results[corrID] + require.True(t, ok, "result for %s not found", corrID) + require.Equal(t, expectedAllowed, result.Allowed, "unexpected allowed for %s", corrID) + } + }) + } + }) + + t.Run("Anonymous permission checks", func(t *testing.T) { + testCases := []batchCheckTestCase{ + { + name: "should allow anonymous with permission", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + permissions: []accesscontrol.Permission{{Action: "dashboards:read", Scope: "dashboards:uid:dash1"}}, + expectedResults: map[string]bool{"check1": true}, + }, + { + name: "should deny anonymous without permission", + checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + permissions: []accesscontrol.Permission{{Action: "dashboards:read", Scope: "dashboards:uid:dash2"}}, + expectedResults: map[string]bool{"check1": false}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + store := &fakeStore{userPermissions: tc.permissions} + s.store = store + s.permissionStore = store + s.identityStore = &fakeIdentityStore{} + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "anonymous:0", + Checks: tc.checks, + }) + require.NoError(t, err) + require.NotNil(t, resp) + for corrID, expectedAllowed := range tc.expectedResults { + result, ok := resp.Results[corrID] + require.True(t, ok, "result for %s not found", corrID) + require.Equal(t, expectedAllowed, result.Allowed, "unexpected allowed for %s", corrID) + } + }) + } + }) + + t.Run("Rendering permission checks", func(t *testing.T) { + t.Run("should allow rendering with permission", func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "render:0", + Checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.True(t, resp.Results["check1"].Allowed) + }) + + t.Run("should deny rendering access to another app resources", func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "render:0", + Checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "another.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.False(t, resp.Results["check1"].Allowed) + require.NotEmpty(t, resp.Results["check1"].Error) + }) + }) + + t.Run("Invalid subject returns errors for all checks", func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + store := &fakeStore{} + s.store = store + s.permissionStore = store + s.identityStore = &fakeIdentityStore{} + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "invalid:12", + Checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash2", + CorrelationId: "check2", + }, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.Len(t, resp.Results, 2) + for _, result := range resp.Results { + require.False(t, result.Allowed) + require.NotEmpty(t, result.Error) + } + }) + + t.Run("Grouping optimization", func(t *testing.T) { + t.Run("should batch permission lookups for same action", func(t *testing.T) { + s := setupService() + ctx := types.WithAuthInfo(context.Background(), callingService) + userID := &store.UserIdentifiers{UID: "test-uid", ID: 1} + fStore := &fakeStore{ + userID: userID, + userPermissions: []accesscontrol.Permission{ + {Action: "dashboards:read", Scope: "dashboards:uid:dash1"}, + {Action: "dashboards:read", Scope: "dashboards:uid:dash2"}, + }, + } + s.store = fStore + s.permissionStore = fStore + s.identityStore = &fakeIdentityStore{} + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "user:test-uid", + Checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash2", + CorrelationId: "check2", + }, + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash3", + CorrelationId: "check3", + }, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.True(t, resp.Results["check1"].Allowed) + require.True(t, resp.Results["check2"].Allowed) + require.False(t, resp.Results["check3"].Allowed) + + // Verify permissions were fetched only once (1 call for userID + 1 call for basicRole + 1 call for permissions) + require.Equal(t, 3, fStore.calls) + }) + }) +} + +func TestService_CacheBatchCheck(t *testing.T) { + callingService := authn.NewAccessTokenAuthInfo(authn.Claims[authn.AccessTokenClaims]{ + Claims: jwt.Claims{ + Subject: types.NewTypeID(types.TypeAccessPolicy, "some-service"), + Audience: []string{"authzservice"}, + }, + Rest: authn.AccessTokenClaims{Namespace: "org-12"}, + }) + + ctx := types.WithAuthInfo(context.Background(), callingService) + userID := &store.UserIdentifiers{UID: "test-uid", ID: 1} + + t.Run("Allow based on cached permissions", func(t *testing.T) { + s := setupService() + + s.idCache.Set(ctx, userIdentifierCacheKey("org-12", "test-uid"), *userID) + s.permCache.Set(ctx, userPermCacheKey("org-12", "test-uid", "dashboards:read"), map[string]bool{"dashboards:uid:dash1": true}) + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "user:test-uid", + Checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash1", + CorrelationId: "check1", + }, + }, + }) + require.NoError(t, err) + require.True(t, resp.Results["check1"].Allowed) + }) + + t.Run("Fallback to database on cache miss", func(t *testing.T) { + s := setupService() + + // Populate database but not cache + fStore := &fakeStore{ + userID: userID, + userPermissions: []accesscontrol.Permission{{Action: "dashboards:read", Scope: "dashboards:uid:dash2"}}, + } + s.store = fStore + s.permissionStore = fStore + s.identityStore = &fakeIdentityStore{} + + s.idCache.Set(ctx, userIdentifierCacheKey("org-12", "test-uid"), *userID) + + resp, err := s.BatchCheck(ctx, &authzv1.BatchCheckRequest{ + Subject: "user:test-uid", + Checks: []*authzv1.BatchCheckItem{ + { + Namespace: "org-12", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: "get", + Name: "dash2", + CorrelationId: "check1", + }, + }, + }) + require.NoError(t, err) + require.True(t, resp.Results["check1"].Allowed) + }) +} + func setupService() *Service { cache := cache.NewLocalCache(cache.Config{Expiry: 5 * time.Minute, CleanupInterval: 5 * time.Minute}) logger := log.New("authz-rbac-service") diff --git a/pkg/services/authz/zanzana/server/server_batch_check.go b/pkg/services/authz/zanzana/server/server_batch_check.go index 97fc1ed4e76..035fd52df13 100644 --- a/pkg/services/authz/zanzana/server/server_batch_check.go +++ b/pkg/services/authz/zanzana/server/server_batch_check.go @@ -99,14 +99,24 @@ func (s *Server) batchCheck(ctx context.Context, r *authzv1.BatchCheckRequest) ( }, nil } - namespace := r.GetNamespace() - if err := authorize(ctx, namespace, s.cfg); err != nil { - return nil, err + // Group items by namespace + itemsByNamespace := make(map[string][]*authzv1.BatchCheckItem) + for _, item := range items { + ns := item.GetNamespace() + itemsByNamespace[ns] = append(itemsByNamespace[ns], item) } - store, err := s.getStoreInfo(ctx, namespace) - if err != nil { - return nil, err + // Authorize and get store info for each namespace + stores := make(map[string]*storeInfo) + for namespace := range itemsByNamespace { + if err := authorize(ctx, namespace, s.cfg); err != nil { + return nil, err + } + store, err := s.getStoreInfo(ctx, namespace) + if err != nil { + return nil, err + } + stores[namespace] = store } contextuals, err := s.getContextuals(r.GetSubject()) @@ -117,30 +127,26 @@ func (s *Server) batchCheck(ctx context.Context, r *authzv1.BatchCheckRequest) ( results := make(map[string]*authzv1.BatchCheckResult, len(items)) subject := r.GetSubject() - // Phase 1: Check GroupResource access (broadest permissions) - // Example: user has "get" on "dashboards" group_resource → all dashboards allowed - s.runGroupResourcePhase(ctx, store, subject, items, contextuals, results) - if len(results) == len(items) { - return s.buildResponse(results), nil - } + // Process each namespace separately + for namespace, nsItems := range itemsByNamespace { + store := stores[namespace] - // Phase 2: Check folder permission inheritance (can_get, can_create, etc. on parent folder) - // Example: user has "can_get" on folder-A → all dashboards in folder-A allowed - s.runFolderPermissionPhase(ctx, store, subject, items, contextuals, results) - if len(results) == len(items) { - return s.buildResponse(results), nil - } + // Phase 1: Check GroupResource access (broadest permissions) + // Example: user has "get" on "dashboards" group_resource → all dashboards allowed + s.runGroupResourcePhase(ctx, store, subject, nsItems, contextuals, results) - // Phase 3: Check folder subresource access (folder_get, folder_create, etc.) - // Example: user has "folder_get" on folder-A → dashboards in folder-A allowed via subresource - s.runFolderSubresourcePhase(ctx, store, subject, items, contextuals, results) - if len(results) == len(items) { - return s.buildResponse(results), nil - } + // Phase 2: Check folder permission inheritance (can_get, can_create, etc. on parent folder) + // Example: user has "can_get" on folder-A → all dashboards in folder-A allowed + s.runFolderPermissionPhase(ctx, store, subject, nsItems, contextuals, results) - // Phase 4: Check direct resource access - // Example: user has "get" directly on dashboard-123 - s.runDirectResourcePhase(ctx, store, subject, items, contextuals, results) + // Phase 3: Check folder subresource access (folder_get, folder_create, etc.) + // Example: user has "folder_get" on folder-A → dashboards in folder-A allowed via subresource + s.runFolderSubresourcePhase(ctx, store, subject, nsItems, contextuals, results) + + // Phase 4: Check direct resource access + // Example: user has "get" directly on dashboard-123 + s.runDirectResourcePhase(ctx, store, subject, nsItems, contextuals, results) + } // Mark any remaining unresolved items as denied for _, item := range items { diff --git a/pkg/services/authz/zanzana/server/server_batch_check_test.go b/pkg/services/authz/zanzana/server/server_batch_check_test.go index fb337561278..411391cf368 100644 --- a/pkg/services/authz/zanzana/server/server_batch_check_test.go +++ b/pkg/services/authz/zanzana/server/server_batch_check_test.go @@ -15,16 +15,16 @@ func testBatchCheck(t *testing.T, server *Server) { // Helper to create a batch check request newReq := func(subject string, items []*authzv1.BatchCheckItem) *authzv1.BatchCheckRequest { return &authzv1.BatchCheckRequest{ - Subject: subject, - Namespace: namespace, - Checks: items, + Subject: subject, + Checks: items, } } - // Helper to create a batch check item with correlation ID + // Helper to create a batch check item with correlation ID (uses default namespace) newItem := func(verb, group, resource, subresource, folder, name string) *authzv1.BatchCheckItem { correlationID := fmt.Sprintf("%s-%s-%s-%s", group, resource, folder, name) return &authzv1.BatchCheckItem{ + Namespace: namespace, Verb: verb, Group: group, Resource: resource, @@ -189,4 +189,114 @@ func testBatchCheck(t *testing.T, server *Server) { assert.True(t, res.Results[fmt.Sprintf("%s-%s-%s-%s", dashboardGroup, dashboardResource, "6", "12")].Allowed) assert.False(t, res.Results[fmt.Sprintf("%s-%s-%s-%s", dashboardGroup, dashboardResource, "1", "13")].Allowed) }) + + // Cross-namespace tests + t.Run("cross-namespace: items with explicit namespace should be authorized against their own namespace", func(t *testing.T) { + // Helper to create item with explicit namespace + newItemWithNamespace := func(ns, verb, group, resource, subresource, folder, name string) *authzv1.BatchCheckItem { + correlationID := fmt.Sprintf("%s-%s-%s-%s-%s", ns, group, resource, folder, name) + return &authzv1.BatchCheckItem{ + Namespace: ns, + Verb: verb, + Group: group, + Resource: resource, + Subresource: subresource, + Name: name, + Folder: folder, + CorrelationId: correlationID, + } + } + + // user:1 has access to dashboard 1 in folder 1 in "default" namespace + // Both items use explicit namespace + res, err := server.BatchCheck(newContextWithNamespace(), &authzv1.BatchCheckRequest{ + Subject: "user:1", + Checks: []*authzv1.BatchCheckItem{ + // Item in default namespace (should be allowed - user:1 has access) + newItemWithNamespace(namespace, utils.VerbGet, dashboardGroup, dashboardResource, "", "1", "1"), + // Another item in default namespace with different correlation ID + newItem(utils.VerbGet, dashboardGroup, dashboardResource, "", "1", "1"), + }, + }) + require.NoError(t, err) + require.Len(t, res.Results, 2) + + assert.True(t, res.Results[fmt.Sprintf("%s-%s-%s-%s-%s", namespace, dashboardGroup, dashboardResource, "1", "1")].Allowed) + assert.True(t, res.Results[fmt.Sprintf("%s-%s-%s-%s", dashboardGroup, dashboardResource, "1", "1")].Allowed) + }) + + t.Run("cross-namespace: items from different namespaces in same batch", func(t *testing.T) { + newItemWithNamespace := func(ns, verb, group, resource, subresource, folder, name string) *authzv1.BatchCheckItem { + correlationID := fmt.Sprintf("%s-%s-%s-%s-%s", ns, group, resource, folder, name) + return &authzv1.BatchCheckItem{ + Namespace: ns, + Verb: verb, + Group: group, + Resource: resource, + Subresource: subresource, + Name: name, + Folder: folder, + CorrelationId: correlationID, + } + } + + // user:2 has group_resource access in "default" namespace + // They should have access in default but not in other-namespace (no tuples there) + res, err := server.BatchCheck(newContextWithNamespace(), &authzv1.BatchCheckRequest{ + Subject: "user:2", + Checks: []*authzv1.BatchCheckItem{ + // Items in default namespace (should be allowed - user:2 has group_resource access) + newItemWithNamespace(namespace, utils.VerbGet, dashboardGroup, dashboardResource, "", "1", "1"), + newItemWithNamespace(namespace, utils.VerbGet, dashboardGroup, dashboardResource, "", "2", "2"), + // Items in other-namespace (should be denied - no tuples in other-namespace) + newItemWithNamespace("other-namespace", utils.VerbGet, dashboardGroup, dashboardResource, "", "1", "1"), + newItemWithNamespace("other-namespace", utils.VerbGet, dashboardGroup, dashboardResource, "", "2", "2"), + }, + }) + require.NoError(t, err) + require.Len(t, res.Results, 4) + + // Default namespace items should be allowed + assert.True(t, res.Results[fmt.Sprintf("%s-%s-%s-%s-%s", namespace, dashboardGroup, dashboardResource, "1", "1")].Allowed) + assert.True(t, res.Results[fmt.Sprintf("%s-%s-%s-%s-%s", namespace, dashboardGroup, dashboardResource, "2", "2")].Allowed) + // Other namespace items should be denied (no permissions in that namespace) + assert.False(t, res.Results[fmt.Sprintf("%s-%s-%s-%s-%s", "other-namespace", dashboardGroup, dashboardResource, "1", "1")].Allowed) + assert.False(t, res.Results[fmt.Sprintf("%s-%s-%s-%s-%s", "other-namespace", dashboardGroup, dashboardResource, "2", "2")].Allowed) + }) + + t.Run("cross-namespace: mixed results across multiple namespaces", func(t *testing.T) { + newItemWithNamespace := func(ns, verb, group, resource, subresource, folder, name string) *authzv1.BatchCheckItem { + correlationID := fmt.Sprintf("%s-%s-%s-%s-%s", ns, group, resource, folder, name) + return &authzv1.BatchCheckItem{ + Namespace: ns, + Verb: verb, + Group: group, + Resource: resource, + Subresource: subresource, + Name: name, + Folder: folder, + CorrelationId: correlationID, + } + } + + // user:1 has specific access to dashboard 1 in folder 1 + // user:2 would have broader access, but we're testing user:1 + res, err := server.BatchCheck(newContextWithNamespace(), &authzv1.BatchCheckRequest{ + Subject: "user:1", + Checks: []*authzv1.BatchCheckItem{ + // Allowed in default namespace + newItemWithNamespace(namespace, utils.VerbGet, dashboardGroup, dashboardResource, "", "1", "1"), + // Denied in default namespace (user:1 doesn't have access to dashboard 2) + newItemWithNamespace(namespace, utils.VerbGet, dashboardGroup, dashboardResource, "", "2", "2"), + // Denied in other-namespace (no tuples) + newItemWithNamespace("other-namespace", utils.VerbGet, dashboardGroup, dashboardResource, "", "1", "1"), + }, + }) + require.NoError(t, err) + require.Len(t, res.Results, 3) + + assert.True(t, res.Results[fmt.Sprintf("%s-%s-%s-%s-%s", namespace, dashboardGroup, dashboardResource, "1", "1")].Allowed) + assert.False(t, res.Results[fmt.Sprintf("%s-%s-%s-%s-%s", namespace, dashboardGroup, dashboardResource, "2", "2")].Allowed) + assert.False(t, res.Results[fmt.Sprintf("%s-%s-%s-%s-%s", "other-namespace", dashboardGroup, dashboardResource, "1", "1")].Allowed) + }) } diff --git a/pkg/services/authz/zanzana/server/server_bench_test.go b/pkg/services/authz/zanzana/server/server_bench_test.go index 6ffed51990e..312429151da 100644 --- a/pkg/services/authz/zanzana/server/server_bench_test.go +++ b/pkg/services/authz/zanzana/server/server_bench_test.go @@ -588,9 +588,8 @@ func BenchmarkBatchCheck(b *testing.B) { // Helper to create batch check requests using the new authzv1 API newBatchCheckReq := func(subject string, items []*authzv1.BatchCheckItem) *authzv1.BatchCheckRequest { return &authzv1.BatchCheckRequest{ - Subject: subject, - Namespace: benchNamespace, - Checks: items, + Subject: subject, + Checks: items, } } @@ -600,6 +599,7 @@ func BenchmarkBatchCheck(b *testing.B) { for i := 0; i < batchCheckSize && i < len(resources); i++ { resource := resources[i] items = append(items, &authzv1.BatchCheckItem{ + Namespace: benchNamespace, Verb: utils.VerbGet, Group: benchDashboardGroup, Resource: benchDashboardResource, @@ -617,6 +617,7 @@ func BenchmarkBatchCheck(b *testing.B) { for _, folder := range folders { if folderDepths[folder] == depth && len(items) < batchCheckSize { items = append(items, &authzv1.BatchCheckItem{ + Namespace: benchNamespace, Verb: utils.VerbGet, Group: benchDashboardGroup, Resource: benchDashboardResource, @@ -630,6 +631,7 @@ func BenchmarkBatchCheck(b *testing.B) { for len(items) < batchCheckSize && len(folders) > 0 { folder := folders[len(items)%len(folders)] items = append(items, &authzv1.BatchCheckItem{ + Namespace: benchNamespace, Verb: utils.VerbGet, Group: benchDashboardGroup, Resource: benchDashboardResource, @@ -732,6 +734,7 @@ func BenchmarkBatchCheck(b *testing.B) { for i := 0; i < batchCheckSize; i++ { folder := data.folders[i%len(data.folders)] items = append(items, &authzv1.BatchCheckItem{ + Namespace: benchNamespace, Verb: utils.VerbGet, Group: benchDashboardGroup, Resource: benchDashboardResource, diff --git a/pkg/storage/unified/resource/access.go b/pkg/storage/unified/resource/access.go index c0ddd9e6613..90abbffcea4 100644 --- a/pkg/storage/unified/resource/access.go +++ b/pkg/storage/unified/resource/access.go @@ -152,6 +152,67 @@ func (c authzLimitedClient) Check(ctx context.Context, id claims.AuthInfo, req c return resp, nil } +// BatchCheck implements claims.AccessClient. +func (c authzLimitedClient) BatchCheck(ctx context.Context, id claims.AuthInfo, req claims.BatchCheckRequest) (claims.BatchCheckResponse, error) { + ctx, span := tracer.Start(ctx, "resource.authzLimitedClient.BatchCheck", trace.WithAttributes( + attribute.Int("num_checks", len(req.Checks)), + attribute.Bool("fallback_used", FallbackUsed(ctx)), + )) + defer span.End() + + if FallbackUsed(ctx) { + span.SetStatus(codes.Error, "BatchCheck not supported with fallback") + return claims.BatchCheckResponse{}, fmt.Errorf("BatchCheck not supported when fallback is used") + } + + // Filter checks to only those that require RBAC and validate namespace + rbacChecks := make([]claims.BatchCheckItem, 0, len(req.Checks)) + allowedByDefault := make(map[string]bool, len(req.Checks)) + + for _, check := range req.Checks { + if !claims.NamespaceMatches(id.GetNamespace(), check.Namespace) { + span.SetStatus(codes.Error, "Namespace mismatch") + span.RecordError(claims.ErrNamespaceMismatch) + return claims.BatchCheckResponse{}, claims.ErrNamespaceMismatch + } + + if c.IsCompatibleWithRBAC(check.Group, check.Resource) { + rbacChecks = append(rbacChecks, check) + } else { + allowedByDefault[check.CorrelationID] = true + } + } + + // If all checks are allowed by default, return early + if len(rbacChecks) == 0 { + results := make(map[string]claims.BatchCheckResult, len(req.Checks)) + for _, check := range req.Checks { + results[check.CorrelationID] = claims.BatchCheckResult{ + Allowed: true, + } + } + return claims.BatchCheckResponse{Results: results}, nil + } + + // Call the underlying client with RBAC checks + resp, err := c.client.BatchCheck(ctx, id, claims.BatchCheckRequest{Checks: rbacChecks}) + if err != nil { + c.logger.FromContext(ctx).Error("BatchCheck failed", "error", err, "num_checks", len(rbacChecks)) + span.SetStatus(codes.Error, fmt.Sprintf("batch check failed: %v", err)) + span.RecordError(err) + return resp, err + } + + // Merge results with allowed-by-default checks + for correlationID := range allowedByDefault { + resp.Results[correlationID] = claims.BatchCheckResult{ + Allowed: true, + } + } + + return resp, nil +} + // Compile implements claims.AccessClient. func (c authzLimitedClient) Compile(ctx context.Context, id claims.AuthInfo, req claims.ListRequest) (claims.ItemChecker, claims.Zookie, error) { t := time.Now() diff --git a/pkg/storage/unified/resource/access_test.go b/pkg/storage/unified/resource/access_test.go index 5d664aaba74..0ffd89cbc9d 100644 --- a/pkg/storage/unified/resource/access_test.go +++ b/pkg/storage/unified/resource/access_test.go @@ -159,6 +159,97 @@ func TestNamespaceMatching(t *testing.T) { } } +func TestAuthzLimitedClient_BatchCheck(t *testing.T) { + mockClient := authlib.FixedAccessClient(true) + client := NewAuthzLimitedClient(mockClient, AuthzOptions{}) + + t.Run("returns error when fallback is used", func(t *testing.T) { + ctx := WithFallback(context.Background()) + req := authlib.BatchCheckRequest{ + Checks: []authlib.BatchCheckItem{ + { + CorrelationID: "0", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: utils.VerbGet, + Namespace: "stacks-1", + Name: "test-dashboard", + }, + }, + } + + _, err := client.BatchCheck(ctx, &identity.StaticRequester{Namespace: "stacks-1"}, req) + require.Error(t, err) + assert.Contains(t, err.Error(), "fallback") + }) + + t.Run("works normally without fallback", func(t *testing.T) { + ctx := context.Background() + req := authlib.BatchCheckRequest{ + Checks: []authlib.BatchCheckItem{ + { + CorrelationID: "0", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: utils.VerbGet, + Namespace: "stacks-1", + Name: "test-dashboard", + }, + }, + } + + resp, err := client.BatchCheck(ctx, &identity.StaticRequester{Namespace: "stacks-1"}, req) + require.NoError(t, err) + require.Len(t, resp.Results, 1) + assert.True(t, resp.Results["0"].Allowed) + }) + + t.Run("returns error on namespace mismatch", func(t *testing.T) { + ctx := context.Background() + req := authlib.BatchCheckRequest{ + Checks: []authlib.BatchCheckItem{ + { + CorrelationID: "0", + Group: "dashboard.grafana.app", + Resource: "dashboards", + Verb: utils.VerbGet, + Namespace: "stacks-2", // Different namespace + Name: "test-dashboard", + }, + }, + } + + _, err := client.BatchCheck(ctx, &identity.StaticRequester{Namespace: "stacks-1"}, req) + require.Error(t, err) + assert.ErrorIs(t, err, authlib.ErrNamespaceMismatch) + }) + + t.Run("allows non-RBAC resources by default", func(t *testing.T) { + // Use a client that would deny if checked + denyClient := authlib.FixedAccessClient(false) + client := NewAuthzLimitedClient(denyClient, AuthzOptions{}) + + ctx := context.Background() + req := authlib.BatchCheckRequest{ + Checks: []authlib.BatchCheckItem{ + { + CorrelationID: "0", + Group: "unknown.group", + Resource: "unknown.resource", + Verb: utils.VerbGet, + Namespace: "stacks-1", + Name: "test", + }, + }, + } + + resp, err := client.BatchCheck(ctx, &identity.StaticRequester{Namespace: "stacks-1"}, req) + require.NoError(t, err) + require.Len(t, resp.Results, 1) + assert.True(t, resp.Results["0"].Allowed, "non-RBAC resources should be allowed by default") + }) +} + // TestNamespaceMatchingFallback tests namespace matching in Check and Compile methods when fallback is used func TestNamespaceMatchingFallback(t *testing.T) { // Create a mock client that always returns allowed=true