fix: ensure Check evaluates deterministically on the eval boundary case

internal/graph/check.go

@@ -182,11 +182,13 @@ func union(ctx context.Context, concurrencyLimit uint32, handlers ...CheckHandle
 		close(resultChan)
 	}()
 
+	var err error
 	for i := 0; i < len(handlers); i++ {
 		select {
 		case result := <-resultChan:
 			if result.err != nil {
-				return &openfgapb.CheckResponse{Allowed: false}, result.err
+				err = result.err
+				continue
 			}
 
 			if result.resp.GetAllowed() {
@@ -197,7 +199,7 @@ func union(ctx context.Context, concurrencyLimit uint32, handlers ...CheckHandle
 		}
 	}
 
-	return &openfgapb.CheckResponse{Allowed: false}, nil
+	return &openfgapb.CheckResponse{Allowed: false}, err
 }
 
 // intersection implements a CheckFuncReducer that requires all of the provided CheckHandlerFunc to resolve
@@ -215,12 +217,13 @@ func intersection(ctx context.Context, concurrencyLimit uint32, handlers ...Chec
 		close(resultChan)
 	}()
 
+	var err error
 	for i := 0; i < len(handlers); i++ {
 		select {
 		case result := <-resultChan:
-
 			if result.err != nil {
-				return &openfgapb.CheckResponse{Allowed: false}, result.err
+				err = result.err
+				continue
 			}
 
 			if !result.resp.GetAllowed() {
@@ -231,6 +234,10 @@ func intersection(ctx context.Context, concurrencyLimit uint32, handlers ...Chec
 		}
 	}
 
+	if err != nil {
+		return &openfgapb.CheckResponse{Allowed: false}, err
+	}
+
 	return &openfgapb.CheckResponse{Allowed: true}, nil
 }
 

internal/graph/check_test.go

@@ -0,0 +1,74 @@
+package graph
+
+import (
+	"context"
+	"testing"
+
+	parser "github.com/craigpastro/openfga-dsl-parser/v2"
+	"github.com/oklog/ulid/v2"
+	"github.com/openfga/openfga/pkg/storage/memory"
+	"github.com/openfga/openfga/pkg/tuple"
+	"github.com/openfga/openfga/pkg/typesystem"
+	"github.com/stretchr/testify/require"
+	openfgav1 "go.buf.build/openfga/go/openfga/api/openfga/v1"
+)
+
+func TestResolveCheckDeterministic(t *testing.T) {
+
+	ds := memory.New(10, 24)
+
+	storeID := ulid.Make().String()
+
+	err := ds.Write(context.Background(), storeID, nil, []*openfgav1.TupleKey{
+		tuple.NewTupleKey("document:1", "viewer", "group:eng#member"),
+		tuple.NewTupleKey("document:1", "editor", "group:other1#member"),
+		tuple.NewTupleKey("document:2", "editor", "group:eng#member"),
+		tuple.NewTupleKey("document:2", "allowed", "user:jon"),
+		tuple.NewTupleKey("document:2", "allowed", "user:x"),
+		tuple.NewTupleKey("group:eng", "member", "group:fga#member"),
+		tuple.NewTupleKey("group:eng", "member", "user:jon"),
+		tuple.NewTupleKey("group:other1", "member", "group:other2#member"),
+	})
+	require.NoError(t, err)
+
+	checker := NewLocalChecker(ds, 100)
+
+	typedefs := parser.MustParse(`
+	type user
+
+	type group
+	  relations
+	    define member: [user, group#member] as self
+
+	type document
+	  relations
+	    define allowed: [user] as self
+	    define viewer: [group#member] as self or editor
+	    define editor: [group#member] as self and allowed
+
+	`)
+
+	ctx := typesystem.ContextWithTypesystem(context.Background(), typesystem.New(
+		&openfgav1.AuthorizationModel{
+			Id:              ulid.Make().String(),
+			TypeDefinitions: typedefs,
+			SchemaVersion:   typesystem.SchemaVersion1_1,
+		},
+	))
+
+	resp, err := checker.ResolveCheck(ctx, &ResolveCheckRequest{
+		StoreID:            storeID,
+		TupleKey:           tuple.NewTupleKey("document:1", "viewer", "user:jon"),
+		ResolutionMetadata: &ResolutionMetadata{Depth: 2},
+	})
+	require.NoError(t, err)
+	require.True(t, resp.Allowed)
+
+	resp, err = checker.ResolveCheck(ctx, &ResolveCheckRequest{
+		StoreID:            storeID,
+		TupleKey:           tuple.NewTupleKey("document:2", "editor", "user:x"),
+		ResolutionMetadata: &ResolutionMetadata{Depth: 2},
+	})
+	require.ErrorIs(t, err, ErrResolutionDepthExceeded)
+	require.Nil(t, resp)
+}

pkg/server/server_test.go

@@ -43,11 +43,7 @@ func init() {
 }
 
 func TestServerWithPostgresDatastore(t *testing.T) {
-	testDatastore := storagefixtures.RunDatastoreTestContainer(t, "postgres")
-
-	uri := testDatastore.GetConnectionURI(true)
-	ds, err := postgres.New(uri, sqlcommon.NewConfig())
-	require.NoError(t, err)
+	ds := MustBootstrapDatastore(t, "postgres")
 	defer ds.Close()
 
 	test.RunAllTests(t, ds)
@@ -71,17 +67,14 @@ func TestServerWithPostgresDatastoreAndExplicitCredentials(t *testing.T) {
 }
 
 func TestServerWithMemoryDatastore(t *testing.T) {
-	ds := memory.New(10, 24)
+	ds := MustBootstrapDatastore(t, "memory")
 	defer ds.Close()
+
 	test.RunAllTests(t, ds)
 }
 
 func TestServerWithMySQLDatastore(t *testing.T) {
-	testDatastore := storagefixtures.RunDatastoreTestContainer(t, "mysql")
-
-	uri := testDatastore.GetConnectionURI(true)
-	ds, err := mysql.New(uri, sqlcommon.NewConfig())
-	require.NoError(t, err)
+	ds := MustBootstrapDatastore(t, "mysql")
 	defer ds.Close()
 
 	test.RunAllTests(t, ds)
@@ -619,3 +612,26 @@ func TestObsoleteAuthorizationModels(t *testing.T) {
 		require.Equal(t, codes.Code(openfgapb.ErrorCode_validation_error), e.Code())
 	})
 }
+
+func MustBootstrapDatastore(t testing.TB, engine string) storage.OpenFGADatastore {
+	testDatastore := storagefixtures.RunDatastoreTestContainer(t, engine)
+
+	uri := testDatastore.GetConnectionURI(true)
+
+	var ds storage.OpenFGADatastore
+	var err error
+
+	switch engine {
+	case "memory":
+		ds = memory.New(10, 24)
+	case "postgres":
+		ds, err = postgres.New(uri, sqlcommon.NewConfig())
+	case "mysql":
+		ds, err = mysql.New(uri, sqlcommon.NewConfig())
+	default:
+		t.Fatalf("'%s' is not a supported datastore engine", engine)
+	}
+	require.NoError(t, err)
+
+	return ds
+}