From 13d3c0cec95e9ddeb45d7bebe60181e53f991db0 Mon Sep 17 00:00:00 2001 From: Ronen Hilewicz Date: Mon, 4 Dec 2023 17:55:11 -0500 Subject: [PATCH] Resolve relations and detect cycles. --- cache/expand.go | 20 +- cache/metadata.go | 8 +- cache/path_test.go | 8 +- go.mod | 1 + go.sum | 2 + model/model.go | 239 +++++++++++------- model/model_test.go | 200 ++++++++++----- model/testdata/cycles.yaml | 35 +++ ...ing_object.yaml => undefined_targets.yaml} | 15 +- parser/parse.go | 4 +- parser/parser_test.go | 14 +- parser/permission_visitor.go | 16 +- parser/relation_visitor.go | 17 +- v2/load.go | 17 +- v3/load.go | 6 +- 15 files changed, 394 insertions(+), 208 deletions(-) create mode 100644 model/testdata/cycles.yaml rename model/testdata/{rel_to_missing_object.yaml => undefined_targets.yaml} (59%) diff --git a/cache/expand.go b/cache/expand.go index a2c567c..60c1362 100644 --- a/cache/expand.go +++ b/cache/expand.go @@ -21,18 +21,18 @@ func (c *Cache) ExpandRelation(on model.ObjectName, rn model.RelationName) []mod } // get relation set for given object:relation. - rs := c.model.Objects[on].Relations[rn] + r := c.model.Objects[on].Relations[rn] // include given permission in result set results = append(results, rn) // iterate through relation set, determine if it "unions" with the given relation. - for _, r := range rs { + for _, rt := range r.Union { switch { - case r.Subject != nil && r.Subject.Object == on: - results = append(results, r.Subject.Relation) - case r.Direct != "": - results = append(results, c.ExpandRelation(on, model.RelationName(r.Direct))...) + case rt.Subject != nil && rt.Subject.Object == on: + results = append(results, rt.Subject.Relation) + case rt.Direct != "": + results = append(results, c.ExpandRelation(on, model.RelationName(rt.Direct))...) } } @@ -70,7 +70,7 @@ func (c *Cache) ExpandPermission(on model.ObjectName, pn model.PermissionName) [ } // convert union []string to []model.RelationName. -func (c *Cache) expandUnion(o *model.Object, u ...*model.RelationRef) []model.RelationName { +func (c *Cache) expandUnion(o *model.Object, u ...*model.PermissionRef) []model.RelationName { result := []model.RelationName{} for _, ref := range u { if ref.Base != "" { @@ -79,12 +79,12 @@ func (c *Cache) expandUnion(o *model.Object, u ...*model.RelationRef) []model.Re rn := model.RelationName(ref.RelOrPerm) result = append(result, rn) - exp := lo.FilterMap(o.Relations[rn], func(r *model.Relation, _ int) (*model.RelationRef, bool) { + exp := lo.FilterMap(o.Relations[rn].Union, func(r *model.RelationTerm, _ int) (*model.PermissionRef, bool) { if r.Direct == "" { - return &model.RelationRef{}, false + return &model.PermissionRef{}, false } _, ok := o.Relations[model.RelationName(r.Direct)] - return &model.RelationRef{RelOrPerm: string(r.Direct)}, ok + return &model.PermissionRef{RelOrPerm: string(r.Direct)}, ok }) result = append(result, c.expandUnion(o, exp...)...) diff --git a/cache/metadata.go b/cache/metadata.go index 5718f53..cc7018b 100644 --- a/cache/metadata.go +++ b/cache/metadata.go @@ -97,7 +97,7 @@ func (c *Cache) GetRelationType(objectType, relation string) (*dsc2.RelationType func (*Cache) getRelationPermissions(o *model.Object, rn model.RelationName) []string { permissions := []string{} for pn, p := range o.Permissions { - union := lo.Map(p.Union, func(r *model.RelationRef, _ int) string { + union := lo.Map(p.Union, func(r *model.PermissionRef, _ int) string { if r.Base != "" { panic("arrow permissions not supported yet") } @@ -112,9 +112,9 @@ func (*Cache) getRelationPermissions(o *model.Object, rn model.RelationName) []s func (*Cache) getRelationUnions(o *model.Object, on model.ObjectName, rn model.RelationName) []string { unions := []string{} - for name, rs := range o.Relations { - for _, r := range rs { - if r.Subject != nil && r.Subject.Object == on && r.Subject.Relation == rn { + for name, r := range o.Relations { + for _, rt := range r.Union { + if rt.Subject != nil && rt.Subject.Object == on && rt.Subject.Relation == rn { unions = append(unions, string(name)) } } diff --git a/cache/path_test.go b/cache/path_test.go index 0fa79ff..f749044 100644 --- a/cache/path_test.go +++ b/cache/path_test.go @@ -153,12 +153,12 @@ func expandPerm(m *model.Model, on model.ObjectName, pn model.PermissionName) [] func expandRel(m *model.Model, on model.ObjectName, rn model.RelationName) []*model.ObjectRelation { result := []*model.ObjectRelation{} - relations, ok := m.Objects[on].Relations[rn] + relation, ok := m.Objects[on].Relations[rn] if !ok { return result } - for _, r := range relations { + for _, r := range relation.Union { if r.Direct != "" { result = append(result, &model.ObjectRelation{ Object: r.Direct, @@ -188,10 +188,10 @@ func resolve(m *model.Model, on model.ObjectName, rn model.RelationName) *model. if strings.Contains(rn.String(), v3.ArrowIdentifier) { parts := strings.Split(rn.String(), v3.ArrowIdentifier) - rn := model.RelationName(parts[0]) + rn = model.RelationName(parts[0]) if _, ok := m.Objects[on].Relations[rn]; ok { // if c.RelationExists(on, rn) { - for _, rel := range m.Objects[on].Relations[rn] { + for _, rel := range m.Objects[on].Relations[rn].Union { if rel.Direct != "" { return &model.ObjectRelation{ Object: rel.Direct, diff --git a/go.mod b/go.mod index fc2008b..1326204 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/aserto-dev/go-aserto v0.30.0 github.com/aserto-dev/go-directory v0.30.2 github.com/davecgh/go-spew v1.1.1 + github.com/deckarep/golang-set/v2 v2.5.0 github.com/golang/mock v1.6.0 github.com/hashicorp/go-multierror v1.1.1 github.com/magefile/mage v1.15.0 diff --git a/go.sum b/go.sum index fb97588..2328af6 100644 --- a/go.sum +++ b/go.sum @@ -22,6 +22,8 @@ github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGX github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/deckarep/golang-set/v2 v2.5.0 h1:hn6cEZtQ0h3J8kFrHR/NrzyOoTnjgW1+FmNJzQ7y/sA= +github.com/deckarep/golang-set/v2 v2.5.0/go.mod h1:VAky9rY/yGXJOLEDv3OMci+7wtDpOF4IN+y82NBOac4= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= diff --git a/model/model.go b/model/model.go index bed9df6..0f0474b 100644 --- a/model/model.go +++ b/model/model.go @@ -10,6 +10,7 @@ import ( "github.com/aserto-dev/azm/graph" "github.com/aserto-dev/azm/model/diff" "github.com/aserto-dev/go-directory/pkg/derr" + set "github.com/deckarep/golang-set/v2" "github.com/hashicorp/go-multierror" "github.com/samber/lo" ) @@ -48,36 +49,57 @@ func (pn PermissionName) RN() RelationName { } type Object struct { - Relations map[RelationName][]*Relation `json:"relations,omitempty"` + Relations map[RelationName]*Relation `json:"relations,omitempty"` Permissions map[PermissionName]*Permission `json:"permissions,omitempty"` } +func (o *Object) HasRelOrPerm(name string) bool { + if o == nil { + return false + } + if _, foundRelation := o.Relations[RelationName(name)]; foundRelation { + return true + } + _, foundPermission := o.Permissions[PermissionName(name)] + return foundPermission +} + type Relation struct { + Union []*RelationTerm `json:"union,omitempty"` + SubjectTypes []ObjectName `json:"subject_types,omitempty"` +} + +type RelationTerm struct { Direct ObjectName `json:"direct,omitempty"` Subject *SubjectRelation `json:"subject,omitempty"` Wildcard ObjectName `json:"wildcard,omitempty"` } -type SubjectRelation struct { +type RelationRef struct { Object ObjectName `json:"object,omitempty"` Relation RelationName `json:"relation,omitempty"` } +type SubjectRelation struct { + *RelationRef + SubjectTypes []ObjectName `json:"subject_types,omitempty"` +} + type Permission struct { - Union []*RelationRef `json:"union,omitempty"` - Intersection []*RelationRef `json:"intersection,omitempty"` + Union []*PermissionRef `json:"union,omitempty"` + Intersection []*PermissionRef `json:"intersection,omitempty"` Exclusion *ExclusionPermission `json:"exclusion,omitempty"` } -type RelationRef struct { +type PermissionRef struct { Base RelationName `json:"base,omitempty"` RelOrPerm string `json:"rel_or_perm"` BaseTypes []ObjectName `json:"base_types,omitempty"` } type ExclusionPermission struct { - Include *RelationRef `json:"include,omitempty"` - Exclude *RelationRef `json:"exclude,omitempty"` + Include *PermissionRef `json:"include,omitempty"` + Exclude *PermissionRef `json:"exclude,omitempty"` } type ArrowPermission struct { @@ -115,7 +137,7 @@ func (m *Model) GetGraph() *graph.Graph { } for objectName, obj := range m.Objects { for relName, rel := range obj.Relations { - for _, rl := range rel { + for _, rl := range rel.Union { if string(rl.Direct) != "" { grph.AddEdge(string(objectName), string(rl.Direct), string(relName)) } else if rl.Subject != nil { @@ -154,67 +176,77 @@ func (m *Model) Diff(newModel *Model) *diff.Diff { // Validate enforces the model's internal consistency. // // It enforces the following rules: -// - A relation cannot share the same name as an object. // - Within an object, a permission cannot share the same name as a relation. // - Direct relations must reference existing objects . // - Wildcard relations must reference existing objects. // - Subject relations must reference existing object#relation pairs. // - Arrow permissions (relation->rel_or_perm) must reference existing relations/permissions. func (m *Model) Validate() error { - if err := m.validateUniqueNames(); err != nil { - // if there are name collisions, we can't validate relations or permissions. - return err + // Pass 1 (syntax): ensure no name collisions and all relations reference existing objects/relations. + if err := m.validateReferences(); err != nil { + return derr.ErrInvalidArgument.Err(err) } + // Pass 2: resolve all relations to a set of possible subject types. + if err := m.resolveRelations(); err != nil { + return derr.ErrInvalidArgument.Err(err) + } + + // if err := m.resolvePermissions(); err != nil { + // return derr.ErrInvalidArgument.Err(err) + // } + + return nil +} + +func (m *Model) validateReferences() error { var errs error + for on, o := range m.Objects { - if err := m.validateObjectRels(on, o); err != nil { + validatePerms := true + if err := m.validateUniqueNames(on, o); err != nil { errs = multierror.Append(errs, err) - - // if there are relation errors, we can't validate permissions. - continue + validatePerms = false } - if err := m.validateObjectPerms(on, o); err != nil { + if err := m.validateObjectRels(on, o); err != nil { errs = multierror.Append(errs, err) } - } - if errs != nil { - return derr.ErrInvalidArgument.Err(errs) + if validatePerms { + if err := m.validateObjectPerms(on, o); err != nil { + errs = multierror.Append(errs, err) + } + } } - return nil + + return errs } -func (m *Model) validateUniqueNames() error { - var errs error +func (m *Model) validateUniqueNames(on ObjectName, o *Object) error { + rels := lo.Map(lo.Keys(o.Relations), func(rn RelationName, _ int) string { + return string(rn) + }) + perms := lo.Map(lo.Keys(o.Permissions), func(pn PermissionName, _ int) string { + return string(pn) + }) - for on, o := range m.Objects { - rels := lo.Map(lo.Keys(o.Relations), func(rn RelationName, _ int) string { - return string(rn) - }) - perms := lo.Map(lo.Keys(o.Permissions), func(pn PermissionName, _ int) string { - return string(pn) - }) - - rpCollisions := lo.Intersect(rels, perms) - for _, collision := range rpCollisions { - errs = multierror.Append(errs, derr.ErrInvalidPermission.Msgf( - "permission name '%[1]s:%[2]s' conflicts with '%[1]s:%[2]s' relation", on, collision), - ) - } - } + rpCollisions := lo.Intersect(rels, perms) - if errs != nil { - return derr.ErrInvalidArgument.Err(errs) + var errs error + for _, collision := range rpCollisions { + errs = multierror.Append(errs, derr.ErrInvalidPermission.Msgf( + "permission name '%[1]s:%[2]s' conflicts with '%[1]s:%[2]s' relation", on, collision), + ) } - return nil + + return errs } func (m *Model) validateObjectRels(on ObjectName, o *Object) error { var errs error for rn, rs := range o.Relations { - for _, r := range rs { + for _, r := range rs.Union { switch { case r.Direct != "": if _, ok := m.Objects[r.Direct]; !ok { @@ -251,7 +283,7 @@ func (m *Model) validateObjectRels(on ObjectName, o *Object) error { func (m *Model) validateObjectPerms(on ObjectName, o *Object) error { var errs error for pn, p := range o.Permissions { - var refs []*RelationRef + var refs []*PermissionRef switch { case p.Union != nil: @@ -259,73 +291,102 @@ func (m *Model) validateObjectPerms(on ObjectName, o *Object) error { case p.Intersection != nil: refs = p.Intersection case p.Exclusion != nil: - refs = []*RelationRef{p.Exclusion.Include, p.Exclusion.Exclude} + refs = []*PermissionRef{p.Exclusion.Include, p.Exclusion.Exclude} } for _, ref := range refs { - var bases []ObjectName switch ref.Base { case "": - bases = []ObjectName{on} + if !o.HasRelOrPerm(ref.RelOrPerm) { + errs = multierror.Append(errs, derr.ErrInvalidPermission.Msgf( + "permission '%s:%s' references undefined relation or permission '%s:%s'", on, pn, on, ref.RelOrPerm), + ) + } default: - rel := o.Relations[ref.Base] - if rel == nil { + // validate that base relation exists on this object type. + // at this stage we don't yet resolve the relation to a set of subject types. + if _, hasRelation := o.Relations[ref.Base]; !hasRelation { errs = multierror.Append(errs, derr.ErrInvalidPermission.Msgf( "permission '%s:%s' references undefined relation type '%s:%s'", on, pn, on, ref.Base), ) - continue - } - - for _, r := range rel { - bases = lo.Uniq(append(bases, m.relationSubjectTypes(r)...)) } } - for _, base := range bases { - _, foundRelation := m.Objects[base].Relations[RelationName(ref.RelOrPerm)] - _, foundPermission := m.Objects[base].Permissions[PermissionName(ref.RelOrPerm)] - if !(foundRelation || foundPermission) { - switch base { - case on: - errs = multierror.Append(errs, derr.ErrInvalidPermission.Msgf( - "permission '%s:%s' references undefined relation or permission '%s:%s'", on, pn, base, ref.RelOrPerm), - ) - default: - arrow := fmt.Sprintf("%s->%s", ref.Base, ref.RelOrPerm) - errs = multierror.Append(errs, derr.ErrInvalidPermission.Msgf( - "permission '%s:%s' references '%s', which can resolve to undefined relation or permission '%s:%s' ", - on, pn, arrow, base, ref.RelOrPerm, - )) - } - - continue - } - } + // for _, base := range bases { + // _, foundRelation := m.Objects[base].Relations[RelationName(ref.RelOrPerm)] + // _, foundPermission := m.Objects[base].Permissions[PermissionName(ref.RelOrPerm)] + // if !(foundRelation || foundPermission) { + // switch base { + // case on: + // errs = multierror.Append(errs, derr.ErrInvalidPermission.Msgf( + // "permission '%s:%s' references undefined relation or permission '%s:%s'", on, pn, base, ref.RelOrPerm), + // ) + // default: + // arrow := fmt.Sprintf("%s->%s", ref.Base, ref.RelOrPerm) + // errs = multierror.Append(errs, derr.ErrInvalidPermission.Msgf( + // "permission '%s:%s' references '%s', which can resolve to undefined relation or permission '%s:%s' ", + // on, pn, arrow, base, ref.RelOrPerm, + // )) + // } + + // continue + // } + // } + + // ref.BaseTypes = bases + } + } + + return errs +} - ref.BaseTypes = bases +func (m *Model) resolveRelations() error { + var errs error + for on, o := range m.Objects { + for rn, r := range o.Relations { + seen := set.NewSet(RelationRef{Object: on, Relation: rn}) + subs := m.resolveRelation(r, seen) + switch len(subs) { + case 0: + errs = multierror.Append(errs, derr.ErrInvalidRelation.Msgf( + "relation '%s:%s' is circular and does not resolve to any object types", on, rn), + ) + default: + r.SubjectTypes = subs + } } } return errs } -// relationSubjectTypes returns a list of all object types that can be the subject of the given relation. -func (m *Model) relationSubjectTypes(r *Relation) []ObjectName { - switch { - case r.Direct != "": - return []ObjectName{r.Direct} - case r.Wildcard != "": - return []ObjectName{r.Wildcard} - case r.Subject != nil: - subjRel := m.Objects[r.Subject.Object].Relations[r.Subject.Relation] - return lo.Uniq( - lo.FlatMap(subjRel, func(r *Relation, _ int) []ObjectName { - return m.relationSubjectTypes(r) - }), - ) +type RelSet set.Set[RelationRef] + +func (m *Model) resolveRelation(r *Relation, seen RelSet) []ObjectName { + if len(r.SubjectTypes) > 0 { + // already resolved + return r.SubjectTypes } - return []ObjectName{} + subjectTypes := []ObjectName{} + for _, rt := range r.Union { + switch { + case rt.Direct != "": + subjectTypes = append(subjectTypes, rt.Direct) + case rt.Wildcard != "": + subjectTypes = append(subjectTypes, rt.Wildcard) + case rt.Subject != nil: + if !seen.Contains(*rt.Subject.RelationRef) { + seen.Add(*rt.Subject.RelationRef) + subjectTypes = append(subjectTypes, m.resolveRelation( + m.Objects[rt.Subject.Object].Relations[rt.Subject.Relation], + seen)..., + ) + } + + } + } + return subjectTypes } func (m *Model) subtract(newModel *Model) *diff.Changes { diff --git a/model/model_test.go b/model/model_test.go index f475e19..6dba5a8 100644 --- a/model/model_test.go +++ b/model/model_test.go @@ -1,6 +1,7 @@ package model_test import ( + "bytes" "encoding/json" "os" "testing" @@ -8,6 +9,7 @@ import ( "github.com/aserto-dev/azm/model" v3 "github.com/aserto-dev/azm/v3" "github.com/aserto-dev/go-directory/pkg/derr" + "github.com/hashicorp/go-multierror" "github.com/nsf/jsondiff" stretch "github.com/stretchr/testify/require" ) @@ -17,59 +19,68 @@ var m1 = model.Model{ Objects: map[model.ObjectName]*model.Object{ model.ObjectName("user"): {}, model.ObjectName("group"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("member"): { - &model.Relation{Direct: model.ObjectName("user")}, - &model.Relation{Subject: &model.SubjectRelation{ - Object: model.ObjectName("group"), - Relation: model.RelationName("member"), - }}, + Union: []*model.RelationTerm{ + {Direct: model.ObjectName("user")}, + {Subject: &model.SubjectRelation{ + RelationRef: &model.RelationRef{ + Object: model.ObjectName("group"), + Relation: model.RelationName("member"), + }, + SubjectTypes: []model.ObjectName{}, + }}, + }, }, }, }, + model.ObjectName("folder"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("owner"): { - &model.Relation{Direct: model.ObjectName("user")}, + Union: []*model.RelationTerm{ + {Direct: model.ObjectName("user")}, + }, }, }, Permissions: map[model.PermissionName]*model.Permission{ model.PermissionName("read"): { - Union: []*model.RelationRef{{RelOrPerm: "owner"}}, + Union: []*model.PermissionRef{{RelOrPerm: "owner"}}, }, }, }, model.ObjectName("document"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("parent_folder"): { - {Direct: model.ObjectName("folder")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("folder")}}, }, model.RelationName("writer"): { - {Direct: model.ObjectName("user")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("user")}}, }, model.RelationName("reader"): { - {Direct: model.ObjectName("user")}, - {Wildcard: model.ObjectName("user")}, - }, + Union: []*model.RelationTerm{ + {Direct: model.ObjectName("user")}, + {Wildcard: model.ObjectName("user")}, + }}, }, Permissions: map[model.PermissionName]*model.Permission{ model.PermissionName("edit"): { - Union: []*model.RelationRef{{RelOrPerm: "writer"}}, + Union: []*model.PermissionRef{{RelOrPerm: "writer"}}, }, model.PermissionName("view"): { - Union: []*model.RelationRef{{RelOrPerm: "reader"}, {RelOrPerm: "writer"}}, + Union: []*model.PermissionRef{{RelOrPerm: "reader"}, {RelOrPerm: "writer"}}, }, model.PermissionName("read_and_write"): { - Intersection: []*model.RelationRef{{RelOrPerm: "reader"}, {RelOrPerm: "writer"}}, + Intersection: []*model.PermissionRef{{RelOrPerm: "reader"}, {RelOrPerm: "writer"}}, }, model.PermissionName("can_only_read"): { Exclusion: &model.ExclusionPermission{ - Include: &model.RelationRef{RelOrPerm: "reader"}, - Exclude: &model.RelationRef{RelOrPerm: "writer"}, + Include: &model.PermissionRef{RelOrPerm: "reader"}, + Exclude: &model.PermissionRef{RelOrPerm: "writer"}, }, }, model.PermissionName("read"): { - Union: []*model.RelationRef{{Base: "parent_folder", RelOrPerm: "read"}}, + Union: []*model.PermissionRef{{Base: "parent_folder", RelOrPerm: "read"}}, }, }, }, @@ -150,39 +161,46 @@ func TestDiff(t *testing.T) { Objects: map[model.ObjectName]*model.Object{ model.ObjectName("new_user"): {}, model.ObjectName("group"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("member"): { - &model.Relation{Direct: model.ObjectName("new_user")}, - &model.Relation{Subject: &model.SubjectRelation{ - Object: model.ObjectName("group"), - Relation: model.RelationName("member"), - }}, + Union: []*model.RelationTerm{ + {Direct: model.ObjectName("new_user")}, + {Subject: &model.SubjectRelation{ + RelationRef: &model.RelationRef{ + Object: model.ObjectName("group"), + Relation: model.RelationName("member"), + }, + SubjectTypes: []model.ObjectName{}, + }}, + }, }, }, }, model.ObjectName("folder"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("owner"): { - &model.Relation{Direct: model.ObjectName("new_user")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("new_user")}}, }, model.RelationName("viewer"): { - &model.Relation{Direct: model.ObjectName("new_user")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("new_user")}}, }, }, Permissions: map[model.PermissionName]*model.Permission{ model.PermissionName("read"): { - Union: []*model.RelationRef{{RelOrPerm: "owner"}}, + Union: []*model.PermissionRef{{RelOrPerm: "owner"}}, }, }, }, model.ObjectName("document"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("writer"): { - {Direct: model.ObjectName("new_user")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("new_user")}}, }, model.RelationName("reader"): { - {Direct: model.ObjectName("new_user")}, - {Wildcard: model.ObjectName("new_user")}, + Union: []*model.RelationTerm{ + {Direct: model.ObjectName("new_user")}, + {Wildcard: model.ObjectName("new_user")}, + }, }, }, }, @@ -231,42 +249,49 @@ func TestGraph(t *testing.T) { Version: 2, Objects: map[model.ObjectName]*model.Object{ model.ObjectName("user"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("rel_name"): { - &model.Relation{Direct: model.ObjectName("ext_obj")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("ext_obj")}}, }, }, }, model.ObjectName("ext_obj"): {}, model.ObjectName("group"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("member"): { - &model.Relation{Direct: model.ObjectName("user")}, - &model.Relation{Subject: &model.SubjectRelation{ - Object: model.ObjectName("group"), - Relation: model.RelationName("member"), - }}, + Union: []*model.RelationTerm{ + {Direct: model.ObjectName("user")}, + {Subject: &model.SubjectRelation{ + RelationRef: &model.RelationRef{ + Object: model.ObjectName("group"), + Relation: model.RelationName("member"), + }, + SubjectTypes: []model.ObjectName{}, + }}, + }, }, }, }, model.ObjectName("folder"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("owner"): { - &model.Relation{Direct: model.ObjectName("user")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("user")}}, }, }, }, model.ObjectName("document"): { - Relations: map[model.RelationName][]*model.Relation{ + Relations: map[model.RelationName]*model.Relation{ model.RelationName("parent_folder"): { - {Direct: model.ObjectName("folder")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("folder")}}, }, model.RelationName("writer"): { - {Direct: model.ObjectName("user")}, + Union: []*model.RelationTerm{{Direct: model.ObjectName("user")}}, }, model.RelationName("reader"): { - {Direct: model.ObjectName("user")}, - {Wildcard: model.ObjectName("user")}, + Union: []*model.RelationTerm{ + {Direct: model.ObjectName("user")}, + {Wildcard: model.ObjectName("user")}, + }, }, }, }, @@ -311,54 +336,93 @@ func TestGraph(t *testing.T) { } } +type expectedErrors int + func TestValidation(t *testing.T) { tests := []struct { - name string - manifest string - validate func(error, *stretch.Assertions) + name string + manifest string + expectedErrors expectedErrors + validate func(error, *stretch.Assertions) }{ { "relation/permission collision", "./testdata/rel_perm_collision.yaml", + expectedErrors(1), func(err error, assert *stretch.Assertions) { assert.ErrorContains(err, "permission name 'file:writer' conflicts with 'file:writer' relation") }, }, { - "relation to undefined subject", - "./testdata/rel_to_missing_object.yaml", + "relation/permissions to undefined targets", + "./testdata/undefined_targets.yaml", + expectedErrors(8), func(err error, assert *stretch.Assertions) { - assert.ErrorContains(err, "relation 'file:owner' references undefined object type 'user'") + // relations + assert.ErrorContains(err, "relation 'file:owner' references undefined object type 'person'") assert.ErrorContains(err, "relation 'file:reader' references undefined object type 'team'") assert.ErrorContains(err, "relation 'file:reader' references undefined object type 'project'") assert.ErrorContains(err, "relation 'file:writer' references undefined object type 'team'") assert.ErrorContains(err, "relation 'file:admin' references undefined relation type 'group#admin'") + + // permissions + assert.ErrorContains(err, "permission 'folder:read' references undefined relation type 'folder:viewer'") + assert.ErrorContains(err, "permission 'folder:read' references undefined relation or permission 'folder:editor'") + assert.ErrorContains(err, "permission 'folder:write' references 'owner->write', which references undefined "+ + "relation or permission 'user:write'") }, }, { - "permissions with invalid targets", - "./testdata/invalid_perms.yaml", + "cyclic relation definitions", + "./testdata/cycles.yaml", + expectedErrors(3), func(err error, assert *stretch.Assertions) { - assert.ErrorContains(err, "permission 'file:read' references undefined relation type 'file:viewer'") - assert.ErrorContains(err, "permission 'file:read' references undefined relation or permission 'file:editor'") - assert.ErrorContains(err, "permission 'file:write' references 'owner->write', which can resolve to undefined relation or permission 'user:write'") - + assert.ErrorContains(err, "relation 'team:member' is circular and does not resolve to any object types") + assert.ErrorContains(err, "relation 'team:owner' is circular and does not resolve to any object types") + assert.ErrorContains(err, "relation 'project:owner' is circular and does not resolve to any object types") }, }, + // { + // "permissions with invalid targets", + // "./testdata/invalid_perms.yaml", + // expectedErrors(3), + // func(err error, assert *stretch.Assertions) { + // assert.ErrorContains(err, "permission 'folder:read' references undefined relation type 'folder:viewer'") + // assert.ErrorContains(err, "permission 'folder:read' references undefined relation or permission 'folder:editor'") + // assert.ErrorContains(err, "permission 'folder:write' references 'owner->write', which can resolve to undefined "+ + // "relation or permission 'user:write'") + // }, + // }, } for _, test := range tests { t.Run(test.name, func(tt *testing.T) { assert := stretch.New(tt) - _, err := loadManifest(test.manifest) - assert.Error(err) + m, err := loadManifest(test.manifest) + + // Log the model for debugging purposes. + var b bytes.Buffer + enc := json.NewEncoder(&b) + enc.SetIndent("", " ") + assert.NoError(enc.Encode(m)) + tt.Logf("model: %s", b.String()) + + // verify that we got a load error. + if test.expectedErrors > 0 { + assert.Error(err) + } // verify that the error is of type ErrInvalidArgument - aErr := derr.ErrInvalidArgument - assert.ErrorAs(err, &aErr) - assert.Equal("E20015", aErr.Code) + aerr := derr.ErrInvalidArgument + assert.ErrorAs(err, &aerr) + assert.Equal("E20015", aerr.Code) + + merr := aerr.Unwrap().(*multierror.Error) + assert.NotNil(merr) + test.validate(merr, assert) - test.validate(err, assert) + // verify that we got the expected number of errors. + assert.Len(merr.Errors, int(test.expectedErrors)) }) } } diff --git a/model/testdata/cycles.yaml b/model/testdata/cycles.yaml new file mode 100644 index 0000000..575aaed --- /dev/null +++ b/model/testdata/cycles.yaml @@ -0,0 +1,35 @@ +# yaml-language-server: $schema=manifest.json +--- + +# model +model: + version: 3 + +types: + user: + relations: + # valid: mutually recursive relations are allowed as long as there is at lease one + # direct assignment. + parent: user | user#child + child: user | user#parent + + group: + relations: + # valid: mutually recursive relations with a concrete wildcard assignment. + member: user:* | group#guest + guest: group#member + + team: + relations: + # invalid: cyclic definition without any concrete type assignment. + member: team#member + + # invalid: cyclic definition without any concrete type assignment. + owner: project#owner + + project: + relations: + # invalid: cyclic definition without any concrete type assignment. + owner: team#owner + + diff --git a/model/testdata/rel_to_missing_object.yaml b/model/testdata/undefined_targets.yaml similarity index 59% rename from model/testdata/rel_to_missing_object.yaml rename to model/testdata/undefined_targets.yaml index 9959035..4ed16d3 100644 --- a/model/testdata/rel_to_missing_object.yaml +++ b/model/testdata/undefined_targets.yaml @@ -6,6 +6,7 @@ model: version: 3 types: + user: {} group: relations: member: group @@ -13,7 +14,7 @@ types: file: relations: # invalid: type 'user' undefined - owner: user + owner: person # invalid: types 'team' and 'project' undefined reader: project | team:* @@ -25,3 +26,15 @@ types: admin: group#admin + folder: + relations: + owner: user + writer: user + reader: user + + permissions: + # invliad: viewer and editor are undefined + read: viewer->read | editor + + # invalid: owner has subject type 'user' which has no `write' relation of permission.` + write: owner->write diff --git a/parser/parse.go b/parser/parse.go index bb13292..8154de4 100644 --- a/parser/parse.go +++ b/parser/parse.go @@ -5,12 +5,12 @@ import ( "github.com/aserto-dev/azm/model" ) -func ParseRelation(input string) []*model.Relation { +func ParseRelation(input string) []*model.RelationTerm { p := newParser(input) rTree := p.Relation() var v RelationVisitor - return v.Visit(rTree).([]*model.Relation) + return v.Visit(rTree).([]*model.RelationTerm) } func ParsePermission(input string) *model.Permission { diff --git a/parser/parser_test.go b/parser/parser_test.go index c8b1c21..4d815c1 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -11,11 +11,11 @@ import ( func TestRelationParser(t *testing.T) { tests := []struct { input string - validate func([]*model.Relation, *assert.Assertions) + validate func([]*model.RelationTerm, *assert.Assertions) }{ { "user", - func(rel []*model.Relation, assert *assert.Assertions) { + func(rel []*model.RelationTerm, assert *assert.Assertions) { assert.Len(rel, 1) term := rel[0] assert.Equal(model.ObjectName("user"), term.Direct) @@ -25,7 +25,7 @@ func TestRelationParser(t *testing.T) { }, { "name-with-dashes", - func(rel []*model.Relation, assert *assert.Assertions) { + func(rel []*model.RelationTerm, assert *assert.Assertions) { assert.Len(rel, 1) term := rel[0] assert.Equal(model.ObjectName("name-with-dashes"), term.Direct) @@ -35,7 +35,7 @@ func TestRelationParser(t *testing.T) { }, { "group#member", - func(rel []*model.Relation, assert *assert.Assertions) { + func(rel []*model.RelationTerm, assert *assert.Assertions) { assert.Len(rel, 1) term := rel[0] assert.Equal(model.ObjectName("group"), term.Subject.Object) @@ -46,7 +46,7 @@ func TestRelationParser(t *testing.T) { }, { "user:*", - func(rel []*model.Relation, assert *assert.Assertions) { + func(rel []*model.RelationTerm, assert *assert.Assertions) { assert.Len(rel, 1) term := rel[0] assert.Equal(model.ObjectName("user"), term.Wildcard) @@ -56,7 +56,7 @@ func TestRelationParser(t *testing.T) { }, { "user | group", - func(rel []*model.Relation, assert *assert.Assertions) { + func(rel []*model.RelationTerm, assert *assert.Assertions) { assert.Len(rel, 2) assert.Equal(model.ObjectName("user"), rel[0].Direct) assert.Equal(model.ObjectName("group"), rel[1].Direct) @@ -64,7 +64,7 @@ func TestRelationParser(t *testing.T) { }, { "user | group | user:* | group#member", - func(rel []*model.Relation, assert *assert.Assertions) { + func(rel []*model.RelationTerm, assert *assert.Assertions) { assert.Len(rel, 4) assert.Equal(model.ObjectName("user"), rel[0].Direct) assert.Equal(model.ObjectName("group"), rel[1].Direct) diff --git a/parser/permission_visitor.go b/parser/permission_visitor.go index b8a997e..c877feb 100644 --- a/parser/permission_visitor.go +++ b/parser/permission_visitor.go @@ -21,16 +21,16 @@ func (v *PermissionVisitor) Visit(tree antlr.ParseTree) interface{} { func (v *PermissionVisitor) VisitUnionPerm(c *UnionPermContext) interface{} { return &model.Permission{ - Union: lo.Map(c.Union().AllPerm(), func(perm IPermContext, _ int) *model.RelationRef { - return perm.Accept(v).(*model.RelationRef) + Union: lo.Map(c.Union().AllPerm(), func(perm IPermContext, _ int) *model.PermissionRef { + return perm.Accept(v).(*model.PermissionRef) }), } } func (v *PermissionVisitor) VisitIntersectionPerm(c *IntersectionPermContext) interface{} { return &model.Permission{ - Intersection: lo.Map(c.Intersection().AllPerm(), func(perm IPermContext, _ int) *model.RelationRef { - return perm.Accept(v).(*model.RelationRef) + Intersection: lo.Map(c.Intersection().AllPerm(), func(perm IPermContext, _ int) *model.PermissionRef { + return perm.Accept(v).(*model.PermissionRef) }), } } @@ -38,16 +38,16 @@ func (v *PermissionVisitor) VisitIntersectionPerm(c *IntersectionPermContext) in func (v *PermissionVisitor) VisitExclusionPerm(c *ExclusionPermContext) interface{} { return &model.Permission{ Exclusion: &model.ExclusionPermission{ - Include: c.Exclusion().Perm(0).Accept(v).(*model.RelationRef), - Exclude: c.Exclusion().Perm(1).Accept(v).(*model.RelationRef), + Include: c.Exclusion().Perm(0).Accept(v).(*model.PermissionRef), + Exclude: c.Exclusion().Perm(1).Accept(v).(*model.PermissionRef), }, } } func (v *PermissionVisitor) VisitDirectPerm(c *DirectPermContext) interface{} { - return &model.RelationRef{RelOrPerm: c.Direct().ID().GetText()} + return &model.PermissionRef{RelOrPerm: c.Direct().ID().GetText()} } func (v *PermissionVisitor) VisitArrowPerm(c *ArrowPermContext) interface{} { - return &model.RelationRef{Base: model.RelationName(c.Arrow().ID(0).GetText()), RelOrPerm: c.Arrow().ID(1).GetText()} + return &model.PermissionRef{Base: model.RelationName(c.Arrow().ID(0).GetText()), RelOrPerm: c.Arrow().ID(1).GetText()} } diff --git a/parser/relation_visitor.go b/parser/relation_visitor.go index 3025fe1..19ac04f 100644 --- a/parser/relation_visitor.go +++ b/parser/relation_visitor.go @@ -20,22 +20,25 @@ func (v *RelationVisitor) Visit(tree antlr.ParseTree) interface{} { } func (v *RelationVisitor) VisitRelation(c *RelationContext) interface{} { - return lo.Map(c.AllRel(), func(rel IRelContext, _ int) *model.Relation { - return rel.Accept(v).(*model.Relation) + return lo.Map(c.AllRel(), func(rel IRelContext, _ int) *model.RelationTerm { + return rel.Accept(v).(*model.RelationTerm) }) } func (v *RelationVisitor) VisitDirectRel(c *DirectRelContext) interface{} { - return &model.Relation{Direct: model.ObjectName(c.Direct().ID().GetText())} + return &model.RelationTerm{Direct: model.ObjectName(c.Direct().ID().GetText())} } func (v *RelationVisitor) VisitWildcardRel(c *WildcardRelContext) interface{} { - return &model.Relation{Wildcard: model.ObjectName(c.Wildcard().ID().GetText())} + return &model.RelationTerm{Wildcard: model.ObjectName(c.Wildcard().ID().GetText())} } func (v *RelationVisitor) VisitSubjectRel(c *SubjectRelContext) interface{} { - return &model.Relation{Subject: &model.SubjectRelation{ - Object: model.ObjectName(c.Subject().ID(0).GetText()), - Relation: model.RelationName(c.Subject().ID(1).GetText()), + return &model.RelationTerm{Subject: &model.SubjectRelation{ + RelationRef: &model.RelationRef{ + Object: model.ObjectName(c.Subject().ID(0).GetText()), + Relation: model.RelationName(c.Subject().ID(1).GetText()), + }, + SubjectTypes: []model.ObjectName{}, }} } diff --git a/v2/load.go b/v2/load.go index f7f99db..cc098bc 100644 --- a/v2/load.go +++ b/v2/load.go @@ -32,7 +32,7 @@ func Load(r io.Reader) (*model.Model, error) { // create object type if not exists if _, ok := m.Objects[on]; !ok { m.Objects[on] = &model.Object{ - Relations: map[model.RelationName][]*model.Relation{}, + Relations: map[model.RelationName]*model.Relation{}, Permissions: map[model.PermissionName]*model.Permission{}, } } @@ -43,7 +43,7 @@ func Load(r io.Reader) (*model.Model, error) { // create all relation instances for relName := range obj { if _, ok := o.Relations[model.RelationName(relName)]; !ok { - o.Relations[model.RelationName(relName)] = []*model.Relation{} + o.Relations[model.RelationName(relName)] = &model.Relation{Union: []*model.RelationTerm{}} } } @@ -55,9 +55,12 @@ func Load(r io.Reader) (*model.Model, error) { return nil, azm.ErrRelationNotFound.Msg(v) } - rs = append(rs, &model.Relation{Subject: &model.SubjectRelation{ - Object: on, - Relation: model.RelationName(relName), + rs.Union = append(rs.Union, &model.RelationTerm{Subject: &model.SubjectRelation{ + RelationRef: &model.RelationRef{ + Object: on, + Relation: model.RelationName(relName), + }, + SubjectTypes: []model.ObjectName{}, }}) o.Relations[model.RelationName(v)] = rs @@ -71,11 +74,11 @@ func Load(r io.Reader) (*model.Model, error) { // if permission does not exist, create permission definition. if pd, ok := o.Permissions[pn]; !ok { p := &model.Permission{ - Union: []*model.RelationRef{{RelOrPerm: relName}}, + Union: []*model.PermissionRef{{RelOrPerm: relName}}, } o.Permissions[pn] = p } else { - pd.Union = append(pd.Union, &model.RelationRef{RelOrPerm: relName}) + pd.Union = append(pd.Union, &model.PermissionRef{RelOrPerm: relName}) o.Permissions[pn] = pd } } diff --git a/v3/load.go b/v3/load.go index 84a4e8a..2a2fcda 100644 --- a/v3/load.go +++ b/v3/load.go @@ -31,12 +31,16 @@ func Load(r io.Reader) (*model.Model, error) { for on, o := range manifest.ObjectTypes { log.Debug().Str("object", string(on)).Msg("loading object") - relations := lo.MapEntries(o.Relations, func(rn RelationName, rd string) (model.RelationName, []*model.Relation) { + relationTerms := lo.MapEntries(o.Relations, func(rn RelationName, rd string) (model.RelationName, []*model.RelationTerm) { log.Debug().Str("object", string(on)).Str("relation", string(rn)).Msg("loading relation") return model.RelationName(rn), parser.ParseRelation(rd) }) + relations := lo.MapEntries(relationTerms, func(rn model.RelationName, rts []*model.RelationTerm) (model.RelationName, *model.Relation) { + return rn, &model.Relation{Union: rts} + }) + permissions := lo.MapEntries(o.Permissions, func(pn PermissionName, pd string) (model.PermissionName, *model.Permission) { log.Debug().Str("object", string(on)).Str("permission", string(pn)).Msg("loading permission")