diff --git a/README.md b/README.md index 31fa8e5f..dd90e3a6 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,22 @@ tests: assertions: member: true moderator: false + # 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: + 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 +574,35 @@ 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 + # 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 assertions: diff --git a/cmd/store/import.go b/cmd/store/import.go index a6ffb27a..b334c725 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 @@ -232,13 +236,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 := storetest.GetEffectiveUsers(checkTest) + objects := storetest.GetEffectiveObjects(checkTest) + + for _, user := range users { + 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 abaa9c07..edb0e031 100644 --- a/cmd/store/import_test.go +++ b/cmd/store/import_test.go @@ -21,6 +21,36 @@ 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, + }, + } + + 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} @@ -38,9 +68,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 +85,54 @@ 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: "import store with multi object 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{ + { + User: "user:peter", + Objects: []string{"document:doc1", "document:doc2"}, + Assertions: map[string]bool{"reader": true}, + }, + }, + }, + }, + }, + }, { name: "create new store without assertions", mockWriteAssertions: false, @@ -107,7 +185,16 @@ func TestImportStore(t *testing.T) { defer mockCtrl.Finish() if test.mockWriteAssertions { - setupWriteAssertionsMock(mockCtrl, mockFgaClient, expectedAssertions, expectedOptions) + expected := expectedAssertions + + 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) } else { mockFgaClient.EXPECT().WriteAssertions(gomock.Any()).Times(0) } 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 diff --git a/example/model.fga.yaml b/example/model.fga.yaml index a7cfaaad..86dad78b 100644 --- a/example/model.fga.yaml +++ b/example/model.fga.yaml @@ -47,6 +47,33 @@ tests: can_view: true can_write: true 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 + - 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 @@ -69,6 +96,7 @@ 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: diff --git a/internal/storetest/localtest.go b/internal/storetest/localtest.go index 466bfb85..6f6cec61 100644 --- a/internal/storetest/localtest.go +++ b/internal/storetest/localtest.go @@ -25,54 +25,60 @@ func RunLocalCheckTest( options ModelTestOptions, ) []ModelTestCheckSingleResult { results := []ModelTestCheckSingleResult{} + users := GetEffectiveUsers(checkTest) + + objects := GetEffectiveObjects(checkTest) + for _, user := range users { + 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, + } - 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 + ) - 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: checkTest.User, - Relation: relation, - Object: checkTest.Object, - }, - Context: ctx, - }, - ) - if err != nil { - result.Error = err - } + 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) } return results 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/remotetest.go b/internal/storetest/remotetest.go index 864673a7..e13d9e2a 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 := GetEffectiveUsers(checkTest) + objects := GetEffectiveObjects(checkTest) + + for _, user := range users { + 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) + } + } } return results diff --git a/internal/storetest/storedata.go b/internal/storetest/storedata.go index 56ad6889..d47f33f7 100644 --- a/internal/storetest/storedata.go +++ b/internal/storetest/storedata.go @@ -26,14 +26,25 @@ 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" ) +// 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" yaml:"user"` - Object string `json:"object" yaml:"object"` - 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 { @@ -137,3 +148,34 @@ func (storeData *StoreData) LoadTuples(basePath string) error { return nil } + +//nolint:cyclop +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 { + 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 { + err := fmt.Errorf("test %s check %d: %w", test.Name, index, ErrUserRequired) + errs = errors.Join(errs, err) + } + + if check.Object != "" && len(check.Objects) > 0 { + 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 { + err := fmt.Errorf("test %s check %d: %w", test.Name, index, ErrObjectRequired) + errs = errors.Join(errs, err) + } + } + } + + if errs != nil { + return clierrors.ValidationError("StoreFormat", 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..942b97ff --- /dev/null +++ b/internal/storetest/storedata_test.go @@ -0,0 +1,78 @@ +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()) + + 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{{ + User: "user:1", Assertions: map[string]bool{"read": true}, + }}}}} + require.Error(t, invalidNone.Validate()) +} + +func TestModelTestCheckStructTagsOmitEmpty(t *testing.T) { + t.Parallel() + + // 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) + + // 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) + + // 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) +} 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 diff --git a/internal/storetest/utils.go b/internal/storetest/utils.go new file mode 100644 index 00000000..0eb56980 --- /dev/null +++ b/internal/storetest/utils.go @@ -0,0 +1,17 @@ +package storetest + +func GetEffectiveUsers(checkTest ModelTestCheck) []string { + if len(checkTest.Users) > 0 { + return checkTest.Users + } + + return []string{checkTest.User} +} + +func GetEffectiveObjects(checkTest ModelTestCheck) []string { + if len(checkTest.Objects) > 0 { + return checkTest.Objects + } + + return []string{checkTest.Object} +}