fix: panic with unknown subject set during expand (#1139)

ory · Nov 18, 2022 · 1f3c568 · 1f3c568
1 parent ef73f05
commit 1f3c568
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 50 deletions.
diff --git a/internal/expand/engine.go b/internal/expand/engine.go
@@ -41,67 +41,68 @@ func (e *Engine) BuildTree(ctx context.Context, subject relationtuple.Subject, r
 		restDepth = globalMaxDepth
 	}
 
-	if subSet, isSubjectSet := subject.(*relationtuple.SubjectSet); isSubjectSet {
-		ctx, wasAlreadyVisited := graph.CheckAndAddVisited(ctx, subject)
-		if wasAlreadyVisited {
+	subSet, isSubjectSet := subject.(*relationtuple.SubjectSet)
+	if !isSubjectSet {
+		// is SubjectID
+		return &relationtuple.Tree{
+			Type:    ketoapi.TreeNodeLeaf,
+			Subject: subject,
+		}, nil
+	}
+
+	ctx, wasAlreadyVisited := graph.CheckAndAddVisited(ctx, subject)
+	if wasAlreadyVisited {
+		return nil, nil
+	}
+
+	subTree := &relationtuple.Tree{
+		Type:    ketoapi.TreeNodeUnion,
+		Subject: subject,
+	}
+
+	var (
+		rels     []*relationtuple.RelationTuple
+		nextPage string
+	)
+	// do ... while nextPage != ""
+	for ok := true; ok; ok = nextPage != "" {
+		var err error
+		rels, nextPage, err = e.d.RelationTupleManager().GetRelationTuples(
+			ctx,
+			&relationtuple.RelationQuery{
+				Relation:  &subSet.Relation,
+				Object:    &subSet.Object,
+				Namespace: &subSet.Namespace,
+			},
+			x.WithToken(nextPage),
+		)
+		if err != nil {
+			return nil, err
+		} else if len(rels) == 0 {
 			return nil, nil
 		}
 
-		subTree := &relationtuple.Tree{
-			Type:    ketoapi.TreeNodeUnion,
-			Subject: subject,
+		if restDepth <= 1 {
+			subTree.Type = ketoapi.TreeNodeLeaf
+			return subTree, nil
 		}
 
-		var (
-			rels     []*relationtuple.RelationTuple
-			nextPage string
-		)
-		// do ... while nextPage != ""
-		for ok := true; ok; ok = nextPage != "" {
-			var err error
-			rels, nextPage, err = e.d.RelationTupleManager().GetRelationTuples(
-				ctx,
-				&relationtuple.RelationQuery{
-					Relation:  &subSet.Relation,
-					Object:    &subSet.Object,
-					Namespace: &subSet.Namespace,
-				},
-				x.WithToken(nextPage),
-			)
+		children := make([]*relationtuple.Tree, len(rels))
+		for ri, r := range rels {
+			child, err := e.BuildTree(ctx, r.Subject, restDepth-1)
 			if err != nil {
 				return nil, err
-			} else if len(rels) == 0 {
-				return nil, nil
-			}
-
-			if restDepth <= 1 {
-				subTree.Type = ketoapi.TreeNodeLeaf
-				return subTree, nil
 			}
-
-			children := make([]*relationtuple.Tree, len(rels))
-			for ri, r := range rels {
-				child, err := e.BuildTree(ctx, r.Subject, restDepth-1)
-				if err != nil {
-					return nil, err
-				}
-				if child == nil {
-					child = &relationtuple.Tree{
-						Type:    ketoapi.TreeNodeLeaf,
-						Subject: r.Subject,
-					}
+			if child == nil {
+				child = &relationtuple.Tree{
+					Type:    ketoapi.TreeNodeLeaf,
+					Subject: r.Subject,
 				}
-				children[ri] = child
 			}
-			subTree.Children = append(subTree.Children, children...)
+			children[ri] = child
 		}
-
-		return subTree, nil
+		subTree.Children = append(subTree.Children, children...)
 	}
 
-	// is SubjectID
-	return &relationtuple.Tree{
-		Type:    ketoapi.TreeNodeLeaf,
-		Subject: subject,
-	}, nil
+	return subTree, nil
 }
diff --git a/internal/expand/engine_test.go b/internal/expand/engine_test.go
@@ -374,4 +374,15 @@ func TestEngine(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, expectedTree, tree)
 	})
+
+	t.Run("case=returns result on unknown subject", func(t *testing.T) {
+		_, e := newTestEngine(t, []*namespace.Namespace{})
+		tree, err := e.BuildTree(context.Background(), &relationtuple.SubjectSet{
+			Namespace: "unknown",
+			Object:    uuid.Must(uuid.NewV4()),
+			Relation:  "rel",
+		}, 100)
+		require.NoError(t, err)
+		assert.Nil(t, tree)
+	})
 }
diff --git a/internal/expand/handler.go b/internal/expand/handler.go
@@ -96,6 +96,11 @@ func (h *handler) getExpand(w http.ResponseWriter, r *http.Request, _ httprouter
 		h.d.Writer().WriteError(w, r, err)
 		return
 	}
+	if res == nil {
+		h.d.Writer().Write(w, r, herodot.ErrNotFound.WithError("no relation tuple found"))
+		return
+	}
+
 	tree, err := h.d.Mapper().ToTree(r.Context(), res)
 	if err != nil {
 		h.d.Writer().WriteError(w, r, err)