diff --git a/assets/tests/consolidated_1_1_tests.yaml b/assets/tests/consolidated_1_1_tests.yaml index adce082d70..ef5c52ad51 100644 --- a/assets/tests/consolidated_1_1_tests.yaml +++ b/assets/tests/consolidated_1_1_tests.yaml @@ -6517,7 +6517,9 @@ tests: object: document:1 relation: can_view expectation: - - user:* # TODO but user:bob doesn't have access + - user:* + expectedExcludedUsers: + - user:bob - request: filters: - user diff --git a/internal/test/listusers/listusers.go b/internal/test/listusers/listusers.go index e35e4de373..19f47cbfae 100644 --- a/internal/test/listusers/listusers.go +++ b/internal/test/listusers/listusers.go @@ -11,11 +11,12 @@ import ( ) type Assertion struct { - Request *TestListUsersRequest - ContextualTuples []*openfgav1.TupleKey `json:"contextualTuples"` - Context *structpb.Struct - Expectation []string - ErrorCode int `json:"errorCode"` // If ErrorCode is non-zero then we expect that the ListUsers call failed. + Request *TestListUsersRequest + ContextualTuples []*openfgav1.TupleKey `json:"contextualTuples"` + Context *structpb.Struct + Expectation []string + ExpectedExcludedUsers []string `json:"expectedExcludedUsers"` + ErrorCode int `json:"errorCode"` // If ErrorCode is non-zero then we expect that the ListUsers call failed. } type TestListUsersRequest struct { @@ -28,14 +29,23 @@ func (t *TestListUsersRequest) ToString() string { return fmt.Sprintf("object=%s, relation=%s, filters=%v", t.Object, t.Relation, strings.Join(t.Filters, ", ")) } -func FromProtoResponse(r *openfgav1.ListUsersResponse) []string { +func FromUsersProto(r []*openfgav1.User) []string { var users []string - for _, user := range r.GetUsers() { + for _, user := range r { users = append(users, tuple.UserProtoToString(user)) } return users } +func FromObjectOrUsersetProto(u []*openfgav1.ObjectOrUserset) []string { + var users []string + for _, user := range u { + users = append(users, tuple.FromObjectOrUsersetProto(user)) + } + + return users +} + func (t *TestListUsersRequest) ToProtoRequest() *openfgav1.ListUsersRequest { var protoFilters []*openfgav1.UserTypeFilter diff --git a/pkg/server/commands/listusers/list_users.go b/pkg/server/commands/listusers/list_users.go index 3228c68b72..6bf3466922 100644 --- a/pkg/server/commands/listusers/list_users.go +++ b/pkg/server/commands/listusers/list_users.go @@ -95,8 +95,9 @@ func (r *internalListUsersRequest) GetContext() *structpb.Struct { } type listUsersResponse struct { - Users []*openfgav1.User - Metadata listUsersResponseMetadata + Users []*openfgav1.User + ExcludedUsers []*openfgav1.ObjectOrUserset + Metadata listUsersResponseMetadata } type listUsersResponseMetadata struct { @@ -110,6 +111,13 @@ func (r *listUsersResponse) GetUsers() []*openfgav1.User { return r.Users } +func (r *listUsersResponse) GetExcludedUsers() []*openfgav1.ObjectOrUserset { + if r == nil { + return []*openfgav1.ObjectOrUserset{} + } + return r.ExcludedUsers +} + func (r *listUsersResponse) GetMetadata() listUsersResponseMetadata { if r == nil { return listUsersResponseMetadata{} diff --git a/pkg/server/commands/listusers/list_users_rpc.go b/pkg/server/commands/listusers/list_users_rpc.go index d9a64cc5e8..308d48828b 100644 --- a/pkg/server/commands/listusers/list_users_rpc.go +++ b/pkg/server/commands/listusers/list_users_rpc.go @@ -47,6 +47,33 @@ type expandResponse struct { err error } +// userRelationshipStatus represents the status of a relationship that a given user/subject has with respect to a specific relation. +// +// A user/subject either does or does not have a relationship, which represents that +// they either explicitly do have a relationship or explicitly do not. +type userRelationshipStatus int + +const ( + HasRelationship userRelationshipStatus = iota + NoRelationship +) + +type foundUser struct { + user *openfgav1.User + excludedUsers []*openfgav1.User + + // relationshipStatus indicates whether the user explicitly does or does not have + // a specific relationship with respect to the relation being expanded. It almost + // exclusively applies to behavior stemming from exclusion rewrite rules. + // + // As users/subjects are being expanded we propagate the relationship status with + // respect to the relation being evaluated so that we can handle subjects which + // have been explicitly excluded from a relationship and where that relation is + // contained under the subtracted branch of another exclusion. This allows us to + // buble up the subject from the subtracted branch of the exclusion. + relationshipStatus userRelationshipStatus +} + type ListUsersQueryOption func(l *listUsersQuery) func WithListUsersQueryLogger(l logger.Logger) ListUsersQueryOption { @@ -126,11 +153,12 @@ func (l *listUsersQuery) ListUsers( defer span.End() cancellableCtx, cancelCtx := context.WithCancel(ctx) - defer cancelCtx() if l.deadline != 0 { cancellableCtx, cancelCtx = context.WithTimeout(cancellableCtx, l.deadline) defer cancelCtx() } + defer cancelCtx() + l.ds = storagewrappers.NewCombinedTupleReader( storagewrappers.NewBoundedConcurrencyTupleReader(l.ds, l.maxConcurrentReads), req.GetContextualTuples(), @@ -164,11 +192,17 @@ func (l *listUsersQuery) ListUsers( foundUsersCh := l.buildResultsChannel() expandErrCh := make(chan error, 1) - foundUsersUnique := make(map[tuple.UserString]struct{}, 1000) + foundUsersUnique := make(map[tuple.UserString]foundUser, 1000) + excludedUsersUnique := make(map[tuple.UserString]struct{}, 1000) + doneWithFoundUsersCh := make(chan struct{}, 1) go func() { - for foundObject := range foundUsersCh { - foundUsersUnique[tuple.UserProtoToString(foundObject)] = struct{}{} + for foundUser := range foundUsersCh { + foundUsersUnique[tuple.UserProtoToString(foundUser.user)] = foundUser + for _, exception := range foundUser.excludedUsers { + excludedUsersUnique[tuple.UserProtoToString(exception)] = struct{}{} + } + if l.maxResults > 0 { if uint32(len(foundUsersUnique)) >= l.maxResults { span.SetAttributes(attribute.Bool("max_results_found", true)) @@ -206,13 +240,30 @@ func (l *listUsersQuery) ListUsers( cancelCtx() foundUsers := make([]*openfgav1.User, 0, len(foundUsersUnique)) - for foundUser := range foundUsersUnique { - foundUsers = append(foundUsers, tuple.StringToUserProto(foundUser)) + for foundUserKey, foundUser := range foundUsersUnique { + if foundUser.relationshipStatus == NoRelationship { + continue + } + if _, userIsExcluded := excludedUsersUnique[foundUserKey]; userIsExcluded { + continue + } + + foundUsers = append(foundUsers, tuple.StringToUserProto(foundUserKey)) + } + + var excludedUsers []*openfgav1.ObjectOrUserset + if len(foundUsers) > 0 { + excludedUsers = make([]*openfgav1.ObjectOrUserset, 0, len(excludedUsersUnique)) + for foundExcludedUser := range excludedUsersUnique { + excludedUsers = append(excludedUsers, tuple.StringToObjectOrUserset(foundExcludedUser)) + } } + span.SetAttributes(attribute.Int("result_count", len(foundUsers))) return &listUsersResponse{ - Users: foundUsers, + Users: foundUsers, + ExcludedUsers: excludedUsers, Metadata: listUsersResponseMetadata{ DatastoreQueryCount: datastoreQueryCount.Load(), }, @@ -238,7 +289,7 @@ func doesHavePossibleEdges(typesys *typesystem.TypeSystem, req *openfgav1.ListUs func (l *listUsersQuery) expand( ctx context.Context, req *internalListUsersRequest, - foundUsersChan chan<- *openfgav1.User, + foundUsersChan chan<- foundUser, ) expandResponse { ctx, span := tracer.Start(ctx, "expand") defer span.End() @@ -263,16 +314,17 @@ func (l *listUsersQuery) expand( for _, userFilter := range req.GetUserFilters() { if reqObjectType == userFilter.GetType() && reqRelation == userFilter.GetRelation() { - user := &openfgav1.User{ - User: &openfgav1.User_Userset{ - Userset: &openfgav1.UsersetUser{ - Type: reqObjectType, - Id: reqObjectID, - Relation: reqRelation, + trySendResult(ctx, foundUser{ + user: &openfgav1.User{ + User: &openfgav1.User_Userset{ + Userset: &openfgav1.UsersetUser{ + Type: reqObjectType, + Id: reqObjectID, + Relation: reqRelation, + }, }, }, - } - trySendResult(ctx, user, foundUsersChan) + }, foundUsersChan) } } @@ -305,7 +357,7 @@ func (l *listUsersQuery) expandRewrite( ctx context.Context, req *internalListUsersRequest, rewrite *openfgav1.Userset, - foundUsersChan chan<- *openfgav1.User, + foundUsersChan chan<- foundUser, ) expandResponse { ctx, span := tracer.Start(ctx, "expandRewrite") defer span.End() @@ -325,20 +377,7 @@ func (l *listUsersQuery) expandRewrite( case *openfgav1.Userset_Difference: resp = l.expandExclusion(ctx, req, rewrite, foundUsersChan) case *openfgav1.Userset_Union: - pool := pool.New().WithContext(ctx) - pool.WithCancelOnError() - pool.WithMaxGoroutines(int(l.resolveNodeBreadthLimit)) - - children := rewrite.Union.GetChild() - for _, childRewrite := range children { - childRewriteCopy := childRewrite - pool.Go(func(ctx context.Context) error { - resp := l.expandRewrite(ctx, req, childRewriteCopy, foundUsersChan) - return resp.err - }) - } - - resp.err = pool.Wait() + resp = l.expandUnion(ctx, req, rewrite, foundUsersChan) default: panic("unexpected userset rewrite encountered") } @@ -352,7 +391,7 @@ func (l *listUsersQuery) expandRewrite( func (l *listUsersQuery) expandDirect( ctx context.Context, req *internalListUsersRequest, - foundUsersChan chan<- *openfgav1.User, + foundUsersChan chan<- foundUser, ) expandResponse { ctx, span := tracer.Start(ctx, "expandDirect") defer span.End() @@ -426,7 +465,10 @@ LoopOnIterator: for _, f := range req.GetUserFilters() { if f.GetType() == userObjectType { user := tuple.StringToUserProto(tuple.BuildObject(userObjectType, userObjectID)) - trySendResult(ctx, user, foundUsersChan) + + trySendResult(ctx, foundUser{ + user: user, + }, foundUsersChan) } } continue @@ -458,7 +500,7 @@ func (l *listUsersQuery) expandIntersection( ctx context.Context, req *internalListUsersRequest, rewrite *openfgav1.Userset_Intersection, - foundUsersChan chan<- *openfgav1.User, + foundUsersChan chan<- foundUser, ) expandResponse { ctx, span := tracer.Start(ctx, "expandIntersection") defer span.End() @@ -467,11 +509,11 @@ func (l *listUsersQuery) expandIntersection( pool.WithMaxGoroutines(int(l.resolveNodeBreadthLimit)) childOperands := rewrite.Intersection.GetChild() - intersectionFoundUsersChans := make([]chan *openfgav1.User, len(childOperands)) + intersectionFoundUsersChans := make([]chan foundUser, len(childOperands)) for i, rewrite := range childOperands { i := i rewrite := rewrite - intersectionFoundUsersChans[i] = make(chan *openfgav1.User, 1) + intersectionFoundUsersChans[i] = make(chan foundUser, 1) pool.Go(func(ctx context.Context) error { resp := l.expandRewrite(ctx, req, rewrite, intersectionFoundUsersChans[i]) return resp.err @@ -497,12 +539,22 @@ func (l *listUsersQuery) expandIntersection( wildcardCount := atomic.Uint32{} wildcardKey := tuple.TypedPublicWildcard(req.GetUserFilters()[0].GetType()) foundUsersCountMap := make(map[string]uint32, 0) + excludedUsersMap := make(map[string]struct{}, 0) for _, foundUsersChan := range intersectionFoundUsersChans { - go func(foundUsersChan chan *openfgav1.User) { + go func(foundUsersChan chan foundUser) { defer wg.Done() foundUsersMap := make(map[string]uint32, 0) for foundUser := range foundUsersChan { - key := tuple.UserProtoToString(foundUser) + key := tuple.UserProtoToString(foundUser.user) + for _, excludedUser := range foundUser.excludedUsers { + key := tuple.UserProtoToString(excludedUser) + mu.Lock() + excludedUsersMap[key] = struct{}{} + mu.Unlock() + } + if foundUser.relationshipStatus == NoRelationship { + continue + } foundUsersMap[key]++ } @@ -526,14 +578,108 @@ func (l *listUsersQuery) expandIntersection( } wg.Wait() + excludedUsers := []*openfgav1.User{} + for key := range excludedUsersMap { + excludedUsers = append(excludedUsers, tuple.StringToUserProto(key)) + } + for key, count := range foundUsersCountMap { // Compare the number of times the specific user was returned for // all intersection operands plus the number of wildcards. // If this summed value equals the number of operands, the user satisfies // the intersection expression and can be sent on `foundUsersChan` if (count + wildcardCount.Load()) == uint32(len(childOperands)) { - trySendResult(ctx, tuple.StringToUserProto(key), foundUsersChan) + fu := foundUser{ + user: tuple.StringToUserProto(key), + excludedUsers: excludedUsers, + } + trySendResult(ctx, fu, foundUsersChan) + } + } + + return expandResponse{ + err: <-errChan, + } +} + +func (l *listUsersQuery) expandUnion( + ctx context.Context, + req *internalListUsersRequest, + rewrite *openfgav1.Userset_Union, + foundUsersChan chan<- foundUser, +) expandResponse { + ctx, span := tracer.Start(ctx, "expandUnion") + defer span.End() + pool := pool.New().WithContext(ctx) + pool.WithCancelOnError() + pool.WithMaxGoroutines(int(l.resolveNodeBreadthLimit)) + + childOperands := rewrite.Union.GetChild() + unionFoundUsersChans := make([]chan foundUser, len(childOperands)) + for i, rewrite := range childOperands { + i := i + rewrite := rewrite + unionFoundUsersChans[i] = make(chan foundUser, 1) + pool.Go(func(ctx context.Context) error { + resp := l.expandRewrite(ctx, req, rewrite, unionFoundUsersChans[i]) + return resp.err + }) + } + + errChan := make(chan error, 1) + + go func() { + err := pool.Wait() + for i := range unionFoundUsersChans { + close(unionFoundUsersChans[i]) + } + errChan <- err + close(errChan) + }() + + var mu sync.Mutex + + var wg sync.WaitGroup + wg.Add(len(childOperands)) + + foundUsersMap := make(map[string]struct{}, 0) + excludedUsersCountMap := make(map[string]uint32, 0) + for _, foundUsersChan := range unionFoundUsersChans { + go func(foundUsersChan chan foundUser) { + defer wg.Done() + + for foundUser := range foundUsersChan { + key := tuple.UserProtoToString(foundUser.user) + for _, excludedUser := range foundUser.excludedUsers { + key := tuple.UserProtoToString(excludedUser) + mu.Lock() + excludedUsersCountMap[key]++ + mu.Unlock() + } + if foundUser.relationshipStatus == NoRelationship { + continue + } + mu.Lock() + foundUsersMap[key] = struct{}{} + mu.Unlock() + } + }(foundUsersChan) + } + wg.Wait() + + excludedUsers := []*openfgav1.User{} + for key, count := range excludedUsersCountMap { + if count == uint32(len(childOperands)) { + excludedUsers = append(excludedUsers, tuple.StringToUserProto(key)) + } + } + + for key := range foundUsersMap { + fu := foundUser{ + user: tuple.StringToUserProto(key), + excludedUsers: excludedUsers, } + trySendResult(ctx, fu, foundUsersChan) } return expandResponse{ @@ -545,12 +691,12 @@ func (l *listUsersQuery) expandExclusion( ctx context.Context, req *internalListUsersRequest, rewrite *openfgav1.Userset_Difference, - foundUsersChan chan<- *openfgav1.User, + foundUsersChan chan<- foundUser, ) expandResponse { ctx, span := tracer.Start(ctx, "expandExclusion") defer span.End() - baseFoundUsersCh := make(chan *openfgav1.User, 1) - subtractFoundUsersCh := make(chan *openfgav1.User, 1) + baseFoundUsersCh := make(chan foundUser, 1) + subtractFoundUsersCh := make(chan foundUser, 1) var baseError error go func() { @@ -568,15 +714,17 @@ func (l *listUsersQuery) expandExclusion( close(subtractFoundUsersCh) }() - baseFoundUsersMap := make(map[string]struct{}, 0) + baseFoundUsersMap := make(map[string]foundUser, 0) for fu := range baseFoundUsersCh { - key := tuple.UserProtoToString(fu) - baseFoundUsersMap[key] = struct{}{} + key := tuple.UserProtoToString(fu.user) + baseFoundUsersMap[key] = fu } - subtractFoundUsersMap := make(map[string]struct{}, len(baseFoundUsersMap)) + + subtractFoundUsersMap := make(map[string]foundUser, len(baseFoundUsersMap)) + for fu := range subtractFoundUsersCh { - key := tuple.UserProtoToString(fu) - subtractFoundUsersMap[key] = struct{}{} + key := tuple.UserProtoToString(fu.user) + subtractFoundUsersMap[key] = fu } if subtractHasCycle { @@ -591,14 +739,72 @@ func (l *listUsersQuery) expandExclusion( } wildcardKey := tuple.TypedPublicWildcard(req.GetUserFilters()[0].GetType()) + + _, baseWildcardExists := baseFoundUsersMap[wildcardKey] _, subtractWildcardExists := subtractFoundUsersMap[wildcardKey] - for key := range baseFoundUsersMap { - if _, isSubtracted := subtractFoundUsersMap[key]; !isSubtracted && !subtractWildcardExists { - // Iterate over base users because at minimum they need to pass - // but then they are further compared to the subtracted users map. - // If users exist in both maps, they are excluded. Only users that exist - // solely in the base map will be returned. - trySendResult(ctx, tuple.StringToUserProto(key), foundUsersChan) + + for userKey, fu := range baseFoundUsersMap { + subtractedUser, userIsSubtracted := subtractFoundUsersMap[userKey] + _, wildcardSubtracted := subtractFoundUsersMap[wildcardKey] + + switch { + case baseWildcardExists: + if !userIsSubtracted && !wildcardSubtracted { + trySendResult(ctx, foundUser{ + user: tuple.StringToUserProto(userKey), + }, foundUsersChan) + } + + for subtractedUserKey, subtractedFu := range subtractFoundUsersMap { + if tuple.IsTypedWildcard(subtractedUserKey) { + if !userIsSubtracted { + trySendResult(ctx, foundUser{ + user: tuple.StringToUserProto(userKey), + relationshipStatus: NoRelationship, + }, foundUsersChan) + } + continue + } + + if subtractedFu.relationshipStatus == NoRelationship { + trySendResult(ctx, foundUser{ + user: tuple.StringToUserProto(subtractedUserKey), + relationshipStatus: HasRelationship, + }, foundUsersChan) + } + + // a found user under the subtracted branch causes the subtracted user to have a negated relationship with respect + // to the base relation and is excluded since a wildcard is contained under the base branch. + if subtractedFu.relationshipStatus == HasRelationship { + trySendResult(ctx, foundUser{ + user: tuple.StringToUserProto(subtractedUserKey), + relationshipStatus: NoRelationship, + excludedUsers: []*openfgav1.User{ + tuple.StringToUserProto(subtractedUserKey), + }, + }, foundUsersChan) + } + } + case subtractWildcardExists, userIsSubtracted: + if subtractedUser.relationshipStatus == HasRelationship { + trySendResult(ctx, foundUser{ + user: tuple.StringToUserProto(userKey), + relationshipStatus: NoRelationship, + }, foundUsersChan) + } + + if subtractedUser.relationshipStatus == NoRelationship { + trySendResult(ctx, foundUser{ + user: tuple.StringToUserProto(userKey), + relationshipStatus: HasRelationship, + }, foundUsersChan) + } + + default: + trySendResult(ctx, foundUser{ + user: tuple.StringToUserProto(userKey), + relationshipStatus: fu.relationshipStatus, + }, foundUsersChan) } } @@ -615,7 +821,7 @@ func (l *listUsersQuery) expandTTU( ctx context.Context, req *internalListUsersRequest, rewrite *openfgav1.Userset_TupleToUserset, - foundUsersChan chan<- *openfgav1.User, + foundUsersChan chan<- foundUser, ) expandResponse { ctx, span := tracer.Start(ctx, "expandTTU") defer span.End() @@ -714,16 +920,17 @@ func enteredCycle(req *internalListUsersRequest) bool { return false } -func (l *listUsersQuery) buildResultsChannel() chan *openfgav1.User { - foundUsersCh := make(chan *openfgav1.User, serverconfig.DefaultListUsersMaxResults) +func (l *listUsersQuery) buildResultsChannel() chan foundUser { + foundUsersCh := make(chan foundUser, serverconfig.DefaultListUsersMaxResults) maxResults := l.maxResults if maxResults > 0 { - foundUsersCh = make(chan *openfgav1.User, maxResults) + foundUsersCh = make(chan foundUser, maxResults) } + return foundUsersCh } -func trySendResult(ctx context.Context, user *openfgav1.User, foundUsersCh chan<- *openfgav1.User) { +func trySendResult(ctx context.Context, user foundUser, foundUsersCh chan<- foundUser) { select { case <-ctx.Done(): return diff --git a/pkg/server/commands/listusers/list_users_rpc_test.go b/pkg/server/commands/listusers/list_users_rpc_test.go index c7723aa0d7..80c6cdbab1 100644 --- a/pkg/server/commands/listusers/list_users_rpc_test.go +++ b/pkg/server/commands/listusers/list_users_rpc_test.go @@ -3,6 +3,7 @@ package listusers import ( "context" "fmt" + "sync/atomic" "testing" "time" @@ -20,6 +21,7 @@ import ( "github.com/openfga/openfga/pkg/storage/memory" "github.com/openfga/openfga/pkg/storage/storagewrappers" + storagetest "github.com/openfga/openfga/pkg/storage/test" "github.com/openfga/openfga/pkg/testutils" "github.com/openfga/openfga/pkg/tuple" "github.com/openfga/openfga/pkg/typesystem" @@ -31,6 +33,7 @@ type ListUsersTests []struct { model string tuples []*openfgav1.TupleKey expectedUsers []string + butNot []string expectedErrorMsg string } @@ -504,12 +507,12 @@ func TestListUsersUsersets(t *testing.T) { model: `model schema 1.1 type user - + type org relations define member: [user] define admin: [org#member] - + type repo relations define owner: [org] @@ -536,7 +539,7 @@ func TestListUsersUsersets(t *testing.T) { model: `model schema 1.1 type user - + type org relations define member: [user] @@ -627,14 +630,14 @@ func TestListUsersTTU(t *testing.T) { model: `model schema 1.1 type user - + type folder relations define owner: [user] define editor: [user] or owner define viewer: [user] or owner or editor define unrelated_not_computed: [user] - + type document relations define parent: [folder] @@ -665,8 +668,8 @@ func TestListUsersTTU(t *testing.T) { }, model: `model schema 1.1 - type user - type folder + type user + type folder relations define parent: [folder] define viewer: [user] or viewer from parent`, @@ -1029,7 +1032,7 @@ func TestListUsersConditions(t *testing.T) { condition isEqualToFive(param1: int) { param1 == 5 } - + condition isEqualToTen(param2: int) { param2 == 10 }`, @@ -1063,7 +1066,7 @@ func TestListUsersConditions(t *testing.T) { condition isEqualToFive(param1: int) { param1 == 5 } - + condition isEqualToTen(param2: int) { param2 == 10 }`, @@ -1279,6 +1282,105 @@ func TestListUsersIntersection(t *testing.T) { }, expectedUsers: []string{"user:will"}, }, + { + name: "excluded_subjects_of_subtracted_branches_propagate", + req: &openfgav1.ListUsersRequest{ + Object: &openfgav1.Object{Type: "document", Id: "1"}, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{ + { + Type: "user", + }, + }, + }, + model: `model + schema 1.1 + type user + + type document + relations + define viewer: viewer1 and viewer2 + define viewer1: [user,user:*] but not blocked + define viewer2: [user,user:*] but not blocked + define blocked: [user,user:*]`, + + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer1", "user:*"), + tuple.NewTupleKey("document:1", "viewer2", "user:*"), + + tuple.NewTupleKey("document:1", "blocked", "user:maria"), + }, + expectedUsers: []string{"user:*"}, + butNot: []string{"user:maria"}, + }, + { + name: "intersection_and_excluded_users_1", + req: &openfgav1.ListUsersRequest{ + Object: &openfgav1.Object{Type: "document", Id: "1"}, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{ + { + Type: "user", + }, + }, + }, + model: ` + model + schema 1.1 + + type user + type document + relations + define b: [user:*] but not b1 + define b1: [user] + define a: [user:*] but not a1 + define a1: [user] + define viewer: a and b`, + + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "b", "user:*"), + tuple.NewTupleKey("document:1", "b1", "user:will"), + tuple.NewTupleKey("document:1", "a", "user:*"), + tuple.NewTupleKey("document:1", "a1", "user:maria"), + }, + expectedUsers: []string{"user:*"}, + butNot: []string{"user:maria", "user:will"}, + }, + { + name: "intersection_and_excluded_users_2", + req: &openfgav1.ListUsersRequest{ + Object: &openfgav1.Object{Type: "document", Id: "1"}, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{ + { + Type: "user", + }, + }, + }, + model: `model + schema 1.1 + type user + + type document + relations + define viewer: viewer1 and viewer2 + define viewer1: [user,user:*] but not blocked1 + define viewer2: [user,user:*] but not blocked2 + define blocked1: [user,user:*] + define blocked2: [user,user:*]`, + + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer1", "user:*"), + tuple.NewTupleKey("document:1", "viewer2", "user:*"), + + tuple.NewTupleKey("document:1", "blocked1", "user:will"), + + tuple.NewTupleKey("document:1", "blocked1", "user:maria"), + tuple.NewTupleKey("document:1", "blocked2", "user:maria"), + }, + expectedUsers: []string{"user:*"}, + butNot: []string{"user:maria", "user:will"}, + }, } tests.runListUsersTestCases(t) } @@ -1381,6 +1483,72 @@ func TestListUsersUnion(t *testing.T) { }, expectedUsers: []string{"user:jon", "user:maria", "user:will"}, }, + { + name: "union_and_excluded_users_1", + req: &openfgav1.ListUsersRequest{ + Object: &openfgav1.Object{Type: "document", Id: "1"}, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{ + { + Type: "user", + }, + }, + }, + model: `model + schema 1.1 + type user + + type document + relations + define viewer: viewer1 or viewer2 + define viewer1: [user,user:*] but not blocked + define viewer2: [user,user:*] but not blocked + define blocked: [user,user:*]`, + + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer1", "user:*"), + tuple.NewTupleKey("document:1", "viewer2", "user:*"), + + tuple.NewTupleKey("document:1", "blocked", "user:maria"), + }, + expectedUsers: []string{"user:*"}, + butNot: []string{"user:maria"}, + }, + { + name: "union_and_excluded_users_2", + req: &openfgav1.ListUsersRequest{ + Object: &openfgav1.Object{Type: "document", Id: "1"}, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{ + { + Type: "user", + }, + }, + }, + model: `model + schema 1.1 + type user + + type document + relations + define viewer: viewer1 or viewer2 + define viewer1: [user,user:*] but not blocked1 + define viewer2: [user,user:*] but not blocked2 + define blocked1: [user] + define blocked2: [user]`, + + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer1", "user:*"), + tuple.NewTupleKey("document:1", "viewer2", "user:*"), + + tuple.NewTupleKey("document:1", "blocked1", "user:will"), + + tuple.NewTupleKey("document:1", "blocked1", "user:maria"), + tuple.NewTupleKey("document:1", "blocked2", "user:maria"), + }, + expectedUsers: []string{"user:*"}, + butNot: []string{"user:maria"}, + }, } tests.runListUsersTestCases(t) } @@ -1465,13 +1633,13 @@ func TestListUsersExclusion(t *testing.T) { }, model: `model schema 1.1 - + type org relations define blocked: [user] - - type user - + + type user + type document relations define parent: [org] @@ -1502,19 +1670,19 @@ func TestListUsersExclusion(t *testing.T) { }, model: `model schema 1.1 - + type user - + type org relations define blocked: [user] - + type folder relations define blocked: blocked from org define org: [org] define viewer: [user] - + type document relations define parent: [folder] @@ -1545,9 +1713,9 @@ func TestListUsersExclusion(t *testing.T) { }, model: `model schema 1.1 - + type user - + type group relations define member: [user, group#member] but not blocked @@ -1603,9 +1771,9 @@ func TestListUsersExclusion(t *testing.T) { }, model: `model schema 1.1 - + type user - + type document relations define blocked: [user, document#viewer] @@ -1628,9 +1796,9 @@ func TestListUsersExclusionWildcards(t *testing.T) { model := `model schema 1.1 - + type user - + type document relations define blocked: [user:*,user] @@ -1711,6 +1879,7 @@ func TestListUsersExclusionWildcards(t *testing.T) { tuple.NewTupleKey("document:1", "blocked", "user:maria"), }, expectedUsers: []string{"user:*"}, + butNot: []string{"user:maria"}, }, { name: "exclusion_and_wildcards_5", @@ -1742,15 +1911,24 @@ func TestListUsersExclusionWildcards(t *testing.T) { }, }, }, - model: model, + model: `model + schema 1.1 + + type user + + type document + relations + define blocked: [user:*,user] + define viewer: [user:*,user] but not blocked`, tuples: []*openfgav1.TupleKey{ - tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "viewer", "user:*"), // base wildcard tuple.NewTupleKey("document:1", "viewer", "user:maria"), tuple.NewTupleKey("document:1", "viewer", "user:jon"), tuple.NewTupleKey("document:1", "blocked", "user:jon"), tuple.NewTupleKey("document:1", "blocked", "user:will"), }, expectedUsers: []string{"user:*", "user:maria"}, + butNot: []string{"user:jon", "user:will"}, }, } tests.runListUsersTestCases(t) @@ -1919,7 +2097,7 @@ func TestListUsersEdgePruning(t *testing.T) { type user type document - relations + relations define viewer: [user]`, tuples: []*openfgav1.TupleKey{ tuple.NewTupleKey("document:1", "viewer", "user:maria"), @@ -2279,13 +2457,13 @@ func TestListUsersWildcardsAndIntersection(t *testing.T) { }, model: `model schema 1.1 - + type user - + type group relations define member: [user:*, user] - + type document relations define group: [group] @@ -2321,7 +2499,7 @@ func TestListUsersCycleDetection(t *testing.T) { l := NewListUsersQuery(mockDatastore, WithResolveNodeLimit(maximumRecursiveDepth)) channelDone := make(chan struct{}) - channelWithResults := make(chan *openfgav1.User) + channelWithResults := make(chan foundUser) channelWithError := make(chan error, 1) model := testutils.MustTransformDSLToProtoWithID(` model @@ -2378,6 +2556,180 @@ func TestListUsersCycleDetection(t *testing.T) { }) } +func TestListUsersChainedNegation(t *testing.T) { + t.Cleanup(func() { + goleak.VerifyNone(t) + }) + + model := `model + schema 1.1 + type user + type document + relations + define viewer: [user, user:*] but not blocked + define blocked: [user, user:*] but not unblocked + define unblocked: [user, user:*]` + + tests := ListUsersTests{ + { + name: "chained_negation_1", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + }, + expectedUsers: []string{"user:jon"}, + }, + { + name: "chained_negation_2", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + tuple.NewTupleKey("document:1", "unblocked", "user:jon"), + }, + expectedUsers: []string{"user:jon"}, + }, + { + name: "chained_negation_3", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + tuple.NewTupleKey("document:1", "unblocked", "user:*"), + tuple.NewTupleKey("document:1", "unblocked", "user:maria"), + }, + expectedUsers: []string{"user:jon"}, + }, + { + name: "chained_negation_4", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "viewer", "user:maria"), + tuple.NewTupleKey("document:1", "unblocked", "user:jon"), + }, + expectedUsers: []string{"user:*", "user:maria"}, + }, + { + name: "chained_negation_5", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "blocked", "user:*"), + tuple.NewTupleKey("document:1", "unblocked", "user:*"), + + tuple.NewTupleKey("document:1", "blocked", "user:jon"), + + tuple.NewTupleKey("document:1", "blocked", "user:maria"), + tuple.NewTupleKey("document:1", "unblocked", "user:maria"), + }, + expectedUsers: []string{"user:*", "user:maria", "user:jon"}, + }, + { + name: "chained_negation_6", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + tuple.NewTupleKey("document:1", "blocked", "user:jon"), + tuple.NewTupleKey("document:1", "unblocked", "user:jon"), + }, + expectedUsers: []string{"user:jon"}, + }, + { + name: "chained_negation_7", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + tuple.NewTupleKey("document:1", "blocked", "user:jon"), + tuple.NewTupleKey("document:1", "unblocked", "user:*"), + }, + expectedUsers: []string{"user:jon"}, + }, + { + name: "chained_negation_8", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + tuple.NewTupleKey("document:1", "blocked", "user:*"), + tuple.NewTupleKey("document:1", "unblocked", "user:*"), + tuple.NewTupleKey("document:1", "unblocked", "user:maria"), + }, + expectedUsers: []string{"user:jon"}, + }, + { + name: "chained_negation_9", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "blocked", "user:*"), + tuple.NewTupleKey("document:1", "unblocked", "user:jon"), + }, + expectedUsers: []string{"user:jon"}, + }, + { + name: "chained_negation_10", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "blocked", "user:jon"), + tuple.NewTupleKey("document:1", "blocked", "user:maria"), + tuple.NewTupleKey("document:1", "unblocked", "user:jon"), + tuple.NewTupleKey("document:1", "unblocked", "user:poovam"), + }, + expectedUsers: []string{"user:*", "user:jon"}, + butNot: []string{"user:maria"}, + }, + { + name: "chained_negation_11", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + tuple.NewTupleKey("document:1", "blocked", "user:*"), + tuple.NewTupleKey("document:1", "unblocked", "user:jon"), + }, + expectedUsers: []string{"user:jon"}, + }, + { + name: "chained_negation_12", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + tuple.NewTupleKey("document:1", "blocked", "user:jon"), + tuple.NewTupleKey("document:1", "unblocked", "user:*"), + }, + expectedUsers: []string{"user:*", "user:jon"}, + }, + { + name: "chained_negation_13", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "viewer", "user:jon"), + tuple.NewTupleKey("document:1", "viewer", "user:maria"), + tuple.NewTupleKey("document:1", "blocked", "user:maria"), + tuple.NewTupleKey("document:1", "unblocked", "user:*"), + }, + expectedUsers: []string{"user:*", "user:jon", "user:maria"}, + }, + { + name: "chained_negation_14", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "blocked", "user:*"), + tuple.NewTupleKey("document:1", "blocked", "user:maria"), + tuple.NewTupleKey("document:1", "unblocked", "user:*"), + }, + expectedUsers: []string{"user:*", "user:maria"}, + }, + { + name: "chained_negation_15", + tuples: []*openfgav1.TupleKey{ + tuple.NewTupleKey("document:1", "viewer", "user:*"), + tuple.NewTupleKey("document:1", "blocked", "user:maria"), + tuple.NewTupleKey("document:1", "unblocked", "user:*"), + }, + expectedUsers: []string{"user:*", "user:maria"}, + }, + } + + for i := range tests { + tests[i].model = model + tests[i].req = &openfgav1.ListUsersRequest{ + Object: &openfgav1.Object{Type: "document", Id: "1"}, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{{Type: "user"}}, + } + } + + tests.runListUsersTestCases(t) +} + func TestListUsersDepthExceeded(t *testing.T) { t.Cleanup(func() { goleak.VerifyNone(t) @@ -2507,7 +2859,7 @@ func TestListUsersStorageErrors(t *testing.T) { model schema 1.1 type user - + type document relations define a: [user] @@ -2563,13 +2915,18 @@ func (testCases ListUsersTests) runListUsersTestCases(t *testing.T) { require.Equal(t, test.expectedErrorMsg, actualErrorMsg) actualUsers := resp.GetUsers() - actualCompare := make([]string, len(actualUsers)) - for i, u := range resp.GetUsers() { + for i, u := range actualUsers { actualCompare[i] = tuple.UserProtoToString(u) } - require.ElementsMatch(t, actualCompare, test.expectedUsers) + + exceptUsers := resp.GetExcludedUsers() + actualCompare = make([]string, len(exceptUsers)) + for i, u := range exceptUsers { + actualCompare[i] = tuple.FromObjectOrUsersetProto(u) + } + require.ElementsMatch(t, actualCompare, test.butNot) }) } } @@ -2705,15 +3062,15 @@ func TestListUsersDatastoreQueryCount(t *testing.T) { model := parser.MustTransformDSLToProto(`model schema 1.1 type user - + type company relations define member: [user] - + type org relations define member: [user] - + type document relations define wildcard: [user:*] @@ -3108,7 +3465,6 @@ func TestListUsersConfig_Deadline(t *testing.T) { allResults: []*openfgav1.User{ {User: &openfgav1.User_Object{Object: &openfgav1.Object{Type: "user", Id: "1"}}}, }, - expectError: "context deadline exceeded", }, `deadline_very_high_returns_everything`: { inputModel: ` @@ -3281,3 +3637,108 @@ func TestListUsersConfig_MaxConcurrency(t *testing.T) { }) } } + +func TestListUsers_ExpandExclusionHandler(t *testing.T) { + t.Cleanup(func() { + goleak.VerifyNone(t) + }) + + ds := memory.New() + t.Cleanup(ds.Close) + + t.Run("avoid_producing_explicitly_negated_subjects", func(t *testing.T) { + modelStr := ` + model + schema 1.1 + + type user + + type document + relations + define restricted: [user, user:*] + define viewer: [user, user:*] but not restricted + ` + storeID, model := storagetest.BootstrapFGAStore(t, ds, modelStr, []string{ + "document:1#viewer@user:*", + "document:1#viewer@user:jon", + "document:1#restricted@user:jon", + }) + + l := NewListUsersQuery(ds, WithResolveNodeLimit(maximumRecursiveDepth)) + channelDone := make(chan struct{}) + channelWithResults := make(chan foundUser) + channelWithError := make(chan error, 1) + + typesys := typesystem.New(model) + ctx := typesystem.ContextWithTypesystem(context.Background(), typesys) + + relation, err := typesys.GetRelation("document", "viewer") + require.NoError(t, err) + + rewrite := relation.GetRewrite().GetUserset().(*openfgav1.Userset_Difference) + + go func() { + resp := l.expandExclusion(ctx, &internalListUsersRequest{ + ListUsersRequest: &openfgav1.ListUsersRequest{ + StoreId: storeID, + AuthorizationModelId: model.GetId(), + Object: &openfgav1.Object{ + Type: "document", + Id: "1", + }, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{{ + Type: "user", + }}, + }, + visitedUsersetsMap: map[string]struct{}{}, + datastoreQueryCount: new(atomic.Uint32), + }, rewrite, channelWithResults) + if resp.err != nil { + channelWithError <- resp.err + return + } + + channelDone <- struct{}{} + }() + + var actualResults []struct { + user string + relationshipStatus userRelationshipStatus + } + + OUTER: + for { + select { + case <-channelWithError: + require.FailNow(t, "expected 0 errors") + case result := <-channelWithResults: + actualResults = append(actualResults, struct { + user string + relationshipStatus userRelationshipStatus + }{user: tuple.UserProtoToString(result.user), relationshipStatus: result.relationshipStatus}) + case <-channelDone: + break OUTER + } + } + + require.Len(t, actualResults, 3) + require.ElementsMatch(t, []struct { + user string + relationshipStatus userRelationshipStatus + }{ + { + user: "user:*", + relationshipStatus: HasRelationship, + }, + { + user: "user:jon", + relationshipStatus: NoRelationship, + }, + { + user: "user:jon", + relationshipStatus: NoRelationship, + }, + }, actualResults) + }) +} diff --git a/pkg/server/list_users.go b/pkg/server/list_users.go index 6c7d8b02ce..eb583e1d0a 100644 --- a/pkg/server/list_users.go +++ b/pkg/server/list_users.go @@ -92,7 +92,8 @@ func (s *Server) ListUsers( ).Observe(datastoreQueryCount) return &openfgav1.ListUsersResponse{ - Users: resp.GetUsers(), + Users: resp.GetUsers(), + ExcludedUsers: resp.GetExcludedUsers(), }, nil } diff --git a/pkg/server/list_users_test.go b/pkg/server/list_users_test.go index 042500a889..fb31c4a39e 100644 --- a/pkg/server/list_users_test.go +++ b/pkg/server/list_users_test.go @@ -181,7 +181,15 @@ func TestModelIdNotFound(t *testing.T) { ctx := context.Background() req := &openfgav1.ListUsersRequest{ - StoreId: "some-store-id", + StoreId: ulid.Make().String(), + Object: &openfgav1.Object{ + Type: "document", + Id: "1", + }, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{ + {Type: "user"}, + }, } mockController := gomock.NewController(t) @@ -208,13 +216,23 @@ func TestModelIdNotFound(t *testing.T) { func TestExperimentalListUsers(t *testing.T) { ctx := context.Background() - req := &openfgav1.ListUsersRequest{} + storeID := ulid.Make().String() + req := &openfgav1.ListUsersRequest{ + StoreId: storeID, + Object: &openfgav1.Object{ + Type: "document", + Id: "1", + }, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{ + {Type: "user"}, + }, + } mockController := gomock.NewController(t) defer mockController.Finish() mockDatastore := mockstorage.NewMockOpenFGADatastore(mockController) - mockDatastore.EXPECT().FindLatestAuthorizationModel(gomock.Any(), gomock.Any()).Return(nil, storage.ErrNotFound) // error demonstrates that main code path is reached server := MustNewServerWithOpts( WithDatastore(mockDatastore), @@ -231,12 +249,39 @@ func TestExperimentalListUsers(t *testing.T) { require.Equal(t, codes.Unimplemented, e.Code()) }) - t.Run("list_users_does_not_error_if_experimentally_enabled", func(t *testing.T) { + t.Run("list_users_returns_error_if_latest_model_not_found", func(t *testing.T) { + mockDatastore.EXPECT().FindLatestAuthorizationModel(gomock.Any(), gomock.Any()).Return(nil, storage.ErrNotFound) // error demonstrates that main code path is reached + server.experimentals = []ExperimentalFeatureFlag{ExperimentalEnableListUsers} _, err := server.ListUsers(ctx, req) - require.Error(t, err) - require.Equal(t, "rpc error: code = Code(2020) desc = No authorization models found for store ''", err.Error()) + st, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, codes.Code(openfgav1.ErrorCode_latest_authorization_model_not_found), st.Code()) + }) + + t.Run("list_users_returns_error_if_model_not_found", func(t *testing.T) { + mockDatastore.EXPECT(). + ReadAuthorizationModel(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, storage.ErrNotFound) + + server.experimentals = []ExperimentalFeatureFlag{ExperimentalEnableListUsers} + _, err := server.ListUsers(ctx, &openfgav1.ListUsersRequest{ + StoreId: storeID, + AuthorizationModelId: ulid.Make().String(), + Object: &openfgav1.Object{ + Type: "document", + Id: "1", + }, + Relation: "viewer", + UserFilters: []*openfgav1.UserTypeFilter{ + {Type: "user"}, + }, + }) + + st, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, codes.Code(openfgav1.ErrorCode_authorization_model_not_found), st.Code()) }) } diff --git a/pkg/tuple/tuple.go b/pkg/tuple/tuple.go index 9a92cbdf68..37a763fa39 100644 --- a/pkg/tuple/tuple.go +++ b/pkg/tuple/tuple.go @@ -173,6 +173,47 @@ func UserProtoToString(obj *openfgav1.User) UserString { } } +// FromObjectOrUsersetProto returns a string from a ObjectOrUserset proto. Ex: 'user:maria' or 'group:fga#member'. It is +// the opposite from StringToObjectOrUserset function. +func FromObjectOrUsersetProto(obj *openfgav1.ObjectOrUserset) UserString { + switch user := obj.GetUser().(type) { + case *openfgav1.ObjectOrUserset_Object: + return fmt.Sprintf("%s:%s", user.Object.GetType(), user.Object.GetId()) + case *openfgav1.ObjectOrUserset_Userset: + return fmt.Sprintf("%s:%s#%s", user.Userset.GetType(), user.Userset.GetId(), user.Userset.GetRelation()) + default: + panic("unsupported type") + } +} + +// StringToObjectOrUserset returns a ObjectOrUserset proto from a string. Ex: 'group:fga#member'. +// It is the opposite from FromObjectOrUsersetProto function. +func StringToObjectOrUserset(userKey UserString) *openfgav1.ObjectOrUserset { + userObj, userRel := SplitObjectRelation(userKey) + userObjType, userObjID := SplitObject(userObj) + + if userRel == "" { + return &openfgav1.ObjectOrUserset{ + User: &openfgav1.ObjectOrUserset_Object{ + Object: &openfgav1.Object{ + Type: userObjType, + Id: userObjID, + }, + }, + } + } + + return &openfgav1.ObjectOrUserset{ + User: &openfgav1.ObjectOrUserset_Userset{ + Userset: &openfgav1.UsersetUser{ + Type: userObjType, + Id: userObjID, + Relation: userRel, + }, + }, + } +} + // StringToUserProto returns a User proto from a string. Ex: 'user:maria#member'. // It is the opposite from FromUserProto function. func StringToUserProto(userKey UserString) *openfgav1.User { diff --git a/pkg/tuple/tuple_test.go b/pkg/tuple/tuple_test.go index 95373e52c2..f6fe0b636a 100644 --- a/pkg/tuple/tuple_test.go +++ b/pkg/tuple/tuple_test.go @@ -545,3 +545,81 @@ func TestParseTupleString(t *testing.T) { }) } } + +func TestFromObjectOrUsersetProto(t *testing.T) { + for _, tc := range []struct { + name string + input *openfgav1.ObjectOrUserset + expected string + }{ + { + name: "user_with_id", + input: &openfgav1.ObjectOrUserset{ + User: &openfgav1.ObjectOrUserset_Object{ + Object: &openfgav1.Object{ + Type: "user", + Id: "id-123", + }, + }, + }, + expected: "user:id-123", + }, + { + name: "user_with_id_and_relation", + input: &openfgav1.ObjectOrUserset{ + User: &openfgav1.ObjectOrUserset_Userset{ + Userset: &openfgav1.UsersetUser{ + Type: "user", + Id: "id-123", + Relation: "member", + }, + }, + }, + expected: "user:id-123#member", + }, + } { + t.Run(tc.name, func(t *testing.T) { + actual := FromObjectOrUsersetProto(tc.input) + require.Equal(t, tc.expected, actual) + }) + } +} + +func TestToObjectOrUsersetProto(t *testing.T) { + for _, tc := range []struct { + name string + input string + expected *openfgav1.ObjectOrUserset + }{ + { + name: "user_with_id", + input: "user:id-123", + expected: &openfgav1.ObjectOrUserset{ + User: &openfgav1.ObjectOrUserset_Object{ + Object: &openfgav1.Object{ + Type: "user", + Id: "id-123", + }, + }, + }, + }, + { + name: "user_with_id_and_relation", + input: "user:id-123#member", + expected: &openfgav1.ObjectOrUserset{ + User: &openfgav1.ObjectOrUserset_Userset{ + Userset: &openfgav1.UsersetUser{ + Type: "user", + Id: "id-123", + Relation: "member", + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + actual := StringToObjectOrUserset(tc.input) + require.Equal(t, tc.expected, actual) + }) + } +} diff --git a/tests/listusers/listusers.go b/tests/listusers/listusers.go index d9c543c9cf..8b60f8f036 100644 --- a/tests/listusers/listusers.go +++ b/tests/listusers/listusers.go @@ -170,7 +170,9 @@ func runTest(t *testing.T, test individualTest, client ClientInterface, contextT if assertion.ErrorCode == 0 { require.NoError(t, err, detailedInfo) - require.ElementsMatch(t, assertion.Expectation, listuserstest.FromProtoResponse(resp), detailedInfo) + require.ElementsMatch(t, assertion.Expectation, listuserstest.FromUsersProto(resp.GetUsers()), detailedInfo) + + require.ElementsMatch(t, assertion.ExpectedExcludedUsers, listuserstest.FromObjectOrUsersetProto(resp.GetExcludedUsers()), detailedInfo) // assert 2: each user in the response of ListUsers should return check -> true for _, user := range resp.GetUsers() {