From 2b0ccc46a23ac8550f3d2267ece7656a0abe5b09 Mon Sep 17 00:00:00 2001 From: Ronen Hilewicz Date: Thu, 21 Mar 2024 18:10:44 -0400 Subject: [PATCH] Fix relation inversion --- graph/inverse_test.go | 1 - graph/objects_test.go | 5 +-- model/inverse.go | 73 ++++++++++++++++++++++++------------------- model/validate.go | 7 ++--- 4 files changed, 46 insertions(+), 40 deletions(-) diff --git a/graph/inverse_test.go b/graph/inverse_test.go index 88c32d0..9a03df4 100644 --- a/graph/inverse_test.go +++ b/graph/inverse_test.go @@ -121,7 +121,6 @@ func manifest(m *model.Model) (*v3.Manifest, error) { case p.IsExclusion(): terms = []*model.PermissionTerm{p.Exclusion.Include, p.Exclusion.Exclude} operator = " - " - } return name, strings.Join(lo.Map(terms, func(pt *model.PermissionTerm, _ int) string { diff --git a/graph/objects_test.go b/graph/objects_test.go index b5fb045..f2d5458 100644 --- a/graph/objects_test.go +++ b/graph/objects_test.go @@ -70,6 +70,7 @@ type searchTest struct { var searchObjectsTests = []searchTest{ // Relations + // {"doc:?#viewer@user:f1_viewer", []object{{"doc", "doc1"}, {"doc", "doc2"}, {"doc", "doc3"}}}, {"folder:?#owner@user:f1_owner", []object{{"folder", "folder1"}}}, {"folder:?#viewer@group:f1_viewers#member", []object{{"folder", "folder1"}}}, {"group:?#member@user:user2", []object{{"group", "d1_viewers"}}}, @@ -90,7 +91,7 @@ var searchObjectsTests = []searchTest{ // wildcard {"doc:?#viewer@user:user1", []object{{"doc", "doc1"}, {"doc", "doc2"}}}, {"doc:?#viewer@user:f1_owner", []object{{"doc", "doc1"}, {"doc", "doc2"}}}, - {"doc:?#viewer@user:user2", []object{{"doc", "doc2"}}}, + {"doc:?#viewer@user:user2", []object{{"doc", "doc1"}, {"doc", "doc2"}}}, {"doc:?#viewer@user:*", []object{{"doc", "doc2"}}}, // // Permissions @@ -103,7 +104,7 @@ var searchObjectsTests = []searchTest{ {"doc:?#can_write@user:f1_owner", []object{{"doc", "doc1"}, {"doc", "doc2"}, {"doc", "doc3"}}}, {"doc:?#can_read@user:f1_owner", []object{{"doc", "doc1"}, {"doc", "doc2"}, {"doc", "doc3"}}}, {"doc:?#can_share@user:f1_owner", []object{{"doc", "doc1"}, {"doc", "doc2"}, {"doc", "doc3"}}}, - {"doc:?#can_invite@user:f1_owner", []object{{"doc", "doc1"}, {"doc", "doc2"}, {"doc", "doc3"}}}, + {"doc:?#can_invite@user:f1_owner", []object{{"doc", "doc2"}, {"doc", "doc3"}}}, // {"folder:?#is_owner@group:f1_viewers", []object{}}, // {"folder:?#can_create_file@group:f1_viewers", []object{}}, // {"folder:?#can_read@group:f1_viewers", []object{{"folder", "folder1"}}}, diff --git a/model/inverse.go b/model/inverse.go index f07237b..0352b8a 100644 --- a/model/inverse.go +++ b/model/inverse.go @@ -13,7 +13,7 @@ func (m *Model) Invert() *Model { type inverter struct { m *Model im *Model - subst map[ObjectName]map[RelationName]RelationName + subst map[RelationName]RelationName } func newInverter(m *Model) *inverter { @@ -24,7 +24,7 @@ func newInverter(m *Model) *inverter { Objects: lo.MapValues(m.Objects, func(o *Object, _ ObjectName) *Object { return NewObject() }), Metadata: m.Metadata, }, - subst: map[ObjectName]map[RelationName]RelationName{}, + subst: map[RelationName]RelationName{}, } } @@ -33,21 +33,23 @@ func (i *inverter) invert() *Model { for rn, r := range o.Relations { i.invertRelation(on, rn, r) } + } + for on, o := range i.m.Objects { for pn, p := range o.Permissions { kind := kindOf(p) - ipn := i.iName(on, pn) + ipn := irel(on, pn) for _, pt := range p.Terms() { switch { case pt.IsArrow(): baseRel := o.Relations[pt.Base] - itip := i.iName(on, pt.Base) + itip := i.irelSub(on, pt.Base) for _, subj := range p.SubjectTypes { ip := permissionOrNew(i.im.Objects[subj], ipn, kind) for _, baseSubj := range baseRel.SubjectTypes { - ip.AddTerm(&PermissionTerm{Base: i.iName(baseSubj, pt.RelOrPerm), RelOrPerm: itip}) + ip.AddTerm(&PermissionTerm{Base: i.irelSub(baseSubj, pt.RelOrPerm), RelOrPerm: itip}) // create a subject relation to expand the recursive permission r := relationOrNew(i.im.Objects[baseSubj], itip) @@ -56,25 +58,26 @@ func (i *inverter) invert() *Model { } case o.HasRelation(pt.RelOrPerm): - for _, rr := range o.Relations[pt.RelOrPerm].Union { + r := o.Relations[pt.RelOrPerm] + for _, rr := range r.Union { ip := permissionOrNew(i.im.Objects[rr.Object], ipn, kind) - ip.AddTerm(&PermissionTerm{RelOrPerm: i.iName(on, pt.RelOrPerm)}) + ip.AddTerm(&PermissionTerm{RelOrPerm: i.irelSub(on, pt.RelOrPerm)}) - if rr.IsSubject() { - term := &PermissionTerm{Base: i.iName(rr.Object, rr.Relation), RelOrPerm: ipn} - ip.AddTerm(term) + // if rr.IsSubject() { + // term := &PermissionTerm{Base: i.irelSub(rr.Object, rr.Relation), RelOrPerm: ipn} + // ip.AddTerm(term) - for _, subj := range subjs(i.m.Objects[rr.Object], rr.Relation) { - ip = permissionOrNew(i.im.Objects[subj], ipn, kind) - ip.AddTerm(term) - } - } + // for _, subj := range subjs(i.m.Objects[rr.Object], rr.Relation) { + // ip = permissionOrNew(i.im.Objects[subj], ipn, kind) + // ip.AddTerm(term) + // } + // } } case o.HasPermission(pt.RelOrPerm): for _, subj := range subjs(o, pt.RelOrPerm) { ip := permissionOrNew(i.im.Objects[subj], ipn, kind) - ip.AddTerm(&PermissionTerm{RelOrPerm: i.iName(on, pt.RelOrPerm)}) + ip.AddTerm(&PermissionTerm{RelOrPerm: i.irelSub(on, pt.RelOrPerm)}) } } } @@ -86,35 +89,39 @@ func (i *inverter) invert() *Model { func (i *inverter) invertRelation(on ObjectName, rn RelationName, r *Relation) { for _, rr := range r.Union { - irn := rrel(on, rn) + irn := irel(on, rn) i.im.Objects[rr.Object].Relations[irn] = &Relation{Union: []*RelationRef{{Object: on}}} - if rr.IsSubject() && rr.Object == on { + if rr.IsSubject() { // add a synthetic permission to reverse the expansion of the subject relation + srel := i.m.Objects[rr.Object].Relations[rr.Relation] + subjects := lo.Uniq(append([]ObjectName{rr.Object}, srel.SubjectTypes...)) ipn := rsrel(on, rn) - for _, subj := range r.Union { - p := permissionOrNew(i.im.Objects[subj.Object], ipn, permissionKindUnion) + for _, subj := range subjects { + p := permissionOrNew(i.im.Objects[subj], ipn, permissionKindUnion) p.AddTerm(&PermissionTerm{RelOrPerm: irn}) - p.AddTerm(&PermissionTerm{Base: ipn, RelOrPerm: ipn}) - i.addSubstitution(subj.Object, rn, ipn) + rel := irel(rr.Object, rr.Relation) + base := lo.Ternary(rr.Object == on, rel, i.sub(rel)) + p.AddTerm(&PermissionTerm{Base: base, RelOrPerm: ipn}) + i.addSubstitution(irn, ipn) } } } } -func (i *inverter) iName(on ObjectName, rn RelationName) RelationName { - if pn, ok := i.subst[on][rn]; ok { +func (i *inverter) irelSub(on ObjectName, rn RelationName) RelationName { + return i.sub(irel(on, rn)) +} + +func (i *inverter) sub(rn RelationName) RelationName { + if pn, ok := i.subst[rn]; ok { return pn } - return rrel(on, rn) + return rn } -func (i *inverter) addSubstitution(on ObjectName, rn, pn RelationName) { - if i.subst[on] == nil { - i.subst[on] = map[RelationName]RelationName{} - } - - i.subst[on][rn] = pn +func (i *inverter) addSubstitution(rn, pn RelationName) { + i.subst[rn] = pn } type permissionKind int @@ -172,12 +179,12 @@ func permissionOrNew(o *Object, pn RelationName, kind permissionKind) *Permissio return p } -func rrel(on ObjectName, rn RelationName) RelationName { +func irel(on ObjectName, rn RelationName) RelationName { return RelationName(fmt.Sprintf("%s_%s", on, rn)) } func rsrel(on ObjectName, rn RelationName) RelationName { - return RelationName(fmt.Sprintf("r_%s", rrel(on, rn))) + return RelationName(fmt.Sprintf("r_%s", irel(on, rn))) } func subjs(o *Object, rn RelationName) []ObjectName { diff --git a/model/validate.go b/model/validate.go index e90d677..a5cc5ad 100644 --- a/model/validate.go +++ b/model/validate.go @@ -192,14 +192,13 @@ func (m *Model) resolveRelation(r *Relation, seen relSet) []ObjectName { subjectTypes := set.NewSet[ObjectName]() for _, rr := range r.Union { - switch { - case rr.IsSubject(): + subjectTypes.Add(rr.Object) + + if rr.IsSubject() { if !seen.Contains(*rr) { seen.Add(*rr) subjectTypes.Append(m.resolveRelation(m.Objects[rr.Object].Relations[rr.Relation], seen)...) } - default: - subjectTypes.Add(rr.Object) } } return subjectTypes.ToSlice()