From 25028cb715bb8526ad49868774275e4262896ae2 Mon Sep 17 00:00:00 2001
From: Ronen Hilewicz <ronen@aserto.com>
Date: Wed, 11 Sep 2024 17:01:34 -0400
Subject: [PATCH] Fix model inversion bug

With the standard `user` and `group` types, the following definition
causes an error when inverting the model:

```yaml
resource:
  relations:
    editor: user | group#member | editors#member

editors:
  relations:
    member: user | group#member
```

This commit fixes the problem and adds tests.
---
 graph/check_test.yaml | 11 +++++++++++
 model/inverse.go      | 16 +++++++++++++---
 model/validate.go     |  7 ++++++-
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/graph/check_test.yaml b/graph/check_test.yaml
index c8985ae..723dbef 100644
--- a/graph/check_test.yaml
+++ b/graph/check_test.yaml
@@ -76,3 +76,14 @@ types:
       owner: user
     permissions:
       can_delete: owner & parent->can_delete
+
+  # A group#member can be a resource#editor in two ways:
+  # 1. Excplicit assignement (resource#editor@group#member)
+  # 2. Indirectly via editors#member@group#member
+  resource:
+    relations:
+      editor: user | group#member | editors#member
+  editors:
+    relations:
+      member: user | group#member
+
diff --git a/model/inverse.go b/model/inverse.go
index b3916f4..29e67ee 100644
--- a/model/inverse.go
+++ b/model/inverse.go
@@ -78,8 +78,8 @@ func (i *inverter) invertRelation(on ObjectName, rn RelationName, r *Relation) {
 	unionObjs := lo.Associate(r.Union, func(rr *RelationRef) (ObjectName, bool) { return rr.Object, true })
 
 	for _, rr := range r.Union {
-		isrn := InverseRelation(on, rn, rr.Relation)
-		i.im.Objects[rr.Object].Relations[isrn] = &Relation{Union: []*RelationRef{{Object: on}}}
+		irn := InverseRelation(on, rn, rr.Relation)
+		i.addInvertedRelation(i.im.Objects[rr.Object], on, irn)
 		if rr.IsSubject() {
 			// add a synthetic permission to reverse the expansion of the subject relation
 			srel := i.m.Objects[rr.Object].Relations[rr.Relation]
@@ -92,12 +92,22 @@ func (i *inverter) invertRelation(on ObjectName, rn RelationName, r *Relation) {
 				if _, ok := unionObjs[subj.Object]; ok {
 					p.AddTerm(&PermissionTerm{RelOrPerm: InverseRelation(on, rn, subj.Relation)})
 				}
-				p.AddTerm(&PermissionTerm{Base: InverseRelation(rr.Object, rr.Relation, subj.Relation), RelOrPerm: isrn})
+				p.AddTerm(&PermissionTerm{Base: InverseRelation(rr.Object, rr.Relation, subj.Relation), RelOrPerm: irn})
 			}
 		}
 	}
 }
 
+func (i *inverter) addInvertedRelation(o *Object, on ObjectName, rn RelationName) {
+	o.Relations[rn] = &Relation{Union: []*RelationRef{{Object: on}}}
+	// Add a sythetic permission that resolves to the inverted relation.
+	// Having these permissions makes it easier to invert permissions that reference
+	// subject relations.
+	ipn := PermForRel(rn)
+	p := permissionOrNew(o, ipn, permissionKindUnion)
+	p.AddTerm(&PermissionTerm{RelOrPerm: rn})
+}
+
 func (i *inverter) applySubstitutions(o *Object) {
 	for pn, p := range o.Permissions {
 		for _, pt := range p.Terms() {
diff --git a/model/validate.go b/model/validate.go
index f45d630..de147e7 100644
--- a/model/validate.go
+++ b/model/validate.go
@@ -302,7 +302,12 @@ func (v *validator) resolvePermissions() error {
 }
 
 func (v *validator) resolvePermission(ref *RelationRef, seen relSet) (objSet, relSet) {
-	p := v.Objects[ref.Object].Permissions[ref.Relation]
+	p, ok := v.Objects[ref.Object].Permissions[ref.Relation]
+	if !ok {
+		// No such permission. Most likely a bug in the model inversion logic.
+		// Return empty sets which result in a validation error.
+		return set.NewSet[ObjectName](), set.NewSet[RelationRef]()
+	}
 
 	if len(p.SubjectTypes) > 0 {
 		// already resolved