From dfa8f786d297bb8f7e310af1b9e6c1147a804bd4 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 31 Oct 2024 14:34:48 +0100 Subject: [PATCH] Zanzana: fix generic schema (#95648) * Change schema so that resource checks on a folder walks the tree --- .../accesscontrol/dualwrite/collectors.go | 32 ++++++++++++-- .../accesscontrol/dualwrite/reconciler.go | 10 ++++- .../dualwrite/resource_reconciler.go | 44 ++++++++++++------- pkg/services/authz/zanzana/common/tuple.go | 28 +++++++++--- .../authz/zanzana/schema/resource.fga | 31 +++++++------ pkg/services/authz/zanzana/server/server.go | 6 +-- .../authz/zanzana/server/server_check.go | 4 +- .../authz/zanzana/server/server_check_test.go | 14 ++++++ .../authz/zanzana/server/server_list.go | 7 ++- .../authz/zanzana/server/server_test.go | 3 ++ pkg/services/authz/zanzana/zanzana.go | 19 ++++++++ 11 files changed, 147 insertions(+), 51 deletions(-) diff --git a/pkg/services/accesscontrol/dualwrite/collectors.go b/pkg/services/accesscontrol/dualwrite/collectors.go index 26ba0bd9c41..2034e5606c8 100644 --- a/pkg/services/accesscontrol/dualwrite/collectors.go +++ b/pkg/services/accesscontrol/dualwrite/collectors.go @@ -110,7 +110,7 @@ func folderTreeCollector2(store db.DB) legacyTupleCollector { // managedPermissionsCollector collects managed permissions into provided tuple map. // It will only store actions that are supported by our schema. Managed permissions can // be directly mapped to user/team/role without having to write an intermediate role. -func managedPermissionsCollector2(store db.DB) legacyTupleCollector { +func managedPermissionsCollector2(store db.DB, kind string) legacyTupleCollector { return func(ctx context.Context) (map[string]map[string]*openfgav1.TupleKey, error) { query := ` SELECT u.uid as user_uid, t.uid as team_uid, p.action, p.kind, p.identifier, r.org_id @@ -122,6 +122,7 @@ func managedPermissionsCollector2(store db.DB) legacyTupleCollector { LEFT JOIN team t ON tr.team_id = t.id LEFT JOIN builtin_role br ON r.id = br.role_id WHERE r.name LIKE 'managed:%' + AND p.kind = ? ` type Permission struct { RoleName string `xorm:"role_name"` @@ -135,7 +136,7 @@ func managedPermissionsCollector2(store db.DB) legacyTupleCollector { var permissions []Permission err := store.WithDbSession(ctx, func(sess *db.Session) error { - return sess.SQL(query).Find(&permissions) + return sess.SQL(query, kind).Find(&permissions) }) if err != nil { @@ -164,6 +165,19 @@ func managedPermissionsCollector2(store db.DB) legacyTupleCollector { tuples[tuple.Object] = make(map[string]*openfgav1.TupleKey) } + // For resource actions on folders we need to merge the tuples into one with combined + // group_resources. + if zanzana.IsFolderResourceTuple(tuple) { + key := tupleStringWithoutCondition(tuple) + if t, ok := tuples[tuple.Object][key]; ok { + zanzana.MergeFolderResourceTuples(t, tuple) + } else { + tuples[tuple.Object][key] = tuple + } + + continue + } + tuples[tuple.Object][tuple.String()] = tuple } @@ -171,6 +185,14 @@ func managedPermissionsCollector2(store db.DB) legacyTupleCollector { } } +func tupleStringWithoutCondition(tuple *openfgav1.TupleKey) string { + c := tuple.Condition + tuple.Condition = nil + s := tuple.String() + tuple.Condition = c + return s +} + func zanzanaCollector(client zanzana.Client, relations []string) zanzanaTupleCollector { return func(ctx context.Context, client zanzana.Client, object string) (map[string]*openfgav1.TupleKey, error) { // list will use continuation token to collect all tuples for object and relation @@ -213,7 +235,11 @@ func zanzanaCollector(client zanzana.Client, relations []string) zanzanaTupleCol return nil, err } for _, t := range tuples { - out[t.Key.String()] = t.Key + if zanzana.IsFolderResourceTuple(t.Key) { + out[tupleStringWithoutCondition(t.Key)] = t.Key + } else { + out[t.Key.String()] = t.Key + } } } diff --git a/pkg/services/accesscontrol/dualwrite/reconciler.go b/pkg/services/accesscontrol/dualwrite/reconciler.go index 4a1a3cc16cd..0392ddbd992 100644 --- a/pkg/services/accesscontrol/dualwrite/reconciler.go +++ b/pkg/services/accesscontrol/dualwrite/reconciler.go @@ -72,8 +72,14 @@ func NewZanzanaReconciler(client zanzana.Client, store db.DB, lock *serverlock.S client, ), newResourceReconciler( - "managed permissison", - managedPermissionsCollector2(store), + "managed folder permissions", + managedPermissionsCollector2(store, zanzana.KindFolders), + zanzanaCollector(client, zanzana.Folder2Relations), + client, + ), + newResourceReconciler( + "managed dashboard permissions", + managedPermissionsCollector2(store, zanzana.KindDashboards), zanzanaCollector(client, zanzana.ResourceRelations), client, ), diff --git a/pkg/services/accesscontrol/dualwrite/resource_reconciler.go b/pkg/services/accesscontrol/dualwrite/resource_reconciler.go index 9d1cbb9a24b..32cc1160225 100644 --- a/pkg/services/accesscontrol/dualwrite/resource_reconciler.go +++ b/pkg/services/accesscontrol/dualwrite/resource_reconciler.go @@ -4,8 +4,9 @@ import ( "context" "fmt" - "github.com/grafana/grafana/pkg/services/authz/zanzana" openfgav1 "github.com/openfga/api/proto/openfga/v1" + + "github.com/grafana/grafana/pkg/services/authz/zanzana" ) // legacyTupleCollector collects tuples groupd by object and tupleKey @@ -47,13 +48,25 @@ func (r resourceReconciler) reconcile(ctx context.Context) error { // 3. Check if tuples from grafana db exists in zanzana and if not add them to writes for key, t := range tuples { - _, ok := zanzanaTuples[key] + stored, ok := zanzanaTuples[key] if !ok { writes = append(writes, t) + continue + } + + // 4. For folder resource tuples we also need to compare the stored group_resources + if zanzana.IsFolderResourceTuple(t) && t.String() != stored.String() { + deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ + User: t.User, + Relation: t.Relation, + Object: t.Object, + }) + + writes = append(writes, t) } } - // 4. Check if tuple from zanzana don't exists in grafana db, if not add them to deletes. + // 5. Check if tuple from zanzana don't exists in grafana db, if not add them to deletes. for key, tuple := range zanzanaTuples { _, ok := tuples[key] if !ok { @@ -70,19 +83,6 @@ func (r resourceReconciler) reconcile(ctx context.Context) error { return nil } - // FIXME: batch them together - if len(writes) > 0 { - err := batch(writes, 100, func(items []*openfgav1.TupleKey) error { - return r.client.Write(ctx, &openfgav1.WriteRequest{ - Writes: &openfgav1.WriteRequestWrites{TupleKeys: items}, - }) - }) - - if err != nil { - return err - } - } - if len(deletes) > 0 { err := batch(deletes, 100, func(items []*openfgav1.TupleKeyWithoutCondition) error { return r.client.Write(ctx, &openfgav1.WriteRequest{ @@ -95,5 +95,17 @@ func (r resourceReconciler) reconcile(ctx context.Context) error { } } + if len(writes) > 0 { + err := batch(writes, 100, func(items []*openfgav1.TupleKey) error { + return r.client.Write(ctx, &openfgav1.WriteRequest{ + Writes: &openfgav1.WriteRequestWrites{TupleKeys: items}, + }) + }) + + if err != nil { + return err + } + } + return nil } diff --git a/pkg/services/authz/zanzana/common/tuple.go b/pkg/services/authz/zanzana/common/tuple.go index 11e3be3b732..7ed1e50e2d0 100644 --- a/pkg/services/authz/zanzana/common/tuple.go +++ b/pkg/services/authz/zanzana/common/tuple.go @@ -9,10 +9,14 @@ import ( const ( resourceType = "resource" + resourceTypeFolder = "folder2" namespaceType = "namespace" - folderResourceType = "folder_resource" ) +func FolderResourceRelation(relation string) string { + return fmt.Sprintf("%s_%s", resourceType, relation) +} + func NewTypedIdent(typ string, name string) string { return fmt.Sprintf("%s:%s", typ, name) } @@ -21,8 +25,8 @@ func NewResourceIdent(group, resource, name string) string { return fmt.Sprintf("%s:%s/%s", resourceType, FormatGroupResource(group, resource), name) } -func NewFolderResourceIdent(group, resource, folder string) string { - return fmt.Sprintf("%s:%s/%s", folderResourceType, FormatGroupResource(group, resource), folder) +func NewFolderIdent(name string) string { + return fmt.Sprintf("folder2:%s", name) } func NewNamespaceResourceIdent(group, resource string) string { @@ -52,13 +56,15 @@ func NewResourceTuple(subject, relation, group, resource, name string) *openfgav func NewFolderResourceTuple(subject, relation, group, resource, folder string) *openfgav1.TupleKey { return &openfgav1.TupleKey{ User: subject, - Relation: relation, - Object: NewFolderResourceIdent(group, resource, folder), + Relation: FolderResourceRelation(relation), + Object: NewFolderIdent(folder), Condition: &openfgav1.RelationshipCondition{ - Name: "group_filter", + Name: "folder_group_filter", Context: &structpb.Struct{ Fields: map[string]*structpb.Value{ - "resource_group": structpb.NewStringValue(FormatGroupResource(group, resource)), + "group_resources": structpb.NewListValue(&structpb.ListValue{ + Values: []*structpb.Value{structpb.NewStringValue(FormatGroupResource(group, resource))}, + }), }, }, }, @@ -73,6 +79,14 @@ func NewNamespaceResourceTuple(subject, relation, group, resource string) *openf } } +func NewFolderParentTuple(folder, parent string) *openfgav1.TupleKey { + return &openfgav1.TupleKey{ + Object: NewFolderIdent(folder), + Relation: "parent", + User: NewFolderIdent(parent), + } +} + func NewFolderTuple(subject, relation, name string) *openfgav1.TupleKey { return NewTypedTuple("folder2", subject, relation, name) } diff --git a/pkg/services/authz/zanzana/schema/resource.fga b/pkg/services/authz/zanzana/schema/resource.fga index d329a85d004..010412c9a73 100644 --- a/pkg/services/authz/zanzana/schema/resource.fga +++ b/pkg/services/authz/zanzana/schema/resource.fga @@ -18,9 +18,9 @@ type folder2 define parent: [folder2] # Action sets - define view: [user, team#member, role#assignee] or edit - define edit: [user, team#member, role#assignee] or admin - define admin: [user, team#member, role#assignee] + define view: [user, team#member, role#assignee] or edit or view from parent + define edit: [user, team#member, role#assignee] or admin or edit from parent + define admin: [user, team#member, role#assignee] or admin from parent define read: [user, team#member, role#assignee] or view or read from parent define create: [user, team#member, role#assignee] or edit or create from parent @@ -29,18 +29,16 @@ type folder2 define permissions_read: [user, team#member, role#assignee] or admin or permissions_read from parent define permissions_write: [user, team#member, role#assignee] or admin or permissions_write from parent -type folder_resource - relations - define view: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or edit - define edit: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or admin - define admin: [user with group_filter, team#member with group_filter, role#assignee with group_filter] + define resource_view: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_edit or resource_view from parent + define resource_edit: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_admin or resource_edit from parent + define resource_admin: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_admin from parent - define read: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or view - define create: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or edit - define write: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or edit - define delete: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or edit - define permissions_read: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or admin - define permissions_write: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or admin + define resource_read: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_view or resource_read from parent + define resource_create: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_edit or resource_create from parent + define resource_write: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_edit or resource_write from parent + define resource_delete: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_edit or resource_delete from parent + define resource_permissions_read: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_admin or resource_permissions_read from parent + define resource_permissions_write: [user with folder_group_filter, team#member with folder_group_filter, role#assignee with folder_group_filter] or resource_admin or resource_permissions_write from parent type resource relations @@ -55,7 +53,12 @@ type resource define permissions_read: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or admin define permissions_write: [user with group_filter, team#member with group_filter, role#assignee with group_filter] or admin + condition group_filter(requested_group: string, resource_group: string) { resource_group == requested_group } +condition folder_group_filter(requested_group: string, group_resources: list) { + requested_group in group_resources +} + diff --git a/pkg/services/authz/zanzana/server/server.go b/pkg/services/authz/zanzana/server/server.go index 4aa25b11130..ea36e8da56c 100644 --- a/pkg/services/authz/zanzana/server/server.go +++ b/pkg/services/authz/zanzana/server/server.go @@ -17,9 +17,9 @@ import ( ) const ( - resourceType = "resource" - namespaceType = "namespace" - folderResourceType = "folder_resource" + resourceType = "resource" + namespaceType = "namespace" + folderTypePrefix = "folder2:" ) var _ authzv1.AuthzServiceServer = (*Server)(nil) diff --git a/pkg/services/authz/zanzana/server/server_check.go b/pkg/services/authz/zanzana/server/server_check.go index 436c5a6c4b5..a5ea5823a6c 100644 --- a/pkg/services/authz/zanzana/server/server_check.go +++ b/pkg/services/authz/zanzana/server/server_check.go @@ -113,8 +113,8 @@ func (s *Server) checkGeneric(ctx context.Context, r *authzv1.CheckRequest) (*au AuthorizationModelId: s.modelID, TupleKey: &openfgav1.CheckRequestTupleKey{ User: r.GetSubject(), - Relation: relation, - Object: common.NewFolderResourceIdent(r.GetGroup(), r.GetResource(), r.GetFolder()), + Relation: common.FolderResourceRelation(relation), + Object: common.NewFolderIdent(r.GetFolder()), }, Context: &structpb.Struct{ Fields: map[string]*structpb.Value{ diff --git a/pkg/services/authz/zanzana/server/server_check_test.go b/pkg/services/authz/zanzana/server/server_check_test.go index a51a27391ea..d4982c9de62 100644 --- a/pkg/services/authz/zanzana/server/server_check_test.go +++ b/pkg/services/authz/zanzana/server/server_check_test.go @@ -93,4 +93,18 @@ func testCheck(t *testing.T, server *Server) { require.NoError(t, err) assert.True(t, res.GetAllowed()) }) + + t.Run("user:8 should be able to read all resoruce:dashboard.grafana.app/dashboar in folder 6 through folder 5", func(t *testing.T) { + res, err := server.Check(context.Background(), newRead("user:8", dashboardGroup, dashboardResource, "6", "10")) + require.NoError(t, err) + assert.True(t, res.GetAllowed()) + + res, err = server.Check(context.Background(), newRead("user:8", dashboardGroup, dashboardResource, "5", "11")) + require.NoError(t, err) + assert.True(t, res.GetAllowed()) + + res, err = server.Check(context.Background(), newRead("user:8", folderGroup, folderResource, "4", "12")) + require.NoError(t, err) + assert.False(t, res.GetAllowed()) + }) } diff --git a/pkg/services/authz/zanzana/server/server_list.go b/pkg/services/authz/zanzana/server/server_list.go index f02249d6764..520f3f4ea5b 100644 --- a/pkg/services/authz/zanzana/server/server_list.go +++ b/pkg/services/authz/zanzana/server/server_list.go @@ -85,8 +85,8 @@ func (s *Server) listGeneric(ctx context.Context, r *authzextv1.ListRequest) (*a folders, err := s.openfga.ListObjects(ctx, &openfgav1.ListObjectsRequest{ StoreId: s.storeID, AuthorizationModelId: s.modelID, - Type: "folder_resource", - Relation: relation, + Type: "folder2", + Relation: common.FolderResourceRelation(relation), User: r.GetSubject(), Context: &structpb.Struct{ Fields: map[string]*structpb.Value{ @@ -138,9 +138,8 @@ func directObjects(group, resource string, objects []string) []string { } func folderObject(group, resource string, objects []string) []string { - prefix := fmt.Sprintf("%s:%s/%s/", folderResourceType, group, resource) for i := range objects { - objects[i] = strings.TrimPrefix(objects[i], prefix) + objects[i] = strings.TrimPrefix(objects[i], folderTypePrefix) } return objects } diff --git a/pkg/services/authz/zanzana/server/server_test.go b/pkg/services/authz/zanzana/server/server_test.go index 2383d2bf5ff..0ff323bf96e 100644 --- a/pkg/services/authz/zanzana/server/server_test.go +++ b/pkg/services/authz/zanzana/server/server_test.go @@ -75,6 +75,9 @@ func setup(t *testing.T, testDB db.DB, cfg *setting.Cfg) *Server { common.NewFolderResourceTuple("user:5", "view", dashboardGroup, dashboardResource, "1"), common.NewFolderTuple("user:6", "read", "1"), common.NewNamespaceResourceTuple("user:7", "read", folderGroup, folderResource), + common.NewFolderParentTuple("5", "4"), + common.NewFolderParentTuple("6", "5"), + common.NewFolderResourceTuple("user:8", "read", dashboardGroup, dashboardResource, "5"), }, }, }) diff --git a/pkg/services/authz/zanzana/zanzana.go b/pkg/services/authz/zanzana/zanzana.go index 8349f691e19..4d838174dbd 100644 --- a/pkg/services/authz/zanzana/zanzana.go +++ b/pkg/services/authz/zanzana/zanzana.go @@ -35,9 +35,18 @@ const ( RelationDelete string = "delete" RelationPermissionsRead string = "permissions_read" RelationPermissionsWrite string = "permissions_write" + + FolderResourceRelationAdmin string = "resource_admin" + FolderResourceRelationRead string = "resource_read" + FolderResourceRelationWrite string = "resource_write" + FolderResourceRelationCreate string = "resource_create" + FolderResourceRelationDelete string = "resource_delete" + FolderResourceRelationPermissionsRead string = "resource_permissions_read" + FolderResourceRelationPermissionsWrite string = "resource_permissions_write" ) var ResourceRelations = []string{RelationRead, RelationWrite, RelationCreate, RelationDelete, RelationPermissionsRead, RelationPermissionsWrite} +var Folder2Relations = append(ResourceRelations, FolderResourceRelationRead, FolderResourceRelationWrite, FolderResourceRelationCreate, FolderResourceRelationDelete, FolderResourceRelationPermissionsRead, FolderResourceRelationPermissionsWrite) const ( KindOrg string = "org" @@ -130,6 +139,16 @@ func TranslateToResourceTuple(subject string, action, kind, name string) (*openf return common.NewTypedTuple(translation.typ, subject, m.relation, name), true } +func IsFolderResourceTuple(t *openfgav1.TupleKey) bool { + return strings.HasPrefix(t.Object, TypeFolder2) && strings.HasPrefix(t.Relation, "resource_") +} + +func MergeFolderResourceTuples(a, b *openfgav1.TupleKey) { + va := a.Condition.Context.Fields["group_resources"] + vb := b.Condition.Context.Fields["group_resources"] + va.GetListValue().Values = append(va.GetListValue().Values, vb.GetListValue().Values...) +} + func TranslateToOrgTuple(user string, action string, orgID int64) (*openfgav1.TupleKey, bool) { typeTranslation, ok := actionKindTranslations[KindOrg] if !ok {