diff --git a/internal/graph/check.go b/internal/graph/check.go index 7868b9ef69..d085a6ff4b 100644 --- a/internal/graph/check.go +++ b/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 } diff --git a/internal/graph/check_test.go b/internal/graph/check_test.go new file mode 100644 index 0000000000..48058786fb --- /dev/null +++ b/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) +} diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 03e49ef593..3a68b324bb 100644 --- a/pkg/server/server_test.go +++ b/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 +}