Zanzana: Optimize batch check (#96669)

* Restructure check code so we only check namespace access once for each GroupResource during for batch

pkg/services/authz/zanzana/common/info.go

@@ -11,14 +11,14 @@ type TypeInfo struct {
 }
 
 var typedResources = map[string]TypeInfo{
-	NewNamespaceResourceIdent(
+	FormatGroupResource(
 		folderalpha1.FolderResourceInfo.GroupResource().Group,
 		folderalpha1.FolderResourceInfo.GroupResource().Resource,
 	): {Type: "folder"},
 }
 
 func GetTypeInfo(group, resource string) (TypeInfo, bool) {
-	info, ok := typedResources[NewNamespaceResourceIdent(group, resource)]
+	info, ok := typedResources[FormatGroupResource(group, resource)]
 	return info, ok
 }
 

pkg/services/authz/zanzana/server/server.go

@@ -37,8 +37,8 @@ type Server struct {
 }
 
 type storeInfo struct {
-	Id                   string
+	ID      string
-	AuthorizationModelId string
+	ModelID string
 }
 
 type ServerOption func(s *Server)

pkg/services/authz/zanzana/server/server_batch_check.go

@@ -17,48 +17,58 @@ func (s *Server) BatchCheck(ctx context.Context, r *authzextv1.BatchCheckRequest
 		Groups: make(map[string]*authzextv1.BatchCheckGroupResource),
 	}
 
-	subject := r.GetSubject()
+	store, err := s.getStoreInfo(ctx, r.GetNamespace())
+	if err != nil {
+		return nil, err
+	}
 
-	for _, item := range r.Items {
+	groupResourceAccess := make(map[string]bool)
-		groupPrefix := common.FormatGroupResource(item.GetGroup(), item.GetResource())
+
-		allowed, err := s.batchCheckItem(ctx, subject, r.Namespace, item)
+	for _, item := range r.GetItems() {
+		res, err := s.batchCheckItem(ctx, r, item, store, groupResourceAccess)
 		if err != nil {
 			return nil, err
 		}
 
-		if _, ok := batchRes.Groups[groupPrefix]; !ok {
+		groupResource := common.FormatGroupResource(item.GetGroup(), item.GetResource())
-			batchRes.Groups[groupPrefix] = &authzextv1.BatchCheckGroupResource{
+		if _, ok := batchRes.Groups[groupResource]; !ok {
+			batchRes.Groups[groupResource] = &authzextv1.BatchCheckGroupResource{
 				Items: make(map[string]bool),
 			}
 		}
-		batchRes.Groups[groupPrefix].Items[item.GetName()] = allowed
+		batchRes.Groups[groupResource].Items[item.GetName()] = res.GetAllowed()
 	}
 
 	return batchRes, nil
 }
 
-func (s *Server) batchCheckItem(ctx context.Context, subject string, namespace string, item *authzextv1.BatchCheckItem) (bool, error) {
+func (s *Server) batchCheckItem(
-	req := &authzv1.CheckRequest{
+	ctx context.Context,
-		Namespace:   namespace,
+	r *authzextv1.BatchCheckRequest,
-		Subject:     subject,
+	item *authzextv1.BatchCheckItem,
-		Verb:        item.GetVerb(),
+	store *storeInfo,
-		Group:       item.GetGroup(),
+	groupResourceAccess map[string]bool,
-		Resource:    item.GetResource(),
+) (*authzv1.CheckResponse, error) {
-		Name:        item.GetName(),
+	var (
-		Folder:      item.GetFolder(),
+		relation      = common.VerbMapping[item.GetVerb()]
-		Subresource: item.GetSubresource(),
+		groupResource = common.FormatGroupResource(item.GetGroup(), item.GetResource())
+	)
+
+	allowed, ok := groupResourceAccess[groupResource]
+	if !ok {
+		res, err := s.checkNamespace(ctx, r.GetSubject(), relation, item.GetGroup(), item.GetResource(), store)
+		if err != nil {
+			return nil, err
+		}
+		groupResourceAccess[groupResource] = res.GetAllowed()
+	}
+
+	if allowed {
+		return &authzv1.CheckResponse{Allowed: true}, nil
 	}
 
-	var res *authzv1.CheckResponse
-	var err error
 	if info, ok := common.GetTypeInfo(item.GetGroup(), item.GetResource()); ok {
-		res, err = s.checkTyped(ctx, req, info)
+		return s.checkTyped(ctx, r.GetSubject(), relation, item.GetName(), info, store)
-	} else {
-		res, err = s.checkGeneric(ctx, req)
 	}
-	if err != nil {
+	return s.checkGeneric(ctx, r.GetSubject(), relation, item.GetGroup(), item.GetResource(), item.GetName(), item.GetFolder(), store)
-		return false, err
-	}
-
-	return res.Allowed, nil
 }

pkg/services/authz/zanzana/server/server_check.go

@@ -14,28 +14,39 @@ func (s *Server) Check(ctx context.Context, r *authzv1.CheckRequest) (*authzv1.C
 	ctx, span := tracer.Start(ctx, "authzServer.Check")
 	defer span.End()
 
-	if info, ok := common.GetTypeInfo(r.GetGroup(), r.GetResource()); ok {
+	store, err := s.getStoreInfo(ctx, r.GetNamespace())
-		return s.checkTyped(ctx, r, info)
-	}
-	return s.checkGeneric(ctx, r)
-}
-
-// checkNamespace checks if subject has access through namespace
-func (s *Server) checkNamespace(ctx context.Context, r *authzv1.CheckRequest) (*authzv1.CheckResponse, error) {
-	storeInf, err := s.getStoreInfo(ctx, r.Namespace)
 	if err != nil {
 		return nil, err
 	}
 
 	relation := common.VerbMapping[r.GetVerb()]
 
+	// Check if subject has access through namespace
+	res, err := s.checkNamespace(ctx, r.GetSubject(), relation, r.GetGroup(), r.GetResource(), store)
+	if err != nil {
+		return nil, err
+	}
+
+	if res.GetAllowed() {
+		return res, nil
+	}
+
+	if info, ok := common.GetTypeInfo(r.GetGroup(), r.GetResource()); ok {
+		return s.checkTyped(ctx, r.GetSubject(), relation, r.GetName(), info, store)
+	}
+	return s.checkGeneric(ctx, r.GetSubject(), relation, r.GetGroup(), r.GetResource(), r.GetName(), r.GetFolder(), store)
+}
+
+// checkTyped performes check on the root "namespace". If subject has access through the namespace they have access to
+// every resource for that "GroupResource".
+func (s *Server) checkNamespace(ctx context.Context, subject, relation, group, resource string, store *storeInfo) (*authzv1.CheckResponse, error) {
 	res, err := s.openfga.Check(ctx, &openfgav1.CheckRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              store.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: store.ModelID,
 		TupleKey: &openfgav1.CheckRequestTupleKey{
-			User:     r.GetSubject(),
+			User:     subject,
 			Relation: relation,
-			Object:   common.NewNamespaceResourceIdent(r.GetGroup(), r.GetResource()),
+			Object:   common.NewNamespaceResourceIdent(group, resource),
 		},
 	})
 	if err != nil {
@@ -45,22 +56,16 @@ func (s *Server) checkNamespace(ctx context.Context, r *authzv1.CheckRequest) (*
 	return &authzv1.CheckResponse{Allowed: res.GetAllowed()}, nil
 }
 
-func (s *Server) checkTyped(ctx context.Context, r *authzv1.CheckRequest, info common.TypeInfo) (*authzv1.CheckResponse, error) {
+// checkTyped performes checks on our typed resources e.g. folder.
-	storeInf, err := s.getStoreInfo(ctx, r.Namespace)
+func (s *Server) checkTyped(ctx context.Context, subject, relation, name string, info common.TypeInfo, store *storeInfo) (*authzv1.CheckResponse, error) {
-	if err != nil {
+	// Check if subject has direct access to resource
-		return nil, err
-	}
-
-	relation := common.VerbMapping[r.GetVerb()]
-
-	// 1. check if subject has direct access to resource
 	res, err := s.openfga.Check(ctx, &openfgav1.CheckRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              store.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: store.ModelID,
 		TupleKey: &openfgav1.CheckRequestTupleKey{
-			User:     r.GetSubject(),
+			User:     subject,
 			Relation: relation,
-			Object:   common.NewTypedIdent(info.Type, r.GetName()),
+			Object:   common.NewTypedIdent(info.Type, name),
 		},
 	})
 	if err != nil {
@@ -71,40 +76,30 @@ func (s *Server) checkTyped(ctx context.Context, r *authzv1.CheckRequest, info c
 		return &authzv1.CheckResponse{Allowed: true}, nil
 	}
 
-	// 2. check if subject has access through namespace
+	return &authzv1.CheckResponse{Allowed: false}, nil
-	nsRes, err := s.checkNamespace(ctx, r)
-	if err != nil {
-		return nil, err
-	}
-
-	return &authzv1.CheckResponse{Allowed: nsRes.GetAllowed()}, nil
 }
 
-func (s *Server) checkGeneric(ctx context.Context, r *authzv1.CheckRequest) (*authzv1.CheckResponse, error) {
+// checkGeneric check our generic "resource" type.
-	storeInf, err := s.getStoreInfo(ctx, r.Namespace)
+func (s *Server) checkGeneric(ctx context.Context, subject, relation, group, resource, name, folder string, store *storeInfo) (*authzv1.CheckResponse, error) {
-	if err != nil {
+	groupResource := structpb.NewStringValue(common.FormatGroupResource(group, resource))
-		return nil, err
-	}
 
-	relation := common.VerbMapping[r.GetVerb()]
+	// Check if subject has direct access to resource
-	// 1. check if subject has direct access to resource
 	res, err := s.openfga.Check(ctx, &openfgav1.CheckRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              store.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: store.ModelID,
 		TupleKey: &openfgav1.CheckRequestTupleKey{
-			User:     r.GetSubject(),
+			User:     subject,
 			Relation: relation,
-			Object:   common.NewResourceIdent(r.GetGroup(), r.GetResource(), r.GetName()),
+			Object:   common.NewResourceIdent(group, resource, name),
 		},
 		Context: &structpb.Struct{
 			Fields: map[string]*structpb.Value{
-				"requested_group": structpb.NewStringValue(common.FormatGroupResource(r.GetGroup(), r.GetResource())),
+				"requested_group": groupResource,
 			},
 		},
 	})
 
 	if err != nil {
-		// FIXME: wrap error
 		return nil, err
 	}
 
@@ -112,32 +107,22 @@ func (s *Server) checkGeneric(ctx context.Context, r *authzv1.CheckRequest) (*au
 		return &authzv1.CheckResponse{Allowed: true}, nil
 	}
 
-	// 2. check if subject has access through namespace
+	if folder == "" {
-	nsRes, err := s.checkNamespace(ctx, r)
-	if err != nil {
-		return nil, err
-	}
-
-	if nsRes.GetAllowed() {
-		return &authzv1.CheckResponse{Allowed: true}, nil
-	}
-
-	if r.Folder == "" {
 		return &authzv1.CheckResponse{Allowed: false}, nil
 	}
 
-	// 3. check if subject has access as a sub resource for the folder
+	// Check if subject has access as a sub resource for the folder
 	res, err = s.openfga.Check(ctx, &openfgav1.CheckRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              store.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: store.ModelID,
 		TupleKey: &openfgav1.CheckRequestTupleKey{
-			User:     r.GetSubject(),
+			User:     subject,
 			Relation: common.FolderResourceRelation(relation),
-			Object:   common.NewFolderIdent(r.GetFolder()),
+			Object:   common.NewFolderIdent(folder),
 		},
 		Context: &structpb.Struct{
 			Fields: map[string]*structpb.Value{
-				"requested_group": structpb.NewStringValue(common.FormatGroupResource(r.GetGroup(), r.GetResource())),
+				"requested_group": groupResource,
 			},
 		},
 	})

pkg/services/authz/zanzana/server/server_list.go

@@ -16,31 +16,22 @@ func (s *Server) List(ctx context.Context, r *authzextv1.ListRequest) (*authzext
 	ctx, span := tracer.Start(ctx, "authzServer.List")
 	defer span.End()
 
-	if info, ok := common.GetTypeInfo(r.GetGroup(), r.GetResource()); ok {
+	store, err := s.getStoreInfo(ctx, r.Namespace)
-		return s.listTyped(ctx, r, info)
-	}
-
-	return s.listGeneric(ctx, r)
-}
-
-func (s *Server) listTyped(ctx context.Context, r *authzextv1.ListRequest, info common.TypeInfo) (*authzextv1.ListResponse, error) {
-	storeInf, err := s.getStoreInfo(ctx, r.Namespace)
 	if err != nil {
 		return nil, err
 	}
 
 	relation := common.VerbMapping[r.GetVerb()]
 
-	// 1. check if subject has access through namespace because then they can read all of them
+	res, err := s.checkNamespace(
-	res, err := s.openfga.Check(ctx, &openfgav1.CheckRequest{
+		ctx,
-		StoreId:              storeInf.Id,
+		r.GetSubject(),
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		relation,
-		TupleKey: &openfgav1.CheckRequestTupleKey{
+		r.GetGroup(),
-			User:     r.GetSubject(),
+		r.GetResource(),
-			Relation: relation,
+		store,
-			Object:   common.NewNamespaceResourceIdent(r.GetGroup(), r.GetResource()),
+	)
-		},
+
-	})
 	if err != nil {
 		return nil, err
 	}
@@ -49,13 +40,21 @@ func (s *Server) listTyped(ctx context.Context, r *authzextv1.ListRequest, info
 		return &authzextv1.ListResponse{All: true}, nil
 	}
 
-	// 2. List all resources user has access too
+	if info, ok := common.GetTypeInfo(r.GetGroup(), r.GetResource()); ok {
+		return s.listTyped(ctx, r.GetSubject(), relation, info, store)
+	}
+
+	return s.listGeneric(ctx, r.GetSubject(), relation, r.GetGroup(), r.GetResource(), store)
+}
+
+func (s *Server) listTyped(ctx context.Context, subject, relation string, info common.TypeInfo, store *storeInfo) (*authzextv1.ListResponse, error) {
+	// List all resources user has access too
 	listRes, err := s.openfga.ListObjects(ctx, &openfgav1.ListObjectsRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              store.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: store.ModelID,
 		Type:                 info.Type,
 		Relation:             relation,
-		User:                 r.GetSubject(),
+		User:                 subject,
 	})
 	if err != nil {
 		return nil, err
@@ -66,42 +65,19 @@ func (s *Server) listTyped(ctx context.Context, r *authzextv1.ListRequest, info
 	}, nil
 }
 
-func (s *Server) listGeneric(ctx context.Context, r *authzextv1.ListRequest) (*authzextv1.ListResponse, error) {
+func (s *Server) listGeneric(ctx context.Context, subject, relation, group, resource string, store *storeInfo) (*authzextv1.ListResponse, error) {
-	storeInf, err := s.getStoreInfo(ctx, r.Namespace)
+	groupResource := structpb.NewStringValue(common.FormatGroupResource(group, resource))
-	if err != nil {
-		return nil, err
-	}
 
-	relation := common.VerbMapping[r.GetVerb()]
+	// 1. List all folders subject has access to resource type in
-
-	// 1. check if subject has access through namespace because then they can read all of them
-	res, err := s.openfga.Check(ctx, &openfgav1.CheckRequest{
-		StoreId:              storeInf.Id,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
-		TupleKey: &openfgav1.CheckRequestTupleKey{
-			User:     r.GetSubject(),
-			Relation: relation,
-			Object:   common.NewNamespaceResourceIdent(r.GetGroup(), r.GetResource()),
-		},
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	if res.Allowed {
-		return &authzextv1.ListResponse{All: true}, nil
-	}
-
-	// 2. List all folders subject has access to resource type in
 	folders, err := s.openfga.ListObjects(ctx, &openfgav1.ListObjectsRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              store.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: store.ModelID,
 		Type:                 common.TypeFolder,
 		Relation:             common.FolderResourceRelation(relation),
-		User:                 r.GetSubject(),
+		User:                 subject,
 		Context: &structpb.Struct{
 			Fields: map[string]*structpb.Value{
-				"requested_group": structpb.NewStringValue(common.FormatGroupResource(r.GetGroup(), r.GetResource())),
+				"requested_group": groupResource,
 			},
 		},
 	})
@@ -109,16 +85,16 @@ func (s *Server) listGeneric(ctx context.Context, r *authzextv1.ListRequest) (*a
 		return nil, err
 	}
 
-	// 3. List all resource directly assigned to subject
+	// 2. List all resource directly assigned to subject
 	direct, err := s.openfga.ListObjects(ctx, &openfgav1.ListObjectsRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              store.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: store.ModelID,
 		Type:                 common.TypeResource,
 		Relation:             relation,
-		User:                 r.GetSubject(),
+		User:                 subject,
 		Context: &structpb.Struct{
 			Fields: map[string]*structpb.Value{
-				"requested_group": structpb.NewStringValue(common.FormatGroupResource(r.GetGroup(), r.GetResource())),
+				"requested_group": groupResource,
 			},
 		},
 	})
@@ -128,7 +104,7 @@ func (s *Server) listGeneric(ctx context.Context, r *authzextv1.ListRequest) (*a
 
 	return &authzextv1.ListResponse{
 		Folders: folderObject(folders.GetObjects()),
-		Items:   directObjects(r.GetGroup(), r.GetResource(), direct.GetObjects()),
+		Items:   directObjects(group, resource, direct.GetObjects()),
 	}, nil
 }
 

pkg/services/authz/zanzana/server/server_read.go

@@ -19,7 +19,7 @@ func (s *Server) Read(ctx context.Context, req *authzextv1.ReadRequest) (*authze
 	}
 
 	res, err := s.openfga.Read(ctx, &openfgav1.ReadRequest{
-		StoreId: storeInf.Id,
+		StoreId: storeInf.ID,
 		TupleKey: &openfgav1.ReadRequestTupleKey{
 			User:     req.GetTupleKey().GetUser(),
 			Relation: req.GetTupleKey().GetRelation(),

pkg/services/authz/zanzana/server/server_store.go

@@ -30,8 +30,8 @@ func (s *Server) getStoreInfo(ctx context.Context, namespace string) (*storeInfo
 	}
 
 	info = storeInfo{
-		Id:                   store.GetId(),
+		ID:      store.GetId(),
-		AuthorizationModelId: modelID,
+		ModelID: modelID,
 	}
 
 	s.stores[namespace] = info

pkg/services/authz/zanzana/server/server_test.go

@@ -71,8 +71,8 @@ func setup(t *testing.T, testDB db.DB, cfg *setting.Cfg) *Server {
 
 	// seed tuples
 	_, err = openfga.Write(context.Background(), &openfgav1.WriteRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              storeInf.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: storeInf.ModelID,
 		Writes: &openfgav1.WriteRequestWrites{
 			TupleKeys: []*openfgav1.TupleKey{
 				common.NewResourceTuple("user:1", "read", dashboardGroup, dashboardResource, "1"),

pkg/services/authz/zanzana/server/server_write.go

@@ -29,8 +29,8 @@ func (s *Server) Write(ctx context.Context, req *authzextv1.WriteRequest) (*auth
 	}
 
 	writeReq := &openfgav1.WriteRequest{
-		StoreId:              storeInf.Id,
+		StoreId:              storeInf.ID,
-		AuthorizationModelId: storeInf.AuthorizationModelId,
+		AuthorizationModelId: storeInf.ModelID,
 	}
 	if len(writeTuples) > 0 {
 		writeReq.Writes = &openfgav1.WriteRequestWrites{