diff --git a/pkg/services/authz/zanzana/server/server_mutate.go b/pkg/services/authz/zanzana/server/server_mutate.go index bd339534fe5..15a57404bbc 100644 --- a/pkg/services/authz/zanzana/server/server_mutate.go +++ b/pkg/services/authz/zanzana/server/server_mutate.go @@ -6,8 +6,10 @@ import ( "fmt" "time" - authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" + openfgav1 "github.com/openfga/api/proto/openfga/v1" "go.opentelemetry.io/otel/codes" + + authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" ) type OperationGroup string @@ -119,3 +121,65 @@ func groupByOperation(operations []*authzextv1.MutateOperation) (map[OperationGr return grouped, nil } + +func deduplicateTupleKeys(writeTuples []*openfgav1.TupleKey, deleteTuples []*openfgav1.TupleKeyWithoutCondition) ([]*openfgav1.TupleKey, []*openfgav1.TupleKeyWithoutCondition) { + deduplicatedWriteTuples := make([]*openfgav1.TupleKey, 0) + deduplicatedDeleteTuples := make([]*openfgav1.TupleKeyWithoutCondition, 0) + + writeTupleMap := make(map[string]bool) + + for _, writeTuple := range writeTuples { + id := getTupleKeyID(writeTuple) + if !writeTupleMap[id] { + writeTupleMap[id] = true + deduplicatedWriteTuples = append(deduplicatedWriteTuples, writeTuple) + } + } + + // Prioritize writes over deletes. Deletes do not have a condition, so we don't know if write tuple is different from delete one. + for _, deleteTuple := range deleteTuples { + id := getTupleKeyID(deleteTuple) + if !writeTupleMap[id] { + writeTupleMap[id] = true + deduplicatedDeleteTuples = append(deduplicatedDeleteTuples, deleteTuple) + } + } + + return deduplicatedWriteTuples, deduplicatedDeleteTuples +} + +func (s *Server) writeTuples(ctx context.Context, store *storeInfo, writeTuples []*openfgav1.TupleKey, deleteTuples []*openfgav1.TupleKeyWithoutCondition) error { + writeReq := &openfgav1.WriteRequest{ + StoreId: store.ID, + AuthorizationModelId: store.ModelID, + } + + writeTuples, deleteTuples = deduplicateTupleKeys(writeTuples, deleteTuples) + + if len(writeTuples) > 0 { + writeReq.Writes = &openfgav1.WriteRequestWrites{ + TupleKeys: writeTuples, + OnDuplicate: "ignore", + } + } + + if len(deleteTuples) > 0 { + writeReq.Deletes = &openfgav1.WriteRequestDeletes{ + TupleKeys: deleteTuples, + OnMissing: "ignore", + } + } + + _, err := s.openfga.Write(ctx, writeReq) + return err +} + +type TupleKey interface { + GetUser() string + GetRelation() string + GetObject() string +} + +func getTupleKeyID(t TupleKey) string { + return fmt.Sprintf("%s:%s:%s", t.GetUser(), t.GetRelation(), t.GetObject()) +} diff --git a/pkg/services/authz/zanzana/server/server_mutate_folder.go b/pkg/services/authz/zanzana/server/server_mutate_folder.go index 3d92347f404..6d07492b788 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_folder.go +++ b/pkg/services/authz/zanzana/server/server_mutate_folder.go @@ -52,24 +52,7 @@ func (s *Server) mutateFolders(ctx context.Context, store *storeInfo, operations return nil } - writeReq := &openfgav1.WriteRequest{ - StoreId: store.ID, - AuthorizationModelId: store.ModelID, - } - if len(writeTuples) > 0 { - writeReq.Writes = &openfgav1.WriteRequestWrites{ - TupleKeys: writeTuples, - OnDuplicate: "ignore", - } - } - if len(deleteTuples) > 0 { - writeReq.Deletes = &openfgav1.WriteRequestDeletes{ - TupleKeys: deleteTuples, - OnMissing: "ignore", - } - } - - _, err := s.openfga.Write(ctx, writeReq) + err := s.writeTuples(ctx, store, writeTuples, deleteTuples) if err != nil { s.logger.Error("failed to write folder tuples", "error", err) return err diff --git a/pkg/services/authz/zanzana/server/server_mutate_org_role.go b/pkg/services/authz/zanzana/server/server_mutate_org_role.go index bda9decb3d5..843c62859dd 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_org_role.go +++ b/pkg/services/authz/zanzana/server/server_mutate_org_role.go @@ -50,24 +50,7 @@ func (s *Server) mutateOrgRoles(ctx context.Context, store *storeInfo, operation return nil } - writeReq := &openfgav1.WriteRequest{ - StoreId: store.ID, - AuthorizationModelId: store.ModelID, - } - if len(writeTuples) > 0 { - writeReq.Writes = &openfgav1.WriteRequestWrites{ - TupleKeys: writeTuples, - OnDuplicate: "ignore", - } - } - if len(deleteTuples) > 0 { - writeReq.Deletes = &openfgav1.WriteRequestDeletes{ - TupleKeys: deleteTuples, - OnMissing: "ignore", - } - } - - _, err := s.openfga.Write(ctx, writeReq) + err := s.writeTuples(ctx, store, writeTuples, deleteTuples) if err != nil { s.logger.Error("failed to write user org role tuples", "error", err) return err diff --git a/pkg/services/authz/zanzana/server/server_mutate_resourcepermissions.go b/pkg/services/authz/zanzana/server/server_mutate_resourcepermissions.go index fa8b5467235..f85f31900f6 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_resourcepermissions.go +++ b/pkg/services/authz/zanzana/server/server_mutate_resourcepermissions.go @@ -47,24 +47,7 @@ func (s *Server) mutateResourcePermissions(ctx context.Context, store *storeInfo } } - writeReq := &openfgav1.WriteRequest{ - StoreId: store.ID, - AuthorizationModelId: store.ModelID, - } - if len(writeTuples) > 0 { - writeReq.Writes = &openfgav1.WriteRequestWrites{ - TupleKeys: writeTuples, - OnDuplicate: "ignore", - } - } - if len(deleteTuples) > 0 { - writeReq.Deletes = &openfgav1.WriteRequestDeletes{ - TupleKeys: deleteTuples, - OnMissing: "ignore", - } - } - - _, err := s.openfga.Write(ctx, writeReq) + err := s.writeTuples(ctx, store, writeTuples, deleteTuples) if err != nil { s.logger.Error("failed to write resource permission tuples", "error", err) return err diff --git a/pkg/services/authz/zanzana/server/server_mutate_rolebindings.go b/pkg/services/authz/zanzana/server/server_mutate_rolebindings.go index 3b18566bee2..faf23d1f1ed 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_rolebindings.go +++ b/pkg/services/authz/zanzana/server/server_mutate_rolebindings.go @@ -44,24 +44,7 @@ func (s *Server) mutateRoleBindings(ctx context.Context, store *storeInfo, opera } } - writeReq := &openfgav1.WriteRequest{ - StoreId: store.ID, - AuthorizationModelId: store.ModelID, - } - if len(writeTuples) > 0 { - writeReq.Writes = &openfgav1.WriteRequestWrites{ - TupleKeys: writeTuples, - OnDuplicate: "ignore", - } - } - if len(deleteTuples) > 0 { - writeReq.Deletes = &openfgav1.WriteRequestDeletes{ - TupleKeys: deleteTuples, - OnMissing: "ignore", - } - } - - _, err := s.openfga.Write(ctx, writeReq) + err := s.writeTuples(ctx, store, writeTuples, deleteTuples) if err != nil { s.logger.Error("failed to write resource role binding tuples", "error", err) return err diff --git a/pkg/services/authz/zanzana/server/server_mutate_roles.go b/pkg/services/authz/zanzana/server/server_mutate_roles.go index 4c19b1fd288..c0471fdbf17 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_roles.go +++ b/pkg/services/authz/zanzana/server/server_mutate_roles.go @@ -41,24 +41,7 @@ func (s *Server) mutateRoles(ctx context.Context, store *storeInfo, operations [ } } - writeReq := &openfgav1.WriteRequest{ - StoreId: store.ID, - AuthorizationModelId: store.ModelID, - } - if len(writeTuples) > 0 { - writeReq.Writes = &openfgav1.WriteRequestWrites{ - TupleKeys: writeTuples, - OnDuplicate: "ignore", - } - } - if len(deleteTuples) > 0 { - writeReq.Deletes = &openfgav1.WriteRequestDeletes{ - TupleKeys: deleteTuples, - OnMissing: "ignore", - } - } - - _, err := s.openfga.Write(ctx, writeReq) + err := s.writeTuples(ctx, store, writeTuples, deleteTuples) if err != nil { s.logger.Error("failed to write resource role binding tuples", "error", err) return err diff --git a/pkg/services/authz/zanzana/server/server_mutate_teambindings.go b/pkg/services/authz/zanzana/server/server_mutate_teambindings.go index 81e1c9cb437..96690bb96d8 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_teambindings.go +++ b/pkg/services/authz/zanzana/server/server_mutate_teambindings.go @@ -43,24 +43,7 @@ func (s *Server) mutateTeamBindings(ctx context.Context, store *storeInfo, opera } } - writeReq := &openfgav1.WriteRequest{ - StoreId: store.ID, - AuthorizationModelId: store.ModelID, - } - if len(writeTuples) > 0 { - writeReq.Writes = &openfgav1.WriteRequestWrites{ - TupleKeys: writeTuples, - OnDuplicate: "ignore", - } - } - if len(deleteTuples) > 0 { - writeReq.Deletes = &openfgav1.WriteRequestDeletes{ - TupleKeys: deleteTuples, - OnMissing: "ignore", - } - } - - _, err := s.openfga.Write(ctx, writeReq) + err := s.writeTuples(ctx, store, writeTuples, deleteTuples) if err != nil { s.logger.Error("failed to write resource role binding tuples", "error", err) return err diff --git a/pkg/services/authz/zanzana/server/server_mutate_test.go b/pkg/services/authz/zanzana/server/server_mutate_test.go index 70dc1ea2fb8..c1fcfabbe43 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_test.go +++ b/pkg/services/authz/zanzana/server/server_mutate_test.go @@ -5,6 +5,7 @@ import ( openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" v1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" @@ -133,3 +134,66 @@ func testMutate(t *testing.T, srv *Server) { require.Len(t, res.Tuples, 0) }) } + +func TestDeduplicateTupleKeys(t *testing.T) { + t.Run("should deduplicate write tuples", func(t *testing.T) { + writeTuples := []*openfgav1.TupleKey{ + {User: "user:1", Relation: "get", Object: "object:1"}, + {User: "user:1", Relation: "get", Object: "object:2"}, + } + deleteTuples := []*openfgav1.TupleKeyWithoutCondition{ + {User: "user:1", Relation: "get", Object: "object:1"}, + {User: "user:2", Relation: "get", Object: "object:2"}, + } + + deduplicatedWriteTuples, deduplicatedDeleteTuples := deduplicateTupleKeys(writeTuples, deleteTuples) + require.Len(t, deduplicatedWriteTuples, 2) + require.ElementsMatch(t, deduplicatedWriteTuples, []*openfgav1.TupleKey{ + {User: "user:1", Relation: "get", Object: "object:1"}, + {User: "user:1", Relation: "get", Object: "object:2"}, + }) + + require.Len(t, deduplicatedDeleteTuples, 1) + require.ElementsMatch(t, deduplicatedDeleteTuples, []*openfgav1.TupleKeyWithoutCondition{ + {User: "user:2", Relation: "get", Object: "object:2"}, + }) + }) + + t.Run("should deduplicate write tuples with conditions", func(t *testing.T) { + writeTuples := []*openfgav1.TupleKey{ + {User: "user:1", Relation: "get", Object: "object:1", Condition: &openfgav1.RelationshipCondition{Name: "condition:1", Context: &structpb.Struct{Fields: map[string]*structpb.Value{ + "field:1": structpb.NewStringValue("value:1"), + }}}}, + {User: "user:1", Relation: "get", Object: "object:2"}, + } + deleteTuples := []*openfgav1.TupleKeyWithoutCondition{ + {User: "user:1", Relation: "get", Object: "object:1"}, + } + + deduplicatedWriteTuples, deduplicatedDeleteTuples := deduplicateTupleKeys(writeTuples, deleteTuples) + require.Len(t, deduplicatedWriteTuples, 2) + require.ElementsMatch(t, deduplicatedWriteTuples, []*openfgav1.TupleKey{ + {User: "user:1", Relation: "get", Object: "object:1", Condition: &openfgav1.RelationshipCondition{Name: "condition:1", Context: &structpb.Struct{Fields: map[string]*structpb.Value{ + "field:1": structpb.NewStringValue("value:1"), + }}}}, + {User: "user:1", Relation: "get", Object: "object:2"}, + }) + + require.Len(t, deduplicatedDeleteTuples, 0) + }) + + t.Run("should do nothing for no duplicates", func(t *testing.T) { + writeTuples := []*openfgav1.TupleKey{ + {User: "user:1", Relation: "get", Object: "object:1"}, + } + deleteTuples := []*openfgav1.TupleKeyWithoutCondition{ + {User: "user:2", Relation: "get", Object: "object:2"}, + } + + deduplicatedWriteTuples, deduplicatedDeleteTuples := deduplicateTupleKeys(writeTuples, deleteTuples) + require.Len(t, deduplicatedWriteTuples, 1) + require.ElementsMatch(t, deduplicatedWriteTuples, writeTuples) + require.Len(t, deduplicatedDeleteTuples, 1) + require.ElementsMatch(t, deduplicatedDeleteTuples, deleteTuples) + }) +}