From 2ca7968c32f15204d8991b00ac49e72f2fdbf0af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Aguiar?= Date: Mon, 9 Jun 2025 12:36:40 -0300 Subject: [PATCH 01/15] Add users list support in store check tests --- README.md | 40 ++++++++++----- cmd/store/import.go | 21 +++++--- cmd/store/import_test.go | 51 +++++++++++++++++-- internal/storetest/localtest.go | 86 +++++++++++++++++--------------- internal/storetest/remotetest.go | 33 +++++++----- internal/storetest/storedata.go | 1 + 6 files changed, 156 insertions(+), 76 deletions(-) diff --git a/README.md b/README.md index 39565a74..763a4eeb 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,13 @@ tests: assertions: member: true moderator: false + # checks can also be defined for multiple users sharing the same expectation + - object: group:employees + users: + - user: user:3 + - user: user:4 + assertions: + member: true ``` If using `output-file`, the response will be written to the specified file on disk. If the desired file already exists, you will be prompted to overwrite the file. @@ -558,19 +565,26 @@ tests: # required - name: test-1 description: testing that the model works # optional # tuple_file: ./tuples.json # tuples that would apply per test - tuples: - - user: user:anne - relation: owner - object: folder:1 - check: # a set of checks to run - - user: user:anne - object: folder:1 - assertions: - # a set of expected results for each relation - can_view: true - can_write: true - can_share: false - list_objects: # a set of list objects to run + tuples: + - user: user:anne + relation: owner + object: folder:1 + check: # a set of checks to run + - user: user:anne + object: folder:1 + assertions: + # a set of expected results for each relation + can_view: true + can_write: true + can_share: false + # checks can group multiple users that share the same expected results + - object: folder:2 + users: + - user:beth + - user:carl + assertions: + can_view: false + list_objects: # a set of list objects to run - user: user:anne type: folder assertions: diff --git a/cmd/store/import.go b/cmd/store/import.go index a6ffb27a..24637a9b 100644 --- a/cmd/store/import.go +++ b/cmd/store/import.go @@ -232,13 +232,20 @@ func getCheckAssertions(checkTests []storetest.ModelTestCheck) []client.ClientAs var assertions []client.ClientAssertion for _, checkTest := range checkTests { - for relation, expectation := range checkTest.Assertions { - assertions = append(assertions, client.ClientAssertion{ - User: checkTest.User, - Relation: relation, - Object: checkTest.Object, - Expectation: expectation, - }) + users := []string{checkTest.User} + if len(checkTest.Users) > 0 { + users = checkTest.Users + } + + for _, user := range users { + for relation, expectation := range checkTest.Assertions { + assertions = append(assertions, client.ClientAssertion{ + User: user, + Relation: relation, + Object: checkTest.Object, + Expectation: expectation, + }) + } } } diff --git a/cmd/store/import_test.go b/cmd/store/import_test.go index abaa9c07..177e7860 100644 --- a/cmd/store/import_test.go +++ b/cmd/store/import_test.go @@ -21,6 +21,21 @@ func TestImportStore(t *testing.T) { Object: "document:doc1", Expectation: true, }} + + multiUserAssertions := []client.ClientAssertion{ + { + User: "user:anne", + Relation: "reader", + Object: "document:doc1", + Expectation: true, + }, + { + User: "user:peter", + Relation: "reader", + Object: "document:doc1", + Expectation: true, + }, + } modelID, storeID := "model-1", "store-1" expectedOptions := client.ClientWriteAssertionsOptions{AuthorizationModelId: &modelID, StoreId: &storeID} @@ -38,9 +53,9 @@ func TestImportStore(t *testing.T) { mockCreateStore: true, testStore: storetest.StoreData{ Model: `type user - type document - relations - define reader: [user]`, + type document + relations + define reader: [user]`, Tests: []storetest.ModelTest{ { Name: "Test", @@ -55,6 +70,30 @@ func TestImportStore(t *testing.T) { }, }, }, + { + name: "import store with multi user assertions", + mockWriteAssertions: true, + mockWriteModel: true, + mockCreateStore: true, + testStore: storetest.StoreData{ + Model: `type user + type document + relations + define reader: [user]`, + Tests: []storetest.ModelTest{ + { + Name: "Test", + Check: []storetest.ModelTestCheck{ + { + Users: []string{"user:anne", "user:peter"}, + Object: "document:doc1", + Assertions: map[string]bool{"reader": true}, + }, + }, + }, + }, + }, + }, { name: "create new store without assertions", mockWriteAssertions: false, @@ -107,7 +146,11 @@ func TestImportStore(t *testing.T) { defer mockCtrl.Finish() if test.mockWriteAssertions { - setupWriteAssertionsMock(mockCtrl, mockFgaClient, expectedAssertions, expectedOptions) + expected := expectedAssertions + if test.name == "import store with multi user assertions" { + expected = multiUserAssertions + } + setupWriteAssertionsMock(mockCtrl, mockFgaClient, expected, expectedOptions) } else { mockFgaClient.EXPECT().WriteAssertions(gomock.Any()).Times(0) } diff --git a/internal/storetest/localtest.go b/internal/storetest/localtest.go index 466bfb85..8b617e70 100644 --- a/internal/storetest/localtest.go +++ b/internal/storetest/localtest.go @@ -24,55 +24,63 @@ func RunLocalCheckTest( tuples []client.ClientContextualTupleKey, options ModelTestOptions, ) []ModelTestCheckSingleResult { - results := []ModelTestCheckSingleResult{} - for relation, expectation := range checkTest.Assertions { - result := ModelTestCheckSingleResult{ - Request: client.ClientCheckRequest{ - User: checkTest.User, - Relation: relation, - Object: checkTest.Object, - ContextualTuples: tuples, - Context: checkTest.Context, - }, - Expected: expectation, - } - - var ( - ctx *structpb.Struct - err error - ) + results := []ModelTestCheckSingleResult{} - if checkTest.Context != nil { - ctx, err = structpb.NewStruct(*checkTest.Context) - } + users := []string{checkTest.User} + if len(checkTest.Users) > 0 { + users = checkTest.Users + } - if err != nil { - result.Error = err - } else { - response, err := RunSingleLocalCheckTest(fgaServer, - &pb.CheckRequest{ - StoreId: *options.StoreID, - AuthorizationModelId: *options.ModelID, - TupleKey: &pb.CheckRequestTupleKey{ - User: checkTest.User, - Relation: relation, - Object: checkTest.Object, - }, - Context: ctx, + for _, user := range users { + for relation, expectation := range checkTest.Assertions { + result := ModelTestCheckSingleResult{ + Request: client.ClientCheckRequest{ + User: user, + Relation: relation, + Object: checkTest.Object, + ContextualTuples: tuples, + Context: checkTest.Context, }, + Expected: expectation, + } + + var ( + ctx *structpb.Struct + err error ) + + if checkTest.Context != nil { + ctx, err = structpb.NewStruct(*checkTest.Context) + } + if err != nil { result.Error = err - } + } else { + response, err := RunSingleLocalCheckTest(fgaServer, + &pb.CheckRequest{ + StoreId: *options.StoreID, + AuthorizationModelId: *options.ModelID, + TupleKey: &pb.CheckRequestTupleKey{ + User: user, + Relation: relation, + Object: checkTest.Object, + }, + Context: ctx, + }, + ) + if err != nil { + result.Error = err + } - if response != nil { - result.Got = &response.Allowed - result.TestResult = result.IsPassing() + if response != nil { + result.Got = &response.Allowed + result.TestResult = result.IsPassing() + } } - } - results = append(results, result) + results = append(results, result) + } } return results diff --git a/internal/storetest/remotetest.go b/internal/storetest/remotetest.go index 864673a7..79e02ab4 100644 --- a/internal/storetest/remotetest.go +++ b/internal/storetest/remotetest.go @@ -34,19 +34,26 @@ func RunRemoteCheckTest( ) []ModelTestCheckSingleResult { results := []ModelTestCheckSingleResult{} - for relation, expectation := range checkTest.Assertions { - result := RunSingleRemoteCheckTest( - fgaClient, - client.ClientCheckRequest{ - User: checkTest.User, - Relation: relation, - Object: checkTest.Object, - Context: checkTest.Context, - ContextualTuples: tuples, - }, - expectation, - ) - results = append(results, result) + users := []string{checkTest.User} + if len(checkTest.Users) > 0 { + users = checkTest.Users + } + + for _, user := range users { + for relation, expectation := range checkTest.Assertions { + result := RunSingleRemoteCheckTest( + fgaClient, + client.ClientCheckRequest{ + User: user, + Relation: relation, + Object: checkTest.Object, + Context: checkTest.Context, + ContextualTuples: tuples, + }, + expectation, + ) + results = append(results, result) + } } return results diff --git a/internal/storetest/storedata.go b/internal/storetest/storedata.go index 56ad6889..ab152c95 100644 --- a/internal/storetest/storedata.go +++ b/internal/storetest/storedata.go @@ -31,6 +31,7 @@ import ( type ModelTestCheck struct { User string `json:"user" yaml:"user"` + Users []string `json:"users" yaml:"users"` Object string `json:"object" yaml:"object"` Context *map[string]interface{} `json:"context" yaml:"context,omitempty"` Assertions map[string]bool `json:"assertions" yaml:"assertions"` From 66564cf00643e35f561c8601a36046516eebf4c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Aguiar?= Date: Mon, 9 Jun 2025 17:48:47 -0300 Subject: [PATCH 02/15] fix lint --- cmd/store/import_test.go | 1 + internal/storetest/localtest.go | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/store/import_test.go b/cmd/store/import_test.go index 177e7860..ae52dfaa 100644 --- a/cmd/store/import_test.go +++ b/cmd/store/import_test.go @@ -150,6 +150,7 @@ func TestImportStore(t *testing.T) { if test.name == "import store with multi user assertions" { expected = multiUserAssertions } + setupWriteAssertionsMock(mockCtrl, mockFgaClient, expected, expectedOptions) } else { mockFgaClient.EXPECT().WriteAssertions(gomock.Any()).Times(0) diff --git a/internal/storetest/localtest.go b/internal/storetest/localtest.go index 8b617e70..2e444812 100644 --- a/internal/storetest/localtest.go +++ b/internal/storetest/localtest.go @@ -24,10 +24,9 @@ func RunLocalCheckTest( tuples []client.ClientContextualTupleKey, options ModelTestOptions, ) []ModelTestCheckSingleResult { - results := []ModelTestCheckSingleResult{} - users := []string{checkTest.User} + if len(checkTest.Users) > 0 { users = checkTest.Users } From 8492b49980662fc3ef5479a31ce77689ce00975a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Aguiar?= Date: Fri, 13 Jun 2025 14:51:32 -0300 Subject: [PATCH 03/15] chore: fixed example --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 763a4eeb..db85fe57 100644 --- a/README.md +++ b/README.md @@ -306,8 +306,8 @@ tests: # checks can also be defined for multiple users sharing the same expectation - object: group:employees users: - - user: user:3 - - user: user:4 + - user:3 + - user:4 assertions: member: true ``` From a645ae0496e230c19e1e75262ab80b1c2a809326 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Aguiar?= Date: Fri, 13 Jun 2025 15:29:02 -0300 Subject: [PATCH 04/15] refactor: consolidate user selection logic --- cmd/store/import.go | 13 +++++++++---- internal/storetest/localtest.go | 6 +----- internal/storetest/remotetest.go | 5 +---- internal/storetest/utils.go | 9 +++++++++ 4 files changed, 20 insertions(+), 13 deletions(-) create mode 100644 internal/storetest/utils.go diff --git a/cmd/store/import.go b/cmd/store/import.go index 24637a9b..43de54a7 100644 --- a/cmd/store/import.go +++ b/cmd/store/import.go @@ -232,10 +232,7 @@ func getCheckAssertions(checkTests []storetest.ModelTestCheck) []client.ClientAs var assertions []client.ClientAssertion for _, checkTest := range checkTests { - users := []string{checkTest.User} - if len(checkTest.Users) > 0 { - users = checkTest.Users - } + users := getEffectiveUsers(checkTest) for _, user := range users { for relation, expectation := range checkTest.Assertions { @@ -252,6 +249,14 @@ func getCheckAssertions(checkTests []storetest.ModelTestCheck) []client.ClientAs return assertions } +func getEffectiveUsers(checkTest storetest.ModelTestCheck) []string { + if len(checkTest.Users) > 0 { + return checkTest.Users + } + + return []string{checkTest.User} +} + func createProgressBar(total int) *progressbar.ProgressBar { return progressbar.NewOptions(total, progressbar.OptionSetWriter(os.Stderr), diff --git a/internal/storetest/localtest.go b/internal/storetest/localtest.go index 2e444812..52f800c5 100644 --- a/internal/storetest/localtest.go +++ b/internal/storetest/localtest.go @@ -25,11 +25,7 @@ func RunLocalCheckTest( options ModelTestOptions, ) []ModelTestCheckSingleResult { results := []ModelTestCheckSingleResult{} - users := []string{checkTest.User} - - if len(checkTest.Users) > 0 { - users = checkTest.Users - } + users := getEffectiveUsers(checkTest) for _, user := range users { for relation, expectation := range checkTest.Assertions { diff --git a/internal/storetest/remotetest.go b/internal/storetest/remotetest.go index 79e02ab4..7dcfdcf5 100644 --- a/internal/storetest/remotetest.go +++ b/internal/storetest/remotetest.go @@ -34,10 +34,7 @@ func RunRemoteCheckTest( ) []ModelTestCheckSingleResult { results := []ModelTestCheckSingleResult{} - users := []string{checkTest.User} - if len(checkTest.Users) > 0 { - users = checkTest.Users - } + users := getEffectiveUsers(checkTest) for _, user := range users { for relation, expectation := range checkTest.Assertions { diff --git a/internal/storetest/utils.go b/internal/storetest/utils.go new file mode 100644 index 00000000..e14655b9 --- /dev/null +++ b/internal/storetest/utils.go @@ -0,0 +1,9 @@ +package storetest + +func getEffectiveUsers(checkTest ModelTestCheck) []string { + if len(checkTest.Users) > 0 { + return checkTest.Users + } + + return []string{checkTest.User} +} From e762df215481aa4db8f55e1c7a1ceaf22fc16afc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Aguiar?= Date: Fri, 13 Jun 2025 15:53:21 -0300 Subject: [PATCH 05/15] refactor: centralize getEffectiveUsers --- cmd/store/import.go | 10 +--------- internal/storetest/localtest.go | 2 +- internal/storetest/remotetest.go | 2 +- internal/storetest/utils.go | 2 +- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/cmd/store/import.go b/cmd/store/import.go index 43de54a7..613718ee 100644 --- a/cmd/store/import.go +++ b/cmd/store/import.go @@ -232,7 +232,7 @@ func getCheckAssertions(checkTests []storetest.ModelTestCheck) []client.ClientAs var assertions []client.ClientAssertion for _, checkTest := range checkTests { - users := getEffectiveUsers(checkTest) + users := storetest.GetEffectiveUsers(checkTest) for _, user := range users { for relation, expectation := range checkTest.Assertions { @@ -249,14 +249,6 @@ func getCheckAssertions(checkTests []storetest.ModelTestCheck) []client.ClientAs return assertions } -func getEffectiveUsers(checkTest storetest.ModelTestCheck) []string { - if len(checkTest.Users) > 0 { - return checkTest.Users - } - - return []string{checkTest.User} -} - func createProgressBar(total int) *progressbar.ProgressBar { return progressbar.NewOptions(total, progressbar.OptionSetWriter(os.Stderr), diff --git a/internal/storetest/localtest.go b/internal/storetest/localtest.go index 52f800c5..72ff148f 100644 --- a/internal/storetest/localtest.go +++ b/internal/storetest/localtest.go @@ -25,7 +25,7 @@ func RunLocalCheckTest( options ModelTestOptions, ) []ModelTestCheckSingleResult { results := []ModelTestCheckSingleResult{} - users := getEffectiveUsers(checkTest) + users := GetEffectiveUsers(checkTest) for _, user := range users { for relation, expectation := range checkTest.Assertions { diff --git a/internal/storetest/remotetest.go b/internal/storetest/remotetest.go index 7dcfdcf5..a8fbb4c2 100644 --- a/internal/storetest/remotetest.go +++ b/internal/storetest/remotetest.go @@ -34,7 +34,7 @@ func RunRemoteCheckTest( ) []ModelTestCheckSingleResult { results := []ModelTestCheckSingleResult{} - users := getEffectiveUsers(checkTest) + users := GetEffectiveUsers(checkTest) for _, user := range users { for relation, expectation := range checkTest.Assertions { diff --git a/internal/storetest/utils.go b/internal/storetest/utils.go index e14655b9..b9240d86 100644 --- a/internal/storetest/utils.go +++ b/internal/storetest/utils.go @@ -1,6 +1,6 @@ package storetest -func getEffectiveUsers(checkTest ModelTestCheck) []string { +func GetEffectiveUsers(checkTest ModelTestCheck) []string { if len(checkTest.Users) > 0 { return checkTest.Users } From 6ea62b01ed41d25b5cb9aca320f8e93cec5cd19f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Aguiar?= Date: Tue, 17 Jun 2025 16:00:55 -0300 Subject: [PATCH 06/15] Validate check definitions and add tests --- README.md | 2 ++ cmd/store/import.go | 4 ++++ internal/storetest/read-from-input.go | 4 ++++ internal/storetest/storedata.go | 23 +++++++++++++++++++ internal/storetest/storedata_test.go | 32 +++++++++++++++++++++++++++ internal/storetest/tests.go | 4 ++++ 6 files changed, 69 insertions(+) create mode 100644 internal/storetest/storedata_test.go diff --git a/README.md b/README.md index db85fe57..645147a0 100644 --- a/README.md +++ b/README.md @@ -306,6 +306,7 @@ tests: # checks can also be defined for multiple users sharing the same expectation - object: group:employees users: + # either "user" or "users" may be provided, but not both - user:3 - user:4 assertions: @@ -584,6 +585,7 @@ tests: # required - user:carl assertions: can_view: false + # either "user" or "users" may be provided, but not both list_objects: # a set of list objects to run - user: user:anne type: folder diff --git a/cmd/store/import.go b/cmd/store/import.go index 613718ee..1936ff42 100644 --- a/cmd/store/import.go +++ b/cmd/store/import.go @@ -121,6 +121,10 @@ func importStore( maxTuplesPerWrite, maxParallelRequests int, fileName string, ) (*CreateStoreAndModelResponse, error) { + if err := storeData.Validate(); err != nil { + return nil, err //nolint:wrapcheck + } + response, err := createOrUpdateStore(ctx, clientConfig, fgaClient, storeData, format, storeID, fileName) if err != nil { return nil, err diff --git a/internal/storetest/read-from-input.go b/internal/storetest/read-from-input.go index 0ee9ebcd..c02f894a 100644 --- a/internal/storetest/read-from-input.go +++ b/internal/storetest/read-from-input.go @@ -54,5 +54,9 @@ func ReadFromFile(fileName string, basePath string) (authorizationmodel.ModelFor return format, nil, err } + if err = storeData.Validate(); err != nil { + return format, nil, err + } + return format, &storeData, nil } diff --git a/internal/storetest/storedata.go b/internal/storetest/storedata.go index ab152c95..38fd12a9 100644 --- a/internal/storetest/storedata.go +++ b/internal/storetest/storedata.go @@ -26,6 +26,7 @@ import ( "github.com/openfga/go-sdk/client" "github.com/openfga/cli/internal/authorizationmodel" + "github.com/openfga/cli/internal/clierrors" "github.com/openfga/cli/internal/tuplefile" ) @@ -138,3 +139,25 @@ func (storeData *StoreData) LoadTuples(basePath string) error { return nil } + +func (storeData *StoreData) Validate() error { + var errs error + + for _, test := range storeData.Tests { + for index, check := range test.Check { + if check.User != "" && len(check.Users) > 0 { + msg := fmt.Sprintf("test %s check %d cannot contain both 'user' and 'users'", test.Name, index) + errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) + } else if check.User == "" && len(check.Users) == 0 { + msg := fmt.Sprintf("test %s check %d must specify 'user' or 'users'", test.Name, index) + errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) + } + } + } + + if errs != nil { + return clierrors.ValidationError("storetests", errs.Error()) //nolint:wrapcheck + } + + return nil +} diff --git a/internal/storetest/storedata_test.go b/internal/storetest/storedata_test.go new file mode 100644 index 00000000..7130a2ef --- /dev/null +++ b/internal/storetest/storedata_test.go @@ -0,0 +1,32 @@ +package storetest + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestStoreDataValidate(t *testing.T) { + t.Parallel() + + validSingle := StoreData{Tests: []ModelTest{{Name: "t1", Check: []ModelTestCheck{{ + User: "user:1", Object: "doc:1", Assertions: map[string]bool{"read": true}, + }}}}} + assert.NoError(t, validSingle.Validate()) + + validUsers := StoreData{Tests: []ModelTest{{Name: "t1", Check: []ModelTestCheck{{ + Users: []string{"user:1", "user:2"}, Object: "doc:1", Assertions: map[string]bool{"read": true}, + }}}}} + assert.NoError(t, validUsers.Validate()) + + invalidBoth := StoreData{Tests: []ModelTest{{Name: "t1", Check: []ModelTestCheck{{ + User: "user:1", Users: []string{"user:2"}, Object: "doc:1", Assertions: map[string]bool{"read": true}, + }}}}} + require.Error(t, invalidBoth.Validate()) + + invalidNone := StoreData{Tests: []ModelTest{{Name: "t1", Check: []ModelTestCheck{{ + Object: "doc:1", Assertions: map[string]bool{"read": true}, + }}}}} + require.Error(t, invalidNone.Validate()) +} diff --git a/internal/storetest/tests.go b/internal/storetest/tests.go index 725f9f9e..360ae194 100644 --- a/internal/storetest/tests.go +++ b/internal/storetest/tests.go @@ -36,6 +36,10 @@ func RunTests( ) (TestResults, error) { testResults := TestResults{} + if err := storeData.Validate(); err != nil { + return testResults, err + } + fgaServer, authModel, stopServerFn, err := getLocalServerModelAndTuples(storeData, format) if err != nil { return testResults, err From d24c6803e4d7cc08b916f44b2c46a7d963bfb516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Aguiar?= Date: Wed, 18 Jun 2025 09:27:25 -0300 Subject: [PATCH 07/15] Add support for multi-object checks --- README.md | 16 ++++++ cmd/store/import.go | 17 +++--- cmd/store/import_test.go | 49 +++++++++++++++-- internal/storetest/localtest.go | 81 ++++++++++++++-------------- internal/storetest/remotetest.go | 29 +++++----- internal/storetest/storedata.go | 10 ++++ internal/storetest/storedata_test.go | 12 ++++- internal/storetest/utils.go | 8 +++ 8 files changed, 159 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 645147a0..ccefde92 100644 --- a/README.md +++ b/README.md @@ -306,7 +306,15 @@ tests: # checks can also be defined for multiple users sharing the same expectation - object: group:employees users: + # checks can also target multiple objects with the same expectation + - objects: + - group:admins + - group:employees + user: user:1 + assertions: + moderator: false # either "user" or "users" may be provided, but not both + # either "object" or "objects" may be provided, but not both - user:3 - user:4 assertions: @@ -585,7 +593,15 @@ tests: # required - user:carl assertions: can_view: false + # checks can group multiple objects that share the same expected results + - objects: + - folder:1 + - folder:2 + user: user:beth + assertions: + can_write: false # either "user" or "users" may be provided, but not both + # either "object" or "objects" may be provided, but not both list_objects: # a set of list objects to run - user: user:anne type: folder diff --git a/cmd/store/import.go b/cmd/store/import.go index 1936ff42..b334c725 100644 --- a/cmd/store/import.go +++ b/cmd/store/import.go @@ -237,15 +237,18 @@ func getCheckAssertions(checkTests []storetest.ModelTestCheck) []client.ClientAs for _, checkTest := range checkTests { users := storetest.GetEffectiveUsers(checkTest) + objects := storetest.GetEffectiveObjects(checkTest) for _, user := range users { - for relation, expectation := range checkTest.Assertions { - assertions = append(assertions, client.ClientAssertion{ - User: user, - Relation: relation, - Object: checkTest.Object, - Expectation: expectation, - }) + for _, object := range objects { + for relation, expectation := range checkTest.Assertions { + assertions = append(assertions, client.ClientAssertion{ + User: user, + Relation: relation, + Object: object, + Expectation: expectation, + }) + } } } } diff --git a/cmd/store/import_test.go b/cmd/store/import_test.go index ae52dfaa..edb0e031 100644 --- a/cmd/store/import_test.go +++ b/cmd/store/import_test.go @@ -36,6 +36,21 @@ func TestImportStore(t *testing.T) { Expectation: true, }, } + + multiObjectAssertions := []client.ClientAssertion{ + { + User: "user:peter", + Relation: "reader", + Object: "document:doc1", + Expectation: true, + }, + { + User: "user:peter", + Relation: "reader", + Object: "document:doc2", + Expectation: true, + }, + } modelID, storeID := "model-1", "store-1" expectedOptions := client.ClientWriteAssertionsOptions{AuthorizationModelId: &modelID, StoreId: &storeID} @@ -75,6 +90,30 @@ func TestImportStore(t *testing.T) { mockWriteAssertions: true, mockWriteModel: true, mockCreateStore: true, + testStore: storetest.StoreData{ + Model: `type user + type document + relations + define reader: [user]`, + Tests: []storetest.ModelTest{ + { + Name: "Test", + Check: []storetest.ModelTestCheck{ + { + Users: []string{"user:anne", "user:peter"}, + Object: "document:doc1", + Assertions: map[string]bool{"reader": true}, + }, + }, + }, + }, + }, + }, + { + name: "import store with multi object assertions", + mockWriteAssertions: true, + mockWriteModel: true, + mockCreateStore: true, testStore: storetest.StoreData{ Model: `type user type document @@ -85,8 +124,8 @@ func TestImportStore(t *testing.T) { Name: "Test", Check: []storetest.ModelTestCheck{ { - Users: []string{"user:anne", "user:peter"}, - Object: "document:doc1", + User: "user:peter", + Objects: []string{"document:doc1", "document:doc2"}, Assertions: map[string]bool{"reader": true}, }, }, @@ -147,8 +186,12 @@ func TestImportStore(t *testing.T) { if test.mockWriteAssertions { expected := expectedAssertions - if test.name == "import store with multi user assertions" { + + switch test.name { + case "import store with multi user assertions": expected = multiUserAssertions + case "import store with multi object assertions": + expected = multiObjectAssertions } setupWriteAssertionsMock(mockCtrl, mockFgaClient, expected, expectedOptions) diff --git a/internal/storetest/localtest.go b/internal/storetest/localtest.go index 72ff148f..6f6cec61 100644 --- a/internal/storetest/localtest.go +++ b/internal/storetest/localtest.go @@ -27,54 +27,57 @@ func RunLocalCheckTest( results := []ModelTestCheckSingleResult{} users := GetEffectiveUsers(checkTest) + objects := GetEffectiveObjects(checkTest) for _, user := range users { - for relation, expectation := range checkTest.Assertions { - result := ModelTestCheckSingleResult{ - Request: client.ClientCheckRequest{ - User: user, - Relation: relation, - Object: checkTest.Object, - ContextualTuples: tuples, - Context: checkTest.Context, - }, - Expected: expectation, - } + for _, object := range objects { + for relation, expectation := range checkTest.Assertions { + result := ModelTestCheckSingleResult{ + Request: client.ClientCheckRequest{ + User: user, + Relation: relation, + Object: object, + ContextualTuples: tuples, + Context: checkTest.Context, + }, + Expected: expectation, + } - var ( - ctx *structpb.Struct - err error - ) + var ( + ctx *structpb.Struct + err error + ) - if checkTest.Context != nil { - ctx, err = structpb.NewStruct(*checkTest.Context) - } + if checkTest.Context != nil { + ctx, err = structpb.NewStruct(*checkTest.Context) + } - if err != nil { - result.Error = err - } else { - response, err := RunSingleLocalCheckTest(fgaServer, - &pb.CheckRequest{ - StoreId: *options.StoreID, - AuthorizationModelId: *options.ModelID, - TupleKey: &pb.CheckRequestTupleKey{ - User: user, - Relation: relation, - Object: checkTest.Object, - }, - Context: ctx, - }, - ) if err != nil { result.Error = err + } else { + response, err := RunSingleLocalCheckTest(fgaServer, + &pb.CheckRequest{ + StoreId: *options.StoreID, + AuthorizationModelId: *options.ModelID, + TupleKey: &pb.CheckRequestTupleKey{ + User: user, + Relation: relation, + Object: object, + }, + Context: ctx, + }, + ) + if err != nil { + result.Error = err + } + + if response != nil { + result.Got = &response.Allowed + result.TestResult = result.IsPassing() + } } - if response != nil { - result.Got = &response.Allowed - result.TestResult = result.IsPassing() - } + results = append(results, result) } - - results = append(results, result) } } diff --git a/internal/storetest/remotetest.go b/internal/storetest/remotetest.go index a8fbb4c2..e13d9e2a 100644 --- a/internal/storetest/remotetest.go +++ b/internal/storetest/remotetest.go @@ -35,21 +35,24 @@ func RunRemoteCheckTest( results := []ModelTestCheckSingleResult{} users := GetEffectiveUsers(checkTest) + objects := GetEffectiveObjects(checkTest) for _, user := range users { - for relation, expectation := range checkTest.Assertions { - result := RunSingleRemoteCheckTest( - fgaClient, - client.ClientCheckRequest{ - User: user, - Relation: relation, - Object: checkTest.Object, - Context: checkTest.Context, - ContextualTuples: tuples, - }, - expectation, - ) - results = append(results, result) + for _, object := range objects { + for relation, expectation := range checkTest.Assertions { + result := RunSingleRemoteCheckTest( + fgaClient, + client.ClientCheckRequest{ + User: user, + Relation: relation, + Object: object, + Context: checkTest.Context, + ContextualTuples: tuples, + }, + expectation, + ) + results = append(results, result) + } } } diff --git a/internal/storetest/storedata.go b/internal/storetest/storedata.go index 38fd12a9..4de4cf0b 100644 --- a/internal/storetest/storedata.go +++ b/internal/storetest/storedata.go @@ -34,6 +34,7 @@ type ModelTestCheck struct { User string `json:"user" yaml:"user"` Users []string `json:"users" yaml:"users"` Object string `json:"object" yaml:"object"` + Objects []string `json:"objects" yaml:"objects"` Context *map[string]interface{} `json:"context" yaml:"context,omitempty"` Assertions map[string]bool `json:"assertions" yaml:"assertions"` } @@ -140,6 +141,7 @@ func (storeData *StoreData) LoadTuples(basePath string) error { return nil } +//nolint:cyclop func (storeData *StoreData) Validate() error { var errs error @@ -152,6 +154,14 @@ func (storeData *StoreData) Validate() error { msg := fmt.Sprintf("test %s check %d must specify 'user' or 'users'", test.Name, index) errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) } + + if check.Object != "" && len(check.Objects) > 0 { + msg := fmt.Sprintf("test %s check %d cannot contain both 'object' and 'objects'", test.Name, index) + errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) + } else if check.Object == "" && len(check.Objects) == 0 { + msg := fmt.Sprintf("test %s check %d must specify 'object' or 'objects'", test.Name, index) + errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) + } } } diff --git a/internal/storetest/storedata_test.go b/internal/storetest/storedata_test.go index 7130a2ef..052ea705 100644 --- a/internal/storetest/storedata_test.go +++ b/internal/storetest/storedata_test.go @@ -20,13 +20,23 @@ func TestStoreDataValidate(t *testing.T) { }}}}} assert.NoError(t, validUsers.Validate()) + validObjects := StoreData{Tests: []ModelTest{{Name: "t1", Check: []ModelTestCheck{{ + User: "user:1", Objects: []string{"doc:1", "doc:2"}, Assertions: map[string]bool{"read": true}, + }}}}} + assert.NoError(t, validObjects.Validate()) + invalidBoth := StoreData{Tests: []ModelTest{{Name: "t1", Check: []ModelTestCheck{{ User: "user:1", Users: []string{"user:2"}, Object: "doc:1", Assertions: map[string]bool{"read": true}, }}}}} require.Error(t, invalidBoth.Validate()) + invalidObjectBoth := StoreData{Tests: []ModelTest{{Name: "t1", Check: []ModelTestCheck{{ + User: "user:1", Object: "doc:1", Objects: []string{"doc:2"}, Assertions: map[string]bool{"read": true}, + }}}}} + require.Error(t, invalidObjectBoth.Validate()) + invalidNone := StoreData{Tests: []ModelTest{{Name: "t1", Check: []ModelTestCheck{{ - Object: "doc:1", Assertions: map[string]bool{"read": true}, + User: "user:1", Assertions: map[string]bool{"read": true}, }}}}} require.Error(t, invalidNone.Validate()) } diff --git a/internal/storetest/utils.go b/internal/storetest/utils.go index b9240d86..0eb56980 100644 --- a/internal/storetest/utils.go +++ b/internal/storetest/utils.go @@ -7,3 +7,11 @@ func GetEffectiveUsers(checkTest ModelTestCheck) []string { return []string{checkTest.User} } + +func GetEffectiveObjects(checkTest ModelTestCheck) []string { + if len(checkTest.Objects) > 0 { + return checkTest.Objects + } + + return []string{checkTest.Object} +} From 8e2696985d26ad358504faf435431319784be3d6 Mon Sep 17 00:00:00 2001 From: Andres Aguiar Date: Wed, 18 Jun 2025 19:38:51 -0300 Subject: [PATCH 08/15] fix: example was not working --- example/folder-document-access_tuples.json | 5 ----- example/model.fga | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/example/folder-document-access_tuples.json b/example/folder-document-access_tuples.json index f5994df1..8d63abc7 100644 --- a/example/folder-document-access_tuples.json +++ b/example/folder-document-access_tuples.json @@ -9084,11 +9084,6 @@ "relation": "can_view", "user": "user:58" }, - { - "object": "document:578", - "relation": "can_view", - "user": "user:410" - }, { "object": "document:25", "relation": "can_view", diff --git a/example/model.fga b/example/model.fga index 3a60a000..f146193e 100644 --- a/example/model.fga +++ b/example/model.fga @@ -21,4 +21,4 @@ type document define viewer: [user] or owner or parent_owner define can_share: owner define can_write: owner or parent_owner - define can_view: viewer or viewer from parent + define can_view: [user] or viewer or viewer from parent From acea0365d21930b3078b126346c3b7ac33fa70ec Mon Sep 17 00:00:00 2001 From: Andres Aguiar Date: Wed, 18 Jun 2025 19:39:08 -0300 Subject: [PATCH 09/15] chore: added example for multiple objects --- example/model.fga.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/example/model.fga.yaml b/example/model.fga.yaml index a7cfaaad..baa14eef 100644 --- a/example/model.fga.yaml +++ b/example/model.fga.yaml @@ -47,6 +47,16 @@ tests: can_view: true can_write: true can_share: false + + - users: + - user:anne + objects: + - folder:product-2021 + - folder:product-2021Q1 + assertions: + can_view: true + can_write: true + list_objects: # Each list objects test is made of: a user, an object type and the expected result for one or more relations - user: user:anne type: folder @@ -69,6 +79,8 @@ tests: - folder:product-2021Q1 can_write: [] can_share: [] + + list_users: # Each list user test is made of: an object, a user filter and the expected result for one or more relations - object: folder:product-2021 user_filter: From 55d1d41bacb445ac33a9a389c714d53cd7cb39ca Mon Sep 17 00:00:00 2001 From: Andres Aguiar Date: Wed, 18 Jun 2025 19:39:21 -0300 Subject: [PATCH 10/15] chore: simplified error messages --- example/school.yaml | 74 +++++++++++++++++++++++++++++++++ internal/storetest/storedata.go | 10 ++--- 2 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 example/school.yaml diff --git a/example/school.yaml b/example/school.yaml new file mode 100644 index 00000000..64af8261 --- /dev/null +++ b/example/school.yaml @@ -0,0 +1,74 @@ + +model: | + model + schema 1.1 + + type user + + type school + relations + define member:[user] + define restricted_member:[user] + define approved_restricted_member:[user] + + define reader:member + define restricted_reader:approved_restricted_member + + type repository + relations + define direct_reader:[user] + define school_reader:[school] + define restricted_school_reader:[school] + define reader: direct_reader or reader from school_reader or restricted_reader from restricted_school_reader + +tuples: + - user: user:alice + relation: direct_reader + object: repository:math-notes + + - user: user:bob + relation: member + object: school:harvard + + - user: school:harvard + relation: school_reader + object: repository:physics + + - user: user:charlie + relation: restricted_member + object: school:mit + + - user: user:charlie + relation: approved_restricted_member + object: school:mit + + - user: school:mit + relation: restricted_school_reader + object: repository:confidential-lab + +tests: + - check: + - user: user:alice + object: repository:math-notes + assertions: + reader : true + + - user: user:bob + object: repository:physics + assertions: + reader : true + + - user: user:charlie + object: repository:confidential-lab + assertions: + reader : true + + - user: user:bob + object: repository:confidential-lab + assertions: + reader : false + + - user: user:alice + object: repository:physics + assertions: + reader : false \ No newline at end of file diff --git a/internal/storetest/storedata.go b/internal/storetest/storedata.go index 4de4cf0b..ee66f46e 100644 --- a/internal/storetest/storedata.go +++ b/internal/storetest/storedata.go @@ -149,24 +149,24 @@ func (storeData *StoreData) Validate() error { for index, check := range test.Check { if check.User != "" && len(check.Users) > 0 { msg := fmt.Sprintf("test %s check %d cannot contain both 'user' and 'users'", test.Name, index) - errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) + errs = errors.Join(errs, fmt.Errorf("%s", msg)) } else if check.User == "" && len(check.Users) == 0 { msg := fmt.Sprintf("test %s check %d must specify 'user' or 'users'", test.Name, index) - errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) + errs = errors.Join(errs, fmt.Errorf("%s", msg)) } if check.Object != "" && len(check.Objects) > 0 { msg := fmt.Sprintf("test %s check %d cannot contain both 'object' and 'objects'", test.Name, index) - errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) + errs = errors.Join(errs, fmt.Errorf("%s", msg)) } else if check.Object == "" && len(check.Objects) == 0 { msg := fmt.Sprintf("test %s check %d must specify 'object' or 'objects'", test.Name, index) - errs = errors.Join(errs, fmt.Errorf("%w: %s", clierrors.ErrValidation, msg)) + errs = errors.Join(errs, fmt.Errorf("%s", msg)) } } } if errs != nil { - return clierrors.ValidationError("storetests", errs.Error()) //nolint:wrapcheck + return clierrors.ValidationError("StoreFormat", errs.Error()) //nolint:wrapcheck } return nil From 6b03abe981e747c46e6d797f58607a3e7e2476f4 Mon Sep 17 00:00:00 2001 From: Andres Aguiar Date: Wed, 18 Jun 2025 22:17:14 -0300 Subject: [PATCH 11/15] fix: fixed export command to not to write empty users/objects --- internal/storetest/storedata.go | 8 +- internal/storetest/storedata_test.go | 111 +++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/internal/storetest/storedata.go b/internal/storetest/storedata.go index ee66f46e..c8eee216 100644 --- a/internal/storetest/storedata.go +++ b/internal/storetest/storedata.go @@ -31,10 +31,10 @@ import ( ) type ModelTestCheck struct { - User string `json:"user" yaml:"user"` - Users []string `json:"users" yaml:"users"` - Object string `json:"object" yaml:"object"` - Objects []string `json:"objects" yaml:"objects"` + User string `json:"user,omitempty" yaml:"user,omitempty"` + Users []string `json:"users,omitempty" yaml:"users,omitempty"` + Object string `json:"object,omitempty" yaml:"object,omitempty"` + Objects []string `json:"objects,omitempty" yaml:"objects,omitempty"` Context *map[string]interface{} `json:"context" yaml:"context,omitempty"` Assertions map[string]bool `json:"assertions" yaml:"assertions"` } diff --git a/internal/storetest/storedata_test.go b/internal/storetest/storedata_test.go index 052ea705..d1b82c05 100644 --- a/internal/storetest/storedata_test.go +++ b/internal/storetest/storedata_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) func TestStoreDataValidate(t *testing.T) { @@ -40,3 +41,113 @@ func TestStoreDataValidate(t *testing.T) { }}}}} require.Error(t, invalidNone.Validate()) } + +func TestModelTestCheckYAMLOmitEmpty(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input ModelTestCheck + expected string + }{ + { + name: "only user field populated", + input: ModelTestCheck{ + User: "user:anne", + Object: "folder:product-2021", + Assertions: map[string]bool{"can_view": true}, + }, + expected: `user: user:anne +object: folder:product-2021 +assertions: + can_view: true +`, + }, + { + name: "only users field populated", + input: ModelTestCheck{ + Users: []string{"user:anne", "user:bob"}, + Object: "folder:product-2021", + Assertions: map[string]bool{"can_view": true}, + }, + expected: `users: + - user:anne + - user:bob +object: folder:product-2021 +assertions: + can_view: true +`, + }, + { + name: "only object field populated", + input: ModelTestCheck{ + User: "user:anne", + Object: "folder:product-2021", + Assertions: map[string]bool{"can_view": true}, + }, + expected: `user: user:anne +object: folder:product-2021 +assertions: + can_view: true +`, + }, + { + name: "only objects field populated", + input: ModelTestCheck{ + User: "user:anne", + Objects: []string{"folder:product-2021", "folder:product-2022"}, + Assertions: map[string]bool{"can_view": true}, + }, + expected: `user: user:anne +objects: + - folder:product-2021 + - folder:product-2022 +assertions: + can_view: true +`, + }, + { + name: "users and objects fields populated", + input: ModelTestCheck{ + Users: []string{"user:anne", "user:bob"}, + Objects: []string{"folder:product-2021", "folder:product-2022"}, + Assertions: map[string]bool{"can_view": true}, + }, + expected: `users: + - user:anne + - user:bob +objects: + - folder:product-2021 + - folder:product-2022 +assertions: + can_view: true +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + yamlBytes, err := yaml.Marshal(tt.input) + require.NoError(t, err) + + assert.Equal(t, tt.expected, string(yamlBytes)) + + // Also verify that empty fields are not present in the output + yamlStr := string(yamlBytes) + if tt.input.User == "" { + assert.NotContains(t, yamlStr, "user: ") + } + if len(tt.input.Users) == 0 { + assert.NotContains(t, yamlStr, "users:") + } + if tt.input.Object == "" { + assert.NotContains(t, yamlStr, "object: ") + } + if len(tt.input.Objects) == 0 { + assert.NotContains(t, yamlStr, "objects:") + } + }) + } +} From 08ea0e863f04667cd45a1bf1de12f50a2d7221c8 Mon Sep 17 00:00:00 2001 From: Andres Aguiar Date: Wed, 18 Jun 2025 22:34:13 -0300 Subject: [PATCH 12/15] fix: linter issues --- example/model.fga.yaml | 2 - internal/storetest/storedata.go | 36 +++++--- internal/storetest/storedata_test.go | 131 ++++++--------------------- 3 files changed, 50 insertions(+), 119 deletions(-) diff --git a/example/model.fga.yaml b/example/model.fga.yaml index baa14eef..462ace72 100644 --- a/example/model.fga.yaml +++ b/example/model.fga.yaml @@ -47,7 +47,6 @@ tests: can_view: true can_write: true can_share: false - - users: - user:anne objects: @@ -56,7 +55,6 @@ tests: assertions: can_view: true can_write: true - list_objects: # Each list objects test is made of: a user, an object type and the expected result for one or more relations - user: user:anne type: folder diff --git a/internal/storetest/storedata.go b/internal/storetest/storedata.go index c8eee216..d47f33f7 100644 --- a/internal/storetest/storedata.go +++ b/internal/storetest/storedata.go @@ -30,13 +30,21 @@ import ( "github.com/openfga/cli/internal/tuplefile" ) +// Static error variables for validation. +var ( + ErrUserAndUsersConflict = errors.New("cannot contain both 'user' and 'users'") + ErrUserRequired = errors.New("must specify 'user' or 'users'") + ErrObjectAndObjectsConflict = errors.New("cannot contain both 'object' and 'objects'") + ErrObjectRequired = errors.New("must specify 'object' or 'objects'") +) + type ModelTestCheck struct { - User string `json:"user,omitempty" yaml:"user,omitempty"` - Users []string `json:"users,omitempty" yaml:"users,omitempty"` - Object string `json:"object,omitempty" yaml:"object,omitempty"` - Objects []string `json:"objects,omitempty" yaml:"objects,omitempty"` - Context *map[string]interface{} `json:"context" yaml:"context,omitempty"` - Assertions map[string]bool `json:"assertions" yaml:"assertions"` + User string `json:"user,omitempty" yaml:"user,omitempty"` + Users []string `json:"users,omitempty" yaml:"users,omitempty"` + Object string `json:"object,omitempty" yaml:"object,omitempty"` + Objects []string `json:"objects,omitempty" yaml:"objects,omitempty"` + Context *map[string]interface{} `json:"context" yaml:"context,omitempty"` + Assertions map[string]bool `json:"assertions" yaml:"assertions"` } type ModelTestListObjects struct { @@ -148,19 +156,19 @@ func (storeData *StoreData) Validate() error { for _, test := range storeData.Tests { for index, check := range test.Check { if check.User != "" && len(check.Users) > 0 { - msg := fmt.Sprintf("test %s check %d cannot contain both 'user' and 'users'", test.Name, index) - errs = errors.Join(errs, fmt.Errorf("%s", msg)) + err := fmt.Errorf("test %s check %d: %w", test.Name, index, ErrUserAndUsersConflict) + errs = errors.Join(errs, err) } else if check.User == "" && len(check.Users) == 0 { - msg := fmt.Sprintf("test %s check %d must specify 'user' or 'users'", test.Name, index) - errs = errors.Join(errs, fmt.Errorf("%s", msg)) + err := fmt.Errorf("test %s check %d: %w", test.Name, index, ErrUserRequired) + errs = errors.Join(errs, err) } if check.Object != "" && len(check.Objects) > 0 { - msg := fmt.Sprintf("test %s check %d cannot contain both 'object' and 'objects'", test.Name, index) - errs = errors.Join(errs, fmt.Errorf("%s", msg)) + err := fmt.Errorf("test %s check %d: %w", test.Name, index, ErrObjectAndObjectsConflict) + errs = errors.Join(errs, err) } else if check.Object == "" && len(check.Objects) == 0 { - msg := fmt.Sprintf("test %s check %d must specify 'object' or 'objects'", test.Name, index) - errs = errors.Join(errs, fmt.Errorf("%s", msg)) + err := fmt.Errorf("test %s check %d: %w", test.Name, index, ErrObjectRequired) + errs = errors.Join(errs, err) } } } diff --git a/internal/storetest/storedata_test.go b/internal/storetest/storedata_test.go index d1b82c05..942b97ff 100644 --- a/internal/storetest/storedata_test.go +++ b/internal/storetest/storedata_test.go @@ -5,7 +5,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v3" ) func TestStoreDataValidate(t *testing.T) { @@ -42,112 +41,38 @@ func TestStoreDataValidate(t *testing.T) { require.Error(t, invalidNone.Validate()) } -func TestModelTestCheckYAMLOmitEmpty(t *testing.T) { +func TestModelTestCheckStructTagsOmitEmpty(t *testing.T) { t.Parallel() - tests := []struct { - name string - input ModelTestCheck - expected string - }{ - { - name: "only user field populated", - input: ModelTestCheck{ - User: "user:anne", - Object: "folder:product-2021", - Assertions: map[string]bool{"can_view": true}, - }, - expected: `user: user:anne -object: folder:product-2021 -assertions: - can_view: true -`, - }, - { - name: "only users field populated", - input: ModelTestCheck{ - Users: []string{"user:anne", "user:bob"}, - Object: "folder:product-2021", - Assertions: map[string]bool{"can_view": true}, - }, - expected: `users: - - user:anne - - user:bob -object: folder:product-2021 -assertions: - can_view: true -`, - }, - { - name: "only object field populated", - input: ModelTestCheck{ - User: "user:anne", - Object: "folder:product-2021", - Assertions: map[string]bool{"can_view": true}, - }, - expected: `user: user:anne -object: folder:product-2021 -assertions: - can_view: true -`, - }, - { - name: "only objects field populated", - input: ModelTestCheck{ - User: "user:anne", - Objects: []string{"folder:product-2021", "folder:product-2022"}, - Assertions: map[string]bool{"can_view": true}, - }, - expected: `user: user:anne -objects: - - folder:product-2021 - - folder:product-2022 -assertions: - can_view: true -`, - }, - { - name: "users and objects fields populated", - input: ModelTestCheck{ - Users: []string{"user:anne", "user:bob"}, - Objects: []string{"folder:product-2021", "folder:product-2022"}, - Assertions: map[string]bool{"can_view": true}, - }, - expected: `users: - - user:anne - - user:bob -objects: - - folder:product-2021 - - folder:product-2022 -assertions: - can_view: true -`, - }, + // Test that struct can handle single user/object + checkSingle := ModelTestCheck{ + User: "user:anne", + Object: "folder:product-2021", + Assertions: map[string]bool{"can_view": true}, } + assert.NotEmpty(t, checkSingle.User) + assert.Empty(t, checkSingle.Users) + assert.NotEmpty(t, checkSingle.Object) + assert.Empty(t, checkSingle.Objects) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - yamlBytes, err := yaml.Marshal(tt.input) - require.NoError(t, err) - - assert.Equal(t, tt.expected, string(yamlBytes)) + // Test that struct can handle multiple users + checkUsers := ModelTestCheck{ + Users: []string{"user:anne", "user:bob"}, + Object: "folder:product-2021", + Assertions: map[string]bool{"can_view": true}, + } + assert.Empty(t, checkUsers.User) + assert.NotEmpty(t, checkUsers.Users) + assert.Len(t, checkUsers.Users, 2) - // Also verify that empty fields are not present in the output - yamlStr := string(yamlBytes) - if tt.input.User == "" { - assert.NotContains(t, yamlStr, "user: ") - } - if len(tt.input.Users) == 0 { - assert.NotContains(t, yamlStr, "users:") - } - if tt.input.Object == "" { - assert.NotContains(t, yamlStr, "object: ") - } - if len(tt.input.Objects) == 0 { - assert.NotContains(t, yamlStr, "objects:") - } - }) + // Test that struct can handle multiple objects + checkObjects := ModelTestCheck{ + User: "user:anne", + Objects: []string{"folder:product-2021", "folder:product-2022"}, + Assertions: map[string]bool{"can_view": true}, } + assert.NotEmpty(t, checkObjects.User) + assert.Empty(t, checkObjects.Object) + assert.NotEmpty(t, checkObjects.Objects) + assert.Len(t, checkObjects.Objects, 2) } From 7403288651d7f2ce03ffdc6e1a43682bb86dacc5 Mon Sep 17 00:00:00 2001 From: Andres Aguiar Date: Thu, 19 Jun 2025 09:03:19 -0300 Subject: [PATCH 13/15] chore: removed unneeded file and extra whitespaces in .fga.yaml --- example/model.fga.yaml | 2 +- example/school.yaml | 74 ------------------------------------------ 2 files changed, 1 insertion(+), 75 deletions(-) delete mode 100644 example/school.yaml diff --git a/example/model.fga.yaml b/example/model.fga.yaml index 462ace72..46740f7b 100644 --- a/example/model.fga.yaml +++ b/example/model.fga.yaml @@ -54,7 +54,7 @@ tests: - folder:product-2021Q1 assertions: can_view: true - can_write: true + can_write: true list_objects: # Each list objects test is made of: a user, an object type and the expected result for one or more relations - user: user:anne type: folder diff --git a/example/school.yaml b/example/school.yaml deleted file mode 100644 index 64af8261..00000000 --- a/example/school.yaml +++ /dev/null @@ -1,74 +0,0 @@ - -model: | - model - schema 1.1 - - type user - - type school - relations - define member:[user] - define restricted_member:[user] - define approved_restricted_member:[user] - - define reader:member - define restricted_reader:approved_restricted_member - - type repository - relations - define direct_reader:[user] - define school_reader:[school] - define restricted_school_reader:[school] - define reader: direct_reader or reader from school_reader or restricted_reader from restricted_school_reader - -tuples: - - user: user:alice - relation: direct_reader - object: repository:math-notes - - - user: user:bob - relation: member - object: school:harvard - - - user: school:harvard - relation: school_reader - object: repository:physics - - - user: user:charlie - relation: restricted_member - object: school:mit - - - user: user:charlie - relation: approved_restricted_member - object: school:mit - - - user: school:mit - relation: restricted_school_reader - object: repository:confidential-lab - -tests: - - check: - - user: user:alice - object: repository:math-notes - assertions: - reader : true - - - user: user:bob - object: repository:physics - assertions: - reader : true - - - user: user:charlie - object: repository:confidential-lab - assertions: - reader : true - - - user: user:bob - object: repository:confidential-lab - assertions: - reader : false - - - user: user:alice - object: repository:physics - assertions: - reader : false \ No newline at end of file From 90e9dd82374b3a4283140cdbf691ba7f43556a57 Mon Sep 17 00:00:00 2001 From: Andres Aguiar Date: Thu, 19 Jun 2025 09:20:27 -0300 Subject: [PATCH 14/15] chore: improved example --- example/model.fga.yaml | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/example/model.fga.yaml b/example/model.fga.yaml index 46740f7b..965c3c35 100644 --- a/example/model.fga.yaml +++ b/example/model.fga.yaml @@ -49,12 +49,31 @@ tests: can_share: false - users: - user:anne + - user:beth + object: folder:product-2021 + assertions: + # These assertions are run for each user + can_view: true + can_share: false + - user: user:anne objects: - folder:product-2021 - folder:product-2021Q1 assertions: + # These assertions are run for each object can_view: true - can_write: true + can_write: true + - users: + - user:peter + - user:john + objects: + - folder:product-2021 + - folder:product-2021Q1 + assertions: + # These assertions are run for each user and each object + can_view: false + can_write: false + list_objects: # Each list objects test is made of: a user, an object type and the expected result for one or more relations - user: user:anne type: folder @@ -78,7 +97,6 @@ tests: can_write: [] can_share: [] - list_users: # Each list user test is made of: an object, a user filter and the expected result for one or more relations - object: folder:product-2021 user_filter: From 0044a634e47cd2edfa1c262532869a0c63c2b3f0 Mon Sep 17 00:00:00 2001 From: Andres Aguiar Date: Thu, 19 Jun 2025 09:43:50 -0300 Subject: [PATCH 15/15] chore: removed trailing whitespace --- example/model.fga.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/example/model.fga.yaml b/example/model.fga.yaml index 965c3c35..86dad78b 100644 --- a/example/model.fga.yaml +++ b/example/model.fga.yaml @@ -52,17 +52,17 @@ tests: - user:beth object: folder:product-2021 assertions: - # These assertions are run for each user + # These assertions are run for each user can_view: true can_share: false - - user: user:anne + - user: user:anne objects: - folder:product-2021 - folder:product-2021Q1 assertions: # These assertions are run for each object can_view: true - can_write: true + can_write: true - users: - user:peter - user:john